Fix #2567 (wrong target type for failed null check)

cowtowncoder · cowtowncoder · commit f760b6d850ac · 2019-12-09T20:51:01.000-08:00
diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -984,6 +984,8 @@ João Guerra (joca-bt@github)
   * Reported #2473: Array index missing in path of `JsonMappingException` for `Collection<String>`,
     with custom deserializer
   (2.10.1)
+  * Reported #2567: Incorrect target type for arrays when providing nulls and nulls are disabled
+  (2.10.2)
 
 Ryan Bohn (bohnman@github)
   * Reported #2475: `StringCollectionSerializer` calls `JsonGenerator.setCurrentValue(value)`,
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -14,6 +14,8 @@ Project: jackson-databind
  (reported by Fabian L)
 #2560: Check `WRAP_EXCEPTIONS` in `CollectionDeserializer.handleNonArray()`
  (reported by Stefan W)
+#2567: Incorrect target type for arrays when providing nulls and nulls are disabled
+ (reported by João G)
 
 2.10.1 (09-Nov-2019)
 
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/NullsFailProvider.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/NullsFailProvider.java
@@ -23,7 +23,12 @@ protected NullsFailProvider(PropertyName name, JavaType type) {
     }
 
     public static NullsFailProvider constructForProperty(BeanProperty prop) {
-        return new NullsFailProvider(prop.getFullName(), prop.getType());
+        return constructForProperty(prop, prop.getType());
+    }
+
+    // @since 2.10.2
+    public static NullsFailProvider constructForProperty(BeanProperty prop, JavaType type) {
+        return new NullsFailProvider(prop.getFullName(), type);
     }
 
     public static NullsFailProvider constructForRootValue(JavaType t) {
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/PrimitiveArrayDeserializers.java
@@ -110,9 +110,11 @@ public JsonDeserializer<?> createContextual(DeserializationContext ctxt,
             nuller = NullsConstantProvider.skipper();
         } else if (nullStyle == Nulls.FAIL) {
             if (property == null) {
-                nuller = NullsFailProvider.constructForRootValue(ctxt.constructType(_valueClass));
+                // 09-Dec-2019, tatu: [databind#2567] need to ensure correct target type
+                nuller = NullsFailProvider.constructForRootValue(ctxt.constructType(_valueClass.getComponentType()));
             } else {
-                nuller = NullsFailProvider.constructForProperty(property);
+                // 09-Dec-2019, tatu: [databind#2567] need to ensure correct target type
+                nuller = NullsFailProvider.constructForProperty(property, property.getType().getContentType());
             }
         }
         if ((unwrapSingle == _unwrapSingle) && (nuller == _nuller)) {
@@ -198,29 +200,7 @@ public T deserialize(JsonParser p, DeserializationContext ctxt, T existing) thro
     /* Helper methods for sub-classes
     /********************************************************
      */
-    
-    /*
-     * Convenience method that constructs a concatenation of two arrays,
-     * with the type they have.
-     *
-     * @since 2.9
-    @SuppressWarnings("unchecked")
-    public static <T> T concatArrays(T array1, T array2)
-    {
-        int len1 = Array.getLength(array1);
-        if (len1 == 0) {
-            return array2;
-        }
-        int len2 = Array.getLength(array2);
-        if (len2 == 0) {
-            return array1;
-        }
-        Object result = Arrays.copyOf((Object[]) array1, len1 + len2);
-        System.arraycopy(array2, 0, result, len1, len2);
-        return (T) result;
-    }
-    */
-    
+
     @SuppressWarnings("unchecked")
     protected T handleNonArray(JsonParser p, DeserializationContext ctxt) throws IOException
     {
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdDeserializer.java
@@ -1098,6 +1098,20 @@ protected NullValueProvider findContentNullProvider(DeserializationContext ctxt,
         if (nulls == Nulls.SKIP) {
             return NullsConstantProvider.skipper();
         }
+        // 09-Dec-2019, tatu: [databind#2567] need to ensure correct target type (element,
+        //    not container), so inlined here before calling _findNullProvider
+        if (nulls == Nulls.FAIL) {
+            if (prop == null) {
+                JavaType type = ctxt.constructType(valueDeser.handledType());
+                // should always be container? But let's double-check just in case:
+                if (type.isContainerType()) {
+                    type = type.getContentType();
+                }
+                return NullsFailProvider.constructForRootValue(type);
+            }
+            return NullsFailProvider.constructForProperty(prop, prop.getType().getContentType());
+        }
+
         NullValueProvider prov = _findNullProvider(ctxt, prop, nulls, valueDeser);
         if (prov != null) {
             return prov;
diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsForContentTest.java b/src/test/java/com/fasterxml/jackson/databind/deser/filter/NullConversionsForContentTest.java
@@ -62,6 +62,7 @@ public void testFailOnNullFromDefaults() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"values\"");
+            assertEquals(String.class, e.getTargetType());
         }
 
         // or configured for type:
@@ -73,6 +74,7 @@ public void testFailOnNullFromDefaults() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"values\"");
+            assertEquals(String.class, e.getTargetType());
         }
     }
     
@@ -96,6 +98,7 @@ public void testFailOnNullWithCollections() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(Integer.class, e.getTargetType());
         }
 
         // List<String>
@@ -104,6 +107,7 @@ public void testFailOnNullWithCollections() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(String.class, e.getTargetType());
         }
     }
 
@@ -116,6 +120,7 @@ public void testFailOnNullWithArrays() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(Object.class, e.getTargetType());
         }
 
         // String[]
@@ -124,6 +129,7 @@ public void testFailOnNullWithArrays() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(String.class, e.getTargetType());
         }
     }
 
@@ -137,20 +143,23 @@ public void testFailOnNullWithPrimitiveArrays() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(Boolean.TYPE, e.getTargetType());
         }
         // int[]
         try {
             MAPPER.readValue(JSON, new TypeReference<NullContentFail<int[]>>() { });
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(Integer.TYPE, e.getTargetType());
         }
         // double[]
         try {
             MAPPER.readValue(JSON, new TypeReference<NullContentFail<double[]>>() { });
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(Double.TYPE, e.getTargetType());
         }
     }
 
@@ -163,6 +172,7 @@ public void testFailOnNullWithMaps() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(String.class, e.getTargetType());
         }
 
         // Then: EnumMap<Enum,String>
@@ -172,6 +182,7 @@ public void testFailOnNullWithMaps() throws Exception
             fail("Should not pass");
         } catch (InvalidNullException e) {
             verifyException(e, "property \"noNulls\"");
+            assertEquals(String.class, e.getTargetType());
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,12 @@ protected NullsFailProvider(PropertyName name, JavaType type) {`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`public static NullsFailProvider constructForProperty(BeanProperty prop) {`
`26`		`- return new NullsFailProvider(prop.getFullName(), prop.getType());`
	`26`	`+ return constructForProperty(prop, prop.getType());`
	`27`	`+ }`
	`28`	`+`
	`29`	`+ // @since 2.10.2`
	`30`	`+ public static NullsFailProvider constructForProperty(BeanProperty prop, JavaType type) {`
	`31`	`+ return new NullsFailProvider(prop.getFullName(), type);`
`27`	`32`	`}`
`28`	`33`
`29`	`34`	`public static NullsFailProvider constructForRootValue(JavaType t) {`
Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ public void testFailOnNullFromDefaults() throws Exception`
`62`	`62`	`fail("Should not pass");`
`63`	`63`	`} catch (InvalidNullException e) {`
`64`	`64`	`verifyException(e, "property \"values\"");`
	`65`	`+ assertEquals(String.class, e.getTargetType());`
`65`	`66`	`}`
`66`	`67`
`67`	`68`	`// or configured for type:`
`@@ -73,6 +74,7 @@ public void testFailOnNullFromDefaults() throws Exception`
`73`	`74`	`fail("Should not pass");`
`74`	`75`	`} catch (InvalidNullException e) {`
`75`	`76`	`verifyException(e, "property \"values\"");`
	`77`	`+ assertEquals(String.class, e.getTargetType());`
`76`	`78`	`}`
`77`	`79`	`}`
`78`	`80`
`@@ -96,6 +98,7 @@ public void testFailOnNullWithCollections() throws Exception`
`96`	`98`	`fail("Should not pass");`
`97`	`99`	`} catch (InvalidNullException e) {`
`98`	`100`	`verifyException(e, "property \"noNulls\"");`
	`101`	`+ assertEquals(Integer.class, e.getTargetType());`
`99`	`102`	`}`
`100`	`103`
`101`	`104`	`// List<String>`
`@@ -104,6 +107,7 @@ public void testFailOnNullWithCollections() throws Exception`
`104`	`107`	`fail("Should not pass");`
`105`	`108`	`} catch (InvalidNullException e) {`
`106`	`109`	`verifyException(e, "property \"noNulls\"");`
	`110`	`+ assertEquals(String.class, e.getTargetType());`
`107`	`111`	`}`
`108`	`112`	`}`
`109`	`113`
`@@ -116,6 +120,7 @@ public void testFailOnNullWithArrays() throws Exception`
`116`	`120`	`fail("Should not pass");`
`117`	`121`	`} catch (InvalidNullException e) {`
`118`	`122`	`verifyException(e, "property \"noNulls\"");`
	`123`	`+ assertEquals(Object.class, e.getTargetType());`
`119`	`124`	`}`
`120`	`125`
`121`	`126`	`// String[]`
`@@ -124,6 +129,7 @@ public void testFailOnNullWithArrays() throws Exception`
`124`	`129`	`fail("Should not pass");`
`125`	`130`	`} catch (InvalidNullException e) {`
`126`	`131`	`verifyException(e, "property \"noNulls\"");`
	`132`	`+ assertEquals(String.class, e.getTargetType());`
`127`	`133`	`}`
`128`	`134`	`}`
`129`	`135`
`@@ -137,20 +143,23 @@ public void testFailOnNullWithPrimitiveArrays() throws Exception`
`137`	`143`	`fail("Should not pass");`
`138`	`144`	`} catch (InvalidNullException e) {`
`139`	`145`	`verifyException(e, "property \"noNulls\"");`
	`146`	`+ assertEquals(Boolean.TYPE, e.getTargetType());`
`140`	`147`	`}`
`141`	`148`	`// int[]`
`142`	`149`	`try {`
`143`	`150`	`MAPPER.readValue(JSON, new TypeReference<NullContentFail<int[]>>() { });`
`144`	`151`	`fail("Should not pass");`
`145`	`152`	`} catch (InvalidNullException e) {`
`146`	`153`	`verifyException(e, "property \"noNulls\"");`
	`154`	`+ assertEquals(Integer.TYPE, e.getTargetType());`
`147`	`155`	`}`
`148`	`156`	`// double[]`
`149`	`157`	`try {`
`150`	`158`	`MAPPER.readValue(JSON, new TypeReference<NullContentFail<double[]>>() { });`
`151`	`159`	`fail("Should not pass");`
`152`	`160`	`} catch (InvalidNullException e) {`
`153`	`161`	`verifyException(e, "property \"noNulls\"");`
	`162`	`+ assertEquals(Double.TYPE, e.getTargetType());`
`154`	`163`	`}`
`155`	`164`	`}`
`156`	`165`
`@@ -163,6 +172,7 @@ public void testFailOnNullWithMaps() throws Exception`
`163`	`172`	`fail("Should not pass");`
`164`	`173`	`} catch (InvalidNullException e) {`
`165`	`174`	`verifyException(e, "property \"noNulls\"");`
	`175`	`+ assertEquals(String.class, e.getTargetType());`
`166`	`176`	`}`
`167`	`177`
`168`	`178`	`// Then: EnumMap<Enum,String>`
`@@ -172,6 +182,7 @@ public void testFailOnNullWithMaps() throws Exception`
`172`	`182`	`fail("Should not pass");`
`173`	`183`	`} catch (InvalidNullException e) {`
`174`	`184`	`verifyException(e, "property \"noNulls\"");`
	`185`	`+ assertEquals(String.class, e.getTargetType());`
`175`	`186`	`}`
`176`	`187`	`}`
`177`	`188`