highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-12 05:32:22 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#35735: Add state to HTTPRequest

Browse Source 
9954aa7728 http: don't parse any new requests from a client if m_req_busy = true (Matthew Zipkin)
c7db3ae1f9 test: cover HTTPRequest state machine (Matthew Zipkin)
90676e24ad Add state to HTTPRequest to avoid duplicate work over I/O cycles (Matthew Zipkin)
507e528e84 http: reuse HTTPHeaders to parse chunked trailer (Matthew Zipkin)
902d8908c9 http: only read one HTTPRequest at a time per client (Matthew Zipkin)

Pull request description:

  This PR reduces the memory consumption of the HTTP Server when reading data from connected clients, and improves performance especially when requests are large (i.e. requiring multiple TCP packets).

  In https://github.com/bitcoin/bitcoin/pull/35182 the server copies as much data as it can from the socket into application memory, and then tries to parse as many complete HTTP requests as possible from that data. If a request is discovered to be incomplete, the in-progress request is abandoned. The server tries again on the next I/O cycle to read the same data from the buffer, duplicating work as many times as it takes before the client finishes sending the request (or times out).

  This PR implements two improvements to this:
  1. Only parse one request at a time from the receive buffer. The server processes requests from each client in series anyway.
  2. Add state to `HTTPRequest` so it can be filled with data from the receive buffer over multiple I/O loop iterations without losing progress.

  If a client sends large or multiple requests, that data will sit in the kernel's socket buffer instead of the application memory. Eventually the socket buffer will fill up and TCP backpressure will kick in, dropping the TCP window to 0 and blocking the client from sending any more.

  A state machine for `HTTPRemoteClient` was [discussed previously](https://github.com/bitcoin/bitcoin/pull/35182#pullrequestreview-4322490068) to control resource consumption. Another nice benefit of this model (for a follow-up PR) will be to insert the RPC authentication check after reading 8kB-limited headers but before the 32MB-limited request body.

ACKs for top commit:
  winterrdog:
    re-ACK 9954aa7728
  janb84:
    re ACK 9954aa7728
  frankomosh:
    ACK 9954aa7728.
  fjahr:
    ACK 9954aa7728

Tree-SHA512: b7c913114283fbf1f360b40f6c65a01390a26731bf3b166f460ec260f9206f25d738b3a06887bfa839911c1c6aaf634448181da47a752a9a881aebd907e44868
This commit is contained in:
merge-script
2026-08-17 10:19:34 +01:00
parent e0992599a6 9954aa7728
commit 4800cb7aea
4 changed files with 617 additions and 178 deletions
Download Patch File Download Diff File Expand all files Collapse all files

132
test/functional/interface_http.py
View File

@@ -9,6 +9,8 @@ from test_framework.netutil import NETWORK_ERRORS
from test_framework.util import assert_equal, str_to_b64str
import http.client
import socket
import threading
import time
import urllib.parse
@@ -114,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()
@@ -222,31 +225,45 @@ class HTTPBasicsTest (BitcoinTestFramework):
assert_equal(response4.status, http.client.OK)
conn = BitcoinHTTPConnection(self.node)
# Split off the send into a background thread. When the server detects
# the excessive size it will stop reading from the socket, but the client
# will continue trying to write until the backpressure eventually
# drops the TCP window size to 0. While the send operation is blocking until
# it times out, we can still receive the server's response in the foreground.
def send_excessive_body(self, conn):
try:
# Excessive body size is invalid
conn.post_raw('/', f'{{"jsonrpc": "2.0", "id": "0", "method": "submitblock", "params": ["{"0" * bytes_above_limit}"]}}')
# On some platforms (e.g. Windows) the whole request may be
# accepted into the OS send buffer before the server disconnects.
# It's ok to allow that, the server-side behavior is asserted in
# the foreground thread via the 413 response.
self.log.info("Client finished sending request before connection was terminated")
except NETWORK_ERRORS:
self.log.info("Client did not finish sending request before connection was terminated")
send_thread = threading.Thread(target=send_excessive_body, args=(self, conn))
send_thread.start()
response5 = conn.recv_raw().decode()
assert "413 Content too large" in response5
try:
# Excessive body size is invalid
conn.post_raw('/', f'{{"jsonrpc": "2.0", "id": "0", "method": "submitblock", "params": ["{"0" * bytes_above_limit}"]}}')
self.log.info("Client finished sending request before connection was terminated")
except NETWORK_ERRORS:
self.log.info("Client did not finish sending request before connection was terminated")
# The server will send a 413 response and disconnect but due to a race
# condition, the python client may or may not read the response before
# detecting the broken socket (which it may still be trying to write to).
try:
response5 = conn.conn.getresponse()
assert_equal(response5.status, http.client.REQUEST_ENTITY_TOO_LARGE)
self.log.info(f"Client got expected response status {response5.status}")
assert conn.sock_closed()
except NETWORK_ERRORS:
self.log.info("Client did not read response before disconnecting")
conn.conn.sock.shutdown(socket.SHUT_RDWR)
self.log.info("Send thread force-closed by test framework")
except OSError:
self.log.info("Send thread was already closed by RST from server")
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)
@@ -254,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
@@ -268,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):
@@ -314,27 +348,41 @@ class HTTPBasicsTest (BitcoinTestFramework):
b'3' * 10000000,
b'"]}'
]
try:
conn.conn.request(
method='POST',
url='/',
body=iter(body_chunked),
headers=headers_chunked,
encode_chunked=True)
self.log.info("Client finished sending request before connection was terminated")
except NETWORK_ERRORS:
self.log.info("Client did not finish sending request before connection was terminated")
# The server will send a 413 response and disconnect but due to a race
# condition, the python client may or may not read the response before
# detecting the broken socket (which it may still be trying to write to).
# Split off the send into a background thread. When the server detects
# the excessive size it will stop reading from the socket, but the client
# will continue trying to write until the backpressure eventually
# drops the TCP window size to 0. While the send operation is blocking until
# it times out, we can still receive the server's response in the foreground.
def send_excessive_chunked(self, conn):
try:
conn.conn.request(
method='POST',
url='/',
body=iter(body_chunked),
headers=headers_chunked,
encode_chunked=True)
# On some platforms (e.g. Windows) the whole request may be
# accepted into the OS send buffer before the server disconnects.
# It's ok to allow that, the server-side behavior is asserted in
# the foreground thread via the 413 response.
self.log.info("Client finished sending request before connection was terminated")
except NETWORK_ERRORS:
self.log.info("Client did not finish sending request before connection was terminated")
send_thread = threading.Thread(target=send_excessive_chunked, args=(self, conn))
send_thread.start()
response2 = conn.recv_raw().decode()
assert "413 Content too large" in response2
try:
response2 = conn.conn.getresponse()
assert_equal(response2.status, http.client.REQUEST_ENTITY_TOO_LARGE)
self.log.info(f"Client got expected response status {response2.status}")
assert conn.sock_closed()
except NETWORK_ERRORS:
self.log.info("Client did not read response before disconnecting")
conn.conn.sock.shutdown(socket.SHUT_RDWR)
self.log.info("Send thread force-closed by test framework")
except OSError:
self.log.info("Send thread was already closed by RST from server")
send_thread.join()
def check_idle_timeout(self):