Fixes for #138: Move null checking to end of the conditional branch (#…

…139)
FasterXML · Dec 19, 2023 · 8854403 · 8854403
1 parent 3b95db3
commit 8854403
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 9 deletions.
diff --git a/...java/com/fasterxml/jackson/datatype/guava/deser/GuavaImmutableCollectionDeserializer.java b/...java/com/fasterxml/jackson/datatype/guava/deser/GuavaImmutableCollectionDeserializer.java
@@ -64,18 +64,17 @@ protected T _deserializeContents(JsonParser p, DeserializationContext ctxt)
                     continue;
                 }
                 value = _resolveNullToValue(ctxt);
-                if (value == null) {
-                    if (value == null) {
-                        _tryToAddNull(p, ctxt, builder);
-                        continue;
-                    }
-                }
             } else if (typeDeser == null) {
                 value = valueDes.deserialize(p, ctxt);
             } else {
                 value = valueDes.deserializeWithType(p, ctxt, typeDeser);
             }
 
+            if (value == null) {
+                _tryToAddNull(p, ctxt, builder);
+                continue;
+            }
+
             builder.add(value);
         }
         // No class outside of the package will be able to subclass us,

diff --git a/guava/src/test/java/com/fasterxml/jackson/datatype/guava/fuzz/Fuzz138_65117Test.java b/guava/src/test/java/com/fasterxml/jackson/datatype/guava/fuzz/Fuzz138_65117Test.java
@@ -0,0 +1,31 @@
+package com.fasterxml.jackson.datatype.guava.fuzz;
+
+import org.junit.Assert;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.exc.MismatchedInputException;
+import com.fasterxml.jackson.datatype.guava.ModuleTestBase;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Unit tests for verifying the fixes for OSS-Fuzz issues
+ * work as expected
+ * (see [datatypes-collections#138]).
+ */
+public class Fuzz138_65117Test extends ModuleTestBase
+{
+    private final ObjectMapper MAPPER = mapperWithModule();
+
+    public void testOSSFuzzIssue65117() throws Exception
+    {
+        final TypeReference<?> ref = new TypeReference<ImmutableList<Integer>>() {};
+        MismatchedInputException e = Assert.assertThrows(
+                MismatchedInputException.class,
+            () ->  MAPPER.readValue("[\"\"s(", ref));
+        verifyException(e, "Guava `Collection` of type ");
+        verifyException(e, "does not accept `null` values");
+    }
+}
diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -125,7 +125,9 @@ Muhammad Khalikov (@mukham12)
    (2.17.0)
 
 Arthur Chan (@arthurscchan)
-  * Contributed #124: Some deserializers throw unexpected `NullPointerException`
+  * Contributed #124: (guava) Some deserializers throw unexpected `NullPointerException`
     when handling invalid input
    (2.17.0)
-
+  * Contributed #138: (guava) `GuavaCollectionDeserializer` still throws NPE in
+    some circumstances
+   (2.17.0)
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -22,9 +22,11 @@ Active Maintainers:
  (contributed by Muhammad K)
 #124: (guava) Some deserializers throw unexpected `NullPointerException` when
   handling invalid input
- (contibuted by Arthur C)
+ (contributed by Arthur C)
 #136 (guava) Fix for failing Guava `Optional` test
  (contributed by Muhammad K)
+#138 (guava) `GuavaCollectionDeserializer` still throws NPE in some circumstances
+ (contributed by Arthur C)
 
 2.16.0 (15-Nov-2023)