* fix(rest): authorize @StrutsParameter on record/creator-bound REST body properties
ParameterAuthorizingModule enforces @StrutsParameter on REST/JSON body
deserialization by wrapping each property's deserializeAndSet/
deserializeSetAndReturn. Jackson never calls either method for
creator-bound properties (Java records, @JsonCreator constructors,
@ConstructorProperties) — it calls SettableBeanProperty#deserialize
directly, which is declared final and bypasses the wrapper entirely.
With struts.parameters.requireAnnotations enabled, any record-typed
field anywhere in a REST action's request body was populated with no
authorization check at all.
Add AuthorizingValueDeserializer, which wraps the property's value
deserializer instead of the property itself, and install it from
AuthorizingSettableBeanProperty#withValueDeserializer — scoped to
CreatorProperty so ordinary setter/field/builder properties, already
authorized via the existing wrapper, aren't checked twice.
* fix(rest): treat redaction-induced construction failures as unauthorized, not fatal
AuthorizingValueDeserializer substitutes null for a rejected creator-bound
property (record component, @JsonCreator/@ConstructorProperties param).
For reference-typed, unvalidated components this is a harmless stand-in
for "not set" -- but two cases turn that substitution into an unhandled
exception that crashes deserialization of the entire request body instead
of just dropping the unauthorized subtree:
- A record/constructor with its own non-null validation (e.g. a compact
constructor doing Objects.requireNonNull) throws
ValueInstantiationException when the redacted component reaches it.
- With DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES enabled, Jackson
itself throws MismatchedInputException when a primitive-typed creator
component is redacted to null.
Add RedactionAwareDeserializer, wrapping every bean-type deserializer via
a new BeanDeserializerModifier#modifyDeserializer hook. It tracks (via a
new redaction-scope stack in ParameterAuthorizationContext) whether the
object currently under construction had a property redacted by
authorization; if construction then throws, the object is treated as
unauthorized (returns null) instead of propagating the raw exception --
matching the same fail-closed outcome already used when a non-creator
nested property is rejected outright. A guard test confirms genuine,
unrelated validation failures (nothing redacted) still propagate
normally, so real client errors aren't masked.
Also verified (and added regression coverage for) the other gaps raised
in review: static factory-method @JsonCreator, @ConstructorProperties,
top-level records, 3-level nested creator chains, and List/Map creator
params whose elements are further creator-bound or plain-POJO types --
all of these were already handled correctly by the existing
withValueDeserializer interception.
* test(rest): cover array creator param; document redaction edge cases
Addresses the three non-blocking review notes on WW-5642:
- Add testArrayOfRecordsAsCreatorParam_elementsAuthorizedByIndexedPath
and a WithArray fixture, exercising the type.isArray() branch of
AuthorizingValueDeserializer#prefixForNested so the collection matrix
(List/Map/array) is fully covered.
- Document in AuthorizingValueDeserializer that redacting a primitive
creator component becomes the type default (0/false) when
FAIL_ON_NULL_FOR_PRIMITIVES is off -- a deliberate choice, the client
value never lands either way.
- Document in RedactionAwareDeserializer that a redaction co-located with
an unrelated mapping error is folded into "object dropped" -- a
deliberate fail-closed trade-off, never exposing a partial object.
---------
Co-authored-by: g0w6y <g0w6y@users.noreply.github.com>
* WW-5626 spike: validate Jackson per-property authorization mechanism
Validates that the Approach C design is feasible before committing to a detailed
implementation plan. Wraps each SettableBeanProperty via BeanDeserializerModifier;
intercepts deserializeAndSet to authorize against a path built from a ThreadLocal
Deque; uses skipChildren() to discard unauthorized values; uses [0] suffix for
collection/map/array elements to match ParametersInterceptor depth semantics.
Findings:
- Delegating base class via 'protected delegate' field is the right pattern
- addOrReplaceProperty(prop, true) is the correct builder API
- Reject-at-parent skips all nested deserialization (better security than two-phase
copy: setter side effects on unauthorized properties never fire)
- JavaType#isCollectionLikeType/isMapLikeType/isArrayType detects the indexed-path case
Spike is kept under .../spike/ as a learning artifact; it will be replaced by
production code + tests in subsequent commits.
* WW-5626 add ParameterAuthorizationContext for deserializer-level authorization
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5626 address review feedback on ParameterAuthorizationContext
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5626 add AuthorizationAwareContentTypeHandler marker interface
* WW-5626 add AuthorizingSettableBeanProperty for Jackson per-property authorization
* WW-5626 add ParameterAuthorizingModule installing the property wrapper on Jackson mappers
* WW-5626 register ParameterAuthorizingModule on default Jackson REST handlers
* WW-5626 use AuthorizationAwareContentTypeHandler path when handler supports it
* WW-5626 add integration tests proving the new Jackson authorization path is used
* WW-5626 deprecate XStreamHandler in favor of JacksonXmlHandler
* WW-5626 remove Jackson auth spike; replaced by production tests
* WW-5626 make JuneauXmlHandler authorization-aware via post-parse walk
Implements AuthorizationAwareContentTypeHandler. When ParameterAuthorizationContext
is active (set by ContentTypeInterceptor when requireAnnotations=true), the handler
walks the parsed result tree and copies only authorized properties to the target,
descending into nested beans/collections/maps/arrays with indexed-path semantics
([0] suffix) for parity with ParametersInterceptor.
Note: Juneau parses the entire result tree before our walk runs, so setter side
effects on transient nested objects can fire even for unauthorized properties —
those transient objects are then discarded. This is functionally equivalent to the
legacy two-phase copy in ContentTypeInterceptor; only the Jackson handlers achieve
the stronger guarantee where unauthorized subtrees are never instantiated at all
(they use Jackson's BeanDeserializerModifier + skipChildren).
When no context is bound (default config), behavior is unchanged: parser.parse +
BeanUtils.copyProperties.
* WW-5626 add JuneauXmlHandler integration tests for @StrutsParameter authorization
* WW-5626 test(rest): cover JuneauXmlHandler post-parse walk for collections, maps, arrays
Sonar reported 51 uncovered new lines in JuneauXmlHandler (48.8% coverage on the
post-parse authorization walk — the security-critical code path the branch exists
to introduce). Add integration coverage for the previously-uncovered branches:
- collection-of-scalars (List<String> tags)
- collection-of-beans (List<Address> addresses)
- map-of-scalars (Map<String,String> attributes)
- array-of-scalars (String[] aliases)
- empty collection
- malformed XML wrapped as IOException
Also drop two unnecessary casts (Sonar S1905) on lines 243/252 — the unchecked
conversion happens at the return statement, the explicit casts were redundant
under the existing @SuppressWarnings("unchecked").
Add @Override on the inline AnyConstraintMatcher.matches override (Sonar S1161).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5626 test(rest): cover AuthorizingSettableBeanProperty builder-path deserialization
Sonar reported 11 uncovered new lines on AuthorizingSettableBeanProperty (66.7%
coverage). All 11 are in deserializeSetAndReturn — the alternate Jackson entry
point used for builder-pattern deserialization, never triggered by setter-based
fixtures like Person.
Add an @JsonDeserialize(builder=...) fixture (ImmutablePerson) that forces
Jackson to use BuilderBasedDeserializer, which dispatches property writes
through deserializeSetAndReturn. Three new tests exercise the path:
inactive-context pass-through, top-level authorization, and full rejection.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5626 refactor(rest): extract helpers from ContentTypeInterceptor.intercept
Sonar S3776 flagged intercept() at cognitive complexity 16 (limit 15). Extract
the body-handling branches into named helpers:
- openBodyReader: encoding-aware reader from the request InputStream
- applyRequestBody: dispatcher between requireAnnotations on/off paths
- applyWithAuthorizationContext: bind + delegate + unbind for AuthorizationAware handlers
- applyTwoPhaseDeserialize: legacy fresh-instance + copyAuthorizedProperties path
intercept() drops to ~12 lines and reads as a flat sequence: resolve target,
delegate body application, invoke. Each helper carries the comment that
explains the security model for its branch.
Add @Override on the inline AnyConstraintMatcher.matches override (Sonar S1161).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5626 add ParameterAuthorizer#resolveTarget for centralized ModelDriven resolution
Move the ValueStack peek logic that derives the target object from action+ModelDriven
state out of ParametersInterceptor and into ParameterAuthorizer. Callers that need
both authorization and the resolved target (for downstream OGNL allowlisting) can
now call resolveTarget once and reuse the result.
* WW-5626 delegate ModelDriven target resolution to ParameterAuthorizer
Replace the inline ValueStack peek in ParametersInterceptor#isParameterAnnotatedAndAllowlist
with a call to ParameterAuthorizer#resolveTarget. The ModelDriven import is no longer
needed in this class.
* WW-5626 defensively skip non-String JSON keys in authorization filter
The (String) cast in filterUnauthorizedKeysRecursive threw ClassCastException
for any custom JSONReader producing non-String keys. Replace with an instanceof
pattern that debug-logs and skips entries whose key cannot be converted to a
parameter path.
* WW-5626 add real JacksonJsonHandler integration tests for @StrutsParameter filtering
The existing ContentTypeInterceptorTest uses mock ContentTypeHandlers, so its
requireAnnotations=true tests verify only that intercept() returns SUCCESS — they
assert nothing about which properties were actually filtered. These integration
tests use a real JacksonJsonHandler + a real StrutsParameterAuthorizer to verify
end-to-end property-level filtering for top-level annotated/unannotated properties
and nested properties at varying authorized depths.
The SecureRestAction fixture documents a semantic divergence: REST's recursive
copy authorizes each path level independently, so depth-0 authorization on the
top-level property requires @StrutsParameter on the setter even when nested
field access is the actual goal. ParametersInterceptor only requires the getter
annotation. This divergence is tracked for the Approach C refactor.
* WW-5626 make ParameterAuthorizer#resolveTarget a default method to preserve SAM
Making resolveTarget abstract broke ParameterAuthorizer as a functional interface,
which the existing JSON and REST plugin tests rely on for lambda-based stubs:
interceptor.setParameterAuthorizer((parameterName, target, action) -> true);
The default returns the action unchanged — adequate for lambda test stubs whose
authorization decisions don't depend on the resolved target. The production
implementation (StrutsParameterAuthorizer) overrides this with the proper
ModelDriven value-stack peek.
* 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>
* WW-5537 Add InternalDestroyable and ContextAwareDestroyable interfaces
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 ContainerHolder: ThreadLocal with AtomicLong generation counter
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 FinalizableReferenceQueue: volatile instance, join, classloader null
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 ScopeInterceptor.clearLocks: add synchronized block
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 CompoundRootAccessor, DefaultFileManager: implement InternalDestroyable
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Add InternalDestroyable adapter classes for static cache cleanup
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Register InternalDestroyable beans in struts-beans.xml
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 JSON plugin: add JSONCacheDestroyable for BeanInfo cache cleanup
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Dispatcher.cleanup: refactor into focused methods with InternalDestroyable discovery
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Rewrite DispatcherCleanupTest for InternalDestroyable discovery
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Add log4j-web for proper Log4j2 lifecycle in Servlet container
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Dispatcher.destroyObjectFactory: add early return on null, use pattern matching
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Fix @since annotations: 7.1.0 -> 7.2.0
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Add Container.destroy() to clear internal caches on undeploy
Container now exposes a destroy() method that clears factories, injectors,
constructors, and ThreadLocals. This releases Class<?> keys and JDK
DelegatingClassLoader instances that pin the webapp classloader.
DefaultConfiguration.destroy() calls container.destroy() and
reloadContainer() delegates to destroy() to avoid duplication.
Also fixes JSONCacheDestroyable referencing non-existent DefaultJSONWriter
(renamed to StrutsJSONWriter).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Fix Container.destroy(): don't clear factories, don't call from reloadContainer
factories must remain intact because existing code holds direct
references to the Container after destroyConfiguration() and expects
it to still resolve dependencies (e.g. during configuration reload).
reloadContainer() reverted to clearing packageContexts/loadedFileNames
directly — calling destroy() there nulled the container reference and
cleared state needed during the bootstrap transition.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Restore destroy() call in reloadContainer()
The test failures were caused by factories.clear() in
Container.destroy(), not by calling destroy() from reloadContainer().
Now that factories.clear() is removed, destroy() is safe to call
here — it clears packageContexts, loadedFileNames, and the container's
reflection caches in one place.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* WW-5537 Fix Sonar issues: thread-safe FinalizableReferenceQueue, empty method comments
- Replace volatile field with AtomicReference in FinalizableReferenceQueue
for proper thread safety using getAndSet()
- Add comments to empty destroy() implementations in test mocks
- Replace deprecated new URL() with URI.toURL() in DispatcherCleanupTest
- Add comments to empty listener methods in DispatcherCleanupTest
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* [maven-release-plugin] prepare release STRUTS_7_1_1
* [maven-release-plugin] rollback the release of STRUTS_7_1_1
* [maven-release-plugin] prepare release STRUTS_7_1_1
* [maven-release-plugin] prepare for next development iteration
ⳕⲛτⲉⲅⲥⲉⳏτⲟⲅ 🕵🏻