Lukasz Lenart created WW-5820:
---------------------------------
Summary: 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
Fix For: 7.5.0
{{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)
