From 5a2d625e08c7cee0931e42d63a916621ceaa7709 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Thu, 31 Oct 2024 13:34:19 -0400 Subject: [PATCH] http: read requests from connected clients --- src/httpserver.cpp | 77 +++++++++++++++++++++++++++++++++++ src/httpserver.h | 27 +++++++++++- src/test/httpserver_tests.cpp | 23 ++++++++++- 3 files changed, 123 insertions(+), 4 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index e7a3b8fe694..d3783eb1cb0 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -906,6 +906,24 @@ bool HTTPRequest::LoadBody(LineReader& reader) return true; } +bool HTTPClient::ReadRequest(std::unique_ptr& req) +{ + LineReader reader(m_recv_buffer, MAX_HEADERS_SIZE); + + if (!req->LoadControlData(reader)) return false; + if (!req->LoadHeaders(reader)) return false; + if (!req->LoadBody(reader)) return false; + + // Remove the bytes read out of the buffer. + // If one of the above calls throws an error, the caller must + // catch it and disconnect the client. + m_recv_buffer.erase( + m_recv_buffer.begin(), + m_recv_buffer.begin() + (reader.it - reader.start)); + + return true; +} + bool HTTPServer::EventNewConnectionAccepted(NodeId node_id, const CService& me, const CService& them) @@ -918,4 +936,63 @@ bool HTTPServer::EventNewConnectionAccepted(NodeId node_id, m_no_clients = false; return true; } + +void HTTPServer::EventGotData(NodeId node_id, std::span data) +{ + // Get the HTTPClient + auto client{GetClientById(node_id)}; + if (client == nullptr) { + return; + } + + // Copy data from socket buffer to client receive buffer + client->m_recv_buffer.insert( + client->m_recv_buffer.end(), + reinterpret_cast(data.data()), + reinterpret_cast(data.data() + data.size()) + ); + + // Try reading (potentially multiple) HTTP requests from the buffer + while (client->m_recv_buffer.size() > 0) { + // Create a new request object and try to fill it with data from the receive buffer + auto req = std::make_unique(client); + try { + // Stop reading if we need more data from the client to parse a complete request + if (!client->ReadRequest(req)) break; + } catch (const std::runtime_error& e) { + LogDebug( + BCLog::HTTP, + "Error reading HTTP request from client %s (id=%lld): %s\n", + client->m_origin, + client->m_node_id, + e.what()); + + // We failed to read a complete request from the buffer + // TODO: respond with HTTP_BAD_REQUEST and disconnect + + break; + } + + // We read a complete request from the buffer into the queue + LogDebug( + BCLog::HTTP, + "Received a %s request for %s from %s (id=%lld)\n", + req->m_method, + req->m_target, + req->m_client->m_origin, + req->m_client->m_node_id); + + // handle request + m_request_dispatcher(std::move(req)); + } +} + +std::shared_ptr HTTPServer::GetClientById(NodeId node_id) const +{ + auto it{m_connected_clients.find(node_id)}; + if (it != m_connected_clients.end()) { + return it->second; + } + return nullptr; +} } // namespace http_bitcoin diff --git a/src/httpserver.h b/src/httpserver.h index 33de955667d..acd5b9fee54 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -240,6 +240,8 @@ public: std::string StringifyHeaders() const; }; +class HTTPClient; + class HTTPRequest { public: @@ -251,6 +253,13 @@ public: HTTPHeaders m_headers; std::string m_body; + // Keep a pointer to the client that made the request so + // we know who to respond to. + std::shared_ptr m_client; + explicit HTTPRequest(std::shared_ptr client) : m_client(client) {}; + // Null client for unit tests + explicit HTTPRequest() : m_client(nullptr) {}; + // Readers return false if they need more data from the // socket to parse properly. They throw errors if // the data is invalid. @@ -274,11 +283,19 @@ public: // Ok to remain null for unit tests. HTTPServer* m_server; + // In lieu of an intermediate transport class like p2p uses, + // we copy data from the socket buffer to the client object + // and attempt to read HTTP requests from here. + std::vector m_recv_buffer{}; + explicit HTTPClient(NodeId node_id, CService addr) : m_node_id(node_id), m_addr(addr) { m_origin = addr.ToStringAddrPort(); }; + // Try to read an HTTP request from the receive buffer + bool ReadRequest(std::unique_ptr& req); + // Disable copies (should only be used as shared pointers) HTTPClient(const HTTPClient&) = delete; HTTPClient& operator=(const HTTPClient&) = delete; @@ -287,13 +304,19 @@ public: class HTTPServer : public SockMan { public: + explicit HTTPServer(std::function)> func) : m_request_dispatcher(func) {}; + // Set in the Sockman I/O loop and only checked by main thread when shutting // down to wait for all clients to be disconnected. std::atomic_bool m_no_clients{true}; - //! Connected clients with live HTTP connections std::unordered_map> m_connected_clients; + // What to do with HTTP requests once received, validated and parsed + std::function)> m_request_dispatcher; + + std::shared_ptr GetClientById(NodeId node_id) const; + /** * Be notified when a new connection has been accepted. * @param[in] node_id Id of the newly accepted connection. @@ -320,7 +343,7 @@ public: * @param[in] node_id Connection for which the data arrived. * @param[in] data Received data. */ - virtual void EventGotData(NodeId node_id, std::span data) override {}; + virtual void EventGotData(NodeId node_id, std::span data) override; /** * Called when the remote peer has sent an EOF on the socket. This is a graceful diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index 309e8995238..627251bbb10 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -10,6 +10,7 @@ #include +using http_bitcoin::HTTPClient; using http_bitcoin::HTTPHeaders; using http_bitcoin::HTTPRequest; using http_bitcoin::HTTPResponse; @@ -307,8 +308,17 @@ BOOST_AUTO_TEST_CASE(http_client_server_tests) // Prepare queue of accepted_sockets: just one connection with no data accepted_sockets->Push(std::move(connected_socket)); - // Instantiate server - HTTPServer server = HTTPServer(); + // Prepare a request handler that just stores received requests so we can examine them + // Mutex is required to prevent a race between this test's main thread and the Sockman I/O loop. + Mutex requests_mutex; + std::deque> requests; + auto StoreRequest = [&](std::unique_ptr req) { + LOCK(requests_mutex); + requests.push_back(std::move(req)); + }; + + // Instantiate server with dead-end request handler + HTTPServer server = HTTPServer(StoreRequest); BOOST_REQUIRE(server.m_no_clients); // This address won't actually get used because we stubbed CreateSock() @@ -333,6 +343,15 @@ BOOST_AUTO_TEST_CASE(http_client_server_tests) } BOOST_REQUIRE(!server.m_no_clients); + { + LOCK(requests_mutex); + // Connected client should have one request already from the static content. + BOOST_CHECK_EQUAL(requests.size(), 1); + + // Check the received request + BOOST_CHECK_EQUAL(requests.front()->m_body, "{\"method\":\"getblockcount\",\"params\":[],\"id\":1}\n"); + } + // Close server server.interruptNet(); // Wait for I/O loop to finish, after all sockets are closed