TextNode.equals() throws NullPointerException when TextNode constructed with null

[Javed6234]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

new TextNode(null) == new TextNode(null) returns false.
When debugging this issue, it actually causes an NPE in TextNode's equals method.

Version Information

Version: 2.13.5
Java 11
Spring Boot: 2.5.15

Reproduction

TextNode nodeOne = new TextNode(null);
TextNode nodeTwo = new TextNode(null);
nodeOne.equals(nodeTwo);

Expected behavior

I would expect this to return true as both values are equal

Additional context

No response

[pjfanning]

Does this happen if you use the equals method? That is the more correct way to check for value equality

[Javed6234]

@pjfanning Yeah, sorry, should have used equals as an example under Reproduction, I have updated it now
((TextNode) o).textValue().equals(this._value) in

public boolean equals(Object o) {
        if (o == this) {
            return true;
        } else if (o == null) {
            return false;
        } else {
            return o instanceof TextNode ? ((TextNode)o)._value.equals(this._value) : false;
        }
    }

causes the NPE

[pjfanning]

@Javed6234 see #4379

[Javed6234]

@pjfanning Lovely, thank you!

[pjfanning]

@Javed6234 Looking at the code in TextNode and equivalent classes like DecimalNode, it is clear that they were never intended to support nulls. The constructors on these classes are public but not really intended to be used by non-Jackson code. They are public to faciliatate modular Jackson code.

Why do you need to use new TextNode(null)?

@cowtowncoder in Jackson 3, we should probably throw an exception if some calls the TextNode/DecimalNode/BigIntegerNode constructors with null inputs. I would argue that it too late in Jackson 2 to make these constructors strict about null inputs. I have created #4379 and #4380 to work around immediate issues with equals and hashCode but users who somehow get null data into instances of these classes will likely get NullPointerExceptions elsewhere (eg DecimalMode doubleValue() method is not null safe and can't be made null safe).

[cowtowncoder]

Ok, so, no, this is NOT a supported use case. We should, I think, fail with exception on new TextNode(null).
Same goes for other Object-valued JsonNodes like you suggested.

But there is of course the pesky question of backwards compatibility, so I agree that it is probably too late to fix this in 2.x.

I do agree that no NPE should be thrown for equals() and hashCode().
And probably equals() should return true for those cases (even if that has minor backwards-compatibility implications).

Thank you @Javed6234 for reporting this problem.

[Javed6234]

My use case is that the string I'm passing into the TextNode can be null sometimes.
But I could just work around it by checking if the string is null before creating the TextNode.
Thanks @pjfanning @cowtowncoder for the quick fix!

[cowtowncoder]

For 2.x, then, null will be allowed and supported. For 3.0 we'll change that.

Thank you once again for reporting this @Javed6234 !