Zero length payloads
CandleCandle opened this issue · 4 comments
Symptoms
When the other end of the connection sends a zero length payload (e.g. with a ping) the server behaves oddly.
Analysis
_FrameDecoder correctly reads the frame header puts itself into _ExpectMaskKeyAndPayload state, then returns zero to _TCPConnectionNotify. _handle_frame which then calls TCPConnection.expect(0). The next read puts the _FrameDecoder into an even more invalid state.
Background
In trying to re-use most of the frame parsing and connection handling code in a client context (There's no point in trying to re-implement it), and using the echo server at websocket.org as a separate implementation of the server, I discovered that it sends a ping frame 30 seconds after connecting which has a zero-length payload. This is probably a bug with their server implementation, and I found no mention of zero length payloads in RFC 6455, but it seems like something that should be handled.
The server implementation sends 0x89 0x00 0x00 0x00 0x00 0x00 as it's ping packet, which looks like another issue on their end, If they are sending any payload, then the size should be non-zero.
In section RFC 6455, section 5.2:
Mask: 1 bit
Defines whether the "Payload data" is masked. If set to 1, a
masking key is present in masking-key, and this is used to unmask
the "Payload data" as per Section 5.3. All frames sent from
client to server have this bit set to 1.
So a client frame must have mask bit set to 1 and have a masking key. If client doesn't follow the RFC, we can close the connection with status code 1002 (protocol error), and we lack these checks currently.
All you say in your comment is correct, but I don't think it's relevant to this issue. If the payload length is zero, then the frame parser is expecting a payload, when it should be expecting the next frame header.
from https://tools.ietf.org/html/rfc6455#section-5.2 :
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-------+-+-------------+-------------------------------+
|F|R|R|R| opcode|M| Payload len | Extended payload length |
|I|S|S|S| (4) |A| (7) | (16/64) |
|N|V|V|V| |S| | (if payload len==126/127) |
| |1|2|3| |K| | |
+-+-+-+-+-------+-+-------------+ - - - - - - - - - - - - - - - +
This issue will occur with the first two bytes being either
0 1
0123456789012346
0000100100000000
or
0 1
0123456789012346
0000100110000000
0 1
0123456789012346
0000100100000000
Yes, the frame decoder fails in this not-masked case, expecting 0 bytes.
And in the second case, the frame decoder should works correctly, expecting a 4 bytes mask key and zero-length payload.
More work is needed to make the decoder works for both server-side and client-side.
#9 fixes the zero length payload issue.
https://github.com/CandleCandle/pony-websocket/tree/client-side-changes adds client-side support.