Skip to content

Include the charset when DefaultHtmlRenderer sets the content type#15976

Open
codeconsole wants to merge 3 commits into
apache:8.0.xfrom
codeconsole:fix/html-renderer-charset
Open

Include the charset when DefaultHtmlRenderer sets the content type#15976
codeconsole wants to merge 3 commits into
apache:8.0.xfrom
codeconsole:fix/html-renderer-charset

Conversation

@codeconsole

Copy link
Copy Markdown
Contributor

respond() rendering HTML sets the response content type to the bare mime type name (text/html) — DefaultHtmlRenderer is the only renderer that skips GrailsWebUtil.getContentType; its JSON and XML siblings both append the charset.

Effect

A bare text/html leaves the response charset to the servlet container (ISO-8859-1 on Tomcat). GSP only applies its configured text/html;charset=UTF-8 when the response has no content type yet, so it backs off — the response writer then encodes Latin-1 and every non-Latin-1 character on the page is written as a literal ? (Czech, Cyrillic, Thai, CJK; Latin-1-mappable accents survive, which makes the corruption easy to miss on Western-European pages).

Why it went unnoticed

  • It only triggers when the Accept header negotiates a concrete HTML mime type. Browsers always send one; curl and most HTTP test clients send Accept: */*, which negotiates MimeType.ALL, skips the content-type stamp, and gets a correct UTF-8 page.
  • The SiteMesh 2 filter historically masked it by unconditionally forcing UTF-8 onto decorated responses. The SiteMesh 3 view-resolver integration (Upgrade SiteMesh to 3.3.0-M3 #15975) deliberately respects the content type the application sets — exposing this latent bug on every scaffolded (respond-rendered) page.

Fix

Mirror DefaultJsonRenderer: an encoding property defaulting to GrailsWebUtil.DEFAULT_ENCODING (UTF-8), applied via GrailsWebUtil.getContentType(mimeType.name, encoding).

Verification

  • New spec pins text/html;charset=UTF-8 on the negotiated render path (fails against the old code); :grails-rest-transforms:test and :grails-controllers:test green.
  • Live: a generated application's scaffolded page requested with a real browser Accept header serves all 18 locales charset=UTF-8, strict byte-validated (slovenčina, čeština, русский, 中文 intact); in Chrome, document.characterSet is now UTF-8 and the locale dropdown renders every native name correctly.

Related: #15974, #15975.

respond() rendering HTML set the response content type to the bare mime
type name ("text/html"), unlike the JSON and XML renderers, which run it
through GrailsWebUtil.getContentType to append the charset. A bare
content type leaves the response charset to the servlet container
(ISO-8859-1 on Tomcat), and GSP — which only applies its configured
UTF-8 content type when none is set yet — backs off, so the response
writer encodes Latin-1 and every non-Latin-1 character on the page is
written as a literal question mark.

The bug only bites requests whose Accept header negotiates to a concrete
HTML mime type: browsers always send one, while header-less clients
(curl and most HTTP test clients) negotiate MimeType.ALL, skip the
content-type stamp entirely, and get the correct UTF-8 page — which is
why it survived unnoticed. Historically the SiteMesh 2 filter also
masked it by unconditionally forcing UTF-8 onto decorated responses;
the SiteMesh 3 view-resolver integration respects the content type the
application sets, exposing the latent bug on every scaffolded page.

Mirror the JSON renderer: an encoding property defaulting to UTF-8,
applied through GrailsWebUtil.getContentType.
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.6709%. Comparing base (08bd0bf) to head (937f5b3).
⚠️ Report is 12 commits behind head on 8.0.x.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##                8.0.x     #15976   +/-   ##
=============================================
  Coverage     49.6709%   49.6709%           
- Complexity      17017      17019    +2     
=============================================
  Files            2004       2004           
  Lines           93896      93896           
  Branches        16448      16448           
=============================================
  Hits            46639      46639           
+ Misses          40078      40077    -1     
- Partials         7179       7180    +1     
Files with missing lines Coverage Δ
...ns/web/rest/render/html/DefaultHtmlRenderer.groovy 46.4286% <100.0000%> (ø)

... and 3 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.


String suffix = ''

String encoding = GrailsWebUtil.DEFAULT_ENCODING

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be configurable and then this assigned to default if not set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — done in 814b384. The encoding is now wired from configuration: DefaultRendererRegistry gained an encoding property that it propagates to every default renderer it creates (HTML, JSON, XML, and the Errors container renderers, plus the fallback HTML renderers the JSON/XML renderers construct when no view matches), and RestResponderGrailsPlugin populates it from the existing grails.converters.encoding setting, falling back to UTF-8 when unset. I reused that key rather than introducing a new one since these renderers are part of the same respond() pipeline the converters serve — one knob for response encoding. The setting is also documented in additional-spring-configuration-metadata.json so IDEs offer completion for it. Renderers registered as beans keep the independently settable encoding property. Covered by new specs at renderer level (custom encoding reaches the content type) and registry level (propagation to all default renderers).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we default this to the configured value if set, otherwise use this default thought?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in bb78ee0 — the property is now annotated @Value('${grails.converters.encoding:UTF-8}') so it resolves the configured value when set and falls back to UTF-8 otherwise, on the registry and all the renderer classes. The plugin-side wiring is removed.

@bito-code-review

Copy link
Copy Markdown

Yes, that is a good suggestion. Making the encoding configurable while defaulting to GrailsWebUtil.DEFAULT_ENCODING would provide more flexibility for different environments while maintaining a sensible default. You could update the class to allow setting the encoding via a property or constructor, ensuring it falls back to the default if not explicitly provided.

grails-rest-transforms/src/main/groovy/org/grails/plugins/web/rest/render/html/DefaultHtmlRenderer.groovy

String encoding = GrailsWebUtil.DEFAULT_ENCODING

    DefaultHtmlRenderer(Class<T> targetType, String encoding = GrailsWebUtil.DEFAULT_ENCODING) {
        this.targetType = targetType
        this.encoding = encoding
    }

Wire the existing grails.converters.encoding setting through
DefaultRendererRegistry into every default renderer it creates — HTML,
JSON, XML and the Errors container renderers — and into the fallback
HTML renderers the JSON/XML renderers construct when no view matches,
falling back to UTF-8 when unset. Renderers registered as beans keep
their independently settable encoding property.

Document the setting in the configuration metadata so IDEs offer it.
Replace the plugin-wired encoding property on DefaultRendererRegistry with
@value('${grails.converters.encoding:UTF-8}') so the setting is driven by
the environment for any registry or renderer registered as a Spring bean,
not just the bean definition RestResponderGrailsPlugin happens to create.
The same annotation now covers DefaultHtmlRenderer, DefaultJsonRenderer,
DefaultXmlRenderer, AbstractLinkingRenderer and AbstractVndErrorRenderer,
so custom renderer beans (HAL, Atom, Vnd.Error) pick the setting up
automatically. The field default remains UTF-8 for renderers constructed
outside Spring; the registry still propagates its encoding to the default
renderers it instantiates itself.
@codeconsole

Copy link
Copy Markdown
Contributor Author

Good call — moved the resolution into the classes themselves in bb78ee0. DefaultRendererRegistry and every renderer with an encoding property (DefaultHtmlRenderer/Json/Xml, AbstractLinkingRenderer, AbstractVndErrorRenderer) now carry @Value('${grails.converters.encoding:UTF-8}'), and the explicit wiring in RestResponderGrailsPlugin is gone. That way any renderer registered as a bean — including user-registered HAL/Atom/Vnd.Error renderers — honors the config, not just the registry bean the plugin defines. The field default stays UTF-8 for instances constructed outside Spring, and the registry still pushes its encoding onto the non-bean defaults it creates. Added a spec that boots a context with grails.converters.encoding=ISO-8859-1 and verifies both the registry and its renderers resolve it from the environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants