keybase / saltpack

a modern crypto messaging format
https://saltpack.org/
BSD 3-Clause "New" or "Revised" License
989 stars 62 forks source link

Clarify MessagePack encoding in spec #60

Closed akalin-keybase closed 6 years ago

akalin-keybase commented 6 years ago

MessagePack has some ambiguity in encoding numbers, so clarify that the version numbers and mode should be encoded as positive fixnums.

Also clarify ambiguity re. encoding strings, bytes arrays, and arrays.

Also add tests.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 84.502% when pulling 9ba6280d6dc329a9668f8b46756bee4616439f7a on akalin/spec-int into 282df2166aa71f1283de12d39a10addbb30bafb4 on master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 84.502% when pulling 30d9f7a640197c7d6878b88c8791c468992e1487 on akalin/spec-int into 282df2166aa71f1283de12d39a10addbb30bafb4 on master.

maxtaco commented 6 years ago

You might also say that all encodings must use the minimal number of bytes. This might be more general. For instance isn’t it allowed to encode a one element array in several ways? Of course the smallest representation is the one we expect.

akalin-keybase commented 6 years ago

Ah, you're right! Let me rework this PR, then...

akalin-keybase commented 6 years ago

Okay, added some wording re. encoding strings, byte arrays, and arrays. Turns out we still have to specify int encodings, though -- in general, there are multiply ways to encode some ints minimally, so we have to at least clarify that the ints we have are small enough to fit in 1 byte. PTAL!