From 9954aa77280ecd67816e784815c6478a973f6635 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Thu, 6 Aug 2026 17:23:44 -0400 Subject: [PATCH] http: don't parse any new requests from a client if m_req_busy = true --- src/httpserver.cpp | 12 ++++++------ test/functional/interface_http.py | 32 ++++++++++++++++++++++++------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 16215170141..d72e9d9fd7e 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -1016,6 +1016,11 @@ void HTTPServer::ThreadSocketHandler() void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr& client) const { + // If we are already handling a request from + // this client, do nothing. We'll check again on the next I/O + // loop iteration. + if (client->m_req_busy) return; + if (!client->m_req) { client->m_req = std::make_unique(client); } @@ -1048,12 +1053,7 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptrm_req_busy) return; - - // Otherwise, if the request is ready, hand it to a worker. + // If the request is ready, hand it to a worker. if (client->m_req->GetState() == HTTPRequest::State::Complete) { LogDebug( BCLog::HTTP, diff --git a/test/functional/interface_http.py b/test/functional/interface_http.py index 13500152588..3e5d1832fb0 100755 --- a/test/functional/interface_http.py +++ b/test/functional/interface_http.py @@ -116,7 +116,8 @@ class HTTPBasicsTest (BitcoinTestFramework): self.check_keepalive_connection() self.check_close_connection() self.check_excessive_request_size() - self.check_pipelining() + self.check_pipelining(with_invalid_second_request=False) + self.check_pipelining(with_invalid_second_request=True) self.check_chunked_transfer() self.check_idle_timeout() self.check_server_busy_idle_timeout() @@ -257,12 +258,12 @@ class HTTPBasicsTest (BitcoinTestFramework): send_thread.join() - def check_pipelining(self): + def check_pipelining(self, with_invalid_second_request): """ Requests are responded to in the order in which they were received See https://www.rfc-editor.org/rfc/rfc7230#section-6.3.2 """ - self.log.info("Check pipelining") + self.log.info("Check pipelining" + (" with invalid second request" if with_invalid_second_request else "")) tip_height = self.node.getblockcount() conn = BitcoinHTTPConnection(self.node) conn.set_timeout(5) @@ -270,7 +271,10 @@ class HTTPBasicsTest (BitcoinTestFramework): # Send two requests in a row. # The first request will block the second indefinitely conn.post_raw('/', f'{{"method": "waitforblockheight", "params": [{tip_height + 1}]}}') - conn.post_raw('/', '{"method": "getblockcount"}') + if with_invalid_second_request: + conn.post_raw(f'/{"x" * MAX_HEADERS_SIZE * 2}', '{"method": "getblockcount"}') + else: + conn.post_raw('/', '{"method": "getblockcount"}') try: # The server should not respond to the second request until the first @@ -284,16 +288,30 @@ class HTTPBasicsTest (BitcoinTestFramework): # Use a separate http connection to generate a block self.generate(self.node, 1, sync_fun=self.no_op) - # Wait for two responses to be received + # Wait for responses to be received + if with_invalid_second_request: + OK = 1 + BAD = 1 + else: + OK = 2 + BAD = 0 res = b"" - while res.count(b"result") != 2: + while True: res += conn.recv_raw() + if res.count(b"HTTP/1.1 200") == OK and res.count(b"HTTP/1.1 400") == BAD: + break # waitforblockheight was responded to first, and then getblockcount # which includes the block added after the request was made chunks = res.split(b'"result":') assert chunks[1].startswith(b'{"hash":') - assert chunks[2].startswith(bytes(f'{tip_height + 1}', 'utf8')) + if with_invalid_second_request: + # The response to the in-flight first request is sent before the + # error generated by parsing the second one, even though the second + # request could have been rejected much earlier. + assert res.index(b"HTTP/1.1 200") < res.index(b"HTTP/1.1 400") + else: + assert chunks[2].startswith(bytes(f'{tip_height + 1}', 'utf8')) def check_chunked_transfer(self):