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