Fix #2195 (last changes)

FasterXML · May 31, 2019 · 535b1dc · 535b1dc
1 parent e0cd19b
commit 535b1dc
Show file tree

Hide file tree

Showing 4 changed files with 175 additions and 6 deletions.
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -21,6 +21,8 @@ Project: jackson-databind
 #2187: Make `JsonNode.toString()` use shared `ObjectMapper` to produce valid json
 #2189: `TreeTraversingParser` does not check int bounds
  (reported by Alexander S)
+#2195: Add abstraction `PolymorphicTypeValidator`, for limiting subtypes allowed by
+  default typing, `@JsonTypeInfo`
 #2196: Type safety for `readValue()` with `TypeReference`
  (suggested by nguyenfilip@github)
 #2204: Add `JsonNode.isEmpty()` as convenience alias

diff --git a/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java b/src/main/java/com/fasterxml/jackson/databind/DatabindContext.java
@@ -274,7 +274,7 @@ private JavaType _resolveAndValidateGeneric(JavaType baseType, String subClass,
         }
         return subType;
     }
-    
+
     protected <T> T _throwNotASubtype(JavaType baseType, String subType) throws JsonMappingException {
         throw invalidTypeIdException(baseType, subType, "Not a subtype");
     }
@@ -290,7 +290,7 @@ protected <T> T _throwSubtypeClassNotAllowed(JavaType baseType, String subType,
         throw invalidTypeIdException(baseType, subType,
                 "Configured `PolymorphicTypeValidator` (of type "+ClassUtil.classNameOf(ptv)+") denied resolution");
     }
-    
+
     /**
      * Helper method for constructing exception to indicate that given type id
      * could not be resolved to a valid subtype of specified base type.

diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/BasicPolymorphicTypeValidator.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/BasicPolymorphicTypeValidator.java
@@ -111,11 +111,17 @@ public boolean match(Class<?> clazz) {
          * nominal base type's class name matches given {@link Pattern}
          * For example, call to
          *<pre>
-         *    builder.allowIfBaseType(Pattern.compile("com\\.mycompany\\.")
+         *    builder.allowIfBaseType(Pattern.compile("com\\.mycompany\\..*")
          *</pre>
          * would indicate that any polymorphic properties where declared base type
          * is in package {@code com.mycompany} would allow all legal (assignment-compatible)
          * subtypes.
+         *<p>
+         * NOTE! {@link Pattern} match is applied using
+         *<code>
+         *   if (patternForBase.matcher(typeId).matches()) { }
+         *</code>
+         * that is, it must match the whole class name, not just part.
          */
         public Builder allowIfBaseType(final Pattern patternForBase) {
             return _appendBaseMatcher(new TypeMatcher() {
@@ -197,6 +203,12 @@ public boolean match(Class<?> clazz) {
          *</pre>
          * would indicate that any polymorphic values in package {@code com.mycompany}
          * would be allowed.
+         *<p>
+         * NOTE! {@link Pattern} match is applied using
+         *<code>
+         *   if (patternForSubType.matcher(typeId).matches()) { }
+         *</code>
+         * that is, it must match the whole class name, not just part.
          */
         public Builder allowIfSubType(final Pattern patternForSubType) {
             return _appendSubNameMatcher(new NameMatcher() {

diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/vld/BasicPTVTest.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/vld/BasicPTVTest.java
@@ -1,7 +1,10 @@
 package com.fasterxml.jackson.databind.jsontype.vld;
 
+import java.util.regex.Pattern;
+
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping;
+import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
 import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
 import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator;
 import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator;
@@ -71,10 +74,11 @@ protected NumberWrapper() { }
 
     /*
     /**********************************************************************
-    /* Test methods: annotated
+    /* Test methods: by base type, pass
     /**********************************************************************
      */
 
+    // First: test simple Base-type-as-class allowing
     public void testAllowByBaseClass() throws Exception {
         final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
                 .allowIfBaseType(BaseValue.class)
@@ -94,7 +98,7 @@ public void testAllowByBaseClass() throws Exception {
             mapper.readValue(json2, NumberWrapper.class);
             fail("Should not pass");
         } catch (InvalidTypeIdException e) {
-            verifyException(e, "Could not resolve type id `java.lang.Byte`");
+            verifyException(e, "Could not resolve type id 'java.lang.Byte'");
             verifyException(e, "as a subtype of");
         }
 
@@ -104,9 +108,160 @@ public void testAllowByBaseClass() throws Exception {
                         .allowIfBaseType(Number.class)
                         .build(), DefaultTyping.NON_FINAL)
                 .build();
-        NumberWrapper nw = mapper2.readValue(json, NumberWrapper.class);
+        NumberWrapper nw = mapper2.readValue(json2, NumberWrapper.class);
         assertNotNull(nw);
         assertEquals(Byte.valueOf((byte) 4), nw.value);
     }
+
+    // Then subtype-prefix
+    public void testAllowByBaseClassPrefix() throws Exception {
+        final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
+                .allowIfBaseType("com.fasterxml.")
+                .build();
+        ObjectMapper mapper = jsonMapperBuilder()
+                .enableDefaultTyping(ptv, DefaultTyping.NON_FINAL)
+                .build();        
+
+        // First, test accepted case
+        final String json = mapper.writeValueAsString(BaseValueWrapper.withA(42));
+        BaseValueWrapper w = mapper.readValue(json, BaseValueWrapper.class);
+        assertEquals(42, w.value.x);
+
+        // then non-accepted
+        final String json2 = mapper.writeValueAsString(new NumberWrapper(Byte.valueOf((byte) 4)));
+        try {
+            mapper.readValue(json2, NumberWrapper.class);
+            fail("Should not pass");
+        } catch (InvalidTypeIdException e) {
+            verifyException(e, "Could not resolve type id 'java.lang.Byte'");
+            verifyException(e, "as a subtype of");
+        }
+    }
+
+    // Then subtype-pattern
+    public void testAllowByBaseClassPattern() throws Exception {
+        final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
+                .allowIfBaseType(Pattern.compile("\\w+\\.fasterxml\\..+"))
+                .build();
+        ObjectMapper mapper = jsonMapperBuilder()
+                .enableDefaultTyping(ptv, DefaultTyping.NON_FINAL)
+                .build();        
+
+        // First, test accepted case
+        final String json = mapper.writeValueAsString(BaseValueWrapper.withA(42));
+        BaseValueWrapper w = mapper.readValue(json, BaseValueWrapper.class);
+        assertEquals(42, w.value.x);
+
+        // then non-accepted
+        final String json2 = mapper.writeValueAsString(new NumberWrapper(Byte.valueOf((byte) 4)));
+        try {
+            mapper.readValue(json2, NumberWrapper.class);
+            fail("Should not pass");
+        } catch (InvalidTypeIdException e) {
+            verifyException(e, "Could not resolve type id 'java.lang.Byte'");
+            verifyException(e, "as a subtype of");
+        }
+    }
+
+    // And finally, block by specific direct-match base type
+    public void testDenyByBaseClass() throws Exception {
+        final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
+                // indicate that all subtypes `BaseValue` would be fine
+                .allowIfBaseType(BaseValue.class)
+                // but that nominal base type MUST NOT be `Object.class`
+                .denyForExactBaseType(Object.class)
+                .build();
+        ObjectMapper mapper = jsonMapperBuilder()
+                .enableDefaultTyping(ptv, DefaultTyping.NON_FINAL)
+                .build();        
+        final String json = mapper.writeValueAsString(new ObjectWrapper(new ValueA(15)));
+        try {
+            mapper.readValue(json, ObjectWrapper.class);
+            fail("Should not pass");
+
+        // NOTE: different exception type since denial was for whole property, not just specific values
+        } catch (InvalidDefinitionException e) {
+            verifyException(e, "denied resolution of all subtypes of base type `java.lang.Object`");
+        }
+    }
+
+    /*
+    /**********************************************************************
+    /* Test methods: by sub type
+    /**********************************************************************
+     */
+
+    public void testAllowBySubClass() throws Exception {
+        final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
+                .allowIfSubType(ValueB.class)
+                .build();
+        ObjectMapper mapper = jsonMapperBuilder()
+                .enableDefaultTyping(ptv, DefaultTyping.NON_FINAL)
+                .build();        
+
+        // First, test accepted case
+        final String json = mapper.writeValueAsString(BaseValueWrapper.withB(42));
+        BaseValueWrapper w = mapper.readValue(json, BaseValueWrapper.class);
+        assertEquals(42, w.value.x);
+
+        // then non-accepted
+        try {
+            mapper.readValue(mapper.writeValueAsString(BaseValueWrapper.withA(43)),
+                    BaseValueWrapper.class);
+            fail("Should not pass");
+        } catch (InvalidTypeIdException e) {
+            verifyException(e, "Could not resolve type id 'com.fasterxml.jackson.");
+            verifyException(e, "as a subtype of");
+        }
+    }
+
+    public void testAllowBySubClassPrefix() throws Exception {
+        final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
+                .allowIfSubType(ValueB.class.getName())
+                .build();
+        ObjectMapper mapper = jsonMapperBuilder()
+                .enableDefaultTyping(ptv, DefaultTyping.NON_FINAL)
+                .build();        
+
+        // First, test accepted case
+        final String json = mapper.writeValueAsString(BaseValueWrapper.withB(42));
+        BaseValueWrapper w = mapper.readValue(json, BaseValueWrapper.class);
+        assertEquals(42, w.value.x);
+
+        // then non-accepted
+        try {
+            mapper.readValue(mapper.writeValueAsString(BaseValueWrapper.withA(43)),
+                    BaseValueWrapper.class);
+            fail("Should not pass");
+        } catch (InvalidTypeIdException e) {
+            verifyException(e, "Could not resolve type id 'com.fasterxml.jackson.");
+            verifyException(e, "as a subtype of");
+        }
+    }
+
+    public void testAllowBySubClassPattern() throws Exception {
+        final PolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder()
+                .allowIfSubType(Pattern.compile(Pattern.quote(ValueB.class.getName())))
+                .build();
+        ObjectMapper mapper = jsonMapperBuilder()
+                .enableDefaultTyping(ptv, DefaultTyping.NON_FINAL)
+                .build();        
+
+        // First, test accepted case
+        final String json = mapper.writeValueAsString(BaseValueWrapper.withB(42));
+        BaseValueWrapper w = mapper.readValue(json, BaseValueWrapper.class);
+        assertEquals(42, w.value.x);
+
+        // then non-accepted
+        try {
+            mapper.readValue(mapper.writeValueAsString(BaseValueWrapper.withA(43)),
+                    BaseValueWrapper.class);
+            fail("Should not pass");
+        } catch (InvalidTypeIdException e) {
+            verifyException(e, "Could not resolve type id 'com.fasterxml.jackson.");
+            verifyException(e, "as a subtype of");
+        }
+    }
 }
 
+