From 10bbae302fb7d659629c93dffc4e9cfde762f78c Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:19:49 +0200 Subject: [PATCH] refactor: Expose HTTPRemoteClient fields to tests through methods Enables making the fields private later. --- src/httpserver.h | 10 ++ src/test/httpserver_tests.cpp | 248 ++++++++++++++++------------------ 2 files changed, 127 insertions(+), 131 deletions(-) diff --git a/src/httpserver.h b/src/httpserver.h index de4af1d3c7a..f06d300c08a 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -596,6 +596,16 @@ public: * @returns false if we are done with this client and HTTPServer can skip the next read operation from it. */ bool MaybeSendBytesFromBuffer() EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex, !m_sock_mutex); + + //! Used for tests. + //! @{ + const std::string& GetRecvBuffer() const { return m_recv_buffer; } + const HTTPRequest* GetRequest() const { return m_req.get(); } + //! @} + +protected: + //! Used for tests. + std::string& MutateRecvBuffer() { return m_recv_buffer; } }; /** Initialize HTTP server. diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index 5459fb49e29..6d79f0dd32e 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -493,119 +493,114 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests) void receive(std::string_view s) { - m_recv_buffer.insert( - m_recv_buffer.end(), - s.begin(), - s.end()); + MutateRecvBuffer().append(s); } }; { // Step through state machine std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!client->GetRequest()); client->receive("POST / HTTP/1.0\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsHeaders); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsHeaders); client->receive("Host: 127.0.0.1\n" "Content-Length: 10\n\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); client->receive("I miss you\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); + auto req{HTTPRemoteClient::TryReadRequest(client)}; + BOOST_REQUIRE(req); + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); } { // Read body over multiple data pushes, multiple requests in same push std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!client->GetRequest()); client->receive("POST / HTTP/1.0\n" "Host: 127.0.0.1\n" "Content-Length: 10\n\n" "I miss"); - client->ReadRequest(*client->m_req); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); // Because of the Content-Length header we know the body is not complete - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Finish sending first request and include second request in the same buffer client->receive(" you" "GET /endpoint HTTP/1.0\n\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); - BOOST_CHECK_EQUAL(client->m_req->ReadBody(), "I miss you"); + auto req{HTTPRemoteClient::TryReadRequest(client)}; + BOOST_REQUIRE(req); + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); + BOOST_CHECK_EQUAL(req->GetURI(), "/"); + BOOST_CHECK(!client->GetRequest()); + BOOST_CHECK_EQUAL(req->ReadBody(), "I miss you"); + req->WriteReply(HTTP_OK, ""); // Mark client as no longer busy // Next request sitting in buffer - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 24); - // Complete first request hasn't been moved yet, expect no-op - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 24); - - // Reset m_req - client->m_req = std::make_unique(client); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 24); // Read second request - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); - BOOST_CHECK_EQUAL(client->m_req->GetURI(), "/endpoint"); - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 0); + req = HTTPRemoteClient::TryReadRequest(client); + BOOST_REQUIRE(req); + BOOST_CHECK(!client->GetRequest()); + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); + BOOST_CHECK_EQUAL(req->GetURI(), "/endpoint"); + BOOST_CHECK_EQUAL(req->ReadBody().size(), 0); // Buffer is cleared - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 0); + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 0); } { // A Content-Length body is drained out of the receive buffer as it // arrives, instead of accumulating there until the request is complete. std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!client->GetRequest()); client->receive("POST / HTTP/1.0\n" "Content-Length: 30000\n\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + HTTPRemoteClient::TryReadRequest(client); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Body arrives in 10kB pieces. Each one is copied onto m_body and // erased from the receive buffer, which never holds more than one piece. for (int i = 1; i <= 3; ++i) { client->receive(std::string(10000, 'x')); - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 10000); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 10000 * i); - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 0); + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 10000); + auto req{HTTPRemoteClient::TryReadRequest(client)}; + if (i < 3) { + BOOST_CHECK(!req.get()); + BOOST_CHECK_EQUAL(client->GetRequest()->ReadBody().size(), 10000 * i); + } else { + BOOST_CHECK(req.get()); + BOOST_CHECK_EQUAL(req->ReadBody().size(), 10000 * i); + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); + BOOST_CHECK(!client->GetRequest()); + } + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 0); } - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); } { // A body sent in the same push as the next request is split correctly std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!client->GetRequest()); client->receive("POST / HTTP/1.0\n" "Content-Length: 4\n\n" "body" "GET /next HTTP/1.0\n\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); - BOOST_CHECK_EQUAL(client->m_req->ReadBody(), "body"); + auto req{HTTPRemoteClient::TryReadRequest(client)}; + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); + BOOST_CHECK_EQUAL(req->ReadBody(), "body"); // Only the second request is left over - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 20); + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 20); } { // Chunked transfer with state std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - - BOOST_CHECK(!client->m_req->GetChunkSize()); - BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 0); - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 0); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!client->GetRequest()); // First chunk is incomplete client->receive("GET / HTTP/1.0\n" @@ -613,136 +608,129 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests) "\n" "10\n" R"({"method)"); - client->ReadRequest(*client->m_req); - BOOST_CHECK(client->m_req->GetChunkSize()); - BOOST_CHECK_EQUAL(*client->m_req->GetChunkSize(), 16); - BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 8); - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 8); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_REQUIRE(client->GetRequest()->GetChunkSize()); + BOOST_CHECK_EQUAL(*client->GetRequest()->GetChunkSize(), 16); + BOOST_CHECK_EQUAL(client->GetRequest()->GetChunkProgress(), 8); + BOOST_CHECK_EQUAL(client->GetRequest()->ReadBody().size(), 8); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // More data arrives, chunk is completed. client->receive(R"(":"getbl)""\n"); - client->ReadRequest(*client->m_req); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); // State is reset - BOOST_CHECK(!client->m_req->GetChunkSize()); - BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 0); + BOOST_CHECK(!client->GetRequest()->GetChunkSize()); + BOOST_CHECK_EQUAL(client->GetRequest()->GetChunkProgress(), 0); // New data is added to body but body is still incomplete - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 16); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK_EQUAL(client->GetRequest()->ReadBody().size(), 16); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Next chunk arrives without terminal CRLF client->receive("a\n" R"(ockcount"})"); - client->ReadRequest(*client->m_req); - BOOST_CHECK(client->m_req->GetChunkSize()); - BOOST_CHECK_EQUAL(*client->m_req->GetChunkSize(), 10); - BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 10); - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 26); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK(client->GetRequest()->GetChunkSize()); + BOOST_CHECK_EQUAL(*client->GetRequest()->GetChunkSize(), 10); + BOOST_CHECK_EQUAL(client->GetRequest()->GetChunkProgress(), 10); + BOOST_CHECK_EQUAL(client->GetRequest()->ReadBody().size(), 26); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Chunk terminal CRLF arrives with final (size 0) chunk client->receive("\n0\n\n"); - client->ReadRequest(*client->m_req); + auto req{HTTPRemoteClient::TryReadRequest(client)}; // Body size hasn't changed - BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 26); + BOOST_CHECK_EQUAL(req->ReadBody().size(), 26); // We're done - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); - BOOST_CHECK_EQUAL(client->m_req->ReadBody(), R"({"method":"getblockcount"})"); + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); + BOOST_CHECK_EQUAL(req->ReadBody(), R"({"method":"getblockcount"})"); } { // Invalid headers: error state stops reading std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); // Request is in the buffer client->receive("POST / HTTP/1.0\n" - "Host: 127.0.0.1\n" - "Invalid header with no colon\n" + "Host: 127.0.0.1\n"); + BOOST_CHECK(!client->GetRecvBuffer().empty()); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsHeaders); + client->receive("Invalid header with no colon\n" "\n" "body is not read"); - BOOST_CHECK(!client->m_recv_buffer.empty()); - // Reading throws an error, sets state - BOOST_CHECK_EXCEPTION(client->ReadRequest(*client->m_req), - std::runtime_error, - HasReason{"HTTP header missing colon (:)"}); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Error); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::Error); // We read up to the invalid line - BOOST_CHECK_EQUAL(client->m_req->GetHeader("Host"), "127.0.0.1"); + BOOST_CHECK_EQUAL(client->GetRequest()->GetHeader("Host"), "127.0.0.1"); // Buffer was cleared, client should just be disconnected now - BOOST_CHECK(client->m_recv_buffer.empty()); + BOOST_CHECK(client->GetRecvBuffer().empty()); // Even if more data comes in, trying to read again in error state is a no-op client->receive("Content-Length: 2\n\nok"); - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 21); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 21); + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 21); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRecvBuffer().size(), 21); } { // Headers sent in batches that are below MAX_HEADERS_SIZE but the total is excessive std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!client->GetRequest()); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::Init); client->receive("POST /huge HTTP/1.0\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsHeaders); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsHeaders); for (int i = 0; i < 410; ++i) { client->receive("key:value\n"); } - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsHeaders); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsHeaders); for (int i = 0; i < 409; ++i) { client->receive("key:value\n"); } - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsHeaders); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsHeaders); // We're at 819 x 10-byte headers // The limit is 8192, three more bytes should throw. client->receive("k:\n"); - BOOST_CHECK_EXCEPTION(client->ReadRequest(*client->m_req), - std::runtime_error, - HasReason{"HTTP headers exceed size limit"}); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Error); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::Error); } { // Client sends chunks that are below the limit but the total is excessive std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::Init); client->receive("POST /huge HTTP/1.0\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsHeaders); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsHeaders); client->receive("Transfer-Encoding: chunked\n\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Send 16-byte chunk client->receive("10\nno auto updates!\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // The next chunk will be of size 32MiB - 16 + 1, below the limit // on its own but not if it were added to the total cumulative body so far. // We don't need to actually send or prepare this amount of data. client->receive("1fffff1\n"); - BOOST_CHECK_EXCEPTION(client->ReadRequest(*client->m_req), - http_bitcoin::ContentTooLargeError, - HasReason{"Chunk will exceed max body size"}); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Error); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::Error); } { // Ensure chunk trailer is parsed over state lines std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); + BOOST_CHECK(!client->GetRequest()); // Send a 1-byte chunk then send the 0-chunk with a trailer but no terminal CRLF client->receive("GET / HTTP/1.0\n" @@ -752,29 +740,29 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests) "x\n" "0\n" "Digest: sha-4=deadbeef\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Send first part of another trailer line client->receive("Expires:"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Finish the trailer line client->receive("never\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // Terminate client->receive("\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete); - BOOST_CHECK_EQUAL(client->m_req->ReadBody(), "x"); + auto req{HTTPRemoteClient::TryReadRequest(client)}; + BOOST_CHECK_EQUAL(req->GetState(), HTTPRequest::State::Complete); + BOOST_CHECK_EQUAL(req->ReadBody(), "x"); } { // Ensure chunk trailer counts towards the headers size limit std::shared_ptr client{std::make_shared()}; - client->m_req = std::make_unique(client); + BOOST_CHECK(!client->GetRequest()); client->receive("POST /huge HTTP/1.0\n" "Transfer-Encoding: chunked\n"); // 27 bytes @@ -785,16 +773,14 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests) "1\n" "x\n" "0\n"); - client->ReadRequest(*client->m_req); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::NeedsBody); // We're in the trailer section with a total of 8188 bytes of headers. // The limit is 8192, five more bytes should throw. client->receive("k:vv\n"); - BOOST_CHECK_EXCEPTION(client->ReadRequest(*client->m_req), - std::runtime_error, - HasReason{"HTTP headers exceed size limit"}); - BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Error); + BOOST_CHECK(!HTTPRemoteClient::TryReadRequest(client)); + BOOST_CHECK_EQUAL(client->GetRequest()->GetState(), HTTPRequest::State::Error); } }