[
https://issues.apache.org/jira/browse/THRIFT-6178?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-6178:
-------------------------------
Description:
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):
{code}
readBuffer_.resetBuffer(length);
uint8_t* buffer = readBuffer_.getWritePtr(length);
read = transport_->read(buffer, length);
readBuffer_.wroteBytes(read);
if (read < length) {
return false;
}
{code}
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.
was:
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.
> 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):
> {code}
> readBuffer_.resetBuffer(length);
> uint8_t* buffer = readBuffer_.getWritePtr(length);
> read = transport_->read(buffer, length);
> readBuffer_.wroteBytes(read);
> if (read < length) {
> return false;
> }
> {code}
> 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)
