jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

Resolving forward references of collections might result in corrupt behaviour.

Open NielsDoucet opened this issue 7 years ago • 4 comments

We have a curious case where removing items from a HashSet is no longer possible, if the set is deserialized by jackson using the bean deserializer (v2.8.6).

Testcase illustrating the issue: DeserializationTest.java.txt Note that testRemovalOfNestedSetElementsAfterDeserialization fails.

The main issue is the fact that we initialize the id field of the abstract base class with a random UUID through the constructor. This constructor is used by jackson to instantiate the objects, which is correct behaviour. What I think goes wrong is related to the following code fragment: https://github.com/HeadOnAPlate/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdValueProperty.java#L90-L95 I think it binds the item too soon, instead of waiting until the deserialization of the id value has fully completed, which in our case would overwrite the id on the current instance in the deserialization-context. This bind then adds the instance to the collection being deserialized, using the hashcode of the random id value instead of the proper id value. Any contains/remove check on the set will fail, as the calculated hashcode is different from the one used when adding the item to said set, and thus the bucket where the object should reside within the hashsets backing hashmap is missed (although it could randomly succeed based on pure luck putting both the old hashcode and the new hashcode inside the same bucket).

I did not test, but updating the idProp before trying to resolve it seems like the intuitive way to fix this.

I understand that initializing the id with a random value and relying on the setId later on to 'fix' the actual id value while also using the id as our hashcode, isn't clean But as @JsonCreator doesn't work well within class hierarchies, this seems like the cleanest code for our model.

NielsDoucet avatar Mar 07 '17 15:03 NielsDoucet