[
https://issues.apache.org/jira/browse/THRIFT-6242?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6242.
--------------------------------
Fix Version/s: 0.25.0
Assignee: Jens Geyer
Resolution: Fixed
> 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
> Assignee: Jens Geyer
> Priority: Major
> Labels: Breaking-Change
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {{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)
