12 ms·

Add extra stuff to a “standard” encoding? Sure, why not

219 points by l0b0 3y ago

cratermoon 3y ago
What library was that, so I know to avoid it?
- upon_drumhead 3y ago
  KRPC perhaps https://krpc.github.io/krpc/communication-protocols/tcpip.html https://krpc.github.io/krpc/communication-protocols/tcpip.ht...
- loeg 3y ago
  https://github.com/protobufjs/protobuf.js/issues/987 https://github.com/protobufjs/protobuf.js/issues/987 maybe (based on "And yes, that string in this post is entirely deliberate").
  - cratermoon 3y ago
    Ah yes, you've got it.
  - cowsandmilk 3y ago
    Seems like protoduf.js has the exact same methods as Google’s implementations with the same names (encode/decode to not prepend with length, encodeDelimited/decodeDelimited for prepending length). It is hard for me to say they’re adding to the standard when they’re just replicating Google’s libraries. https://github.com/protobufjs/protobuf.js#toolset https://github.com/protobufjs/protobuf.js#toolset
kpw94 3y ago
> we started looking at this protobuf library he had selected, and sure enough, the author decided it was a good idea to prepend the message with the message length encoded as a varint. > WHY? Oh, why?! > And yes, it turns out that other people have noticed this anomaly. It's screwed up encoding and decoding in their projects, unsurprisingly. We found a (still-open) bug report from 2018, among others If anyone knows which library/language these issues the author is talking about are in, please tell us. I'd like to avoid that library if possible
- vlovich123 3y ago
  I feel like I recall seeing something similar in nanopb.
- loeg 3y ago
  > If anyone knows which library/language these issues the author is talking about are in, please tell us. https://github.com/protobufjs/protobuf.js/issues/987 https://github.com/protobufjs/protobuf.js/issues/987 maybe (based on "And yes, that string in this post is entirely deliberate").
  - simiones 3y ago
    This is a bug related to decoding what the user believe is a protobuf file but actually is a length-prepended protobuf file. The article would complain about whoever wrote that file, not about the code that is reading it.
- WirelessGigabit 3y ago
  https://github.com/protobufjs/protobuf.js/wiki/How-to-reverse-engineer-a-buffer-by-hand https://github.com/protobufjs/protobuf.js/wiki/How-to-revers... Prepending the message with the length means the message is length-limited. Seems standard practice here.
  - gabipurcaru 3y ago
    yep, I think it's quite standard. The BitTorrent protocol also uses it extensively : https://wiki.theory.org/BitTorrentSpecification#Bencoding https://wiki.theory.org/BitTorrentSpecification#Bencoding
- lowbloodsugar 3y ago
  That would be the official implementations of protobuf from the people that created it.
makeitdouble 3y ago
On one side it's not great, it could be made more obvious to the library user. On the other side, if they wrote multiple projects and never noticed the behavior, how bad can it be ? It would be interesting if that extra header optimized the processing a lot, pushing other libraries to have it as an option.
- alexchamberlain 3y ago
  > On the other side, if they wrote multiple projects and never noticed the behavior, how bad can it be ? Many projects will choose a standard encoding to give them language independence, but start by using the same language and libraries on both ends of the pipe. Therefore,you might not notice the library is incompatible until quite late in a project's development when you try to replace a component with a seemingly compatible, alternative implementation.
  - 0xDEAFBEAD 3y ago
    It looks like Protobuf actually has a test suite to ensure compliance with the protocol: https://github.com/protocolbuffers/protobuf/tree/main/conformance#readme https://github.com/protocolbuffers/protobuf/tree/main/confor... Seems like a good idea for protocols in general to have an official test suite, as a way to address this problem
    - sa46 3y ago
      The conformance suite tests encode and decode for single messages; you wouldn’t run it for a custom framing implementation; The gRPC interop tests are closer to what you’d want. Framing appears higher up the stack, as an RPC transport, or structured storage like the recordio format referenced by the author. The article sounds like the client expected application/protobuf but the server sent application/custom-protobuf-framing.
      - lifthrasiir 3y ago
        I think it is easy to confuse the serialization format from framing because many serialization formats do self-delimit. And the protobuf terminology of "message" may suggest framing...
userbinator 3y ago
They don't have their own framing. ...and thus it was added. We finally had to get down to individual bytes from the network dump to try to sort it out. The perils of abstraction strike again. Chances are that if you had just written the code to directly send and receive the data you wanted, since you control both ends and know exactly what the bytes will be, you'd never have run into this.
- eru 3y ago
  > Chances are that if you had just written the code to directly send and receive the data you wanted, since you control both ends and know exactly what the bytes will be, you'd never have run into this. Instead you would have run into different problems..
- lifthrasiir 3y ago
  It's indeed a shame that protobuf has so many bit patterns available for the first byte (lowermost 3 bits can't be 6 or 7, and 4 cannot be also used alone), and some of them could have been used for a unified length-delimited framing format.
lifthrasiir 3y ago
I actually went through all projects listed in [1] because I remember this very quirk. It turns out that there are many such libraries that have two variants of encode/decode functions, where the second variant prepends a varint length. In my brief inspection there do exist a few libraries with only the second variant (e.g. Rust quick-protobuf), which is legitimately problematic [2]. But if the project in question was indeed protobuf.js (see loeg's comments), it clearly distinguishes encode/decode vs. encodeDelimited/decodeDelimited. So I believe the project should not be blamed, and the better question would be why so many people chose to add this exact helper. Well, because Google itself also had the same helper [3] [4]! So at this point protobuf should just standardize this simple framing format [5], instead of claiming that protobuf has no obligation to define one. [1] https://github.com/protocolbuffers/protobuf/blob/main/docs/third_party.md https://github.com/protocolbuffers/protobuf/blob/main/docs/t... [2] https://github.com/tafia/quick-protobuf/issues/130 https://github.com/tafia/quick-protobuf/issues/130 [3] https://protobuf.dev/reference/java/api-docs/com/google/protobuf/MessageLite#writeDelimitedTo-java.io.OutputStream- https://protobuf.dev/reference/java/api-docs/com/google/prot... [4] https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/util/delimited_message_util.cc https://github.com/protocolbuffers/protobuf/blob/main/src/go... [5] Use an explicitly different name though, so that the meaning of "encoding/decoding protobuf messages" doesn't change.
- cowsandmilk 3y ago
  Yep, and this variant to the encoding is documented at https://protobuf.dev/programming-guides/techniques/#streaming https://protobuf.dev/programming-guides/techniques/#streamin... Definitely seems to be a routine addition to the standard supported by many libraries.
  - lifthrasiir 3y ago
    > Yep, and this variant to the encoding is documented at [...] It only suggests the length prefix and doesn't define the exact encoding at all.
- crabbone 3y ago
  This is not a "variant". Top-level "message" has no length. Implementations that add length are free to do it in whatever way they like, but that's not part of the format because the format itself doesn't have a concept of "sequence of messages". It has lists, but those need to be inside of messages. And since some do it in the same way, sometimes it works. The two typical approaches I found in the wild: use varint as a length and do nothing at all. Typically, the second one implies that the user, if they want to send a sequence of messages need to get creative and invent some form of connecting them together. GRPC is the first kind. So, all those using GRPC rather than straight-up Protobuf are shielded from this problem.
jeffbee 3y ago
This is very similar to a trap I fell into with some off-brand LZ4 libraries that are laying around on Github. One, called "golz4" sounded harmless but it's actually designed to be compatible with this python mistake: https://python-lz4.readthedocs.io/en/stable/lz4.block.html https://python-lz4.readthedocs.io/en/stable/lz4.block.html Length-prefixing an otherwise standard format, by default, kept me confused for some hours.
- xyzzy_plugh 3y ago
  Yes, I fall into this trap several times a decade as well. It's incredibly annoying. Especially when you're reading a spec and it says "LZ4" but implementations implicitly expect the length prefix. Drives me nuts!
saghm 3y ago
Stepping up an abstraction level in this discussion...does anyone have any insight into _why_ an encoding format wouldn't want to have length prefixes standardized as part of the expected header of a message? From what I can tell, there isn't a strong argument against it; assuming you're comfortable with limiting messages to under 2^32 bits, prefixing an unsigned length should only take four bytes per message, which doesn't seem like it would ever be a bottleneck, and it allows the receiving side of a message to know up front exactly how much memory to allocate, and it makes it much easier to write correct parsing code that also handles edge cases (e.g. making it obvious to explicitly handle a message that's much larger than the amount of memory you're willing to allocate). The fact that there are formats out there that don't mandate length prefixing makes me think I might be missing something though, so I'd be interested to hear counterarguments.
- lifthrasiir 3y ago
  The length should be known in advance in order to be written, so the message cannot be incrementally written. You need more complex framing scheme like Consistent Overhead Byte Stuffing for that. And many applications do want a variable number of length bytes because i) 4 bytes is actually too long for short messages and ii) some message can exceed 2^32 bytes. Not to say the generic varint encoding is good for this purpose, though [1]. [1] If you ever have to design one, make sure that reading the first byte is enough to determine the number of subsequent length bytes.
  - saghm 3y ago
    > 4 bytes is actually too long for short messages Would it ever be an actual bottleneck though? If it's not actually impeding throughput, I feel like this is more of an aesthetic argument than a technical one, and one where I'd happily sacrifice aesthetics to make the code simpler. > some message can exceed 2^32 bytes Fair enough, but that just makes the question "would 8 bytes per message ever actually be a bottleneck", which I'm still not convinced would ever be the case
    - lifthrasiir 3y ago
jimmytucson 3y ago
> author decided it was a good idea to prepend the message with the message length encoded as a varint. > WHY? Oh, why?! Uh oh. Is this my HN moment? This is exactly how I implemented it at my company. We had to write many protobuf messages to one file in bulk (in parallel). I did a fair amount of research before designing this and didn’t find any standard for separating protobuf messages (in fact, found that there explicitly isn’t a standard in that protobuf doesn’t care). So I thought rather than using some “special” control character, like a null byte, which would inevitably be not-so-special and collide with somebody else’s (like Schema Registry’s “magic byte”), I’d use something meaningful like the number of bytes the following record is. As for why I chose varint instead of just picking an interger size, well for one I got nerd-sniped by varint encoding and thought it would be cool to try and implement it in Scala. Secondly, I thought if I chose a fixed size integer, no matter what size I pick, my users will always surprise me and exceed it at least once, and when that happens, kaboom! I wanted to future proof this without wasting 64 goddamn bytes in front of each message, and also I got nerd-sniped, OK?!? Someone on my team recently shared one of these files outside the company and so I really hope she’s not talking about me but that’s a crazy coincidence if not!
- lifthrasiir 3y ago
  You can also define a wrapper message with a single repeated field. The resulting encoding would be `varint(field id * 8 + 1) varint(length)` plus the actual message, so it can be also easily generated from and parsed back into a raw byte sequence without protoc.
  - xyzzy_plugh 3y ago
    This approach is problematic because, as far as I know, all implementations expect to deserialize from a complete buffer -- at least all official implementations. That means the entire "stream" would have to be buffered in memory. You can, of course, simply read field by field, but few libraries expose the ability to do that simply. And the naive operation then becomes quite problematic indeed.
- pavlov 3y ago
  > “I wanted to future proof this without wasting 64 goddamn bytes in front of each message” 64 bytes would be a 512-bit integer. That seems like excessive future proofing for the length of any message that would be transmitted before the Sun runs out of fuel.
basicoperation 3y ago
I spent several hours earlier this year helping a colleague debug a protobuf deserialisation error, where I noticed that the first byte was a varint-encoded length of the rest of the data! He did eventually get things working, though I never followed up on the root cause. I can’t wait to get back from vacation to ask if it was this.
notpushkin 3y ago
Website looks down to me, here's an archive copy: https://archive.ph/OkylM https://archive.ph/OkylM
blauditore 3y ago
Isn't this exactly what `writeDelimitedTo` does? https://protobuf.dev/reference/java/api-docs/com/google/protobuf/MessageLite.html#writeDelimitedTo-java.io.OutputStream- https://protobuf.dev/reference/java/api-docs/com/google/prot... I thought this was common and well-known, but apparently not.
nly 3y ago
Pedantry regarding the article: The field prefix byte in Protobuf doesn't really encode "tag and type" as stated in the article, it encodes tag and size (whether the field is fixed 64bit, fixed 32bit, varint, or variable size) This is pretty self evident when you look at how submessages are encoded the same way as strings, both are just arbitrary variable length blobs. You cannot reliable determine from a Protobuf message whether a field is an integer, a double, a bool, or an enum without the schema. Protobufs is a TLV format that just happens to have a compact binary encoding.
ninepoints 3y ago
Folks, if you send raw PB over the wire in a protocol without framing, it _needs_ to be length prefixed somehow. This type of "bug" isn't an unreasonable thing to do, nor uncommon even. If I had a gripe, it's that I hate varints. Lots of wasted cycles to save not that many bytes in the grand scheme of things, especially when you consider other forms of compression layered on top, MTU, etc.
- catiopatio 3y ago
  It’s absolutely an unreasonable thing to do. You don’t conflate framing with payload by emitting invalid non-standard framed data from a “protobuf” encoder. They’re separate concerns and need to remain that way.
  - lowbloodsugar 3y ago
    Except that it’s part of the official implementation from google. It’s how you write multiple such messages to a stream.
    - catiopatio 3y ago
      There’s a non-default `writeDelimitedTo()` that emits a length-prefixed message. The result is not protobuf and doesn’t claim to be. Looking at my employer’s protobuf runtime for our major programming language, we don’t even support it.
      - lowbloodsugar 3y ago
        reading and writing length delimited messages is part of the official implementations. [1] I don't understand the claim of "non-default". There are multiple ways to write protobuf data, and this is one of them. It's actually the only one I've ever used, because why would you not? If your implementation doesn't support something in the reference implementation, that seems like your problem, not anyone else's. [1] https://protobuf.dev/reference/csharp/api-docs/class/google/protobuf/message-parser https://protobuf.dev/reference/csharp/api-docs/class/google/...
radarsat1 3y ago
Not protobuf, but liblo, a library I maintain for Open Sound Control, ended up supporting two different packetization standards for stream-based transmission, because there was a lack of this in the original specification. Originally OSC was intended to be "transport independent" and in practice only used for UDP transmission, so packetization was left as a problem for implementations to figure out. Cue the predictable problem of different incompatible ideas. In the specification update it was suggested to use SLIP encoding for packetization, but prior to this the library had already implemented exactly this kind of length-prefix encoding. So now the library allows to select one or the other for sending, and for backwards compatibility on reception the library tries to sniff which packetization protocol is in use. (Fortunately OSC has a predictable first character, and 99% of the time this does not line up with the first byte of the length prefix, so when it starts with that character, the software switches to SLIP mode and scans for the end-of-packet code, otherwise it assumes there is a length prefix.) It's not ideal, but it works. But it would have been simpler if the original specification had mentioned stream-based transmission. I still get the occasional question about "what are these extra bytes" at the beginning of the message when TCP is used. Overall I find that a lot of standards have had to be hacked a bit to support packetized transmission, including JSON [0], etc. It's an odd thing to leave out, but admittedly it is indeed a transport problem and most often not considered part of the "file format". So I don't know what the best solution is, but I guess it would be good if data format specifications say something about how to handle multiple serialized instances in a stream if no other standards apply. Interestingly YAML seemed to have thought about this with the "---" and "..." symbols, even in version 1.0 of the spec [1]. [0]: https://jsonlines.org/ https://jsonlines.org/ [1]: https://yaml.org/spec/1.0/index.html https://yaml.org/spec/1.0/index.html
jk_i_am_a_robot 3y ago
Everybody discovers too late that protocol buffers doesn't have any typing and then hacks on a header solution of their own. Tale as old as time.
- hinkley 3y ago
  Or everyone thinks all the complaints are haters and ignores them.
lowbloodsugar 3y ago
In fact the sender chose the standard writeDelimited and the receiver chose to read without the delimiter. Both ways are in the standard, but both ends have to agree. One could argue that is a flaw in protobuf itself, but the problem here was not a non-standard implementation. The problem here was lack of experience and hasty finger pointing.
- hinkley 3y ago
  I would argue that. Preambles aren’t some quaint thing that old idiots stuck in protocols. They knew what they were doing.
kentonv 3y ago
This post's outrage is misplaced. Protobuf encoders/decoders commonly implement two formats: delimited format, and non-delimited format. Typically non-delimited is the default, but delimited format is supported by many implementations including Google's main first-party ones. In fact, the Java implementation shipped with this support when Protobuf was first released 15 years ago (I wrote it). C++ originally left it as an exercise for the application (it's not too hard to implement), but I eventually added helper functions due to demand. Both formats can be described as "standard", at least to the extent that anything in Protobuf is a standard. So clearly the bug here is that one person was writing the delimited format and the other person was reading the non-delimited format. Maybe the confusion was the result of insufficient documentation but certainly not from a library author doing something crazy.
- deleted 3y ago
  [deleted]
- xyzzy_plugh 3y ago
  It's sort of misplaced, I'll agree. Merely using the delimited format without any other sort of framing is almost always a bad idea because of precisely the ambiguity TFA discusses. I'm pretty sure delimited streams are rarely used in the wild instead of something more robust/elaborate such as recordio, which specifically are almost always prefixed with a few magic bytes to mitigate this problem. Edit: Also, why is there no publicly available recordio specification? Infuriating.
  - blauditore 3y ago
    > I'm pretty sure delimited streams are rarely used in the wild They are somewhat common inside Google at least.
rf15 3y ago
In my job I often have to deal with developers who were supposed to implement a standard and went with a "more like guidelines" mentality. So I'm with the author here: Either you claim to support protobuff and fully implement the spec as-is, or you cannot claim to support protobuf (with maybe some leeway on a "receive-only" label if you support a superset). Otherwise you have to name/specify your protocol modification.
crabbone 3y ago
Oh... Protobuf... I had a displeasure to implement the binary encoder / decoder and IML parser. This is such a clown fiesta with so much advertising around it... I think some venerable Google's guru blessed it at some point or something like that, but when it comes down to the actual format and the tools it's like some not very bright first-second year student designed and implemented it. So, here are just few things that made me decide once and forever never to touch this format: * It cannot do streaming. It pretends that it can, but it cannot. The problem is that things that should be interpreted as hash-tables or call them "structs" don't require that key-value pairs have unique keys. Also, the last duplicate wins. So, unless you finished parsing all the pairs you cannot call any handlers / construct the "struct" because you don't know if you have the right values for it. * The grammar is written by someone who... maaaay have seen a grammar... once... long time ago. It makes absolutely no sense. It was written after this mess was somehow implemented, but was never really checked. It's pure nonsense and nobody wants to fix it because nobody really knows how it's supposed to work, nor would they know how to encode in any grammar the actual behavior of the parser. * Some details like default values for fields that aren't sent or bad (ambiguous) syntax that mixes package names and namespaces... * Unnecessary constraints on field names that are motivated by unnecessary functionality to translate Protobuf to JSON. * (C++-specific) the idea of adding messages together is the pinnacle of first-year C++ programmers: they must override a very commonly used operator in a way that breaks every contract that operator makes and for no gains except to confuse and to inconvenience everyone using this operator (usually, accidentally). --- So, not surprisingly, there's another "feature" in Protobuf that needn't exist, shouldn't exist, and probably most implementations don't implement, but yee-haw! Someone did add it. My understanding here is that the underlying problem is as follows: "messages" (the composite unit in Protobuf) don't encode their length at top-level. That's idiotic, but that's how it is. So, anyone who wants to send a sequence of messages needs to invent a tiny little bit on top of Protobuf format to tell the other end how to separate the incoming messages. Different Protobuf implementations do this differently. Some use the same encoding as Protobuf uses for varint, others use fixed-length int, some send a whole special header which, beside other things, contains the length information... My guess is that what happened is that OP and his/her friend chanced on two implementations that didn't agree on how to separate messages sent in sequence. Quite possibly, one implementation wasn't even designed to send messages in sequence, and so had no mechanism for separating them (possibly implying that however uses the library should implement that) while the other one had some mechanism in place. -- I've been there with eg. Scala and C sending messages to each other. This is probably more common that just Scala + C.

219 points by l0b0 3y ago

cratermoon 3y ago

What library was that, so I know to avoid it?

upon_drumhead 3y ago
KRPC perhaps https://krpc.github.io/krpc/communication-protocols/tcpip.html https://krpc.github.io/krpc/communication-protocols/tcpip.ht...
loeg 3y ago
https://github.com/protobufjs/protobuf.js/issues/987 https://github.com/protobufjs/protobuf.js/issues/987 maybe (based on "And yes, that string in this post is entirely deliberate").
- cratermoon 3y ago
  Ah yes, you've got it.
- cowsandmilk 3y ago
  Seems like protoduf.js has the exact same methods as Google’s implementations with the same names (encode/decode to not prepend with length, encodeDelimited/decodeDelimited for prepending length). It is hard for me to say they’re adding to the standard when they’re just replicating Google’s libraries. https://github.com/protobufjs/protobuf.js#toolset https://github.com/protobufjs/protobuf.js#toolset

kpw94 3y ago

> we started looking at this protobuf library he had selected, and sure enough, the author decided it was a good idea to prepend the message with the message length encoded as a varint. > WHY? Oh, why?! > And yes, it turns out that other people have noticed this anomaly. It's screwed up encoding and decoding in their projects, unsurprisingly. We found a (still-open) bug report from 2018, among others If anyone knows which library/language these issues the author is talking about are in, please tell us. I'd like to avoid that library if possible

vlovich123 3y ago
I feel like I recall seeing something similar in nanopb.
loeg 3y ago
> If anyone knows which library/language these issues the author is talking about are in, please tell us. https://github.com/protobufjs/protobuf.js/issues/987 https://github.com/protobufjs/protobuf.js/issues/987 maybe (based on "And yes, that string in this post is entirely deliberate").
- simiones 3y ago
  This is a bug related to decoding what the user believe is a protobuf file but actually is a length-prepended protobuf file. The article would complain about whoever wrote that file, not about the code that is reading it.
WirelessGigabit 3y ago
https://github.com/protobufjs/protobuf.js/wiki/How-to-reverse-engineer-a-buffer-by-hand https://github.com/protobufjs/protobuf.js/wiki/How-to-revers... Prepending the message with the length means the message is length-limited. Seems standard practice here.
- gabipurcaru 3y ago
  yep, I think it's quite standard. The BitTorrent protocol also uses it extensively : https://wiki.theory.org/BitTorrentSpecification#Bencoding https://wiki.theory.org/BitTorrentSpecification#Bencoding
lowbloodsugar 3y ago
That would be the official implementations of protobuf from the people that created it.

makeitdouble 3y ago

On one side it's not great, it could be made more obvious to the library user. On the other side, if they wrote multiple projects and never noticed the behavior, how bad can it be ? It would be interesting if that extra header optimized the processing a lot, pushing other libraries to have it as an option.

alexchamberlain 3y ago
> On the other side, if they wrote multiple projects and never noticed the behavior, how bad can it be ? Many projects will choose a standard encoding to give them language independence, but start by using the same language and libraries on both ends of the pipe. Therefore,you might not notice the library is incompatible until quite late in a project's development when you try to replace a component with a seemingly compatible, alternative implementation.
- 0xDEAFBEAD 3y ago
  It looks like Protobuf actually has a test suite to ensure compliance with the protocol: https://github.com/protocolbuffers/protobuf/tree/main/conformance#readme https://github.com/protocolbuffers/protobuf/tree/main/confor... Seems like a good idea for protocols in general to have an official test suite, as a way to address this problem
  - sa46 3y ago
    The conformance suite tests encode and decode for single messages; you wouldn’t run it for a custom framing implementation; The gRPC interop tests are closer to what you’d want. Framing appears higher up the stack, as an RPC transport, or structured storage like the recordio format referenced by the author. The article sounds like the client expected application/protobuf but the server sent application/custom-protobuf-framing.
    - lifthrasiir 3y ago
      I think it is easy to confuse the serialization format from framing because many serialization formats do self-delimit. And the protobuf terminology of "message" may suggest framing...

userbinator 3y ago

They don't have their own framing. ...and thus it was added. We finally had to get down to individual bytes from the network dump to try to sort it out. The perils of abstraction strike again. Chances are that if you had just written the code to directly send and receive the data you wanted, since you control both ends and know exactly what the bytes will be, you'd never have run into this.

eru 3y ago
> Chances are that if you had just written the code to directly send and receive the data you wanted, since you control both ends and know exactly what the bytes will be, you'd never have run into this. Instead you would have run into different problems..
lifthrasiir 3y ago
It's indeed a shame that protobuf has so many bit patterns available for the first byte (lowermost 3 bits can't be 6 or 7, and 4 cannot be also used alone), and some of them could have been used for a unified length-delimited framing format.

lifthrasiir 3y ago

I actually went through all projects listed in [1] because I remember this very quirk. It turns out that there are many such libraries that have two variants of encode/decode functions, where the second variant prepends a varint length. In my brief inspection there do exist a few libraries with only the second variant (e.g. Rust quick-protobuf), which is legitimately problematic [2]. But if the project in question was indeed protobuf.js (see loeg's comments), it clearly distinguishes encode/decode vs. encodeDelimited/decodeDelimited. So I believe the project should not be blamed, and the better question would be why so many people chose to add this exact helper. Well, because Google itself also had the same helper [3] [4]! So at this point protobuf should just standardize this simple framing format [5], instead of claiming that protobuf has no obligation to define one. [1] https://github.com/protocolbuffers/protobuf/blob/main/docs/third_party.md https://github.com/protocolbuffers/protobuf/blob/main/docs/t... [2] https://github.com/tafia/quick-protobuf/issues/130 https://github.com/tafia/quick-protobuf/issues/130 [3] https://protobuf.dev/reference/java/api-docs/com/google/protobuf/MessageLite#writeDelimitedTo-java.io.OutputStream- https://protobuf.dev/reference/java/api-docs/com/google/prot... [4] https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/util/delimited_message_util.cc https://github.com/protocolbuffers/protobuf/blob/main/src/go... [5] Use an explicitly different name though, so that the meaning of "encoding/decoding protobuf messages" doesn't change.

cowsandmilk 3y ago
Yep, and this variant to the encoding is documented at https://protobuf.dev/programming-guides/techniques/#streaming https://protobuf.dev/programming-guides/techniques/#streamin... Definitely seems to be a routine addition to the standard supported by many libraries.
- lifthrasiir 3y ago
  > Yep, and this variant to the encoding is documented at [...] It only suggests the length prefix and doesn't define the exact encoding at all.
crabbone 3y ago
This is not a "variant". Top-level "message" has no length. Implementations that add length are free to do it in whatever way they like, but that's not part of the format because the format itself doesn't have a concept of "sequence of messages". It has lists, but those need to be inside of messages. And since some do it in the same way, sometimes it works. The two typical approaches I found in the wild: use varint as a length and do nothing at all. Typically, the second one implies that the user, if they want to send a sequence of messages need to get creative and invent some form of connecting them together. GRPC is the first kind. So, all those using GRPC rather than straight-up Protobuf are shielded from this problem.

jeffbee 3y ago

This is very similar to a trap I fell into with some off-brand LZ4 libraries that are laying around on Github. One, called "golz4" sounded harmless but it's actually designed to be compatible with this python mistake: https://python-lz4.readthedocs.io/en/stable/lz4.block.html https://python-lz4.readthedocs.io/en/stable/lz4.block.html Length-prefixing an otherwise standard format, by default, kept me confused for some hours.

xyzzy_plugh 3y ago
Yes, I fall into this trap several times a decade as well. It's incredibly annoying. Especially when you're reading a spec and it says "LZ4" but implementations implicitly expect the length prefix. Drives me nuts!

saghm 3y ago

Stepping up an abstraction level in this discussion...does anyone have any insight into _why_ an encoding format wouldn't want to have length prefixes standardized as part of the expected header of a message? From what I can tell, there isn't a strong argument against it; assuming you're comfortable with limiting messages to under 2^32 bits, prefixing an unsigned length should only take four bytes per message, which doesn't seem like it would ever be a bottleneck, and it allows the receiving side of a message to know up front exactly how much memory to allocate, and it makes it much easier to write correct parsing code that also handles edge cases (e.g. making it obvious to explicitly handle a message that's much larger than the amount of memory you're willing to allocate). The fact that there are formats out there that don't mandate length prefixing makes me think I might be missing something though, so I'd be interested to hear counterarguments.

lifthrasiir 3y ago
The length should be known in advance in order to be written, so the message cannot be incrementally written. You need more complex framing scheme like Consistent Overhead Byte Stuffing for that. And many applications do want a variable number of length bytes because i) 4 bytes is actually too long for short messages and ii) some message can exceed 2^32 bytes. Not to say the generic varint encoding is good for this purpose, though [1]. [1] If you ever have to design one, make sure that reading the first byte is enough to determine the number of subsequent length bytes.
- saghm 3y ago
  > 4 bytes is actually too long for short messages Would it ever be an actual bottleneck though? If it's not actually impeding throughput, I feel like this is more of an aesthetic argument than a technical one, and one where I'd happily sacrifice aesthetics to make the code simpler. > some message can exceed 2^32 bytes Fair enough, but that just makes the question "would 8 bytes per message ever actually be a bottleneck", which I'm still not convinced would ever be the case
  - lifthrasiir 3y ago

jimmytucson 3y ago

> author decided it was a good idea to prepend the message with the message length encoded as a varint. > WHY? Oh, why?! Uh oh. Is this my HN moment? This is exactly how I implemented it at my company. We had to write many protobuf messages to one file in bulk (in parallel). I did a fair amount of research before designing this and didn’t find any standard for separating protobuf messages (in fact, found that there explicitly isn’t a standard in that protobuf doesn’t care). So I thought rather than using some “special” control character, like a null byte, which would inevitably be not-so-special and collide with somebody else’s (like Schema Registry’s “magic byte”), I’d use something meaningful like the number of bytes the following record is. As for why I chose varint instead of just picking an interger size, well for one I got nerd-sniped by varint encoding and thought it would be cool to try and implement it in Scala. Secondly, I thought if I chose a fixed size integer, no matter what size I pick, my users will always surprise me and exceed it at least once, and when that happens, kaboom! I wanted to future proof this without wasting 64 goddamn bytes in front of each message, and also I got nerd-sniped, OK?!? Someone on my team recently shared one of these files outside the company and so I really hope she’s not talking about me but that’s a crazy coincidence if not!

lifthrasiir 3y ago
You can also define a wrapper message with a single repeated field. The resulting encoding would be `varint(field id * 8 + 1) varint(length)` plus the actual message, so it can be also easily generated from and parsed back into a raw byte sequence without protoc.
- xyzzy_plugh 3y ago
  This approach is problematic because, as far as I know, all implementations expect to deserialize from a complete buffer -- at least all official implementations. That means the entire "stream" would have to be buffered in memory. You can, of course, simply read field by field, but few libraries expose the ability to do that simply. And the naive operation then becomes quite problematic indeed.
pavlov 3y ago
> “I wanted to future proof this without wasting 64 goddamn bytes in front of each message” 64 bytes would be a 512-bit integer. That seems like excessive future proofing for the length of any message that would be transmitted before the Sun runs out of fuel.

basicoperation 3y ago

I spent several hours earlier this year helping a colleague debug a protobuf deserialisation error, where I noticed that the first byte was a varint-encoded length of the rest of the data! He did eventually get things working, though I never followed up on the root cause. I can’t wait to get back from vacation to ask if it was this.

notpushkin 3y ago

Website looks down to me, here's an archive copy: https://archive.ph/OkylM https://archive.ph/OkylM

blauditore 3y ago

Isn't this exactly what `writeDelimitedTo` does? https://protobuf.dev/reference/java/api-docs/com/google/protobuf/MessageLite.html#writeDelimitedTo-java.io.OutputStream- https://protobuf.dev/reference/java/api-docs/com/google/prot... I thought this was common and well-known, but apparently not.

nly 3y ago

Pedantry regarding the article: The field prefix byte in Protobuf doesn't really encode "tag and type" as stated in the article, it encodes tag and size (whether the field is fixed 64bit, fixed 32bit, varint, or variable size) This is pretty self evident when you look at how submessages are encoded the same way as strings, both are just arbitrary variable length blobs. You cannot reliable determine from a Protobuf message whether a field is an integer, a double, a bool, or an enum without the schema. Protobufs is a TLV format that just happens to have a compact binary encoding.

ninepoints 3y ago

Folks, if you send raw PB over the wire in a protocol without framing, it _needs_ to be length prefixed somehow. This type of "bug" isn't an unreasonable thing to do, nor uncommon even. If I had a gripe, it's that I hate varints. Lots of wasted cycles to save not that many bytes in the grand scheme of things, especially when you consider other forms of compression layered on top, MTU, etc.

catiopatio 3y ago
It’s absolutely an unreasonable thing to do. You don’t conflate framing with payload by emitting invalid non-standard framed data from a “protobuf” encoder. They’re separate concerns and need to remain that way.
- lowbloodsugar 3y ago
  Except that it’s part of the official implementation from google. It’s how you write multiple such messages to a stream.
  - catiopatio 3y ago
    There’s a non-default `writeDelimitedTo()` that emits a length-prefixed message. The result is not protobuf and doesn’t claim to be. Looking at my employer’s protobuf runtime for our major programming language, we don’t even support it.
    - lowbloodsugar 3y ago
      reading and writing length delimited messages is part of the official implementations. [1] I don't understand the claim of "non-default". There are multiple ways to write protobuf data, and this is one of them. It's actually the only one I've ever used, because why would you not? If your implementation doesn't support something in the reference implementation, that seems like your problem, not anyone else's. [1] https://protobuf.dev/reference/csharp/api-docs/class/google/protobuf/message-parser https://protobuf.dev/reference/csharp/api-docs/class/google/...

radarsat1 3y ago

Not protobuf, but liblo, a library I maintain for Open Sound Control, ended up supporting two different packetization standards for stream-based transmission, because there was a lack of this in the original specification. Originally OSC was intended to be "transport independent" and in practice only used for UDP transmission, so packetization was left as a problem for implementations to figure out. Cue the predictable problem of different incompatible ideas. In the specification update it was suggested to use SLIP encoding for packetization, but prior to this the library had already implemented exactly this kind of length-prefix encoding. So now the library allows to select one or the other for sending, and for backwards compatibility on reception the library tries to sniff which packetization protocol is in use. (Fortunately OSC has a predictable first character, and 99% of the time this does not line up with the first byte of the length prefix, so when it starts with that character, the software switches to SLIP mode and scans for the end-of-packet code, otherwise it assumes there is a length prefix.) It's not ideal, but it works. But it would have been simpler if the original specification had mentioned stream-based transmission. I still get the occasional question about "what are these extra bytes" at the beginning of the message when TCP is used. Overall I find that a lot of standards have had to be hacked a bit to support packetized transmission, including JSON [0], etc. It's an odd thing to leave out, but admittedly it is indeed a transport problem and most often not considered part of the "file format". So I don't know what the best solution is, but I guess it would be good if data format specifications say something about how to handle multiple serialized instances in a stream if no other standards apply. Interestingly YAML seemed to have thought about this with the "---" and "..." symbols, even in version 1.0 of the spec [1]. [0]: https://jsonlines.org/ https://jsonlines.org/ [1]: https://yaml.org/spec/1.0/index.html https://yaml.org/spec/1.0/index.html

jk_i_am_a_robot 3y ago

Everybody discovers too late that protocol buffers doesn't have any typing and then hacks on a header solution of their own. Tale as old as time.

hinkley 3y ago
Or everyone thinks all the complaints are haters and ignores them.

lowbloodsugar 3y ago

In fact the sender chose the standard writeDelimited and the receiver chose to read without the delimiter. Both ways are in the standard, but both ends have to agree. One could argue that is a flaw in protobuf itself, but the problem here was not a non-standard implementation. The problem here was lack of experience and hasty finger pointing.

hinkley 3y ago
I would argue that. Preambles aren’t some quaint thing that old idiots stuck in protocols. They knew what they were doing.

kentonv 3y ago

This post's outrage is misplaced. Protobuf encoders/decoders commonly implement two formats: delimited format, and non-delimited format. Typically non-delimited is the default, but delimited format is supported by many implementations including Google's main first-party ones. In fact, the Java implementation shipped with this support when Protobuf was first released 15 years ago (I wrote it). C++ originally left it as an exercise for the application (it's not too hard to implement), but I eventually added helper functions due to demand. Both formats can be described as "standard", at least to the extent that anything in Protobuf is a standard. So clearly the bug here is that one person was writing the delimited format and the other person was reading the non-delimited format. Maybe the confusion was the result of insufficient documentation but certainly not from a library author doing something crazy.

deleted 3y ago
[deleted]
xyzzy_plugh 3y ago
It's sort of misplaced, I'll agree. Merely using the delimited format without any other sort of framing is almost always a bad idea because of precisely the ambiguity TFA discusses. I'm pretty sure delimited streams are rarely used in the wild instead of something more robust/elaborate such as recordio, which specifically are almost always prefixed with a few magic bytes to mitigate this problem. Edit: Also, why is there no publicly available recordio specification? Infuriating.
- blauditore 3y ago
  > I'm pretty sure delimited streams are rarely used in the wild They are somewhat common inside Google at least.

rf15 3y ago

In my job I often have to deal with developers who were supposed to implement a standard and went with a "more like guidelines" mentality. So I'm with the author here: Either you claim to support protobuff and fully implement the spec as-is, or you cannot claim to support protobuf (with maybe some leeway on a "receive-only" label if you support a superset). Otherwise you have to name/specify your protocol modification.

crabbone 3y ago

Oh... Protobuf... I had a displeasure to implement the binary encoder / decoder and IML parser. This is such a clown fiesta with so much advertising around it... I think some venerable Google's guru blessed it at some point or something like that, but when it comes down to the actual format and the tools it's like some not very bright first-second year student designed and implemented it. So, here are just few things that made me decide once and forever never to touch this format: * It cannot do streaming. It pretends that it can, but it cannot. The problem is that things that should be interpreted as hash-tables or call them "structs" don't require that key-value pairs have unique keys. Also, the last duplicate wins. So, unless you finished parsing all the pairs you cannot call any handlers / construct the "struct" because you don't know if you have the right values for it. * The grammar is written by someone who... maaaay have seen a grammar... once... long time ago. It makes absolutely no sense. It was written after this mess was somehow implemented, but was never really checked. It's pure nonsense and nobody wants to fix it because nobody really knows how it's supposed to work, nor would they know how to encode in any grammar the actual behavior of the parser. * Some details like default values for fields that aren't sent or bad (ambiguous) syntax that mixes package names and namespaces... * Unnecessary constraints on field names that are motivated by unnecessary functionality to translate Protobuf to JSON. * (C++-specific) the idea of adding messages together is the pinnacle of first-year C++ programmers: they must override a very commonly used operator in a way that breaks every contract that operator makes and for no gains except to confuse and to inconvenience everyone using this operator (usually, accidentally). --- So, not surprisingly, there's another "feature" in Protobuf that needn't exist, shouldn't exist, and probably most implementations don't implement, but yee-haw! Someone did add it. My understanding here is that the underlying problem is as follows: "messages" (the composite unit in Protobuf) don't encode their length at top-level. That's idiotic, but that's how it is. So, anyone who wants to send a sequence of messages needs to invent a tiny little bit on top of Protobuf format to tell the other end how to separate the incoming messages. Different Protobuf implementations do this differently. Some use the same encoding as Protobuf uses for varint, others use fixed-length int, some send a whole special header which, beside other things, contains the length information... My guess is that what happened is that OP and his/her friend chanced on two implementations that didn't agree on how to separate messages sent in sequence. Quite possibly, one implementation wasn't even designed to send messages in sequence, and so had no mechanism for separating them (possibly implying that however uses the library should implement that) while the other one had some mechanism in place. -- I've been there with eg. Scala and C sending messages to each other. This is probably more common that just Scala + C.