[ 
https://issues.apache.org/jira/browse/WW-5712?focusedWorklogId=1039745&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1039745
 ]

ASF GitHub Bot logged work on WW-5712:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Sep/26 08:22
            Start Date: 05/Sep/26 08:22
    Worklog Time Spent: 10m 
      Work Description: carrerasdarren-cell commented on PR #1889:
URL: https://github.com/apache/struts/pull/1889#issuecomment-5550559947

   Addressed the requested deprecated-API cleanup: both token buffers now use 
`DeserializationContext.bufferAsCopyOfValue(parser)`.
   
   Added compatibility tests for JSON method/field any-setters and values 
buffered before a property-based constructor, with `USE_BIG_DECIMAL_FOR_FLOATS` 
both disabled and enabled, plus XML numeric-text round trips. The four new 
tests pass on both the preceding implementation and this update; they establish 
compatibility for these cases rather than demonstrate another defect.
   
   Validation: Core and REST compiled with JDK 17 and the pinned Jackson 2.22.1 
dependency. All six selected compatibility/positive-path tests passed, with no 
failures, errors, or skips; `git diff --check` is clean. The full upstream 
suite was not run locally.
   
   Assisted by OpenAI Codex.
   




Issue Time Tracking
-------------------

    Worklog Id:     (was: 1039745)
    Time Spent: 1h  (was: 50m)

> ParameterAuthorizingModule does not wrap the Jackson any-setter in the REST 
> plugin
> ----------------------------------------------------------------------------------
>
>                 Key: WW-5712
>                 URL: https://issues.apache.org/jira/browse/WW-5712
>             Project: Struts 2
>          Issue Type: Bug
>            Reporter: Lukasz Lenart
>            Priority: Major
>             Fix For: 7.4.0
>
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> {{ParameterAuthorizingModule.updateBuilder}} wraps the properties returned by 
> {{BeanDeserializerBuilder.getProperties()}} with 
> {{AuthorizingSettableBeanProperty}}, but it never consults 
> {{BeanDeserializerBuilder.getAnySetter()}}. The {{SettableAnyProperty}} is 
> therefore left unwrapped, so unknown keys dispatched to a Jackson 
> {{@JsonAnySetter}} are set without passing through the {{@StrutsParameter}} 
> authorization context — while an ordinary unannotated setter on the same 
> class, in the same request body, is correctly rejected.
> All three any-setter forms are affected (method, field and {{@JsonCreator}} 
> parameter). Both {{JacksonJsonHandler}} and {{JacksonXmlHandler}} register 
> the module, so JSON and XML bodies behave the same. {{@JsonUnwrapped}} is not 
> affected, and an any-setter beneath a rejected parent is already skipped 
> through {{skipChildren()}}.
> The obvious fix does not work: path-based authorization resolves a 
> parameter's root name against declared members of the target, so 
> {{pathFor(key)}} matches nothing for a dynamic key by construction. Routing 
> any-setter keys through the existing check would reject every key and break 
> every existing {{@JsonAnySetter}} user. Consent has to be expressed at the 
> sink rather than at the key.
> Proposed shape:
> * A new annotation member, {{@StrutsParameter(allowDynamicKeys = true)}}, 
> marking one any-setter as permitted to receive arbitrary keys. A plain 
> {{@StrutsParameter}} is deliberately not sufficient — arbitrary-key access 
> should have to be typed out, not granted by accident.
> * Enforcement behind a plugin-local constant, 
> {{struts.rest.anySetter.requireAnnotations}}, defaulting to {{false}}, so 
> nothing changes for existing applications until they opt in. Same shape as 
> {{struts.chaining.requireAnnotations}}; the default can be revisited in a 
> major release.
> * {{depth()}} keeps its current meaning, counted beneath the dynamic key. The 
> default {{depth = 0}} accepts a scalar value under any key and rejects a 
> nested object under it; {{depth = 1}} permits one level of nesting. That is 
> the rule an annotated {{Map}} property already follows.
> * {{@Target}} stays {{METHOD, FIELD}}. A {{@JsonCreator}}-parameter 
> any-setter has nowhere to carry the marker and is rejected with a warning 
> when enforcement is on. Narrowing that rare form now is preferable to 
> advertising an annotation placement the framework does not honour elsewhere, 
> and {{@Target}} can be widened later but never narrowed.
> * The wrapper mirrors {{AuthorizingSettableBeanProperty}}: delegate untouched 
> when the authorization context is inactive; on rejection log a warning, mark 
> the context redacted and call {{skipChildren()}} so nested graphs are never 
> constructed.
> Test coverage should include default-off compatibility, the method and field 
> forms, missing and explicit consent, depth 0/1/2, creator parameters, 
> unauthorized parents, {{@JsonUnwrapped}}, both the JSON and XML handlers, and 
> context cleanup.
> Documentation follow-up: the {{@StrutsParameter}} page states that the JSON 
> and REST plugins authorize each property during deserialization "so that 
> unauthorized fields are never set". That is not accurate as written — correct 
> the wording and document the any-setter behaviour explicitly.
> Reported by Darren Carreras (GitHub: carrerasdarren-cell), who has a patch 
> implementing the above and will open the pull request.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to