#36204 http: disconnect clients that never finish a request

ReviewabilityReviewsAgreementSize
#36204 janb84 http: disconnect clients that never finish a requestNeeds rebase0Mild67 + 183 tests
  • Disconnect HTTP clients that trickle bytes without completing requests, preventing connection slot exhaustion
  • Ensure legitimate RPC clients can always connect even if slow clients occupy sockets
  • Needs mechanical rebase due to conflicts with master, but the logic is ready to review
  • Concept approval because it mitigates slow-loris attacks and invalid automated bug reports (pinheadmz)
  • Concept approval and praised test cleanup (hodlinator)
  • Unaddressed objection: test assertion allows duration under RPCSERVERTIMEOUT, masking premature closes (hodlinator)
  • 67 lines added or modified outside tests
  • 183 lines added or modified in tests
  • 12 lines removed in total
  • 4 files, 2 commits
RPC / REST / ZMQ#14 of 55
P3 · DoS protection
  • P3 because it improves robustness against DoS rather than fixing an urgent remote vulnerability
  • RPC binds to localhost by default, limiting the exposure to local exhaustion
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds a completion deadline to the internal HTTP server to disconnect clients that take too long to finish sending a request. The deadline timer starts on the first byte of a request, expires after the duration set by -rpcservertimeout, and is not reset by subsequent reads. The deadline is cleared once request parsing completes, deferred while worker threads process a request, restarted while response bytes are sent, and disabled when -rpcservertimeout is zero.

Problem: The existing -rpcservertimeout only measures inactivity, resetting every time bytes are read from a socket. A client trickling bytes without ever sending a complete HTTP request can hold a connection slot indefinitely without triggering authentication. A small number of such connections can exhaust all available slots (default 16, or -rpcmaxconnections), preventing legitimate RPC clients from connecting.

Evidence of importance:

  • PR description explains the vulnerability: "The -rpcservertimeout timer only measures inactivity. Every read from a client resets it, so a client that keeps sending bytes without ever completing a request can hold the connection for ever. Connection slots are capped by default 16 or by -rpcmaxconnections. Once those slots are taken, the server stops accepting new connections."
  • A new functional test (check_trickled_request_timeout in test/functional/interface_http.py) demonstrates that with -rpcmaxconnections=1, trickling a request byte-by-byte blocks subsequent RPC calls until the new request deadline closes the stalled socket.
  • Author notes the scope and impact: "The impact is low because RPC port binds to localhost by default, so this is a robustness fix rather than a remotely exploitable security issue."

Reviewability

Ready · Needs rebase

Conflicts with master; review the logic now, the rebase is mechanical.

Waiting on (from the thread read):

  • author: response to hodlinator's objection regarding the test assertion duration 2026-09-18

Agreement

Mild

objection that needs an answer has none yet (hodlinator)

Thread notes: Added Concept ACK from pinheadmz; the PR remains waiting on the author to respond to hodlinator's objection in rc:4046818492.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
hodlinator (member)suggestion, approachOrder condition checks so !m_req_busy comes first as the cheapest checkaccepted · reviewer agreed with author's rationaleansweredyes2026-09-11: 'nanonit: Feels like `!m_req_busy` would be the cheapest check and be the first one (same for `is_idle`).'
Status from: 2026-09-17: hodlinator: 'Agree on this.'
hodlinator (member)suggestion, approachConsider adopting mockable steady clock and FakeSteadyClock from #36159 in httpserver_testsaccepted · reviewer left it to author's preference after author asked if #36159 must land firstansweredyes2026-09-11: 'Not sure time drift is an issue with these tests, but #36159's 9afe15897e8dbee9429722d0db7859d895919bf4 switches to the mockable steady clock and freezes time in 784ac4fb9b4b3b9a5614e4ea54c108e1246ce5'
Status from: 2026-09-17: hodlinator: 'I'm happy however the changes get in.'
hodlinator (member)suggestion, approachSend the first byte in the thread instead of sending it before thread launchfixed · fix pushed · author confirmed and reworked the testansweredyes2026-09-11: 'Seems to work fine without send the first byte here and instead sending it in the thread?'
Status from: 2026-09-14: janb84: 'Yes and I reworked the test, thanks'
hodlinator (member)question, approachWhether tricklers need to be cleaned up if the test failsanswered · author confirmed test cleanup is handledansweredyes2026-09-11: 'If the test fails for some reason we don't need to clean up the `tricklers` right? See 659671ac3db7e5157178efe4d9f8bce7d92ea237'
Status from: 2026-09-14: janb84: 'correct, thanks, see above'
hodlinator (member)objection, correctnessTest assertion allowing duration less than RPCSERVERTIMEOUT could allow premature socket reclamation to pass unnoticedopenansweredno2026-09-18: 'Why would we allow *less* than `RPCSERVERTIMEOUT` (which is only `2`)?'

Support:

  • hodlinator (member) (Concept ACK, 2026-09-11): (no reason given) [not substantive]
  • hodlinator (member) (2026-09-18) [read]: Nice how you cleaned up the test, makes it easy to grasp.
  • pinheadmz (member) (Concept ACK, 2026-10-30): this mitigates slow-loris and perhaps most importantly, mitigates the LLM "findings" reports we keep getting

Participants:

  • hodlinator (member, objection, 13 statements 2026-09-11 to 2026-09-18): Concept ACK, but objected to test assertion allowing socket reclamation duration less than RPCSERVERTIMEOUT
  • pinheadmz (member, support, 1 statements 2026-10-30 to 2026-10-30): Concept ACK, noting it mitigates slow-loris and LLM report churn

Files

193 lines under test/bench/ci.

  • src/test/httpserver_tests.cpp +120/-10
  • test/functional/interface_http.py +63/-0
  • src/httpserver.cpp +42/-2
  • src/httpserver.h +25/-0

Card

The short text the ranking pass compares across PRs.

Adds a completion deadline timer to the HTTP server to disconnect clients that do not finish sending a complete request within the timeout window. Currently, -rpcservertimeout only tracks idle time between reads, so a slowloris-style client trickling bytes can hold a connection slot indefinitely; occupying all connection slots (default 16) blocks legitimate RPC clients before authentication is checked. The author notes that because the RPC port binds to localhost by default, this serves primarily as a local robustness fix against connection exhaustion.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-31T01:17, confidence high, input hash c233da5075d2a9e7
thread read through 2026-10-30 (23 statements), head 99e880356b
labels: RPC/REST/ZMQ, Needs rebase

Categories

RPC / REST / ZMQ

P3 · DoS protection

Prevents slow-loris connection slot exhaustion by disconnecting HTTP clients that fail to complete requests within -rpcservertimeout. As noted by the author, impact is moderate because RPC binds to localhost by default, making this a deferred robustness fix rather than an urgent remote vulnerability.

Membership: Matches RPC/REST/ZMQ label and modifies HTTP server connection timeout logic in src/httpserver.cpp and src/httpserver.h.

Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0