WW-5726 fix(rest): decline the in-place merge of a polymorphic REST body property

plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingSettableBeanProperty.java

@@ -41,7 +41,9 @@
  * {@link #setAndReturn}, which apply the same authorization to the already-materialized value.</p>
  *
  * <p>When {@link ParameterAuthorizationContext#isActive()} is {@code false}, this wrapper is a
- * straight pass-through to the delegate — no overhead for default-config requests.</p>
+ * straight pass-through to the delegate — no overhead for default-config requests. The one decision
+ * taken regardless is made when the deserializer is built, not per request: a polymorphic property
+ * is never merged in place, see {@link AuthorizingValueDeserializer#supportsUpdate}.</p>
  *
  * @since 7.2.0
  */
@@ -79,13 +81,14 @@ protected SettableBeanProperty withDelegate(SettableBeanProperty d) {
      * parameters, never reach {@link #deserializeAndSet}/{@link #deserializeSetAndReturn}: Jackson calls
      * the {@code final} {@code SettableBeanProperty#deserialize} directly, through this property's own
      * value deserializer. Wrap that deserializer with {@link AuthorizingValueDeserializer} for every
-     * property; it owns the path push for nested members on both the direct and the buffered path.
+     * property; it owns the path push for nested members on both the direct and the buffered path,
+     * and refuses the in-place merge of a polymorphic value that would otherwise skip both.
      */
     @Override
     public SettableBeanProperty withValueDeserializer(JsonDeserializer<?> deser) {
         JsonDeserializer<?> effective = deser;
         if (!(deser instanceof AuthorizingValueDeserializer)) {
-            effective = new AuthorizingValueDeserializer(deser, memberName, getType());
+            effective = new AuthorizingValueDeserializer(deser, memberName, getType(), getValueTypeDeserializer());
         }
         return _with(delegate.withValueDeserializer(effective));
     }

plugins/rest/src/main/java/org/apache/struts2/rest/handler/jackson/AuthorizingValueDeserializer.java

@@ -19,6 +19,7 @@
 package org.apache.struts2.rest.handler.jackson;
 
 import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationConfig;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.JsonDeserializer;
@@ -44,16 +45,39 @@ final class AuthorizingValueDeserializer extends DelegatingDeserializer {
 
     private final String propertyName;
     private final JavaType propertyType;
+    private final transient TypeDeserializer valueTypeDeserializer;
 
-    AuthorizingValueDeserializer(JsonDeserializer<?> delegate, String propertyName, JavaType propertyType) {
+    AuthorizingValueDeserializer(JsonDeserializer<?> delegate, String propertyName, JavaType propertyType,
+                                 TypeDeserializer valueTypeDeserializer) {
         super(delegate);
         this.propertyName = propertyName;
         this.propertyType = propertyType;
+        this.valueTypeDeserializer = valueTypeDeserializer;
     }
 
     @Override
     protected JsonDeserializer<?> newDelegatingInstance(JsonDeserializer<?> newDelegatee) {
-        return new AuthorizingValueDeserializer(newDelegatee, propertyName, propertyType);
+        return new AuthorizingValueDeserializer(newDelegatee, propertyName, propertyType, valueTypeDeserializer);
+    }
+
+    /**
+     * Merging into a non-null polymorphic value is Jackson's one path past both wrappers: the
+     * {@code final} {@code SettableBeanProperty#deserializeWith} deserializes in place through a
+     * deserializer resolved for the existing value's class. {@code BeanDeserializerBase.resolve()}
+     * asks the value deserializer first, whichever property wrapper it has built by then, so
+     * declining keeps the property on the ordinary authorized path: the value is replaced and the
+     * body must carry its type id. Jackson ignores the declined merge under
+     * {@code MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE} and reports a bad definition otherwise.
+     */
+    @Override
+    public Boolean supportsUpdate(DeserializationConfig config) {
+        if (valueTypeDeserializer == null) {
+            return super.supportsUpdate(config);
+        }
+        LOG.warn("Merge disabled for polymorphic REST body property [{}]: an in-place polymorphic merge"
+                + " cannot be authorized, so the value is replaced and the body must carry its type id",
+                propertyName);
+        return Boolean.FALSE;
     }
 
     @Override

plugins/rest/src/test/java/org/apache/struts2/rest/handler/jackson/ParameterAuthorizingModuleTest.java

@@ -19,23 +19,29 @@
 package org.apache.struts2.rest.handler.jackson;
 
 import com.fasterxml.jackson.annotation.JsonAnySetter;
+import com.fasterxml.jackson.annotation.JsonBackReference;
 import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIdentityInfo;
+import com.fasterxml.jackson.annotation.JsonManagedReference;
 import com.fasterxml.jackson.annotation.JsonMerge;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonSubTypes;
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.annotation.JsonUnwrapped;
+import com.fasterxml.jackson.annotation.ObjectIdGenerators;
 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.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.PropertyNamingStrategies;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
 import com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder;
 import com.fasterxml.jackson.databind.deser.BeanDeserializerModifier;
 import com.fasterxml.jackson.databind.deser.SettableAnyProperty;
+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.XmlMapper;
@@ -584,6 +590,56 @@ public void testMergeIntoExistingValueIsAuthorized() throws Exception {
         assertNull("merged into an unauthorized property ?", result.address.city);
     }
 
+    public void testMergedPolymorphicPropertyWithInitialValueIsAuthorized() throws Exception {
+        // @JsonMerge into a non-null polymorphic value would otherwise deserialize in place through
+        // Jackson's typed merge, which reaches neither wrapper: the property is never authorized and
+        // the subtype's members are checked against the enclosing bean's grants.
+        bind((path, t, a) -> "owner".equals(path), new MergingKennel());
+        MergingKennel result = mapper.readValue("{\"pet\":{\"owner\":\"alice\"}}", MergingKennel.class);
+        assertNull("subtype member assigned through an unauthorized merged property ?",
+                ((Dog) result.pet).owner);
+    }
+
+    public void testMergedPolymorphicPropertyIsReplacedThroughTheAuthorizedPath() throws Exception {
+        MergingKennel bean = new MergingKennel();
+        Animal initial = bean.pet;
+        bind((path, t, a) -> "pet".equals(path) || "pet.owner".equals(path), bean);
+        mapper.readerForUpdating(bean).readValue("{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"}}");
+        assertEquals("alice", ((Dog) bean.pet).owner);
+        assertNotSame("merge must be disabled for a polymorphic property", initial, bean.pet);
+    }
+
+    public void testMergedPolymorphicPropertyBehindObjectIdWrapperIsAuthorized() throws Exception {
+        // @JsonIdentityInfo makes resolve() copy the property into an ObjectIdReferenceProperty
+        // before it decides on merging; the merge must still be refused there.
+        bind((path, t, a) -> "owner".equals(path), new MergingIdentifiedKennel());
+        MergingIdentifiedKennel result = mapper.readValue("{\"pet\":{\"owner\":\"alice\"}}",
+                MergingIdentifiedKennel.class);
+        assertNull("subtype member assigned through an unauthorized merged property ?",
+                ((IdentifiedDog) result.pet).owner);
+    }
+
+    public void testMergedPolymorphicPropertyBehindManagedReferenceWrapperIsAuthorized() throws Exception {
+        bind((path, t, a) -> "owner".equals(path), new MergingManagedKennel());
+        MergingManagedKennel result = mapper.readValue("{\"pet\":{\"owner\":\"alice\"}}",
+                MergingManagedKennel.class);
+        assertNull("subtype member assigned through an unauthorized merged property ?",
+                ((ManagedDog) result.pet).owner);
+    }
+
+    public void testMergedPolymorphicPropertyIsRefusedWhenUnmergeableIsNotIgnored() throws Exception {
+        ObjectMapper strict = new ObjectMapper()
+                .disable(MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE)
+                .registerModule(new ParameterAuthorizingModule());
+        bind((path, t, a) -> true, new MergingKennel());
+        try {
+            strict.readValue("{\"pet\":{\"@type\":\"dog\",\"owner\":\"alice\"}}", MergingKennel.class);
+            fail("a polymorphic merge the module cannot authorize must be reported as a bad definition");
+        } catch (InvalidDefinitionException expected) {
+            assertTrue(expected.getMessage(), expected.getMessage().contains("cannot be merged"));
+        }
+    }
+
     public void testBufferedSetterInsideDynamicKeyScopeIsAuthorizedByDepth() throws Exception {
         // Inside a dynamic-key scope the buffered path must consult the same depth rule as the
         // direct path, not the annotation authorizer (which rejects everything here).
@@ -816,6 +872,46 @@ public static class MergingBean {
         public Address address = new Address();
     }
 
+    public static class MergingKennel {
+        @JsonMerge
+        public Animal pet = new Dog();
+        public String owner;
+    }
+
+    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type")
+    @JsonSubTypes(@JsonSubTypes.Type(value = IdentifiedDog.class, name = "dog"))
+    @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id")
+    public abstract static class IdentifiedAnimal {
+    }
+
+    public static class IdentifiedDog extends IdentifiedAnimal {
+        public String owner;
+    }
+
+    public static class MergingIdentifiedKennel {
+        @JsonMerge
+        public IdentifiedAnimal pet = new IdentifiedDog();
+        public String owner;
+    }
+
+    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "@type")
+    @JsonSubTypes(@JsonSubTypes.Type(value = ManagedDog.class, name = "dog"))
+    public abstract static class ManagedAnimal {
+        @JsonBackReference
+        public MergingManagedKennel kennel;
+    }
+
+    public static class ManagedDog extends ManagedAnimal {
+        public String owner;
+    }
+
+    public static class MergingManagedKennel {
+        @JsonMerge
+        @JsonManagedReference
+        public ManagedAnimal pet = new ManagedDog();
+        public String owner;
+    }
+
     public static class CreatorHolder {
         public final CreatorWithSetter inner;