facebook/fbthrift

Compact protocol specification off-by-one error

terrelln opened this issue · 2 comments

The compact protocol specification seems to have a slight error in the definition of list/set encoding:

https://github.com/facebook/fbthrift/blob/main/thrift/doc/spec/protocol/data.md#listset-1

Specifically this paragraph:

Lists and sets are serialized in exactly the same way. The first byte contains the length (if less than 16) and the type code of the inner elements. If the length is at least 16, then it is encoded as a variable length integer. The serialized elements come next. First, the type code of the inner elements is written, then the 32-bit length, then the elements.

It states that a variable length integer if the length is at least 16, but the variable length integer must also be present if the length is 15. It would just be zero.

Good catch, thanks. I plan to change this part of the spec to the following:

Lists and sets are serialized as follows. If the length is less than 15, then the first byte contains the length and the element type code. If the length is 15 or more, then the first byte contain 0xF and the element type code followed by the length serialized as a varint-encoded 32-bit integer. Serialized elements come next.

Note that we write the length as varint, not 0, in case it is equal to 15.

Awesome, thanks for the fix!