modified a copy of the check_pipelining functional test from interface_http.py into check_pipelined_malformed_disconnect and the issue showed up
<details>
<summary>the functional test i used
</summary>
diff --git a/test/functional/interface_http.py b/test/functional/interface_http.py
index 1350015258..3402a86c46 100755
--- a/test/functional/interface_http.py
+++ b/test/functional/interface_http.py
@@ -117,6 +117,7 @@ class HTTPBasicsTest (BitcoinTestFramework):
self.check_close_connection()
self.check_excessive_request_size()
self.check_pipelining()
+ self.check_pipelined_malformed_disconnect()
self.check_chunked_transfer()
self.check_idle_timeout()
self.check_server_busy_idle_timeout()
@@ -131,6 +132,50 @@ class HTTPBasicsTest (BitcoinTestFramework):
self.check_invalid_http_version()
self.check_whitespace_in_headers()
+ def check_pipelined_malformed_disconnect(self):
+ """
+ Regression check: a malformed pipelined request must not cause the
+ server to drop the reply to an earlier, still-in-flight valid request
+ on the same connection.
+ """
+
+ self.log.info("+ check malformed pipelined request does not drop a busy reply")
+ tip_height = self.node.getblockcount()
+ conn = BitcoinHTTPConnection(self.node)
+ conn.set_timeout(5)
+
+ # A: slow, valid request. blocks server-side until a new block comes back
+ conn.post_raw('/', f'{{"method": "waitforblockheight", "params": [{tip_height + 1}]}}')
+
+ # B: send valid HTTP request line/headers with an invalid header field pipelined
+ # just after A, while A is still busy so as to force an instant 400 parse error
+ malformed_b = (
+ b"POST / HTTP/1.1\r\n"
+ b"Host: 127.0.0.1\r\n"
+ b"Very-Very-Bad-Header-With-No-Colon\r\n\r\n"
+ )
+ conn.send_raw(malformed_b)
+
+ # right now, A has not yet been unblocked, so nothing should come back, otherwise that is a BUG
+ try:
+ early_data = conn.recv_raw()
+ assert False, f"+ server sent data prematurely while A was busy (NOT nice): {early_data!r}"
+ except TimeoutError:
+ pass
+
+ # unblock A
+ self.generate(self.node, 1, sync_fun=self.no_op)
+
+ # A's reply should still arrive. If the bug is present, the server
+ # disconnected the client back when B failed to parse (while A's worker
+ # was still onto sth), silently dropping A's queued reply -- recv_raw()
+ # would then time out or just return nothing instead
+ try:
+ res = conn.recv_raw()
+ except TimeoutError:
+ assert False, "A's reply was dropped: server disconnected while A was still busy handling B's parse failure"
+
+ assert b'"hash"' in res, f"expected waitforblockheight's reply, got: {res!r}"
+
def check_default_connection(self):
self.log.info("Checking default HTTP/1.1 connection persistence")
</details>
<details>
<summary>output i got
</summary>
Temporary test directory at /tmp/test_runner_₿_🏃_20260806_221242
Remaining jobs: [interface_http.py]
1/1 - interface_http.py failed, Duration: 2 s
stdout:
2026-08-06T19:12:43.015370Z TestFramework (INFO): PRNG seed is: 3727582087972046593
2026-08-06T19:12:43.066100Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20260806_221242/interface_http_0
2026-08-06T19:12:45.221845Z TestFramework (INFO): + check malformed pipelined request does not drop a busy reply
2026-08-06T19:12:45.273692Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
File "/home/hacked/Documents/btc/btc-core/my-btc-fork/test/functional/test_framework/test_framework.py", line 145, in main
self.run_test()
File "/home/hacked/Documents/btc/btc-core/my-btc-fork/build/test/functional/interface_http.py", line 120, in run_test
self.check_pipelined_malformed_disconnect()
File "/home/hacked/Documents/btc/btc-core/my-btc-fork/build/test/functional/interface_http.py", line 161, in check_pipelined_malformed_disconnect
assert False, f""+ server sent data prematurely while A was busy (NOT nice): {early_data!r}"
^^^^^
AssertionError: "+ server sent data prematurely while A was busy (NOT nice): b'HTTP/1.1 400 Bad Request\r\nDate: Thu, 06 Aug 2026 19:12:45 GMT\r\nContent-Length: 0\r\nContent-Type: text/html; charset=ISO-8859-1\r\n\r\n'
2026-08-06T19:12:45.327671Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
2026-08-06T19:12:45.328113Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20260806_221242/interface_http_0
2026-08-06T19:12:45.328342Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20260806_221242/interface_http_0/test_framework.log
2026-08-06T19:12:45.328713Z TestFramework (ERROR):
2026-08-06T19:12:45.329174Z TestFramework (ERROR): Hint: Call /home/hacked/Documents/btc/btc-core/my-btc-fork/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20260806_221242/interface_http_0' to consolidate all logs
...
</details>
--
the request processing does not seem strictly serial
rough idea for a fix:
how about if we checked if the server was still processing a request from the client at these spots:
- before parsing another request (in
HTTPServer::MaybeDispatchRequestsFromClient) and,
- before disconnecting a client while a request is still being handled (in
HTTPServer::DisconnectClients())
<details>
<summary>sth like this
</summary>
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 288a8a477e..b3b4c66a88 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -1010,6 +1010,11 @@ void HTTPServer::ThreadSocketHandler()
void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr<HTTPRemoteClient>& 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<HTTPRequest>(client);
}
@@ -1042,11 +1047,6 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr<HTTPRemot
return;
}
- // 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;
-
// Otherwise, if the request is ready, hand it to a worker.
if (client->m_req->GetState() == HTTPRequest::State::Complete) {
LogDebug(
@@ -1068,6 +1068,10 @@ void HTTPServer::DisconnectClients()
const auto now{Now<SteadySeconds>()};
size_t erased = std::erase_if(m_connected,
[&](auto& client) {
+ // Don't disconnect a client if the server is busy with its request in order to avoid premature disconnection
+ if (client->m_req_busy) {
+ return false;
+ }
// First check for idle timeout. We reset the timer when we send and receive data,
// but if the server is busy handling a request we should ignore the timeout until
// the reply is sent. If we did erase the shared_ptr<HTTPRemoteClient> reference in m_connected
</details>
thoughts ?
