From 922b08d375351e313dd92fbefdb166ee27838ac0 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 26 Jun 2026 15:25:44 -0400 Subject: [PATCH] test: socket error handling in HTTPServer using ErrorSock mock socket Implements a child class of DynSock which is used as the mock socket for HTTPServer unit tests. The ErrorSock::Send() method raises a non-permanent error on the first HTTPRequest::WriteReply() and then succeeds after the second. In httpserver_tests.cpp use this mechanism to ensure that the server retries a send operation if such an error is encountered, and cover both optimistic (worker thread WriteReply()) and non-optimistic (I/O thread SocketHandlerConnected()) send paths. --- src/test/httpserver_tests.cpp | 119 +++++++++++++++++++++++++++++++++ src/test/util/setup_common.cpp | 18 ----- src/test/util/setup_common.h | 12 +++- 3 files changed, 129 insertions(+), 20 deletions(-) diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index 33c8b7769b2..8e66d3d4c4c 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -628,4 +629,122 @@ BOOST_AUTO_TEST_CASE(http_server_socket_tests) server.StopListening(); } +BOOST_AUTO_TEST_CASE(http_socket_error_tests) +{ + // Hard-code the server's request handler to respond to each request with + // an incremented block count. + int height{0}; + HTTPServer server{[&](std::shared_ptr req) { + req->WriteReply(HTTP_OK, strprintf("height: %d\n", height++)); + }}; + + // All replies will be the same size + static constexpr std::size_t reply_length = std::string_view{ + "HTTP/1.1 200 OK\r\n" + "Date: Thu, 01 Jan 2026 00:00:00 GMT\r\n" // All RFC1123 dates are 29 characters + "Content-Length: 10\r\n" + "Content-Type: text/html; charset=ISO-8859-1\r\n" + "\r\n" + "height: 0\n" + }.size(); + + /** + * A mocked Sock derived from DynSock whose Send() only succeeds when there is more than + * one reply being sent (send buffer length > reply_length). Otherwise it returns + * a recoverable error (WSAEAGAIN). + * + * After it sends successfully once, it continues to always succeed. + * + * Useful for testing "try again" logic around non-blocking socket Send() failures. + */ + class ErrorSock : public DynSock + { + public: + explicit ErrorSock(std::shared_ptr pipes) : DynSock{std::move(pipes)} {} + DynSock& operator=(Sock&&) override { assert(false); return *this; } + + ssize_t Send(const void* buf, size_t len, int flags) const override + { + if (len <= reply_length && !m_have_sent) { + #ifdef WIN32 + WSASetLastError(WSAEWOULDBLOCK); + #else + errno = WSAEAGAIN; + #endif + return -1; + } else { + m_have_sent = true; + return DynSock::Send(buf, len, flags); + } + } + + mutable bool m_have_sent{false}; + }; + + // Simpler server startup than the last test + CService addr_bind{Lookup("0.0.0.0", /*portDefault=*/0, /*fAllowLookup=*/false).value()}; + BOOST_REQUIRE(server.BindAndStartListening(addr_bind)); + server.StartSocketsThreads(); + + // Prepare initial requests + int num_requests = 3; + // Use keep-alive so the server holds the connection open for all requests. + std::string keepalive_request{full_request}; + keepalive_request.replace(keepalive_request.find("Connection: close"), 17, "Connection: keep-alive"); + // Combine all requests so they are read from the socket on a single iteration of the I/O loop + std::string all_requests; + for (int i = 0; i < num_requests; i++) { + all_requests += keepalive_request; + } + + // Watch the log messages to ensure that the first two replies were sent + // together. This indicates the non-optimistic send path was used + // because a reply was already sitting in the send buffer when a second reply + // was added. + DebugLogHelper find_two_replies{strprintf("Sent %d bytes to client", reply_length * 2), + [&](const std::string* s) { + return true; + }}; + // Last reply should be sent on its own by optimistic send path, because + // the send buffer was empty when the reply was written. + DebugLogHelper find_one_reply{strprintf("Sent %d bytes to client", reply_length), + [&](const std::string* s) { + return true; + }}; + + // Connect the ErrorSock as mock client with the preloaded data and get a handle on the I/O pipes + std::shared_ptr mock_client_socket_pipes{ + ConnectClient(std::as_bytes(std::span(all_requests))) + }; + + // Wait up to one minute for the last reply from the server + std::string actual; + char buf[0x10000] = {}; + int attempts = 1000; + while (attempts > 0) + { + ssize_t bytes_read = mock_client_socket_pipes->send.GetBytes(buf, sizeof(buf), 0); + if (bytes_read > 0) { + actual.append(buf, bytes_read); + if (actual.find(strprintf("height: %d", num_requests - 1)) != std::string::npos) { + break; + } + } + std::this_thread::sleep_for(10ms); + --attempts; + } + + // All replies were received + for (int i = 0; i < num_requests; i++) { + BOOST_REQUIRE(actual.find(strprintf("height: %d", i)) != std::string::npos); + } + + // Close the keep-alive connection + server.DisconnectAllClients(); + + server.InterruptNet(); + server.JoinSocketsThreads(); + server.StopListening(); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 830fca0ea9e..bcf637784d7 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -657,24 +657,6 @@ SocketTestingSetup::~SocketTestingSetup() CreateSock = m_create_sock_orig; } -std::shared_ptr SocketTestingSetup::ConnectClient(std::span data) -{ - // I/O pipes for a mock Connected Socket we can read and write to. - auto connected_socket_pipes(std::make_shared()); - - // Insert the payload - connected_socket_pipes->recv.PushBytes(data.data(), data.size()); - - // Create the Mock Connected Socket that represents a client. - // It needs I/O pipes but its queue can remain empty - std::unique_ptr connected_socket{std::make_unique(connected_socket_pipes)}; - - // Push into the queue of Accepted Sockets returned by the local CreateSock() - m_accepted_sockets.Push(std::move(connected_socket)); - - return connected_socket_pipes; -} - /** * @returns a real block (0000000000013b8ab2cd513b0261a14096412195a72a0c4827d229dcc7e0f7af) * with 9 txs. diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index e44c7e728bf..7818224e554 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -257,10 +257,18 @@ public: ~SocketTestingSetup(); /** - * Connect to the socket with a mock client (a DynSock) and send pre-loaded data. + * Connect to the socket with a mock client and send pre-loaded data. * Returns the I/O pipes from the mock client so we can read response data sent to it. + * Template parameter selects the socket type: DynSock by default. */ - std::shared_ptr ConnectClient(std::span data); + template + std::shared_ptr ConnectClient(std::span data) + { + auto connected_socket_pipes(std::make_shared()); + connected_socket_pipes->recv.PushBytes(data.data(), data.size()); + m_accepted_sockets.Push(std::make_unique(connected_socket_pipes)); + return connected_socket_pipes; + } private: //! Save the original value of CreateSock here and restore it when the test ends.