I had the crazy idea of re-implementing some of the syncthing BEP protocol in another language to experiment with a few features. In the process, I think I’ve discovered a subtle difference between the documented and implemented protocol - specifically, in the pre-authentication Hello message.
In the documentation, it states that the Length field is a uint16, whereas [the code appears to read/write uint32-s] (https://github.com/syncthing/syncthing/blob/master/lib/protocol/hello.go#L126).
Have I understood that correctly?
One minor piece of feedback for someone reading the protocol spec for the first time, it would be good to make it blindingly obvious that the int32/int16s are unsigned. I know it doesn’t make sense any other way, but I had a five minute moment of confusion.
I guess you’re right. Before the 0.14 protocol update it said:
Hello messages MUST be prefixed with a magic number 0x9F79BC40
represented in network byte order (BE), followed by 4 bytes representing the
size of the message in network byte order (BE), followed by the content of
the Hello message itself. The size of the contents of Hello message MUST be
less or equal to 1024 bytes.
So, for 0.13 and below the Length field is an unit32. Maybe @calmh intended to shrink the field to 16 bits for the upcoming 0.14 and forgot to change the code. Or the documentation is just wrong…
Yeah, that’s an implementation bug - it should be an int16 as stated in the specs. We should fix that before the 0.14 release. Thanks for noticing!
As for integer signedness, the spec explains:
Non protocol buffer integers are always represented in network byte order (i.e., big endian) and are signed unless stated otherwise, but when describing message lengths negative values do not make sense and the most significant bit MUST be zero.
You can see the int16 as an uint15 if you like, with the above note. The distinction is irrelevant other than imposing a practical 32 KiB limit on the hello and header messages. It stems from a general policy of keeping integers used for arithmetic signed. You’ll see the same in the actual protocol buffer messages where file size is a signed integer (but no negative sized files exist) etc.
Awesome; would you like a Github issue?
Thanks for the pointing out the salient part of the spec on signedness.
Yes please. I’ll write up the fix now.