mirror of
https://github.com/apache/struts.git
synced 2026-08-31 11:24:28 +00:00
quactv
c3a887085d
* WW-5624 fix(security): enforce @StrutsParameter on JSON/REST body deserialization Extract ParameterAuthorizer service from ParametersInterceptor to share @StrutsParameter annotation enforcement across all input channels. The json-plugin (JSONInterceptor) and rest-plugin (ContentTypeInterceptor) previously bypassed @StrutsParameter checks when deserializing request bodies, allowing mass assignment even when struts.parameters.requireAnnotations=true. Changes: - New ParameterAuthorizer interface and DefaultParameterAuthorizer impl - JSONInterceptor: filter unauthorized Map keys before populateObject() - ContentTypeInterceptor: two-phase deserialization (fresh instance then copy authorized properties) when requireAnnotations=true; direct deserialization for backward compat when disabled - OGNL ThreadAllowlist side effects remain in ParametersInterceptor only - Full DI wiring: struts-beans.xml + StrutsBeanSelectionProvider + DefaultConfiguration - 15 new unit tests for ParameterAuthorizer, 2 for JSON plugin, 2 for REST plugin; 32 existing regression tests verified * WW-5624 address review feedback from lukaszlenart on PR #1657 1. Rename DefaultParameterAuthorizer → StrutsParameterAuthorizer per Struts naming convention (inline suggestion) 2. Narrow ModelDriven exemption: require action instanceof ModelDriven before exempting target from @StrutsParameter checks. Prevents non-ModelDriven root objects (e.g. JSONInterceptor.root) from bypassing annotation enforcement. 3. Recursive JSON key filtering: filterUnauthorizedKeys() now recurses into nested Maps and Lists, building dot-notation paths (e.g. "address.city") for path-aware @StrutsParameter(depth=N) checks. 4. Deep REST property copy: copyAuthorizedProperties() now recurses into nested bean types with path-aware authorization. Collections, Maps, primitives, and java.lang/java.time types are copied directly. 5. Null-skip semantics preserved and documented: in two-phase deserialization, null in freshInstance is indistinguishable from "not present in request" — clearing would destroy pre-initialized fields. Kept as intentional design choice with inline documentation. 6. No-arg constructor fallback: when target class lacks a no-arg constructor, falls back to single-phase deserialization with post-scrub of unauthorized properties, preserving backward compat. 7. New regression tests: - Non-ModelDriven target with different object (must not exempt) - Nested JSON keys recursively filtered - Non-action root object still checked by authorizer All 280+ core tests, 124 JSON tests, 76 REST tests pass with 0 regressions. * WW-5624: v3 — fix indexed-path depth parity with ParametersInterceptor Four gaps identified by lukaszlenart's April 10 review are now fully addressed: 1. JSON filterUnauthorizedList: pass prefix+"[0]" instead of bare prefix so that list element properties gain one extra '[' in their path — e.g. "publicPojoListDepthOne[0].key" (depth=2) is now correctly rejected when @StrutsParameter(depth=1), matching ParametersInterceptor semantics. Also recurse into nested List<List<Map>> via an else-if branch. 2. REST copyAuthorizedProperties: add authTarget parameter (always = root action/model, passed unchanged through all recursion levels). isAuthorized() now checks the full path against the root class, so "address.city" is looked up on the action, not on the Address object. 3. REST Collection/Map/array deep authorization: replaced the as-is copy with deepCopyAuthorizedCollection(), deepCopyAuthorizedMap(), and deepCopyAuthorizedArray() helpers — each iterates elements with path+"[0]" prefix, authorizing every complex element individually. No-arg fallback skips the element rather than copying an unfiltered object graph (security fix over plan's original as-is suggestion). 4. REST scrubUnauthorizedProperties: now fully recursive via scrubUnauthorizedPropertiesRecursive() — visits nested beans, collection elements, and map values with authTarget always pointing to the root. Includes identity-based visited-set to guard against circular reference cycles. Tests: core 2920 + json 124 + rest 76 = 3120, 0 failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * WW-5624: v3.1 — fix collection type, identity set, isNestedBeanType coverage Three correctness/security issues identified by independent review: 1. deepCopyAuthorizedCollection/deepCopyAuthorizedMap type preservation: Previously always returned ArrayList/LinkedHashMap. If the action field is typed Set<Pojo> or SortedMap<K,V>, writeMethod.invoke would throw IllegalArgumentException. Now: SortedSet→TreeSet, Set→LinkedHashSet, List→ArrayList; SortedMap→TreeMap, Map→LinkedHashMap. 2. scrubUnauthorizedPropertiesRecursive visited-set identity safety: Replaced Set<Integer>+System.identityHashCode (not collision-safe) with Collections.newSetFromMap(new IdentityHashMap<>()) which uses reference equality (==). A hash collision could have caused a valid nested object to be skipped, leaving unauthorized properties un-scrubbed. 3. isNestedBeanType now excludes all standard-library leaf packages: java.util.* non-Collection/Map types (UUID, Currency, Locale, Date), java.time.* (all temporal types, not just Temporal subinterface), java.net.*, java.io.*, java.nio.*. Previously UUID etc. would return true, causing the code to recurse into their internal fields and silently drop the value when no @StrutsParameter annotation matched. Tests: json 124 + rest 76 = 200, 0 failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * WW-5624: v4 — close bulk-copy fallback, reject body when no no-arg ctor Two remaining gaps addressed per lukaszlenart's April 11 review: 1. copyAuthorizedProperties bulk-copy fallback removed: When a nested target bean is null and createFreshInstance fails (no no-arg constructor), the previous code fell back to writeMethod.invoke(target, sourceValue) — copying the whole nested object graph without per-path authorization. Now logs a warning and skips the property entirely (same policy as deepCopyAuthorizedCollection elements with no no-arg constructor). 2. Top-level no-arg constructor fallback changed from scrub to reject: When requireAnnotations=true and the target class has no no-arg constructor, body deserialization is now rejected entirely (handler.toObject is never called). The previous best-effort scrub path could not guarantee that all nested unauthorized properties were nulled out. scrubUnauthorizedProperties and its recursive helper are removed as dead code. Tests: rest 76, 0 failures. --------- Co-authored-by: tranquac <tranquac@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Struts 2 Plugins
A set of officially supported plugins which are provided with the framework, you can read more about them in the documentation.