lukaszlenart opened a new pull request, #1872: URL: https://github.com/apache/struts/pull/1872
Fixes [WW-5698](https://issues.apache.org/jira/browse/WW-5698) > **Decision still needed before merge: target release.** This is a behavioural change (see Compatibility). The ticket flags 7.4.0 vs 8.0.0 vs gating behind `requireAnnotations.transitionMode` as an open question. The PR is raised against `main` for review; retarget if 8.0.0 is preferred. ## Problem `StrutsParameterAuthorizer.isAuthorized(...)` returned `true` for *any* parameter name once the action implemented `ModelDriven`: ```java if (target != action && action instanceof ModelDriven) { return true; } ``` OGNL then resolves that name against the whole `CompoundRoot`, which holds the model **on top of the action**. Authorization was decided about the model while the write could land on the action. The practical result: the `@StrutsParameter` requirement did not apply to a `ModelDriven` action's own members. Same unannotated setter, declared on the action class in both cases, with `struts.parameters.requireAnnotations=true`: ``` plain action actionSecret=... -> not bound (correctly rejected) ModelDriven action actionSecret=... -> bound ``` ## Change The exemption now covers what it was meant to cover: - a property declared by **the model** is exempt — returning an object from `getModel()` declares it request surface, and that is the whole point of the exemption - a property declared by **the action** is subject to the annotation requirement, as it would be on any other action - a property declared by **neither** is still allowed That third case matters for compatibility. A model bound through a custom OGNL property accessor — a Map-backed model, most commonly — declares no bean property, and such a name cannot be reaching a member of the action either. Rejecting it would break those applications, so it is explicitly allowed. The model is checked **first**, so a model property that shadows an action property still binds without an annotation, matching OGNL's own resolution against the stack top. ## Compatibility An application whose `ModelDriven` action relies on binding unannotated members **declared on the action** will stop binding them and will need those members annotated with `@StrutsParameter`. That is the same migration those members would have needed had the action not been `ModelDriven`. Model binding itself is unchanged. ## Tests Six new cases in `ParameterAuthorizerTest`, covering the rejection, the annotated action member, the model property, the shadowed property, and the declared-on-neither escape. Both new branches were mutation-checked rather than trusted because they passed: - removing the model-first check fails **only** the shadowing test — so the ordering is load-bearing - removing the declared-on-neither escape fails the new test **and** the pre-existing `modelDriven_targetIsModel_allAuthorized` — so that escape is what preserves existing behaviour Green: `core` 3202, `json` 166, `rest` 124 (including `ParameterAuthorizingModuleTest`). Note: a full-reactor `mvn test` currently fails to compile `struts2-tiles-plugin` (`package org.apache.velocity.tools.view does not exist`). That is pre-existing — it reproduces identically on unmodified `main` — and unrelated to this change, but it does mean the four modules after tiles were not exercised. ## Related [WW-5697](https://issues.apache.org/jira/browse/WW-5697) / #1871 came from the same triage. Different cause, different fix; the two overlap only in that a `ModelDriven` action is the easiest way to reach both. 🤖 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]
