lukaszlenart opened a new pull request, #1674:
URL: https://github.com/apache/struts/pull/1674

   Fixes [WW-5626](https://issues.apache.org/jira/browse/WW-5626).
   
   **Part 2 of 2.** Builds on the cleanup PR #1673 (do not merge until that one 
lands; base will be retargeted to `main` afterwards).
   
   ## What this changes
   
   Replaces the post-hoc reflection-based property filtering in 
`ContentTypeInterceptor` with per-property authorization performed *during* 
deserialization, for the default Jackson REST handlers.
   
   ## Architecture
   
   - **`ParameterAuthorizationContext`** (core) — `ThreadLocal` holder for 
per-request `(authorizer, target, action, pathStack)`. The interceptor binds it 
before invoking authorization-aware handlers and unbinds in a `finally` block.
   - **`AuthorizationAwareContentTypeHandler`** (rest) — marker interface. 
Handlers implementing it signal that their parser honors 
`ParameterAuthorizationContext` for per-property `@StrutsParameter` enforcement.
   - **`ParameterAuthorizingModule`** (rest) — Jackson `SimpleModule` with a 
`BeanDeserializerModifier` that wraps every `SettableBeanProperty` with 
`AuthorizingSettableBeanProperty`. Registered once on each handler's mapper.
   - **`AuthorizingSettableBeanProperty`** (rest) — wraps each property; checks 
`ParameterAuthorizationContext.isAuthorized(path)` before delegating, calls 
`JsonParser.skipChildren()` for unauthorized values, manages the path stack 
with `[0]` suffix for collection/map/array-typed properties.
   - **`JacksonJsonHandler` / `JacksonXmlHandler`** — register the module on 
construction; declare `implements AuthorizationAwareContentTypeHandler`.
   - **`ContentTypeInterceptor`** — when `requireAnnotations=true` AND `handler 
instanceof AuthorizationAwareContentTypeHandler`: binds context, calls 
`handler.toObject(...)` directly. Otherwise: legacy two-phase copy (preserved 
as-is for non-Jackson handlers).
   - **`XStreamHandler`** — `@Deprecated(since="7.2.0", forRemoval=true)` with 
Javadoc pointing users to `JacksonXmlHandler`. XStream's deserialization CVE 
history and per-class allowlist requirement justify removing it in a future 
major.
   
   ## Why this is better than the cleanup PR's two-phase copy
   
   - **No no-arg constructor requirement** for the target class
   - **Stronger security**: rejecting at the parent property means the 
unauthorized JSON subtree is discarded entirely — Jackson never instantiates 
nested objects, so setter side effects on unauthorized properties never fire 
(vs. two-phase copy which deserializes everything then nulls out)
   - **No reflection-based deep copy** for Jackson handlers — ~250 lines of 
post-hoc filtering bypassed
   - **No fragile `isNestedBeanType` package-name heuristic** — Jackson's 
`JavaType` introspection handles type detection correctly
   
   ## Out of scope (preserved as legacy fallback)
   
   The legacy two-phase copy in `ContentTypeInterceptor` is **not** removed — 
`XStreamHandler`, `JuneauXmlHandler`, and any custom handlers without the 
marker interface continue to use it. A future PR migrating those handlers can 
finally delete the dead code.
   
   ## Spike
   
   A spike that validated the Jackson `BeanDeserializerModifier` mechanism is 
in commit `6110b8f6b` and removed in commit `0166c6a0b` (replaced by the 
production tests in `ParameterAuthorizingModuleTest`).
   
   ## Test plan
   
   - [x] `mvn test -DskipAssembly -pl core` — 
`ParameterAuthorizationContextTest` 12 new tests pass; full core suite zero 
regressions
   - [x] `mvn test -DskipAssembly -pl plugins/rest` — 
`ParameterAuthorizingModuleTest` 8 new tests pass; 
`ContentTypeInterceptorIntegrationTest` 7 tests pass (5 from the cleanup PR now 
silently exercise the new path + 2 new tests proving Jackson path is in use)
   - [x] `mvn test -DskipAssembly -pl 
'!plugins/bean-validation,!plugins/tiles'` — full multi-module BUILD SUCCESS 
(the two excluded modules fail identically on `main` for unrelated reasons: 
`AnnotationFormatError` from Hibernate Validator on a test model, and missing 
Velocity dependencies in the tiles plugin)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to