ekmett / bytes

Serialization primitives that work with both cereal and binary.
http://hackage.haskell.org/package/bytes
Other
22 stars 13 forks source link

Serialization of Integer fails for -1 #54

Open jprupp opened 3 years ago

jprupp commented 3 years ago

There are issues with this library and the default encoding of negative Integer.

Encoding with the cereal library:

λ :m + Data.Serialize Data.ByteString.Base16
λ let bs = encode (-1 :: Integer)
λ encodeBase16 bs
"00ffffffff"
λ decode bs :: Either String Integer
Right (-1)

Encoding with the bytes library:

λ let bs = runPutS $ serialize (-1 :: Integer)
λ encodeBase16 bs
"ff"
λ runGetS deserialize bs :: Either String Integer
Left "too few bytes\nFrom:\tdemandInput\n\n"
λ let bl = runPutL $ serialize (-1 :: Integer)
λ Data.ByteString.Lazy.Base16.encodeBase16 bl
"ff"
λ runGetL deserialize bl :: Integer
*** Exception: Data.Binary.Get.runGet at position 1: not enough bytes
CallStack (from HasCallStack):
  error, called at libraries/binary/src/Data/Binary/Get.hs:351:5 in binary-0.8.8.0:Data.Binary.Get
ekmett commented 1 year ago

This seems like a pretty big bug, TBH. I think we should definitely do a patch, which will necessarily change the format, so should require a major version bump. The question is what the right patch looks like. e.g. Transmit sign then data? Only for the Integer case? In general?

jprupp commented 1 year ago

Probably make it do the same that the Cereal library does.