Jens Geyer created THRIFT-6192:
----------------------------------
Summary: THttpServer matches a header name by prefix, and
Content-Length goes through atoi
Key: THRIFT-6192
URL: https://issues.apache.org/jira/browse/THRIFT-6192
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
h2. Header names
{{THttpServer::parseHeader}} compares a header name for as many characters as
the peer sent
before the colon:
{code:cpp}
size_t sz = colon - header;
if (THRIFT_strncasecmp(header, "Transfer-Encoding", sz) == 0) { ... }
else if (THRIFT_strncasecmp(header, "Content-length", sz) == 0) { ... }
else if (strncmp(header, "X-Forwarded-For", sz) == 0) { ... }
{code}
Every name that is a _prefix_ of one the transport knows is therefore accepted
as that name.
Measured against the unmodified library:
||Header sent||{{contentLength_}}||{{chunked_}}||
|{{Content-length: 5}}|5|0|
|{{C: 5}}|5|0|
|{{co: 7}}|7|0|
|{{Content-len: 9}}|9|0|
|{{Transfer-Encoding: chunked}}|0|1|
|{{T: chunked}}|0|1|
|{{Cookie: 5}}|0|0|
{{Cookie}} is the control: it is not a prefix of either name and is correctly
ignored.
A header name is the whole token before the colon (RFC 9110 5.1). Reading it
otherwise means
this transport and every other party on the connection disagree about where a
message ends --
{{C: 5}} gives Thrift a five byte body where a parser following the grammar
sees none.
{{lib/cpp/src/thrift/transport/TWebSocketServer.h}} overrides {{parseHeader}}
with the same
idiom, so {{U: websocket}}, {{C: Upgrade}} and {{S: <key>}} satisfy the
handshake. Note that
{{S:}} reaches the {{Sec-WebSocket-Key}} arm, which is tested before the
version one. The same
function has {{std::string toHash = value + 1;}}, which steps over the space
after the colon by
assuming one is there, and reads one past the terminator for a header with an
empty value.
{{THttpClient::parseHeader}} is not affected -- it builds {{const string
name(header, colon)}}
and uses {{boost::iequals}}, a full equality.
h2. Content-Length
{{contentLength_ = atoi(value)}}, in both the server and the client. {{atoi}}
cannot report a
negative number, a value too large for the {{uint32_t}} member, or text that is
not a number:
||Header sent||{{contentLength_}}||
|{{Content-length: -1}}|4294967295|
|{{Content-length: 99999999999}}|1215752191|
|{{Content-length: abc}}|0|
|{{Content-length: 5x}}|5|
RFC 9110 8.6 has {{Content-Length}} as {{1\*DIGIT}}, which leaves room for none
of these.
h2. Behaviour change
Refusing an abbreviated header name, and refusing a malformed
{{Content-Length}}, can turn away
traffic that works today. Both belong in the release notes, and a
{{lib/cpp/README.md}} entry
comes with the patch.
_Filed with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
