This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 0be1f3f30 WW-5747 fix(rest): leave the parser at the end of an object
the redaction wrapper drops (#1946)
0be1f3f30 is described below
commit 0be1f3f30e1cb8ae62d67ecff6fd5227c65c957c
Author: Lukasz Lenart <[email protected]>
AuthorDate: Tue Sep 15 16:14:47 2026 +0200
WW-5747 fix(rest): leave the parser at the end of an object the redaction
wrapper drops (#1946)
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.
The wrapper now records the context of the object or array it is
entered on and, before returning null, 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 unwrapped and any-setter values from; those
report no nesting depth at all, so 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 whose type id is not the first key is read from
Jackson's JsonParserSequence, spliced from a buffer of the keys before
the id and the real parser, and that value alone straddles the splice.
The buffer holds no start token, so that value is the one entered
mid-object on a buffer context while the parser is a splice - a bean
wholly inside the buffer is entered on its start token - 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, and through the handlers' root the read fails; either replaces
a silent desynchronisation. A bean nested inside such a value lies
wholly on one side of the splice and is dropped normally.
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../jackson/RedactionAwareDeserializer.java | 70 ++++++++
.../jackson/ParameterAuthorizingModuleTest.java | 194 +++++++++++++++++++++
2 files changed, 264 insertions(+)
diff --git
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/RedactionAwareDeserializer.java
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/RedactionAwareDeserializer.java
index c70ecfd6f..dd1835523 100644
---
a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/RedactionAwareDeserializer.java
+++
b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/RedactionAwareDeserializer.java
@@ -19,6 +19,9 @@
package org.apache.struts2.rest.handler.jackson;
import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonStreamContext;
+import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.core.util.JsonParserSequence;
import com.fasterxml.jackson.databind.BeanProperty;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
@@ -26,6 +29,7 @@ import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.deser.BeanDeserializerBase;
import com.fasterxml.jackson.databind.deser.impl.ObjectIdReader;
import com.fasterxml.jackson.databind.deser.std.DelegatingDeserializer;
+import com.fasterxml.jackson.databind.util.TokenBufferReadContext;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext;
@@ -88,12 +92,68 @@ final class RedactionAwareDeserializer extends
DelegatingDeserializer {
return contextual;
}
+ /**
+ * The context of the object or array about to be read, when the parser
stands on its start token
+ * or already inside it; {@code null} for a scalar, which the failed read
consumes whole.
+ */
+ private static JsonStreamContext structuredValue(JsonParser p) {
+ JsonToken token = p.currentToken();
+ if (token == null || !(token.isStructStart() || token ==
JsonToken.FIELD_NAME)) {
+ return null;
+ }
+ return p.getParsingContext();
+ }
+
+ /**
+ * Dropping the object must leave the parser on its end token, or the
fields left unread land in
+ * the enclosing bean. The end is found by context identity, which every
parser keeps, including
+ * the token buffers Jackson replays unwrapped and any-setter values from.
The one value that
+ * cannot be followed is a polymorphic one Jackson reads from a parser
spliced from a buffer and
+ * the real parser — a type id that is not the first key, or a visible one
— because that value
+ * straddles the splice. The buffer holds no start token, so such a value
is entered mid-object on
+ * a buffer context while the parser is a splice, and that is what is
refused. The test also
+ * catches a type-id-first bean inside an outer value's buffer, which
could have been followed;
+ * its drop escalates to the enclosing bean instead, which errs on the
side of dropping more.
+ */
+ private static boolean canResync(JsonParser p, JsonToken entry,
JsonStreamContext value) {
+ return value == null || !(p instanceof JsonParserSequence)
+ || !(value instanceof TokenBufferReadContext) || entry !=
JsonToken.FIELD_NAME;
+ }
+
+ private static void skipToEndOf(JsonParser p, JsonStreamContext value)
throws IOException {
+ if (value == null) {
+ return;
+ }
+ JsonToken token = p.hasCurrentToken() ? p.currentToken() :
p.nextToken();
+ while (token != null) {
+ if (token.isStructStart()) {
+ p.skipChildren();
+ token = p.currentToken();
+ }
+ if (token.isStructEnd() && !within(p.getParsingContext(), value)) {
+ return;
+ }
+ token = p.nextToken();
+ }
+ }
+
+ private static boolean within(JsonStreamContext context, JsonStreamContext
value) {
+ for (JsonStreamContext current = context; current != null; current =
current.getParent()) {
+ if (current == value) {
+ return true;
+ }
+ }
+ return false;
+ }
+
@Override
public Object deserialize(JsonParser p, DeserializationContext ctxt)
throws IOException {
if (!ParameterAuthorizationContext.isActive()) {
return super.deserialize(p, ctxt);
}
ParameterAuthorizationContext.pushRedactionScope();
+ JsonToken entry = p.currentToken();
+ JsonStreamContext value = structuredValue(p);
boolean swallowed = false;
try {
try {
@@ -102,6 +162,15 @@ final class RedactionAwareDeserializer extends
DelegatingDeserializer {
if
(!ParameterAuthorizationContext.wasRedactedInCurrentScope()) {
throw e;
}
+ if (!canResync(p, entry, value)) {
+ // The nearest enclosing object that can leave its parser
in order drops itself.
+ LOG.warn("REST body object of type [{}] failed to
construct after @StrutsParameter " +
+ "redaction dropped one of its properties
and cannot be dropped in place; " +
+ "leaving it to the enclosing object: {}",
+ handledType() != null ? handledType().getName() :
"?", e.getMessage());
+ swallowed = true;
+ throw e;
+ }
// If this object had a property redacted AND also hit an
unrelated mapping error,
// the two are indistinguishable here, so the unrelated error
is folded into
// "object dropped". This is deliberately fail-closed: we
never expose a
@@ -109,6 +178,7 @@ final class RedactionAwareDeserializer extends
DelegatingDeserializer {
LOG.warn("REST body object of type [{}] failed to construct
after @StrutsParameter " +
"redaction dropped one of its properties;
treating the object as unauthorized: {}",
handledType() != null ? handledType().getName() : "?",
e.getMessage());
+ skipToEndOf(p, value);
swallowed = true;
return null;
}
diff --git
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
index 7edc42b20..c0837a8e4 100644
---
a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
+++
b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java
@@ -34,6 +34,7 @@ import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
@@ -689,6 +690,116 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
assertNull("id member authorized by the referring bean's grant for
[child.k] ?", result.child.id.k);
}
+ public void testDroppedObjectLeavesTheParserAtItsEnd_nullObjectId() throws
Exception {
+ // The id bean fails on a redacted member, so the null id fails
Jackson's binding mid-object;
+ // the dropped object's remaining fields must not be read by the
parent.
+ Set<String> granted = Set.of("child", "child.id", "child.name",
"other", "name");
+ bind((path, t, a) -> granted.contains(path), new StrictHolder());
+ StrictHolder result = mapper.readValue(
+
"{\"child\":{\"id\":{\"k\":\"a\"},\"name\":\"x\"},\"other\":\"o\",\"name\":\"h\"}",
+ StrictHolder.class);
+ assertNull(result.child);
+ assertEquals("o", result.other);
+ assertEquals("child's name read into the parent ?", "h", result.name);
+ }
+
+ public void testDroppedObjectLeavesTheParserAtItsEnd_creatorFailure()
throws Exception {
+ // A creator with one parameter constructs as soon as it arrives; a
redacted primitive fails it
+ // with the rest of the object, including a nested one, still unread.
+ mapper.configure(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES,
true);
+ Set<String> granted = Set.of("money", "money.note", "money.name",
"other", "name");
+ bind((path, t, a) -> granted.contains(path), new StrictHolder());
+ StrictHolder result = mapper.readValue(
+
"{\"money\":{\"amount\":5,\"detail\":{\"x\":1},\"name\":\"x\"},\"other\":\"o\",\"name\":\"h\"}",
+ StrictHolder.class);
+ assertNull(result.money);
+ assertEquals("o", result.other);
+ assertEquals("h", result.name);
+ }
+
+ public void testDroppedPolymorphicObjectWithLateTypeIdFailsTheRead()
throws Exception {
+ // With the type id after other keys Jackson reads the subtype from a
parser spliced from a
+ // buffer and the real parser; the value straddles the splice, so its
drop cannot resync and
+ // is not swallowed. Read into an existing bean as the handlers do, so
nothing above it can
+ // drop itself instead.
+ mapper.configure(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES,
true);
+ Set<String> granted = Set.of("pet", "pet.name");
+ bind((path, t, a) -> granted.contains(path), new StrictHolder());
+ try {
+ mapper.readerForUpdating(new StrictHolder())
+
.readValue("{\"pet\":{\"legs\":4,\"@type\":\"strict\",\"name\":\"x\"},\"name\":\"h\"}");
+ fail("a drop that cannot resync the parser must fail the read");
+ } catch (JsonMappingException expected) {
+ assertTrue(expected.getMessage(),
expected.getMessage().contains("legs"));
+ }
+ }
+
+ public void testDroppedObjectNestedInALateTypeIdValueStillResyncs() throws
Exception {
+ // A bean nested inside the spliced value lies wholly on one side of
the splice.
+ Set<String> granted = Set.of("pet", "pet.legs", "pet.name",
"pet.owner", "name");
+ bind((path, t, a) -> granted.contains(path), new StrictHolder());
+ StrictHolder result = mapper.readValue(
+
"{\"pet\":{\"legs\":4,\"@type\":\"strict\",\"owner\":{\"k\":\"a\"},\"name\":\"x\"},\"name\":\"h\"}",
+ StrictHolder.class);
+ StrictPet pet = (StrictPet) result.pet;
+ assertNull(pet.owner);
+ assertEquals("x", pet.name);
+ assertEquals("h", result.name);
+ }
+
+ public void
testDroppedLateTypeIdValueNestedInAnotherLateTypeIdValueDoesNotLeak() throws
Exception {
+ // The inner splice is based on the outer value's buffer, so its root
context has a buffer
+ // parent; it still straddles its own splice and must be refused,
escalating the drop.
+ mapper.configure(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES,
true);
+ Set<String> granted = Set.of("mid", "mid.pet", "mid.pet.name",
"mid.pet.owner", "mid.name", "name");
+ bind((path, t, a) -> granted.contains(path), new PolyOuter());
+ PolyOuter result = mapper.readValue(
+
"{\"mid\":{\"pet\":{\"legs\":4,\"@type\":\"strict\",\"owner\":{\"k\":\"a\"},\"name\":\"pet\"},"
+ +
"\"name\":\"midname\",\"@type\":\"mid\"},\"name\":\"outer\"}",
+ PolyOuter.class);
+ assertNull("a drop that cannot resync must escalate, not leak the
pet's fields into mid", result);
+ }
+
+ public void testRefusedResyncDropsTheNearestEnclosingObject() throws
Exception {
+ // The parent had no redaction of its own; the refused drop below it
must still mark it.
+ mapper.configure(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES,
true);
+ Set<String> granted = Set.of("mid", "mid.secret", "mid.pet",
"mid.pet.name", "name");
+ bind((path, t, a) -> granted.contains(path), new StrictOuter());
+ StrictOuter result = mapper.readValue(
+
"{\"mid\":{\"secret\":\"s\",\"pet\":{\"legs\":4,\"@type\":\"strict\",\"name\":\"pet\"}},\"name\":\"outer\"}",
+ StrictOuter.class);
+ assertNull(result.mid);
+ assertEquals("outer", result.name);
+ }
+
+ public void
testDroppedObjectUnderADroppedPolymorphicChildLeavesTheParserAtItsEnd() throws
Exception {
+ // Jackson clears the parser's current token before splicing for a
late type id; when the
+ // subtype's drop is refused and the parent's own wrapper swallows, it
must still resync.
+ mapper.configure(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES,
true);
+ Set<String> granted = Set.of("mid", "mid.pet", "mid.pet.name", "name");
+ bind((path, t, a) -> granted.contains(path), new StrictOuter());
+ StrictOuter result = mapper.readValue(
+
"{\"mid\":{\"secret\":\"s\",\"pet\":{\"legs\":4,\"@type\":\"strict\",\"name\":\"pet\"}},\"name\":\"outer\"}",
+ StrictOuter.class);
+ assertNull(result.mid);
+ assertEquals("dropped pet's name read into the outer bean ?", "outer",
result.name);
+ }
+
+ public void testDroppedObjectInsideAnUnwrappedReplayStopsAtItsEnd() throws
Exception {
+ // Unwrapped properties are replayed from a token buffer whose
contexts carry no nesting
+ // depth; a drop there must stop at the object's end, not drain the
buffer.
+ Set<String> granted = Set.of("inner", "inner.keys", "inner.name",
"other");
+ bind((path, t, a) -> granted.contains(path), new UnwrappedHolder());
+ UnwrappedHolder result = mapper.readValue(
+
"{\"keys\":[{\"k\":\"a\"},{\"k\":\"b\"}],\"name\":\"n\",\"other\":\"o\"}",
+ UnwrappedHolder.class);
+ assertNotNull("unwrapped bean dropped because the replay was drained
?", result.inner);
+ assertEquals(2, result.inner.keys.size());
+ assertNull(result.inner.keys.get(0));
+ assertEquals("n", result.inner.name);
+ assertEquals("o", result.other);
+ }
+
public void testCreatorBoundObjectIdIsAssignedByTheCreatorOnly() throws
Exception {
// Jackson skips the post-construction write of a creator-bound id
(records have no setter
// for it); the wrapper must keep that skip and leave the id to the
authorized creator path.
@@ -1104,6 +1215,89 @@ public class ParameterAuthorizingModuleTest extends
TestCase {
public record IdentifiedRecord(int id, String name) {
}
+ public record StrictKey(String k) {
+ public StrictKey {
+ java.util.Objects.requireNonNull(k);
+ }
+ }
+
+ @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class,
property = "id")
+ public static class StrictIdentified {
+ public StrictKey id;
+ public String name;
+ }
+
+ public static class StrictMoney {
+ public final int amount;
+ public Address detail;
+ public String name;
+
+ @JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
+ public StrictMoney(@JsonProperty("amount") int amount) {
+ this.amount = amount;
+ }
+ }
+
+ @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type")
+ @JsonSubTypes(@JsonSubTypes.Type(value = StrictPet.class, name = "strict"))
+ public abstract static class StrictAnimal {
+ }
+
+ public static class StrictPet extends StrictAnimal {
+ public final int legs;
+ public StrictKey owner;
+ public String name;
+
+ @JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
+ public StrictPet(@JsonProperty("legs") int legs) {
+ this.legs = legs;
+ }
+ }
+
+ public static class StrictMid {
+ public String secret;
+ public StrictAnimal pet;
+ }
+
+ public static class StrictOuter {
+ public StrictMid mid;
+ public String name;
+ }
+
+ public static class UnwrappedInner {
+ public List<StrictKey> keys;
+ public String name;
+ }
+
+ public static class UnwrappedHolder {
+ @JsonUnwrapped
+ public UnwrappedInner inner;
+ public String other;
+ }
+
+ @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type")
+ @JsonSubTypes(@JsonSubTypes.Type(value = StrictMidPoly.class, name =
"mid"))
+ public abstract static class StrictContainer {
+ }
+
+ public static class StrictMidPoly extends StrictContainer {
+ public StrictAnimal pet;
+ public String name;
+ }
+
+ public static class PolyOuter {
+ public StrictContainer mid;
+ public String name;
+ }
+
+ public static class StrictHolder {
+ public StrictIdentified child;
+ public StrictMoney money;
+ public StrictAnimal pet;
+ public String other;
+ public String name;
+ }
+
public static class Key {
public String k;
