Fix #2433

Author: cowtowncoder

release-notes/CREDITS-2.x

@@ -914,3 +914,7 @@ Vladimir Tsanev (tsachev@github)
   * Contributed #2415: Builder-based POJO deserializer should pass builder instance, not type,
     to `handleUnknownVanilla()` to fix earlier #822
    (2.10.0)
+
+David Becker (dsbecker@github)
+  * Suggested #2433: Improve `NullNode.equals()`
+   (2.10.0)

release-notes/VERSION-2.x

@@ -29,6 +29,8 @@ Project: jackson-databind
 #2425: Add global config override setting for `@JsonFormat.lenient()`
 #2428: Use "activateDefaultTyping" over "enableDefaultTyping" in 2.10 with new methods
 #2430: Change `ObjectMapper.valueToTree()` to convert `null` to `NullNode`
+#2433: Improve `NullNode.equals()`
+ (suggested by David B)
 
 2.10.0.pr1 (19-Jul-2019)
 

src/main/java/com/fasterxml/jackson/databind/JsonNode.java

@@ -1077,7 +1077,7 @@ protected <T extends JsonNode> T _this() {
 
     /**
      * Helper method that throws {@link IllegalArgumentException} as a result of
-     * violating "required-constraint" for this node (for {@link #require() or related
+     * violating "required-constraint" for this node (for {@link #required} or related
      * methods).
      */
     protected <T> T _reportRequiredViolation(String msgTemplate, Object...args) {

src/main/java/com/fasterxml/jackson/databind/node/NullNode.java

@@ -11,7 +11,7 @@
  * This singleton value class is used to contain explicit JSON null
  * value.
  */
-public final class NullNode
+public class NullNode
     extends ValueNode
 {
     // // Just need a fly-weight singleton
@@ -67,7 +67,11 @@ public final void serialize(JsonGenerator g, SerializerProvider provider)
 
     @Override
     public boolean equals(Object o) {
-        return (o == this);
+        // 29-Aug-2019, tatu: [databind#2433] Since custom sub-classes are allowed (bad idea probably),
+        //     need to do better comparison
+        if (o == this) return true;
+        if (!(o instanceof NullNode)) return false;
+        return ((NullNode) o).isNull();
     }
 
     @Override

src/test/java/com/fasterxml/jackson/databind/node/TestNullNode.java

@@ -17,6 +17,11 @@ final static class CovarianceBean {
         public void setArray(ArrayNode n) { _array = n; }
     }
 
+    @SuppressWarnings("serial")
+    static class MyNull extends NullNode { }
+    
+    private final ObjectMapper MAPPER = sharedMapper();
+    
     public void testBasicsWithNullNode() throws Exception
     {
         // Let's use something that doesn't add much beyond JsonNode base
@@ -59,7 +64,7 @@ public void testBasicsWithNullNode() throws Exception
     public void testNullHandling() throws Exception
     {
         // First, a stand-alone null
-        JsonNode n = objectReader().readTree("null");
+        JsonNode n = MAPPER.readTree("null");
         assertNotNull(n);
         assertTrue(n.isNull());
         assertFalse(n.isNumber());
@@ -81,21 +86,32 @@ public void testNullHandling() throws Exception
 
     public void testNullSerialization() throws Exception
     {
-        ObjectMapper mapper = new ObjectMapper();
         StringWriter sw = new StringWriter();
-        mapper.writeValue(sw, NullNode.instance);
+        MAPPER.writeValue(sw, NullNode.instance);
         assertEquals("null", sw.toString());
     }
 
     public void testNullHandlingCovariance() throws Exception
     {
         String JSON = "{\"object\" : null, \"array\" : null }";
-        CovarianceBean bean = objectMapper().readValue(JSON, CovarianceBean.class);
+        CovarianceBean bean = MAPPER.readValue(JSON, CovarianceBean.class);
 
         ObjectNode on = bean._object;
         assertNull(on);
 
         ArrayNode an = bean._array;
         assertNull(an);
     }
+
+    @SuppressWarnings("unlikely-arg-type")
+    public void testNullEquality() throws Exception
+    {
+        JsonNode n = MAPPER.nullNode();
+        assertTrue(n.isNull());
+        assertEquals(n, new MyNull());
+        assertEquals(new MyNull(), n);
+
+        assertFalse(n.equals(null));
+        assertFalse(n.equals("foo"));
+    }
 }