From b1f8e986bfe896a68bc3db464f399f3bd533430e Mon Sep 17 00:00:00 2001 From: yhirose Date: Tue, 3 Sep 2024 00:47:39 -0400 Subject: [PATCH] Fix #1908 (#1910) * Fix #1908 * Code format --- httplib.h | 29 +++++++++++++++++++++++++++-- test/test.cc | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/httplib.h b/httplib.h index 542f62c..b7be298 100644 --- a/httplib.h +++ b/httplib.h @@ -2790,6 +2790,10 @@ inline bool stream_line_reader::getline() { fixed_buffer_used_size_ = 0; glowable_buffer_.clear(); +#ifndef CPPHTTPLIB_ALLOW_LF_AS_LINE_TERMINATOR + char prev_byte = 0; +#endif + for (size_t i = 0;; i++) { char byte; auto n = strm_.read(&byte, 1); @@ -2806,7 +2810,12 @@ inline bool stream_line_reader::getline() { append(byte); +#ifdef CPPHTTPLIB_ALLOW_LF_AS_LINE_TERMINATOR if (byte == '\n') { break; } +#else + if (prev_byte == '\r' && byte == '\n') { break; } + prev_byte = byte; +#endif } return true; @@ -2862,7 +2871,8 @@ inline bool mmap::open(const char *path) { // If the following line doesn't compile due to QuadPart, update Windows SDK. // See: // https://github.com/yhirose/cpp-httplib/issues/1903#issuecomment-2316520721 - if (static_cast(size.QuadPart) > std::numeric_limits::max()) { + if (static_cast(size.QuadPart) > + std::numeric_limits::max()) { // `size_t` might be 32-bits, on 32-bits Windows. return false; } @@ -4049,7 +4059,22 @@ inline bool read_headers(Stream &strm, Headers &headers) { auto end = line_reader.ptr() + line_reader.size() - line_terminator_len; parse_header(line_reader.ptr(), end, - [&](const std::string &key, const std::string &val) { + [&](const std::string &key, std::string &val) { + // NOTE: From RFC 9110: + // Field values containing CR, LF, or NUL characters are + // invalid and dangerous, due to the varying ways that + // implementations might parse and interpret those + // characters; a recipient of CR, LF, or NUL within a field + // value MUST either reject the message or replace each of + // those characters with SP before further processing or + // forwarding of that message. + for (auto &c : val) { + switch (c) { + case '\0': + case '\n': + case '\r': c = ' '; break; + } + } headers.emplace(key, val); }); } diff --git a/test/test.cc b/test/test.cc index d960ab5..09a2eba 100644 --- a/test/test.cc +++ b/test/test.cc @@ -4718,6 +4718,11 @@ static void test_raw_request(const std::string &req, svr.Put("/put_hi", [&](const Request & /*req*/, Response &res) { res.set_content("ok", "text/plain"); }); + svr.Get("/header_field_value_check", [&](const Request &req, Response &res) { + auto val = req.get_header_value("Test"); + EXPECT_EQ("[ ]", val); + res.set_content("ok", "text/plain"); + }); // Server read timeout must be longer than the client read timeout for the // bug to reproduce, probably to force the server to process a request @@ -4851,6 +4856,12 @@ TEST(ServerRequestParsingTest, InvalidSpaceInURL) { EXPECT_EQ("HTTP/1.1 400 Bad Request", out.substr(0, 24)); } +TEST(ServerRequestParsingTest, InvalidFieldValueContains_CR_LF_NUL) { + std::string request( + "GET /header_field_value_check HTTP/1.1\r\nTest: [\r\x00\n]\r\n\r\n", 55); + test_raw_request(request); +} + TEST(ServerStopTest, StopServerWithChunkedTransmission) { Server svr; @@ -7572,3 +7583,26 @@ TEST(FileSystemTest, FileAndDirExistenceCheck) { EXPECT_FALSE(detail::is_file(dir_path)); EXPECT_TRUE(detail::is_dir(dir_path)); } + +TEST(DirtyDataRequestTest, HeadFieldValueContains_CR_LF_NUL) { + Server svr; + + svr.Get("/test", [&](const Request &req, Response &) { + auto val = req.get_header_value("Test"); + EXPECT_EQ(val.size(), 7u); + EXPECT_EQ(val, "_ _ _"); + }); + + auto thread = std::thread([&]() { svr.listen(HOST, PORT); }); + + auto se = detail::scope_exit([&] { + svr.stop(); + thread.join(); + ASSERT_FALSE(svr.is_running()); + }); + + svr.wait_until_ready(); + + Client cli(HOST, PORT); + cli.Get("/test", {{"Test", "_\n\r_\n\r_"}}); +}