[
https://issues.apache.org/jira/browse/WW-5747?focusedWorklogId=1041664&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1041664
]
ASF GitHub Bot logged work on WW-5747:
--------------------------------------
Author: ASF GitHub Bot
Created on: 15/Sep/26 13:04
Start Date: 15/Sep/26 13:04
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1946:
URL: https://github.com/apache/struts/pull/1946
Fixes [WW-5747](https://issues.apache.org/jira/browse/WW-5747)
## Problem
`RedactionAwareDeserializer` drops a bean whose construction fails after a
`@StrutsParameter` redaction by catching the `JsonMappingException` and
returning `null`. It returned without moving the parser, so whatever tokens of
the dropped object were still unread went to the *enclosing* bean: a
creator-bound bean failing on its last creator parameter with fields after it,
or a bean whose bean-typed `@JsonIdentityInfo` id failed to construct and could
not be bound, left its remaining fields to the parent — a same-named parent
property took the child's value, the parent's own later properties were lost,
and an unknown field failed the parent inside a scope already marked redacted,
dropping the root.
## Change
- On entry the wrapper records the context of the object or array it is
entered on. Before returning `null` it skips to that value's end token,
consuming nested structures whole. The end is recognised by **context
identity** — the first end token whose context no longer descends from the
recorded one — which every parser keeps, including the token buffers Jackson
replays `@JsonUnwrapped` and any-setter values from (those report no nesting
depth at all; a depth comparison drained them). A parser left without a current
token, as Jackson does before splicing a late type id, is advanced first. A
scalar entry needs nothing.
- A polymorphic value Jackson reads from a **spliced** parser (type id not
the first key, or `visible = true`) straddles the splice and cannot be
followed: it is the value entered mid-object on a buffer context while the
parser is a `JsonParserSequence`, and its drop is not attempted. The exception
propagates with the enclosing scope marked, so the nearest enclosing bean that
can leave its parser in order drops itself; through the handlers'
`readerForUpdating` root the read fails. Either replaces a silent
desynchronisation. A bean nested inside such a value lies wholly on one side
and is dropped normally. The refusal logs a WARN naming the type, so the
resulting Jackson error can be tied to authorization.
## Review trail
Three code-review and two security-review passes, each finding a deeper
parser shape: depth → identity (token buffers), cleared current token after
Jackson's splice, refusal too broad (nested beans inside a spliced value),
refusal too narrow (a splice whose base parser is itself a buffer), parent
scope not marked on refusal. All are regression tests now. Security: nothing
newly introduced; every direction is "consume more or fail", never "leave
unread for the parent".
## Tests
`ParameterAuthorizingModuleTest`, nine new cases: null-id drop with trailing
fields; creator failure with a nested object and trailing fields; late-type-id
drop fails the read through `readerForUpdating`; a bean nested in a
late-type-id value still drops in place; a refused drop marks the parent so it
drops itself; a late-type-id value nested in another's buffer escalates rather
than leaking; parent swallowing a refused subtype drop after
`clearCurrentToken()`; drop inside an `@JsonUnwrapped` replay stops at the
record's end.
`mvn test -DskipAssembly -pl plugins/rest`: 210 tests, 0 failures.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1041664)
Remaining Estimate: 0h
Time Spent: 10m
> RedactionAwareDeserializer leaves the parser mid-object when it drops a REST
> body object
> ----------------------------------------------------------------------------------------
>
> Key: WW-5747
> URL: https://issues.apache.org/jira/browse/WW-5747
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Summary
> {{RedactionAwareDeserializer}} wraps every bean deserializer in the REST
> plugin's Jackson authorization and, when construction fails after a
> {{@StrutsParameter}} redaction marked its scope, catches the
> {{JsonMappingException}} and returns {{null}} so the whole object is dropped
> rather than partially exposed. It returns without moving the parser: whatever
> tokens of the dropped object remain unread are then consumed by the
> _enclosing_ bean's property loop.
> h2. Current behaviour
> Any failure that fires before the dropped object's last field desyncs the
> read. Two shapes reach it:
> * a creator-bound bean whose construction fails when its last creator
> parameter arrives (a redacted primitive under
> {{FAIL_ON_NULL_FOR_PRIMITIVES}}, a compact constructor rejecting {{null}})
> while non-creator fields follow it in the body;
> * a bean whose property-based {{@JsonIdentityInfo}} id is a bean that itself
> failed to construct on a redacted member: the {{null}} id fails Jackson's
> binding at the {{id}} property (WW-5746).
> Reproduced with this fixture, {{child.id.k}} denied and every other path
> granted:
> {code:java}public record StrictKey(String k) {
> public StrictKey {
> Objects.requireNonNull(k);
> }
> }
> @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class,
> property = "id")
> public class StrictIdentified {
> public StrictKey id;
> public String name;
> }
> public class StrictHolder {
> public StrictIdentified child;
> public String other;
> public String name;
> }
> {code}
> {code:json}{"child": {"id": {"k": "a"}, "name": "x"}, "other": "o"}
> {code}
> {{child}} is {{null}} as intended, but the child's {{name}} value {{x}} was
> bound to the _holder's_ {{name}}, and the holder's own {{other}} was lost
> because its object read ended at the child's closing brace. Without a
> same-named property on the parent the stray field raises "Unrecognized
> field", which — the parent's scope being marked redacted — silently drops the
> whole root object instead.
> The misplaced value still passes the parent property's own
> {{@StrutsParameter}} check at its own path, so this is data landing in the
> wrong property, not an authorization bypass.
> h2. Proposed change
> Resync before returning {{null}}: record the parser's nesting depth (and
> whether the entry token was {{START_OBJECT}}, {{START_ARRAY}} or a
> {{FIELD_NAME}} inside the object) when the wrapper is entered, and on a
> swallowed failure skip tokens — {{skipChildren()}} on any nested start token
> — until the matching {{END_OBJECT}} or {{END_ARRAY}} at that depth has been
> consumed. A scalar entry token needs no resync. Cover both shapes above with
> tests, asserting the parent's later properties still bind and the dropped
> object's fields never reach the parent.
> h2. Compatibility notes
> No configuration or API change. Only bodies that already trigger an object
> drop are affected, and only in that the enclosing object is now read
> correctly afterwards.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
