[
https://issues.apache.org/jira/browse/WW-5820?focusedWorklogId=1052391&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1052391
]
ASF GitHub Bot logged work on WW-5820:
--------------------------------------
Author: ASF GitHub Bot
Created on: 04/Nov/26 10:11
Start Date: 04/Nov/26 10:11
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on PR #2073:
URL: https://github.com/apache/struts/pull/2073#issuecomment-6628145423
Thanks again for this one, it's merged. On your WW-5761 questions:
1. **Target:** yes, you can take WW-5761, but it won't be in 7.5.0. Please
open it against `main` behind its own constant, off by default. With the flag
off, nothing changes, so it can go into 7.6.0. Turning it on by default stays
an 8.0.0 decision.
2. **Where the state lives:** a per-parameter guard on the binding stack's
`SecurityMemberAccess` is fine. That bean is `scope="prototype"` and every
value stack gets its own instance, so state on the instance used by
`toNewStack` isn't shared between requests.
One related direction, so you don't duplicate it: orphan-oss/ognl#755
proposes letting the embedding application supply OGNL's property-name → member
resolution, with a 3.4.x backport. Once that is released, Struts will install a
resolver and `StrutsParameterAuthorizer` will query it instead of copying
OGNL's rules (`findBindableAccessor` / `accessorBaseNames`). That will be a
separate ticket, and you're welcome to take it too. It overlaps with WW-5761
only at the authorizer: your enforcement at member resolution still covers what
a pre-check can't see. So keep the authorizer changes in your PR minimal;
they're likely to be reshaped by that work.
Issue Time Tracking
-------------------
Worklog Id: (was: 1052391)
Time Spent: 1.5h (was: 1h 20m)
> 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: 1.5h
> 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)
