Jens-G opened a new pull request, #3802: URL: https://github.com/apache/thrift/pull/3802
[THRIFT-6192](https://issues.apache.org/jira/browse/THRIFT-6192) `THttpServer::parseHeader` compares a header name for as many characters as the peer sent before the colon: ```cpp 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 is 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 — not a prefix of either name, 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. `TWebSocketServer::parseHeader` is the same idiom on the same base class, so `U: websocket`, `C: Upgrade` and `S: <key>` satisfy the handshake — and `S:` reaches the `Sec-WebSocket-Key` arm, which is tested before the version one. Fixed here too, along with the `std::string toHash = value + 1;` beside it, 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. `Content-Length` went through `atoi()`, which 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 | It is now parsed with `strtoll` against RFC 9110 8.6's `1*DIGIT`. `THttpClient` shares the new helper; its name comparison was already a full `boost::iequals` equality and needed nothing. ### Tests Nine cases in a new `lib/cpp/test/THttpHeaderParseTest.cpp`, written before the change, wired into both `CMakeLists.txt` and `Makefile.am`. **Six fail against the unmodified library.** The two covering a bad number assert the length that was *parsed* rather than that the read threw — an over-declared length outruns a five-byte body and throws on the unmodified library too, so `BOOST_CHECK_THROW` alone would have passed either way. `UnitTests` is otherwise unchanged: `TWebSocketServerTest` 14/14, `THttpBufferBoundTest` 4/4. ### Behaviour change Refusing an abbreviated header name, and refusing a malformed `Content-Length`, can turn away traffic that works today. `lib/cpp/README.md` gains a Breaking Changes entry under 0.25.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
