Jens-G opened a new pull request, #3780: URL: https://github.com/apache/thrift/pull/3780
`TWebSocketServer::readFrame` sizes its read buffer from the payload length the frame header declares, before a single payload byte has arrived. `UINT32_MAX` is the only bound, so the fourteen bytes of a header carrying a 64-bit length decide the size of the allocation. Measured on a Linux x86-64 build: a header declaring `0xFFFFFFFF` takes the process from a `VmPeak` of 10,512 kB to 4,205,092 kB and asks the transport underneath for 4,294,967,295 bytes, with no payload sent at all. `resetBuffer()` constructs a fresh `TMemoryBuffer` of that size, so the allocation happens whether or not the bytes ever turn up, and the buffer is a member that lives as long as the connection does. The transport has a `TConfiguration` and consults neither of its limits on this path. This holds the declared length to `TConfiguration::maxFrameSize` — the ceiling `TFramedTransport::readFrame` already applies to its own frames — and refuses anything above it with close code 1009 *Message Too Big*, which is what this transport already sends when a frame is too large for it. ### Compatibility — worth a release note `maxFrameSize` defaults to 16384000, so a peer sending a single WebSocket frame larger than that is refused where it was accepted before. The bound follows whatever an operator sets. ### Tests `lib/cpp/test/TWebSocketServerTest.cpp`, the first tests this transport has had. They assert the largest read the server asked of the transport underneath it rather than merely that the read failed — a payload that never arrives ends the frame either way, so *"did it fail?"* passes on the unmodified library too. - Two fail before the change: `67108864 > 1024` and `32768 > 1024`. All six pass after. - Four are regression guards that pass either way: an ordinary frame, a frame of exactly the maximum, two frames in a row, and a length with the high bit set. - Full `bin/UnitTests`: 106 of 107 before and after. The one failure, `TServerSocketTest/test_bind_to_address`, is a pre-existing environment failure on this host, unrelated and present on `master`. The test is registered under `if(OPENSSL_FOUND AND WITH_OPENSSL)` in CMake, because that is the condition under which `TWebSocketServer.cpp` is built at all; the autotools build compiles it unconditionally, so `Makefile.am` needs no guard. ### Separable, found while checking this and deliberately not in scope `readFrame` reads the payload with a single `transport_->read()` and treats a short read as end of stream. Verified by execution: an ordinary 40-byte frame delivered as 20 + 20 bytes makes `readAll` return 0, so any frame whose payload does not arrive in one read is silently dropped. That is a separate correctness defect, it changes blocking behaviour to fix, and the bound added here is the precondition for fixing it safely — `readAll` on an unbounded declared length would wait for 4 GiB. It will follow as its own ticket. 🤖 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]
