engine.io-protocol icon indicating copy to clipboard operation
engine.io-protocol copied to clipboard

Binary messages should be of a different packet type to plaintext messages.

Open vxern opened this issue 2 years ago • 4 comments

Due to the nature of data being transmitted, message packets with a binary payload need different handling (identifying, encoding, decoding). This is already being done, but the specification attempts to make these types agree by attempting to bunch them together under a single packet message with ID 4, but the same specification, on the other hand, also identifies binary packets differently (using a b character).

So, given that not only are the packets handled differently, stored differently, but also is identified differently and transmitted differently (binary packets have MIME type application/octet-stream, text packets have MIME type text/plain), wouldn't it simply make more sense to have two message packet types, rather than trying hard to make them coexist under a single packet type as is currently being done?

vxern avatar Apr 30 '23 11:04 vxern

Hi! Thanks for opening this :+1:

I'm not sure about what you are suggesting, what would be the benefits of having two distinct packet types?

darrachequesne avatar May 01 '23 06:05 darrachequesne

Initially, I was suggesting that these packets should be of a different type. However, having thought more on this, I realised that they actually already are different. Due to this, I'm suggesting to just make the specification more clear in that these packets are different:

  • They have different IDs: b for binary, 4 for text.
  • They are encoded differently: base64 for binary, utf8 for text.
  • They impact the Content-Type header value differently: application/octet-stream for binary, text/plain for text.

Naturally, because of these differences, on both the server-side and the client-side, this data needs completely different handling. It would help for the specification to support this, rather than it implying that perhaps binary and text data should be bunched up 'under the same roof', as is currently being done in the JS implementation. This isn't a great practice.

vxern avatar May 02 '23 12:05 vxern

Alternatively to having different types, whether the message packet was binary or text could have been originally identified within the packet itself using some kind of flag, thus eliminating the need for a different packet type. For example, text message packets would become 40, binary packets would become 41. However, this isn't the case as per the specification, and luckily so because it's not any more practical or different to just having a different ID.

vxern avatar May 02 '23 12:05 vxern

They are encoded differently: base64 for binary, utf8 for text.

Please note that this is only valid for HTTP long-polling, with WebSocket the binary payload is sent as is, without modification.

It would help for the specification to support this, rather than it implying that perhaps binary and text data should be bunched up 'under the same roof', as is currently being done in the JS implementation. This isn't a great practice.

This is done to reduce the number of HTTP requests used with HTTP long-polling, several packets (plain text and binary) can be concatenated for performance purposes.

darrachequesne avatar May 02 '23 12:05 darrachequesne

I think this can be closed now. Please reopen if needed.

darrachequesne avatar Jul 01 '24 19:07 darrachequesne