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