data.fressian icon indicating copy to clipboard operation
data.fressian copied to clipboard

WriteHandler incorrectly serializes maps of size 8 to PersistentHashMap instead of PersistentArrayMap

Open IngridMorstrad opened this issue 1 year ago • 2 comments

Line 189 in fressian.clj (link) is (if (< (.size kvs) 16), when it should be (if (<= (.size kvs) 16).

Important background knowledge: Clojure uses a PersistentArrayMap as the backing map if the size of the map is 8 or lower, and a PersistentHashMap otherwise.

The line of code above causes a map of size 8 to be deserialized as a PersistentHashMap. This means that when a map of size 8 is serialized, and then deserialized, the type of the map changes. Specifically, it starts off as a PersistentArrayMap, and the deserialization converts it into a PersistentHashMap. This also means that the second serialization (in a series of serialization -> deserialization -> serialization calls) will return a different value from the first one.

I can send a pull request to fix for this, but want to discuss if we should upgrade the fressian version, since making the change without a major version upgrade can cause existing clients to break (since serialization of the map of size 8 will lead to a different value with the change from < to <=).

IngridMorstrad avatar Nov 05 '24 05:11 IngridMorstrad

This also means that the second serialization (in a series of serialization -> deserialization -> serialization calls) will return a different value from the first one.

Different how? Field ordering?

rschmitt avatar Nov 05 '24 05:11 rschmitt

Yes, the serialized bytes are in a different order.

IngridMorstrad avatar Nov 05 '24 09:11 IngridMorstrad