Jens-G opened a new pull request, #3781:
URL: https://github.com/apache/thrift/pull/3781
> **Stacked on #3780 (THRIFT-6177).** Only the second commit belongs to this
ticket. Merge #3780 first and this rebases to a single commit.
`TWebSocketServer::readFrame` reads a frame's payload with a single call to
the transport underneath and treats anything short of the whole payload as end
of stream:
```cpp
read = transport_->read(buffer, length);
readBuffer_.wroteBytes(read);
if (read < length) {
return false;
}
```
`TSocket::read` performs one `recv()` and returns whatever that produced,
and the transport factory hands `TWebSocketServer` the accepted `TSocket`
itself, so the payload arrives in as many pieces as the network chose.
Returning `false` makes `readAll_virt` return 0, which the caller reads as a
closed connection.
So no frame whose payload does not arrive in one read can be received at
all. Demonstrated with a `TServerSocket`, a `TWebSocketServer` over the
accepted socket, and a client writing a well-formed 4000-byte frame as 2000 +
2000 bytes with a pause between them:
```
client: payload 4000 bytes, sent as 2000 + 2000 with 300 ms between
server: readAll returned 0 of 4000
```
and with this change, over the same socket:
```
server: readAll returned 4000 of 4000
server: payload intact: yes
```
In practice this is any frame past the path MTU, so the transport could not
carry a Thrift message beyond roughly 1.4 kB over an ordinary Ethernet path.
### The fix
Read the payload with `readAll()`, which is how
`TFramedTransport::readFrame` reads its own frames, and keep the existing
signal for a peer that leaves in the middle of one: `readAll` raises
`END_OF_FILE`, which is caught and turned back into the same `return false` the
caller sees today. Nothing else about the contract changes.
### Why it is stacked
`readAll()` waits for the number of bytes the frame declared. That is only
safe once that number is bounded, which is what #3780 does — without it, a
header declaring `0xFFFFFFFF` would make the server wait for 4 GB rather than
return. With it, the wait is bounded by `TConfiguration::maxFrameSize` and by
the socket's own receive timeout, the same two limits that bound
`TFramedTransport`.
### Tests
Two more cases in `lib/cpp/test/TWebSocketServerTest.cpp`:
- a frame delivered as 8 + 1000 + the rest arrives whole — fails before the
change with `0 != 4000`;
- a frame whose payload stops after 500 bytes is still reported as end of
stream rather than handed over half-read — passes either way, and is there so
that waiting for the rest of a frame cannot quietly become handing a half-read
one to the protocol.
Full `bin/UnitTests`: 108 of 109. 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]
