lukaszlenart opened a new pull request, #1681: URL: https://github.com/apache/struts/pull/1681
## Summary Fixes [WW-5627](https://issues.apache.org/jira/browse/WW-5627) — `CookieInterceptor.populateCookieValueIntoStack` calls `stack.setValue` directly (`CookieInterceptor.java:339,348` pre-fix), bypassing `StrutsParameterAuthorizer` so cookies write to action setters that are not annotated with `@StrutsParameter` even when `struts.parameters.requireAnnotations=true`. This PR retrofits the `@StrutsParameter` enforcement onto the cookie input channel: - New `ParameterAllowlister` interface (extension point) + default `OgnlParameterAllowlister` impl extracted from `ParametersInterceptor.performOgnlAllowlisting`. Mirrors the `ParameterAuthorizer` pair from WW-5626. Apps can swap implementations via `<constant name="struts.parameterAllowlister" value="..."/>`. - `ParametersInterceptor` delegates to the new component (pure refactor, all 35 existing tests pass unchanged). - `CookieInterceptor` adds a new `protected void populateCookieValueIntoStack(name, value, map, stack, action)` extension hook. The default impl runs `parameterAuthorizer.isAuthorized` → `parameterAllowlister.allowlistAuthorizedPath` → existing 4-arg form. The 4-arg form is `@Deprecated(since="7.2.0")` with body unchanged, so existing subclass overrides automatically inherit the gate. - DI registration in `struts-beans.xml`, `StrutsBeanSelectionProvider`, and `DefaultConfiguration.bootstrapFactories()`. **Default-config invariant:** when `struts.parameters.requireAnnotations=false` (the 7.x default), `ParameterAuthorizer.isAuthorized` short-circuits to `true`, so existing apps see no behavior change. Only opt-in apps see cookies gated. ## Behavior matrix | Config | Cookie name | Setter `@StrutsParameter`? | Outcome | |---|---|---|---| | `requireAnnotations=false` (default) | any | any | Injected (unchanged) | | `requireAnnotations=true` | `flatName` | yes | Injected | | `requireAnnotations=true` | `flatName` | no | Skipped, debug log | | `requireAnnotations=true` | `user.role` | yes (depth ≥1) | Injected; `ThreadAllowlist` primed | | `requireAnnotations=true` | `user.role` | no / depth too low | Skipped, debug log | | `requireAnnotations=true`, transition | `flatName` | no | Injected (depth-0 exemption) | | `requireAnnotations=true`, ModelDriven model | any | n/a | Injected against model | ## Migration notes (for the Confluence wiki) > **Behavior change for `struts.parameters.requireAnnotations=true` adopters:** `CookieInterceptor` now enforces `@StrutsParameter` annotations on action setters when `struts.parameters.requireAnnotations=true`. Apps using `<param name="cookiesName">*</param>` together with annotation enforcement must annotate any setter intended to receive cookie values, or that cookie will be silently skipped (debug-logged). No change for the default config. ## Stacking Stacked on top of `WW-5626-approach-c` (where `ParameterAuthorizer` and `StrutsParameterAuthorizer` were introduced). When WW-5626 merges to `main`, this PR's base will be retargeted automatically. ## Test plan - [x] `mvn test -DskipAssembly -pl core` — 2952/2952 green. - [x] `mvn test -DskipAssembly -pl plugins/json` — 125/125 green (transitive contract sanity). - [x] New: `OgnlParameterAllowlisterTest` (6 unit tests). - [x] New: `CookieInterceptorAnnotationTest` (8 tests covering the full behavior matrix above, including subclass-override-of-deprecated-hook gets the gate, ModelDriven exemption, transition mode). - [x] Existing `CookieInterceptorTest` (9 tests) updated to wire pass-through lambdas via a `disableAuthorizationGate(...)` helper since those tests construct `CookieInterceptor` via `new` rather than the container. - [x] Existing `ParametersInterceptorTest`, `ActionMappingParametersInterceptorTest`, `StaticParametersInterceptorTest`, `StrutsParameterAnnotationTest` all green after the refactor. ## Out of scope (separate Jira candidates) - `ScopeInterceptor` (`ScopeInterceptor.java:306,328`) has the same `stack.setValue` pattern from session/application scope — likely warrants its own ticket. - `ParametersInterceptor.hasValidAnnotated*` helpers (lines 432–548) appear to be dead post-WW-5626 (only `StrutsParameterAuthorizer`'s versions are reached at runtime). Cleanup is a separate audit. - `ParameterNameAware` consultation in `CookieInterceptor` — not currently consulted; adding it would be a larger behavior change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
