From 7f1d7a9411d70479bacd74f590b0564238f552ea Mon Sep 17 00:00:00 2001 From: James Fredley Date: Wed, 1 Jul 2026 02:22:19 -0400 Subject: [PATCH 1/2] Harden unsafe controller render defaults Default inspect-style render output to text/plain so unstructured values are not served as HTML. Render file responses as attachments by default, escape unsafe filename characters in Content-Disposition, and document the inline opt-out. Assisted-by: Hephaestus:gpt-5.5 --- .../support/ResponseRenderer.groovy | 31 +++- .../support/ResponseRendererSpec.groovy | 156 ++++++++++++++++++ grails-doc/src/en/ref/Controllers/render.adoc | 11 +- .../web/servlet/RenderMethodTests.groovy | 28 +++- 4 files changed, 217 insertions(+), 9 deletions(-) create mode 100644 grails-controllers/src/test/groovy/grails/artefact/controller/support/ResponseRendererSpec.groovy diff --git a/grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy b/grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy index 8ef0a3576e2..f1d1fef9fb1 100644 --- a/grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy +++ b/grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy @@ -134,7 +134,7 @@ trait ResponseRenderer extends WebAttributes { GrailsWebRequest webRequest = (GrailsWebRequest) RequestContextHolder.currentRequestAttributes() HttpServletResponse response = webRequest.currentResponse webRequest.renderView = false - applyContentType(response, null, object) + applyContentType(response, null, object, true, 'text/plain') try { response.writer.write(object.inspect()) @@ -391,14 +391,12 @@ trait ResponseRenderer extends WebAttributes { if (!hasContentType) { hasContentType = detectContentTypeFromFileName(webRequest, response, argMap, fileName) } - if (fnO) { - response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "$DISPOSITION_HEADER_PREFIX\"$fileName\"") - } } if (!hasContentType) { throw new ControllerExecutionException( 'Argument [file] of render method specified without valid [contentType] argument') } + applyFileDisposition(response, argMap, fileName) InputStream input try { @@ -432,7 +430,7 @@ trait ResponseRenderer extends WebAttributes { response.contentType = GrailsWebUtil.getContentType(MimeType.JSON.name, DEFAULT_ENCODING) renderWritable((JSONElement) argMap, response) } else { - applyContentType(response, argMap, argMap) + applyContentType(response, argMap, argMap, true, 'text/plain') try { response.writer.write(argMap.inspect()) } @@ -520,8 +518,12 @@ trait ResponseRenderer extends WebAttributes { } private boolean applyContentType(HttpServletResponse response, Map argMap, Object renderArgument, boolean useDefault) { + applyContentType(response, argMap, renderArgument, useDefault, TEXT_HTML) + } + + private boolean applyContentType(HttpServletResponse response, Map argMap, Object renderArgument, boolean useDefault, String defaultContentType) { boolean contentTypeIsDefault = true - String contentType = resolveContentTypeBySourceType(renderArgument, useDefault ? TEXT_HTML : null) + String contentType = resolveContentTypeBySourceType(renderArgument, useDefault ? defaultContentType : null) String encoding = DEFAULT_ENCODING if (argMap != null) { if (argMap.containsKey(ARGUMENT_CONTENT_TYPE)) { @@ -540,6 +542,23 @@ trait ResponseRenderer extends WebAttributes { false } + private void applyFileDisposition(HttpServletResponse response, Map argMap, String fileName) { + if (response.getHeader(HttpHeaders.CONTENT_DISPOSITION) != null) { + return + } + if (Boolean.TRUE.equals(argMap.get('inline'))) { + return + } + String disposition = fileName ? "$DISPOSITION_HEADER_PREFIX\"${escapeContentDispositionFilename(fileName)}\"" : 'attachment' + response.setHeader(HttpHeaders.CONTENT_DISPOSITION, disposition) + } + + private String escapeContentDispositionFilename(String fileName) { + fileName.replace('\\', '\\\\') + .replace('"', '\\"') + .replaceAll('[\\x00-\\x1F\\x7F]', '_') + } + private void setContentType(HttpServletResponse response, String contentType, String encoding) { setContentType(response, contentType, encoding, false) } diff --git a/grails-controllers/src/test/groovy/grails/artefact/controller/support/ResponseRendererSpec.groovy b/grails-controllers/src/test/groovy/grails/artefact/controller/support/ResponseRendererSpec.groovy new file mode 100644 index 00000000000..798c4187e01 --- /dev/null +++ b/grails-controllers/src/test/groovy/grails/artefact/controller/support/ResponseRendererSpec.groovy @@ -0,0 +1,156 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.artefact.controller.support + +import grails.util.GrailsWebMockUtil +import org.grails.web.servlet.mvc.GrailsWebRequest +import org.grails.web.servlet.mvc.ParameterCreationListener +import org.springframework.mock.web.MockHttpServletRequest +import org.springframework.mock.web.MockHttpServletResponse +import org.springframework.web.context.WebApplicationContext +import org.springframework.web.context.request.RequestContextHolder +import spock.lang.Specification + +class ResponseRendererSpec extends Specification { + + void cleanup() { + RequestContextHolder.setRequestAttributes(null) + } + + void 'rendering an object uses text plain for inspect output'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + def renderer = new TestResponseRenderer() + + when: + renderer.render(new InspectableResponseValue('')) + + then: + webRequest.response.contentType == 'text/plain;charset=utf-8' + webRequest.response.contentAsString == '' + } + + void 'rendering an unrecognized map uses text plain for inspect output'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + def renderer = new TestResponseRenderer() + + when: + renderer.render([unsafe: '']) + + then: + webRequest.response.contentType == 'text/plain;charset=utf-8' + webRequest.response.contentAsString == "['unsafe':'']" + } + + void 'file renders are attachments by default without a file name'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + def renderer = new TestResponseRenderer() + + when: + renderer.render(file: ''.bytes, contentType: 'image/svg+xml') + + then: + webRequest.response.getHeader('Content-Disposition') == 'attachment' + webRequest.response.contentAsByteArray == ''.bytes + } + + void 'file renders use the resolved file name for attachment disposition'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + File file = File.createTempFile('grails-render-', '.txt') + file.text = 'download body' + def renderer = new TestResponseRenderer() + + when: + renderer.render(file: file, contentType: 'text/plain') + + then: + webRequest.response.getHeader('Content-Disposition') == "attachment;filename=\"${file.name}\"" + webRequest.response.contentAsString == 'download body' + + cleanup: + file.delete() + } + + void 'file renders escape unsafe attachment file names'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + def renderer = new TestResponseRenderer() + + when: + renderer.render(file: 'download body'.bytes, contentType: 'text/plain', fileName: 'a"b\\c\r\n.txt') + + then: + webRequest.response.getHeader('Content-Disposition') == 'attachment;filename="a\\"b\\\\c__.txt"' + webRequest.response.contentAsString == 'download body' + } + + void 'file renders may explicitly opt into inline disposition'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + def renderer = new TestResponseRenderer() + + when: + renderer.render(file: ''.bytes, contentType: 'image/svg+xml', inline: true) + + then: + webRequest.response.getHeader('Content-Disposition') == null + } + + void 'file renders preserve an existing content disposition header'() { + given: + GrailsWebRequest webRequest = bindWebRequest() + webRequest.response.setHeader('Content-Disposition', 'inline') + def renderer = new TestResponseRenderer() + + when: + renderer.render(file: ''.bytes, contentType: 'image/svg+xml') + + then: + webRequest.response.getHeader('Content-Disposition') == 'inline' + } + + private GrailsWebRequest bindWebRequest() { + WebApplicationContext applicationContext = Mock(WebApplicationContext) + applicationContext.getBeansOfType(ParameterCreationListener) >> [:] + GrailsWebMockUtil.bindMockWebRequest( + applicationContext, + new MockHttpServletRequest(), + new MockHttpServletResponse()) + } +} + +class TestResponseRenderer implements ResponseRenderer { +} + +class InspectableResponseValue { + + private final String inspectedValue + + InspectableResponseValue(String inspectedValue) { + this.inspectedValue = inspectedValue + } + + @Override + String toString() { + inspectedValue + } +} diff --git a/grails-doc/src/en/ref/Controllers/render.adoc b/grails-doc/src/en/ref/Controllers/render.adoc index cc2a2347184..d3a101dd173 100644 --- a/grails-doc/src/en/ref/Controllers/render.adoc +++ b/grails-doc/src/en/ref/Controllers/render.adoc @@ -83,8 +83,11 @@ render(contentType: "application/json") { // render with status code render(status: 503, text: "Failed to update book ${b.id}") -// render a file +// render a file as an attachment render(file: new File(absolutePath), fileName: "book.pdf") + +// render a file inline +render(file: new File(absolutePath), inline: true) ---- @@ -92,6 +95,9 @@ render(file: new File(absolutePath), fileName: "book.pdf") A multi-purpose method for rendering responses to the client which is best illustrated with a few examples! Warning - this method does not always support multiple parameters. For example, if you specify both collection and model, the model parameter will be ignored. + +Rendering an arbitrary object, or a map without a recognized render argument, writes the Groovy `inspect()` value as `text/plain` by default. + Parameters Parameters: @@ -109,5 +115,6 @@ Parameters: * `encoding` (optional) - The encoding of the response * `plugin` (optional) - The plugin to look for the template in * `status` (optional) - The HTTP status code to use -* `file` (optional) - The byte[], java.io.File, or inputStream you wish to send with the response +* `file` (optional) - The byte[], java.io.File, or inputStream you wish to send with the response. File responses are rendered with `Content-Disposition: attachment` by default. * `fileName` (optional) - For specifying an attachment file name while rendering a file. +* `inline` (optional) - Set to `true` while rendering a file to omit the default attachment `Content-Disposition` header. diff --git a/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy b/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy index bd1bc8478ee..d7e27d4a9ba 100644 --- a/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy +++ b/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy @@ -51,13 +51,26 @@ class RenderMethodTests extends Specification implements ControllerUnitTest Date: Thu, 2 Jul 2026 00:49:09 -0400 Subject: [PATCH 2/2] Document hardened render defaults and assert content types Add a Grails 8 upgrade guide section covering the text/plain inspect default for object and fallback map rendering, the attachment Content-Disposition default with filename escaping for file renders, and the inline and explicit content type opt-outs. Assert the text/plain content type in the controller-level render tests. Assisted-by: opencode:gpt-5.5 --- .../src/en/guide/upgrading/upgrading80x.adoc | 20 +++++++++++++++++++ .../web/servlet/RenderMethodTests.groovy | 2 ++ 2 files changed, 22 insertions(+) diff --git a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc index 5eae1ae3fb5..aade8db321d 100644 --- a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc +++ b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc @@ -1304,3 +1304,23 @@ grails: - Presto - Trident ---- + +==== 30. Render Defaults Harden Unsafe Content + +Grails 8 hardens controller `render(...)` defaults for responses that could otherwise be interpreted as browser-rendered HTML. +Rendering an arbitrary object with `render(object)`, or rendering a map that does not contain a recognized render argument, now writes the Groovy `inspect()` value as `text/plain` by default instead of `text/html`. + +File renders now set `Content-Disposition: attachment` by default. +If a `fileName` is supplied, unsafe filename characters are escaped before the value is written to the `Content-Disposition` header. + +Set an explicit `contentType` when a response should use another media type, and set `inline: true` when a file response should not receive the default attachment header. +The hardened defaults only apply when no content type has been set, so assigning `response.contentType` before calling `render(object)` also overrides them. +If you need a custom disposition, set the `Content-Disposition` response header explicitly before rendering the file; Grails preserves an existing header. + +For example: + +[source,groovy] +---- +render(text: myObject.inspect(), contentType: 'text/html') +render(file: new File(absolutePath), inline: true) +---- diff --git a/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy b/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy index d7e27d4a9ba..4a617580816 100644 --- a/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy +++ b/grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy @@ -125,6 +125,7 @@ class RenderMethodTests extends Specification implements ControllerUnitTest