Jens Geyer created THRIFT-6230:
----------------------------------
Summary: Port the WebSocket frame reading fixes of THRIFT-6178,
THRIFT-6179 and THRIFT-6180 to the D library
Key: THRIFT-6230
URL: https://issues.apache.org/jira/browse/THRIFT-6230
Project: Thrift
Issue Type: Bug
Components: D - Library
Reporter: Jens Geyer
Fix For: 0.25.0
{{lib/d/src/thrift/transport/websocket.d}} ({{TServerWebSocketTransport}})
still reads frames the way the C++ {{TWebSocketServer}} did before THRIFT-6178,
THRIFT-6179 and THRIFT-6180. The two transports share their design, and the D
one has all three defects on master. Since commit 79dc86d55 the module is
built, tested and installed by {{lib/d/Makefile.am}}.
h3. What happens
* *A payload that does not arrive in one read is lost* (as in THRIFT-6178).
{{readFrame()}} reads the payload with a single {{transport_.read()}} and takes
anything short of the declared length for the end of the stream.
{{TSocket.read()}} returns what one {{receive()}} produced, so a frame that
spans more than one segment ends the connection. A 4,000-byte frame served
1,000 bytes per read: {{readFrame()}} returns {{false}}.
* *Every Ping costs a stack frame* (as in THRIFT-6179). A Ping is answered with
{{pong(); return readFrame();}}. 2,000 one-byte Pings, 14 kB on the wire, take
the reader 256,368 bytes down the stack, and nothing limits how many Pings a
peer sends.
* *Empty and control frames are mis-framed* (as in THRIFT-6180).
** The masking key of a masked frame with no payload is not read, so the next
header is parsed out of it: an empty Ping followed by a data frame ends in
{{TTransportException: Reserved bits must be zeroes}}.
** {{writeFrameHeader()}} takes its length from the write buffer, which
describes the frame only when {{flush()}} calls it. A Pong for an 8-byte Ping
is written as {{8A 00}} followed by the eight bytes, and a Close as {{88 00 03
F1}}.
h3. Change
The three C++ changes, ported:
* read the payload with {{readAll()}}; a peer that leaves part-way through a
frame ({{END_OF_FILE}}) is still reported as the end of the stream,
* loop over frames instead of calling {{readFrame()}} again after a Ping,
* read the masking key of every masked frame, reset the read buffer for a frame
with no payload, and pass {{writeFrameHeader()}} the length it describes ({{8A
08 ...}} and {{88 02 03 F1}} afterwards).
Seven unittests in {{websocket.d}}: six fail before the change, the seventh
guards {{flush()}}.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
