codeconsole commented on code in PR #16768:
URL: https://github.com/apache/grails-core/pull/16768#discussion_r4381718079
##########
grails-doc/src/en/guide/upgrading/upgrading81x.adoc:
##########
@@ -72,3 +72,19 @@ grailsApplication.config.refresh()
`refresh()` is a method of the configuration implementation rather than of the
`Config` interface, so it is a call for dynamically compiled test code and may
change with the implementation. It reads the environment again and merges the
property sources again, which replaces a value the test set on the
configuration directly wherever a property source defines the same key. Call it
again once the variables are removed: the configuration keeps the environment
it last read.
Two smaller differences: variable names are upper-cased with `Locale.ROOT`, as
Spring does, so a key containing `i` matches its upper-cased variable under a
Turkish default locale too; and `getProperty(null)` returns the default value
instead of throwing a `NullPointerException`.
+
+==== 5. The Character Encoding Filter Runs First
+
+Grails 8.1 registers its character encoding filter at
`Ordered.HIGHEST_PRECEDENCE`, the order Spring Boot gives its own, instead of
at -180. Once a filter reads a request parameter, the servlet container has
decoded the request body, as ISO-8859-1 unless an encoding was set by then, so
a filter ordered ahead of the encoding filter that read a parameter garbled
non-ASCII form input for the rest of the request, for example `Café` for
`Café`.
+
+Every filter ordered between `Ordered.HIGHEST_PRECEDENCE + 1` and -181 now
runs after the encoding filter instead of before it, and sees the request
encoding set. That includes asset-pipeline's filter at
`GrailsFilters.ASSET_PIPELINE_FILTER` (-190), a filter at
`GrailsFilters.FIRST`, and filters registered at `Ordered.HIGHEST_PRECEDENCE +
n`:
+
+* In a packaged application (a WAR or an executable jar), asset-pipeline
copies the request's encoding to the precompiled assets it serves, so every
asset response now declares a charset, for example `text/css;charset=UTF-8`,
whether or not `grails.filter.forceEncoding` is set. Browsers use it for
stylesheets and scripts instead of detecting their encoding. Assets served by
asset-pipeline's development runtime still have no charset.
+* A filter that reads a request body sent without a charset through
`getReader()` now decodes it as UTF-8, or as `grails.filter.encoding` says,
instead of as ISO-8859-1.
+* With `grails.filter.forceEncoding: true`, a response such a filter writes
without a charset is now encoded with the forced encoding instead of
ISO-8859-1, and a request encoding such a filter sets is no longer replaced by
the forced one.
+
+A filter registered at exactly `Ordered.HIGHEST_PRECEDENCE` through a
`FilterRegistrationBean` still runs before the encoding filter.
+
+===== Required Actions
+
+For most applications **no changes are required**. Check that packaged
stylesheets and scripts with non-ASCII characters are in the charset their
responses now declare: UTF-8, or the encoding `grails.filter.encoding` names. A
filter that has to run before the encoding filter can be registered at
`Ordered.HIGHEST_PRECEDENCE`; a filter that reads request parameters needs a
later order than that.
Review Comment:
Done in 9715d0239d: the required actions now say "through a
`FilterRegistrationBean`". app1 registers a `FilterRegistrationBean` at
`HIGHEST_PRECEDENCE` on `/encoding/*` that records `request.characterEncoding`,
and `CharacterEncodingSpec` checks that it finds none for the form it posts
without a charset. Moved to `HIGHEST_PRECEDENCE + 1`, the same filter finds the
encoding set and the case fails. I left `GrailsCorsFilter` at
`HIGHEST_PRECEDENCE`: as you say, it reads no parameters.
##########
grails-doc/src/en/guide/upgrading/upgrading81x.adoc:
##########
@@ -72,3 +72,19 @@ grailsApplication.config.refresh()
`refresh()` is a method of the configuration implementation rather than of the
`Config` interface, so it is a call for dynamically compiled test code and may
change with the implementation. It reads the environment again and merges the
property sources again, which replaces a value the test set on the
configuration directly wherever a property source defines the same key. Call it
again once the variables are removed: the configuration keeps the environment
it last read.
Two smaller differences: variable names are upper-cased with `Locale.ROOT`, as
Spring does, so a key containing `i` matches its upper-cased variable under a
Turkish default locale too; and `getProperty(null)` returns the default value
instead of throwing a `NullPointerException`.
+
+==== 5. The Character Encoding Filter Runs First
+
+Grails 8.1 registers its character encoding filter at
`Ordered.HIGHEST_PRECEDENCE`, the order Spring Boot gives its own, instead of
at -180. Once a filter reads a request parameter, the servlet container has
decoded the request body, as ISO-8859-1 unless an encoding was set by then, so
a filter ordered ahead of the encoding filter that read a parameter garbled
non-ASCII form input for the rest of the request, for example `Café` for
`Café`.
+
+Every filter ordered between `Ordered.HIGHEST_PRECEDENCE + 1` and -181 now
runs after the encoding filter instead of before it, and sees the request
encoding set. That includes asset-pipeline's filter at
`GrailsFilters.ASSET_PIPELINE_FILTER` (-190), a filter at
`GrailsFilters.FIRST`, and filters registered at `Ordered.HIGHEST_PRECEDENCE +
n`:
Review Comment:
Confirmed when run, and added in 367e2dbb7e, as the first bullet and in the
required actions. In app1 with `grails.filter.encoding: ISO-8859-1`, a PUT form
carrying `title=Caf%E9` without a charset reached the controller as `Caf�` with
the encoding filter at -180 and as `Café` at `HIGHEST_PRECEDENCE`.
##########
grails-web-common/src/main/groovy/org/grails/web/config/http/GrailsFilters.java:
##########
@@ -28,7 +30,14 @@ public enum GrailsFilters {
FIRST,
ASSET_PIPELINE_FILTER,
- CHARACTER_ENCODING_FILTER,
+
+ /**
+ * Runs ahead of every other filter, as Spring Boot's own encoding filter
does. A filter that reads a request
+ * parameter before the encoding is set makes the servlet container decode
the request body with its default
+ * charset (ISO-8859-1), for the rest of the request.
+ */
+ CHARACTER_ENCODING_FILTER(Ordered.HIGHEST_PRECEDENCE),
Review Comment:
Done in d4843e885f. `FIRST` now has Javadoc saying it is the first slot
after the encoding filter. `GrailsSecurityHeadersAutoConfiguration` calls it
the outermost Grails slot after the encoding filter, which writes none of its
headers, and lists the encoding filter among the filters that run outside the
headers filter.
##########
grails-doc/src/en/guide/upgrading/upgrading81x.adoc:
##########
@@ -72,3 +72,19 @@ grailsApplication.config.refresh()
`refresh()` is a method of the configuration implementation rather than of the
`Config` interface, so it is a call for dynamically compiled test code and may
change with the implementation. It reads the environment again and merges the
property sources again, which replaces a value the test set on the
configuration directly wherever a property source defines the same key. Call it
again once the variables are removed: the configuration keeps the environment
it last read.
Two smaller differences: variable names are upper-cased with `Locale.ROOT`, as
Spring does, so a key containing `i` matches its upper-cased variable under a
Turkish default locale too; and `getProperty(null)` returns the default value
instead of throwing a `NullPointerException`.
+
+==== 5. The Character Encoding Filter Runs First
Review Comment:
c86e47e4e5 adds a "Grails 8.1: The Character Encoding Filter Runs First"
section to `grails-8-upgrade`, with the topic in its description, the 8.1 guide
in its sources, checklist entries and a pitfall. The skill is already published
with each Grails release, so its 8.1 release covers 8.0 → 8.1 without a second
skill. A generic `grails-upgrade` skill, versioned with Grails and replacing
`grails-8-upgrade` and `grails-9-upgrade`, sounds right to me. It renames
published artifacts on two branches, so I'd rather do it in its own PR.
##########
grails-web-common/src/main/groovy/org/grails/web/config/http/GrailsFilters.java:
##########
@@ -28,7 +30,14 @@ public enum GrailsFilters {
FIRST,
ASSET_PIPELINE_FILTER,
- CHARACTER_ENCODING_FILTER,
+
+ /**
+ * Runs ahead of every other filter, as Spring Boot's own encoding filter
does. A filter that reads a request
+ * parameter before the encoding is set makes the servlet container decode
the request body with its default
+ * charset (ISO-8859-1), for the rest of the request.
+ */
+ CHARACTER_ENCODING_FILTER(Ordered.HIGHEST_PRECEDENCE),
Review Comment:
Done in 21c9793ca7. Single sign-out is registered at
`GrailsFilters.CHARACTER_ENCODING_FILTER.order + 1`, the line #16759 uses on
7.0.x, which here is `HIGHEST_PRECEDENCE + 1`. `CasSingleSignOutEncodingSpec`,
the same spec as in #16760, posts `Café – 🍂` with single sign-out enabled
against the CAS container: with single sign-out back at `HIGHEST_PRECEDENCE`
the controller gets `Café – ðŸ‚`, and with the change the text as typed.
`CasSingleSignOutSpec` and the rest of the CAS example pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
