Harden unsafe controller render defaults

[jamesfredley]

What was found

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.
• render(file: ...) responses could be displayed inline unless callers explicitly supplied a safer Content-Disposition.
• Generated Content-Disposition filenames did not escape unsafe quoted-string characters consistently.

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

What was fixed

• Default inspect-style object and unrecognized map rendering to text/plain.
• 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-out behavior in the controller render reference.

Verification

• ./gradlew.bat --no-daemon :grails-controllers:test --tests "grails.artefact.controller.support.ResponseRendererSpec"
• ./gradlew.bat --no-daemon :grails-test-suite-uber:test --tests "org.grails.web.servlet.RenderMethodTests"
• ./gradlew.bat --no-daemon :grails-doc:publishGuide -x aggregateGroovydoc
• ./gradlew.bat --no-daemon :grails-controllers:test :grails-test-suite-uber:test
• ./gradlew.bat --no-daemon clean aggregateViolations :grails-test-report:check --continue
• Combined report: 13,634 tests, 0 failures, 0 errors.
• Review gate: Codex GREEN and Oracle GREEN for diff 082eaf0baaba9aad69bc8b48f7db19d44bc9ba50.

[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.4712%. Comparing base (a326766) to head (7f1d7a9).
⚠️ Report is 23 commits behind head on 8.0.x.

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #15801        +/-   ##
==================================================
+ Coverage     49.4560%   49.4712%   +0.0151%     
- Complexity      16640      16684        +44     
==================================================
  Files            1944       1947         +3     
  Lines           92379      92468        +89     
  Branches        16130      16150        +20     
==================================================
+ Hits            45687      45745        +58     
- Misses          39593      39618        +25     
- Partials         7099       7105         +6     

see 16 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.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 7f1d7a9
▶️ Tests: 50095 executed
⚪️ Checks: 47/47 completed

---

Learn more about TestLens at testlens.app.

[jdaugherty]

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

[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.