Client: cpp
matchName(), which backs DefaultClientAccessManager, honoured a wildcard in
the leftmost label of a certificate name even when no label followed it, so
a dNSName or common name of "*", "f*" or "*." matched every single-label
host name. A wildcard stands in for the leftmost label of a longer name
(RFC 6125 6.4.3); a pattern whose wildcard label has no label after it no
longer matches.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Client: cpp
evhttp_make_request() can invoke the response callback synchronously, for
example on an immediate connection failure, before sendAndRecvMessage() had
queued the completion; finish() then ran on an empty completion queue. The
completion is now queued before the request is made, and removed again if the
request does not start, so finish() always finds the matching entry. finish()
also returns early on an empty queue instead of reading from it. The container
is a std::deque so the entry just queued can be removed from the back without
disturbing other pending requests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
fromString() returned a value that extraction had never assigned when the
numeric token was empty, and returned a clamped value when the token was
outside the target type's range, because it tested only bad() and eof(). It
now value-initialises the result and tests fail(), so an empty token and an
out-of-range value both raise a TProtocolException through readJSONInteger()
and readJSONDouble() instead. Two cases in JSONProtoTest cover them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
TSSLSocket::authorize() offered a certificate's IP subjectAltName entries to
the AccessManager on every connection, compared against the address returned
by getpeername(). In client mode that is the address the configured host
resolved to, so a client opened with a host name accepted a certificate that
named only that address.
The IP entries are now consulted in client mode only when the host the socket
was opened with is itself an address literal, recognised the way
TSocket::open() resolves it: getaddrinfo() with AI_NUMERICHOST. Connecting to
an address literal, server mode, and a socket opened from a descriptor with no
host are unchanged. An AccessManager of your own is not asked about IP entries
for a client opened with a host name.
Adds regression cases to SecurityTest, aligns the AccessManager documentation
in TSSLSocket.h and lib/cpp/README.md, and notes the change under Breaking
Changes for 0.26.0.
Co-Authored-By: Jens Geyer <jensg@apache.org>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Client: cpp
macOS rejects an RLIMIT_AS limit with EINVAL, so the child always exited with 2
and the case failed there. The child now exits with 77 in that case and the
case reports that it has nothing to check; any other setrlimit failure still
fails it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Client: cpp
With a non-blocking dispatch, serve() rejects and closes a client right
after accepting it, while the test client may still be writing its
request. When the close got there first, the send failed, the test
discarded that client, and if that happened to every rejected client
the check found no drop. too_many_pending_tasks_does_not_end_the_server
failed that way in about a third of the Windows CI jobs.
A fired client whose send fails with a transport error other than a
timeout now counts as dropped, like one that reads end-of-stream or a
reset.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Client: cpp
TThreadedServer::onClientConnected() inserts the new client into its active-client
map and then starts a worker thread for it. When the thread cannot be started --
the process is out of threads, so std::thread's constructor throws -- two things
went wrong on the default (non-detached) configuration:
- Thread::start() had already run setState(starting) but left thread_ null, and
~Thread()/join() dereferenced thread_ unconditionally for a non-detached thread,
so destroying the failed worker crashed.
- The exception is not a TTransportException, so the accept loop's only catch did
not handle it; it unwound out of serve() and the server stopped accepting, with
the client left in the active-client map.
Null-guard ~Thread()/join() so a never-started thread can be destroyed safely, and
catch the failure in onClientConnected(), remove the client from the map, and keep
serving -- the way TThreadPoolServer drops one client on a rejected dispatch and
continues.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
TConnection::transition() can close a connection while a request is processed
inline, which returns the connection object to the server to be reused or
deleted; workSocket() then read the connection's members. transition() now
reports whether the connection is still owned by the calling I/O thread, and
workSocket() stops working the socket as soon as it is not.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
ASN1_STRING_to_UTF8() returns a negative length and leaves its output
pointer untouched for a commonName whose string type it cannot convert.
The fallback loop now scopes that pointer to each iteration and consults
the length before the name is used or released.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
A THeader frame applies each transform it lists in turn, so the work one
frame can request grows with the number of transforms rather than with its
declared size. A valid frame never lists more than one, so refuse a larger
count when the header is parsed, before the transform list is built.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
TQIODeviceTransport::readAll() retries a zero-length read after waiting on
the device. When the wait fails and the device can no longer deliver -- a
socket that has reached the unconnected state, or a non-socket device at its
end -- it now ends with END_OF_FILE instead of retrying without progress.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
A failed accept() reached libevent's listen callback as an exception and
ended the only listener loop. The callback now logs that one failure and
keeps accepting, the same boundary the connection and notification
callbacks already use.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
A thread-pool dispatch that the ThreadManager rejected -- a full pending
queue, or a non-blocking add() that could not enqueue -- threw past serve()
and stopped the server. That one client is now closed and serving continues,
as the Java TThreadPoolServer does on a rejected execution.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
lib/cpp/test and test/cpp built the SSL and non-blocking server tests against
OpenSSL and libevent without adding all of their include and library paths, so
they did not build when either is outside the default search path, as with
Homebrew. lib/cpp already adds them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Client: cpp
TPipeServer did not implement interruptChildren(), so TServer::stop() could
not end a client that was waiting on a pipe, and serve() did not return while
a client stayed connected.
TPipeServer now shares one event with every pipe it accepts.
interruptChildren() sets it. From then on, read() on such a pipe throws
TTransportException::INTERRUPTED, even when the client has sent data, as it
does on a TSocket that TServerSocket accepted. A write() that waits for the
client is interrupted as well. Closing the server sets the event too, again as
with TServerSocket, and setInterruptableChildren(false) turns all of this off;
it must be called before listen(). Anonymous pipes do synchronous I/O and are
not interrupted.
This changes behavior: TServer::stop() now disconnects the clients of a
TPipeServer instead of waiting for them, as it does for TServerSocket since
THRIFT-2441. setInterruptableChildren(false) restores the old behavior.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Client: cpp
close() may run on a different thread than an in-flight read() or write().
Guard the implementation pointer with a critical section and hand each caller
its own reference, so that the implementation cannot be destroyed while a call
is using it.
close() now detaches the implementation before releasing it and then cancels
it, so that a read() or write() waiting on the pipe stops waiting for the peer
and throws TTransportException::INTERRUPTED. Every overlapped wait also watches
a cancel event of the implementation, which is set once and never reset, so a
cancel that comes before the I/O is issued is not lost. setPipeHandle() does
the same with the implementation it replaces. Anonymous pipes use synchronous
I/O and are not interrupted; their handles stay open until a call in progress
returns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Client: cpp
The completion callback was the only place that deleted the per-request
RequestContext, so a processor that threw before invoking that callback --
as an async dispatcher does on a body too short to parse a message header --
left the context and its buffers allocated. Hold the context in a
std::unique_ptr and release ownership to the callback only after
processor_->process() returns without throwing: the throw path frees it as
the stack unwinds, and the success path still deletes it exactly once.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
THeaderTransport::writeVarint32 emitted the continuation bytes by
shifting a signed int32 to the right, which sign-extends, so a negative
value never cleared its high bits and was not encoded as the canonical
five-byte varint the reader decodes. Encode over an unsigned copy so
the low 32 bits of any input round-trip through
writeVarint32/readVarint32.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
THeaderTransport::untransform and transform initialize a zlib stream and
free it with inflateEnd/deflateEnd on the success path, but some error
checks between init and that call exited before reaching it. Free the
stream on those paths too, and move untransform's post-transform
frame-size check after inflateEnd, so the stream is always released once
it has been initialized.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
TWebSocketServer overrides readAll_virt(), which reads WebSocket frames. The
non-virtual readAll() it inherited, however, comes from
TVirtualTransport<THttpTransport> and reads through THttpTransport::read().
A readAll() called on a TWebSocketServer, rather than through a TTransport,
therefore read the HTTP stream instead of the frame payload. TWebSocketServer
now defines readAll() to call readAll_virt(), as the note on
TVirtualTransport::readAll() suggests, so both calls behave alike.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
readAll on TWebSocketServer now returns the number of bytes it was asked
for, drawing on further frames when one frame does not carry them all, and
raises at end of stream once a request has begun. That is the contract the
base transport and the other bindings' readAll already meet.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Generated C++ list decoders sized the vector with resize() from the element
count in the list header before reading any element, then read into each slot by
index, so a peer could name a count it never backs with data and have the decoder
allocate for all of it. The generator now reserve()s a capacity capped by
apache::thrift::protocol::preallocSize (1024 elements) and appends each element
with emplace_back(), so the vector still ends with every element read but the
up-front allocation is bounded; larger lists grow as elements are added. Maps and
sets were already built by insertion and are unchanged. This mirrors
thrift.PreallocSize in the Go binding and prealloc_size in Rust.
Client: cpp
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: build
- thriftl.ll: include <stdint.h> in a %top block. Flex defines INT8_MIN and
the other limit macros itself when not compiled as C99, guarded by
#ifndef, and MSVC's stdint.h then redefined them (9 x C4005 per run).
- lib/py/setup.py: pass -std=c++11 only to non-MSVC compilers; cl.exe
warned D9002 and ignored it.
- CMakeLists.txt: set CMP0144 to NEW next to CMP0074. The MSVC build passes
LIBEVENT_ROOT and CMake 3.27+ warned that find_package ignores it.
- FindClangTools.cmake: declare FPHSA_NAME_MISMATCHED; the module finds
three tools under names that differ from its own by design.
- SecurityTest.cpp: cast away the shared_ptr that exists only to make the
constructor throw (C4858).
Left for later, see the ticket: LNK4098 from libevent built /MT (image
change, after THRIFT-6237), C4244 in protocol.tcc (THRIFT-6069 rewrites the
file), and CS0114 in the generated netstd Xception.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Client: cpp
The previous commit resolved the bind address with
AI_PASSIVE|AI_V4MAPPED|AI_ADDRCONFIG and fell back to AI_PASSIVE|AI_V4MAPPED,
on the grounds that AI_V4MAPPED is ignored for AF_UNSPEC queries. That holds
for glibc but not for Android: bionic's getaddrinfo() fails any flag outside
its AI_MASK, which does not include AI_V4MAPPED, with EAI_BADFLAGS before it
looks at the family. With the ANDROID branch gone, both attempts were rejected
there and every TCP server socket's listen() threw "Could not resolve host for
server socket." -- the case ab72ebe56 had worked around.
Resolve with exactly the flags TSocket::open() uses instead,
AI_PASSIVE|AI_ADDRCONFIG and then AI_PASSIVE alone. On glibc nothing changes
-- both flag sets give the same addresses for "localhost", "::1", "127.0.0.1"
and the wildcard -- and on Android the first attempt is what the ANDROID
branch passed, so it resolves as before and only gains the fallback.
AddressResolutionHelper's default flags carried the same AI_V4MAPPED. Nothing
in the library relies on the default, but the header is installed, so it now
defaults to AI_ADDRCONFIG alone. TServerExceptionTest.cpp explained its fixed
127.0.0.1 with the servers' old flags; that comment no longer describes the
library and is reworded.
Nothing in CI builds for Android, so bionic's flag check was emulated with an
LD_PRELOAD getaddrinfo() that rejects flags outside its AI_MASK. Before this
commit TNonblockingServerTest and TServerExceptionTest fail under it; after
it they pass, and so does TServerSocketTest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0167dH5uDP18NpE5dNpnbzfK
Client: cpp
TServerSocket::listen() and TNonblockingServerSocket::listen() resolved the
bind address with AI_PASSIVE|AI_V4MAPPED, while TSocket::open() resolves the
connect address with AI_PASSIVE|AI_ADDRCONFIG. AI_ADDRCONFIG does not count a
loopback address as a configured one, so on a host that carries ::1 on lo and
no other IPv6 address -- a default Docker container, for instance -- the two
disagree about what "localhost" means: the server binds ::1 and the client
dials 127.0.0.1, and the connect fails with ECONNREFUSED. The IPV6_V6ONLY=0
the server sets does not help, because the bind is to ::1 rather than to ::.
Last worked in 0.13.0. THRIFT-5186 removed AI_ADDRCONFIG from the server
sockets in 9b9567b23 so that a host with no configured address could still
resolve localhost, and gave TSocket::open() a retry without the flag for the
same case. The client kept the flag; the servers did not, and the asymmetry
is what breaks. THRIFT-5880 later widened that client retry to EAI_ADDRFAMILY,
but it is conditional on resolution failing and does not fire here.
Resolve on the server with AI_ADDRCONFIG as well, and fall back to resolving
without it when that leaves nothing to bind -- the same shape TSocket::open()
already has, so both sides agree again while THRIFT-5186's case keeps working.
The fallback is load-bearing rather than defensive: an explicit "::1" fails
with EAI_ADDRFAMILY under AI_ADDRCONFIG on such a host. The ANDROID branch is
now redundant and goes away; Android already passed AI_ADDRCONFIG and only
gains the fallback, since AI_V4MAPPED is ignored for AF_UNSPEC queries.
lib/cpp/test/TServerSocketTest.cpp's test_bind_to_address already covered this
for TServerSocket and failed in such a container; TNonblockingServerSocket had
no equivalent coverage and now has one. Both were confirmed to fail against the
unmodified library before the fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
Size the zlib transform buffer from deflateBound() and compress in a single
pass, and let the write buffer grow to the size of the compressed frame.
Link zlib into the autotools UnitTests target, whose tests call it directly,
as TransportTest and ZlibTest already do.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
THeaderTransport::transform() copied the deflate result back into the
framing buffer without sizing that buffer to the compressed length.
Incompressible input expands under deflate, so a frame that filled the
buffer no longer fit once compressed. The compression loop also grew the
scratch buffer only up to wBufSize_ + DEFAULT_BUFFER_SIZE, so a larger
result restarted deflate into the same buffer and its total length ran
past the end.
Size the scratch buffer to zlib's worst-case bound, compress in a single
pass, and grow the write buffer when the compressed frame is larger than
the source. Add two regression tests: a small incompressible frame that
round-trips through the transform, and a large one whose emitted payload
is inflated directly and compared, so it holds without a sanitizer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
bad_alloc_does_not_end_the_process failed intermittently on AppVeyor, 11 of
229 jobs since it was added, either crashing or hanging until the 300 s
ctest timeout. Two independent races, one behind each symptom, so both are
fixed here.
The crash: the fixture stopped the server and joined the serve thread but
never stopped the ThreadManager. Members are destroyed in reverse
declaration order, so the server went before the thread manager, and
~TNonblockingServer deletes every TConnection and the IO threads along with
their notification pipe. A pool task still unwinding from the injected
failure then reached notifyIOThread() -- and close() -- on the TConnection
it holds by raw pointer. The fixture destructor now stops the ThreadManager
after joining the serve thread and before the members go;
ThreadManager::stop() returns only once each worker has finished the task in
its hands, so nothing is left running over the objects being destroyed.
The hang: FailsFirstCallProcessor failed whichever call reached process()
first, tracked in a plain bool that both pool workers read and wrote. When
the call from canCommunicate() won that race it consumed the failure and got
no reply, and its client had no receive timeout, so the test body blocked
until ctest killed it. The flag is now claimed with an atomic exchange, the
processor signals a Monitor as it claims it, and the test waits for that
signal before opening the second connection -- so the failure is always
taken by the call meant to receive it. The exchange matters on its own:
read-then-write let two calls each see the flag unset and both throw, which
is what the Windows logs showed as a doubled bad_alloc line.
canCommunicate() also sets a 10 s receive timeout. Every call it makes is a
localhost round-trip of a few bytes, so this only bounds the failure: a
regression reports in seconds with the transport exception rather than as an
opaque 300 s timeout.
Verified by forcing each schedule rather than waiting for the race:
- delaying the first arrival at process() so the second connection claims
the failure reproduces the hang exactly; without the wait the case fails
in 10 s with THRIFT_EAGAIN, with it the case passes.
- holding the failing task in flight past the end of the test body puts
ASan on the use-after-free with the ThreadManager stop removed --
notifyIOThread() reading a TConnection freed by ~TNonblockingServer()
through Fixture::~Fixture() -- and reports nothing with it present.
Then 5 clean runs of the full suite, and 200 runs of this case under ASan
with eight in parallel.
Not touched: allocation_failure_on_the_io_thread_does_not_end_the_process
fails under ASan because its RLIMIT_AS child collides with ASan's own
allocator. That reproduces identically on master.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
THRIFT-6174 selected const-qualified X509 types in authorize() from OpenSSL
1.1.0 onwards, on the grounds that every consumer of those values had taken
const since 1.1.0. X509_NAME_get_index_by_NID() is the exception. It was
constified in 3.0, and 1.1.1 still declares it as
int X509_NAME_get_index_by_NID(X509_NAME *name, int nid, int lastpos);
so a const name cannot be passed to it there and the library does not compile.
AppVeyor is the only CI configuration that builds against a pre-3.0 library --
it reports 1.1.1w -- and all five of its MSVC jobs failed with C2664 on that
call. Every GitHub Actions job builds against 3.x and stayed green.
Moving the three guards to 3.0 keeps the const path compiled by those 3.x
configurations, which was THRIFT-6174's reason for picking 1.1.0 over 4.0
rather than shipping an arm that nothing ever compiles. Below 3.0 all three
getters still return non-const pointers, and every consumer takes them either
way, so that arm is the code that shipped before THRIFT-6174.
c_glib needs no matching change: thrift_ssl_socket.c does no X509_NAME lookup
at all, it matches the host through SSL_set1_host().
The regression test is the existing AppVeyor matrix, which builds against
OpenSSL 1.1.1w; no CI configuration had to be added to cover this.
Verified by building lib/cpp and its tests against a from-source 1.1.1w and
against the distribution 3.0.2. Before the change 1.1.1w fails with exactly
one error, the same one MSVC reports; after it both libraries build clean and
ctest is 57/57 on each. A preprocessor probe confirms 1.1.1w takes the
non-const arm and 3.0.2 the const one, and SecurityTest runs its full matrix
on both -- 29 cells on the 1.1.1w build, which has SSLv3 enabled, and 20 on
3.0.2, which does not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp,c_glib
OpenSSL 4.0 removes SSLv3_method(), TLSv1_method(), TLSv1_1_method(),
TLSv1_2_method(), ERR_remove_state() and ASN1_STRING_data(), and returns
const pointers from X509_get_subject_name(), X509_NAME_get_entry() and
X509_NAME_ENTRY_get_data(). Neither binding compiles against it.
The per-version methods are replaced by TLS_method() with the protocol
window pinned through SSL_CTX_set_min_proto_version() and
SSL_CTX_set_max_proto_version(), which is OpenSSL's own documented
migration and reproduces the old methods exactly: TLSv1_2_method() meant
TLS 1.2 only, and min == max == TLS1_2_VERSION means the same. No enum
value is dropped, so SSLv3, TLSv1_0, TLSv1_1, TLSv1_2 and LATEST keep
working on 4.0 rather than starting to throw "Unknown protocol".
The replacement is selected from 1.1.0 onwards rather than only on 4.0,
because no CI configuration builds against 4.0: gating it there would ship
a code path that nothing ever compiles, let alone runs. At >= 1.1.0 the
existing SSL matrix tests cover it on the OpenSSL 3.x that CI does have.
The same reasoning applies to the const qualifiers, whose consumers have
all taken const since 1.1.0. LibreSSL is excluded throughout: it reports
OPENSSL_VERSION_NUMBER as 0x20000000L but keeps the pre-1.1.0 signatures,
so it stays on the path it compiles today.
SSLTLS is untouched. SSLv23_method() is not removed in 4.0 -- it is an
unconditional #define for TLS_method() -- so that arm needed no change,
and the protocol floor for it still comes from the SSL_CTX_set_options()
call below, not from a version window.
SSLv3 keeps SSLv3_method() below 4.0, which is the only release that
removes it, rather than becoming a TLS_method() window like the others:
SSL_CTX_set_min_proto_version() consults the security level and would
refuse SSL3_VERSION at the default one, where the method function does not,
and the context is built before TSSLSocketFactory::ciphers() lowers it. On
4.0 the protocol is gone outright and the arm is compiled out.
ERR_remove_state() has been an empty stub since 1.1.0, so in c_glib it is
gated to where it still does something: before 1.1.0, and on LibreSSL,
which implements it and has never shipped OPENSSL_thread_stop(). The C++
binding gets the matching guard so LibreSSL reaches the ERR_remove_state()
arm instead of losing per-thread cleanup on both paths.
Tests:
- SecurityTest gains explicit_protocol_version_window, which asserts the
min/max window per enum value instead of merely that a context was
created -- the latter passes on a remapping that silently leaves the
floor to the library default. Proven two-state: with the window not
pinned it fails 8 assertions, LATEST among them, reporting floor 0.
It also asserts that an unavailable SSLv3 is reported rather than
quietly satisfied with another version.
- Both matrix tests skip SSLv3 on 4.0. OPENSSL_NO_SSL3 is not defined
there, so without this they try SSLv3 and time out.
- testtransportsslsocket gains the same two cases for c_glib.
- TSSLSocketMatchNameTest built its subject name through
X509_get_subject_name(), which is const as of 4.0, then mutated it; it
now builds a separate X509_NAME. It also had no SIGPIPE handler while
deliberately provoking rejected handshakes, so it died with status 141
in 6 of 20 runs before this change and 11 of 20 after -- the allocation
pattern shifts the race. It is ignored now, as SecurityTest already
does, and 25 of 25 runs pass.
Verified by building and running lib/cpp and lib/c_glib against three
libraries: the distribution OpenSSL 3.0.2, a from-source 4.0.2, and a
from-source 3.0.2 configured with enable-ssl3 so that the one otherwise
uncompiled arm is covered. All 16 matrix cells pass identically on the
first two, and all 25 on the third -- cell for cell what master produces
there. Unpatched master fails to compile against 4.0.2 with 7 errors in
C++ and 5 in c_glib.
Co-Authored-By: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Co-Authored-By: Jaipaul Cheernam <jaipaul.cheernam@est.tech>
Co-Authored-By: Brad Smith <brad@comstyle.com>
Co-Authored-By: loqs <7595223+loqs@users.noreply.github.com>
Co-Authored-By: Finn R. Gärtner <65015656+FinnRG@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Client: cpp
TConnection reserved the whole declared frame the moment it read the
four-byte length prefix: transition() reallocated the read buffer to the
frame size before any payload byte had arrived, so a peer could make the
server reserve up to the maximum frame size by sending only a header.
The buffer now grows toward the frame size in SOCKET_RECV as bytes are
actually read -- by doubling, with the growth guarded against uint32_t
overflow -- so the reservation tracks the payload received. transition()
reserves only the length prefix and a small initial read. A frame that
fully arrives ends at the same buffer size as before; a header with no
payload reserves next to nothing. A large frame that does arrive pays
the amortized doubling copy any grow-as-you-go buffer pays.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
TNonblockingServer kept its own frame-size limit and created accepted
sockets with no TConfiguration, so an operator who configured a maximum
frame or message size got no effect: the server enforced only its own
frame-size ceiling, and each accepted socket used the default 100 MB
message budget.
The server now holds a TConfiguration. getMaxFrameSize()/setMaxFrameSize()
operate on it, and it is handed to every accepted socket through the
server transport, so the configured maximum message size bounds each
connection and the layered transports built on top of it. A frame larger
than the message budget is refused before the read buffer is grown for
it, alongside the existing frame-size check.
setMaxFrameSize() clamps to INT_MAX, since the configuration stores the
frame size as int; a value above INT_MAX would otherwise wrap to a
negative int and read back as a huge size_t, disabling the limit.
TTransport::setConfiguration() now re-seeds the message-size budget from
the new configuration. It previously swapped the pointer without
re-seeding, so a configuration installed after construction never took
effect; it has no callers within the library.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Client: cpp
TSSLSocketMatchNameTest runs its listener on a boost::thread, but its LDADD
carried neither $(BOOST_SYSTEM_LDADD) nor $(BOOST_THREAD_LDADD), so the
autotools build could not link it:
undefined reference to `typeinfo for boost::detail::thread_data_base'
Every other test here that starts a thread already links both. The CMake build
supplies them on its own, which is why lib-cpp passes under sca.yml and fails
under build.yml.
Verified by building the test and running it: it links against libboost_thread
and reports 8 test cases with no errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
TSSLSocket::authorize() walks the subjectAltName entries and then falls
through to commonName whenever no entry answered ALLOW. DefaultClientAccess
Manager::verify returns SKIP for a name that does not match, not DENY, so
"the certificate names other hosts" and "the certificate names no hosts"
arrive at that fallthrough as the same state, and commonName gets to answer
for a certificate that already had its say.
RFC 6125 6.4.4 and RFC 9525 6.3 put it the other way round: once a dNSName
is present it is the identity, and commonName is for certificates that
carry none. authorize() now records whether a dNSName was seen at all,
separately from whether one matched, and consults commonName only when none
was.
The test drives real handshakes against certificates built in the test
rather than fixtures, because what is under test is the relationship
between two fields of one certificate. Three cases: a matching
subjectAltName is accepted, a non-matching one is refused even though the
commonName would have matched, and a certificate with no subjectAltName is
still served by its commonName. The middle one fails against the unmodified
library.
lib/cpp/README.md documented the old rule -- "if all subjectAltName extensions
were skipped, the common name field would be checked" -- and got two details of
it wrong besides: the common name reaches the application through (b), not (c),
and "data" is the common name, not an IP address. That paragraph is rewritten,
and the change is written up under 0.25.0 in Breaking Changes.
Two things fell out of writing it, both fixed here:
- TSSLSocketMatchNameTest was in Makefile.am but not in CMakeLists.txt, so
ctest never ran it. It is wired into both now.
- TSSLSocket::open() performs no handshake -- it is deferred to the first
I/O through checkHandshake() -- so a test that opens and closes proves
nothing about the certificate. The first version of this test did exactly
that and passed against unfixed code. The client now reads a byte, and
the comment on connectAccepts() says why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
TConnectedClient::run() caught TTransportException and TException and nothing
else, so any other exception left the client loop. Where it went from there
depends on the server: TSimpleServer runs the loop on the serve thread, so it
escaped serve() and the listening socket closed; TThreadedServer runs it on a
thread of its own, whose entry point has no handler either. Neither has anything
above it that catches, so the process ended.
Measured, one request whose processing throws std::bad_alloc:
TThreadedServer SIGABRT
TSimpleServer SIGABRT
TThreadPoolServer survives -- the thread manager's worker catches it
Three of the four servers, and the exception need not come from a handler:
reading a message whose declared sizes do not fit in memory raises std::bad_alloc
before any generated per-function try is entered, which is why the C++ generated
dispatch does not turn it into a TApplicationException.
The loop now also catches std::exception and everything else, doing what the
TException clause beside it already does -- log it and disconnect, which is what
that clause's own comment says it is for. cleanup() runs as before.
lib/cpp/test/TServerExceptionTest.cpp covers all three servers and is wired into
both CMakeLists.txt and Makefile.am; a test in only one of them is a test that
does not run under ctest. Two of the three cases fail against the unmodified
library. It binds 127.0.0.1 rather than localhost on purpose: TServerSocket
resolves with AI_V4MAPPED and TSocket with AI_ADDRCONFIG, so where localhost is
::1 and the loopback has no IPv6 address the two disagree and the test measures
that instead (THRIFT-6191).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
libevent is a C library, and TNonblockingServer hands it two callbacks:
eventHandler(), which drives TConnection::workSocket(), and notifyHandler(),
which drives TConnection::transition(). An exception thrown under either does
not reach a handler -- it unwinds into C frames and reaches std::terminate.
transition() can throw: growing the read buffer to the frame size the peer
declared ends in throw std::bad_alloc() when the allocation fails, and that is
the only throw in the function that is not already caught. The frame size is the
peer's choice up to the server's maximum, so the peer chooses the size of the
allocation that has to succeed.
Measured against the unmodified library, with the address space capped and the
maximum frame size raised above it: one connection sending a four byte frame
header and nothing else brings down the process with "terminate called after
throwing an instance of 'std::bad_alloc'" and SIGABRT. That is worse than the
exit(1) removed in the previous commit -- it needs no thread manager, so it
applies to the default configuration.
Both callbacks now catch at the boundary and close the connection, which is
what every other error path in workSocket() and transition() already does. The
process keeps serving its other connections; measured again, it does.
The test forks, caps the address space in the child and lets the child exit 0
if it survives, because the cap cannot be undone for the rest of the suite. It
is guarded for POSIX; RLIMIT_AS has no Windows equivalent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
Task::run() caught std::bad_alloc apart from every other exception and answered
it with exit(1), so one request whose processing could not be allocated ended
the process for every other client connected to it. The catch clause below it
already handles std::exception -- logs it and hands the connection back to the
I/O thread -- and the inline, non-thread-pooled path in the same file has always
just logged and closed the connection. The special case is removed and
std::bad_alloc takes that same route.
Measured: a TNonblockingServer with a thread manager, one framed call whose
processing throws std::bad_alloc, and the process is gone with status 1. The
allocation that fails need not be in application code -- generated container
deserialization allocates from a declared element count before any handler
runs, which is what makes this reachable from the wire.
lib/cpp/test/TNonblockingServerTest.cpp gains a case; the fixture learns to run
a server on the thread-pooled path, since every existing case uses the inline
one. Against the unmodified library the test binary does not fail an assertion,
it exits mid-run and takes the other four cases with it.
The case deliberately does not wait for a reply on the doomed connection. A
task that throws writes nothing, and the connection reads an empty write buffer
as no reply being owed. That is not the same thing: a void method is not a
oneway method, it still owes a reply, and an exception is one of the replies it
can owe. So that one caller waits for ever. The confusion is pre-existing,
applies to every exception this catch handles, and differs from the inline path,
which closes the connection; left alone here and recorded as its own question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
Two cases in ToStringTest call std::locale::global() and never put back what
was there. Setting it is the point -- they prove to_string() does not follow
the global locale, which is why TToString.h imbues a default one on the
streams it builds -- but leaving it set is not. Every stream constructed later
in the same UnitTests binary picks it up, in suites that have nothing to do
with this one, and numbers formatted through an ostringstream come out with
digit grouping.
That has already cost a debugging session. A case in THttpBufferBoundTest
formats a chunk size with "size << std::hex << 4096" and got "1.000" rather
than "1000", so the transport under test read a different stream, and the case
passed or failed depending on whether it ran alone or in a full run --
ToStringTest is disabled by default and only runs when the whole binary does.
A scoped guard restores the previous global locale on exit. The cases still
set the locale for their own duration, so what they assert is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thrift-0.24.0/lib/cpp/src/thrift/transport/TBufferTransports.h:110:32:
error: ‘ptrdiff_t’ does not name a type
Signed-off-by: Bernd Kuhls <bernd@kuhls.net>
Client: cpp
The declared content length, and the number of chunks in a chunked body, are
numbers the peer chooses, and neither was measured against anything.
readContent() writes straight into readBuffer_, so the body does not pass
through the line buffer whose growth is already bounded, and
remainingMessageSize_ is never decremented anywhere in this transport -- the
checkReadBytesAvailable() in read() therefore only ever caps a single caller
request, and the allowance it checks against never depletes.
Measured against the unmodified library, with maxMessageSize at 100 MB: a
request declaring 4294967295 and then sending 200 MB left 205 MB resident, the
declared number itself costing nothing because nothing is allocated up front.
A chunked body has no declared total at all and simply runs until the peer
stops.
readContent() now charges what it is about to read against maxMessageSize and
refuses the message once it would exceed it, with the total reset by
readHeaders() so that each message on a keep-alive connection gets its own
allowance.
Four tests. Two fail against the unmodified library, and both count the bytes
the peer was asked for rather than asking whether something threw: an
over-declared length runs the stream out and throws either way, so "did it
throw" would have passed against the defect. 262188 bytes served against a
65536 maximum for the declared case, 262708 for the chunked one. The other two
are regression guards -- a body inside the maximum still arrives, and a second
request on the same connection still gets a full allowance.
The chunked test builds its chunk sizes through an ostringstream, and a stream
follows whatever global locale is in force, so 4096 came out as "1.000" and
stopped being a chunk size. It imbues the classic locale, which is what
TToString.h does and for the same reason. The case that left a grouping locale
set for the rest of the binary is fixed separately under THRIFT-6194; the
imbue here stays as the local guarantee.
lib/cpp/README.md gains a third paragraph under Breaking Changes 0.25.0: a
deployment exchanging bodies larger than the configured maximum has to raise it.
Stacked on THRIFT-6192, which shares the file and the README section.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
THttpServer::parseHeader compared a name for as many characters as the peer
had sent before the colon:
size_t sz = colon - header;
if (THRIFT_strncasecmp(header, "Content-length", sz) == 0) ...
so every name that is a prefix of one the transport knows was accepted as that
name. Measured against the unmodified library: "C: 5", "co: 7" and
"Content-len: 9" each set the content length, and "T: chunked" switched on
chunked decoding. A header name is the whole token before the colon (RFC 9110
5.1), and a transport that reads it otherwise disagrees with every other party
on the connection about where a message ends -- one of these requests carries a
five byte body here and none at all to a parser that follows the grammar.
TWebSocketServer::parseHeader is the same idiom on the same base class, and the
handshake is satisfied by "U: websocket", "C: Upgrade" and "S: <key>"; note
that "S:" reaches the Sec-WebSocket-Key arm, which is tested before the version
one. Fixed here as well, together with the "value + 1" beside it that dropped
the space after the colon by stepping over it, and read one past the terminator
for a header that has no value at all.
Content-Length went through atoi(), which cannot report that it was handed a
negative number, something that is not a number, or a value too large to hold.
"Content-length: -1" therefore arrived in the uint32_t member as 4294967295.
It is now parsed with strtoll against RFC 9110 8.6's 1*DIGIT and refused
otherwise; the client had the same line and shares the new helper, though its
name comparison was already an equality and needed nothing.
lib/cpp/README.md gains a Breaking Changes entry under 0.25.0: refusing an
abbreviated name, and refusing a malformed Content-Length, can turn away
traffic that works today.
Nine tests, written first. Six fail against the unmodified library. The two
that cover a bad number assert the length that was parsed rather than that the
read threw, because an over-declared length outruns a five byte body and throws
on the unmodified library too -- "did it throw" would have passed either way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TNonblockingServer caps the frame it will accept at its own MAX_FRAME_SIZE,
256 * 1024 * 1024 since THRIFT-1337 landed it in 2011 -- years before
TConfiguration existed. The library-wide default is
TConfiguration::DEFAULT_MAX_FRAME_SIZE, 16384000, described in its own header
as "used consistently across all Thrift libraries". The two disagree by a
factor of 16:
TNonblockingServer::getMaxFrameSize() 268435456
TConfiguration::DEFAULT_MAX_FRAME_SIZE 16384000
TConfiguration appears nowhere in TNonblockingServer.h or .cpp, so the server
neither reads a configured value nor inherits the default; it starts from its
own constant and only setMaxFrameSize() moves it. That constant is what sizes
the read buffer, since TConnection::transition() resets the buffer to the
length the peer declared before any payload byte has arrived.
Java's AbstractNonblockingServer has no such gap: its per-frame limit comes
from trans_.getMaxFrameSize(), that is from TConfiguration, and it keeps a
separate aggregate read-buffer budget on top of it.
So point MAX_FRAME_SIZE at TConfiguration::DEFAULT_MAX_FRAME_SIZE. The
constant is private, so this is not an API change, and setMaxFrameSize()
still overrides it.
This lowers a shipped default. A deployment that accepts frames between
16,384,000 and 268,435,456 bytes on TNonblockingServer today and does not
call setMaxFrameSize() will start closing those connections, so it needs a
release note.
One test. It fails before the change on 268435456 != 16384000, and pins that
the setter still wins so the escape hatch cannot quietly go away.
Client: cpp
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Client: cpp
THttpTransport keeps the line it is reading in httpBuf_ and doubles
httpBufSize_ whenever a line does not fit. Nothing bounded the doubling, so
a peer that never sends a CRLF decided how large the buffer became, and once
httpBufSize_ reached 2 GiB the next doubling wrapped the uint32_t.
Hold the growth to TConfiguration::maxMessageSize, never below the buffer the
transport starts with so a small limit cannot make an ordinary request line
unreadable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Client: cpp
readFrame() reads a four-byte version word out of the frame and then reads
the remainder as sz - 4. sz is unsigned, so a frame declaring fewer bytes
than that word wrapped the read length instead of being rejected. Only the
header-format branch carried a lower bound of its own; move one bound in
front of all three branches.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three related framing defects in TWebSocketServer, all demonstrated against
master.
The masking key of a zero-length masked frame was never consumed. readFrame
read the four bytes inside "if (length > 0)", but RFC 6455 5.2 puts the
masking key in every frame whose MASK bit is set whatever the payload
length, and readFrame itself refuses a client frame that does not set MASK.
The four bytes stayed in the stream and the next header was parsed out of
the masking key:
client: 89 80 37 37 37 37 an empty Ping
82 85 37 37 37 37 ... a five-byte data frame
server: TTransportException: Reserved bits must be zeroes
An empty Ping, an empty Pong and a Close with no status code are all
ordinary traffic, so nothing unusual on the wire is needed to reach it.
A Pong declared a length of zero and then wrote the payload after it.
writeFrameHeader took its length from writeBuffer_.available_read(), which
describes the body only for flush(), the one caller whose body is the write
buffer. A Ping of eight bytes produced
8A 08 ... expected 8A 00 50 49 4E 47 44 41 54 41 written
so a conforming client read a zero-length Pong and then parsed 0x50 as the
next frame header, whose reserved bits are set. failConnection did the same
with its two-byte close code, 88 00 03 F1, which made the reason the
connection ended unreadable.
So pass writeFrameHeader the length it is describing instead of letting it
guess, read the masking key as part of the header it belongs to, and reset
the read buffer for a frame with no payload, so that a Pong for an empty
Ping echoes nothing rather than whatever the frame before it left unread.
On the wire, after: 8A 08 50 49 4E 47 44 41 54 41 and 88 02 03 F1.
Five tests. Four fail before the change -- two on the exception, two on the
bytes written. The fifth is a regression guard on flush(), the caller whose
body really is the write buffer, and passes either way.
Client: cpp
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWebSocketServer::readFrame answered a Ping by calling itself:
case Opcode::Ping:
pong();
return readFrame();
A Ping carries nothing for the caller, so the reader has to go on to the
next frame to satisfy it. Doing that by re-entering readFrame costs one
stack frame per Ping, and nothing bounds how many a peer may send.
Measured against master, with Ping frames of one payload byte -- seven
bytes each on the wire -- followed by one ordinary data frame:
20,000 pings (140 kB) -> 3,200,344 bytes of stack
40,000 pings (280 kB) -> 6,400,344 bytes of stack
60,000 pings (420 kB) -> segmentation fault
160 bytes of stack for every seven bytes of input, so the 8 MB a Linux
thread starts with is gone by about 52,000 pings. It is not build
dependent: gcc 11 does not turn the call into a jump at -O2 any more than
at -O0, and both builds fault at the same point. With the loop the same
run reads from a constant depth -- 416 bytes at 60,000 pings and at a
million, where the wire cost is 7 MB.
A loop is the whole change. Pings are still answered one after another for
as long as a peer cares to send them, no configuration is involved, and
the rest of the body is untouched apart from its indentation.
The test asserts how far below its own frame the reader got rather than
waiting for a crash: 2,000 pings, four orders of magnitude short of
exhausting anything, take the unmodified reader 320,480 bytes deep against
a 64 kB ceiling, and the fix holds it constant.
Worth knowing for anyone reading this code: a Ping with no payload does
not reach this path at all, because readFrame does not consume the four
masking-key bytes of a zero-length masked frame and the next header is
parsed out of the masking key instead. That is a separate defect, filed
separately, which is why the numbers above use one-byte pings.
Client: cpp
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWebSocketServer::readFrame read a frame's payload with a single call to
the transport underneath and treated anything short of the whole payload
as end of stream:
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 itself,
so the payload arrives in as many pieces as the network chose. Returning
false makes readAll_virt return 0, which the caller reads as a closed
connection.
So no frame whose payload does not arrive in one read could be received at
all. Demonstrated with a TServerSocket, a TWebSocketServer over the
accepted socket, and a client writing a well-formed 4000-byte frame as
2000 + 2000 bytes with a pause between: readAll returned 0 of 4000. In
practice that is any frame past the path MTU, so this transport could not
carry a Thrift message beyond roughly 1.4 kB over ordinary Ethernet.
Read the payload with readAll(), which is how TFramedTransport reads its
own frames, and keep the existing signal for a peer that leaves 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 waits for the number of bytes the frame declared, which is only safe
because THRIFT-6177 holds that number to TConfiguration::maxFrameSize; the
wait is bounded by that and by the socket's own receive timeout, the same
two limits that bound TFramedTransport.
Two tests. A frame delivered as 8 + 1000 + the rest now arrives whole,
where before it came back as 0 of 4000; a frame whose payload stops after
500 bytes is still reported as end of stream rather than handed over
half-read, which passes either way.
Client: cpp
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWebSocketServer::readFrame sizes its read buffer from the payload length
the frame header declares, before a single payload byte has arrived:
// size_t is smaller than a ulong on a 32-bit system
if (payloadLength > UINT32_MAX) {
failConnection(CloseCode::MessageTooBig);
return false;
}
auto length = static_cast<uint32_t>(payloadLength);
if (length > 0) {
...
readBuffer_.resetBuffer(length);
uint8_t* buffer = readBuffer_.getWritePtr(length);
read = transport_->read(buffer, length);
UINT32_MAX is the only bound, so the fourteen bytes of a header carrying a
64-bit length decide the size of the allocation. Measured on a Linux x86-64
build: a header declaring 0xFFFFFFFF takes the process from a VmPeak of
10,512 kB to 4,205,092 kB and asks the transport underneath for
4,294,967,295 bytes, with no payload sent at all. resetBuffer() constructs
a fresh TMemoryBuffer of that size, so the allocation happens whether or
not the bytes ever turn up, and the buffer is a member that lives as long
as the connection does.
The transport has a TConfiguration and consults neither of its limits here.
Hold the declared length to TConfiguration::maxFrameSize, the ceiling
TFramedTransport::readFrame already applies to its own frames, and refuse
anything above it with close code 1009 Message Too Big, which is what this
transport already sends when a frame is too large for it.
Worth a release note: maxFrameSize defaults to 16384000, so a peer sending
a single WebSocket frame larger than that is refused where it was accepted
before. The bound follows whatever an operator sets.
Six tests, the first this transport has had. They assert the largest read
the server asked of the transport underneath it rather than merely that the
read failed: a payload that never arrives ends the frame either way, so
"did it fail?" passes on the unmodified library too. Two fail before the
change, 67108864 > 1024 and 32768 > 1024, and all six pass after; the other
four are regression guards -- an ordinary frame, a frame of exactly the
maximum, two frames in a row, and a length with the high bit set -- that
pass either way.
Client: cpp
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TFramedTransport::readFrame() knows the exact size of the frame it hands to the
protocol, but the budget the protocol is checked against stays at
TConfiguration::maxMessageSize for the life of the connection.
updateKnownMessageSize() had no caller anywhere in the C++ library: the
mechanism was there and entirely unused.
That has two consequences. The one reported on this ticket: the budget is a
per-connection allowance that only consume() draws down and only readEnd() or
flush() restores, so a long run of frames read without either exhausts it and
starts refusing frames well within the limit. Sixty-four frames over a
transport with room for any single one of them, but not for their sum, ends in
"MaxMessageSize reached". And the other direction: a 68-byte frame could
declare a 64 MB field, and readStringBody() would resize the caller's string to
64 MB before the following read discovered there was nothing behind it. A
5-byte frame could likewise declare an 8-million-element list.
Three parts:
- readFrame() resets the budget and then binds it to the frame, after the
maxFrameSize check and before the buffer is allocated. The full reset first
is not optional: resetConsumedMessageSize() refuses to grow a budget, so a
frame larger than its predecessor would be rejected outright, and
updateKnownMessageSize() on its own carries the previous frame's consumption
forward. The reset belongs here rather than in readEnd(), which a oneway call
never reaches.
- TBufferBase::read() checks the budget against the bytes it can deliver rather
than the bytes asked for, for a transport that sets the new
budgetBoundToBuffer_. read() may always return less than requested, so once
the budget is one frame, an ordinary "give me up to N bytes" would otherwise
be refused instead of short-read; TransportTest does exactly that with random
chunk sizes. This does not loosen the bound the protocol is held to -- every
allocation-gating check, readStringBody() and the container element counts in
all three protocols, calls checkReadBytesAvailable() directly with the size
the wire declared and does not come through read().
- TNonblockingServer takes the frame apart itself and hands the payload to a
TMemoryBuffer, with an identity transport factory by default, so the protocol
reads straight from that buffer and none of the above reaches it. New public
TMemoryBuffer::bindMessageSizeToBuffer() binds the budget to what the buffer
holds, called at both resetBuffer sites.
THeaderTransport overrides readFrame() and so is unchanged; binding there is a
separate question, since in unframed mode it hands out four bytes at a time.
Twelve tests, written before the change. Five describe traffic that is
legitimate today and pass unmodified; three fail unmodified -- the field case
asserts on the size of the string the protocol was asked to fill, because both
outcomes raise TTransportException(END_OF_FILE) and only the allocation tells
them apart. The remaining four cover the new TMemoryBuffer entry point.
Client: cpp
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
