-
Notifications
You must be signed in to change notification settings - Fork 13
Description
look at this code:
wrench/src/Frame/HybiFrame.php
Lines 96 to 100 in 10cd90c
| } elseif ($this->length <= 65536) { | |
| $this->buffer[self::BYTE_INITIAL_LENGTH] = \chr( | |
| (self::BITFIELD_INITIAL_LENGTH & 126) | $masked_bit | |
| ); | |
| $this->buffer .= \pack('n', $this->length); |
if length is exactly 65536 bytes, \pack('n', 65536) will wrap around to 0 and return "\x00\x00",
that -looks suspicious- and does not match what the websockets/ws library does in
https://github.com/websockets/ws/blob/aa998e38c5f62844eb2fb1ccccb69a9c953ccd4f/lib/sender.js#L131-L137
- in websockets/ws if the data is >= 65536 bytes, it use a 8 byte length header 🤔
I find the actual specification at https://datatracker.ietf.org/doc/html/draft-ietf-hybi-thewebsocketprotocol-17 difficult to read, but it says
frame-payload-length = %x00-7D
/ %x7E frame-payload-length-16
/ %x7F frame-payload-length-63
; 7 bits in length
frame-payload-length-16 = %x0000-FFFF ; 16 bits in length
frame-payload-length-63 = %x0000000000000000-7FFFFFFFFFFFFFFF
; 64 bits in length
I think that means our code should be
elseif ($this->length <= 0xFFFF) {
$this->buffer[self::BYTE_INITIAL_LENGTH] = "\x7E"; // 126
$this->buffer .= \pack('n', $this->length);and that we should not be doing
wrench/src/Frame/HybiFrame.php
Lines 101 to 105 in 10cd90c
| } else { | |
| $this->buffer[self::BYTE_INITIAL_LENGTH] = \chr( | |
| (self::BITFIELD_INITIAL_LENGTH & 127) | $masked_bit | |
| ); | |
but hardcode it to
} else {
$this->buffer[self::BYTE_INITIAL_LENGTH] = "\x7F"; // 127also this part looks bogus:
wrench/src/Frame/HybiFrame.php
Lines 106 to 110 in 10cd90c
| if (\PHP_INT_MAX > 2147483647) { | |
| $this->buffer .= \pack('NN', $this->length >> 32, $this->length); | |
| } else { | |
| $this->buffer .= \pack('NN', 0, $this->length); | |
| } |
pretty sure it should just be
$this->buffer .= \pack('J', $this->length);(maybe J didn't exist when it was originally written? changelog says nothing about when J was created tho)