From b8cd77237b3425f5a589d7fd24dfb3847c39dcd8 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:25:40 +0200 Subject: [PATCH] refactor: Make HTTPRequest::GetHeader() return saner optional type No need to stick to weird old API from libevent-wrapper days. Makes later commits in the PR cleaner. --- src/httprpc.cpp | 6 +++--- src/httpserver.cpp | 5 ++--- src/httpserver.h | 2 +- src/test/fuzz/http_request.cpp | 10 +++++----- src/test/httpserver_tests.cpp | 16 ++++++++-------- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index ed068a34b8e..f9b95dd8361 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -202,8 +202,8 @@ static void HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) return; } // Check authorization - std::pair authHeader = req->GetHeader("authorization"); - if (!authHeader.first) { + std::optional auth_header = req->GetHeader("authorization"); + if (!auth_header) { req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA); req->WriteReply(HTTP_UNAUTHORIZED); return; @@ -213,7 +213,7 @@ static void HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) jreq.context = context; jreq.peerAddr = req->GetPeer().ToStringAddrPort(); jreq.URI = req->GetURI(); - if (!RPCAuthorized(authHeader.second, jreq.authUser)) { + if (!RPCAuthorized(*auth_header, jreq.authUser)) { LogWarning("ThreadRPCServer incorrect password attempt from %s", jreq.peerAddr); /* Deter brute-forcing diff --git a/src/httpserver.cpp b/src/httpserver.cpp index f6a849f5b2e..22dde8a696a 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -693,10 +693,9 @@ std::optional GetQueryParameterFromUri(const std::string_view uri, return std::nullopt; } -std::pair HTTPRequest::GetHeader(const std::string_view hdr) const +std::optional HTTPRequest::GetHeader(const std::string_view hdr) const { - std::optional found{m_headers.FindFirst(hdr)}; - return std::pair{found.has_value(), std::move(found).value_or("")}; + return m_headers.FindFirst(hdr); } void HTTPRequest::WriteHeader(std::string&& hdr, std::string&& value) diff --git a/src/httpserver.h b/src/httpserver.h index 4e1d1a9a716..42d5b155dc0 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -192,7 +192,7 @@ public: CService GetPeer() const; HTTPRequestMethod GetRequestMethod() const { return m_method; } std::optional GetQueryParameter(std::string_view key) const; - std::pair GetHeader(std::string_view hdr) const; + std::optional GetHeader(std::string_view hdr) const; std::string ReadBody() const { return m_body; } void WriteHeader(std::string&& hdr, std::string&& value); diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index 75d2729e94e..3ae5355c5ef 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -51,14 +51,14 @@ FUZZ_TARGET(http_request) // empty string here; LoadBody now populates the body per RFC 9112 framing, so mirror // its branch logic to assert the body matches the framing that produced it. const std::string body = http_request.ReadBody(); - const auto [has_transfer_encoding, transfer_encoding] = http_request.GetHeader("Transfer-Encoding"); - const auto [has_content_length, content_length] = http_request.GetHeader("Content-Length"); - if (has_transfer_encoding && ToLower(transfer_encoding) == "chunked") { + const auto transfer_encoding = http_request.GetHeader("Transfer-Encoding"); + const auto content_length = http_request.GetHeader("Content-Length"); + if (transfer_encoding && ToLower(*transfer_encoding) == "chunked") { // A chunked body is the concatenation of the decoded chunks, bounded by MAX_BODY_SIZE. assert(body.size() <= http_bitcoin::MAX_BODY_SIZE); - } else if (has_content_length) { + } else if (content_length) { // A Content-Length body is exactly that many bytes. - const auto parsed_length{ToIntegral(content_length)}; + const auto parsed_length{ToIntegral(*content_length)}; assert(parsed_length); assert(body.size() == *parsed_length); } else { diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index 2b0d172e1ab..94f9920dd87 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -212,11 +212,11 @@ BOOST_AUTO_TEST_CASE(http_request_tests) BOOST_CHECK_EQUAL(req.GetURI(), "/"); BOOST_CHECK_EQUAL(req.m_version.major, 1); BOOST_CHECK_EQUAL(req.m_version.minor, 1); - BOOST_CHECK_EQUAL(req.m_headers.FindFirst("Host"), "127.0.0.1"); - BOOST_CHECK_EQUAL(req.m_headers.FindFirst("Connection"), "close"); - BOOST_CHECK_EQUAL(req.m_headers.FindFirst("Content-Type"), "application/json"); - BOOST_CHECK_EQUAL(req.m_headers.FindFirst("Authorization"), "Basic X19jb29raWVfXzo5OGQ5ODQ3MWNmNjg0NzAzYTkzN2EzNzk0ZDFlODQ1NjZmYTRkZjJiMzFkYjhhODI4ZGY4MjVjOTg5ZGI4OTVl"); - BOOST_CHECK_EQUAL(req.m_headers.FindFirst("Content-Length"), "46"); + BOOST_CHECK_EQUAL(req.GetHeader("Host"), "127.0.0.1"); + BOOST_CHECK_EQUAL(req.GetHeader("Connection"), "close"); + BOOST_CHECK_EQUAL(req.GetHeader("Content-Type"), "application/json"); + BOOST_CHECK_EQUAL(req.GetHeader("Authorization"), "Basic X19jb29raWVfXzo5OGQ5ODQ3MWNmNjg0NzAzYTkzN2EzNzk0ZDFlODQ1NjZmYTRkZjJiMzFkYjhhODI4ZGY4MjVjOTg5ZGI4OTVl"); + BOOST_CHECK_EQUAL(req.GetHeader("Content-Length"), "46"); BOOST_CHECK_EQUAL(req.m_body.size(), 46); BOOST_CHECK_EQUAL(req.m_body, R"({"method":"getblockcount","params":[],"id":1})""\n"); } @@ -317,7 +317,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests) BOOST_CHECK_EQUAL(req.m_target, "/"); BOOST_CHECK_EQUAL(req.m_version.major, 1); BOOST_CHECK_EQUAL(req.m_version.minor, 0); - BOOST_CHECK_EQUAL(req.m_headers.FindFirst("Host"), "127.0.0.1"); + BOOST_CHECK_EQUAL(req.GetHeader("Host"), "127.0.0.1"); // no body is OK BOOST_CHECK_EQUAL(req.m_body.size(), 0); } @@ -448,7 +448,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests) BOOST_CHECK_EQUAL(req.m_body, R"({"method":"getblockcount"})"); // Chunk Trailer was parsed, but ignored BOOST_CHECK_EQUAL(reader.Remaining(), 0); - BOOST_CHECK(!req.GetHeader("Expires").first); + BOOST_CHECK(!req.GetHeader("Expires")); } { // Invalid "chunked" transfer, using roman numerals instead of hex for chunk length @@ -672,7 +672,7 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests) BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Error); // We read up to the invalid line - BOOST_CHECK_EQUAL(*client->m_req->m_headers.FindFirst("Host"), "127.0.0.1"); + BOOST_CHECK_EQUAL(client->m_req->GetHeader("Host"), "127.0.0.1"); // Buffer was cleared, client should just be disconnected now BOOST_CHECK(client->m_recv_buffer.empty());