[
https://issues.apache.org/jira/browse/WW-5627?focusedWorklogId=1019503&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1019503
]
ASF GitHub Bot logged work on WW-5627:
--------------------------------------
Author: ASF GitHub Bot
Created on: 09/May/26 17:23
Start Date: 09/May/26 17:23
Worklog Time Spent: 10m
Work Description: 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)
Issue Time Tracking
-------------------
Worklog Id: (was: 1019503)
Remaining Estimate: 0h
Time Spent: 10m
> CookieInterceptor bypasses @StrutsParameter authorization
> ---------------------------------------------------------
>
> Key: WW-5627
> URL: https://issues.apache.org/jira/browse/WW-5627
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.2.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Problem
>
> {{CookieInterceptor.populateCookieValueIntoStack}} calls
> {{stack.setValue(cookieName, cookieValue)}} directly
> ({{CookieInterceptor.java}} lines 339 and 348), which:
> - bypasses {{StrutsParameterAuthorizer}} / {{@StrutsParameter}} enforcement,
> - never primes {{ThreadAllowlist}}.
>
> Cookie injection therefore does not participate in the
> parameter-authorization machinery that {{ParametersInterceptor}}, the JSON
> plugin, and (post-[WW-5626|https://issues.apache.org/jira/browse/WW-5626])
> the REST plugin all share.
>
> h2. Impact
> When {{cookiesName=*}} is configured, every public setter on the action — and
> (subject to allowlist) any setter reachable through getter chains from the
> value-stack root — is writeable from cookie input without any
> {{@StrutsParameter}} opt-in. The OGNL allowlist (default {{enable=true}}
> since 7.0) blocks the nested-bean route via {{SecurityMemberAccess}}, but it
> does not require setters to be annotated with {{@StrutsParameter}}, so
> action-level setters remain reachable.
> h2. Proposed fix
>
> Replace {{stack.setValue}} in {{populateCookieValueIntoStack}} with
> {{stack.setParameter}} (or invoke {{StrutsParameterAuthorizer}} directly) so
> cookie injection participates in {{@StrutsParameter}} enforcement and
> {{ThreadAllowlist}} priming, aligning {{CookieInterceptor}} with
> {{ParametersInterceptor}}, the JSON plugin, and the REST plugin.
> h2. Dependencies
>
> Builds on [WW-5626|https://issues.apache.org/jira/browse/WW-5626] — reuses
> {{ParameterAuthorizationContext}} and {{ParameterAuthorizer.resolveTarget}}.
>
> h2. Backward compatibility
>
> Behavior change: apps relying on cookies populating un-annotated setters will
> stop working under strict authorization. Affected users must either add
> {{@StrutsParameter}} to the relevant setters or drop {{cookiesName=*}}.
> Migration-guide entry required.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
