codeconsole opened a new pull request, #16793: URL: https://github.com/apache/grails-core/pull/16793
Follows [jdaugherty's suggestion on #16792](https://github.com/apache/grails-core/pull/16792#issuecomment-6500589856): one enum that says what runs where, so code names a filter's order instead of hard-coding `HIGHEST_PRECEDENCE + 1`. The orders of the servlet filters in a Grails application were spread over three places: `GrailsFilters`, `GrailsFilterOrder` (one constant, -100) and literals in the modules that register the filters. `GrailsFilters` also derived most of its orders from each constant's position, so removing a constant would have moved the ones after it. `GrailsFilters` is already the enum of filter orders, so this PR makes it complete instead of adding a second one: - Every constant has its order explicitly, and the constants are listed in the order the filters run. - New constants for the filters that used a literal: `STARTUP_PROGRESS_FILTER`, `LATENCY_FILTER`, `CORS_FILTER` and `CAS_SINGLE_SIGN_OUT_FILTER`, plus `FORM_CONTENT_FILTER` (Spring Boot's `FormContentFilter`, or Grails' fallback) and `SPRING_SECURITY_FILTER_CHAIN`. - Each of those filters is now registered at its constant's order. **No filter changes order.** `GrailsFiltersSpec` pins every value at what it is today: | Constant | Order | Registered as | |---|---|---| | `STARTUP_PROGRESS_FILTER` | `HIGHEST_PRECEDENCE` | `FilterRegistrationBean` | | `LATENCY_FILTER` | `HIGHEST_PRECEDENCE` | `FilterRegistrationBean` | | `CORS_FILTER` | `HIGHEST_PRECEDENCE` | plain `Filter` bean | | `CHARACTER_ENCODING_FILTER` | `HIGHEST_PRECEDENCE` | plain `Filter` bean | | `CAS_SINGLE_SIGN_OUT_FILTER` | `HIGHEST_PRECEDENCE + 1` | `FilterRegistrationBean` | | `FORM_CONTENT_FILTER` | -9900 | plain `Filter` bean | | `FIRST` (security headers) | -200 | `FilterRegistrationBean` | | `ASSET_PIPELINE_FILTER` | -190 | | | `HIDDEN_HTTP_METHOD_FILTER` | -170 | `FilterRegistrationBean` | | `SITEMESH_FILTER` | -160 | | | `GRAILS_WEB_REQUEST_FILTER` | -150 | `FilterRegistrationBean` | | `LAST` | -110 | | | `SPRING_SECURITY_FILTER_CHAIN` | -100 | `FilterRegistrationBean` | Four filters share `HIGHEST_PRECEDENCE`. Spring Boot runs `FilterRegistrationBean`s of an order before plain `Filter` beans of that order, and within each kind the order their beans are defined in decides; the enum's Javadoc says so. Giving them distinct orders would change which runs first, so it is left for a change of its own. Two details: - `grails-startup-progress` and `grails-testing-support-latency` only compile against `grails-web-common` (`compileOnly`). Their filters are only loaded in a Grails servlet application, which always has it, as startup progress already does for the Servlet API. - The Spring Security plugin looked up `SecurityProperties.DEFAULT_FILTER_ORDER` by reflection. Spring Boot 4 has no such field (`SecurityProperties` moved and lost it), so the lookup always fell back to -100. It now uses `SPRING_SECURITY_FILTER_CHAIN`, which is -100. The plugin guide's filter-ordering section now points to `GrailsFilters`. #### Tests - `GrailsFiltersSpec` (new): every constant's order, the constants are listed in run order, and the encoding filter is ahead of every constant listed after it. - `GrailsFormContentFilterAutoConfigurationSpec`: `FORM_CONTENT_FILTER` equals `OrderedFormContentFilter.DEFAULT_ORDER`, and both Spring Boot's filter and Grails' fallback are registered at it. - Passing: the tests of `grails-web-common` (357), `grails-controllers` (215), `grails-web-url-mappings` (351), `grails-startup-progress` (27), `grails-testing-support-latency` (13) and `grails-spring-security` (232); Checkstyle and CodeNarc on the changed modules; app1's `CharacterEncodingSpec` and `FormContentFunctionalSpec`, the startup-progress example's `StartupProgressPageSpec`, and `CasSingleSignOutEncodingSpec` and `CasSingleSignOutSpec` against the CAS container (`-DTESTCONFIG=cas`). #### Merging up to 9.0.x If #16760 is in 9.0.x by then: - `GrailsFilters` conflicts. Take this side's explicit orders and constants, keep #16760's deprecation of `CHARACTER_ENCODING_FILTER`, and drop its note that the later constants take their orders from their position, which no longer holds. With `FORM_CONTENT_FILTER` naming the slot of a Spring Boot filter, `CHARACTER_ENCODING_FILTER` could do the same for Spring Boot's encoding filter on 9.0 instead of being removed; that is worth deciding there. - `GrailsFiltersSpec` is added on both sides; take this one. #16760's version expects every constant to be ordered after the encoding filter, which the constants at `HIGHEST_PRECEDENCE` here are not. - `SpringSecurityCasGrailsPlugin` conflicts: take `GrailsFilters.CAS_SINGLE_SIGN_OUT_FILTER.order`, the same order as 9.0.x's `Ordered.HIGHEST_PRECEDENCE + 1`. -- 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]
