From cb63fc53f35697e0dae34e8cbd33743c00f909ee Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 15 Sep 2026 16:26:52 +0200 Subject: [PATCH] WW-5748 fix(rest): let Jackson XML's deserializer modifier see the bean deserializer JacksonXmlHandler registered ParameterAuthorizingModule on an XmlMapper whose constructor had already registered JacksonXmlModule. A module's deserializer modifier is inserted at the head of the list, so the authorizing modifier ran first and handed Jackson XML's modifier a RedactionAwareDeserializer, which fails its instanceof BeanDeserializerBase test: the XML wrapper that reads an unwrapped list (@JacksonXmlElementWrapper(useWrapping = false)) was never installed, and every such list failed to deserialize through the XML handler, authorization context or not, since the module was introduced. The handler now builds the XmlMapper without a module and registers JacksonXmlModule after ParameterAuthorizingModule, so the XML modifier runs first and the authorizing wrapper goes around its result. The module's Javadoc states the order for handlers that register it themselves. With Jackson XML's wrapper now inside the authorizing one, the per-property @JsonIdentityInfo reader rebuild in RedactionAwareDeserializer.createContextual (WW-5746) walks the delegating wrappers down to the bean. Jackson XML's wrapper cannot take a new delegatee, so it is rebuilt around the bean and contextualized with a null property, which recomputes its unwrapped names without building the id reader over again; other delegating wrappers get the rebuilt bean through replaceDelegatee. The wrapper only stays around a bean that has an unwrapped list, so the combination is exercised by a bean carrying both. jackson-dataformat-xml is optional for the plugin, so the class naming its wrapper is loaded only once it is known to be present. Co-Authored-By: Claude Opus 5 (1M context) --- .../rest/handler/JacksonXmlHandler.java | 7 +- .../jackson/ParameterAuthorizingModule.java | 4 +- .../jackson/RedactionAwareDeserializer.java | 32 ++++- .../handler/jackson/XmlWrapperSupport.java | 78 ++++++++++++ .../rest/handler/JacksonXmlHandlerTest.java | 119 ++++++++++++++++++ .../ParameterAuthorizingModuleTest.java | 16 ++- 6 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/XmlWrapperSupport.java diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java index a25b151aa6..cfeddee587 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/JacksonXmlHandler.java @@ -19,6 +19,8 @@ package org.apache.struts2.rest.handler; import com.fasterxml.jackson.databind.ObjectReader; +import com.fasterxml.jackson.dataformat.xml.JacksonXmlModule; +import com.fasterxml.jackson.dataformat.xml.XmlFactory; import com.fasterxml.jackson.dataformat.xml.XmlMapper; import org.apache.commons.lang3.BooleanUtils; import org.apache.struts2.ActionInvocation; @@ -44,8 +46,11 @@ public class JacksonXmlHandler implements AuthorizationAwareContentTypeHandler { private final ParameterAuthorizingModule parameterAuthorizingModule = new ParameterAuthorizingModule(); public JacksonXmlHandler() { - mapper = new XmlMapper(); + // Deserializer modifiers run in reverse registration order; Jackson XML's must see Jackson's + // own bean deserializer, so it is registered after the authorizing module. + mapper = new XmlMapper(new XmlFactory(), null); mapper.registerModule(parameterAuthorizingModule); + mapper.registerModule(new JacksonXmlModule()); } @Override diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java index 89ac2f2f27..b9aeae4b66 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModule.java @@ -43,7 +43,9 @@ * external name a {@code @JsonProperty} or naming strategy puts on the wire, since the authorizer * resolves the path against the member. * - *

Register this module once on each handler's mapper (e.g. in the constructor). All per-request + *

Register this module once on each handler's mapper (e.g. in the constructor), and before any + * format module whose deserializer modifier expects Jackson's own bean deserializer: modifiers run in + * reverse registration order, and this one wraps the bean deserializer it is given. All per-request * authorization state is read from the ThreadLocal context, so the module + mapper combination is * thread-safe and reusable across requests.

* 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 dd18355234..512410fd23 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 @@ -81,15 +81,35 @@ protected JsonDeserializer newDelegatingInstance(JsonDeserializer newDeleg public JsonDeserializer createContextual(DeserializationContext ctxt, BeanProperty property) throws JsonMappingException { JsonDeserializer contextual = super.createContextual(ctxt, property); - JsonDeserializer bean = ((DelegatingDeserializer) contextual).getDelegatee(); - if (bean instanceof BeanDeserializerBase beanDeserializer && beanDeserializer.getObjectIdReader() != null) { - ObjectIdReader reader = beanDeserializer.getObjectIdReader(); + JsonDeserializer delegatee = ((DelegatingDeserializer) contextual).getDelegatee(); + JsonDeserializer authorized = withAuthorizedObjectIdReader(ctxt, delegatee); + return authorized == delegatee ? contextual : new RedactionAwareDeserializer(authorized); + } + + /** + * The bean deserializer may sit under further delegating wrappers by then, so the rebuilt one + * is put back through them. + */ + private static JsonDeserializer withAuthorizedObjectIdReader(DeserializationContext ctxt, + JsonDeserializer deserializer) + throws JsonMappingException { + if (deserializer instanceof BeanDeserializerBase bean && bean.getObjectIdReader() != null) { + ObjectIdReader reader = bean.getObjectIdReader(); ObjectIdReader authorized = ParameterAuthorizingModule.authorizedObjectIdReader(reader); - if (authorized != reader) { - return new RedactionAwareDeserializer(beanDeserializer.withObjectIdReader(authorized)); + return authorized == reader ? bean : bean.withObjectIdReader(authorized); + } + if (deserializer instanceof DelegatingDeserializer delegating) { + JsonDeserializer inner = delegating.getDelegatee(); + JsonDeserializer authorized = withAuthorizedObjectIdReader(ctxt, inner); + if (authorized == inner) { + return delegating; + } + if (XmlWrapperSupport.isUnwrappedListWrapper(delegating)) { + return XmlWrapperSupport.rebuildAround(ctxt, (BeanDeserializerBase) authorized); } + return delegating.replaceDelegatee(authorized); } - return contextual; + return deserializer; } /** diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/XmlWrapperSupport.java b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/XmlWrapperSupport.java new file mode 100644 index 0000000000..42ff9e47ae --- /dev/null +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/XmlWrapperSupport.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.rest.handler.jackson; + +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.deser.BeanDeserializerBase; +import com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer; + +/** + * Jackson XML's wrapper for a bean with an unwrapped list cannot take a new delegatee; it recomputes + * itself from the bean it is given when contextualized. {@code jackson-dataformat-xml} is optional + * for the plugin, so the class that names it is only loaded once it is known to be present. + */ +final class XmlWrapperSupport { + + private static final boolean AVAILABLE = available(); + + private XmlWrapperSupport() { + // utility + } + + private static boolean available() { + try { + Class.forName("com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer", + false, XmlWrapperSupport.class.getClassLoader()); + return true; + } catch (ClassNotFoundException | LinkageError absent) { + return false; + } + } + + static boolean isUnwrappedListWrapper(JsonDeserializer deserializer) { + return AVAILABLE && Xml.isUnwrappedListWrapper(deserializer); + } + + /** + * A {@code null} property keeps the re-contextualization from building the bean's id reader + * over again; the property-specific state is already on the bean from its first one. + */ + static JsonDeserializer rebuildAround(DeserializationContext ctxt, BeanDeserializerBase bean) + throws JsonMappingException { + return Xml.rebuildAround(ctxt, bean); + } + + private static final class Xml { + + private Xml() { + } + + static boolean isUnwrappedListWrapper(JsonDeserializer deserializer) { + return deserializer instanceof WrapperHandlingDeserializer; + } + + @SuppressWarnings("unchecked") + static JsonDeserializer rebuildAround(DeserializationContext ctxt, BeanDeserializerBase bean) + throws JsonMappingException { + return (JsonDeserializer) new WrapperHandlingDeserializer(bean).createContextual(ctxt, null); + } + } +} diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/JacksonXmlHandlerTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/JacksonXmlHandlerTest.java index 69fc15380e..fd2bb52d53 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/handler/JacksonXmlHandlerTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/handler/JacksonXmlHandlerTest.java @@ -18,8 +18,15 @@ */ package org.apache.struts2.rest.handler; +import com.fasterxml.jackson.annotation.JsonIdentityInfo; +import com.fasterxml.jackson.annotation.ObjectIdGenerators; +import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper; +import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty; +import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlText; import org.apache.struts2.ActionInvocation; import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext; +import org.apache.struts2.interceptor.parameter.ParameterAuthorizer; import org.apache.struts2.mock.MockActionInvocation; import java.io.Reader; @@ -27,6 +34,8 @@ import java.io.StringWriter; import java.io.Writer; import java.util.Arrays; +import java.util.List; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; @@ -52,6 +61,21 @@ public void setUp() throws Exception { ai = new MockActionInvocation(); } + @Override + public void tearDown() throws Exception { + ParameterAuthorizationContext.unbind(); + super.tearDown(); + } + + private void bind(ParameterAuthorizer authorizer, Object target) { + ParameterAuthorizationContext.bind(authorizer, target, target); + } + + private T read(String body, T target) throws Exception { + handler.toObject(ai, new StringReader(body), target); + return target; + } + public void testObjectToXml() throws Exception { // given SimpleBean obj = new SimpleBean(); @@ -92,4 +116,99 @@ public void testXmlToObject() throws Exception { .containsExactly("Adam", "Ewa"); } + public void testUnwrappedListWithoutContext() throws Exception { + // Jackson XML's deserializer modifier must see Jackson's own bean deserializer, not the + // authorization wrapper, to install its unwrapped-list handling. + UnwrappedListBean bean = read("abn", + new UnwrappedListBean()); + assertEquals(List.of("a", "b"), bean.items); + assertEquals("n", bean.name); + } + + public void testUnwrappedListAuthorized() throws Exception { + Set granted = Set.of("items", "items[0]", "name"); + bind((path, t, a) -> granted.contains(path), new UnwrappedListBean()); + UnwrappedListBean bean = read("abn", + new UnwrappedListBean()); + assertEquals(List.of("a", "b"), bean.items); + assertEquals("n", bean.name); + } + + public void testUnwrappedListRejected() throws Exception { + Set granted = Set.of("name"); + bind((path, t, a) -> granted.contains(path), new UnwrappedListBean()); + UnwrappedListBean bean = read("abn", + new UnwrappedListBean()); + assertNull(bean.items); + assertEquals("n", bean.name); + } + + public void testXmlTextStillReads() throws Exception { + TextBean bean = read("xhello", new TextBean()); + assertEquals("hello", bean.text); + assertEquals("x", bean.attr); + } + + public void testSoleXmlTextWithAttributeReadsAndIsAuthorized() throws Exception { + // A sole text property next to an attribute goes through Jackson XML's text deserializer, + // which the reorder now installs; it must still write through the authorizing property. + TextAttributeBean bean = read("hello", new TextAttributeBean()); + assertEquals("hello", bean.text); + assertEquals("x", bean.attr); + + Set granted = Set.of("attr"); + bind((path, t, a) -> granted.contains(path), new TextAttributeBean()); + TextAttributeBean rejected = read("hello", new TextAttributeBean()); + assertNull(rejected.text); + assertEquals("x", rejected.attr); + } + + public void testBeanTypedObjectIdDeclaredOnTheReferencingPropertyAuthorizedUnderTheIdPath() throws Exception { + // The per-property reader is rebuilt in createContextual through the XML wrapper Jackson + // keeps around a bean with an unwrapped list, inside the redaction wrapper. + Set granted = Set.of("child", "child.id", "child.k", "child.name", "child.tags", "child.tags[0]"); + bind((path, t, a) -> granted.contains(path), new KeyIdentifiedHolder()); + KeyIdentifiedHolder holder = read( + "xalicet", + new KeyIdentifiedHolder()); + assertEquals("alice", holder.child.name); + assertEquals(List.of("t"), holder.child.tags); + assertNull("id member authorized by the referring bean's grant for [child.k] ?", holder.child.id.k); + } + + public static class UnwrappedListBean { + @JacksonXmlElementWrapper(useWrapping = false) + public List items; + public String name; + } + + public static class TextBean { + @JacksonXmlText + public String text; + public String attr; + } + + public static class TextAttributeBean { + @JacksonXmlText + public String text; + @JacksonXmlProperty(isAttribute = true) + public String attr; + } + + public static class Key { + public String k; + } + + public static class PlainKeyed { + public Key id; + public String k; + public String name; + @JacksonXmlElementWrapper(useWrapping = false) + public List tags; + } + + public static class KeyIdentifiedHolder { + @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id") + public PlainKeyed child; + } } 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 c0837a8e49..55c2ab8baa 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 @@ -47,6 +47,8 @@ import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.util.TokenBuffer; +import com.fasterxml.jackson.dataformat.xml.JacksonXmlModule; +import com.fasterxml.jackson.dataformat.xml.XmlFactory; import com.fasterxml.jackson.dataformat.xml.XmlMapper; import junit.framework.TestCase; import org.apache.struts2.interceptor.parameter.ParameterAuthorizationContext; @@ -264,8 +266,7 @@ public void testPropertyCreatorAnySetterPreservesFloatingPointValues() throws Ex public void testXmlAnySetterPreservesNumericTextRoundTrip() throws Exception { String number = "1.2345678901234567890123456789"; for (boolean useBigDecimal : new boolean[]{false, true}) { - XmlMapper xmlMapper = new XmlMapper(); - xmlMapper.registerModule(new ParameterAuthorizingModule(true)); + XmlMapper xmlMapper = enforcingXmlMapper(); xmlMapper.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, useBigDecimal); bind((path, t, a) -> false, new DynamicScalarAnySetterBean()); DynamicScalarAnySetterBean result = xmlMapper.readValue( @@ -408,8 +409,7 @@ public void testDynamicKeyScopeCleanAfterDeserialization() throws Exception { } public void testXmlAnySetterUsesSameOptIn() throws Exception { - XmlMapper xmlMapper = new XmlMapper(); - xmlMapper.registerModule(new ParameterAuthorizingModule(true)); + XmlMapper xmlMapper = enforcingXmlMapper(); bind((path, t, a) -> false, new DynamicScalarAnySetterBean()); DynamicScalarAnySetterBean allowed = xmlMapper.readValue( @@ -1040,6 +1040,14 @@ private ObjectMapper enforcingMapper() { return new ObjectMapper().registerModule(new ParameterAuthorizingModule(true)); } + /** Built the way {@code JacksonXmlHandler} builds its mapper: the XML module registered last. */ + private XmlMapper enforcingXmlMapper() { + XmlMapper xmlMapper = new XmlMapper(new XmlFactory(), null); + xmlMapper.registerModule(new ParameterAuthorizingModule(true)); + xmlMapper.registerModule(new JacksonXmlModule()); + return xmlMapper; + } + public static class Person { public String name; public String role;