[
https://issues.apache.org/jira/browse/WW-5820?focusedWorklogId=1051176&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1051176
]
ASF GitHub Bot logged work on WW-5820:
--------------------------------------
Author: ASF GitHub Bot
Created on: 29/Oct/26 12:31
Start Date: 29/Oct/26 12:31
Worklog Time Spent: 10m
Work Description: emilhuseyn opened a new pull request, #2073:
URL: https://github.com/apache/struts/pull/2073
Fixes [WW-5820](https://issues.apache.org/jira/browse/WW-5820)
`StrutsParameterAuthorizer.isAuthorizedOnModelDrivenAction` allows a
property declared on neither the model nor the action, so Map-backed and
custom-accessor models keep working. The write that follows goes through
`CompoundRootAccessor`, which uses the first object in the root with a matching
setter. After a chain result that can be the previous action, which the
authorizer never looked at.
This takes the first shape proposed in the ticket. Before the fallback is
taken, `findDeclaringObjectBelow` walks the value stack root, skipping the
model and the action, and returns the first object that declares the property.
If there is one, the `@StrutsParameter` requirement is applied to that object.
Otherwise the fallback stays as it was.
The `ParametersInterceptor` write path is not changed. Model properties,
annotated action members and Map-backed models behave exactly as before. Only a
name declared on neither the current model nor the current action, but declared
on another object in the stack, changes behaviour: it now needs the same
annotation a direct request already needs.
Tests in `ParameterAuthorizerTest`, with the stack a chain result leaves
(model, chained ModelDriven action, previous action):
- a name declared only on the previous action without `@StrutsParameter` is
rejected
- the same name with `@StrutsParameter` on the previous action is authorized
- a name declared on nothing in the stack still takes the fallback
(Map-backed model case)
- model properties and the chained action's annotated and unannotated
members are unaffected
`ParameterAuthorizerTest`, `ParametersInterceptorTest` and
`StrutsParameterAnnotationTest` pass locally on JDK 21, along with the full
`core` suite.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1051176)
Remaining Estimate: 0h
Time Spent: 10m
> StrutsParameterAuthorizer's declared-on-neither fallback lets a parameter
> bind to an object below the action in the value stack
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: WW-5820
> URL: https://issues.apache.org/jira/browse/WW-5820
> Project: Struts 2
> Issue Type: Task
> Components: Core
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 7.5.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> {{StrutsParameterAuthorizer.isAuthorizedOnModelDrivenAction}} applies a
> three-way rule since WW-5698: a property declared on the model is exempt from
> {{@StrutsParameter}}, a property declared on the action needs the annotation,
> and a property declared on neither is allowed so that Map-backed and
> custom-accessor models keep working.
> The decision is made about two objects, the model and the action, while the
> write that follows is made against the whole stack.
> {{OgnlValueStack.setParameter}} hands the name to OGNL, and
> {{CompoundRootAccessor.setProperty}} walks every object in the
> {{CompoundRoot}} and invokes the first matching setter. When the root holds
> more than the model and the action, a name that takes the "declared on
> neither" fallback can land on an object the authorizer did not look at.
> The ordinary way the root holds a third object is a chain result:
> {{ActionChainResult.execute}} runs the chained action on the same value stack
> with the same parameters, so the previous action stays in the root below the
> chained action and its model. The convention plugin's
> {{ConventionUnknownHandler}} builds the same chain result implicitly when an
> action {{foo}} returning {{bar}} has a sibling action {{foo-bar}}, so the
> shape also arises without a chain result declared in struts.xml.
> h2. Observed
> Three actions, {{struts.parameters.requireAnnotations=true}}, default stack.
> {{First}} declares an unannotated public setter {{setFoo}}. {{Second}} is
> {{ModelDriven}}, and neither it nor its model declares {{foo}}. {{Plain}} is
> not {{ModelDriven}} and does not declare {{foo}}.
> {code}First directly foo=x -> not bound (rejected,
> no annotation)
> First chained into Plain foo=x -> not bound (rejected, no
> annotation)
> First chained into Second (ModelDriven) foo=x -> bound on First.setFoo
> {code}
> In the third row the authorizer takes the "declared on neither" fallback for
> {{Second}} and its model, and {{CompoundRootAccessor}} finds the setter on
> {{First}}. The model property and the annotated members of {{Second}} bind as
> expected in the same request, so the first two rows are real rejections
> rather than a harness that binds nothing.
> The property copy done by {{ChainingInterceptor}} is not involved. {{chain}}
> runs before {{params}} in {{defaultStack}}, and {{foo}} on {{First}} is still
> null when the copy runs because the parameter phase of {{First}} already
> rejected the name. {{struts.chaining.requireAnnotations}} governs only that
> copy.
> h2. Proposed change
> Make the authorizer's decision and the landing object agree. Two shapes are
> open:
> * Extend the "declared on neither" branch: before allowing the name, look at
> the remaining objects in the {{CompoundRoot}}. Where one of them declares the
> property, apply the annotation requirement to that object instead of taking
> the fallback.
> * Apply the accepted parameter to the object the decision was made about, the
> model or the action, rather than to the whole root. The fallback then keeps
> serving Map-backed models because the model itself absorbs the name. This
> changes how {{ParametersInterceptor}} writes and needs a compatibility look
> before it is chosen.
> h2. Compatibility
> Either shape affects only a request whose parameter name is declared on
> neither the current model nor the current action and is declared on another
> object in the stack. Model binding, annotated action members and Map-backed
> models are unchanged. An application that relies on a chained-into
> {{ModelDriven}} action binding a request parameter onto the previous action
> will stop binding it and needs {{@StrutsParameter}} on that setter, which is
> the requirement the direct request already has.
> h2. Tests
> * Chain into a {{ModelDriven}} action: a name declared only on the previous
> action is rejected.
> * Chain into a {{ModelDriven}} action: the model property, an annotated
> action property and a Map-backed model key still bind.
> * Direct binding and a chain into a plain action keep their current behaviour
> (control cases).
> * The convention plugin's implicit chain reproduces the same case (plugin
> module, optional).
> Reported by Sungho Park (@gigio1023).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
