From 881842cd7228923192e7eaa9ec0b04a71ec915f9 Mon Sep 17 00:00:00 2001 From: yhirose Date: Tue, 18 Aug 2026 20:58:34 -0400 Subject: [PATCH] Match Connection options as tokens rather than whole field values RFC 9110 Section 7.6.1 defines Connection as a comma-separated list of case-insensitive connection options, and Section 5.3 lets that list be split across several field lines. Comparing the whole field value against a single option gets both wrong. A client sending "Connection: keep-alive, close" was answered without a Connection header and its socket was kept open, so the close it asked for was never performed and never announced. An HTTP/1.0 client asking for keep-alive only got it by spelling the option exactly "Keep-Alive"; the lowercase form everyone actually sends closed the connection instead. Route the five Connection checks through has_header_token(), which already walks every field line and compares complete tokens. Expect is left alone: matching "100-continue" as a token would make an unrecognized expectation alongside it look acceptable, where Section 10.1.1 asks for 417. --- httplib.h | 19 ++++--- test/test.cc | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 8 deletions(-) diff --git a/httplib.h b/httplib.h index ba6d92cd..e62f53a4 100644 --- a/httplib.h +++ b/httplib.h @@ -9555,9 +9555,11 @@ inline bool has_framed_body(const Request &req) { } inline bool is_connection_persistent(const Request &req) { - auto conn = req.get_header_value("Connection"); - if (conn == "close") { return false; } - if (req.version == "HTTP/1.0" && conn != "Keep-Alive") { return false; } + if (has_header_token(req.headers, "Connection", "close")) { return false; } + if (req.version == "HTTP/1.0" && + !has_header_token(req.headers, "Connection", "keep-alive")) { + return false; + } return true; } @@ -12561,7 +12563,8 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection, if (need_apply_ranges) { apply_ranges(req, res, content_type, boundary); } // Prepare additional headers - if (close_connection || req.get_header_value("Connection") == "close" || + if (close_connection || + detail::has_header_token(req.headers, "Connection", "close") || 400 <= res.status) { // Don't leave connections open after errors res.set_header("Connection", "close"); } else { @@ -13466,12 +13469,12 @@ Server::process_request(Stream &strm, const std::string &remote_addr, return write_response(strm, close_connection, req, res); } - if (req.get_header_value("Connection") == "close") { + if (detail::has_header_token(req.headers, "Connection", "close")) { connection_closed = true; } if (req.version == "HTTP/1.0" && - req.get_header_value("Connection") != "Keep-Alive") { + !detail::has_header_token(req.headers, "Connection", "keep-alive")) { connection_closed = true; } @@ -13695,7 +13698,7 @@ Server::process_request(Stream &strm, const std::string &remote_addr, // consume the next request (issue #2450). If the response has committed the // connection to close, there is no next request to protect. if (!req.body_consumed_ && detail::has_framed_body(req)) { - if (res.get_header_value("Connection") == "close") { + if (detail::has_header_token(res.headers, "Connection", "close")) { connection_closed = true; } else { int dummy_status; @@ -14479,7 +14482,7 @@ inline bool ClientImpl::handle_request(Stream &strm, Request &req, if (!ret) { return false; } - if (res.get_header_value("Connection") == "close" || + if (detail::has_header_token(res.headers, "Connection", "close") || (res.version == "HTTP/1.0" && res.reason != "Connection established")) { // NOTE: this requires a not-entirely-obvious chain of calls to be correct // for this to be safe. diff --git a/test/test.cc b/test/test.cc index df5ad923..83af17ef 100644 --- a/test/test.cc +++ b/test/test.cc @@ -17062,6 +17062,144 @@ TEST(ForwardedHeadersTest, MultipleCommasXForwardedFor_DoesNotCrash) { // The same rule applies to Accept: a request whose acceptable types are spread // over several field lines must be negotiated against all of them. +// RFC 9110 Section 7.6.1: Connection carries a comma-separated list of +// case-insensitive connection options. Comparing the whole field value against +// one option misses a client that sends several of them, and misses every +// casing but the one written in the comparison. +// +// Sends req, reads the response, then reuses the same socket for a second +// request to find out whether the server kept the connection. +static void probe_connection_reuse(int port, const std::string &req, + bool *announced_close, bool *reusable) { + auto error = Error::Success; + auto sock = detail::create_client_socket( + HOST, "", port, AF_UNSPEC, false, false, nullptr, + /*connection_timeout_sec=*/5, 0, + /*read_timeout_sec=*/1, 0, + /*write_timeout_sec=*/5, 0, std::string(), error); + ASSERT_NE(sock, INVALID_SOCKET); + auto se = detail::scope_exit([&] { detail::close_socket(sock); }); + + const std::string second = "GET /keepalive HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: close\r\n" + "\r\n"; + std::string first_response; + std::string second_response; + + detail::process_client_socket( + sock, 1, 0, 5, 0, 0, std::chrono::steady_clock::time_point::min(), + [&](Stream &strm) { + if (strm.write(req.data(), req.size()) != + static_cast(req.size())) { + return false; + } + + char buf[512]; + detail::stream_line_reader reader(strm, buf, sizeof(buf)); + // Headers plus the two-byte body of the handler below + while (reader.getline()) { + first_response += reader.ptr(); + if (first_response.find("ok") != std::string::npos) { break; } + } + + if (strm.write(second.data(), second.size()) != + static_cast(second.size())) { + return true; + } + detail::stream_line_reader reader2(strm, buf, sizeof(buf)); + while (reader2.getline()) { + second_response += reader2.ptr(); + if (second_response.find("ok") != std::string::npos) { break; } + } + return true; + }); + + *announced_close = + first_response.find("Connection: close") != std::string::npos; + *reusable = second_response.find("HTTP/1.1 200") != std::string::npos; +} + +class ConnectionTokenTest : public ::testing::Test { +protected: + void SetUp() override { + svr_.Get("/keepalive", [](const Request &, Response &res) { + res.set_content("ok", "text/plain"); + }); + port_ = svr_.bind_to_any_port(HOST); + thread_ = thread([&]() { svr_.listen_after_bind(); }); + svr_.wait_until_ready(); + } + + void TearDown() override { + svr_.stop(); + if (thread_.joinable()) { thread_.join(); } + } + + Server svr_; + int port_ = 0; + thread thread_; +}; + +// "close" alongside another option still closes the connection, and the +// response says so rather than leaving the peer to discover it. +TEST_F(ConnectionTokenTest, CloseAmongSeveralOptionsIsHonored) { + bool announced_close = false; + bool reusable = true; + probe_connection_reuse(port_, + "GET /keepalive HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive, close\r\n" + "\r\n", + &announced_close, &reusable); + EXPECT_TRUE(announced_close); + EXPECT_FALSE(reusable); +} + +// "close" split over two field lines is the same list, so it is honored too. +TEST_F(ConnectionTokenTest, CloseOnALaterFieldLineIsHonored) { + bool announced_close = false; + bool reusable = true; + probe_connection_reuse(port_, + "GET /keepalive HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n" + "Connection: close\r\n" + "\r\n", + &announced_close, &reusable); + EXPECT_TRUE(announced_close); + EXPECT_FALSE(reusable); +} + +// Connection options are case-insensitive, so an HTTP/1.0 client asking for +// keep-alive in the spelling everyone actually sends keeps its connection. +TEST_F(ConnectionTokenTest, Http10KeepAliveIsCaseInsensitive) { + bool announced_close = false; + bool reusable = false; + probe_connection_reuse(port_, + "GET /keepalive HTTP/1.0\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n" + "\r\n", + &announced_close, &reusable); + EXPECT_FALSE(announced_close); + EXPECT_TRUE(reusable); +} + +// A token the value merely contains is not the token itself. +TEST_F(ConnectionTokenTest, SubstringOfAnOptionIsNotTheOption) { + bool announced_close = false; + bool reusable = false; + probe_connection_reuse(port_, + "GET /keepalive HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: notclose\r\n" + "\r\n", + &announced_close, &reusable); + EXPECT_FALSE(announced_close); + EXPECT_TRUE(reusable); +} + TEST(RepeatedFieldLinesTest, AcceptCombinesEveryFieldLine) { Server svr;