[
https://issues.apache.org/jira/browse/THRIFT-6178?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6178.
--------------------------------
Assignee: Jens Geyer
Resolution: Fixed
> C++ WebSocket server drops any frame whose payload does not arrive in one read
> ------------------------------------------------------------------------------
>
> Key: THRIFT-6178
> URL: https://issues.apache.org/jira/browse/THRIFT-6178
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> TWebSocketServer::readFrame reads a frame's payload with a single call to the
> underlying transport and treats anything short of the whole payload as end of
> stream (lib/cpp/src/thrift/transport/TWebSocketServer.h):
> readBuffer_.resetBuffer(length);
> uint8_t* buffer = readBuffer_.getWritePtr(length);
> 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 directly, so
> the payload arrives in as many pieces as the network chose to split it into.
> Returning false makes readAll_virt return 0, which the caller reads as a
> closed connection.
> The effect is that no frame whose payload does not arrive in a single read
> can be received at all. Demonstrated with a TServerSocket, a TWebSocketServer
> over the accepted socket and a client that writes a well-formed 4000-byte
> frame as 2000 + 2000 bytes with a pause between them: readAll returns 0 of
> 4000. In practice this is any frame larger than the path MTU, so the
> transport cannot carry a Thrift message beyond roughly 1.4 kB over an
> ordinary Ethernet path.
> The fix is to read the payload with transport_->readAll(), which is what
> TFramedTransport::readFrame does for its own frames, and to keep the existing
> signal for a peer that goes away 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.
> This depends on THRIFT-6177 and should land after it. readAll() waits for the
> number of bytes the frame declared, so it must not be used until that number
> is bounded -- otherwise a header declaring 0xFFFFFFFF would make the server
> wait for 4 GB rather than return. With THRIFT-6177 in place the wait is
> bounded by TConfiguration::maxFrameSize and by the socket's own receive
> timeout, the same two limits that bound TFramedTransport.
> Tests are added to lib/cpp/test/TWebSocketServerTest.cpp: a frame delivered
> in several pieces now arrives whole, and a frame whose payload stops early is
> still reported as end of stream rather than handed over half-read.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
