Jens Geyer created THRIFT-6242:
----------------------------------
Summary: Honour the transport configuration in the C++
TNonblockingServer
Key: THRIFT-6242
URL: https://issues.apache.org/jira/browse/THRIFT-6242
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
{{TNonblockingServer}} kept its own frame-size limit ({{maxFrameSize_}}) and
created every accepted socket with no {{TConfiguration}}, so an operator who
configured a maximum frame or message size got no effect on the nonblocking
server: it enforced only its own frame-size ceiling, and each accepted socket
used the default 100 MB message budget. {{TConfiguration}} appeared in
{{TNonblockingServer}} only as the source of the default frame-size constant
(THRIFT-6183).
This wires the nonblocking server to a {{TConfiguration}}, following the
precedent that the configuration is the authority (THRIFT-6182 for the Java
transport layer, THRIFT-6183 for this server's default):
* The server holds a {{std::shared_ptr<TConfiguration>}}.
{{getMaxFrameSize()}}/{{setMaxFrameSize()}} operate on it, so there is one
source of truth for the frame-size limit.
* The configuration is handed to every accepted socket through the server
transport ({{TNonblockingServerTransport::accept()}} sets it on the socket it
returns, which covers both the plain and the TLS server sockets). The accepted
socket's message-size budget, and the layered transports built on top of it,
then use the operator's limits.
* A frame larger than the configured maximum message size is refused before the
read buffer is grown for it, next to the existing frame-size check
({{checkReadBytesAvailable(readWant_)}}). This is stable across the life of the
connection because the C++ socket budget gates each read and does not
accumulate.
Along the way, {{TTransport::setConfiguration()}} is corrected: it swapped the
configuration pointer but did not re-seed the message-size budget, which is
seeded once at construction, so a configuration installed after construction
never took effect. It now calls {{resetConsumedMessageSize()}}. It has no
callers within the library. (The re-seed loses nothing on an accepted socket:
both {{TSocket::read}} and {{TSSLSocket::read}} only
{{checkReadBytesAvailable(len)}} and never draw the budget down, and the
configuration is installed in {{accept()}} before the socket reads anything.)
Two behaviour notes for the reviewer:
* {{setMaxFrameSize()}} now operates on the shared {{TConfiguration}}, which
the server transport and every already-accepted socket also hold, where it
previously touched a server-private field. That is the intended Option-A
consequence (one authority), but it means a mid-serve {{setMaxFrameSize()}} is
now visible to live connections.
* {{setMaxFrameSize()}} clamps to {{INT_MAX}} because {{TConfiguration}} stores
the frame size as {{int}}; a value above {{INT_MAX}} previously fit the
{{size_t}} field but would now wrap to a negative {{int}}. The clamp only ever
tightens, never loosens.
Compatibility: an operator who sets a {{TConfiguration}} with a maximum below
what the server accepted before will start closing those connections; that is
the point of honouring the configuration. Default behaviour is unchanged (a
default {{TConfiguration}} carries the same 16,384,000 frame size and 100 MB
message size the server used before).
Scope note for the reviewer: this is part (2)+(3) of the nonblocking-server
frame handling. A separate ticket will cover part (4) -- growing the read
buffer as the payload arrives rather than reserving the whole declared frame up
front -- which is a read-state-machine change with a platform-shaped
(memory-observing) test and is kept separate so this change can land on its own.
Tests: three cases added to TNonblockingServerTest -- a frame above a
configured maximum frame size is refused; a frame above a configured maximum
message size (but below the frame ceiling) is refused before the buffer is
grown; and a generous configuration still serves ordinary traffic. All three
fail with the wiring neutralised and pass with it in place; the existing
nonblocking tests are unaffected.
_Drafted with AI assistance (Claude Opus 4.8)._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
