FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.52k stars 1.38k forks source link

JsonNode#deepCopy ClassCast Exceptions #1829

Open D3v01dZA opened 6 years ago

D3v01dZA commented 6 years ago

The current signature of JsonNode#deepCopy looks like this:

public abstract <T extends JsonNode> T deepCopy();

I wondered about the decision to do this as you can quite easily create a ClassCastException using this as follows:

TextNode text = TextNode.valueOf("SomeText"); ObjectNode object = text.deepCopy();

This code is obviously incorrect just by looking at it however this will always compile and throw an exception at runtime. I know this is considered an API change but a signature that prevents this might be:

JsonNode.java public abstract JsonNode deepCopy();

And then be overridden on each subclass with the correct type

ValueNode.java public abstract ValueNode deepCopy();

After looking at this again, the signatures of ObjectNode and ArrayNode are correct but you can still trick it by upcasting to JsonNode and then using deepCopy()

cowtowncoder commented 6 years ago

Hmmh. Yes, it would be possible to use return type co-variance here. This could be done for Jackson 3.x (as it can not be done for 2.x; not something that can go in a patch, and 2.9 is the last 2.x minor version).

cowtowncoder commented 2 years ago

lol on "2.9 is the last". We are at 2.13 now... :)

Still, if done, this needs to go in 3.0.