From 507e528e845034583dd21b884e6debb1ff5307e3 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Thu, 23 Jul 2026 12:38:12 -0400 Subject: [PATCH] http: reuse HTTPHeaders to parse chunked trailer Chunked transfer trailers are just headers that are included at the end of the request. We can parse and validate them with code we already use to read headers. In a future commit we will also be able to use one MAX_HEADERS_SIZE limit to cover both sections. Even though we parse and validate, we ignore these data. --- src/httpserver.cpp | 28 ++++++++++------------------ src/httpserver.h | 4 +++- src/test/httpserver_tests.cpp | 3 ++- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index dd17754bfac..dfae7d10a66 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -298,12 +298,13 @@ void HTTPHeaders::RemoveAll(std::string_view key) m_headers.erase(moved.begin(), moved.end()); } -bool HTTPHeaders::Read(util::LineReader& reader) +bool HTTPHeaders::Read(util::LineReader& reader, bool write) { // Headers https://httpwg.org/specs/rfc9110.html#rfc.section.6.3 // A sequence of Field Lines https://httpwg.org/specs/rfc9110.html#rfc.section.5.2 + size_t start{reader.Consumed()}; while (auto maybe_line = reader.ReadLine()) { - if (reader.Consumed() > MAX_HEADERS_SIZE) throw std::runtime_error("HTTP headers exceed size limit"); + if (reader.Consumed() - start > MAX_HEADERS_SIZE) throw std::runtime_error("HTTP headers exceed size limit"); const std::string_view& line = *maybe_line; @@ -336,7 +337,9 @@ bool HTTPHeaders::Read(util::LineReader& reader) // that can not be empty. if (key.empty()) throw std::runtime_error("Empty HTTP header name"); - Write(std::string(key), std::move(value)); + if (write) { + Write(std::string(key), std::move(value)); + } } return false; @@ -447,22 +450,11 @@ bool HTTPRequest::LoadBody(LineReader& reader) // Last chunk has size 0 if (*chunk_size == 0) { - // Allow (but ignore) Chunked Trailer section, by - // reading CRLF-terminated lines until we read an empty line, - // which indicates the end of this request. + // Validate Chunked Trailer section, which is used for + // additional headers sent at the end of the message. + // At this time we ignore and drop these data after validating. // See https://httpwg.org/specs/rfc9112.html#rfc.section.7.1.2 - const size_t trailer_start{reader.Consumed()}; - while (true) { - auto maybe_trailer = reader.ReadLine(); - if (reader.Consumed() - trailer_start > MAX_HEADERS_SIZE) { - throw std::runtime_error("HTTP chunked trailer exceeds size limit"); - } - if (!maybe_trailer) return false; - if (maybe_trailer->empty()) break; - } - // Complete request has been parsed, reader is now pointing - // to beginning of next request or end of the buffer. - return true; + return m_headers.Read(reader, /*write=*/false); } // We are still expecting more data for this chunk diff --git a/src/httpserver.h b/src/httpserver.h index 1ae4a5e5ee5..d92b2054442 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -104,13 +104,15 @@ public: */ void RemoveAll(std::string_view key); /** + * @param[in] reader A LineReader instance initialized with the client's receive buffer. + * @param[in] write Whether or not to write the parsed data to the object after validation. * @returns false if LineReader hits the end of the buffer before reading an * \n, meaning that we are still waiting on more data from the client. * true after reading an entire HTTP headers section, terminated * by an empty line and \n. * @throws on exceeded read limit and on bad headers syntax (e.g. no ":" in a line) */ - bool Read(util::LineReader& reader); + bool Read(util::LineReader& reader, bool write = true); std::string Stringify() const; private: diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index cd020fc60c1..c03b3e41f78 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -446,8 +446,9 @@ BOOST_AUTO_TEST_CASE(http_request_tests) BOOST_CHECK(req.LoadHeaders(reader)); BOOST_CHECK(req.LoadBody(reader)); BOOST_CHECK_EQUAL(req.m_body, R"({"method":"getblockcount"})"); - // Chunk Trailer was cleared + // Chunk Trailer was parsed, but ignored BOOST_CHECK_EQUAL(reader.Remaining(), 0); + BOOST_CHECK(!req.GetHeader("Expires").first); } { // Invalid "chunked" transfer, using roman numerals instead of hex for chunk length