Lukasz Lenart created WW-5747:
---------------------------------
Summary: 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
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 {{record StrictKey(String k)}} (rejects {{null}}) as the id of
{{StrictIdentified { id; name }}}, held in {{StrictHolder { child; other; name
}}}, body {{{"child":{"id":{"k":"a"},"name":"x"},"other":"o"}}} and
{{child.id.k}} denied: {{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}}/{{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)
