You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2026/07/01 05:04:00 UTC

[jira] [Work logged] (WW-5626) Refactor JSON/REST @StrutsParameter enforcement to per-property authorization

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

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

                Author: ASF GitHub Bot
            Created on: 01/Jul/26 05:03
            Start Date: 01/Jul/26 05:03
    Worklog Time Spent: 10m 
      Work Description: lukaszlenart opened a new pull request, #308:
URL: https://github.com/apache/struts-site/pull/308

   ## Summary
   
   Clarifies how the `@StrutsParameter` `depth` parameter is counted and fixes an incorrect example, based on feedback from Markus (flyingfischer.ch) on the user list after the 7.2.1 announcement.
   
   He hit a hard-to-diagnose case adopting the WW-5626 breaking change: populating a `List<MyObject>` property was rejected with `depth = 1` and required `depth = 2`, even though the element was a flat POJO. The docs previously showed `depth = 1` for exactly this collection scenario.
   
   ## Changes
   
   - **New section "Understanding the `depth` parameter"** with a worked table, explaining that each property access **or** collection/map index counts as one level — so reaching an element's property costs one more level than the same property on a plain object. Includes the correct default (`depth = 0`) and the canonical rule from the annotation ("number of periods or brackets in the parameter name"), verified against `StrutsParameter.java`.
   - **Fixed the collection example** — `getUsers()` now uses `depth = 2` (was `depth = 1`), with a note that this holds even for flat element types.
   - **Added a JSON/REST note**: a body like `{"users":[{"name":"John"}]}` populates `users[0].name`, so the getter must be annotated with `depth = 2`; annotating only the setter is not enough.
   
   ## Context
   
   - JIRA: [WW-5626](https://issues.apache.org/jira/browse/WW-5626)
   - Reported on the `user@struts.apache.org` list in the `[ANN] Apache Struts 7.2.1` thread.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)




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

    Worklog Id:     (was: 1027723)
    Time Spent: 1h 40m  (was: 1.5h)

> Refactor JSON/REST @StrutsParameter enforcement to per-property authorization
> -----------------------------------------------------------------------------
>
>                 Key: WW-5626
>                 URL: https://issues.apache.org/jira/browse/WW-5626
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Plugin - JSON, Plugin - REST
>            Reporter: Lukasz Lenart
>            Assignee: Lukasz Lenart
>            Priority: Major
>             Fix For: 7.2.1
>
>          Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
>   Follow-up to WW-5624. The initial fix enforces {{@StrutsParameter}} on JSON/REST request bodies via post-hoc reflection — {{ContentTypeInterceptor}} deserializes into a fresh instance, then recursively copies only authorized properties. This works but has several drawbacks:
>   * ~250 lines of reflection-based copy logic in {{ContentTypeInterceptor}} ({{copyAuthorizedProperties}}, {{deepCopyAuthorizedCollection}}, {{deepCopyAuthorizedMap}}, {{deepCopyAuthorizedArray}})
>   * Requires a public no-arg constructor on target types; otherwise body deserialization is rejected entirely
>   * Relies on a fragile package-name heuristic ({{isNestedBeanType}}) to distinguish nested beans from leaf types — may misclassify third-party value types
>   h2. Proposed solution
>   Replace the two-phase copy with per-property authorization performed _during_ deserialization, so unauthorized fields are never set.
>   * New optional interface {{AuthorizationAwareContentTypeHandler}} extending {{ContentTypeHandler}}, exposing a property-level authorization callback
>   * {{ContentTypeInterceptor}} checks {{instanceof}} and uses property-level filtering when available; falls back to the existing two-phase copy for handlers that don't implement it (backward compatible)
>   * Jackson handlers ({{JacksonJsonHandler}}, {{JacksonXmlHandler}}) register a {{SimpleModule}} with a {{BeanDeserializerModifier}} that wraps each {{SettableBeanProperty}} with an authorizing decorator, consulting {{ParameterAuthorizer.isAuthorized(path, target, action)}} before {{deserializeAndSet()}}
>   * {{XStreamHandler}} uses an equivalent mechanism (e.g. {{xstream.omitField()}} pre-pass or a custom {{ReflectionConverter}})
>   * Authorization context propagated via {{ThreadLocal}}, consistent with the existing {{ActionContext}} pattern
>   h2. Cleanup also included
>   Minor follow-ups from the WW-5624 review:
>   * Remove redundant {{ModelDriven}} resolution in {{ParametersInterceptor.isParameterAnnotatedAndAllowlist}} (now handled by {{StrutsParameterAuthorizer}})
>   * Replace mock-based REST integration tests with real {{JacksonJsonHandler}} tests that assert actual property filtering behavior
>   * Guard the unchecked {{String}} cast on JSON map keys in {{JSONInterceptor.filterUnauthorizedKeysRecursive}}
>   h2. Outcome
>   * Removes ~200 lines of reflection-based copy logic
>   * Lifts the no-arg constructor requirement
>   * Authorization happens at the right layer (the deserializer), matching {{ParametersInterceptor}}'s per-parameter check semantics



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