[
https://issues.apache.org/jira/browse/WW-5624?focusedWorklogId=1014865&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1014865
]
ASF GitHub Bot logged work on WW-5624:
--------------------------------------
Author: ASF GitHub Bot
Created on: 11/Apr/26 01:05
Start Date: 11/Apr/26 01:05
Worklog Time Spent: 10m
Work Description: tranquac commented on PR #1657:
URL: https://github.com/apache/struts/pull/1657#issuecomment-4227548508
Follow-up commit `df533b3` fixes three additional correctness issues found
during an independent review of the v3 patch:
---
### Fix 1 — Collection/Map type preservation in deep copy helpers
`deepCopyAuthorizedCollection` was always returning `new ArrayList()`. If
the action field is typed as `Set<Pojo>`, the setter has signature `void
setTags(Set<Pojo> s)` — invoking it with an `ArrayList` throws
`IllegalArgumentException` at runtime.
Same class of issue in `deepCopyAuthorizedMap` (always returned
`LinkedHashMap`).
Fix: inspect the source collection/map type and instantiate the appropriate
concrete type:
```java
// Collection:
if (source instanceof SortedSet) {
result = new TreeSet(((SortedSet) source).comparator());
} else if (source instanceof Set) {
result = new LinkedHashSet();
} else {
result = new ArrayList();
}
// Map:
if (source instanceof SortedMap) {
result = new TreeMap(((SortedMap) source).comparator());
} else {
result = new LinkedHashMap();
}
```
---
### Fix 2 — Identity-based visited set in
`scrubUnauthorizedPropertiesRecursive`
The circular reference guard used `Set<Integer>` +
`System.identityHashCode(target)`. `System.identityHashCode` is NOT guaranteed
unique — two distinct live objects can have the same identity hash code. A
collision would cause a valid nested object to be skipped during scrubbing,
leaving unauthorized nested properties un-nulled.
Fix: replaced with `Collections.newSetFromMap(new IdentityHashMap<>())`,
which uses reference equality (`==`) and has no collision risk:
```java
Collections.newSetFromMap(new IdentityHashMap<>()) // identity set
// ...
if (!visited.add(target)) { return; } // target object itself, not its hash
```
---
### Fix 3 — `isNestedBeanType` now excludes all standard-library leaf
packages
Previously only `java.lang.*` and `java.math.*` were excluded. Types like
`UUID`, `Currency`, `Locale` (in `java.util`), `URI`/`URL` (in `java.net`), and
`Path`/`File` (in `java.nio`/`java.io`) returned `true` from
`isNestedBeanType`, causing the code to attempt to recurse into their internal
fields. This would silently drop fields of those types (no `@StrutsParameter`
annotation on internal JDK fields → rejected → value not copied).
Fix: broadened exclusions:
```java
// java.util.* non-Collection/Map leaf types (UUID, Currency, Locale, Date,
etc.)
if (clazz.getName().startsWith("java.util.") &&
!Collection.class.isAssignableFrom(clazz)
&& !Map.class.isAssignableFrom(clazz)) {
return false;
}
// All java.time.* temporal types
if (clazz.getName().startsWith("java.time.")) return false;
// I/O and network value types
if (clazz.getName().startsWith("java.net.") ||
clazz.getName().startsWith("java.io.")
|| clazz.getName().startsWith("java.nio.")) {
return false;
}
```
---
### Test results
```
plugins/json: 124 tests, 0 failures
plugins/rest: 76 tests, 0 failures
```
Issue Time Tracking
-------------------
Worklog Id: (was: 1014865)
Time Spent: 2h 20m (was: 2h 10m)
> Request body population bypasses @StrutsParameter contract outside
> ParametersInterceptor
> ----------------------------------------------------------------------------------------
>
> Key: WW-5624
> URL: https://issues.apache.org/jira/browse/WW-5624
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - JSON, Plugin - REST
> Affects Versions: 7.1.1
> Reporter: Tran Quac
> Priority: Major
> Fix For: 7.2.0
>
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> h2. Summary
> {{@StrutsParameter}} enforcement is currently implemented in
> {{ParametersInterceptor}} for standard request parameter binding, but
> request-body based binders in some plugins bypass that authorization model
> and populate action/model objects directly.
> This creates inconsistent behavior between URL/form parameters and JSON/XML
> request bodies, and may allow mass assignment of properties that would
> normally be rejected by {{ParametersInterceptor}}.
> h2. Affected areas currently identified
> * JSON plugin:
> {{JSONPopulator.populateObject()}} sets properties via direct reflection and
> does not follow the full {{@StrutsParameter}} authorization rules.
> * REST plugin:
> {{JacksonJsonHandler.toObject()}} updates target objects directly via Jackson
> and does not follow the full {{@StrutsParameter}} authorization rules.
> h2. Problem scope
> The issue is broader than checking whether a setter is annotated. The current
> core contract in {{ParametersInterceptor}} also includes:
> * permitted nesting depth
> * authorization based on the exposed root member
> * ModelDriven handling
> * transition mode semantics
> * related allowlisting behavior
> Any request-body binding implementation should align with that same contract,
> otherwise Struts applies different security rules depending on how input
> reaches the action/model.
> h2. Expected direction
> Instead of implementing separate partial checks in each plugin, Struts should
> reuse or extract the shared parameter-binding authorization logic from
> {{ParametersInterceptor}} and apply it consistently across request-body
> binders.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
