Harden unsafe controller render defaults

[jamesfredley]

The Problem

Several controller render paths used browser-interpreted defaults for output that is not guaranteed to be safe HTML:

• render(Object) and unrecognized render(Map) values could produce inspect-style output without forcing a plain-text response type, so arbitrary object/map output could be served and interpreted as HTML.
• render(file: ...) responses could be displayed inline unless the caller explicitly supplied a safer Content-Disposition.
• Generated Content-Disposition filenames did not escape unsafe quoted-string characters consistently.

That made the defaults too permissive for apps that render arbitrary objects, maps, or downloaded files without explicitly setting response headers.

The Fix

• Default inspect-style object and unrecognized map rendering to text/plain so unstructured values are not served as HTML.
• Default file rendering to Content-Disposition: attachment.
• Preserve explicit caller-provided disposition headers, and add an inline: true opt-out for intentional inline file rendering.
• Escape unsafe filename characters in generated attachment headers.
• Document the new defaults and opt-outs in the controller render reference and the Grails 8 upgrade guide.

Opt-outs (unchanged behavior available)

• inline: true for intentional inline file rendering.
• An explicit contentType: on the render call.
• A pre-set response.contentType.

Files

• ResponseRenderer.groovy - text/plain inspect default, attachment file default, filename escaping, opt-outs.
• ResponseRendererSpec.groovy, RenderMethodTests.groovy - cover the new defaults and assert the controller-level response.contentType.
• render.adoc, upgrading80x.adoc (section "Render Defaults Harden Unsafe Content") - document the change and opt-outs.

Testing

• ResponseRendererSpec, RenderMethodTests, and :grails-doc:publishGuide pass. Full combined run: 13,634 tests, 0 failures.

Open discussion

Maintainer feedback agrees with the filename/disposition hardening and asks for more discussion on the default content-type change. A narrower variant that dropped only the text/plain inspect default was tried but reintroduces the browser-interpreted inspect-output risk, so it was not committed. If maintainers decide to split it, the file-disposition and filename-escaping changes can be separated into a smaller PR; the two content-type threads remain intentionally open for that discussion.

[Copilot]

Pull request overview

This pull request hardens Grails controller render(...) defaults to reduce unsafe browser-rendered responses by defaulting inspect-style renders to text/plain and making file responses download as attachments unless explicitly requested inline.

Changes:

• Default render(Object) and unrecognized render(Map) output to text/plain.
• Default file renders to Content-Disposition: attachment, with inline: true opt-out and preservation of explicitly set Content-Disposition.
• Escape unsafe characters in generated Content-Disposition filenames and document the updated defaults.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy	Implements hardened defaults (text/plain for inspect renders; attachment-by-default for files; filename escaping).
grails-controllers/src/test/groovy/grails/artefact/controller/support/ResponseRendererSpec.groovy	Adds focused unit coverage for the new render defaults and header escaping behavior.
grails-test-suite-uber/src/test/groovy/org/grails/web/servlet/RenderMethodTests.groovy	Updates/extends integration tests to validate attachment-by-default and inline opt-out for stream renders.
grails-doc/src/en/ref/Controllers/render.adoc	Documents the new default behaviors and the inline option for file rendering.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.4853%. Comparing base (a326766) to head (4fb4967).
⚠️ Report is 40 commits behind head on 8.0.x.

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #15801        +/-   ##
==================================================
+ Coverage     49.4560%   49.4853%   +0.0292%     
- Complexity      16640      16699        +59     
==================================================
  Files            1944       1947         +3     
  Lines           92379      92474        +95     
  Branches        16130      16152        +22     
==================================================
+ Hits            45687      45761        +74     
- Misses          39593      39606        +13     
- Partials         7099       7107         +8     

see 18 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jdaugherty]

If the content type is unknown, why are we rendering something else?

[jamesfredley]

I looked at narrowing the PR to only the filename/disposition fixes and leaving inspect-style render output on the existing default, but review-gate flagged that as reintroducing the exact browser-interpreted inspect-output risk this branch is trying to avoid. The current implementation still renders the same inspect string; it only changes the default response media type for the inspect fallback from browser-interpreted HTML to text/plain unless the caller explicitly sets a content type. I am leaving this thread unresolved for maintainer discussion rather than treating it as resolved.

[bito-code-review]

The change ensures that when the content type is unknown, the system defaults to 'text/plain' instead of 'text/html'. This is achieved by updating the applyContentType method to accept a defaultContentType parameter, which is then passed to resolveContentTypeBySourceType when the content type cannot be determined.

grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy

private boolean applyContentType(HttpServletResponse response, Map argMap, Object renderArgument, boolean useDefault, String defaultContentType) {
        boolean contentTypeIsDefault = true
        String contentType = resolveContentTypeBySourceType(renderArgument, useDefault ? defaultContentType : null)

[jdaugherty]

I think to change these defaults we need to adhere to our PR summary policy and discuss the default change more thoroughly after a case has been presented.

[jdaugherty]

I'm not sure we should use a default content type. Especially plain text when encoders could cause non-text values to render.

[jdaugherty]

I 100% agree on the filename fixes. The default content type, I'd like to discuss more.

[jamesfredley]

Thanks. I updated the PR body to follow the summary policy more explicitly: it now separates "What was found", "What was fixed", and "Verification".

I agree the default behavior change needs maintainer discussion. The intent here is not to render a different semantic object, but to avoid browser HTML interpretation for inspect-style or otherwise unrecognized render output by forcing text/plain, and to make file renders download by default unless inline: true is explicit. I am leaving the default-change discussion open rather than marking it resolved.

[jamesfredley]

Pre-release review pass (2026-07-02, dual-reviewer: GPT-5.5 oracle + Codex CLI): NEEDS-IMPROVEMENT, now addressed.

Findings: the behavior-breaking render defaults were documented only in the render reference, not the upgrade guide (med), and the controller-level content-type contract was not asserted in RenderMethodTests (low). Both fixed in 4fb4967c08 - see the "Review follow-up" section in the description. One reviewer also caught an invalid render(myObject, contentType: ...) example in the first draft of the docs; the shipped example uses the valid render(text: ..., contentType: ...) form.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 4fb4967
▶️ Tests: 40774 executed
⚪️ Checks: 46/46 completed

---

Learn more about TestLens at testlens.app.