From afee51b4460196bdfdbc068b7fedee71768b890b Mon Sep 17 00:00:00 2001 From: Volker Christian Date: Fri, 10 Jul 2026 12:30:16 +0200 Subject: [PATCH 1/3] Harden HTTP request framing parsing --- src/web/http/Parser.cpp | 144 ++++++++-------------- src/web/http/Parser.h | 4 + src/web/http/client/ResponseParser.cpp | 6 +- src/web/http/decoder/Chunked.cpp | 51 ++++++-- src/web/http/decoder/Fields.cpp | 3 - src/web/http/http_utils.cpp | 73 +++++++++++ src/web/http/http_utils.h | 11 ++ src/web/http/server/RequestParser.cpp | 78 +++++++++++- src/web/http/server/RequestParser.h | 1 + tests/unit/http/HttpMessageParserTest.cpp | 120 +++++++++++++++++- 10 files changed, 373 insertions(+), 118 deletions(-) diff --git a/src/web/http/Parser.cpp b/src/web/http/Parser.cpp index d2c86b6a8c..290c0d8d35 100644 --- a/src/web/http/Parser.cpp +++ b/src/web/http/Parser.cpp @@ -50,8 +50,6 @@ #include "web/http/http_utils.h" -#include -#include #include #include @@ -59,37 +57,6 @@ namespace web::http { - static bool parseContentLengthStrict(const std::string& s, std::size_t& out) { - bool success = false; - - if (!s.empty()) { - unsigned long long value = 0; - - const char* first = s.data(); - const char* last = s.data() + s.size(); - - const auto [ptr, ec] = std::from_chars(first, last, value, 10); - - if (ec == std::errc{} && ptr == last) { - out = static_cast(value); - success = true; - } - } - - return success; - } - - static bool transferEncodingHasChunked(CiStringMap& headers) { - bool hasChunked = false; - - if (headers.contains("Transfer-Encoding")) { - const std::string& encoding = headers["Transfer-Encoding"]; - hasChunked = web::http::ciContains(encoding, "chunked"); - } - - return hasChunked; - } - // HTTP/1.0 and HTTP/1.1 const std::regex Parser::httpVersionRegex("^HTTP/([1])[.]([0-1])$"); @@ -190,77 +157,57 @@ namespace web::http { return consumed; } - void Parser::analyzeHeader() { - bool success = true; - - // Determine message framing. - // RFC 9112 ยง6.3: Transfer-Encoding (chunked) overrides Content-Length. - const bool hasChunked = transferEncodingHasChunked(headers); - - if (hasChunked) { - transferEncoding = TransferEncoding::Chunked; - decoderQueue.emplace_back(new web::http::decoder::Chunked(socketContext)); + void Parser::useChunkedBodyDecoder() { + transferEncoding = TransferEncoding::Chunked; + decoderQueue.emplace_back(new web::http::decoder::Chunked(socketContext)); + configureTrailerDecoder(); + } - if (headers.contains("Trailer")) { - std::string trailers = headers["Trailer"]; + void Parser::useIdentityBodyDecoder(std::size_t length) { + contentLength = length; + transferEncoding = TransferEncoding::Identity; + decoderQueue.emplace_back(new web::http::decoder::Identity(socketContext, contentLength)); + } - while (!trailers.empty()) { - std::string trailerField; - std::tie(trailerField, trailers) = httputils::str_split(trailers, ','); - httputils::str_trimm(trailerField); - trailerFieldsExpected.insert(trailerField); - trailerField.clear(); + bool Parser::configureTrailerDecoder() { + if (headers.contains("Trailer")) { + for (std::string trailerField : httputils::splitCommaSeparatedTokens(headers["Trailer"])) { + if (!trailerField.empty()) { + trailerFieldsExpected.insert(std::move(trailerField)); } - trailerDecoder.setFieldsExpected(trailerFieldsExpected); } - } else if (headers.contains("Content-Length")) { - std::size_t length = 0; + trailerDecoder.setFieldsExpected(trailerFieldsExpected); + } + return true; + } + + bool Parser::allowsCloseDelimitedBody() const { + return true; + } - if (!parseContentLengthStrict(headers["Content-Length"], length)) { + void Parser::analyzeHeader() { + std::size_t length = 0; + switch (httputils::parseContentLength(headers, length)) { + case httputils::ContentLengthParseResult::Absent: + break; + case httputils::ContentLengthParseResult::Valid: + useIdentityBodyDecoder(length); + break; + case httputils::ContentLengthParseResult::Invalid: parseError(400, "Invalid Content-Length"); - success = false; - } else { - contentLength = length; - decoderQueue.emplace_back(new web::http::decoder::Identity(socketContext, contentLength)); - } + return; } - if (success) { - // Transfer-Encoding (other than chunked) is currently not implemented, but we keep the - // existing behavior of not altering the decoder queue here. - if (headers.contains("Transfer-Encoding")) { - const std::string& encoding = headers["Transfer-Encoding"]; - if (web::http::ciContains(encoding, "compressed")) { - // decoderQueue.emplace_back(new web::http::decoder::Compress(socketContext)); - } - if (web::http::ciContains(encoding, "deflate")) { - // decoderQueue.emplace_back(new web::http::decoder::Deflate(socketContext)); - } - if (web::http::ciContains(encoding, "gzip")) { - // decoderQueue.emplace_back(new web::http::decoder::GZip(socketContext)); - } - } - - if (decoderQueue.empty()) { - decoderQueue.emplace_back(new web::http::decoder::HTTP10Response(socketContext)); + if (headers.contains("Transfer-Encoding") && httputils::headerHasToken(headers["Transfer-Encoding"], "chunked")) { + for (const ContentDecoder* contentDecoder : decoderQueue) { + delete contentDecoder; } + decoderQueue.clear(); + useChunkedBodyDecoder(); + } - if (headers.contains("Content-Encoding")) { - const std::string& encoding = headers["Content-Encoding"]; - - if (web::http::ciContains(encoding, "compressed")) { - // decoderQueue.emplace_back(new web::http::decoder::Compress(socketContext)); - } - if (web::http::ciContains(encoding, "deflate")) { - // decoderQueue.emplace_back(new web::http::decoder::Deflate(socketContext)); - } - if (web::http::ciContains(encoding, "gzip")) { - // decoderQueue.emplace_back(new web::http::decoder::GZip(socketContext)); - } - if (web::http::ciContains(encoding, "br")) { - // decoderQueue.emplace_back(new web::http::decoder::Br(socketContext)); - } - } + if (decoderQueue.empty() && allowsCloseDelimitedBody()) { + decoderQueue.emplace_back(new web::http::decoder::HTTP10Response(socketContext)); } } @@ -275,7 +222,7 @@ namespace web::http { std::vector chunk = contentDecoder->getContent(); content.insert(content.end(), chunk.begin(), chunk.end()); - if (transferEncoding == TransferEncoding::Chunked && headers.contains("Trailer")) { + if (transferEncoding == TransferEncoding::Chunked) { parserState = Parser::ParserState::TRAILER; } else { parsingFinished(); @@ -294,7 +241,12 @@ namespace web::http { parseError(trailerDecoder.getErrorCode(), trailerDecoder.getErrorReason()); } else if (trailerDecoder.isComplete()) { web::http::CiStringMap&& trailer = trailerDecoder.getHeader(); - headers.insert(trailer.begin(), trailer.end()); + for (const auto& [field, value] : trailer) { + if (!web::http::ciEquals(field, "Content-Length") && !web::http::ciEquals(field, "Transfer-Encoding") && + !web::http::ciEquals(field, "Host") && !web::http::ciEquals(field, "Connection")) { + headers.insert({field, value}); + } + } parsingFinished(); } diff --git a/src/web/http/Parser.h b/src/web/http/Parser.h index 1695352e75..0fcaaa7299 100644 --- a/src/web/http/Parser.h +++ b/src/web/http/Parser.h @@ -109,6 +109,10 @@ namespace web::http { protected: virtual void analyzeHeader(); + virtual bool allowsCloseDelimitedBody() const; + void useChunkedBodyDecoder(); + void useIdentityBodyDecoder(std::size_t length); + bool configureTrailerDecoder(); private: virtual void parseError(int code, const std::string& reason) = 0; diff --git a/src/web/http/client/ResponseParser.cpp b/src/web/http/client/ResponseParser.cpp index 2982615e61..c4e2d19c74 100644 --- a/src/web/http/client/ResponseParser.cpp +++ b/src/web/http/client/ResponseParser.cpp @@ -113,10 +113,10 @@ namespace web::http::client { if (headers.contains("Connection")) { const std::string& connection = headers["Connection"]; - if (web::http::ciContains(connection, "keep-alive")) { - response.connectionState = ConnectionState::Keep; - } else if (web::http::ciContains(connection, "close")) { + if (httputils::headerHasToken(connection, "close")) { response.connectionState = ConnectionState::Close; + } else if (httputils::headerHasToken(connection, "keep-alive")) { + response.connectionState = ConnectionState::Keep; } } if (headers.contains("Set-Cookie")) { diff --git a/src/web/http/decoder/Chunked.cpp b/src/web/http/decoder/Chunked.cpp index a3ece47a07..e5c3cd845d 100644 --- a/src/web/http/decoder/Chunked.cpp +++ b/src/web/http/decoder/Chunked.cpp @@ -42,7 +42,12 @@ #include "web/http/decoder/Chunked.h" #include "core/socket/stream/SocketContext.h" +#include "web/http/http_utils.h" +#include +#include +#include +#include #include #ifndef DOXYGEN_SHOULD_SKIP_THIS @@ -83,7 +88,7 @@ namespace web::http::decoder { } state = (completed || error) ? -1 : state; - } while (ret > 0 && !completed); + } while (ret > 0 && !completed && !error); break; } @@ -149,14 +154,44 @@ namespace web::http::decoder { CR = false; LF = false; - try { - chunkLenTotal = std::stoul(chunkLenTotalS, &pos, 16); - chunk.resize(chunkLenTotal); + { + std::string chunkSizeToken = chunkLenTotalS; + std::tie(chunkSizeToken, std::ignore) = httputils::str_split(chunkSizeToken, ';'); + httputils::str_trimm(chunkSizeToken); + if (chunkSizeToken.empty() || !std::all_of(chunkSizeToken.begin(), chunkSizeToken.end(), [](unsigned char c) { + return std::isxdigit(c) != 0; + }) || + chunkSizeToken.size() > maxChunkLenTotalS) { + error = true; + state = -1; + break; + } - state = 1; - } catch (std::invalid_argument&) { - error = true; - break; + try { + const unsigned long long parsedChunkLen = std::stoull(chunkSizeToken, &pos, 16); + if (pos != chunkSizeToken.size() || parsedChunkLen > std::numeric_limits::max()) { + error = true; + state = -1; + break; + } + chunkLenTotal = static_cast(parsedChunkLen); + chunk.resize(chunkLenTotal); + + if (chunkLenTotal == 0) { + completed = true; + state = -1; + break; + } + state = 1; + } catch (std::invalid_argument&) { + error = true; + state = -1; + break; + } catch (std::out_of_range&) { + error = true; + state = -1; + break; + } } [[fallthrough]]; diff --git a/src/web/http/decoder/Fields.cpp b/src/web/http/decoder/Fields.cpp index ac6a267f24..cf22112185 100644 --- a/src/web/http/decoder/Fields.cpp +++ b/src/web/http/decoder/Fields.cpp @@ -137,9 +137,6 @@ namespace web::http::decoder { } else if ((std::isblank(headerFieldName.back()) != 0) || (std::isblank(headerFieldName.front()) != 0)) { errorCode = 400; errorReason = "White space before or after field"; - } else if (value.empty()) { - errorCode = 400; - errorReason = "Value of field \"" + headerFieldName + "\" empty"; } else { if (fieldsExpected.empty() || fieldsExpected.contains(headerFieldName)) { httputils::str_trimm(value); diff --git a/src/web/http/http_utils.cpp b/src/web/http/http_utils.cpp index b50b7e8222..77ac2503ec 100644 --- a/src/web/http/http_utils.cpp +++ b/src/web/http/http_utils.cpp @@ -51,9 +51,12 @@ #include #include #include +#include #include #include +#include #include +#include #endif /* DOXYGEN_SHOULD_SKIP_THIS */ @@ -109,6 +112,76 @@ namespace httputils { return text; } + + std::vector splitCommaSeparatedTokens(const std::string& value) { + std::vector tokens; + std::string remaining = value; + + do { + std::string token; + std::tie(token, remaining) = str_split(remaining, ','); + str_trimm(token); + tokens.emplace_back(std::move(token)); + } while (!remaining.empty()); + + return tokens; + } + + bool tokenEquals(const std::string& value, const std::string& token) { + if (value.size() != token.size()) { + return false; + } + + return std::equal(value.begin(), value.end(), token.begin(), [](unsigned char a, unsigned char b) { + return std::tolower(a) == std::tolower(b); + }); + } + + bool headerHasToken(const std::string& value, const std::string& token) { + const std::vector tokens = splitCommaSeparatedTokens(value); + + return std::any_of(tokens.begin(), tokens.end(), [&token](const std::string& headerToken) { + return tokenEquals(headerToken, token); + }); + } + + ContentLengthParseResult parseContentLength(const web::http::CiStringMap& headers, std::size_t& out) { + if (!headers.contains("Content-Length")) { + return ContentLengthParseResult::Absent; + } + + bool haveValue = false; + std::size_t expected = 0; + + for (std::string value : splitCommaSeparatedTokens(headers.at("Content-Length"))) { + if (value.empty()) { + return ContentLengthParseResult::Invalid; + } + + unsigned long long parsed = 0; + const char* first = value.data(); + const char* last = value.data() + value.size(); + const auto [ptr, ec] = std::from_chars(first, last, parsed, 10); + if (ec != std::errc{} || ptr != last || static_cast(static_cast(parsed)) != parsed) { + return ContentLengthParseResult::Invalid; + } + + const std::size_t length = static_cast(parsed); + if (haveValue && length != expected) { + return ContentLengthParseResult::Invalid; + } + expected = length; + haveValue = true; + } + + if (!haveValue) { + return ContentLengthParseResult::Invalid; + } + + out = expected; + return ContentLengthParseResult::Valid; + } + std::pair str_split(const std::string& base, char c_middle) { std::pair split; diff --git a/src/web/http/http_utils.h b/src/web/http/http_utils.h index 25e5d2a8a9..90cf4ded79 100644 --- a/src/web/http/http_utils.h +++ b/src/web/http/http_utils.h @@ -50,6 +50,7 @@ namespace web::http { } // namespace web::http #include +#include #include #include #include @@ -68,6 +69,16 @@ namespace httputils { std::string& str_trimm(std::string& text); + std::vector splitCommaSeparatedTokens(const std::string& value); + + bool tokenEquals(const std::string& value, const std::string& token); + + bool headerHasToken(const std::string& value, const std::string& token); + + enum class ContentLengthParseResult { Absent, Valid, Invalid }; + + ContentLengthParseResult parseContentLength(const web::http::CiStringMap& headers, std::size_t& out); + std::pair str_split(const std::string& base, char c_middle); std::pair str_split_last(const std::string& base, char c_middle); diff --git a/src/web/http/server/RequestParser.cpp b/src/web/http/server/RequestParser.cpp index 00da11c46d..d8363f2b32 100644 --- a/src/web/http/server/RequestParser.cpp +++ b/src/web/http/server/RequestParser.cpp @@ -119,8 +119,78 @@ namespace web::http::server { } } + bool RequestParser::allowsCloseDelimitedBody() const { + return false; + } + void RequestParser::analyzeHeader() { - Parser::analyzeHeader(); + if (httpMinor == 1) { + if (!headers.contains("Host")) { + parseError(400, "Missing Host"); + return; + } + std::string host = headers["Host"]; + httputils::str_trimm(host); + if (host.empty() || host.find(',') != std::string::npos) { + parseError(400, "Invalid Host"); + return; + } + } + + if (headers.contains("Transfer-Encoding")) { + if (headers.contains("Content-Length")) { + parseError(400, "Transfer-Encoding with Content-Length"); + return; + } + + const std::vector codings = httputils::splitCommaSeparatedTokens(headers["Transfer-Encoding"]); + if (codings.empty()) { + parseError(400, "Invalid Transfer-Encoding"); + return; + } + + std::size_t chunkedCount = 0; + for (std::size_t i = 0; i < codings.size(); ++i) { + std::string coding = codings[i]; + std::tie(coding, std::ignore) = httputils::str_split(coding, ';'); + httputils::str_trimm(coding); + + if (coding.empty()) { + parseError(400, "Invalid Transfer-Encoding"); + return; + } + + if (httputils::tokenEquals(coding, "chunked")) { + ++chunkedCount; + if (i + 1 != codings.size()) { + parseError(400, "Chunked Transfer-Encoding not final"); + return; + } + } else { + parseError(501, "Unsupported Transfer-Encoding"); + return; + } + } + + if (chunkedCount != 1 || codings.size() != 1) { + parseError(400, "Invalid Transfer-Encoding"); + return; + } + + useChunkedBodyDecoder(); + } else { + std::size_t length = 0; + switch (httputils::parseContentLength(headers, length)) { + case httputils::ContentLengthParseResult::Absent: + break; + case httputils::ContentLengthParseResult::Valid: + useIdentityBodyDecoder(length); + break; + case httputils::ContentLengthParseResult::Invalid: + parseError(400, "Invalid Content-Length"); + return; + } + } if (parserState == Parser::ParserState::ERROR) { return; @@ -128,10 +198,10 @@ namespace web::http::server { if (headers.contains("Connection")) { const std::string& connection = headers["Connection"]; - if (web::http::ciContains(connection, "keep-alive")) { - request.connectionState = ConnectionState::Keep; - } else if (web::http::ciContains(connection, "close")) { + if (httputils::headerHasToken(connection, "close")) { request.connectionState = ConnectionState::Close; + } else if (httputils::headerHasToken(connection, "keep-alive")) { + request.connectionState = ConnectionState::Keep; } } diff --git a/src/web/http/server/RequestParser.h b/src/web/http/server/RequestParser.h index 9f22ee41b9..3491b40316 100644 --- a/src/web/http/server/RequestParser.h +++ b/src/web/http/server/RequestParser.h @@ -82,6 +82,7 @@ namespace web::http::server { // Parsers and Validators void parseStartLine(const std::string& line) override; void analyzeHeader() override; + bool allowsCloseDelimitedBody() const override; // Exits void parsingFinished() override; diff --git a/tests/unit/http/HttpMessageParserTest.cpp b/tests/unit/http/HttpMessageParserTest.cpp index c1d146fe3e..33a5845c90 100644 --- a/tests/unit/http/HttpMessageParserTest.cpp +++ b/tests/unit/http/HttpMessageParserTest.cpp @@ -185,7 +185,11 @@ namespace { result.errorReason = reason; }); - result.consumed = parser.parse(); + std::size_t consumed = 0; + do { + consumed = parser.parse(); + result.consumed += consumed; + } while (consumed > 0 && !result.parsed && result.errorCode == 0); return result; } @@ -209,7 +213,11 @@ namespace { result.errorReason = reason; }); - result.consumed = parser.parse(); + std::size_t consumed = 0; + do { + consumed = parser.parse(); + result.consumed += consumed; + } while (consumed > 0 && !result.parsed && result.errorCode == 0); return result; } @@ -219,6 +227,109 @@ namespace { int main() { tests::support::TestResult testResult; + + { + const RequestParseResult normalGet = parseRequestMessage("GET / HTTP/1.1\r\nHost: example.test\r\n\r\n"); + testResult.expectTrue(normalGet.parsed, "HTTP/1.1 GET with Host still parses"); + + const RequestParseResult http10Get = parseRequestMessage("GET / HTTP/1.0\r\n\r\n"); + testResult.expectTrue(http10Get.parsed, "HTTP/1.0 request without Host still parses"); + + const RequestParseResult missingHost = parseRequestMessage("GET / HTTP/1.1\r\n\r\n"); + testResult.expectTrue(!missingHost.parsed, "HTTP/1.1 request without Host is rejected"); + testResult.expectEqual(400, missingHost.errorCode, "missing Host is bad request"); + + const RequestParseResult emptyHost = parseRequestMessage("GET / HTTP/1.1\r\nHost:\r\n\r\n"); + testResult.expectTrue(!emptyHost.parsed, "HTTP/1.1 request with empty Host is rejected"); + testResult.expectEqual(400, emptyHost.errorCode, "empty Host is bad request"); + + const RequestParseResult duplicateHost = parseRequestMessage("GET / HTTP/1.1\r\nHost: example.com\r\nHost: other.example\r\n\r\n"); + testResult.expectTrue(!duplicateHost.parsed, "HTTP/1.1 request with duplicate Host is rejected"); + testResult.expectEqual(400, duplicateHost.errorCode, "duplicate Host is bad request"); + } + + { + const RequestParseResult chunked = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n"); + testResult.expectTrue(chunked.parsed, "request parser accepts Transfer-Encoding chunked"); + testResult.expectTrue(chunked.request && bodyToString(chunked.request->body) == "hello", "request parser decodes chunked body"); + + const RequestParseResult chunkExt = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\n\r\n5;foo=bar\r\nhello\r\n0\r\n\r\n"); + testResult.expectTrue(chunkExt.parsed, "chunked decoder accepts and ignores chunk extensions"); + testResult.expectTrue(chunkExt.request && bodyToString(chunkExt.request->body) == "hello", "chunk extension request body is decoded"); + + const RequestParseResult trailer = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\n\r\n0\r\nX-Test: yes\r\n\r\n"); + testResult.expectTrue(trailer.parsed, "chunked decoder consumes trailer section after zero chunk"); + + const RequestParseResult emptyChunked = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\n\r\n0\r\n\r\n"); + testResult.expectTrue(emptyChunked.parsed, "chunked decoder accepts empty chunked body"); + + const RequestParseResult badChunkSize = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\n\r\n5xyz\r\nhello\r\n0\r\n\r\n"); + testResult.expectTrue(!badChunkSize.parsed, "chunked decoder rejects chunk-size trailing garbage"); + testResult.expectEqual(501, badChunkSize.errorCode, "invalid chunk syntax is reported as content decoding error"); + } + + { + const std::vector invalidTransferEncodings = { + "xchunked", "chunkedx", "gzip", "chunked, gzip", "gzip, chunked", "chunked, chunked"}; + for (const std::string& transferEncoding : invalidTransferEncodings) { + const RequestParseResult result = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: " + transferEncoding + "\r\n\r\n"); + testResult.expectTrue(!result.parsed, "request parser rejects unsupported or malformed Transfer-Encoding: " + transferEncoding); + testResult.expectTrue(result.errorCode == 400 || result.errorCode == 501, "Transfer-Encoding rejection is deterministic"); + } + + const RequestParseResult teCl = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunked\r\nContent-Length: 5\r\n\r\n0\r\n\r\n"); + testResult.expectTrue(!teCl.parsed, "request parser rejects Transfer-Encoding with Content-Length"); + testResult.expectEqual(400, teCl.errorCode, "TE plus CL is bad request"); + } + + { + const RequestParseResult cl = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\n\r\nhello"); + testResult.expectTrue(cl.parsed && cl.request && bodyToString(cl.request->body) == "hello", "Content-Length body parses"); + + const RequestParseResult duplicateCl = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\nContent-Length: 5\r\n\r\nhello"); + testResult.expectTrue(duplicateCl.parsed && duplicateCl.request && bodyToString(duplicateCl.request->body) == "hello", "identical duplicate Content-Length parses"); + + const RequestParseResult listCl = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 5, 5\r\n\r\nhello"); + testResult.expectTrue(listCl.parsed && listCl.request && bodyToString(listCl.request->body) == "hello", "identical listed Content-Length parses"); + + const std::vector invalidContentLengths = {"5,6", "abc", "", "5x", "-1", "1 2", "184467440737095516160000"}; + for (const std::string& contentLength : invalidContentLengths) { + const RequestParseResult result = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: " + contentLength + "\r\n\r\n"); + testResult.expectTrue(!result.parsed, "request parser rejects invalid Content-Length: " + contentLength); + testResult.expectEqual(400, result.errorCode, "invalid Content-Length is bad request"); + } + + const RequestParseResult conflictCl = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\nContent-Length: 6\r\n\r\nhello!"); + testResult.expectTrue(!conflictCl.parsed, "request parser rejects conflicting duplicate Content-Length"); + testResult.expectEqual(400, conflictCl.errorCode, "conflicting Content-Length is bad request"); + } + + { + BufferSocketConnection connection("POST / HTTP/1.1\r\nHost: x\r\n\r\nGET /next HTTP/1.1\r\nHost: x\r\n\r\n"); + BufferSocketContext context(&connection); + int parsedCount = 0; + std::vector urls; + web::http::server::RequestParser parser( + &context, + []() {}, + [&parsedCount, &urls](web::http::server::Request&& request) { + ++parsedCount; + urls.emplace_back(request.url); + }, + [](int, const std::string&) {}); + parser.parse(); + parser.parse(); + testResult.expectEqual(2, parsedCount, "request without length does not consume pipelined request as body"); + testResult.expectTrue(urls.size() == 2 && urls[0] == "/" && urls[1] == "/next", "pipelined request after body-less request remains parseable"); + } + + { + testResult.expectTrue(httputils::headerHasToken("close", "close"), "Connection close token is recognized"); + testResult.expectTrue(!httputils::headerHasToken("xclose", "close"), "Connection xclose does not match close"); + testResult.expectTrue(!httputils::headerHasToken("keep-alivex", "keep-alive"), "Connection keep-alivex does not match keep-alive"); + testResult.expectTrue(httputils::headerHasToken("keep-alive, close", "close") && httputils::headerHasToken("keep-alive, close", "keep-alive"), "Connection comma tokens parse exactly when close and keep-alive are present"); + } + { web::http::CiStringMap headers; headers.emplace("Content-Type", "text/plain"); @@ -330,14 +441,14 @@ int main() { testResult.expectTrue(!malformedHeader.errorReason.empty(), "request parser supplies an error reason for malformed header whitespace"); - const RequestParseResult invalidContentLength = parseRequestMessage("POST / HTTP/1.1\r\nContent-Length: nope\r\n\r\n"); + const RequestParseResult invalidContentLength = parseRequestMessage("POST / HTTP/1.1\r\nHost: example.test\r\nContent-Length: nope\r\n\r\n"); testResult.expectTrue(invalidContentLength.started, "request parser starts before rejecting invalid Content-Length"); testResult.expectTrue(!invalidContentLength.parsed, "request parser does not parse invalid Content-Length"); testResult.expectEqual(400, invalidContentLength.errorCode, "request parser reports invalid Content-Length as bad request"); testResult.expectTrue(!invalidContentLength.errorReason.empty(), "request parser supplies an error reason for invalid Content-Length"); - const RequestParseResult incompleteBody = parseRequestMessage("POST / HTTP/1.1\r\nContent-Length: 7\r\n\r\nabc"); + const RequestParseResult incompleteBody = parseRequestMessage("POST / HTTP/1.1\r\nHost: example.test\r\nContent-Length: 7\r\n\r\nabc"); testResult.expectTrue(incompleteBody.started, "request parser starts incomplete body message"); testResult.expectTrue(!incompleteBody.parsed, "request parser leaves incomplete body unparsed without EOF error semantics"); testResult.expectEqual( @@ -399,6 +510,7 @@ int main() { testResult.expectTrue(emptyInput.errorReason.empty(), "response parser leaves error reason empty for empty input boundary"); } + { web::http::CiStringMap headers; headers.emplace("Content-Length", "5"); From 0a27aa43fa94efb48bca12100db2c6a89561f949 Mon Sep 17 00:00:00 2001 From: Volker Christian Date: Fri, 10 Jul 2026 12:48:28 +0200 Subject: [PATCH 2/3] Preserve empty HTTP comma list tokens --- src/web/http/http_utils.cpp | 16 +++++++---- tests/unit/http/HttpMessageParserTest.cpp | 34 +++++++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/web/http/http_utils.cpp b/src/web/http/http_utils.cpp index 77ac2503ec..8dec22c974 100644 --- a/src/web/http/http_utils.cpp +++ b/src/web/http/http_utils.cpp @@ -115,14 +115,20 @@ namespace httputils { std::vector splitCommaSeparatedTokens(const std::string& value) { std::vector tokens; - std::string remaining = value; + std::size_t start = 0; - do { - std::string token; - std::tie(token, remaining) = str_split(remaining, ','); + while (true) { + const std::size_t comma = value.find(',', start); + std::string token = value.substr(start, comma == std::string::npos ? std::string::npos : comma - start); str_trimm(token); tokens.emplace_back(std::move(token)); - } while (!remaining.empty()); + + if (comma == std::string::npos) { + break; + } + + start = comma + 1; + } return tokens; } diff --git a/tests/unit/http/HttpMessageParserTest.cpp b/tests/unit/http/HttpMessageParserTest.cpp index 33a5845c90..bb00db8fff 100644 --- a/tests/unit/http/HttpMessageParserTest.cpp +++ b/tests/unit/http/HttpMessageParserTest.cpp @@ -227,6 +227,16 @@ namespace { int main() { tests::support::TestResult testResult; + { + testResult.expectTrue(httputils::splitCommaSeparatedTokens("chunked,") == std::vector{"chunked", ""}, + "comma splitter preserves trailing empty token"); + testResult.expectTrue(httputils::splitCommaSeparatedTokens(",chunked") == std::vector{"", "chunked"}, + "comma splitter preserves leading empty token"); + testResult.expectTrue(httputils::splitCommaSeparatedTokens(",") == std::vector{"", ""}, + "comma splitter preserves all-empty comma list"); + testResult.expectTrue(httputils::splitCommaSeparatedTokens("") == std::vector{""}, + "comma splitter returns one empty token for empty input"); + } { const RequestParseResult normalGet = parseRequestMessage("GET / HTTP/1.1\r\nHost: example.test\r\n\r\n"); @@ -269,8 +279,17 @@ int main() { } { - const std::vector invalidTransferEncodings = { - "xchunked", "chunkedx", "gzip", "chunked, gzip", "gzip, chunked", "chunked, chunked"}; + const std::vector invalidTransferEncodings = {"xchunked", + "chunkedx", + "gzip", + "chunked, gzip", + "gzip, chunked", + "chunked, chunked", + "chunked,", + ",chunked", + "gzip,", + ",", + ""}; for (const std::string& transferEncoding : invalidTransferEncodings) { const RequestParseResult result = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: " + transferEncoding + "\r\n\r\n"); testResult.expectTrue(!result.parsed, "request parser rejects unsupported or malformed Transfer-Encoding: " + transferEncoding); @@ -292,7 +311,16 @@ int main() { const RequestParseResult listCl = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 5, 5\r\n\r\nhello"); testResult.expectTrue(listCl.parsed && listCl.request && bodyToString(listCl.request->body) == "hello", "identical listed Content-Length parses"); - const std::vector invalidContentLengths = {"5,6", "abc", "", "5x", "-1", "1 2", "184467440737095516160000"}; + const std::vector invalidContentLengths = {"5,6", + "abc", + "", + "5x", + "-1", + "1 2", + "184467440737095516160000", + "5,", + ",5", + ","}; for (const std::string& contentLength : invalidContentLengths) { const RequestParseResult result = parseRequestMessage("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: " + contentLength + "\r\n\r\n"); testResult.expectTrue(!result.parsed, "request parser rejects invalid Content-Length: " + contentLength); From 60e09b9c21836e0ae67ea21a96cd1d9643c70ff3 Mon Sep 17 00:00:00 2001 From: Volker Christian Date: Sat, 11 Jul 2026 09:40:57 +0200 Subject: [PATCH 3/3] Reject header fields missing colon --- src/web/http/decoder/Fields.cpp | 14 ++++++++++++-- tests/unit/http/HttpMessageParserTest.cpp | 11 +++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/web/http/decoder/Fields.cpp b/src/web/http/decoder/Fields.cpp index cf22112185..8ccac2ff9e 100644 --- a/src/web/http/decoder/Fields.cpp +++ b/src/web/http/decoder/Fields.cpp @@ -129,12 +129,22 @@ namespace web::http::decoder { } void Fields::splitLine(const std::string& line) { - auto [headerFieldName, value] = httputils::str_split(line, ':'); + const std::size_t colon = line.find(':'); + + if (colon == std::string::npos) { + errorCode = 400; + errorReason = "Header field missing colon"; + return; + } + + std::string headerFieldName = line.substr(0, colon); + std::string value = line.substr(colon + 1); if (headerFieldName.empty()) { errorCode = 400; errorReason = "Header field empty"; - } else if ((std::isblank(headerFieldName.back()) != 0) || (std::isblank(headerFieldName.front()) != 0)) { + } else if ((std::isblank(static_cast(headerFieldName.back())) != 0) || + (std::isblank(static_cast(headerFieldName.front())) != 0)) { errorCode = 400; errorReason = "White space before or after field"; } else { diff --git a/tests/unit/http/HttpMessageParserTest.cpp b/tests/unit/http/HttpMessageParserTest.cpp index bb00db8fff..aa572273c5 100644 --- a/tests/unit/http/HttpMessageParserTest.cpp +++ b/tests/unit/http/HttpMessageParserTest.cpp @@ -242,6 +242,9 @@ int main() { const RequestParseResult normalGet = parseRequestMessage("GET / HTTP/1.1\r\nHost: example.test\r\n\r\n"); testResult.expectTrue(normalGet.parsed, "HTTP/1.1 GET with Host still parses"); + const RequestParseResult emptyGenericHeader = parseRequestMessage("GET / HTTP/1.1\r\nHost: x\r\nX-Empty:\r\n\r\n"); + testResult.expectTrue(emptyGenericHeader.parsed, "request parser allows empty generic header values"); + const RequestParseResult http10Get = parseRequestMessage("GET / HTTP/1.0\r\n\r\n"); testResult.expectTrue(http10Get.parsed, "HTTP/1.0 request without Host still parses"); @@ -469,6 +472,10 @@ int main() { testResult.expectTrue(!malformedHeader.errorReason.empty(), "request parser supplies an error reason for malformed header whitespace"); + const RequestParseResult missingColon = parseRequestMessage("GET / HTTP/1.1\r\nHost: x\r\nBadHeaderWithoutColon\r\n\r\n"); + testResult.expectTrue(!missingColon.parsed, "request header line without colon is rejected"); + testResult.expectEqual(400, missingColon.errorCode, "missing request header colon is bad request"); + const RequestParseResult invalidContentLength = parseRequestMessage("POST / HTTP/1.1\r\nHost: example.test\r\nContent-Length: nope\r\n\r\n"); testResult.expectTrue(invalidContentLength.started, "request parser starts before rejecting invalid Content-Length"); testResult.expectTrue(!invalidContentLength.parsed, "request parser does not parse invalid Content-Length"); @@ -517,6 +524,10 @@ int main() { testResult.expectTrue(!malformedHeader.errorReason.empty(), "response parser supplies an error reason for malformed header whitespace"); + const ResponseParseResult missingColon = parseResponseMessage("HTTP/1.1 200 OK\r\nBadHeaderWithoutColon\r\n\r\n"); + testResult.expectTrue(!missingColon.parsed, "response header line without colon is rejected"); + testResult.expectEqual(400, missingColon.errorCode, "missing response header colon is bad response"); + const ResponseParseResult invalidContentLength = parseResponseMessage("HTTP/1.1 200 OK\r\nContent-Length: nope\r\n\r\n"); testResult.expectTrue(invalidContentLength.started, "response parser starts before rejecting invalid Content-Length"); testResult.expectTrue(!invalidContentLength.parsed, "response parser does not parse invalid Content-Length");