Jens-G opened a new pull request, #3783:
URL: https://github.com/apache/thrift/pull/3783
> **Stacked on #3780, #3781 and #3782.** Only the fourth commit belongs to
this ticket.
Three related framing defects in `TWebSocketServer`, all demonstrated
against `master`.
### 1. The masking key of a zero-length masked frame is never consumed
`readFrame` reads the four masking-key bytes inside `if (length > 0)`. RFC
6455 §5.2 puts the masking key in every frame whose MASK bit is set, whatever
the payload length — and `readFrame` itself refuses a client frame that does
not set MASK. So for a zero-length frame the four bytes stay in the stream and
the next header is parsed out of the masking key:
```
client: 89 80 37 37 37 37 an empty Ping
82 85 37 37 37 37 ... a five-byte data frame
server: TTransportException: Reserved bits must be zeroes
```
A masking key is arbitrary, so this fails in whatever way the key happens to
encode. An empty Ping, an empty Pong and a Close with no status code are all
ordinary traffic that browsers send.
### 2. A Pong declares a length of zero and writes the payload after it
`writeFrameHeader` took its length from `writeBuffer_.available_read()`,
which describes the body only for `flush()` — the one caller whose body *is*
the write buffer. `pong()` writes `readBuffer_` instead:
```
ping of 8 bytes "PINGDATA" -> 8A 00 50 49 4E 47 44 41 54 41
```
Opcode `0x8A` is FIN|Pong, the declared length is 0, and eight bytes follow
it. A conforming client reads a zero-length Pong and then parses `0x50` as the
next frame header, whose reserved bits are set, and must fail the connection.
### 3. A Close frame does the same
`failConnection` writes its two-byte close code after a header declaring
zero — `88 00 03 F1` — so the reason the connection ended is unreadable to a
conforming peer.
### The change
Pass `writeFrameHeader` the length it is describing instead of letting it
guess; read the masking key as part of the header it belongs to; and reset the
read buffer for a frame with no payload, so a Pong for an empty Ping echoes
nothing rather than whatever the frame before it left unread.
On the wire, after: `8A 08 50 49 4E 47 44 41 54 41` and `88 02 03 F1`.
### Tests
Five. Four fail before the change — two on the exception, two on the bytes
written. The fifth is a regression guard on `flush()`, the caller whose body
really is the write buffer, and passes either way.
Full `bin/UnitTests`: 114 of 115. The one failure,
`TServerSocketTest/test_bind_to_address`, is a pre-existing environment failure
on this host and is present on `master`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
