WW-5748 fix(rest): let Jackson XML's deserializer modifier see the bean deserializer

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

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.
  *
- * <p>Register this module once on each handler's mapper (e.g. in the constructor). All per-request
+ * <p>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.</p>
  *

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;
     }
 
     /**

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 <T> JsonDeserializer<T> 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 <T> JsonDeserializer<T> rebuildAround(DeserializationContext ctxt, BeanDeserializerBase bean)
+                throws JsonMappingException {
+            return (JsonDeserializer<T>) new WrapperHandlingDeserializer(bean).createContextual(ctxt, null);
+        }
+    }
+}

plugins/rest/src/test/java/org/apache/struts2/rest/handler/JacksonXmlHandlerTest.java

@@ -18,15 +18,24 @@
  */
 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;
 import java.io.StringReader;
 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> 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("<bean><items>a</items><items>b</items><name>n</name></bean>",
+                new UnwrappedListBean());
+        assertEquals(List.of("a", "b"), bean.items);
+        assertEquals("n", bean.name);
+    }
+
+    public void testUnwrappedListAuthorized() throws Exception {
+        Set<String> granted = Set.of("items", "items[0]", "name");
+        bind((path, t, a) -> granted.contains(path), new UnwrappedListBean());
+        UnwrappedListBean bean = read("<bean><items>a</items><items>b</items><name>n</name></bean>",
+                new UnwrappedListBean());
+        assertEquals(List.of("a", "b"), bean.items);
+        assertEquals("n", bean.name);
+    }
+
+    public void testUnwrappedListRejected() throws Exception {
+        Set<String> granted = Set.of("name");
+        bind((path, t, a) -> granted.contains(path), new UnwrappedListBean());
+        UnwrappedListBean bean = read("<bean><items>a</items><items>b</items><name>n</name></bean>",
+                new UnwrappedListBean());
+        assertNull(bean.items);
+        assertEquals("n", bean.name);
+    }
+
+    public void testXmlTextStillReads() throws Exception {
+        TextBean bean = read("<bean><attr>x</attr>hello</bean>", 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("<bean attr=\"x\">hello</bean>", new TextAttributeBean());
+        assertEquals("hello", bean.text);
+        assertEquals("x", bean.attr);
+
+        Set<String> granted = Set.of("attr");
+        bind((path, t, a) -> granted.contains(path), new TextAttributeBean());
+        TextAttributeBean rejected = read("<bean attr=\"x\">hello</bean>", 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<String> 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(
+                "<holder><child><id><k>x</k></id><name>alice</name><tags>t</tags></child></holder>",
+                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<String> 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<String> tags;
+    }
+
+    public static class KeyIdentifiedHolder {
+        @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
+        public PlainKeyed child;
+    }
 }

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;