jamesfredley commented on PR #15967: URL: https://github.com/apache/grails-core/pull/15967#issuecomment-4986244054
## Review feedback addressed Merged the latest `8.0.x` and pushed follow-up commits. **Copilot review comment - apply headers in a `finally` block after the chain:** evaluated and intentionally **not** adopted, because for security headers it regresses committed responses. If a downstream `sendRedirect` / `sendError` / `flushBuffer` / streaming write commits the response during the chain, `isCommitted()` is already true on return and a post-chain block would skip **all** hardening headers on exactly the redirect/error/streaming responses that need them. Instead the filter keeps applying the defaults eagerly before the chain with a `containsHeader` gap-fill (so a value set earlier wins, and an explicit downstream `setHeader` still replaces it), and I added: - a regression test that a **redirect-committed** response still carries the default headers, and - an override of `shouldNotFilterErrorDispatch()` to `false` plus a `DispatcherType.ERROR` test, so container **ERROR** redispatches (which reset the response) also get the headers. **Follow-up (documented, not changed here):** when the optional Spring Security plugin is installed, its header writers skip headers already present, so eagerly-set Grails defaults can win over a Spring-Security-configured policy. The robust fix is an on-commit response wrapper that fills only-missing headers after downstream writers run; that is a larger change and Spring Security is an optional plugin, so I left it as a follow-up rather than expand this starter filter's scope. Local verification: `:grails-controllers:test --tests GrailsSecurityHeadersAutoConfigurationSpec` (9 features) passes. -- 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]
