From da7d2e389142d228fbfe24c05dd8b83bb9141d7e Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sat, 29 Aug 2026 15:25:00 +0200 Subject: [PATCH 01/13] http2: stream the request body to the application The whole body was buffered before dispatch and window credit was returned as frames arrived, so a client could grow a worker without the application being called. Dispatch on headers, let the application pull the body off the stream as it reads, and return window credit only for what it consumed. --- docs/content/2026-news.md | 17 + docs/content/guides/http2.md | 1 + gunicorn/asgi/protocol.py | 102 +++-- gunicorn/http2/async_connection.py | 293 +++++++++---- gunicorn/http2/connection.py | 147 +++++-- gunicorn/http2/request.py | 164 +++++-- gunicorn/http2/stream.py | 69 ++- gunicorn/workers/base_async.py | 5 + gunicorn/workers/gthread.py | 5 + tests/test_http2_async_connection.py | 399 +++++++++++++++-- tests/test_http2_connection.py | 453 +++++++++++++++++++- tests/test_http2_h2c.py | 616 +++++++++++++++++++++++++++ tests/test_http2_request.py | 109 ++++- tests/test_http2_stream.py | 82 +++- 14 files changed, 2197 insertions(+), 265 deletions(-) diff --git a/docs/content/2026-news.md b/docs/content/2026-news.md index c4e3fc96cb..ab676adf05 100644 --- a/docs/content/2026-news.md +++ b/docs/content/2026-news.md @@ -1,6 +1,23 @@ # Changelog - 2026 +## Unreleased + +### Bug Fixes + +- **HTTP/2 request bodies were buffered in full before dispatch**: DATA + frames were kept in two buffers, copied twice more when the request was + built, and flow control credit went back to the peer as each frame arrived, + so a client that never sent END_STREAM could grow a worker without the + application being called. A request is now dispatched on its headers and + `wsgi.input` (or ASGI `receive()`) pulls the body from the stream as it + arrives, returning window credit only for what the application has read. + A peer can have no more than the receive window in flight per stream. A + body the application leaves unread is cut off with `RST_STREAM(NO_ERROR)` + once the response is sent. On the ASGI worker, streams on a connection are + served concurrently. Only listeners with `h2` in `http_protocols` were + affected. + ## 26.2.0 - 2026-08-24 ### New Features diff --git a/docs/content/guides/http2.md b/docs/content/guides/http2.md index 3930341fc8..55ebdf5544 100644 --- a/docs/content/guides/http2.md +++ b/docs/content/guides/http2.md @@ -775,6 +775,7 @@ protections against known vulnerabilities. |--------|------------|---------| | Stream Multiplexing Abuse | Limit concurrent streams | `http2_max_concurrent_streams` (default: 100) | | HPACK Bomb | Header size limits | `http2_max_header_list_size` (default: 65536) | +| Unbounded Upload | Body streams to the app; window credit returns as it reads | `http2_initial_window_size` (default: 65535) | | Large Frame Attack | Frame size limits | `http2_max_frame_size` (validated: 16384-16777215) | | Resource Exhaustion | Flow control windows | `http2_initial_window_size` (default: 65535) | | Slow Read (Slowloris) | Connection timeouts | `timeout` and `keepalive` settings | diff --git a/gunicorn/asgi/protocol.py b/gunicorn/asgi/protocol.py index 18b979f365..af7b5640f8 100644 --- a/gunicorn/asgi/protocol.py +++ b/gunicorn/asgi/protocol.py @@ -22,6 +22,7 @@ from gunicorn.asgi.uwsgi import AsyncUWSGIRequest from gunicorn.http.errors import NoMoreData from gunicorn.http2 import negotiation +from gunicorn.http2.errors import HTTP2ErrorCode, HTTP2StreamError from gunicorn.uwsgi.errors import UWSGIParseException @@ -357,6 +358,8 @@ def __init__(self, worker): self.cfg = worker.cfg self.log = worker.log self.app = worker.asgi + # Tasks serving HTTP/2 streams on this connection + self._h2_tasks = set() self.transport = None self.reader = None # Only used for HTTP/2 @@ -1682,12 +1685,19 @@ async def _handle_http2_connection(self, transport, ssl_object, upgrade=None): upgraded = await h2_conn.initiate_upgrade( settings, http1_req, body) self._h2_conn = h2_conn - await self._serve_http2_request( - upgraded, h2_conn, sockname, peername) + self._start_http2_stream(upgraded, h2_conn, sockname, peername) self.worker.nr += 1 - # Main loop - receive and handle requests - while not h2_conn.is_closed and self.worker.alive: + # Main loop: each request is served by its own task as soon + # as its headers are in, while this loop keeps reading frames + # so bodies stream to the tasks and window credit flows back. + # Once the worker stops accepting, or the peer sends a graceful + # GOAWAY, the loop keeps reading until the streams already in + # flight have finished, so their bodies still arrive; streams + # opened meanwhile are refused. + while not h2_conn.is_closed: + if (not self.worker.alive or h2_conn.draining) and not self._h2_tasks: + break try: requests = await h2_conn.receive_data(timeout=1.0) except asyncio.TimeoutError: @@ -1697,23 +1707,24 @@ async def _handle_http2_connection(self, transport, ssl_object, upgrade=None): break for req in requests: - await self._serve_http2_request( - req, h2_conn, sockname, peername) - - # Increment worker request count - self.worker.nr += len(requests) - - # Check max_requests - if self.worker.nr >= self.worker.max_requests: - self.log.info("Autorestarting worker after current request.") - self.worker.alive = False - break + stream_id = req.stream.stream_id + if not self.worker.alive: + await h2_conn.reset_stream( + stream_id, HTTP2ErrorCode.REFUSED_STREAM) + h2_conn.cleanup_stream(stream_id) + continue + self._start_http2_stream(req, h2_conn, sockname, peername) + self.worker.nr += 1 + if self.worker.nr >= self.worker.max_requests: + self.log.info("Autorestarting worker after current request.") + self.worker.alive = False except asyncio.CancelledError: pass except Exception as e: self.log.exception("HTTP/2 connection error: %s", e) finally: + await self._cancel_http2_streams() if hasattr(self, '_h2_conn'): try: await self._h2_conn.close() @@ -1721,6 +1732,22 @@ async def _handle_http2_connection(self, transport, ssl_object, upgrade=None): pass self._close_transport() + def _start_http2_stream(self, req, h2_conn, sockname, peername): + """Serve one stream in its own task.""" + task = self.worker.loop.create_task( + self._serve_http2_request(req, h2_conn, sockname, peername)) + self._h2_tasks.add(task) + task.add_done_callback(self._h2_tasks.discard) + + async def _cancel_http2_streams(self): + """Stop stream tasks still running when the connection ends.""" + tasks = list(self._h2_tasks) + if not tasks: + return + for task in tasks: + task.cancel() + await asyncio.gather(*tasks, return_exceptions=True) + async def _serve_http2_request(self, req, h2_conn, sockname, peername): """Run one HTTP/2 request, answering 500 rather than dropping it.""" try: @@ -1792,7 +1819,7 @@ async def receive(): stream.read_body_chunk(), timeout=30.0 ) - except asyncio.TimeoutError: + except (asyncio.TimeoutError, HTTP2StreamError): return {"type": "http.disconnect"} if chunk is None: @@ -1803,19 +1830,28 @@ async def receive(): "more_body": False, } - if stream._body_complete: + # END_STREAM may have landed with several frames still queued. + done = stream._body_complete and not stream._body_chunks + if done: body_received = True return { "type": "http.request", "body": chunk, - "more_body": not stream._body_complete, + "more_body": not done, } - async def send(message): + # Set once a send loses to the peer's RST_STREAM: the response is + # over, later sends are inert, and none of it is an app failure. + peer_gone = False + + async def send(message): # pylint: disable=too-many-return-statements nonlocal response_started, response_complete, headers_sent nonlocal response_status, response_headers, response_sent, exc_to_raise - nonlocal omits_body, omits_body_warned + nonlocal omits_body, omits_body_warned, peer_gone + + if peer_gone: + return msg_type = message["type"] @@ -1868,24 +1904,24 @@ async def send(message): response_hdrs.extend(headers) # Send headers without end_stream since we have body - stream = h2_conn.streams.get(stream_id) - if stream is None: - exc_to_raise = RuntimeError("Stream closed") + if not await h2_conn.send_response_headers( + stream_id, response_hdrs, end_stream=False): + peer_gone = response_complete = True return - h2_conn.h2_conn.send_headers(stream_id, response_hdrs, end_stream=False) - stream.send_headers(response_hdrs, end_stream=False) - await h2_conn._send_pending_data() headers_sent = True # Stream body immediately if body: - await h2_conn.send_data(stream_id, body, end_stream=not more_body) + if not await h2_conn.send_data(stream_id, body, end_stream=not more_body): + peer_gone = response_complete = True + return response_sent += len(body) if not more_body: if not body: # Empty final chunk - send end_stream - await h2_conn.send_data(stream_id, b"", end_stream=True) + if not await h2_conn.send_data(stream_id, b"", end_stream=True): + peer_gone = True response_complete = True elif msg_type == "http.response.trailers": @@ -1894,7 +1930,8 @@ async def send(message): return trailer_headers = message.get("headers", []) trailers = self._convert_h2_headers(trailer_headers) - await h2_conn.send_trailers(stream_id, trailers) + if not await h2_conn.send_trailers(stream_id, trailers): + peer_gone = True # Only build environ for logging if access logging is enabled access_log_enabled = self.log.access_log_enabled @@ -1918,11 +1955,8 @@ async def send(message): headers = self._convert_h2_headers(response_headers) response_hdrs = [(':status', str(response_status))] response_hdrs.extend(headers) - stream = h2_conn.streams.get(stream_id) - if stream: - h2_conn.h2_conn.send_headers(stream_id, response_hdrs, end_stream=True) - stream.send_headers(response_hdrs, end_stream=True) - await h2_conn._send_pending_data() + await h2_conn.send_response_headers( + stream_id, response_hdrs, end_stream=True) except Exception: self.log.exception("Error in ASGI application") diff --git a/gunicorn/http2/async_connection.py b/gunicorn/http2/async_connection.py index efaf52b981..6ad1f326ed 100644 --- a/gunicorn/http2/async_connection.py +++ b/gunicorn/http2/async_connection.py @@ -17,7 +17,7 @@ HTTP2Error, HTTP2ProtocolError, HTTP2ConnectionError, HTTP2NotAvailable, HTTP2ErrorCode, ) -from .stream import HTTP2Stream +from .stream import HTTP2Stream, StreamState from .request import HTTP2Request @@ -81,6 +81,13 @@ def __init__(self, cfg, reader, writer, client_addr): # They have left the h2 state machine already, so they are held here # for the main receive loop rather than discarded. self._deferred_events = collections.deque() + # Set by the receive loop whenever the peer widens a window, so + # a response task can wait for credit without touching the + # reader the loop owns. + self._window_event = None + self._write_lock = None + # Peer sent a graceful GOAWAY: finish open streams, take no new ones + self.draining = False # Queue of completed requests for the worker self._request_queue = asyncio.Queue() @@ -167,6 +174,8 @@ async def initiate_upgrade(self, settings_header, http1_req, body=b""): stream.receive_headers(pseudo + regular, end_stream=not body) if body: stream.receive_data(body, end_stream=True) + # That body came over HTTP/1, so no window credit is owed. + stream.acked_size = stream.body_size self.streams[1] = stream return HTTP2Request(stream, self.cfg, self.client_addr) @@ -191,6 +200,10 @@ async def receive_data(self, timeout=None): ) else: data = await self.reader.read(self.READ_BUFFER_SIZE) + except asyncio.TimeoutError: + # A subclass of OSError since Python 3.11; the caller's + # timeout is not a socket failure. + raise except (OSError, IOError) as e: raise HTTP2ConnectionError(f"Socket read error: {e}") @@ -233,6 +246,7 @@ async def receive_data(self, timeout=None): # Process events, oldest first: anything set aside during a # flow-control wait arrived before this batch. completed_requests = [] + was_draining = self.draining if self._deferred_events: events = list(self._deferred_events) + list(events) self._deferred_events.clear() @@ -241,6 +255,17 @@ async def receive_data(self, timeout=None): if request is not None: completed_requests.append(request) + if was_draining: + # Streams opened after the peer's GOAWAY are refused. + for request in completed_requests: + stream_id = request.stream.stream_id + self.streams.pop(stream_id, None) + try: + self.h2_conn.reset_stream(stream_id, error_code=HTTP2ErrorCode.REFUSED_STREAM) + except _h2_exceptions.ProtocolError: + pass + completed_requests = [] + # Send any pending data (WINDOW_UPDATE, etc.) await self._send_pending_data() @@ -268,7 +293,7 @@ def _handle_event(self, event): self._handle_stream_reset(event) elif isinstance(event, _h2_events.WindowUpdated): - pass # Flow control update, handled by h2 + self._signal_window() elif isinstance(event, _h2_events.PriorityUpdated): self._handle_priority_updated(event) @@ -276,8 +301,12 @@ def _handle_event(self, event): elif isinstance(event, _h2_events.SettingsAcknowledged): pass # Settings ACK received + elif isinstance(event, _h2_events.RemoteSettingsChanged): + self._signal_window() + elif isinstance(event, _h2_events.ConnectionTerminated): self._handle_connection_terminated(event) + self._signal_window() elif isinstance(event, _h2_events.TrailersReceived): return self._handle_trailers_received(event) @@ -296,34 +325,58 @@ def _handle_request_received(self, event): # Process headers stream.receive_headers(headers, end_stream=False) + # Dispatch on headers: the body streams in behind the request. + return HTTP2Request(stream, self.cfg, self.client_addr) + def _handle_data_received(self, event): - """Handle DataReceived event.""" - stream_id = event.stream_id - data = event.data + """Handle DataReceived event. - stream = self.streams.get(stream_id) + The payload is held on the stream until the application takes it; + window credit goes back through acknowledge_data() at that point. + """ + stream = self.streams.get(event.stream_id) if stream is None: return None - stream.receive_data(data, end_stream=False) + stream.receive_data(event.data, end_stream=False) - # Increment flow control windows (only if data received) - if len(data) > 0: + # Connection-level credit goes straight back so a stream that is + # waiting its turn can never starve the one being served; the + # stream-level window is credited only as the body is consumed. + # Padding is never buffered, so its credit goes back now too. + length = event.flow_controlled_length + padding = length - len(event.data) + try: + if length: + self.h2_conn.increment_flow_control_window(length, stream_id=None) + if padding: + self.h2_conn.increment_flow_control_window(padding, stream_id=event.stream_id) + except (ValueError, _h2_exceptions.ProtocolError): + pass + return None + + def acknowledge_data(self, stream_id, size): + """Credit ``size`` consumed bytes back to the peer. + + Called by the stream as the application takes body data. The + WINDOW_UPDATE frames are written without waiting on drain: they + are small and the reader must not be blocked behind them. + """ + if size <= 0 or self._closed: + return + try: + self.h2_conn.increment_flow_control_window(size, stream_id=stream_id) + except (ValueError, _h2_exceptions.ProtocolError): + return + self._write_pending_nowait() + + def _write_pending_nowait(self): + data = self.h2_conn.data_to_send() + if data: try: - # Update stream-level window - self.h2_conn.increment_flow_control_window(len(data), stream_id=stream_id) - # Update connection-level window - self.h2_conn.increment_flow_control_window(len(data), stream_id=None) - except (ValueError, _h2_exceptions.FlowControlError): - # Window overflow - prepare GOAWAY with FLOW_CONTROL_ERROR - # (will be sent by receive_data's _send_pending_data call) + self.writer.write(data) + except (OSError, IOError): self._closed = True - try: - self.h2_conn.close_connection(error_code=HTTP2ErrorCode.FLOW_CONTROL_ERROR) - except Exception: - pass - - return None def _handle_stream_ended(self, event): """Handle StreamEnded event.""" @@ -333,15 +386,12 @@ def _handle_stream_ended(self, event): if stream is None: return None - # Mark stream as request complete and body complete so the - # receive() closure's _body_complete guard fires, preventing - # the fast path from re-reading already-consumed data from BytesIO. + # The request went out on its headers; only mark the body complete. stream.request_complete = True stream._body_complete = True if stream._body_event: stream._body_event.set() - - return HTTP2Request(stream, self.cfg, self.client_addr) + return None def _handle_stream_reset(self, event): """Handle StreamReset event.""" @@ -352,7 +402,22 @@ def _handle_stream_reset(self, event): stream.reset(event.error_code) def _handle_connection_terminated(self, event): - """Handle ConnectionTerminated event.""" + """Handle ConnectionTerminated event (GOAWAY frame). + + A graceful GOAWAY (NO_ERROR) only forbids new streams; the ones + already established finish (RFC 9113 section 6.8). h2 closes its + connection state on receipt and would refuse to send anything, + so its state is put back to open while those streams drain, and + the connection closes once they are done. Any other error code + closes at once. + + Args: + event: ConnectionTerminated event + """ + if event.error_code == HTTP2ErrorCode.NO_ERROR and self.streams: + self.draining = True + self.h2_conn.state_machine.state = _h2.ConnectionState.SERVER_OPEN + return self._closed = True def _handle_trailers_received(self, event): @@ -364,7 +429,9 @@ def _handle_trailers_received(self, event): return None stream.receive_trailers(event.headers) - return HTTP2Request(stream, self.cfg, self.client_addr) + if stream._body_event: + stream._body_event.set() + return None def _handle_priority_updated(self, event): """Handle PriorityUpdated event (PRIORITY frame). @@ -409,8 +476,30 @@ async def send_informational(self, stream_id, status, headers): response_headers.append((name.lower(), str(value))) # Send headers with end_stream=False (informational, more to follow) - self.h2_conn.send_headers(stream_id, response_headers, end_stream=False) - await self._send_pending_data() + await self._send(lambda: self.h2_conn.send_headers( + stream_id, response_headers, end_stream=False)) + + async def send_response_headers(self, stream_id, headers, end_stream=False): + """Send response HEADERS already carrying ``:status``. + + Returns: + bool: True if sent, False if the stream is gone + """ + stream = self.streams.get(stream_id) + if stream is None: + return False + + def queue(): + self.h2_conn.send_headers(stream_id, headers, end_stream=end_stream) + stream.send_headers(headers, end_stream=end_stream) + + try: + await self._send(queue) + except _h2_exceptions.StreamClosedError: + stream.close() + self.cleanup_stream(stream_id) + return False + return True async def send_response(self, stream_id, status, headers, body=None): """Send a response on a stream. @@ -438,9 +527,11 @@ async def send_response(self, stream_id, status, headers, body=None): try: # Send headers - self.h2_conn.send_headers(stream_id, response_headers, end_stream=end_stream) - stream.send_headers(response_headers, end_stream=end_stream) - await self._send_pending_data() + def queue(): + self.h2_conn.send_headers(stream_id, response_headers, end_stream=end_stream) + stream.send_headers(response_headers, end_stream=end_stream) + + await self._send(queue) # Send body if present if body and len(body) > 0: @@ -452,49 +543,38 @@ async def send_response(self, stream_id, status, headers, body=None): self.cleanup_stream(stream_id) return False + def _signal_window(self): + """Wake response tasks waiting for send credit.""" + if self._window_event is not None: + self._window_event.set() + async def _wait_for_flow_control_window(self, stream_id): """Wait for flow control window to become positive. + The receive loop owns the reader; it processes the peer's + WINDOW_UPDATE and SETTINGS frames and signals here. Waiting on + that signal instead of reading keeps two tasks off one reader. + Returns: int: Available window size, or -1 if waiting failed """ + if self._window_event is None: + self._window_event = asyncio.Event() max_wait_attempts = 50 # ~5 seconds at 100ms per attempt for _ in range(max_wait_attempts): available = self.h2_conn.local_flow_control_window(stream_id) if available > 0: return available - - # Read more data from connection (may receive WINDOW_UPDATE) + if self._closed: + return -1 + stream = self.streams.get(stream_id) + if stream is not None and stream.state is StreamState.CLOSED: + return -1 + self._window_event.clear() try: - incoming = await asyncio.wait_for( - self.reader.read(self.READ_BUFFER_SIZE), - timeout=0.1 - ) - if incoming: - events = self.h2_conn.receive_data(incoming) - # Process events but don't create new requests - for event in events: - if isinstance(event, _h2_events.StreamReset): - if event.stream_id == stream_id: - return -1 - elif isinstance(event, _h2_events.ConnectionTerminated): - self._closed = True - return -1 - else: - # Anything else arriving alongside the - # WINDOW_UPDATE belongs to the main loop. It has - # already left the h2 state machine, so dropping - # it here loses a request or its body for good. - self._deferred_events.append(event) - await self._send_pending_data() - else: - # Connection closed - self._closed = True - return -1 + await asyncio.wait_for(self._window_event.wait(), timeout=0.1) except asyncio.TimeoutError: continue - except _h2_exceptions.ProtocolError: - return -1 return self.h2_conn.local_flow_control_window(stream_id) @@ -516,25 +596,27 @@ async def send_data(self, stream_id, data, end_stream=False): data_to_send = data try: while data_to_send: - available = self.h2_conn.local_flow_control_window(stream_id) - chunk_size = min(available, self.max_frame_size, len(data_to_send)) - + # The window is read and the frame queued under the lock, + # so another stream cannot spend the credit in between. + async with self._lock(): + available = self.h2_conn.local_flow_control_window(stream_id) + chunk_size = min(available, self.max_frame_size, len(data_to_send)) + if chunk_size > 0: + chunk = data_to_send[:chunk_size] + data_to_send = data_to_send[chunk_size:] + is_final = end_stream and len(data_to_send) == 0 + self.h2_conn.send_data(stream_id, chunk, end_stream=is_final) + # Bookkeeping goes with the frame, before the + # drain: a reset processed meanwhile closes the + # stream and must not turn into a send error. + stream.send_data(chunk, end_stream=is_final) + await self._flush_locked() if chunk_size <= 0: # Wait for WINDOW_UPDATE per RFC 7540 Section 6.9.2 - await self._send_pending_data() available = await self._wait_for_flow_control_window(stream_id) if available <= 0: return False - chunk_size = min(available, self.max_frame_size, len(data_to_send)) - chunk = data_to_send[:chunk_size] - data_to_send = data_to_send[chunk_size:] - is_final = end_stream and len(data_to_send) == 0 - - self.h2_conn.send_data(stream_id, chunk, end_stream=is_final) - await self._send_pending_data() - - stream.send_data(data, end_stream=end_stream) return True except (_h2_exceptions.StreamClosedError, _h2_exceptions.FlowControlError): stream.close() @@ -575,9 +657,11 @@ async def send_trailers(self, stream_id, trailers): try: # Send trailers with end_stream=True - self.h2_conn.send_headers(stream_id, trailer_headers, end_stream=True) - stream.send_trailers(trailer_headers) - await self._send_pending_data() + def queue(): + self.h2_conn.send_headers(stream_id, trailer_headers, end_stream=True) + stream.send_trailers(trailer_headers) + + await self._send(queue) return True except _h2_exceptions.StreamClosedError: # Stream was reset by client - clean up gracefully @@ -600,8 +684,8 @@ async def reset_stream(self, stream_id, error_code=0x8): if stream is not None: stream.reset(error_code) - self.h2_conn.reset_stream(stream_id, error_code=error_code) - await self._send_pending_data() + await self._send(lambda: self.h2_conn.reset_stream( + stream_id, error_code=error_code)) async def close(self, error_code=0x0, last_stream_id=None): """Close the connection gracefully with GOAWAY.""" @@ -614,19 +698,40 @@ async def close(self, error_code=0x0, last_stream_id=None): last_stream_id = max(self.streams.keys()) if self.streams else 0 try: - self.h2_conn.close_connection(error_code=error_code) - await self._send_pending_data() + await self._send(lambda: self.h2_conn.close_connection(error_code=error_code)) except Exception: pass + # Not awaited: the writer's protocol is a stand-in that never + # sees connection_lost, so wait_closed() would never return. try: self.writer.close() - await self.writer.wait_closed() except Exception: pass + def _lock(self): + if self._write_lock is None: + self._write_lock = asyncio.Lock() + return self._write_lock + + async def _send(self, queue_frames): + """Queue frames on h2 and put them on the wire. + + Streams are served by concurrent tasks, so both steps happen + under one lock with no await between them: the receive loop + cannot parse a GOAWAY (which clears h2's outbound buffer) in + between, and HEADERS leave in the order they were encoded. + """ + async with self._lock(): + queue_frames() + await self._flush_locked() + async def _send_pending_data(self): - """Send any pending data from h2 to the socket.""" + """Send whatever h2 already has queued.""" + async with self._lock(): + await self._flush_locked() + + async def _flush_locked(self): data = self.h2_conn.data_to_send() if data: try: @@ -642,8 +747,22 @@ def is_closed(self): return self._closed def cleanup_stream(self, stream_id): - """Remove a stream after processing is complete.""" - self.streams.pop(stream_id, None) + """Remove a stream after processing is complete. + + A body the application did not finish reading is cut off with + RST_STREAM(NO_ERROR) (RFC 9113 section 8.1). + """ + stream = self.streams.pop(stream_id, None) + if stream is None: + return + if not stream.body_complete and stream.state is not StreamState.CLOSED: + stream.reset(HTTP2ErrorCode.NO_ERROR) + try: + self.h2_conn.reset_stream(stream_id, error_code=HTTP2ErrorCode.NO_ERROR) + except _h2_exceptions.ProtocolError: + pass + if not self._closed: + self._write_pending_nowait() def __repr__(self): return ( diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index 4bb83bb712..fd1c1fa4f7 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -17,7 +17,7 @@ HTTP2Error, HTTP2ProtocolError, HTTP2ConnectionError, HTTP2NotAvailable, HTTP2ErrorCode, ) -from .stream import HTTP2Stream +from .stream import HTTP2Stream, StreamState from .request import HTTP2Request @@ -81,8 +81,11 @@ def __init__(self, cfg, sock, client_addr): # for the main receive loop rather than discarded. self._deferred_events = collections.deque() - # Completed requests ready for processing - self._pending_requests = [] + # Requests that arrived while a request body was being pulled + # off the socket; handed to the worker on its next call. + self.pending_requests = collections.deque() + # Peer sent a graceful GOAWAY: finish open streams, take no new ones + self.draining = False # Connection settings from config self.initial_window_size = cfg.http2_initial_window_size @@ -172,21 +175,43 @@ def initiate_upgrade(self, settings_header, http1_req, body=None): stream.receive_headers(pseudo + regular, end_stream=not body) if body: stream.receive_data(body, end_stream=True) + # That body came over HTTP/1, so no window credit is owed. + stream.acked_size = stream.body_size self.streams[1] = stream return HTTP2Request(stream, self.cfg, self.client_addr) def receive_data(self, data=None): - """Process received data and return completed requests. + """Process received data and return new requests. + + A request is returned as soon as its headers are in; its body is + read from the stream by the application, arriving frames being + pulled in through pump() as needed. Args: data: Optional bytes to process. If None, reads from socket. Returns: - list: List of HTTP2Request objects for completed requests + list: List of HTTP2Request objects for new requests Raises: HTTP2ConnectionError: On protocol or connection errors """ + if data is None and self.pending_requests: + pending = list(self.pending_requests) + self.pending_requests.clear() + return pending + return self._read_and_process(data) + + def pump(self): + """Read once from the socket while a request body is being consumed. + + Frames for the stream being read land in its buffer; any request + that arrives meanwhile is queued for the worker's next + receive_data() call rather than dropped. + """ + self.pending_requests.extend(self._read_and_process(None)) + + def _read_and_process(self, data): if data is None: try: data = self.sock.recv(self.READ_BUFFER_SIZE) @@ -232,6 +257,7 @@ def receive_data(self, data=None): # Process events, oldest first: anything set aside during a # flow-control wait arrived before this batch. completed_requests = [] + was_draining = self.draining if self._deferred_events: events = list(self._deferred_events) + list(events) self._deferred_events.clear() @@ -240,6 +266,12 @@ def receive_data(self, data=None): if request is not None: completed_requests.append(request) + if was_draining: + # Streams opened after the peer's GOAWAY are refused. + for request in completed_requests: + self.reset_stream(request.stream.stream_id, HTTP2ErrorCode.REFUSED_STREAM) + self.streams.pop(request.stream.stream_id, None) + completed_requests = [] # Send any pending data (WINDOW_UPDATE, etc.) self._send_pending_data() @@ -297,43 +329,55 @@ def _handle_request_received(self, event): self.streams[stream_id] = stream # Process headers - # The StreamEnded event will come separately for GET/HEAD with no body stream.receive_headers(headers, end_stream=False) + # Dispatch on headers: the body streams in behind the request. + return HTTP2Request(stream, self.cfg, self.client_addr) + def _handle_data_received(self, event): """Handle DataReceived event. + The payload is held on the stream until the application takes it; + window credit goes back through acknowledge_data() at that point. + Args: event: DataReceived event with body data - - Returns: - None (request completion handled by StreamEnded) """ - stream_id = event.stream_id - data = event.data - - stream = self.streams.get(stream_id) + stream = self.streams.get(event.stream_id) if stream is None: # Stream was reset or doesn't exist return None - stream.receive_data(data, end_stream=False) - - # Increment flow control windows (only if data received) - if len(data) > 0: - try: - # Update stream-level window - self.h2_conn.increment_flow_control_window(len(data), stream_id=stream_id) - # Update connection-level window - self.h2_conn.increment_flow_control_window(len(data), stream_id=None) - # Send WINDOW_UPDATE frames immediately - self._send_pending_data() - except (ValueError, _h2_exceptions.FlowControlError): - # Window overflow - send FLOW_CONTROL_ERROR and close - self.close(error_code=HTTP2ErrorCode.FLOW_CONTROL_ERROR) + stream.receive_data(event.data, end_stream=False) + # Connection-level credit goes straight back so a stream that is + # waiting its turn can never starve the one being served; the + # stream-level window is credited only as the body is consumed. + # Padding is never buffered, so its credit goes back now too. + length = event.flow_controlled_length + padding = length - len(event.data) + try: + if length: + self.h2_conn.increment_flow_control_window(length, stream_id=None) + if padding: + self.h2_conn.increment_flow_control_window(padding, stream_id=event.stream_id) + except (ValueError, _h2_exceptions.ProtocolError): + pass return None + def acknowledge_data(self, stream_id, size): + """Credit ``size`` consumed bytes back to the peer. + + Called by the stream as the application takes body data. + """ + if size <= 0 or self._closed: + return + try: + self.h2_conn.increment_flow_control_window(size, stream_id=stream_id) + except (ValueError, _h2_exceptions.ProtocolError): + return + self._send_pending_data() + def _handle_stream_ended(self, event): """Handle StreamEnded event. @@ -349,16 +393,12 @@ def _handle_stream_ended(self, event): if stream is None: return None - # Mark stream as request complete and body complete so the - # receive() closure's _body_complete guard fires, preventing - # the fast path from re-reading already-consumed data from BytesIO. + # The request went out on its headers; only mark the body complete. stream.request_complete = True stream._body_complete = True if stream._body_event: stream._body_event.set() - - # Create request object - return HTTP2Request(stream, self.cfg, self.client_addr) + return None def _handle_stream_reset(self, event): """Handle StreamReset event (RST_STREAM frame). @@ -376,9 +416,20 @@ def _handle_stream_reset(self, event): def _handle_connection_terminated(self, event): """Handle ConnectionTerminated event (GOAWAY frame). + A graceful GOAWAY (NO_ERROR) only forbids new streams; the ones + already established finish (RFC 9113 section 6.8). h2 closes its + connection state on receipt and would refuse to send anything, + so its state is put back to open while those streams drain, and + the connection closes once they are done. Any other error code + closes at once. + Args: event: ConnectionTerminated event """ + if event.error_code == HTTP2ErrorCode.NO_ERROR and self.streams: + self.draining = True + self.h2_conn.state_machine.state = _h2.ConnectionState.SERVER_OPEN + return self._closed = True # Could log event.error_code and event.additional_data @@ -398,9 +449,9 @@ def _handle_trailers_received(self, event): return None stream.receive_trailers(event.headers) - - # Trailers always end the request - return HTTP2Request(stream, self.cfg, self.client_addr) + if stream._body_event: + stream._body_event.set() + return None def _handle_priority_updated(self, event): """Handle PriorityUpdated event (PRIORITY frame). @@ -749,10 +800,32 @@ def is_closed(self): def cleanup_stream(self, stream_id): """Remove a stream after processing is complete. + A body the application did not finish reading is cut off with + RST_STREAM(NO_ERROR): the response is complete and the peer is + only asked to stop sending (RFC 9113 section 8.1). + Args: stream_id: The stream ID to clean up """ - self.streams.pop(stream_id, None) + stream = self.streams.pop(stream_id, None) + if stream is None: + return + if not stream.body_complete and stream.state is not StreamState.CLOSED: + stream.reset(HTTP2ErrorCode.NO_ERROR) + try: + self.h2_conn.reset_stream(stream_id, error_code=HTTP2ErrorCode.NO_ERROR) + except _h2_exceptions.ProtocolError: + pass + if self.draining and not self.streams and not self.pending_requests: + # The peer's GOAWAY is honoured once the last established + # stream is done. + self.close() + return + if not self._closed: + try: + self._send_pending_data() + except HTTP2ConnectionError: + pass def __repr__(self): return ( diff --git a/gunicorn/http2/request.py b/gunicorn/http2/request.py index 647109e9ce..6d1fa38790 100644 --- a/gunicorn/http2/request.py +++ b/gunicorn/http2/request.py @@ -9,31 +9,77 @@ Provides a Request-compatible interface for HTTP/2 streams. """ -from io import BytesIO - from gunicorn.http.message import ( HeaderPolicy, RFC9110_5_5_INVALID_AND_DANGEROUS, ) from gunicorn.http.errors import InvalidHeader +from gunicorn.http2.errors import HTTP2StreamError +from gunicorn.http2.stream import StreamState from gunicorn.util import split_request_uri class HTTP2Body: - """Body wrapper for HTTP/2 request data. + """File-like ``wsgi.input`` over an HTTP/2 stream. - Provides a file-like interface to the request body, - compatible with gunicorn's Body class expectations. - """ + Data is taken from the stream as the application reads it. When the + stream holds nothing and the body is not complete, the connection is + asked to read more frames from the socket, so the request is served + while its body is still arriving and the peer only ever has one + receive window of it in flight. - def __init__(self, data): - """Initialize with body data. + Bytes may be passed instead of a stream for a body already in hand. + """ - Args: - data: bytes containing the request body - """ - self._data = BytesIO(data) - self._len = len(data) + def __init__(self, source): + self._buf = b"" + self._eof = False + self._closed = False + if isinstance(source, (bytes, bytearray)): + self._stream = None + self._buf = bytes(source) + self._eof = True + else: + self._stream = source + + def _next_chunk(self): + """Return the next payload, or None once the body is complete.""" + if self._eof: + return None + stream = self._stream + while True: + chunk = stream.pop_chunk() + if chunk is not None: + return chunk + if stream.body_complete: + self._eof = True + return None + connection = stream.connection + if stream.state is StreamState.CLOSED or connection.is_closed: + raise HTTP2StreamError( + stream.stream_id, + "stream closed before its request body was complete") + pump = getattr(connection, "pump", None) + if pump is None: + # Nothing here can wait for frames; the caller reads via + # read_body_chunk() instead. + self._eof = True + return None + pump() + + def _fill(self, size): + """Hold at least ``size`` bytes, or everything up to the end.""" + if self._closed: + raise ValueError("I/O operation on closed file.") + parts = [self._buf] + held = len(self._buf) + while size is None or held < size: + chunk = self._next_chunk() + if chunk is None: + break + parts.append(chunk) + held += len(chunk) + self._buf = b"".join(parts) if len(parts) > 1 else parts[0] def read(self, size=None): """Read data from the body. @@ -44,9 +90,15 @@ def read(self, size=None): Returns: bytes: The requested data """ - if size is None: - return self._data.read() - return self._data.read(size) + if size is None or size < 0: + self._fill(None) + data, self._buf = self._buf, b"" + return data + if size == 0: + return b"" + self._fill(size) + data, self._buf = self._buf[:size], self._buf[size:] + return data def readline(self, size=None): """Read a line from the body. @@ -57,9 +109,29 @@ def readline(self, size=None): Returns: bytes: A line of data """ - if size is None: - return self._data.readline() - return self._data.readline(size) + if size is not None and size < 0: + size = None + if size == 0: + return b"" + if self._closed: + raise ValueError("I/O operation on closed file.") + while True: + idx = self._buf.find(b"\n") + if idx >= 0: + end = idx + 1 + if size is not None: + end = min(end, size) + break + if size is not None and len(self._buf) >= size: + end = size + break + chunk = self._next_chunk() + if chunk is None: + end = len(self._buf) if size is None else min(size, len(self._buf)) + break + self._buf += chunk + data, self._buf = self._buf[:end], self._buf[end:] + return data def readlines(self, hint=None): """Read all lines from the body. @@ -70,19 +142,32 @@ def readlines(self, hint=None): Returns: list: List of lines """ - return self._data.readlines(hint) + lines = [] + total = 0 + while True: + line = self.readline() + if not line: + break + lines.append(line) + total += len(line) + if hint is not None and 0 < hint <= total: + break + return lines + + def close(self): + """Drop what is held; further reads raise ValueError.""" + self._closed = True + self._buf = b"" def __iter__(self): """Iterate over lines in the body.""" - return iter(self._data) - - def __len__(self): - """Return the content length.""" - return self._len + return self - def close(self): - """Close the body stream.""" - self._data.close() + def __next__(self): + line = self.readline() + if not line: + raise StopIteration + return line class HTTP2Request(HeaderPolicy): @@ -170,17 +255,8 @@ def __init__(self, stream, cfg, peer_addr): self.headers = [(n, v) for n, v in self.headers if n != 'HOST'] self.headers.append(('HOST', authority)) - # Trailers (if any) - self.trailers = [] - if stream.trailers: - self.trailers = [ - (name.upper(), value) - for name, value in stream.trailers - ] - - # Body - HTTP/2 streams have complete body data - body_data = stream.get_request_body() - self.body = HTTP2Body(body_data) + # Body: read from the stream as it arrives. + self.body = HTTP2Body(stream) # Connection state self.must_close = False @@ -198,6 +274,16 @@ def __init__(self, stream, cfg, peer_addr): self.priority_weight = stream.priority_weight self.priority_depends_on = stream.priority_depends_on + @property + def trailers(self): + """Trailing headers, available once the whole body has been read.""" + if not self.stream.trailers: + return [] + return [ + (name.upper(), value) + for name, value in self.stream.trailers + ] + def force_close(self): """Force the connection to close after this request.""" self.must_close = True diff --git a/gunicorn/http2/stream.py b/gunicorn/http2/stream.py index 34b7be18d3..acbd8415dc 100644 --- a/gunicorn/http2/stream.py +++ b/gunicorn/http2/stream.py @@ -10,7 +10,6 @@ """ from enum import Enum, auto -from io import BytesIO from .errors import HTTP2StreamError @@ -49,7 +48,6 @@ def __init__(self, stream_id, connection): # Request data self.request_headers = [] - self.request_body = BytesIO() self.request_complete = False # Response data @@ -71,8 +69,14 @@ def __init__(self, stream_id, connection): self.priority_depends_on = 0 self.priority_exclusive = False - # Streaming body support (avoids buffering entire uploads) + # Request body: DATA payloads in arrival order, held only until + # the application takes them. Flow-control credit for a payload + # goes back to the peer when it is taken, not when it arrives, so + # what sits here is bounded by the receive window. body_size + # counts what arrived, acked_size what has been credited back. self._body_chunks = [] + self.body_size = 0 + self.acked_size = 0 self._body_event = None # Lazy-init asyncio.Event self._body_complete = False @@ -125,6 +129,9 @@ def receive_headers(self, headers, end_stream=False): if end_stream: self._half_close_remote() self.request_complete = True + self._body_complete = True + if self._body_event: + self._body_event.set() def receive_data(self, data, end_stream=False): """Process received DATA frame with streaming support. @@ -142,15 +149,12 @@ def receive_data(self, data, end_stream=False): f"Cannot receive data in state {self.state.name}" ) - # Add to chunks queue for streaming reads if data: self._body_chunks.append(data) + self.body_size += len(data) if self._body_event: self._body_event.set() - # Also write to legacy BytesIO for compatibility - self.request_body.write(data) - if end_stream: self._half_close_remote() self.request_complete = True @@ -244,6 +248,9 @@ def reset(self, error_code=0x8): self.state = StreamState.CLOSED self.response_complete = True self.request_complete = True + # Wake a reader waiting on the body; it finds the stream closed. + if self._body_event: + self._body_event.set() def close(self): """Close this stream normally.""" @@ -290,13 +297,46 @@ def _half_close_remote(self): f"Cannot half-close remote in state {self.state.name}" ) + @property + def body_complete(self): + """True once END_STREAM (or trailers) arrived for the request.""" + return self._body_complete + + @property + def unacked_size(self): + """Bytes received on this stream not yet credited back to the peer.""" + return self.body_size - self.acked_size + def get_request_body(self): - """Get the complete request body. + """Join whatever body data is currently held, without taking it. + + Returns: + bytes: The request body data received so far + """ + if len(self._body_chunks) == 1: + return self._body_chunks[0] + return b"".join(self._body_chunks) + + def pop_chunk(self): + """Take the next held DATA payload and credit it back to the peer. Returns: - bytes: The request body data + bytes: The payload, or None when nothing is held right now. """ - return self.request_body.getvalue() + if not self._body_chunks: + return None + chunk = self._body_chunks.pop(0) + self._acknowledge(len(chunk)) + return chunk + + def _acknowledge(self, size): + """Return flow-control credit for ``size`` consumed bytes.""" + if size <= 0: + return + self.acked_size += size + ack = getattr(self.connection, "acknowledge_data", None) + if ack is not None: + ack(self.stream_id, size) async def read_body_chunk(self): """Read next body chunk asynchronously for streaming. @@ -316,12 +356,17 @@ async def read_body_chunk(self): while True: # Return chunk if available - if self._body_chunks: - return self._body_chunks.pop(0) + chunk = self.pop_chunk() + if chunk is not None: + return chunk # No more data expected if self._body_complete: return None + if self.state is StreamState.CLOSED: + raise HTTP2StreamError( + self.stream_id, + "stream closed before its request body was complete") # Wait for more data self._body_event.clear() diff --git a/gunicorn/workers/base_async.py b/gunicorn/workers/base_async.py index 9adc913a7e..e12f2d2d4a 100644 --- a/gunicorn/workers/base_async.py +++ b/gunicorn/workers/base_async.py @@ -15,6 +15,7 @@ from gunicorn.http.errors import InvalidH2CPreface from gunicorn.http2 import negotiation from gunicorn.http2.response import HTTP2Response +from gunicorn.http2.errors import HTTP2StreamError from gunicorn.workers import base ALREADY_HANDLED = object() @@ -194,6 +195,10 @@ def handle_http2(self, listener, client, addr, preface=b"", for req in requests: try: self.handle_http2_request(listener_name, req, client, addr, h2_conn) + except HTTP2StreamError as e: + # The peer reset the stream while its body was + # being read; there is no one left to answer. + self.log.debug("HTTP/2 stream closed: %s", e) except Exception as e: self.log.exception("Error handling HTTP/2 request") try: diff --git a/gunicorn/workers/gthread.py b/gunicorn/workers/gthread.py index 08391eb1d0..18ce82dce8 100644 --- a/gunicorn/workers/gthread.py +++ b/gunicorn/workers/gthread.py @@ -30,6 +30,7 @@ from ..http.errors import InvalidH2CPreface from ..http2 import negotiation from ..http2.response import HTTP2Response +from ..http2.errors import HTTP2StreamError # Sentinel value to indicate connection should be deferred back to poller @@ -600,6 +601,10 @@ def handle_http2(self, conn): for req in requests: try: self.handle_http2_request(req, conn, h2_conn) + except HTTP2StreamError as e: + # The peer reset the stream while its body was + # being read; there is no one left to answer. + self.log.debug("HTTP/2 stream closed: %s", e) except Exception as e: self.log.exception("Error handling HTTP/2 request") try: diff --git a/tests/test_http2_async_connection.py b/tests/test_http2_async_connection.py index 6d1731297d..cc71a8a8c9 100644 --- a/tests/test_http2_async_connection.py +++ b/tests/test_http2_async_connection.py @@ -17,10 +17,12 @@ import h2.connection import h2.config import h2.events + import h2.errors H2_AVAILABLE = True except ImportError: H2_AVAILABLE = False +from gunicorn.http2.stream import StreamState from gunicorn.http2.errors import ( HTTP2Error, HTTP2ConnectionError ) @@ -938,44 +940,49 @@ async def test_send_data_on_reset_stream(self): class TestAsyncHTTP2WindowOverflowHandling: - """Test window overflow handling in async connection.""" + """A peer sending past the receive window gets GOAWAY(FLOW_CONTROL_ERROR).""" @pytest.mark.asyncio async def test_window_overflow_sends_goaway(self): - """Test that window overflow results in connection close.""" + from hyperframe.frame import DataFrame from gunicorn.http2.async_connection import AsyncHTTP2Connection - from gunicorn.http2.errors import HTTP2ErrorCode + from gunicorn.http2.errors import HTTP2ProtocolError cfg = MockConfig() reader = MockAsyncReader() writer = MockAsyncWriter() conn = AsyncHTTP2Connection(cfg, reader, writer, ('127.0.0.1', 12345)) - # Create client and send preface client_conn = create_client_connection() reader.set_data(client_conn.data_to_send()) await conn.initiate_connection() await conn.receive_data() - - # Mock increment_flow_control_window to raise ValueError (overflow) - def raise_overflow(increment, stream_id=None): - raise ValueError("Flow control window too large") - - conn.h2_conn.increment_flow_control_window = raise_overflow - - # Send a request with data to trigger the overflow + client_conn.receive_data(writer.get_written_data()) client_conn.send_headers(1, [ (':method', 'POST'), (':path', '/'), (':scheme', 'https'), (':authority', 'localhost'), ], end_stream=False) - client_conn.send_data(1, b'test data', end_stream=True) reader.set_data(client_conn.data_to_send()) await conn.receive_data() - # Connection should be closed with FLOW_CONTROL_ERROR + # Nothing is credited back until the application reads, so five + # full frames overrun the 65535 byte window. + frames = b"".join(DataFrame(1, data=b"x" * 16384).serialize() for _ in range(5)) + reader.set_data(frames) + writer.clear() + # The reader hands over 64KiB per call; the overrun lands on the second. + with pytest.raises(HTTP2ProtocolError): + for _ in range(3): + await conn.receive_data() + assert conn.is_closed is True + events = client_conn.receive_data(writer.get_written_data()) + goaway = [e for e in events if isinstance(e, h2.events.ConnectionTerminated)] + # h2 sends one GOAWAY itself before raising; close() sends another. + assert goaway + assert {g.error_code for g in goaway} == {h2.errors.ErrorCodes.FLOW_CONTROL_ERROR} class TestAsyncHTTP2ProtocolErrorHandling: @@ -1023,8 +1030,8 @@ def raise_protocol_error(data): assert conn.is_closed is True -class TestAsyncDeferredFlowControlEvents: - """Events read while waiting on a window must reach the main loop.""" +class TestAsyncWindowWait: + """A response task waits for credit on a signal, never on the reader.""" def _conn(self): from gunicorn.http2.async_connection import AsyncHTTP2Connection @@ -1035,21 +1042,359 @@ def test_queue_starts_empty(self): assert not self._conn()._deferred_events @pytest.mark.asyncio - async def test_events_during_a_window_wait_are_captured(self): + async def test_wait_returns_when_the_receive_loop_signals(self): conn = self._conn() - arriving = mock.Mock(name="RequestReceived") - windows = iter([0, 0, 65535]) + windows = iter([0, 65535]) conn.h2_conn = mock.Mock() conn.h2_conn.local_flow_control_window.side_effect = \ lambda sid: next(windows) - conn.h2_conn.receive_data.return_value = [arriving] - conn.reader = MockAsyncReader(b"frame bytes") + conn.reader = mock.Mock() + conn.reader.read = mock.AsyncMock(side_effect=AssertionError("reader touched")) + + async def widen(): + await asyncio.sleep(0.01) + conn._signal_window() + + asyncio.get_running_loop().create_task(widen()) + assert await conn._wait_for_flow_control_window(1) == 65535 + conn.reader.read.assert_not_called() + + @pytest.mark.asyncio + async def test_wait_gives_up_when_the_connection_closes(self): + conn = self._conn() + conn.h2_conn = mock.Mock() + conn.h2_conn.local_flow_control_window.return_value = 0 + conn._closed = True + assert await conn._wait_for_flow_control_window(1) == -1 + + +class TestAsyncStreamingRequestBody: + """The request goes out on its headers; chunks are read as they arrive.""" + + async def _open_post(self): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + writer.clear() + client.send_headers( + stream_id=1, + headers=[ + (':method', 'POST'), + (':path', '/upload'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=False, + ) + reader.set_data(client.data_to_send()) + requests = await conn.receive_data() + return conn, client, reader, writer, requests + + @pytest.mark.asyncio + async def test_request_is_dispatched_before_the_body_arrives(self): + conn, client, reader, writer, requests = await self._open_post() + assert len(requests) == 1 + assert requests[0].stream.body_complete is False + + @pytest.mark.asyncio + async def test_chunks_are_read_as_they_arrive_and_credited_back(self): + conn, client, reader, writer, requests = await self._open_post() + stream = requests[0].stream + + for _ in range(3): + client.send_data(1, b"x" * 16384, end_stream=False) + reader.set_data(client.data_to_send()) + await conn.receive_data() + assert stream.unacked_size == 3 * 16384 + client.receive_data(writer.get_written_data()) + writer.clear() + # Connection-level credit comes straight back, the stream's + # only once the application reads. + assert client.outbound_flow_control_window == 65535 + assert client.local_flow_control_window(1) == 65535 - 3 * 16384 + + assert await stream.read_body_chunk() == b"x" * 16384 + assert await stream.read_body_chunk() == b"x" * 16384 + assert stream.unacked_size == 16384 + client.receive_data(writer.get_written_data()) + assert client.local_flow_control_window(1) == 65535 - 16384 + + client.send_data(1, b"end", end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + chunks = [] + while True: + chunk = await stream.read_body_chunk() + if chunk is None: + break + chunks.append(chunk) + assert b"".join(chunks) == b"x" * 16384 + b"end" + assert stream.unacked_size == 0 + + @pytest.mark.asyncio + async def test_unread_body_is_reset_with_no_error_on_cleanup(self): + conn, client, reader, writer, requests = await self._open_post() + for _ in range(3): + client.send_data(1, b"a" * 16384, end_stream=False) + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + writer.clear() + + conn.cleanup_stream(1) + + assert 1 not in conn.streams + events = client.receive_data(writer.get_written_data()) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert len(resets) == 1 + assert resets[0].error_code == h2.errors.ErrorCodes.NO_ERROR + assert client.outbound_flow_control_window == 65535 + assert conn.is_closed is False + + @pytest.mark.asyncio + async def test_peer_reset_wakes_a_waiting_reader(self): + from gunicorn.http2.errors import HTTP2StreamError + conn, client, reader, writer, requests = await self._open_post() + stream = requests[0].stream + + async def feed_reset(): + await asyncio.sleep(0.01) + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + reader.set_data(client.data_to_send()) + await conn.receive_data() + + asyncio.get_running_loop().create_task(feed_reset()) + with pytest.raises(HTTP2StreamError): + await asyncio.wait_for(stream.read_body_chunk(), timeout=2) + + +class TestAsyncStreamingEdgeCases: + """Frames for unknown streams and failures while crediting or resetting.""" + + async def _open(self): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + writer.clear() + client.send_headers( + stream_id=1, + headers=[ + (':method', 'POST'), + (':path', '/'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=False, + ) + reader.set_data(client.data_to_send()) + requests = await conn.receive_data() + return conn, client, reader, writer, requests + + @pytest.mark.asyncio + async def test_data_and_trailers_for_a_cleaned_up_stream_are_ignored(self): + conn, client, reader, writer, requests = await self._open() + conn.cleanup_stream(1) + client.send_data(1, b"late", end_stream=False) + client.send_headers(1, [('x-t', '1')], end_stream=True) + reader.set_data(client.data_to_send()) + assert await conn.receive_data() == [] + assert conn.is_closed is False + + @pytest.mark.asyncio + async def test_trailers_wake_reader(self): + conn, client, reader, writer, requests = await self._open() + stream = requests[0].stream + client.send_data(1, b"abc", end_stream=False) + client.send_headers(1, [('x-t', '1')], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + assert await stream.read_body_chunk() == b"abc" + assert await stream.read_body_chunk() is None + assert requests[0].trailers == [('X-T', '1')] + + @pytest.mark.asyncio + async def test_credit_after_close_or_failure_is_dropped(self): + conn, client, reader, writer, requests = await self._open() + stream = requests[0].stream + client.send_data(1, b"abc", end_stream=False) + client.send_data(1, b"def", end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + conn.h2_conn.increment_flow_control_window = mock.Mock( + side_effect=h2.exceptions.StreamClosedError(1)) + assert await stream.read_body_chunk() == b"abc" + conn._closed = True + conn.h2_conn.increment_flow_control_window = mock.Mock() + assert await stream.read_body_chunk() == b"def" + conn.h2_conn.increment_flow_control_window.assert_not_called() + conn.acknowledge_data(1, 0) + + @pytest.mark.asyncio + async def test_cleanup_survives_reset_and_write_failures(self): + conn, client, reader, writer, requests = await self._open() + conn.h2_conn.reset_stream = mock.Mock( + side_effect=h2.exceptions.StreamClosedError(1)) + conn.h2_conn.data_to_send = mock.Mock(return_value=b"frame") + writer.close() + conn.cleanup_stream(1) + assert 1 not in conn.streams + assert conn.is_closed is True + conn.cleanup_stream(1) + + @pytest.mark.asyncio + async def test_frames_for_a_stream_we_dropped_are_ignored(self): + conn, client, reader, writer, requests = await self._open() + conn.streams.pop(1) + client.send_data(1, b"late", end_stream=False) + client.send_headers(1, [('x-t', '1')], end_stream=True) + reader.set_data(client.data_to_send()) + assert await conn.receive_data() == [] + assert conn.is_closed is False + + @pytest.mark.asyncio + async def test_arrival_credit_failure_is_swallowed(self): + conn, client, reader, writer, requests = await self._open() + conn.h2_conn.increment_flow_control_window = mock.Mock( + side_effect=h2.exceptions.ProtocolError("nope")) + client.send_data(1, b"abc", end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + assert await requests[0].stream.read_body_chunk() == b"abc" + + +class TestSendIsAtomicAgainstGoAway: + """HEADERS queued while another task holds the writer survive a GOAWAY.""" + + @pytest.mark.asyncio + async def test_headers_are_not_erased(self): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + from test_http2_connection import frame_types + + class SlowWriter(MockAsyncWriter): + """Once armed, the next drain() blocks until released.""" + + def __init__(self): + super().__init__() + self.release = asyncio.Event() + self.armed = False + + async def drain(self): + if self.armed: + self.armed = False + await self.release.wait() + + reader = MockAsyncReader() + writer = SlowWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + for sid in (1, 3): + client.send_headers(sid, [ + (':method', 'GET'), (':path', f'/{sid}'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + writer.clear() + + # Task 1 holds the writer in drain(); task 2 queues its HEADERS and + # waits; the peer's GOAWAY is parsed meanwhile. + writer.armed = True + t1 = asyncio.get_running_loop().create_task(conn.send_response(1, 200, [], b"one")) + await asyncio.sleep(0.01) + t2 = asyncio.get_running_loop().create_task(conn.send_response(3, 200, [], b"two")) + await asyncio.sleep(0.01) + client.close_connection() + reader.set_data(client.data_to_send()) + t3 = asyncio.get_running_loop().create_task(conn.receive_data()) + await asyncio.sleep(0.01) + writer.release.set() + await asyncio.gather(t1, t2, t3) + + kinds = frame_types(writer.get_written_data()) + assert kinds.count("HeadersFrame") == 2, kinds + assert conn.draining is True + - async def noop(): - return None - conn._send_pending_data = noop +class TestResetDuringDrain: + """A RST_STREAM parsed while a send awaits drain() is not a send error.""" + + async def _race(self, action): + from gunicorn.http2.async_connection import AsyncHTTP2Connection - await conn._wait_for_flow_control_window(1) + class SlowWriter(MockAsyncWriter): + def __init__(self): + super().__init__() + self.release = asyncio.Event() + self.armed = False - assert list(conn._deferred_events) == [arriving], \ - "event read during the wait was dropped" + async def drain(self): + if self.armed: + self.armed = False + await self.release.wait() + + reader = MockAsyncReader() + writer = SlowWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + + writer.armed = True + loop = asyncio.get_running_loop() + sender = loop.create_task(action(conn)) + await asyncio.sleep(0.01) + # The sender holds the lock in drain(); the loop parses the reset + # now and queues behind it for its own flush. + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + reader.set_data(client.data_to_send()) + receiver = loop.create_task(conn.receive_data()) + await asyncio.sleep(0.01) + assert conn.streams[1].state is StreamState.CLOSED + writer.release.set() + await asyncio.wait_for(receiver, timeout=2) + return await asyncio.wait_for(sender, timeout=2) + + @pytest.mark.asyncio + async def test_response_headers(self): + assert await self._race( + lambda c: c.send_response_headers(1, [(':status', '200')])) in (True, False) + + @pytest.mark.asyncio + async def test_full_response(self): + assert await self._race( + lambda c: c.send_response(1, 200, [], b"body")) in (True, False) + + @pytest.mark.asyncio + async def test_data_then_trailers(self): + async def action(conn): + await conn.send_response_headers(1, [(':status', '200')]) + conn.writer.armed = True + await conn.send_data(1, b"x" * 20000, end_stream=False) + return await conn.send_trailers(1, [('x-t', '1')]) + assert await self._race(action) in (True, False) diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index 2a7186a5a3..c1af71472f 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -17,12 +17,13 @@ import h2.config import h2.events import h2.exceptions + import h2.errors H2_AVAILABLE = True except ImportError: H2_AVAILABLE = False from gunicorn.http2.errors import ( - HTTP2Error, HTTP2ConnectionError + HTTP2Error, HTTP2ConnectionError, HTTP2ProtocolError ) @@ -927,42 +928,45 @@ def test_send_data_on_reset_stream(self): class TestHTTP2WindowOverflowHandling: - """Test window overflow handling.""" + """A peer sending past the receive window gets GOAWAY(FLOW_CONTROL_ERROR).""" def test_window_overflow_sends_goaway(self): - """Test that window overflow results in GOAWAY with FLOW_CONTROL_ERROR.""" + from hyperframe.frame import DataFrame from gunicorn.http2.connection import HTTP2ServerConnection - from gunicorn.http2.errors import HTTP2ErrorCode cfg = MockConfig() sock = MockSocket() conn = HTTP2ServerConnection(cfg, sock, ('127.0.0.1', 12345)) conn.initiate_connection() - # Create client and send preface client_conn = create_client_connection() conn.receive_data(client_conn.data_to_send()) - - # Mock increment_flow_control_window to raise ValueError (overflow) - original_increment = conn.h2_conn.increment_flow_control_window - - def raise_overflow(increment, stream_id=None): - raise ValueError("Flow control window too large") - - conn.h2_conn.increment_flow_control_window = raise_overflow - - # Send a request with data to trigger the overflow + client_conn.receive_data(sock.get_sent_data()) client_conn.send_headers(1, [ (':method', 'POST'), (':path', '/'), (':scheme', 'https'), (':authority', 'localhost'), ], end_stream=False) - client_conn.send_data(1, b'test data', end_stream=True) conn.receive_data(client_conn.data_to_send()) - # Connection should be closed with FLOW_CONTROL_ERROR + # Nothing is credited back until the application reads, so five + # full frames overrun the 65535 byte window. Serialized by hand + # because a well-behaved client would refuse to send them. + frames = b"" + for _ in range(5): + f = DataFrame(1, data=b"x" * 16384) + frames += f.serialize() + before = len(sock.get_sent_data()) + with pytest.raises(HTTP2ProtocolError): + conn.receive_data(frames) + assert conn.is_closed is True + events = client_conn.receive_data(sock.get_sent_data()[before:]) + goaway = [e for e in events if isinstance(e, h2.events.ConnectionTerminated)] + # h2 sends one GOAWAY itself before raising; close() sends another. + assert goaway + assert {g.error_code for g in goaway} == {h2.errors.ErrorCodes.FLOW_CONTROL_ERROR} class TestHTTP2ProtocolErrorHandling: @@ -1289,3 +1293,418 @@ def test_end_stream_with_trailers_sends_trailers(self): def test_end_stream_on_unknown_stream(self): conn = self._conn() assert conn.end_stream(999) is False + + +class TestStreamingRequestBody: + """The request goes out on its headers; the body follows as it arrives.""" + + def _open_post(self): + from gunicorn.http2.connection import HTTP2ServerConnection + + sock = MockSocket() + conn = HTTP2ServerConnection(MockConfig(), sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(sock.get_sent_data()) + client.send_headers( + stream_id=1, + headers=[ + (':method', 'POST'), + (':path', '/upload'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=False, + ) + requests = conn.receive_data(client.data_to_send()) + return conn, client, sock, requests + + def _drain(self, client, sock, since): + """Feed the client what the server wrote since ``since``.""" + data = sock.get_sent_data() + client.receive_data(data[since:]) + return len(data) + + def test_request_is_dispatched_before_the_body_arrives(self): + conn, client, sock, requests = self._open_post() + assert len(requests) == 1 + req = requests[0] + assert req.method == 'POST' + assert req.stream.body_complete is False + + def test_body_is_pulled_off_the_socket_as_the_app_reads(self): + conn, client, sock, requests = self._open_post() + req = requests[0] + + client.send_data(1, b"a" * 600, end_stream=False) + client.send_data(1, b"b" * 600, end_stream=True) + sock.set_recv_data(client.data_to_send()) + + # Nothing has been read from the socket yet. + assert req.stream.body_size == 0 + assert req.body.read(700) == b"a" * 600 + b"b" * 100 + assert req.body.read() == b"b" * 500 + assert req.body.read() == b"" + assert req.stream.body_complete is True + + def test_stream_credit_returns_only_for_what_the_app_read(self): + conn, client, sock, requests = self._open_post() + req = requests[0] + + # Fill most of the stream window without ending the stream. + for _ in range(3): + client.send_data(1, b"x" * 16384, end_stream=False) + before = len(sock.get_sent_data()) + conn.receive_data(client.data_to_send()) + + assert req.stream.body_size == 3 * 16384 + assert req.stream.unacked_size == 3 * 16384 + # Connection-level credit comes straight back... + before = self._drain(client, sock, before) + assert client.outbound_flow_control_window == 65535 + # ...the stream window only once the application reads. + assert client.local_flow_control_window(1) == 65535 - 3 * 16384 + + req.body.read(2 * 16384) + assert req.stream.unacked_size == 16384 + self._drain(client, sock, before) + assert client.local_flow_control_window(1) == 65535 - 16384 + + def test_queued_stream_cannot_starve_the_one_being_served(self): + conn, client, sock, requests = self._open_post() + req = requests[0] + client.send_headers( + stream_id=3, + headers=[ + (':method', 'POST'), + (':path', '/other'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=False, + ) + # Stream 3 uses up a whole connection window while stream 1 is + # the one being served. + for _ in range(4): + client.send_data(3, b"y" * 16383, end_stream=False) + before = len(sock.get_sent_data()) + later = conn.receive_data(client.data_to_send()) + assert [r.stream.stream_id for r in later] == [3] + before = self._drain(client, sock, before) + + assert client.outbound_flow_control_window == 65535 + assert client.local_flow_control_window(1) == 65535 + client.send_data(1, b"rest", end_stream=True) + sock.set_recv_data(client.data_to_send()) + assert req.body.read() == b"rest" + + def test_peer_past_the_window_gets_flow_control_error(self): + from hyperframe.frame import DataFrame + conn, client, sock, requests = self._open_post() + frames = b"".join(DataFrame(1, data=b"x" * 16384).serialize() for _ in range(5)) + with pytest.raises(HTTP2ProtocolError): + conn.receive_data(frames) + assert conn.is_closed is True + + def test_unread_body_is_reset_with_no_error_on_cleanup(self): + conn, client, sock, requests = self._open_post() + for _ in range(3): + client.send_data(1, b"a" * 16384, end_stream=False) + before = len(sock.get_sent_data()) + conn.receive_data(client.data_to_send()) + before = self._drain(client, sock, before) + + conn.cleanup_stream(1) + + assert 1 not in conn.streams + events = client.receive_data(sock.get_sent_data()[before:]) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert len(resets) == 1 + assert resets[0].error_code == h2.errors.ErrorCodes.NO_ERROR + # The unread bytes never cost connection-level credit. + assert client.outbound_flow_control_window == 65535 + assert conn.is_closed is False + + def test_complete_body_cleanup_sends_no_reset(self): + conn, client, sock, requests = self._open_post() + client.send_data(1, b"a" * 10, end_stream=True) + conn.receive_data(client.data_to_send()) + assert requests[0].body.read() == b"a" * 10 + before = len(sock.get_sent_data()) + conn.cleanup_stream(1) + events = client.receive_data(sock.get_sent_data()[before:]) + assert not [e for e in events if isinstance(e, h2.events.StreamReset)] + + def test_requests_arriving_during_a_body_read_are_queued(self): + conn, client, sock, requests = self._open_post() + req = requests[0] + + client.send_headers( + stream_id=3, + headers=[ + (':method', 'GET'), + (':path', '/other'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=True, + ) + client.send_data(1, b"body", end_stream=True) + sock.set_recv_data(client.data_to_send()) + + assert req.body.read() == b"body" + queued = conn.receive_data() + assert [r.stream.stream_id for r in queued] == [3] + assert queued[0].path == '/other' + assert conn.receive_data(b"") == [] + + def test_peer_reset_during_a_body_read_raises_stream_error(self): + from gunicorn.http2.errors import HTTP2StreamError + conn, client, sock, requests = self._open_post() + req = requests[0] + client.send_data(1, b"a" * 10, end_stream=False) + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + sock.set_recv_data(client.data_to_send()) + + assert req.body.read(10) == b"a" * 10 + with pytest.raises(HTTP2StreamError): + req.body.read() + + def test_trailers_are_visible_after_the_body(self): + conn, client, sock, requests = self._open_post() + req = requests[0] + client.send_data(1, b"payload", end_stream=False) + client.send_headers(1, [('x-checksum', 'abc')], end_stream=True) + sock.set_recv_data(client.data_to_send()) + + assert req.trailers == [] + assert req.body.read() == b"payload" + assert req.trailers == [('X-CHECKSUM', 'abc')] + + def test_readline_across_frames(self): + conn, client, sock, requests = self._open_post() + req = requests[0] + client.send_data(1, b"line one\nli", end_stream=False) + client.send_data(1, b"ne two\nlast", end_stream=True) + sock.set_recv_data(client.data_to_send()) + + assert req.body.readline() == b"line one\n" + assert req.body.readline(3) == b"lin" + assert list(req.body) == [b"e two\n", b"last"] + + def test_padding_is_credited_back(self): + from hyperframe.frame import DataFrame + conn, client, sock, requests = self._open_post() + req = requests[0] + + frames = b"" + for _ in range(255): + f = DataFrame(1, data=b"x", pad_length=255) + f.flags.add("PADDED") + frames += f.serialize() + conn.receive_data(frames) + assert req.body.read(255) == b"x" * 255 + + # Every flow-controlled byte, padding included, is back. + assert conn.h2_conn.remote_flow_control_window(1) == 65535 + + +def frame_types(data): + """Frame type names in ``data``, parsed without an h2 state machine.""" + from hyperframe.frame import Frame + kinds = [] + while data: + frame, length = Frame.parse_frame_header(memoryview(data[:9])) + frame.parse_body(memoryview(data[9:9 + length])) + kinds.append(type(frame).__name__) + data = data[9 + length:] + return kinds + + +class TestGracefulGoAway: + """GOAWAY(NO_ERROR) drains established streams; other codes close.""" + + def _open(self): + from gunicorn.http2.connection import HTTP2ServerConnection + + sock = MockSocket() + conn = HTTP2ServerConnection(MockConfig(), sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(sock.get_sent_data()) + return conn, client, sock + + def _get(self, client, stream_id, path="/"): + client.send_headers( + stream_id=stream_id, + headers=[ + (':method', 'GET'), + (':path', path), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=True, + ) + + def test_established_stream_is_answered_then_connection_closes(self): + conn, client, sock = self._open() + self._get(client, 1) + client.close_connection() + requests = conn.receive_data(client.data_to_send()) + + assert [r.stream.stream_id for r in requests] == [1] + assert conn.draining is True + assert conn.is_closed is False + + before = len(sock.get_sent_data()) + assert conn.send_response(1, 200, [], b"hello") is True + conn.cleanup_stream(1) + assert conn.is_closed is True + + # The h2 client is closed once it has sent GOAWAY, so the frames + # are checked raw, as a real client would still read them. + kinds = frame_types(sock.get_sent_data()[before:]) + assert "HeadersFrame" in kinds + assert "DataFrame" in kinds + assert kinds[-1] == "GoAwayFrame" + + def test_stream_opened_after_goaway_is_refused(self): + conn, client, sock = self._open() + self._get(client, 1) + client.close_connection() + conn.receive_data(client.data_to_send()) + + # h2 forbids the client from opening streams after its GOAWAY; + # a raw HEADERS frame stands in for a misbehaving peer. + from hyperframe.frame import HeadersFrame + encoder = client.encoder + f = HeadersFrame(3, data=encoder.encode([ + (':method', 'GET'), (':path', '/late'), + (':scheme', 'https'), (':authority', 'localhost')])) + f.flags.add('END_HEADERS') + f.flags.add('END_STREAM') + before = len(sock.get_sent_data()) + assert conn.receive_data(f.serialize()) == [] + assert 3 not in conn.streams + assert 1 in conn.streams + assert "RstStreamFrame" in frame_types(sock.get_sent_data()[before:]) + + def test_goaway_with_error_closes_at_once(self): + conn, client, sock = self._open() + self._get(client, 1) + client.close_connection(error_code=h2.errors.ErrorCodes.PROTOCOL_ERROR) + conn.receive_data(client.data_to_send()) + assert conn.is_closed is True + assert conn.draining is False + + def test_goaway_with_nothing_in_flight_closes(self): + conn, client, sock = self._open() + client.close_connection() + conn.receive_data(client.data_to_send()) + assert conn.is_closed is True + + +class TestStreamingEdgeCases: + """Frames for unknown streams and failures while crediting or resetting.""" + + def _open(self): + from gunicorn.http2.connection import HTTP2ServerConnection + + sock = MockSocket() + conn = HTTP2ServerConnection(MockConfig(), sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(sock.get_sent_data()) + return conn, client, sock + + def _post(self, client, stream_id=1): + client.send_headers( + stream_id=stream_id, + headers=[ + (':method', 'POST'), + (':path', '/'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=False, + ) + + def test_data_and_trailers_for_a_cleaned_up_stream_are_ignored(self): + conn, client, sock = self._open() + self._post(client) + requests = conn.receive_data(client.data_to_send()) + conn.cleanup_stream(1) + client.send_data(1, b"late", end_stream=False) + client.send_headers(1, [('x-t', '1')], end_stream=True) + assert conn.receive_data(client.data_to_send()) == [] + assert conn.is_closed is False + + def test_credit_after_close_is_dropped(self): + conn, client, sock = self._open() + self._post(client) + req = conn.receive_data(client.data_to_send())[0] + client.send_data(1, b"abc", end_stream=True) + conn.receive_data(client.data_to_send()) + conn.close() + conn.h2_conn.increment_flow_control_window = mock.Mock() + assert req.body.read() == b"abc" + conn.h2_conn.increment_flow_control_window.assert_not_called() + + def test_credit_failure_is_swallowed(self): + conn, client, sock = self._open() + self._post(client) + req = conn.receive_data(client.data_to_send())[0] + client.send_data(1, b"abc", end_stream=True) + conn.receive_data(client.data_to_send()) + conn.h2_conn.increment_flow_control_window = mock.Mock( + side_effect=h2.exceptions.StreamClosedError(1)) + assert req.body.read() == b"abc" + + def test_cleanup_survives_reset_and_write_failures(self): + conn, client, sock = self._open() + self._post(client) + conn.receive_data(client.data_to_send()) + conn.h2_conn.reset_stream = mock.Mock( + side_effect=h2.exceptions.StreamClosedError(1)) + sock.close() + conn.cleanup_stream(1) + assert 1 not in conn.streams + + def test_acknowledge_rejects_non_positive(self): + conn, client, sock = self._open() + conn.h2_conn.increment_flow_control_window = mock.Mock() + conn.acknowledge_data(1, 0) + conn.h2_conn.increment_flow_control_window.assert_not_called() + + def test_frames_for_a_stream_we_dropped_are_ignored(self): + """h2 still tracks the stream; gunicorn no longer does.""" + conn, client, sock = self._open() + self._post(client) + conn.receive_data(client.data_to_send()) + conn.streams.pop(1) + client.send_data(1, b"late", end_stream=False) + client.send_headers(1, [('x-t', '1')], end_stream=True) + assert conn.receive_data(client.data_to_send()) == [] + assert conn.is_closed is False + + def test_cleanup_write_failure_is_swallowed(self): + conn, client, sock = self._open() + self._post(client) + conn.receive_data(client.data_to_send()) + sock.close() + conn.cleanup_stream(1) + assert 1 not in conn.streams + assert conn.is_closed is True + + def test_arrival_credit_failure_is_swallowed(self): + conn, client, sock = self._open() + self._post(client) + req = conn.receive_data(client.data_to_send())[0] + conn.h2_conn.increment_flow_control_window = mock.Mock( + side_effect=h2.exceptions.ProtocolError("nope")) + client.send_data(1, b"abc", end_stream=True) + conn.receive_data(client.data_to_send()) + assert req.body.read() == b"abc" diff --git a/tests/test_http2_h2c.py b/tests/test_http2_h2c.py index ed5ab9f578..02247cd94f 100644 --- a/tests/test_http2_h2c.py +++ b/tests/test_http2_h2c.py @@ -14,6 +14,8 @@ import pytest +from test_http2_connection import frame_types + # Check if h2 is available try: import h2.connection # pylint: disable=unused-import @@ -1091,6 +1093,620 @@ async def until_answered(): asyncio.set_event_loop(None) +@pytest.mark.skipif(not H2_AVAILABLE, reason="h2 library not available") +class TestH2CASGIMaxRequests: + """The request that trips max_requests is still served. + + Streams run as tasks; stopping the receive loop the moment the + counter trips would cancel them before they ran. + """ + + def test_last_request_is_served(self): + import h2.config + import h2.connection + import h2.events + + seen = [] + + async def app(scope, receive, send): + msg = await receive() + seen.append(msg["body"]) + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + proto = None + try: + cfg = h2c_config() + worker = mock.Mock() + worker.cfg = cfg + worker.log = mock.Mock() + worker.asgi = app + worker.loop = loop + worker.nr_conns = 0 + worker.nr = 0 + worker.max_requests = 1 + worker.alive = True + from gunicorn.asgi.protocol import ASGIProtocol + proto = ASGIProtocol(worker) + transport = _FakeTransport() + proto.connection_made(transport) + + client = h2.connection.H2Connection( + config=h2.config.H2Configuration( + client_side=True, header_encoding="utf-8")) + client.initiate_connection() + client.send_headers(1, [ + (":method", "POST"), (":path", "/last"), + (":scheme", "http"), (":authority", "x"), + ], end_stream=False) + proto.data_received(client.data_to_send()) + + async def body_later(): + # The body arrives after the counter has tripped; the + # loop must still be reading for it to reach the app. + await asyncio.sleep(0.05) + client.send_data(1, b"payload", end_stream=True) + proto.data_received(client.data_to_send()) + while b"BODY" not in transport.written: + await asyncio.sleep(0.005) + loop.run_until_complete( + asyncio.wait_for(body_later(), timeout=5)) + + assert worker.nr == 1 + assert worker.alive is False + assert seen == [b"payload"] + assert not worker.log.exception.called + finally: + if proto is not None: + proto._h2c_cancel_timer() + drain_task(loop, proto._task) + loop.close() + asyncio.set_event_loop(None) + + +@pytest.mark.skipif(not H2_AVAILABLE, reason="h2 library not available") +class TestH2CASGIBodyArrivesInOneBatch: + """END_STREAM landing with frames still queued must not cut the body.""" + + def test_every_frame_reaches_the_app(self): + import h2.config + import h2.connection + + got = [] + + async def app(scope, receive, send): + while True: + msg = await receive() + got.append(msg["body"]) + if not msg.get("more_body"): + break + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + proto = None + try: + worker = mock.Mock() + worker.cfg = h2c_config() + worker.log = mock.Mock() + worker.asgi = app + worker.loop = loop + worker.nr_conns = 0 + worker.nr = 0 + worker.max_requests = 1000 + worker.alive = True + from gunicorn.asgi.protocol import ASGIProtocol + proto = ASGIProtocol(worker) + transport = _FakeTransport() + proto.connection_made(transport) + + client = h2.connection.H2Connection( + config=h2.config.H2Configuration( + client_side=True, header_encoding="utf-8")) + client.initiate_connection() + client.send_headers(1, [ + (":method", "POST"), (":path", "/batch"), + (":scheme", "http"), (":authority", "x"), + ], end_stream=False) + for i in range(3): + client.send_data(1, bytes([65 + i]) * 1000, end_stream=False) + client.send_data(1, b"", end_stream=True) + proto.data_received(client.data_to_send()) + + async def until_answered(): + while b"BODY" not in transport.written: + await asyncio.sleep(0.005) + loop.run_until_complete( + asyncio.wait_for(until_answered(), timeout=5)) + + assert b"".join(got) == b"A" * 1000 + b"B" * 1000 + b"C" * 1000 + assert not worker.log.exception.called + finally: + if proto is not None: + proto._h2c_cancel_timer() + drain_task(loop, proto._task) + loop.close() + asyncio.set_event_loop(None) + + +@pytest.mark.skipif(not H2_AVAILABLE, reason="h2 library not available") +class TestH2CASGIResetMidBody: + """A peer reset while the app awaits the body yields http.disconnect.""" + + def test_app_sees_disconnect(self): + import h2.config + import h2.connection + import h2.errors + + got = [] + + async def app(scope, receive, send): + got.append((await receive())["type"]) + got.append((await receive())["type"]) + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + proto = None + try: + worker = mock.Mock() + worker.cfg = h2c_config() + worker.log = mock.Mock() + worker.asgi = app + worker.loop = loop + worker.nr_conns = 0 + worker.nr = 0 + worker.max_requests = 1000 + worker.alive = True + from gunicorn.asgi.protocol import ASGIProtocol + proto = ASGIProtocol(worker) + transport = _FakeTransport() + proto.connection_made(transport) + + client = h2.connection.H2Connection( + config=h2.config.H2Configuration( + client_side=True, header_encoding="utf-8")) + client.initiate_connection() + client.send_headers(1, [ + (":method", "POST"), (":path", "/reset"), + (":scheme", "http"), (":authority", "x"), + ], end_stream=False) + client.send_data(1, b"first", end_stream=False) + proto.data_received(client.data_to_send()) + + async def reset_later(): + while len(got) < 1: + await asyncio.sleep(0.005) + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + proto.data_received(client.data_to_send()) + while len(got) < 2: + await asyncio.sleep(0.005) + loop.run_until_complete( + asyncio.wait_for(reset_later(), timeout=5)) + + assert got == ["http.request", "http.disconnect"] + assert not worker.log.exception.called + finally: + if proto is not None: + proto._h2c_cancel_timer() + drain_task(loop, proto._task) + loop.close() + asyncio.set_event_loop(None) + + +@pytest.mark.skipif(not H2_AVAILABLE, reason="h2 library not available") +class TestH2CASGIGoAwayWithRequest: + """Graceful GOAWAY finishes established streams (RFC 9113 6.8).""" + + def test_request_is_served_then_connection_closes(self): + import h2.config + import h2.connection + + seen = [] + + async def app(scope, receive, send): + await receive() + seen.append(scope["path"]) + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + proto = None + try: + worker = mock.Mock() + worker.cfg = h2c_config() + worker.log = mock.Mock() + worker.asgi = app + worker.loop = loop + worker.nr_conns = 0 + worker.nr = 0 + worker.max_requests = 1000 + worker.alive = True + from gunicorn.asgi.protocol import ASGIProtocol + proto = ASGIProtocol(worker) + transport = _FakeTransport() + proto.connection_made(transport) + + client = h2.connection.H2Connection( + config=h2.config.H2Configuration( + client_side=True, header_encoding="utf-8")) + client.initiate_connection() + client.send_headers(1, [ + (":method", "GET"), (":path", "/bye"), + (":scheme", "http"), (":authority", "x"), + ], end_stream=True) + client.close_connection() + proto.data_received(client.data_to_send()) + + async def until_closed(): + while not transport.closed: + await asyncio.sleep(0.005) + loop.run_until_complete( + asyncio.wait_for(until_closed(), timeout=5)) + + assert seen == ["/bye"] + assert worker.nr == 1 + assert not worker.log.exception.called + # The h2 client is closed once it has sent GOAWAY, so the + # response is checked as raw frames. + kinds = frame_types(transport.written) + assert "HeadersFrame" in kinds + assert b"BODY" in transport.written + assert kinds[-1] == "GoAwayFrame" + finally: + if proto is not None: + proto._h2c_cancel_timer() + drain_task(loop, proto._task) + loop.close() + asyncio.set_event_loop(None) + + +def _asgi_h2_session(app, max_requests=1000): + """An ASGIProtocol on a fake transport with an h2 client aimed at it.""" + import h2.config + import h2.connection + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + worker = mock.Mock() + worker.cfg = h2c_config() + worker.log = mock.Mock() + worker.asgi = app + worker.loop = loop + worker.nr_conns = 0 + worker.nr = 0 + worker.max_requests = max_requests + worker.alive = True + from gunicorn.asgi.protocol import ASGIProtocol + proto = ASGIProtocol(worker) + transport = _FakeTransport() + proto.connection_made(transport) + client = h2.connection.H2Connection( + config=h2.config.H2Configuration( + client_side=True, header_encoding="utf-8")) + client.initiate_connection() + return loop, worker, proto, transport, client + + +def _asgi_h2_teardown(loop, proto): + proto._h2c_cancel_timer() + drain_task(loop, proto._task) + loop.close() + asyncio.set_event_loop(None) + + +def _post_headers(client, stream_id, path, end_stream=False): + client.send_headers(stream_id, [ + (":method", "POST"), (":path", path), + (":scheme", "http"), (":authority", "x"), + ], end_stream=end_stream) + + +@pytest.mark.skipif(not H2_AVAILABLE, reason="h2 library not available") +class TestH2CASGIStreamingPaths: + """The remaining receive() and dispatch paths, driven end to end.""" + + def test_stream_opened_after_max_requests_is_refused(self): + import h2.events + + seen = [] + + async def app(scope, receive, send): + while (await receive()).get("more_body"): + pass + seen.append(scope["path"]) + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + + loop, worker, proto, transport, client = _asgi_h2_session(app, max_requests=1) + try: + _post_headers(client, 1, "/first") + proto.data_received(client.data_to_send()) + + async def later(): + await asyncio.sleep(0.05) + # The counter has tripped; a second stream is refused while + # the first still gets its body and its answer. + _post_headers(client, 3, "/second", end_stream=True) + client.send_data(1, b"payload", end_stream=True) + proto.data_received(client.data_to_send()) + while b"BODY" not in transport.written: + await asyncio.sleep(0.005) + await asyncio.sleep(0.05) + loop.run_until_complete(asyncio.wait_for(later(), timeout=5)) + + assert seen == ["/first"] + assert worker.nr == 1 + events = client.receive_data(transport.written) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [(r.stream_id, r.error_code) for r in resets] == \ + [(3, h2.errors.ErrorCodes.REFUSED_STREAM)] + assert not worker.log.exception.called + finally: + _asgi_h2_teardown(loop, proto) + + def test_empty_end_stream_frame_after_a_read(self): + got = [] + + async def app(scope, receive, send): + while True: + msg = await receive() + got.append((msg["body"], msg.get("more_body"))) + if not msg.get("more_body"): + break + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/late-end") + client.send_data(1, b"part", end_stream=False) + proto.data_received(client.data_to_send()) + + async def later(): + while not got: + await asyncio.sleep(0.005) + client.send_data(1, b"", end_stream=True) + proto.data_received(client.data_to_send()) + while b"BODY" not in transport.written: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(later(), timeout=5)) + + assert got == [(b"part", True), (b"", False)] + finally: + _asgi_h2_teardown(loop, proto) + + def test_receive_after_the_stream_was_reset(self): + import h2.errors + + got = [] + reset_seen = asyncio.Event() + + async def app(scope, receive, send): + got.append((await receive())["type"]) + await reset_seen.wait() + got.append((await receive())["type"]) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/reset") + client.send_data(1, b"first", end_stream=False) + proto.data_received(client.data_to_send()) + + async def later(): + while not got: + await asyncio.sleep(0.005) + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + proto.data_received(client.data_to_send()) + await asyncio.sleep(0.05) + reset_seen.set() + while len(got) < 2: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(later(), timeout=5)) + assert got == ["http.request", "http.disconnect"] + finally: + _asgi_h2_teardown(loop, proto) + + def test_receive_after_the_body_is_complete(self): + got = [] + + async def app(scope, receive, send): + got.append(await receive()) + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + got.append(await receive()) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/done", end_stream=True) + proto.data_received(client.data_to_send()) + + async def until_done(): + while len(got) < 2: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(until_done(), timeout=5)) + assert [m["type"] for m in got] == ["http.request", "http.request"] + assert got[1]["body"] == b"" and got[1]["more_body"] is False + finally: + _asgi_h2_teardown(loop, proto) + + def test_app_failure_mid_stream_answers_500(self): + import h2.events + + async def app(scope, receive, send): + await receive() + raise RuntimeError("boom") + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/boom") + client.send_data(1, b"x", end_stream=False) + proto.data_received(client.data_to_send()) + + async def until_answered(): + while not transport.written or \ + b"500" not in transport.written and \ + not [e for e in client.receive_data(b"")]: + await asyncio.sleep(0.005) + if worker.log.exception.called and \ + len(transport.written) > 0: + break + await asyncio.sleep(0.05) + loop.run_until_complete(asyncio.wait_for(until_answered(), timeout=5)) + + events = client.receive_data(transport.written) + statuses = [dict(e.headers)[":status"] for e in events + if isinstance(e, h2.events.ResponseReceived)] + assert statuses == ["500"] + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.NO_ERROR] + finally: + _asgi_h2_teardown(loop, proto) + + + def test_protocol_error_mid_body_cancels_the_app(self): + got = [] + + async def app(scope, receive, send): + got.append((await receive())["type"]) + try: + await receive() + except asyncio.CancelledError: + got.append("cancelled") + raise + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/broken") + client.send_data(1, b"first", end_stream=False) + proto.data_received(client.data_to_send()) + + async def later(): + while not got: + await asyncio.sleep(0.005) + # DATA on stream 0 is a connection error; hyperframe will + # not build one, so the frame is spelled out by hand. + proto.data_received(b"\x00\x00\x01\x00\x00\x00\x00\x00\x00x") + while len(got) < 2 or not transport.closed: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(later(), timeout=5)) + + assert got == ["http.request", "cancelled"] + assert transport.closed + assert not worker.log.exception.called + assert "GoAwayFrame" in frame_types(transport.written) + finally: + _asgi_h2_teardown(loop, proto) + + +@pytest.mark.skipif(not H2_AVAILABLE, reason="h2 library not available") +class TestH2CASGIResponseSendIsAtomic: + """The ASGI send path queues HEADERS under the writer lock. + + With the lock held by someone else, http.response.start must not + leave HEADERS sitting in h2's buffer where a GOAWAY erases them. + """ + + def _run(self, body): + started = asyncio.Event() + + async def app(scope, receive, send): + await receive() + started.set() + await send({"type": "http.response.start", "status": 200, + "headers": []}) + if body: + await send({"type": "http.response.body", "body": body}) + else: + await send({"type": "http.response.body", "body": b""}) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/atomic", end_stream=True) + proto.data_received(client.data_to_send()) + + async def race(): + while not hasattr(proto, "_h2_conn"): + await asyncio.sleep(0.005) + async with proto._h2_conn._lock(): + await started.wait() + await asyncio.sleep(0.02) # app is now blocked on the lock + client.close_connection() + proto.data_received(client.data_to_send()) + await asyncio.sleep(0.02) # loop parses the GOAWAY + while not transport.closed: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(race(), timeout=5)) + assert not worker.log.exception.called + return frame_types(transport.written) + finally: + _asgi_h2_teardown(loop, proto) + + def test_headers_survive_with_a_body(self): + kinds = self._run(b"BODY") + assert "HeadersFrame" in kinds + assert kinds.index("HeadersFrame") < kinds.index("DataFrame") + + def test_headers_survive_without_a_body(self): + kinds = self._run(b"") + assert "HeadersFrame" in kinds + + def test_reset_during_drain_is_not_an_app_error(self): + """Every send after the peer's reset stays inert, trailers included.""" + import h2.errors + + started = asyncio.Event() + + async def app(scope, receive, send): + await receive() + started.set() + await send({"type": "http.response.start", "status": 200, + "headers": []}) + for chunk in (b"one", b"two", b"three"): + await send({"type": "http.response.body", "body": chunk, + "more_body": True}) + await send({"type": "http.response.body", "body": b""}) + await send({"type": "http.response.trailers", + "headers": [(b"x-t", b"1")]}) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/cancel", end_stream=True) + proto.data_received(client.data_to_send()) + + async def race(): + while not hasattr(proto, "_h2_conn"): + await asyncio.sleep(0.005) + async with proto._h2_conn._lock(): + await started.wait() + await asyncio.sleep(0.02) + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + proto.data_received(client.data_to_send()) + await asyncio.sleep(0.02) + while proto._h2_tasks: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(race(), timeout=5)) + assert not worker.log.exception.called + finally: + _asgi_h2_teardown(loop, proto) + + def test_asgi_path_never_touches_h2_directly(self): + import inspect + from gunicorn.asgi.protocol import ASGIProtocol + src = inspect.getsource(ASGIProtocol._handle_http2_request) + assert "h2_conn.h2_conn." not in src + assert "_send_pending_data" not in src + + @pytest.mark.skipif(not H1C_AVAILABLE, reason="gunicorn_h1c not available") class TestH2CASGIUpgradeOversizedTail: """The fast parser caps remaining(); past the cap the tail is gone. diff --git a/tests/test_http2_request.py b/tests/test_http2_request.py index 75efd737c5..ada1a55f04 100644 --- a/tests/test_http2_request.py +++ b/tests/test_http2_request.py @@ -6,11 +6,12 @@ """Tests for HTTP/2 request and body classes.""" import pytest +from unittest import mock from gunicorn.config import Config from gunicorn.http.errors import InvalidHeader from gunicorn.http2.request import HTTP2Request, HTTP2Body -from gunicorn.http2.stream import HTTP2Stream +from gunicorn.http2.stream import HTTP2Stream, StreamState class MockConnection: @@ -35,11 +36,11 @@ class TestHTTP2Body: def test_init_with_data(self): body = HTTP2Body(b"Hello, World!") - assert len(body) == 13 + assert body.read() == b"Hello, World!" def test_init_empty(self): body = HTTP2Body(b"") - assert len(body) == 0 + assert body.read() == b"" def test_read_all(self): body = HTTP2Body(b"Test data") @@ -101,10 +102,6 @@ def test_iter(self): lines = list(body) assert lines == [b"Line1\n", b"Line2\n", b"Line3"] - def test_len(self): - body = HTTP2Body(b"12345") - assert len(body) == 5 - def test_close(self): body = HTTP2Body(b"test") body.close() @@ -163,10 +160,21 @@ def _make_stream(self, headers, body=b""): stream = HTTP2Stream(stream_id=1, connection=conn) stream.receive_headers(headers, end_stream=(len(body) == 0)) if body: - stream.request_body.write(body) - stream.request_complete = True + stream.state = StreamState.OPEN + stream.receive_data(body, end_stream=True) return stream + def test_bodyless_request_reads_eof_without_touching_the_socket(self): + """An upgraded or END_STREAM'd GET has no body to pull in.""" + conn = MockConnection() + conn.pump = mock.Mock(side_effect=AssertionError("socket read")) + conn.is_closed = False + stream = HTTP2Stream(stream_id=1, connection=conn) + stream.receive_headers([(":method", "GET"), (":path", "/")], end_stream=True) + req = HTTP2Request(stream, Config(), ("127.0.0.1", 1)) + assert req.body.read() == b"" + conn.pump.assert_not_called() + def test_basic_get_request(self): stream = self._make_stream([ (':method', 'GET'), @@ -805,3 +813,86 @@ def test_expect_continue_never_set_on_http2(self): def test_authority_precedence_after_policy(self): req = self._request(self._base(host='attacker.example'), self.UNTRUSTED) assert [v for n, v in req.headers if n == 'HOST'] == ['example.com'] + + +class TestHTTP2BodyStreaming: + """wsgi.input over a stream: sizes, lines, EOF and closure.""" + + def _body(self, frames, pump=True): + """A body whose connection hands over one frame per pump().""" + conn = MockConnection() + conn.is_closed = False + stream = HTTP2Stream(stream_id=1, connection=conn) + stream.receive_headers([(":method", "POST"), (":path", "/")]) + pending = list(frames) + + def do_pump(): + data = pending.pop(0) + stream.receive_data(data, end_stream=not pending) + if pump: + conn.pump = do_pump + return HTTP2Body(stream), stream, conn + + def test_read_pulls_frames_on_demand(self): + body, stream, conn = self._body([b"abc", b"def", b"g"]) + assert body.read(2) == b"ab" + assert stream.body_size == 3 + assert body.read(3) == b"cde" + assert stream.body_size == 6 + assert body.read() == b"fg" + assert body.read() == b"" + assert body.read(1) == b"" + + def test_read_zero_and_negative(self): + body, stream, conn = self._body([b"abc"]) + assert body.read(0) == b"" + assert stream.body_size == 0 + assert body.read(-1) == b"abc" + + def test_readline_with_size(self): + body, stream, conn = self._body([b"alpha\nbe", b"ta\ngamma"]) + assert body.readline(3) == b"alp" + assert body.readline() == b"ha\n" + assert body.readline(-1) == b"beta\n" + assert body.readline(0) == b"" + assert body.readline(100) == b"gamma" + assert body.readline() == b"" + + def test_readline_size_caps_a_found_line(self): + body, stream, conn = self._body([b"ab\ncd\n"]) + assert body.readline(2) == b"ab" + assert body.readline(2) == b"\n" + assert body.readlines() == [b"cd\n"] + + def test_readlines_hint(self): + body, stream, conn = self._body([b"1\n2\n3\n4\n"]) + assert body.readlines(3) == [b"1\n", b"2\n"] + assert list(body) == [b"3\n", b"4\n"] + + def test_close_then_read_raises(self): + body, stream, conn = self._body([b"abc"]) + assert body.read(1) == b"a" + body.close() + with pytest.raises(ValueError): + body.read() + with pytest.raises(ValueError): + body.readline() + + def test_connection_without_pump_reads_what_is_held(self): + """The ASGI path never reads through HTTP2Body; it must not block.""" + body, stream, conn = self._body([b"abc"], pump=False) + stream.receive_data(b"abc") + assert body.read() == b"abc" + assert body.read() == b"" + + def test_closed_connection_raises(self): + from gunicorn.http2.errors import HTTP2StreamError + body, stream, conn = self._body([b"abc"]) + conn.is_closed = True + with pytest.raises(HTTP2StreamError): + body.read() + + def test_readline_size_reached_without_newline(self): + body, stream, conn = self._body([b"abc", b"def"]) + assert body.readline(2) == b"ab" + assert body.readline(10) == b"cdef" diff --git a/tests/test_http2_stream.py b/tests/test_http2_stream.py index 12f2a20855..22e9b0bd64 100644 --- a/tests/test_http2_stream.py +++ b/tests/test_http2_stream.py @@ -5,7 +5,9 @@ """Tests for HTTP/2 stream state management.""" +import asyncio import pytest +from unittest import mock from gunicorn.http2.stream import HTTP2Stream, StreamState from gunicorn.http2.errors import HTTP2StreamError @@ -222,7 +224,7 @@ def test_receive_data_in_open_state(self): stream.receive_data(b"Hello, World!", end_stream=False) - assert stream.request_body.getvalue() == b"Hello, World!" + assert stream.get_request_body() == b"Hello, World!" assert stream.request_complete is False def test_receive_data_with_end_stream(self): @@ -244,7 +246,8 @@ def test_receive_data_accumulates(self): stream.receive_data(b"Part2") stream.receive_data(b"Part3", end_stream=True) - assert stream.request_body.getvalue() == b"Part1Part2Part3" + assert stream.get_request_body() == b"Part1Part2Part3" + assert stream.body_size == 15 def test_receive_data_in_half_closed_local(self): conn = MockConnection() @@ -252,7 +255,7 @@ def test_receive_data_in_half_closed_local(self): stream.state = StreamState.HALF_CLOSED_LOCAL stream.receive_data(b"data", end_stream=False) - assert stream.request_body.getvalue() == b"data" + assert stream.get_request_body() == b"data" def test_receive_data_in_invalid_state(self): conn = MockConnection() @@ -477,6 +480,27 @@ def test_get_body_after_data(self): assert stream.get_request_body() == b"Test body content" + def test_single_copy_is_kept(self): + """One DATA frame is handed back as-is: no join, no second buffer.""" + conn = MockConnection() + stream = HTTP2Stream(stream_id=1, connection=conn) + stream.state = StreamState.OPEN + payload = b"x" * 1024 + stream.receive_data(payload, end_stream=True) + + assert stream.get_request_body() is payload + assert stream.body_size == 1024 + assert not hasattr(stream, "request_body") + + + def test_end_stream_on_headers_completes_the_body(self): + conn = MockConnection() + stream = HTTP2Stream(stream_id=1, connection=conn) + stream.receive_headers([(":method", "GET"), (":path", "/")], end_stream=True) + assert stream.body_complete is True + assert stream.get_request_body() == b"" + + class TestReadBodyChunk: """Test read_body_chunk method.""" @@ -825,3 +849,55 @@ def test_send_trailers_invalid_state_raises(self): # Stream is IDLE, cannot send trailers with pytest.raises(HTTP2StreamError): stream.send_trailers([('trailer', 'value')]) + + +class TestBodyEvents: + """Completion and reset wake a waiting reader.""" + + @pytest.mark.asyncio + async def test_end_stream_on_headers_wakes_reader(self): + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.receive_headers([(":method", "POST"), (":path", "/")]) + reader = asyncio.get_running_loop().create_task(stream.read_body_chunk()) + await asyncio.sleep(0) + stream.receive_headers([("x-trailer", "1")], end_stream=True) + assert await asyncio.wait_for(reader, timeout=1) is None + + @pytest.mark.asyncio + async def test_end_stream_on_data_wakes_reader(self): + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.state = StreamState.OPEN + reader = asyncio.get_running_loop().create_task(stream.read_body_chunk()) + await asyncio.sleep(0) + stream.receive_data(b"", end_stream=True) + assert await asyncio.wait_for(reader, timeout=1) is None + + @pytest.mark.asyncio + async def test_reset_wakes_reader_with_error(self): + from gunicorn.http2.errors import HTTP2StreamError + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.state = StreamState.OPEN + reader = asyncio.get_running_loop().create_task(stream.read_body_chunk()) + await asyncio.sleep(0) + stream.reset() + with pytest.raises(HTTP2StreamError): + await asyncio.wait_for(reader, timeout=1) + + def test_empty_frame_is_not_credited(self): + conn = MockConnection() + conn.acknowledge_data = mock.Mock() + stream = HTTP2Stream(stream_id=1, connection=conn) + stream.state = StreamState.OPEN + stream.receive_data(b"", end_stream=False) + assert stream.pop_chunk() is None + stream.receive_data(b"abc") + assert stream.pop_chunk() == b"abc" + conn.acknowledge_data.assert_called_once_with(1, 3) + assert stream.acked_size == 3 + + def test_pop_without_connection_ack_support(self): + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.state = StreamState.OPEN + stream.receive_data(b"abc") + assert stream.pop_chunk() == b"abc" + assert stream.unacked_size == 0 From 2d5f01f0883a7d9dae79d3280a76cced9e16c894 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:03:56 +0200 Subject: [PATCH 02/13] http2: honour a graceful GOAWAY without touching h2 internals A shared H2Connection subclass keeps the state open on GOAWAY(NO_ERROR) so established streams finish and later ones are refused. --- gunicorn/http2/async_connection.py | 101 ++++++++++++++++-------- gunicorn/http2/connection.py | 113 +++++++++++++++++++-------- gunicorn/http2/h2conn.py | 50 ++++++++++++ gunicorn/http2/request.py | 2 +- gunicorn/http2/stream.py | 25 ++++++ tests/test_http2_async_connection.py | 66 ++++++++++++++++ tests/test_http2_connection.py | 66 ++++++++++++++++ tests/test_http2_request.py | 2 +- 8 files changed, 355 insertions(+), 70 deletions(-) create mode 100644 gunicorn/http2/h2conn.py diff --git a/gunicorn/http2/async_connection.py b/gunicorn/http2/async_connection.py index 6ad1f326ed..efe26bb48b 100644 --- a/gunicorn/http2/async_connection.py +++ b/gunicorn/http2/async_connection.py @@ -18,6 +18,7 @@ HTTP2NotAvailable, HTTP2ErrorCode, ) from .stream import HTTP2Stream, StreamState +from .h2conn import server_connection_class from .request import HTTP2Request @@ -88,6 +89,7 @@ def __init__(self, cfg, reader, writer, client_addr): self._write_lock = None # Peer sent a graceful GOAWAY: finish open streams, take no new ones self.draining = False + self.peer_last_stream_id = None # Queue of completed requests for the worker self._request_queue = asyncio.Queue() @@ -103,7 +105,7 @@ def __init__(self, cfg, reader, writer, client_addr): client_side=False, header_encoding='utf-8', ) - self.h2_conn = _h2.H2Connection(config=config) + self.h2_conn = server_connection_class()(config=config) # Connection state self._closed = False @@ -210,6 +212,8 @@ async def receive_data(self, timeout=None): if not data: # Connection closed by peer self._closed = True + self._abort_all_streams() + self._signal_window() return [] # Feed data to h2 @@ -246,7 +250,6 @@ async def receive_data(self, timeout=None): # Process events, oldest first: anything set aside during a # flow-control wait arrived before this batch. completed_requests = [] - was_draining = self.draining if self._deferred_events: events = list(self._deferred_events) + list(events) self._deferred_events.clear() @@ -255,17 +258,6 @@ async def receive_data(self, timeout=None): if request is not None: completed_requests.append(request) - if was_draining: - # Streams opened after the peer's GOAWAY are refused. - for request in completed_requests: - stream_id = request.stream.stream_id - self.streams.pop(stream_id, None) - try: - self.h2_conn.reset_stream(stream_id, error_code=HTTP2ErrorCode.REFUSED_STREAM) - except _h2_exceptions.ProtocolError: - pass - completed_requests = [] - # Send any pending data (WINDOW_UPDATE, etc.) await self._send_pending_data() @@ -318,6 +310,11 @@ def _handle_request_received(self, event): stream_id = event.stream_id headers = event.headers + if self.draining and stream_id > self.peer_last_stream_id: + # Opened after the peer's GOAWAY: it said it would not process it. + self._reset_quietly(stream_id, HTTP2ErrorCode.REFUSED_STREAM) + return None + # Create new stream stream = HTTP2Stream(stream_id, self) self.streams[stream_id] = stream @@ -404,21 +401,37 @@ def _handle_stream_reset(self, event): def _handle_connection_terminated(self, event): """Handle ConnectionTerminated event (GOAWAY frame). - A graceful GOAWAY (NO_ERROR) only forbids new streams; the ones - already established finish (RFC 9113 section 6.8). h2 closes its - connection state on receipt and would refuse to send anything, - so its state is put back to open while those streams drain, and - the connection closes once they are done. Any other error code - closes at once. - - Args: - event: ConnectionTerminated event + A graceful GOAWAY (NO_ERROR) with streams in flight puts the + connection into draining: the established streams finish, later + ones are refused, and the connection closes once they are done + (RFC 9113 section 6.8). The h2 subclass has already kept its own + state open for that case. Any other GOAWAY closes at once. """ - if event.error_code == HTTP2ErrorCode.NO_ERROR and self.streams: + if (event.error_code == HTTP2ErrorCode.NO_ERROR + and self.h2_conn.peer_goaway_last_stream_id is not None): self.draining = True - self.h2_conn.state_machine.state = _h2.ConnectionState.SERVER_OPEN + self.peer_last_stream_id = event.last_stream_id return self._closed = True + self._abort_all_streams() + + def _abort_all_streams(self): + """The connection is gone: wake every stream still waiting on it.""" + for stream in list(self.streams.values()): + stream.signal_disconnect() + + def abort_streams_nowait(self): + """Called by the protocol on connection loss.""" + self._closed = True + self._abort_all_streams() + self._signal_window() + + def _reset_quietly(self, stream_id, error_code): + """Queue RST_STREAM for a stream h2 knows about, ignoring a closed one.""" + try: + self.h2_conn.reset_stream(stream_id, error_code=error_code) + except _h2_exceptions.ProtocolError: + pass def _handle_trailers_received(self, event): """Handle TrailersReceived event.""" @@ -680,12 +693,22 @@ async def send_error(self, stream_id, status_code, message=None): async def reset_stream(self, stream_id, error_code=0x8): """Reset a stream with RST_STREAM.""" + await self.abort_stream(stream_id, error_code) + + async def abort_stream(self, stream_id, error_code): + """Reset a stream, drop it, and tell the peer. + + Safe on a stream h2 already closed and on a dead socket. + """ stream = self.streams.get(stream_id) if stream is not None: stream.reset(error_code) - - await self._send(lambda: self.h2_conn.reset_stream( - stream_id, error_code=error_code)) + try: + await self._send(lambda: self._reset_quietly(stream_id, error_code)) + except HTTP2ConnectionError: + pass + if stream is not None: + self.cleanup_stream(stream_id) async def close(self, error_code=0x0, last_stream_id=None): """Close the connection gracefully with GOAWAY.""" @@ -698,9 +721,12 @@ async def close(self, error_code=0x0, last_stream_id=None): last_stream_id = max(self.streams.keys()) if self.streams else 0 try: - await self._send(lambda: self.h2_conn.close_connection(error_code=error_code)) + await self._send(lambda: self.h2_conn.close_connection( + error_code=error_code, last_stream_id=last_stream_id)) except Exception: pass + self._abort_all_streams() + self._signal_window() # Not awaited: the writer's protocol is a stand-in that never # sees connection_lost, so wait_closed() would never return. @@ -749,18 +775,25 @@ def is_closed(self): def cleanup_stream(self, stream_id): """Remove a stream after processing is complete. - A body the application did not finish reading is cut off with + A response the application never finished is cut off with + RST_STREAM(INTERNAL_ERROR); a body it did not finish reading with RST_STREAM(NO_ERROR) (RFC 9113 section 8.1). """ stream = self.streams.pop(stream_id, None) if stream is None: return - if not stream.body_complete and stream.state is not StreamState.CLOSED: + if stream.state is StreamState.CLOSED: + pass + elif not stream.response_complete: + # The application never finished its response: the peer must + # not wait for one. + stream.reset(HTTP2ErrorCode.INTERNAL_ERROR) + self._reset_quietly(stream_id, HTTP2ErrorCode.INTERNAL_ERROR) + elif not stream.body_complete: stream.reset(HTTP2ErrorCode.NO_ERROR) - try: - self.h2_conn.reset_stream(stream_id, error_code=HTTP2ErrorCode.NO_ERROR) - except _h2_exceptions.ProtocolError: - pass + self._reset_quietly(stream_id, HTTP2ErrorCode.NO_ERROR) + # A listener still blocked in receive() after the app returned + stream.signal_disconnect() if not self._closed: self._write_pending_nowait() diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index fd1c1fa4f7..d949bf0f94 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -18,6 +18,7 @@ HTTP2NotAvailable, HTTP2ErrorCode, ) from .stream import HTTP2Stream, StreamState +from .h2conn import server_connection_class from .request import HTTP2Request @@ -86,6 +87,7 @@ def __init__(self, cfg, sock, client_addr): self.pending_requests = collections.deque() # Peer sent a graceful GOAWAY: finish open streams, take no new ones self.draining = False + self.peer_last_stream_id = None # Connection settings from config self.initial_window_size = cfg.http2_initial_window_size @@ -98,7 +100,7 @@ def __init__(self, cfg, sock, client_addr): client_side=False, header_encoding='utf-8', ) - self.h2_conn = _h2.H2Connection(config=config) + self.h2_conn = server_connection_class()(config=config) # Read buffer for partial frames self._read_buffer = BytesIO() @@ -202,16 +204,23 @@ def receive_data(self, data=None): return pending return self._read_and_process(data) - def pump(self): + def pump(self, stream_id=None): """Read once from the socket while a request body is being consumed. Frames for the stream being read land in its buffer; any request that arrives meanwhile is queued for the worker's next receive_data() call rather than dropped. + + Args: + stream_id: The stream whose body is being read. """ - self.pending_requests.extend(self._read_and_process(None)) + self.pending_requests.extend(self._read_and_process(None, stream_id)) - def _read_and_process(self, data): + def _read_and_process(self, data, waiting_stream_id=None): + if data is None and self._deferred_events: + # Events set aside during a send-credit wait arrived before + # anything still on the socket; hand them out without reading. + return self._process_events(()) if data is None: try: data = self.sock.recv(self.READ_BUFFER_SIZE) @@ -221,6 +230,7 @@ def _read_and_process(self, data): if not data: # Connection closed by peer self._closed = True + self._abort_all_streams() return [] # Feed data to h2 @@ -254,10 +264,15 @@ def _read_and_process(self, data): self.close(error_code=HTTP2ErrorCode.PROTOCOL_ERROR) raise HTTP2ProtocolError(str(e)) - # Process events, oldest first: anything set aside during a - # flow-control wait arrived before this batch. + return self._process_events(events) + + def _process_events(self, events): + """Run h2 events, oldest first, and return the new requests. + + Anything set aside during a flow-control wait arrived before + this batch, so it goes first. + """ completed_requests = [] - was_draining = self.draining if self._deferred_events: events = list(self._deferred_events) + list(events) self._deferred_events.clear() @@ -266,12 +281,6 @@ def _read_and_process(self, data): if request is not None: completed_requests.append(request) - if was_draining: - # Streams opened after the peer's GOAWAY are refused. - for request in completed_requests: - self.reset_stream(request.stream.stream_id, HTTP2ErrorCode.REFUSED_STREAM) - self.streams.pop(request.stream.stream_id, None) - completed_requests = [] # Send any pending data (WINDOW_UPDATE, etc.) self._send_pending_data() @@ -324,6 +333,11 @@ def _handle_request_received(self, event): stream_id = event.stream_id headers = event.headers + if self.draining and stream_id > self.peer_last_stream_id: + # Opened after the peer's GOAWAY: it said it would not process it. + self._reset_quietly(stream_id, HTTP2ErrorCode.REFUSED_STREAM) + return None + # Create new stream stream = HTTP2Stream(stream_id, self) self.streams[stream_id] = stream @@ -416,22 +430,34 @@ def _handle_stream_reset(self, event): def _handle_connection_terminated(self, event): """Handle ConnectionTerminated event (GOAWAY frame). - A graceful GOAWAY (NO_ERROR) only forbids new streams; the ones - already established finish (RFC 9113 section 6.8). h2 closes its - connection state on receipt and would refuse to send anything, - so its state is put back to open while those streams drain, and - the connection closes once they are done. Any other error code - closes at once. + A graceful GOAWAY (NO_ERROR) with streams in flight puts the + connection into draining: the established streams finish, later + ones are refused, and the connection closes once they are done + (RFC 9113 section 6.8). The h2 subclass has already kept its own + state open for that case. Any other GOAWAY closes at once. Args: event: ConnectionTerminated event """ - if event.error_code == HTTP2ErrorCode.NO_ERROR and self.streams: + if (event.error_code == HTTP2ErrorCode.NO_ERROR + and self.h2_conn.peer_goaway_last_stream_id is not None): self.draining = True - self.h2_conn.state_machine.state = _h2.ConnectionState.SERVER_OPEN + self.peer_last_stream_id = event.last_stream_id return self._closed = True - # Could log event.error_code and event.additional_data + self._abort_all_streams() + + def _abort_all_streams(self): + """The connection is gone: wake every stream still waiting on it.""" + for stream in list(self.streams.values()): + stream.signal_disconnect() + + def _reset_quietly(self, stream_id, error_code): + """Queue RST_STREAM for a stream h2 knows about, ignoring a closed one.""" + try: + self.h2_conn.reset_stream(stream_id, error_code=error_code) + except _h2_exceptions.ProtocolError: + pass def _handle_trailers_received(self, event): """Handle TrailersReceived event. @@ -753,12 +779,24 @@ def reset_stream(self, stream_id, error_code=0x8): stream_id: The stream ID to reset error_code: HTTP/2 error code (default: CANCEL) """ + self.abort_stream(stream_id, error_code) + + def abort_stream(self, stream_id, error_code): + """Reset a stream, drop it, and tell the peer. + + Safe on a stream h2 already closed and on a dead socket. + """ stream = self.streams.get(stream_id) if stream is not None: stream.reset(error_code) - - self.h2_conn.reset_stream(stream_id, error_code=error_code) - self._send_pending_data() + self._reset_quietly(stream_id, error_code) + if stream is not None: + self.cleanup_stream(stream_id) + elif not self._closed: + try: + self._send_pending_data() + except HTTP2ConnectionError: + pass def close(self, error_code=0x0, last_stream_id=None): """Close the connection gracefully with GOAWAY. @@ -777,10 +815,12 @@ def close(self, error_code=0x0, last_stream_id=None): last_stream_id = max(self.streams.keys()) if self.streams else 0 try: - self.h2_conn.close_connection(error_code=error_code) + self.h2_conn.close_connection(error_code=error_code, + last_stream_id=last_stream_id) self._send_pending_data() except Exception: pass # Best effort + self._abort_all_streams() def _send_pending_data(self): """Send any pending data from h2 to the socket.""" @@ -800,9 +840,10 @@ def is_closed(self): def cleanup_stream(self, stream_id): """Remove a stream after processing is complete. - A body the application did not finish reading is cut off with - RST_STREAM(NO_ERROR): the response is complete and the peer is - only asked to stop sending (RFC 9113 section 8.1). + A response the application never finished is cut off with + RST_STREAM(INTERNAL_ERROR). A body it did not finish reading is + cut off with RST_STREAM(NO_ERROR): the response is complete and + the peer is only asked to stop sending (RFC 9113 section 8.1). Args: stream_id: The stream ID to clean up @@ -810,12 +851,16 @@ def cleanup_stream(self, stream_id): stream = self.streams.pop(stream_id, None) if stream is None: return - if not stream.body_complete and stream.state is not StreamState.CLOSED: + if stream.state is StreamState.CLOSED: + pass + elif not stream.response_complete: + # The application never finished its response: the peer must + # not wait for one. + stream.reset(HTTP2ErrorCode.INTERNAL_ERROR) + self._reset_quietly(stream_id, HTTP2ErrorCode.INTERNAL_ERROR) + elif not stream.body_complete: stream.reset(HTTP2ErrorCode.NO_ERROR) - try: - self.h2_conn.reset_stream(stream_id, error_code=HTTP2ErrorCode.NO_ERROR) - except _h2_exceptions.ProtocolError: - pass + self._reset_quietly(stream_id, HTTP2ErrorCode.NO_ERROR) if self.draining and not self.streams and not self.pending_requests: # The peer's GOAWAY is honoured once the last established # stream is done. diff --git a/gunicorn/http2/h2conn.py b/gunicorn/http2/h2conn.py new file mode 100644 index 0000000000..f78c2ea419 --- /dev/null +++ b/gunicorn/http2/h2conn.py @@ -0,0 +1,50 @@ +# -*- coding: utf-8 - +# +# This file is part of gunicorn released under the MIT license. +# See the NOTICE for more information. + +"""The h2 connection class shared by the sync and asyncio HTTP/2 paths.""" + +from .errors import HTTP2NotAvailable + +_cls = None + + +def server_connection_class(): + """Return the H2Connection subclass gunicorn uses on the server side. + + h2 answers a peer GOAWAY by closing its own connection state, which + would refuse every later send. RFC 9113 section 6.8 says a graceful + GOAWAY (NO_ERROR) only forbids new streams and the established ones + finish, so for that case the subclass reports the event and leaves + the state machine alone. Frames that follow the GOAWAY in the same + read are then still processed. + """ + global _cls # pylint: disable=global-statement + if _cls is not None: + return _cls + try: + import h2.connection + import h2.errors + import h2.events + except ImportError: + raise HTTP2NotAvailable() + + class ServerH2Connection(h2.connection.H2Connection): + #: last_stream_id from a graceful peer GOAWAY that left the + #: connection open, None otherwise. + peer_goaway_last_stream_id = None + + def _receive_goaway_frame(self, frame): + if (not self.config.client_side and frame.error_code == 0 + and self.open_inbound_streams): + self.peer_goaway_last_stream_id = frame.last_stream_id + event = h2.events.ConnectionTerminated() + event.error_code = h2.errors.ErrorCodes.NO_ERROR + event.last_stream_id = frame.last_stream_id + event.additional_data = frame.additional_data or None + return [], [event] + return super()._receive_goaway_frame(frame) + + _cls = ServerH2Connection + return _cls diff --git a/gunicorn/http2/request.py b/gunicorn/http2/request.py index 6d1fa38790..9f2e108b10 100644 --- a/gunicorn/http2/request.py +++ b/gunicorn/http2/request.py @@ -65,7 +65,7 @@ def _next_chunk(self): # read_body_chunk() instead. self._eof = True return None - pump() + pump(stream.stream_id) def _fill(self, size): """Hold at least ``size`` bytes, or everything up to the end.""" diff --git a/gunicorn/http2/stream.py b/gunicorn/http2/stream.py index acbd8415dc..383e742b34 100644 --- a/gunicorn/http2/stream.py +++ b/gunicorn/http2/stream.py @@ -80,6 +80,11 @@ def __init__(self, stream_id, connection): self._body_event = None # Lazy-init asyncio.Event self._body_complete = False + # Set once the peer reset the stream or the connection is gone; + # wait_disconnect() lets an ASGI receive() block on it. + self.disconnected = False + self._disconnect_waiter = None + @property def is_client_stream(self): """Check if this is a client-initiated stream (odd stream ID).""" @@ -248,10 +253,30 @@ def reset(self, error_code=0x8): self.state = StreamState.CLOSED self.response_complete = True self.request_complete = True + self.signal_disconnect() + + def signal_disconnect(self): + """Mark the peer as gone and wake anything waiting on it.""" + self.disconnected = True + waiter = self._disconnect_waiter + if waiter is not None and not waiter.done(): + waiter.set_result(None) # Wake a reader waiting on the body; it finds the stream closed. if self._body_event: self._body_event.set() + async def wait_disconnect(self): + """Block until the peer resets the stream or the connection ends.""" + import asyncio + + if self.disconnected: + return + self._disconnect_waiter = asyncio.get_running_loop().create_future() + try: + await self._disconnect_waiter + finally: + self._disconnect_waiter = None + def close(self): """Close this stream normally.""" self.state = StreamState.CLOSED diff --git a/tests/test_http2_async_connection.py b/tests/test_http2_async_connection.py index cc71a8a8c9..a420694663 100644 --- a/tests/test_http2_async_connection.py +++ b/tests/test_http2_async_connection.py @@ -1148,6 +1148,7 @@ async def test_unread_body_is_reset_with_no_error_on_cleanup(self): client.receive_data(writer.get_written_data()) writer.clear() + assert await conn.send_response(1, 200, [], b"done") is True conn.cleanup_stream(1) assert 1 not in conn.streams @@ -1398,3 +1399,68 @@ async def action(conn): await conn.send_data(1, b"x" * 20000, end_stream=False) return await conn.send_trailers(1, [('x-t', '1')]) assert await self._race(action) in (True, False) + + +class TestAsyncGracefulGoAway: + """GOAWAY(NO_ERROR) drains established streams wherever it lands in a read.""" + + @pytest.mark.asyncio + async def test_goaway_followed_by_data_in_one_read(self): + from hyperframe.frame import DataFrame, GoAwayFrame + from gunicorn.http2.async_connection import AsyncHTTP2Connection + from test_http2_connection import frame_types + + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + client.send_headers(1, [ + (':method', 'POST'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=False) + reader.set_data(client.data_to_send()) + requests = await conn.receive_data() + stream = requests[0].stream + + goaway = GoAwayFrame(0, last_stream_id=1, error_code=0) + data = DataFrame(1, data=b"body") + data.flags.add("END_STREAM") + reader.set_data(goaway.serialize() + data.serialize()) + await conn.receive_data() + + assert conn.draining is True + assert conn.is_closed is False + assert await stream.read_body_chunk() == b"body" + writer.clear() + assert await conn.send_response(1, 200, [], b"hello") is True + assert "HeadersFrame" in frame_types(writer.get_written_data()) + + @pytest.mark.asyncio + async def test_unfinished_response_is_reset_with_internal_error(self): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + writer.clear() + + conn.cleanup_stream(1) + + events = client.receive_data(writer.get_written_data()) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index c1af71472f..ce2dae8faa 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -1415,6 +1415,7 @@ def test_unread_body_is_reset_with_no_error_on_cleanup(self): conn.receive_data(client.data_to_send()) before = self._drain(client, sock, before) + assert conn.send_response(1, 200, [], b"done") is True conn.cleanup_stream(1) assert 1 not in conn.streams @@ -1431,11 +1432,25 @@ def test_complete_body_cleanup_sends_no_reset(self): client.send_data(1, b"a" * 10, end_stream=True) conn.receive_data(client.data_to_send()) assert requests[0].body.read() == b"a" * 10 + assert conn.send_response(1, 200, [], b"done") is True before = len(sock.get_sent_data()) conn.cleanup_stream(1) events = client.receive_data(sock.get_sent_data()[before:]) assert not [e for e in events if isinstance(e, h2.events.StreamReset)] + def test_unfinished_response_is_reset_with_internal_error_on_cleanup(self): + """The app returned without completing its response.""" + conn, client, sock, requests = self._open_post() + client.send_data(1, b"a" * 10, end_stream=True) + conn.receive_data(client.data_to_send()) + assert requests[0].body.read() == b"a" * 10 + before = len(sock.get_sent_data()) + conn.cleanup_stream(1) + events = client.receive_data(sock.get_sent_data()[before:]) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] + assert 1 not in conn.streams + def test_requests_arriving_during_a_body_read_are_queued(self): conn, client, sock, requests = self._open_post() req = requests[0] @@ -1591,6 +1606,57 @@ def test_stream_opened_after_goaway_is_refused(self): assert 1 in conn.streams assert "RstStreamFrame" in frame_types(sock.get_sent_data()[before:]) + def test_goaway_followed_by_data_in_one_read(self): + """The rest of the client's write lands after its GOAWAY.""" + from hyperframe.frame import DataFrame, GoAwayFrame + conn, client, sock = self._open() + client.send_headers( + stream_id=1, + headers=[ + (':method', 'POST'), + (':path', '/upload'), + (':scheme', 'https'), + (':authority', 'localhost'), + ], + end_stream=False, + ) + requests = conn.receive_data(client.data_to_send()) + req = requests[0] + + goaway = GoAwayFrame(0, last_stream_id=1, error_code=0) + data = DataFrame(1, data=b"body") + data.flags.add("END_STREAM") + conn.receive_data(goaway.serialize() + data.serialize()) + + assert conn.draining is True + assert conn.is_closed is False + assert conn.h2_conn.peer_goaway_last_stream_id == 1 + assert req.body.read() == b"body" + before = len(sock.get_sent_data()) + assert conn.send_response(1, 200, [], b"hello") is True + conn.cleanup_stream(1) + assert conn.is_closed is True + kinds = frame_types(sock.get_sent_data()[before:]) + assert "HeadersFrame" in kinds + assert kinds[-1] == "GoAwayFrame" + + def test_stream_at_or_below_last_stream_id_is_served(self): + """A stream the peer said it would still process is served.""" + from hyperframe.frame import GoAwayFrame, HeadersFrame + conn, client, sock = self._open() + self._get(client, 1) + conn.receive_data(client.data_to_send()) + conn.receive_data(GoAwayFrame(0, last_stream_id=3, error_code=0).serialize()) + assert conn.draining is True + + f = HeadersFrame(3, data=client.encoder.encode([ + (':method', 'GET'), (':path', '/late'), + (':scheme', 'https'), (':authority', 'localhost')])) + f.flags.add('END_HEADERS') + f.flags.add('END_STREAM') + requests = conn.receive_data(f.serialize()) + assert [r.stream.stream_id for r in requests] == [3] + def test_goaway_with_error_closes_at_once(self): conn, client, sock = self._open() self._get(client, 1) diff --git a/tests/test_http2_request.py b/tests/test_http2_request.py index ada1a55f04..eb988040f9 100644 --- a/tests/test_http2_request.py +++ b/tests/test_http2_request.py @@ -826,7 +826,7 @@ def _body(self, frames, pump=True): stream.receive_headers([(":method", "POST"), (":path", "/")]) pending = list(frames) - def do_pump(): + def do_pump(stream_id=None): data = pending.pop(0) stream.receive_data(data, end_stream=not pending) if pump: From a81c7d66fdbeeae72b1ab1f6f9c55a92872cb192 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:18:08 +0200 Subject: [PATCH 03/13] http2: bound the send-credit wait and stop a response the peer abandoned Wait at most cfg.timeout, handle frames read meanwhile in order, reset the stream with CANCEL on expiry and make the WSGI response raise. --- gunicorn/http2/async_connection.py | 43 ++++++--- gunicorn/http2/connection.py | 121 ++++++++++++++----------- gunicorn/http2/response.py | 26 ++++-- gunicorn/workers/base_async.py | 6 +- gunicorn/workers/gthread.py | 8 +- tests/test_early_hints.py | 2 + tests/test_http2_async_connection.py | 75 ++++++++++++++++ tests/test_http2_connection.py | 130 +++++++++++++++++++++------ tests/test_http2_response.py | 39 ++++++++ 9 files changed, 342 insertions(+), 108 deletions(-) diff --git a/gunicorn/http2/async_connection.py b/gunicorn/http2/async_connection.py index efe26bb48b..5bad0ca04d 100644 --- a/gunicorn/http2/async_connection.py +++ b/gunicorn/http2/async_connection.py @@ -397,6 +397,8 @@ def _handle_stream_reset(self, event): if stream is not None: stream.reset(event.error_code) + # A sender waiting for credit on this stream must re-check. + self._signal_window() def _handle_connection_terminated(self, event): """Handle ConnectionTerminated event (GOAWAY frame). @@ -562,34 +564,43 @@ def _signal_window(self): self._window_event.set() async def _wait_for_flow_control_window(self, stream_id): - """Wait for flow control window to become positive. + """Wait for the stream's send window to become positive. The receive loop owns the reader; it processes the peer's - WINDOW_UPDATE and SETTINGS frames and signals here. Waiting on - that signal instead of reading keeps two tasks off one reader. + WINDOW_UPDATE, SETTINGS, RST_STREAM and GOAWAY frames and signals + here. The wait is bounded by ``cfg.timeout`` (0 means no limit). Returns: - int: Available window size, or -1 if waiting failed + int: Available window size; 0 if the timeout passed first; + -1 if the stream or the connection is gone. """ if self._window_event is None: self._window_event = asyncio.Event() - max_wait_attempts = 50 # ~5 seconds at 100ms per attempt - for _ in range(max_wait_attempts): - available = self.h2_conn.local_flow_control_window(stream_id) - if available > 0: - return available + loop = asyncio.get_running_loop() + timeout = self.cfg.timeout or None + deadline = None if timeout is None else loop.time() + timeout + while True: if self._closed: return -1 stream = self.streams.get(stream_id) if stream is not None and stream.state is StreamState.CLOSED: return -1 + try: + available = self.h2_conn.local_flow_control_window(stream_id) + except _h2_exceptions.ProtocolError: + return -1 + if available > 0: + return available + remaining = None + if deadline is not None: + remaining = deadline - loop.time() + if remaining <= 0: + return 0 self._window_event.clear() try: - await asyncio.wait_for(self._window_event.wait(), timeout=0.1) + await asyncio.wait_for(self._window_event.wait(), timeout=remaining) except asyncio.TimeoutError: - continue - - return self.h2_conn.local_flow_control_window(stream_id) + return 0 async def send_data(self, stream_id, data, end_stream=False): """Send data on a stream. @@ -627,7 +638,11 @@ async def send_data(self, stream_id, data, end_stream=False): if chunk_size <= 0: # Wait for WINDOW_UPDATE per RFC 7540 Section 6.9.2 available = await self._wait_for_flow_control_window(stream_id) - if available <= 0: + if available == 0: + # The peer is there but not reading. + await self.abort_stream(stream_id, HTTP2ErrorCode.CANCEL) + return False + if available < 0: return False return True diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index d949bf0f94..23444429f6 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -11,6 +11,7 @@ import collections import selectors +import time from io import BytesIO from .errors import ( @@ -88,6 +89,8 @@ def __init__(self, cfg, sock, client_addr): # Peer sent a graceful GOAWAY: finish open streams, take no new ones self.draining = False self.peer_last_stream_id = None + # Seconds a stream may make no progress on the wire; 0 means no limit + self.stream_timeout = cfg.timeout or None # Connection settings from config self.initial_window_size = cfg.http2_initial_window_size @@ -606,14 +609,18 @@ def send_response(self, stream_id, status, headers, body=None): self.cleanup_stream(stream_id) return False - def _wait_for_flow_control_window(self, stream_id): - """Wait for flow control window to become positive. + def _wait_for_flow_control_window(self, stream_id, deadline=None): # pylint: disable=too-many-return-statements + """Wait for the stream's send window to become positive. + + Frames read while waiting are handled in order: a reset of this + stream or a GOAWAY goes through the usual handlers (a graceful + GOAWAY keeps the wait going), everything else is set aside for + the main loop. Returns: - int: Available window size, or -1 if waiting failed + int: Available window size; 0 if ``deadline`` passed first; + -1 if the stream or the connection is gone. """ - - max_wait_attempts = 50 # ~5 seconds at 100ms per attempt try: sel = selectors.DefaultSelector() sel.register(self.sock, selectors.EVENT_READ) @@ -621,54 +628,56 @@ def _wait_for_flow_control_window(self, stream_id): # Socket doesn't support selectors (e.g., mock socket) return -1 - result = -1 try: - for _ in range(max_wait_attempts): - available = self.h2_conn.local_flow_control_window(stream_id) + while True: + if self._closed: + return -1 + stream = self.streams.get(stream_id) + if stream is not None and stream.state is StreamState.CLOSED: + return -1 + try: + available = self.h2_conn.local_flow_control_window(stream_id) + except _h2_exceptions.ProtocolError: + return -1 if available > 0: - result = available - break - - ready = sel.select(timeout=0.1) - if ready: - try: - incoming = self.sock.recv(self.READ_BUFFER_SIZE) - except (OSError, IOError, _h2_exceptions.ProtocolError): - break - if not incoming: - self._closed = True - break - try: - events = self.h2_conn.receive_data(incoming) - except _h2_exceptions.ProtocolError: - break - for event in events: - if isinstance(event, _h2_events.StreamReset): - if event.stream_id == stream_id: - result = -1 - break - elif isinstance(event, _h2_events.ConnectionTerminated): - self._closed = True - result = -1 - break - else: - # Anything else arriving alongside the - # WINDOW_UPDATE belongs to the main loop. It has - # already left the h2 state machine, so dropping - # it here loses a request or its body for good. - self._deferred_events.append(event) + return available + wait = 1.0 + if deadline is not None: + remaining = deadline - time.monotonic() + if remaining <= 0: + return 0 + wait = min(remaining, wait) + + if not sel.select(timeout=wait): + continue + try: + incoming = self.sock.recv(self.READ_BUFFER_SIZE) + except (OSError, IOError): + incoming = b"" + if not incoming: + self._closed = True + self._abort_all_streams() + return -1 + try: + events = self.h2_conn.receive_data(incoming) + except _h2_exceptions.ProtocolError: + self.close(error_code=HTTP2ErrorCode.PROTOCOL_ERROR) + return -1 + for event in events: + if (isinstance(event, _h2_events.StreamReset) + and event.stream_id == stream_id): + self._handle_stream_reset(event) + elif isinstance(event, _h2_events.ConnectionTerminated): + self._handle_connection_terminated(event) else: - self._send_pending_data() - continue - break # Break outer loop if inner loop broke - else: - # Loop completed without break - check final window - result = self.h2_conn.local_flow_control_window(stream_id) + # Belongs to the main loop. It has already left the + # h2 state machine, so dropping it here would lose + # a request or its body for good. + self._deferred_events.append(event) + self._send_pending_data() finally: sel.close() - return result - def send_data(self, stream_id, data, end_stream=False): """Send data on a stream. @@ -685,16 +694,25 @@ def send_data(self, stream_id, data, end_stream=False): return False data_to_send = data + deadline = None try: while data_to_send: available = self.h2_conn.local_flow_control_window(stream_id) chunk_size = min(available, self.max_frame_size, len(data_to_send)) if chunk_size <= 0: - # Wait for WINDOW_UPDATE per RFC 7540 Section 6.9.2 + # Wait for WINDOW_UPDATE per RFC 7540 Section 6.9.2, + # for at most stream_timeout without progress. self._send_pending_data() - available = self._wait_for_flow_control_window(stream_id) - if available <= 0: + if deadline is None and self.stream_timeout: + deadline = time.monotonic() + self.stream_timeout + available = self._wait_for_flow_control_window(stream_id, deadline) + if available == 0: + # The peer is there but not reading. + self.abort_stream(stream_id, HTTP2ErrorCode.CANCEL) + return False + if available < 0: + self.cleanup_stream(stream_id) return False chunk_size = min(available, self.max_frame_size, len(data_to_send)) @@ -703,9 +721,10 @@ def send_data(self, stream_id, data, end_stream=False): is_final = end_stream and len(data_to_send) == 0 self.h2_conn.send_data(stream_id, chunk, end_stream=is_final) + stream.send_data(chunk, end_stream=is_final) self._send_pending_data() + deadline = None - stream.send_data(data, end_stream=end_stream) return True except (_h2_exceptions.StreamClosedError, _h2_exceptions.FlowControlError): # Stream was reset by client or flow control error - clean up gracefully diff --git a/gunicorn/http2/response.py b/gunicorn/http2/response.py index a3e3c383ef..60392c7626 100644 --- a/gunicorn/http2/response.py +++ b/gunicorn/http2/response.py @@ -5,6 +5,7 @@ """WSGI response writer for HTTP/2 streams.""" from gunicorn.http.wsgi import Response +from gunicorn.http2.errors import HTTP2StreamError class HTTP2Response(Response): @@ -36,24 +37,33 @@ def can_sendfile(self): # happens to cover HTTP/2 over TLS but not over cleartext. return False + def _aborted(self, what): + # The stream is gone (peer reset, connection lost) or stalled past + # cfg.timeout; the connection has already reset it. Stop the + # application here rather than letting it write into the void. + self._stream_ended = True + raise HTTP2StreamError(self.stream_id, f"response aborted: {what}") + def send_headers(self): if self.headers_sent: return - self.h2_conn.send_response_headers( - self.stream_id, self.status_code, self.headers, end_stream=False - ) + if not self.h2_conn.send_response_headers( + self.stream_id, self.status_code, self.headers, end_stream=False): + self._aborted("stream closed before headers were sent") self.headers_sent = True def _emit_body(self, data): - if not data: + if not data or self._stream_ended: return - self.h2_conn.send_data(self.stream_id, data, end_stream=False) + if not self.h2_conn.send_data(self.stream_id, data, end_stream=False): + self._aborted("stream closed or stalled while sending the body") def close(self): - if not self.headers_sent: - self.send_headers() if self._stream_ended: return + if not self.headers_sent: + self.send_headers() self._stream_ended = True trailers = getattr(self, "trailers", None) - self.h2_conn.end_stream(self.stream_id, trailers=trailers) + if not self.h2_conn.end_stream(self.stream_id, trailers=trailers): + self._aborted("stream closed before the response was complete") diff --git a/gunicorn/workers/base_async.py b/gunicorn/workers/base_async.py index e12f2d2d4a..f96a6b6e2f 100644 --- a/gunicorn/workers/base_async.py +++ b/gunicorn/workers/base_async.py @@ -258,13 +258,13 @@ def handle_http2_request(self, listener_name, req, sock, addr, h2_conn): if hasattr(respiter, "close"): respiter.close() - request_time = datetime.now() - request_start - self.log.access(resp, req, environ, request_time) - except Exception: self.log.exception("Error handling HTTP/2 request") raise finally: + if resp is not None: + # Logged even when the stream was cut off mid-response + self.log.access(resp, req, environ, datetime.now() - request_start) try: self.cfg.post_request(self, req, environ, resp) except Exception: diff --git a/gunicorn/workers/gthread.py b/gunicorn/workers/gthread.py index 18ce82dce8..baced51bf9 100644 --- a/gunicorn/workers/gthread.py +++ b/gunicorn/workers/gthread.py @@ -640,10 +640,10 @@ def handle_http2_request(self, req, conn, h2_conn): environ = {} resp = None stream_id = req.stream.stream_id + request_start = datetime.now() try: self.cfg.pre_request(self, req) - request_start = datetime.now() # Create WSGI environ. The response frames itself as HTTP/2, # so the body streams out instead of being collected first, and @@ -684,10 +684,10 @@ def send_trailers_h2(trailers): if hasattr(respiter, "close"): respiter.close() - request_time = datetime.now() - request_start - self.log.access(resp, req, environ, request_time) - finally: + if resp is not None: + # Logged even when the stream was cut off mid-response + self.log.access(resp, req, environ, datetime.now() - request_start) try: self.cfg.post_request(self, req, environ, resp) except Exception: diff --git a/tests/test_early_hints.py b/tests/test_early_hints.py index 194794b270..fdf7dfa108 100644 --- a/tests/test_early_hints.py +++ b/tests/test_early_hints.py @@ -45,6 +45,8 @@ def __init__(self): self.http2_initial_window_size = 65535 self.http2_max_frame_size = 16384 self.http2_max_header_list_size = 65536 + self.timeout = 30 + self.keepalive = 2 def forwarded_allow_networks(self): return [] diff --git a/tests/test_http2_async_connection.py b/tests/test_http2_async_connection.py index a420694663..1739f1d460 100644 --- a/tests/test_http2_async_connection.py +++ b/tests/test_http2_async_connection.py @@ -772,6 +772,7 @@ class TestAsyncHTTP2FlowControl: @pytest.mark.asyncio async def test_send_data_respects_zero_window(self): """Test that send_data returns False when flow control window is 0.""" + import types from gunicorn.http2.async_connection import AsyncHTTP2Connection cfg = MockConfig() @@ -808,6 +809,8 @@ async def test_send_data_respects_zero_window(self): conn.h2_conn.local_flow_control_window = lambda stream_id: 0 # Try to send data - should return False (not raise) + # The wait is bounded by cfg.timeout; keep the test short. + conn.cfg = types.SimpleNamespace(timeout=0.2) result = await conn.send_data(1, b'Hello, World!') assert result is False @@ -1464,3 +1467,75 @@ async def test_unfinished_response_is_reset_with_internal_error(self): events = client.receive_data(writer.get_written_data()) resets = [e for e in events if isinstance(e, h2.events.StreamReset)] assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] + + +class TestAsyncSendCreditDeadline: + """The send-credit wait is bounded by cfg.timeout and woken by resets.""" + + async def _open_get(self, timeout): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + + cfg = MockConfig() + cfg.set("timeout", timeout) + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(cfg, reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + client.update_settings({h2.settings.SettingCodes.INITIAL_WINDOW_SIZE: 0}) + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + writer.clear() + return conn, client, reader, writer + + @pytest.mark.asyncio + async def test_idle_timeout_resets_the_stream(self): + import types + conn, client, reader, writer = await self._open_get(timeout=1) + conn.cfg = types.SimpleNamespace(timeout=0.2) + assert await conn.send_response_headers(1, [(':status', '200')]) is True + assert await conn.send_data(1, b"x" * 10, end_stream=True) is False + assert 1 not in conn.streams + events = client.receive_data(writer.get_written_data()) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.CANCEL] + + @pytest.mark.asyncio + async def test_reset_wakes_a_window_waiter(self): + conn, client, reader, writer = await self._open_get(timeout=5) + waiter = asyncio.get_running_loop().create_task( + conn._wait_for_flow_control_window(1)) + await asyncio.sleep(0.01) + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + reader.set_data(client.data_to_send()) + await conn.receive_data() + assert await asyncio.wait_for(waiter, timeout=1) == -1 + + @pytest.mark.asyncio + async def test_eof_wakes_a_window_waiter(self): + conn, client, reader, writer = await self._open_get(timeout=5) + waiter = asyncio.get_running_loop().create_task( + conn._wait_for_flow_control_window(1)) + await asyncio.sleep(0.01) + reader.set_eof() + await conn.receive_data() + assert await asyncio.wait_for(waiter, timeout=1) == -1 + + @pytest.mark.asyncio + async def test_timeout_zero_waits_for_credit(self): + conn, client, reader, writer = await self._open_get(timeout=0) + waiter = asyncio.get_running_loop().create_task( + conn._wait_for_flow_control_window(1)) + await asyncio.sleep(0.3) + assert not waiter.done() + client.increment_flow_control_window(100, stream_id=1) + reader.set_data(client.data_to_send()) + await conn.receive_data() + assert await asyncio.wait_for(waiter, timeout=1) == 100 diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index ce2dae8faa..cb16af28cd 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -22,6 +22,7 @@ except ImportError: H2_AVAILABLE = False +from gunicorn.http2.stream import StreamState from gunicorn.http2.errors import ( HTTP2Error, HTTP2ConnectionError, HTTP2ProtocolError ) @@ -1234,34 +1235,6 @@ def test_deferred_events_are_drained_by_receive_data(self): def test_queue_starts_empty(self): assert not self._conn()._deferred_events - def test_events_during_a_window_wait_are_captured(self): - """The real bug: events read while blocked must not be discarded.""" - import socket as _socket - from gunicorn.http2.connection import HTTP2ServerConnection - - server, client = _socket.socketpair() - try: - conn = HTTP2ServerConnection(MockConfig(), server, - ('127.0.0.1', 12345)) - # a request arriving alongside the WINDOW_UPDATE we are waiting on - arriving = mock.Mock(name="RequestReceived") - windows = iter([0, 65535]) - conn.h2_conn = mock.Mock() - conn.h2_conn.local_flow_control_window.side_effect = \ - lambda sid: next(windows) - conn.h2_conn.receive_data.return_value = [arriving] - conn._send_pending_data = lambda: None - - client.sendall(b"frame bytes") - conn._wait_for_flow_control_window(1) - - assert list(conn._deferred_events) == [arriving], \ - "event read during the wait was dropped" - finally: - server.close() - client.close() - - class TestEndStream: """Ending a stream must actually put END_STREAM on the wire.""" @@ -1774,3 +1747,104 @@ def test_arrival_credit_failure_is_swallowed(self): client.send_data(1, b"abc", end_stream=True) conn.receive_data(client.data_to_send()) assert req.body.read() == b"abc" + + +class TestSendCreditWait: + """The send-credit wait handles frames in order and is bounded by cfg.timeout.""" + + def _open_get(self, timeout=30): + """A served GET on stream 1 over a real socketpair, window held at 0.""" + import socket + from gunicorn.http2.connection import HTTP2ServerConnection + + cfg = MockConfig() + cfg.set("timeout", timeout) + server_sock, client_sock = socket.socketpair() + conn = HTTP2ServerConnection(cfg, server_sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + client.update_settings({h2.settings.SettingCodes.INITIAL_WINDOW_SIZE: 0}) + conn.receive_data(client.data_to_send()) + client.receive_data(client_sock.recv(65535)) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + conn.receive_data(client.data_to_send()) + client_sock.setblocking(False) + return conn, client, server_sock, client_sock + + def _server_events(self, client, client_sock): + try: + data = client_sock.recv(65535) + except BlockingIOError: + return [] + return client.receive_data(data) + + def test_deferred_events_are_processed_without_a_read(self): + from gunicorn.http2.connection import HTTP2ServerConnection + + sock = MockSocket() + conn = HTTP2ServerConnection(MockConfig(), sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + marker = mock.Mock(name="event") + conn._deferred_events.append(marker) + conn._handle_event = mock.Mock(return_value="request") + sock.recv = mock.Mock(side_effect=AssertionError("socket read")) + assert conn.receive_data() == ["request"] + conn._handle_event.assert_called_once_with(marker) + assert not conn._deferred_events + + def test_reset_during_wait_marks_stream_and_keeps_other_events(self): + conn, client, server_sock, client_sock = self._open_get() + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + client.send_headers(3, [ + (':method', 'GET'), (':path', '/other'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + client_sock.sendall(client.data_to_send()) + + assert conn._wait_for_flow_control_window(1) == -1 + assert conn.streams[1].state is StreamState.CLOSED + kinds = [type(e).__name__ for e in conn._deferred_events] + assert "RequestReceived" in kinds + assert [r.stream.stream_id for r in conn.receive_data()] == [3] + + def test_graceful_goaway_during_wait_keeps_waiting(self): + from hyperframe.frame import GoAwayFrame + conn, client, server_sock, client_sock = self._open_get() + client_sock.sendall(GoAwayFrame(0, last_stream_id=1, error_code=0).serialize()) + client.increment_flow_control_window(1000, stream_id=1) + client_sock.sendall(client.data_to_send()) + + assert conn._wait_for_flow_control_window(1) == 1000 + assert conn.draining is True + assert conn.is_closed is False + + def test_goaway_with_error_during_wait_returns_minus_one(self): + from hyperframe.frame import GoAwayFrame + conn, client, server_sock, client_sock = self._open_get() + client_sock.sendall(GoAwayFrame(0, last_stream_id=1, error_code=2).serialize()) + assert conn._wait_for_flow_control_window(1) == -1 + assert conn.is_closed is True + + def test_stalled_peer_is_cancelled_after_the_deadline(self): + conn, client, server_sock, client_sock = self._open_get(timeout=1) + conn.stream_timeout = 0.2 + assert conn.send_response_headers(1, 200, [], end_stream=False) is True + assert conn.send_data(1, b"x" * 10, end_stream=True) is False + assert 1 not in conn.streams + events = self._server_events(client, client_sock) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.CANCEL] + + def test_timeout_zero_means_no_deadline(self): + conn, client, server_sock, client_sock = self._open_get(timeout=0) + assert conn.stream_timeout is None + import threading + + def widen(): + client.increment_flow_control_window(100, stream_id=1) + client_sock.sendall(client.data_to_send()) + threading.Timer(0.3, widen).start() + assert conn._wait_for_flow_control_window(1, None) == 100 diff --git a/tests/test_http2_response.py b/tests/test_http2_response.py index cfe85891c6..8bc960d840 100644 --- a/tests/test_http2_response.py +++ b/tests/test_http2_response.py @@ -114,3 +114,42 @@ def test_no_trailers_by_default(self): resp, conn = make_response() resp.close() assert conn.end_stream.call_args.kwargs["trailers"] is None + + +class TestAbortedStream: + """A send the connection refused stops the response instead of continuing.""" + + def test_failed_send_data_raises_stream_error(self): + from gunicorn.http2.errors import HTTP2StreamError + resp, conn = make_response() + conn.send_response_headers.return_value = True + conn.send_data.return_value = False + with pytest.raises(HTTP2StreamError): + resp.write(b"chunk") + + def test_no_write_after_a_failed_send(self): + from gunicorn.http2.errors import HTTP2StreamError + resp, conn = make_response() + conn.send_response_headers.return_value = True + conn.send_data.return_value = False + with pytest.raises(HTTP2StreamError): + resp.write(b"chunk") + resp.write(b"more") + resp.close() + assert resp.h2_conn.send_data.call_count == 1 + conn.end_stream.assert_not_called() + + def test_refused_headers_raise(self): + from gunicorn.http2.errors import HTTP2StreamError + resp, conn = make_response() + conn.send_response_headers.return_value = False + with pytest.raises(HTTP2StreamError): + resp.write(b"chunk") + + def test_failed_end_stream_raises(self): + from gunicorn.http2.errors import HTTP2StreamError + resp, conn = make_response() + conn.send_response_headers.return_value = True + conn.end_stream.return_value = False + with pytest.raises(HTTP2StreamError): + resp.close() From 693336d902457251bfab122bb1bf7bcc56856a2f Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:19:44 +0200 Subject: [PATCH 04/13] http2: never send a second HEADERS block on a stream Refuse it at the connection level and reset the stream with INTERNAL_ERROR instead, so the HPACK table stays intact. --- gunicorn/http2/async_connection.py | 20 +++++++- gunicorn/http2/connection.py | 25 ++++++++-- gunicorn/workers/base_async.py | 4 +- gunicorn/workers/gthread.py | 4 +- tests/test_http2_async_connection.py | 41 +++++++++++++++++ tests/test_http2_connection.py | 69 ++++++++++++++++++++++++++++ 6 files changed, 154 insertions(+), 9 deletions(-) diff --git a/gunicorn/http2/async_connection.py b/gunicorn/http2/async_connection.py index 5bad0ca04d..4f98752b18 100644 --- a/gunicorn/http2/async_connection.py +++ b/gunicorn/http2/async_connection.py @@ -483,6 +483,8 @@ async def send_informational(self, stream_id, status, headers): stream = self.streams.get(stream_id) if stream is None: raise HTTP2Error(f"Stream {stream_id} not found") + if stream.response_headers_sent: + raise HTTP2Error("Informational response after the final headers") # Build headers with :status pseudo-header response_headers = [(':status', str(status))] @@ -501,7 +503,11 @@ async def send_response_headers(self, stream_id, headers, end_stream=False): bool: True if sent, False if the stream is gone """ stream = self.streams.get(stream_id) - if stream is None: + if stream is None or stream.state is StreamState.CLOSED: + return False + if stream.response_headers_sent: + # A second HEADERS block would be encoded into the HPACK table + # before h2 refuses it, corrupting every later response. return False def queue(): @@ -698,7 +704,17 @@ def queue(): return False async def send_error(self, stream_id, status_code, message=None): - """Send an error response on a stream.""" + """Send an error response on a stream. + + Once the peer has a status the stream is cut off with + RST_STREAM(INTERNAL_ERROR) instead: a second HEADERS block would + corrupt the HPACK table. + """ + stream = self.streams.get(stream_id) + if stream is None or stream.response_headers_sent: + await self.abort_stream(stream_id, HTTP2ErrorCode.INTERNAL_ERROR) + return + body = message.encode() if message else b'' headers = [('content-length', str(len(body)))] if body: diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index 23444429f6..9b2121f0bc 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -517,6 +517,8 @@ def send_informational(self, stream_id, status, headers): stream = self.streams.get(stream_id) if stream is None: raise HTTP2Error(f"Stream {stream_id} not found") + if stream.response_headers_sent: + raise HTTP2Error("Informational response after the final headers") # Build headers with :status pseudo-header response_headers = [(':status', str(status))] @@ -537,9 +539,13 @@ def send_response_headers(self, stream_id, status, headers, any number of data frames, then end_stream(). """ stream = self.streams.get(stream_id) - if stream is None: + if stream is None or stream.state is StreamState.CLOSED: # Stream was already cleaned up (reset/closed) return False + if stream.response_headers_sent: + # A second HEADERS block would be encoded into the HPACK table + # before h2 refuses it, corrupting every later response. + return False # Build response headers with :status pseudo-header response_headers = [(':status', str(status))] @@ -547,8 +553,13 @@ def send_response_headers(self, stream_id, status, headers, # HTTP/2 headers must be lowercase response_headers.append((name.lower(), str(value))) - self.h2_conn.send_headers(stream_id, response_headers, - end_stream=end_stream) + try: + self.h2_conn.send_headers(stream_id, response_headers, + end_stream=end_stream) + except _h2_exceptions.StreamClosedError: + stream.close() + self.cleanup_stream(stream_id) + return False stream.send_headers(response_headers, end_stream=end_stream) self._send_pending_data() return True @@ -784,6 +795,14 @@ def send_error(self, stream_id, status_code, message=None): status_code: HTTP status code message: Optional error message body """ + stream = self.streams.get(stream_id) + if stream is None or stream.response_headers_sent: + # Too late for a status: the peer already has one. Cut the + # stream off instead of corrupting the HPACK table with a + # second HEADERS block. + self.abort_stream(stream_id, HTTP2ErrorCode.INTERNAL_ERROR) + return + body = message.encode() if message else b'' headers = [('content-length', str(len(body)))] if body: diff --git a/gunicorn/workers/base_async.py b/gunicorn/workers/base_async.py index f96a6b6e2f..3e5b5956f2 100644 --- a/gunicorn/workers/base_async.py +++ b/gunicorn/workers/base_async.py @@ -203,8 +203,8 @@ def handle_http2(self, listener, client, addr, preface=b"", self.log.exception("Error handling HTTP/2 request") try: h2_conn.send_error(req.stream.stream_id, 500, str(e)) - except Exception: - pass + except Exception as err: + self.log.debug("HTTP/2 error response failed: %s", err) finally: h2_conn.cleanup_stream(req.stream.stream_id) diff --git a/gunicorn/workers/gthread.py b/gunicorn/workers/gthread.py index baced51bf9..c5e4d1127c 100644 --- a/gunicorn/workers/gthread.py +++ b/gunicorn/workers/gthread.py @@ -609,8 +609,8 @@ def handle_http2(self, conn): self.log.exception("Error handling HTTP/2 request") try: h2_conn.send_error(req.stream.stream_id, 500, str(e)) - except Exception: - pass + except Exception as err: + self.log.debug("HTTP/2 error response failed: %s", err) finally: # Cleanup stream after processing h2_conn.cleanup_stream(req.stream.stream_id) diff --git a/tests/test_http2_async_connection.py b/tests/test_http2_async_connection.py index 1739f1d460..c54d23459c 100644 --- a/tests/test_http2_async_connection.py +++ b/tests/test_http2_async_connection.py @@ -1539,3 +1539,44 @@ async def test_timeout_zero_waits_for_credit(self): reader.set_data(client.data_to_send()) await conn.receive_data() assert await asyncio.wait_for(waiter, timeout=1) == 100 + + +class TestAsyncErrorAfterHeaders: + """An error once headers went out resets the stream; HPACK stays intact.""" + + async def _served_get(self): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + writer.clear() + return conn, client, reader, writer + + @pytest.mark.asyncio + async def test_send_error_after_headers_resets_with_internal_error(self): + conn, client, reader, writer = await self._served_get() + assert await conn.send_response_headers(1, [(':status', '200')]) is True + assert await conn.send_response_headers(1, [(':status', '500')]) is False + table = len(conn.h2_conn.encoder.header_table.dynamic_entries) + writer.clear() + + await conn.send_error(1, 500, "boom") + + events = client.receive_data(writer.get_written_data()) + assert not [e for e in events if isinstance(e, h2.events.ResponseReceived)] + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] + assert len(conn.h2_conn.encoder.header_table.dynamic_entries) == table + assert 1 not in conn.streams diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index cb16af28cd..6b59be047d 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -1848,3 +1848,72 @@ def widen(): client_sock.sendall(client.data_to_send()) threading.Timer(0.3, widen).start() assert conn._wait_for_flow_control_window(1, None) == 100 + + +class TestErrorAfterHeaders: + """An error once headers went out resets the stream; HPACK stays intact.""" + + def _served_get(self, stream_id=1): + from gunicorn.http2.connection import HTTP2ServerConnection + + sock = MockSocket() + conn = HTTP2ServerConnection(MockConfig(), sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(sock.get_sent_data()) + client.send_headers(stream_id, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + conn.receive_data(client.data_to_send()) + return conn, client, sock + + def test_send_error_after_headers_resets_with_internal_error(self): + conn, client, sock = self._served_get() + assert conn.send_response_headers(1, 200, [('content-length', '3')]) is True + before = len(sock.get_sent_data()) + table = len(conn.h2_conn.encoder.header_table.dynamic_entries) + + conn.send_error(1, 500, "boom") + + events = client.receive_data(sock.get_sent_data()[before:]) + kinds = [type(e).__name__ for e in events] + assert "ResponseReceived" not in kinds + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] + assert len(conn.h2_conn.encoder.header_table.dynamic_entries) == table + assert 1 not in conn.streams + + def test_later_stream_decodes_correctly_after_a_refused_second_headers(self): + conn, client, sock = self._served_get() + assert conn.send_response_headers(1, 200, [('content-length', '3')]) is True + assert conn.send_response_headers(1, 500, [('x-a', '1')]) is False + conn.send_error(1, 500, "boom") + client.receive_data(sock.get_sent_data()) + + client.send_headers(3, [ + (':method', 'GET'), (':path', '/next'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + conn.receive_data(client.data_to_send()) + before = len(sock.get_sent_data()) + assert conn.send_response(3, 200, [('content-length', '3')], b"abc") is True + events = client.receive_data(sock.get_sent_data()[before:]) + headers = [dict(e.headers) for e in events + if isinstance(e, h2.events.ResponseReceived)] + assert headers == [{b':status': b'200', b'content-length': b'3'}] + + def test_informational_after_final_headers_is_refused(self): + conn, client, sock = self._served_get() + assert conn.send_response_headers(1, 200, []) is True + with pytest.raises(HTTP2Error): + conn.send_informational(1, 103, [('link', '; rel=preload')]) + + def test_cleanup_of_a_never_started_response_resets(self): + conn, client, sock = self._served_get() + before = len(sock.get_sent_data()) + conn.cleanup_stream(1) + events = client.receive_data(sock.get_sent_data()[before:]) + resets = [e for e in events if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] From ce8677ba0999fa75bfe32de23c1f8f136cfc8d48 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:20:55 +0200 Subject: [PATCH 05/13] http2: bound idle connections and stalled body reads on sync workers Read with cfg.keepalive between requests, closing with GOAWAY, and cfg.timeout while a body is pulled, cancelling the stream on expiry. --- gunicorn/http2/connection.py | 18 ++++++++ gunicorn/workers/base_async.py | 10 +++-- gunicorn/workers/gthread.py | 10 +++-- tests/test_http2_connection.py | 79 ++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 8 deletions(-) diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index 9b2121f0bc..504f218f54 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -91,6 +91,8 @@ def __init__(self, cfg, sock, client_addr): self.peer_last_stream_id = None # Seconds a stream may make no progress on the wire; 0 means no limit self.stream_timeout = cfg.timeout or None + # Seconds an idle connection may sit with no stream open; 0 means no limit + self.idle_timeout = cfg.keepalive or None # Connection settings from config self.initial_window_size = cfg.http2_initial_window_size @@ -225,8 +227,24 @@ def _read_and_process(self, data, waiting_stream_id=None): # anything still on the socket; hand them out without reading. return self._process_events(()) if data is None: + # A worker thread or greenlet sits in this read, so the read + # itself has to bound how long a peer may keep it: cfg.timeout + # while a request body is being pulled, cfg.keepalive between + # requests. + timeout = self.stream_timeout if waiting_stream_id is not None else self.idle_timeout + try: + self.sock.settimeout(timeout) + except (OSError, AttributeError): + pass try: data = self.sock.recv(self.READ_BUFFER_SIZE) + except TimeoutError: + if waiting_stream_id is not None: + # The body stalled; HTTP2Body finds the stream closed. + self.abort_stream(waiting_stream_id, HTTP2ErrorCode.CANCEL) + elif not self.streams: + self.close() + return [] except (OSError, IOError) as e: raise HTTP2ConnectionError(f"Socket read error: {e}") diff --git a/gunicorn/workers/base_async.py b/gunicorn/workers/base_async.py index 3e5b5956f2..ac35083101 100644 --- a/gunicorn/workers/base_async.py +++ b/gunicorn/workers/base_async.py @@ -15,7 +15,7 @@ from gunicorn.http.errors import InvalidH2CPreface from gunicorn.http2 import negotiation from gunicorn.http2.response import HTTP2Response -from gunicorn.http2.errors import HTTP2StreamError +from gunicorn.http2.errors import HTTP2ConnectionError, HTTP2StreamError from gunicorn.workers import base ALREADY_HANDLED = object() @@ -195,9 +195,9 @@ def handle_http2(self, listener, client, addr, preface=b"", for req in requests: try: self.handle_http2_request(listener_name, req, client, addr, h2_conn) - except HTTP2StreamError as e: - # The peer reset the stream while its body was - # being read; there is no one left to answer. + except (HTTP2StreamError, HTTP2ConnectionError) as e: + # The peer reset the stream, stalled past the + # timeout, or the socket is gone; nothing to answer. self.log.debug("HTTP/2 stream closed: %s", e) except Exception as e: self.log.exception("Error handling HTTP/2 request") @@ -208,6 +208,8 @@ def handle_http2(self, listener, client, addr, preface=b"", finally: h2_conn.cleanup_stream(req.stream.stream_id) + except HTTP2ConnectionError as e: + self.log.debug("HTTP/2 connection closed: %s", e) except ssl.SSLError as e: if e.args[0] == ssl.SSL_ERROR_EOF: self.log.debug("HTTP/2 SSL connection closed") diff --git a/gunicorn/workers/gthread.py b/gunicorn/workers/gthread.py index c5e4d1127c..2ea2225227 100644 --- a/gunicorn/workers/gthread.py +++ b/gunicorn/workers/gthread.py @@ -30,7 +30,7 @@ from ..http.errors import InvalidH2CPreface from ..http2 import negotiation from ..http2.response import HTTP2Response -from ..http2.errors import HTTP2StreamError +from ..http2.errors import HTTP2ConnectionError, HTTP2StreamError # Sentinel value to indicate connection should be deferred back to poller @@ -601,9 +601,9 @@ def handle_http2(self, conn): for req in requests: try: self.handle_http2_request(req, conn, h2_conn) - except HTTP2StreamError as e: - # The peer reset the stream while its body was - # being read; there is no one left to answer. + except (HTTP2StreamError, HTTP2ConnectionError) as e: + # The peer reset the stream, stalled past the + # timeout, or the socket is gone; nothing to answer. self.log.debug("HTTP/2 stream closed: %s", e) except Exception as e: self.log.exception("Error handling HTTP/2 request") @@ -622,6 +622,8 @@ def handle_http2(self, conn): except http.errors.NoMoreData: self.log.debug("HTTP/2 connection closed by client") + except HTTP2ConnectionError as e: + self.log.debug("HTTP/2 connection closed: %s", e) except ssl.SSLError as e: if e.args[0] == ssl.SSL_ERROR_EOF: self.log.debug("HTTP/2 SSL connection closed") diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index 6b59be047d..1297f489dc 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -1917,3 +1917,82 @@ def test_cleanup_of_a_never_started_response_resets(self): events = client.receive_data(sock.get_sent_data()[before:]) resets = [e for e in events if isinstance(e, h2.events.StreamReset)] assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] + + +class TestSocketTimeouts: + """An idle connection or a stalled body read cannot hold the thread forever.""" + + def _open(self, keepalive=2, timeout=30): + import socket + from gunicorn.http2.connection import HTTP2ServerConnection + + cfg = MockConfig() + cfg.set("keepalive", keepalive) + cfg.set("timeout", timeout) + server_sock, client_sock = socket.socketpair() + conn = HTTP2ServerConnection(cfg, server_sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(client_sock.recv(65535)) + client_sock.setblocking(False) + return conn, client, server_sock, client_sock + + def _events(self, client, client_sock): + try: + return client.receive_data(client_sock.recv(65535)) + except BlockingIOError: + return [] + + def test_idle_connection_gets_goaway_after_keepalive(self): + conn, client, server_sock, client_sock = self._open() + conn.idle_timeout = 0.2 + assert conn.receive_data() == [] + assert conn.is_closed is True + kinds = [type(e).__name__ for e in self._events(client, client_sock)] + assert "ConnectionTerminated" in kinds + + def test_stalled_body_read_resets_the_stream(self): + from gunicorn.http2.errors import HTTP2StreamError + conn, client, server_sock, client_sock = self._open() + conn.stream_timeout = 0.2 + client.send_headers(1, [ + (':method', 'POST'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=False) + client_sock.sendall(client.data_to_send()) + req = conn.receive_data()[0] + with pytest.raises(HTTP2StreamError): + req.body.read() + assert 1 not in conn.streams + assert conn.is_closed is False + resets = [e for e in self._events(client, client_sock) + if isinstance(e, h2.events.StreamReset)] + assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.CANCEL] + + def test_idle_timeout_does_not_close_with_a_stream_open(self): + conn, client, server_sock, client_sock = self._open() + conn.idle_timeout = 0.2 + client.send_headers(1, [ + (':method', 'POST'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=False) + client_sock.sendall(client.data_to_send()) + conn.receive_data() + assert conn.receive_data() == [] + assert conn.is_closed is False + + def test_zero_timeouts_disable_socket_timeout(self): + from gunicorn.http2.connection import HTTP2ServerConnection + + cfg = MockConfig() + cfg.set("keepalive", 0) + cfg.set("timeout", 0) + sock = MockSocket() + conn = HTTP2ServerConnection(cfg, sock, ('127.0.0.1', 12345)) + assert conn.idle_timeout is None + assert conn.stream_timeout is None + sock.settimeout = mock.Mock() + sock.set_recv_data(b"") + conn.receive_data() + sock.settimeout.assert_called_once_with(None) From 0a46fb7c17258c3b20385bbeb1912b529138e94c Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:24:41 +0200 Subject: [PATCH 06/13] http2: drop streams the peer reset before they were served Remove them on RST_STREAM and never hand out a request whose stream is closed, so HEADERS+RST_STREAM floods cannot grow the worker. --- gunicorn/http2/async_connection.py | 8 ++++ gunicorn/http2/connection.py | 26 ++++++++++-- gunicorn/workers/base_async.py | 5 +++ gunicorn/workers/gthread.py | 5 +++ tests/test_http2_connection.py | 67 +++++++++++++++++++++++++++++- 5 files changed, 105 insertions(+), 6 deletions(-) diff --git a/gunicorn/http2/async_connection.py b/gunicorn/http2/async_connection.py index 4f98752b18..0c0b048860 100644 --- a/gunicorn/http2/async_connection.py +++ b/gunicorn/http2/async_connection.py @@ -257,6 +257,14 @@ async def receive_data(self, timeout=None): request = self._handle_event(event) if request is not None: completed_requests.append(request) + # A stream reset in the same batch gets no task at all. + live = [] + for request in completed_requests: + if request.stream.state is StreamState.CLOSED: + self.streams.pop(request.stream.stream_id, None) + else: + live.append(request) + completed_requests = live # Send any pending data (WINDOW_UPDATE, etc.) await self._send_pending_data() diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index 504f218f54..739a3eab97 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -206,9 +206,19 @@ def receive_data(self, data=None): if data is None and self.pending_requests: pending = list(self.pending_requests) self.pending_requests.clear() - return pending + return self._live_requests(pending) return self._read_and_process(data) + def _live_requests(self, requests): + """Drop requests whose stream the peer already reset.""" + live = [] + for request in requests: + if request.stream.state is StreamState.CLOSED: + self.streams.pop(request.stream.stream_id, None) + else: + live.append(request) + return live + def pump(self, stream_id=None): """Read once from the socket while a request body is being consumed. @@ -301,6 +311,7 @@ def _process_events(self, events): request = self._handle_event(event) if request is not None: completed_requests.append(request) + completed_requests = self._live_requests(completed_requests) # Send any pending data (WINDOW_UPDATE, etc.) self._send_pending_data() @@ -444,9 +455,16 @@ def _handle_stream_reset(self, event): stream_id = event.stream_id stream = self.streams.get(stream_id) - if stream is not None: - stream.reset(event.error_code) - # Keep stream in dict for potential cleanup + if stream is None: + return + stream.reset(event.error_code) + # A request the worker has not seen yet is dropped here; one it + # is serving is cleaned up by the worker. Reset streams must not + # accumulate: h2 only counts open ones against the limit. + if any(r.stream is stream for r in self.pending_requests): + self.pending_requests = collections.deque( + r for r in self.pending_requests if r.stream is not stream) + self.streams.pop(stream_id, None) def _handle_connection_terminated(self, event): """Handle ConnectionTerminated event (GOAWAY frame). diff --git a/gunicorn/workers/base_async.py b/gunicorn/workers/base_async.py index ac35083101..de0cc471ba 100644 --- a/gunicorn/workers/base_async.py +++ b/gunicorn/workers/base_async.py @@ -16,6 +16,7 @@ from gunicorn.http2 import negotiation from gunicorn.http2.response import HTTP2Response from gunicorn.http2.errors import HTTP2ConnectionError, HTTP2StreamError +from gunicorn.http2.stream import StreamState from gunicorn.workers import base ALREADY_HANDLED = object() @@ -193,6 +194,10 @@ def handle_http2(self, listener, client, addr, preface=b"", break for req in requests: + if req.stream.state is StreamState.CLOSED: + # Reset by the peer before it could be served + h2_conn.cleanup_stream(req.stream.stream_id) + continue try: self.handle_http2_request(listener_name, req, client, addr, h2_conn) except (HTTP2StreamError, HTTP2ConnectionError) as e: diff --git a/gunicorn/workers/gthread.py b/gunicorn/workers/gthread.py index 2ea2225227..53df04a2e9 100644 --- a/gunicorn/workers/gthread.py +++ b/gunicorn/workers/gthread.py @@ -31,6 +31,7 @@ from ..http2 import negotiation from ..http2.response import HTTP2Response from ..http2.errors import HTTP2ConnectionError, HTTP2StreamError +from ..http2.stream import StreamState # Sentinel value to indicate connection should be deferred back to poller @@ -599,6 +600,10 @@ def handle_http2(self, conn): requests = h2_conn.receive_data() for req in requests: + if req.stream.state is StreamState.CLOSED: + # Reset by the peer before it could be served + h2_conn.cleanup_stream(req.stream.stream_id) + continue try: self.handle_http2_request(req, conn, h2_conn) except (HTTP2StreamError, HTTP2ConnectionError) as e: diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index 1297f489dc..cc966d5c41 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -1789,9 +1789,11 @@ def test_deferred_events_are_processed_without_a_read(self): conn.initiate_connection() marker = mock.Mock(name="event") conn._deferred_events.append(marker) - conn._handle_event = mock.Mock(return_value="request") + request = mock.Mock(name="request") + request.stream.state = StreamState.OPEN + conn._handle_event = mock.Mock(return_value=request) sock.recv = mock.Mock(side_effect=AssertionError("socket read")) - assert conn.receive_data() == ["request"] + assert conn.receive_data() == [request] conn._handle_event.assert_called_once_with(marker) assert not conn._deferred_events @@ -1996,3 +1998,64 @@ def test_zero_timeouts_disable_socket_timeout(self): sock.set_recv_data(b"") conn.receive_data() sock.settimeout.assert_called_once_with(None) + + +class TestStreamFloods: + """HEADERS+RST_STREAM pairs cannot pile up behind a body read.""" + + def _open_post(self, max_streams=100): + from gunicorn.http2.connection import HTTP2ServerConnection + + cfg = MockConfig() + cfg.set("http2_max_concurrent_streams", max_streams) + sock = MockSocket() + conn = HTTP2ServerConnection(cfg, sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(sock.get_sent_data()) + client.send_headers(1, [ + (':method', 'POST'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=False) + req = conn.receive_data(client.data_to_send())[0] + return conn, client, sock, req + + def _get(self, client, stream_id): + client.send_headers(stream_id, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + + def test_reset_before_dispatch_drops_the_stream(self): + conn, client, sock, req = self._open_post() + self._get(client, 3) + client.reset_stream(3, error_code=h2.errors.ErrorCodes.CANCEL) + sock.set_recv_data(client.data_to_send()) + conn.pump(1) + assert 3 not in conn.streams + assert not conn.pending_requests + + def test_reset_after_queueing_is_not_handed_out(self): + conn, client, sock, req = self._open_post() + self._get(client, 3) + sock.set_recv_data(client.data_to_send()) + conn.pump(1) + assert [r.stream.stream_id for r in conn.pending_requests] == [3] + client.reset_stream(3, error_code=h2.errors.ErrorCodes.CANCEL) + sock.set_recv_data(client.data_to_send()) + conn.pump(1) + assert not conn.pending_requests + assert 3 not in conn.streams + assert conn.receive_data(b"") == [] + + def test_flood_of_reset_pairs_leaves_nothing_behind(self): + conn, client, sock, req = self._open_post() + for sid in range(3, 3 + 2 * 500, 2): + self._get(client, sid) + client.reset_stream(sid, error_code=h2.errors.ErrorCodes.CANCEL) + data = client.data_to_send() + for i in range(0, len(data), 65536): + conn.pump(1) if False else conn.receive_data(data[i:i + 65536]) + assert list(conn.streams) == [1] + assert not conn.pending_requests From dd629a0288fe41205a82c0fec517f414e37e6e92 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:25:30 +0200 Subject: [PATCH 07/13] http2: end the stream on an empty final chunk Queue an empty DATA frame carrying END_STREAM in both connection classes. --- gunicorn/http2/async_connection.py | 12 +++++++- gunicorn/http2/connection.py | 28 +++++++---------- tests/test_http2_async_connection.py | 43 ++++++++++++++++++++++++++ tests/test_http2_connection.py | 46 ++++++++++++++++++++++++++++ tests/test_http2_h2c.py | 30 ++++++++++++++++++ 5 files changed, 141 insertions(+), 18 deletions(-) diff --git a/gunicorn/http2/async_connection.py b/gunicorn/http2/async_connection.py index 0c0b048860..40318313ae 100644 --- a/gunicorn/http2/async_connection.py +++ b/gunicorn/http2/async_connection.py @@ -616,7 +616,7 @@ async def _wait_for_flow_control_window(self, stream_id): except asyncio.TimeoutError: return 0 - async def send_data(self, stream_id, data, end_stream=False): + async def send_data(self, stream_id, data, end_stream=False): # pylint: disable=too-many-return-statements """Send data on a stream. Args: @@ -633,6 +633,16 @@ async def send_data(self, stream_id, data, end_stream=False): data_to_send = data try: + if not data_to_send: + if not end_stream: + return True + # An empty DATA frame carrying END_STREAM needs no window + # credit and is how a response with nothing left ends. + async with self._lock(): + self.h2_conn.send_data(stream_id, b"", end_stream=True) + stream.send_data(b"", end_stream=True) + await self._flush_locked() + return True while data_to_send: # The window is read and the frame queued under the lock, # so another stream cannot spend the credit in between. diff --git a/gunicorn/http2/connection.py b/gunicorn/http2/connection.py index 739a3eab97..5660c06a1f 100644 --- a/gunicorn/http2/connection.py +++ b/gunicorn/http2/connection.py @@ -607,22 +607,7 @@ def end_stream(self, stream_id, trailers=None): if trailers: self.send_trailers(stream_id, trailers) return True - # Not send_data(): it chunks against the flow-control window and an - # empty payload skips that loop entirely, so END_STREAM would never - # reach the peer and the client would wait for a response that is - # already finished. - # Not send_data(): it chunks against the flow-control window and an - # empty payload skips that loop entirely, so END_STREAM would never - # reach the peer and the client would wait for a response that is - # already finished. - try: - self.h2_conn.send_data(stream_id, b"", end_stream=True) - self.streams[stream_id].send_data(b"", end_stream=True) - self._send_pending_data() - except _h2_exceptions.StreamClosedError: - self.cleanup_stream(stream_id) - return False - return True + return self.send_data(stream_id, b"", end_stream=True) def send_response(self, stream_id, status, headers, body=None): """Send a response on a stream. @@ -725,7 +710,7 @@ def _wait_for_flow_control_window(self, stream_id, deadline=None): # pylint: di finally: sel.close() - def send_data(self, stream_id, data, end_stream=False): + def send_data(self, stream_id, data, end_stream=False): # pylint: disable=too-many-return-statements """Send data on a stream. Args: @@ -743,6 +728,15 @@ def send_data(self, stream_id, data, end_stream=False): data_to_send = data deadline = None try: + if not data_to_send: + if not end_stream: + return True + # An empty DATA frame carrying END_STREAM needs no window + # credit and is how a response with nothing left ends. + self.h2_conn.send_data(stream_id, b"", end_stream=True) + stream.send_data(b"", end_stream=True) + self._send_pending_data() + return True while data_to_send: available = self.h2_conn.local_flow_control_window(stream_id) chunk_size = min(available, self.max_frame_size, len(data_to_send)) diff --git a/tests/test_http2_async_connection.py b/tests/test_http2_async_connection.py index c54d23459c..29ec6b1df5 100644 --- a/tests/test_http2_async_connection.py +++ b/tests/test_http2_async_connection.py @@ -1580,3 +1580,46 @@ async def test_send_error_after_headers_resets_with_internal_error(self): assert [r.error_code for r in resets] == [h2.errors.ErrorCodes.INTERNAL_ERROR] assert len(conn.h2_conn.encoder.header_table.dynamic_entries) == table assert 1 not in conn.streams + + +class TestAsyncEmptyFinalChunk: + """send_data(b"", end_stream=True) must put END_STREAM on the wire.""" + + async def _served_get(self): + from gunicorn.http2.async_connection import AsyncHTTP2Connection + + reader = MockAsyncReader() + writer = MockAsyncWriter() + conn = AsyncHTTP2Connection(MockConfig(), reader, writer, ('127.0.0.1', 12345)) + await conn.initiate_connection() + client = create_client_connection() + reader.set_data(client.data_to_send()) + await conn.receive_data() + client.receive_data(writer.get_written_data()) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + reader.set_data(client.data_to_send()) + await conn.receive_data() + writer.clear() + return conn, client, reader, writer + + @pytest.mark.asyncio + async def test_empty_final_chunk_ends_the_stream(self): + conn, client, reader, writer = await self._served_get() + assert await conn.send_response_headers(1, [(':status', '200')]) is True + assert await conn.send_data(1, b"part", end_stream=False) is True + assert await conn.send_data(1, b"", end_stream=True) is True + events = client.receive_data(writer.get_written_data()) + kinds = [type(e).__name__ for e in events] + assert kinds == ["ResponseReceived", "DataReceived", "DataReceived", "StreamEnded"] + assert conn.streams[1].response_complete is True + + @pytest.mark.asyncio + async def test_empty_chunk_without_end_stream_sends_nothing(self): + conn, client, reader, writer = await self._served_get() + assert await conn.send_response_headers(1, [(':status', '200')]) is True + writer.clear() + assert await conn.send_data(1, b"", end_stream=False) is True + assert writer.get_written_data() == b"" diff --git a/tests/test_http2_connection.py b/tests/test_http2_connection.py index cc966d5c41..32ed2b898c 100644 --- a/tests/test_http2_connection.py +++ b/tests/test_http2_connection.py @@ -2059,3 +2059,49 @@ def test_flood_of_reset_pairs_leaves_nothing_behind(self): conn.pump(1) if False else conn.receive_data(data[i:i + 65536]) assert list(conn.streams) == [1] assert not conn.pending_requests + + +class TestEmptyFinalChunk: + """send_data(b"", end_stream=True) must put END_STREAM on the wire.""" + + def _served_get(self): + from gunicorn.http2.connection import HTTP2ServerConnection + + sock = MockSocket() + conn = HTTP2ServerConnection(MockConfig(), sock, ('127.0.0.1', 12345)) + conn.initiate_connection() + client = create_client_connection() + conn.receive_data(client.data_to_send()) + client.receive_data(sock.get_sent_data()) + client.send_headers(1, [ + (':method', 'GET'), (':path', '/'), + (':scheme', 'https'), (':authority', 'localhost'), + ], end_stream=True) + conn.receive_data(client.data_to_send()) + return conn, client, sock + + def test_empty_final_chunk_ends_the_stream(self): + conn, client, sock = self._served_get() + before = len(sock.get_sent_data()) + assert conn.send_response_headers(1, 200, []) is True + assert conn.send_data(1, b"part", end_stream=False) is True + assert conn.send_data(1, b"", end_stream=True) is True + events = client.receive_data(sock.get_sent_data()[before:]) + kinds = [type(e).__name__ for e in events] + assert kinds == ["ResponseReceived", "DataReceived", "DataReceived", "StreamEnded"] + assert conn.streams[1].response_complete is True + + def test_empty_chunk_without_end_stream_sends_nothing(self): + conn, client, sock = self._served_get() + assert conn.send_response_headers(1, 200, []) is True + before = len(sock.get_sent_data()) + assert conn.send_data(1, b"", end_stream=False) is True + assert sock.get_sent_data()[before:] == b"" + + def test_end_stream_delegates_to_send_data(self): + conn, client, sock = self._served_get() + before = len(sock.get_sent_data()) + assert conn.send_response_headers(1, 200, []) is True + assert conn.end_stream(1) is True + events = client.receive_data(sock.get_sent_data()[before:]) + assert "StreamEnded" in [type(e).__name__ for e in events] diff --git a/tests/test_http2_h2c.py b/tests/test_http2_h2c.py index 02247cd94f..025517bfa9 100644 --- a/tests/test_http2_h2c.py +++ b/tests/test_http2_h2c.py @@ -1699,6 +1699,36 @@ async def race(): finally: _asgi_h2_teardown(loop, proto) + def test_empty_final_body_message_ends_the_stream(self): + import h2.events + + async def app(scope, receive, send): + await receive() + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"chunk", + "more_body": True}) + await send({"type": "http.response.body", "body": b"", + "more_body": False}) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/stream", end_stream=True) + proto.data_received(client.data_to_send()) + + async def until_done(): + while proto._h2_tasks or not transport.written: + await asyncio.sleep(0.005) + await asyncio.sleep(0.02) + loop.run_until_complete(asyncio.wait_for(until_done(), timeout=5)) + + events = client.receive_data(transport.written) + kinds = [type(e).__name__ for e in events] + assert "StreamEnded" in kinds, kinds + assert not worker.log.exception.called + finally: + _asgi_h2_teardown(loop, proto) + def test_asgi_path_never_touches_h2_directly(self): import inspect from gunicorn.asgi.protocol import ASGIProtocol From ef641215171019a3075e430bda72b31ec06864de Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:29:44 +0200 Subject: [PATCH 08/13] asgi: block in receive() after the body and resume a paused reader receive() waits for the peer to go away instead of returning again at once, and the receive loop resumes a reader paused under backpressure. --- gunicorn/asgi/protocol.py | 55 +++++++++++++++++-- tests/test_http2_h2c.py | 106 +++++++++++++++++++++++++++++++++++-- tests/test_http2_stream.py | 27 ++++++++++ 3 files changed, 179 insertions(+), 9 deletions(-) diff --git a/gunicorn/asgi/protocol.py b/gunicorn/asgi/protocol.py index af7b5640f8..90fa1b30e8 100644 --- a/gunicorn/asgi/protocol.py +++ b/gunicorn/asgi/protocol.py @@ -23,6 +23,7 @@ from gunicorn.http.errors import NoMoreData from gunicorn.http2 import negotiation from gunicorn.http2.errors import HTTP2ErrorCode, HTTP2StreamError +from gunicorn.http2.stream import StreamState from gunicorn.uwsgi.errors import UWSGIParseException @@ -868,6 +869,11 @@ def connection_lost(self, exc): if self._body_receiver is not None: self._body_receiver.signal_disconnect() + # HTTP/2: every stream task learns the peer is gone + h2_conn = getattr(self, '_h2_conn', None) + if h2_conn is not None: + h2_conn.abort_streams_nowait() + # Schedule task cancellation after grace period if task doesn't complete if self._task and not self._task.done(): grace_period = getattr(self.cfg, 'asgi_disconnect_grace_period', 3) @@ -1698,6 +1704,7 @@ async def _handle_http2_connection(self, transport, ssl_object, upgrade=None): while not h2_conn.is_closed: if (not self.worker.alive or h2_conn.draining) and not self._h2_tasks: break + self._maybe_resume_reading() try: requests = await h2_conn.receive_data(timeout=1.0) except asyncio.TimeoutError: @@ -1724,7 +1731,7 @@ async def _handle_http2_connection(self, transport, ssl_object, upgrade=None): except Exception as e: self.log.exception("HTTP/2 connection error: %s", e) finally: - await self._cancel_http2_streams() + await self._finish_http2_streams() if hasattr(self, '_h2_conn'): try: await self._h2_conn.close() @@ -1732,6 +1739,33 @@ async def _handle_http2_connection(self, transport, ssl_object, upgrade=None): pass self._close_transport() + async def _finish_http2_streams(self): + """Tell in-flight streams the peer is gone, then cancel stragglers. + + Same contract as HTTP/1: the application sees http.disconnect + first and gets asgi_disconnect_grace_period to wind down. + """ + tasks = list(self._h2_tasks) + if not tasks: + return + h2_conn = getattr(self, '_h2_conn', None) + if h2_conn is not None: + h2_conn.abort_streams_nowait() + grace = getattr(self.cfg, 'asgi_disconnect_grace_period', 3) + if grace > 0: + await asyncio.wait(tasks, timeout=grace) + await self._cancel_http2_streams() + + def _maybe_resume_reading(self): + """Resume a reader paused by data_received once the loop caught up. + + Only the HTTP/1 loop used to resume; an HTTP/2 connection that + once paused never read again. Half the limit gives hysteresis. + """ + if (self._reading_paused and self.reader is not None + and len(self.reader._buffer) <= self._max_buffer_size // 2): + self._resume_reading() + def _start_http2_stream(self, req, h2_conn, sockname, peername): """Serve one stream in its own task.""" task = self.worker.loop.create_task( @@ -1796,11 +1830,24 @@ async def _handle_http2_request(self, request, h2_conn, sockname, peername): # Track if we've finished receiving body body_received = False - async def receive(): + async def receive(): # pylint: disable=too-many-return-statements nonlocal body_received - # Check if stream is closed or missing - if stream is None or stream.state.name == "CLOSED": + # The peer reset the stream or the connection is gone + if stream is None or stream.disconnected: + return {"type": "http.disconnect"} + if not body_received and stream.state is StreamState.CLOSED: + return {"type": "http.disconnect"} + + if body_received: + # The body has been delivered. Per the ASGI spec the next + # message is http.disconnect: at once if the response is + # already out, otherwise when the peer actually goes away. + # Returning again at once would spin the disconnect + # listeners frameworks run alongside the handler. + if response_complete or peer_gone: + return {"type": "http.disconnect"} + await stream.wait_disconnect() return {"type": "http.disconnect"} # First call: if body already complete (small requests), return it diff --git a/tests/test_http2_h2c.py b/tests/test_http2_h2c.py index 025517bfa9..696faf84dd 100644 --- a/tests/test_http2_h2c.py +++ b/tests/test_http2_h2c.py @@ -493,11 +493,13 @@ def write_eof(self): def set_write_buffer_limits(self, high=None, low=None): pass + paused = False + def pause_reading(self): - pass + self.paused = True def resume_reading(self): - pass + self.paused = False def drain_task(loop, task): @@ -1515,7 +1517,7 @@ async def later(): finally: _asgi_h2_teardown(loop, proto) - def test_receive_after_the_body_is_complete(self): + def test_receive_after_response_complete_returns_disconnect(self): got = [] async def app(scope, receive, send): @@ -1534,8 +1536,102 @@ async def until_done(): while len(got) < 2: await asyncio.sleep(0.005) loop.run_until_complete(asyncio.wait_for(until_done(), timeout=5)) - assert [m["type"] for m in got] == ["http.request", "http.request"] - assert got[1]["body"] == b"" and got[1]["more_body"] is False + assert [m["type"] for m in got] == ["http.request", "http.disconnect"] + finally: + _asgi_h2_teardown(loop, proto) + + def test_receive_after_the_body_blocks_until_the_peer_goes(self): + """The disconnect-listener idiom must suspend, not spin.""" + import h2.errors + + calls = [] + listener_done = [] + + async def app(scope, receive, send): + async def listen(): + while True: + msg = await receive() + calls.append(msg["type"]) + if msg["type"] == "http.disconnect": + listener_done.append(True) + return + task = asyncio.get_running_loop().create_task(listen()) + await asyncio.sleep(0.05) + await task + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/listen", end_stream=True) + proto.data_received(client.data_to_send()) + + async def later(): + await asyncio.sleep(0.1) + # One body message so far, and the listener is parked. + assert calls == ["http.request"], calls + client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) + proto.data_received(client.data_to_send()) + while not listener_done: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(later(), timeout=5)) + assert calls == ["http.request", "http.disconnect"] + assert not worker.log.exception.called + finally: + _asgi_h2_teardown(loop, proto) + + def test_connection_lost_wakes_receive(self): + got = [] + + async def app(scope, receive, send): + got.append((await receive())["type"]) + got.append((await receive())["type"]) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/lost", end_stream=True) + proto.data_received(client.data_to_send()) + + async def later(): + while len(got) < 1: + await asyncio.sleep(0.005) + await asyncio.sleep(0.02) + proto.connection_lost(None) + while len(got) < 2: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(later(), timeout=5)) + assert got == ["http.request", "http.disconnect"] + assert not worker.log.exception.called + finally: + _asgi_h2_teardown(loop, proto) + + def test_reader_resumes_after_a_pause(self): + seen = [] + + async def app(scope, receive, send): + await receive() + seen.append(scope["path"]) + await send({"type": "http.response.start", "status": 200, + "headers": []}) + await send({"type": "http.response.body", "body": b"BODY"}) + + loop, worker, proto, transport, client = _asgi_h2_session(app) + try: + _post_headers(client, 1, "/first", end_stream=True) + proto.data_received(client.data_to_send()) + # Enough control frames in one delivery to trip the reader limit + for _ in range(proto._max_buffer_size // 17 + 1): + client.ping(b"12345678") + proto.data_received(client.data_to_send()) + assert transport.paused is True + + async def until_resumed(): + while transport.paused: + await asyncio.sleep(0.005) + _post_headers(client, 3, "/second", end_stream=True) + proto.data_received(client.data_to_send()) + while len(seen) < 2: + await asyncio.sleep(0.005) + loop.run_until_complete(asyncio.wait_for(until_resumed(), timeout=5)) + assert seen == ["/first", "/second"] finally: _asgi_h2_teardown(loop, proto) diff --git a/tests/test_http2_stream.py b/tests/test_http2_stream.py index 22e9b0bd64..c2566e66e0 100644 --- a/tests/test_http2_stream.py +++ b/tests/test_http2_stream.py @@ -901,3 +901,30 @@ def test_pop_without_connection_ack_support(self): stream.receive_data(b"abc") assert stream.pop_chunk() == b"abc" assert stream.unacked_size == 0 + + +class TestDisconnectWaiter: + """wait_disconnect() blocks until reset or signal_disconnect().""" + + @pytest.mark.asyncio + async def test_reset_resolves_the_waiter(self): + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.state = StreamState.OPEN + waiter = asyncio.get_running_loop().create_task(stream.wait_disconnect()) + await asyncio.sleep(0) + assert not waiter.done() + stream.reset() + await asyncio.wait_for(waiter, timeout=1) + assert stream.disconnected is True + + @pytest.mark.asyncio + async def test_returns_at_once_when_already_disconnected(self): + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.signal_disconnect() + stream.signal_disconnect() + await asyncio.wait_for(stream.wait_disconnect(), timeout=1) + + def test_close_is_not_a_disconnect(self): + stream = HTTP2Stream(stream_id=1, connection=MockConnection()) + stream.close() + assert stream.disconnected is False From 2c1c6a56b1be685f93d01c10d158d6fbeaf4929b Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:33:20 +0200 Subject: [PATCH 09/13] gevent: log an aborted HTTP/2 stream at debug, not as an app error --- gunicorn/workers/base_async.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gunicorn/workers/base_async.py b/gunicorn/workers/base_async.py index de0cc471ba..203b5006d5 100644 --- a/gunicorn/workers/base_async.py +++ b/gunicorn/workers/base_async.py @@ -265,6 +265,9 @@ def handle_http2_request(self, listener_name, req, sock, addr, h2_conn): if hasattr(respiter, "close"): respiter.close() + except (HTTP2StreamError, HTTP2ConnectionError): + # The stream or connection is gone; the caller logs it at debug. + raise except Exception: self.log.exception("Error handling HTTP/2 request") raise From 4d2f53cb6a8bfb1bd9a23816dfa6bafecccb7c6d Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 14:33:20 +0200 Subject: [PATCH 10/13] docs: describe the HTTP/2 review fixes and the timeout and keepalive bounds --- docs/content/2026-news.md | 18 ++++++++++++++++++ gunicorn/config.py | 14 ++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/docs/content/2026-news.md b/docs/content/2026-news.md index ab676adf05..d1c6b35db8 100644 --- a/docs/content/2026-news.md +++ b/docs/content/2026-news.md @@ -17,6 +17,24 @@ once the response is sent. On the ASGI worker, streams on a connection are served concurrently. Only listeners with `h2` in `http_protocols` were affected. +- **HTTP/2 review fixes**: a review of the HTTP/2 support found these, all + fixed on every h2-capable worker unless noted. A graceful `GOAWAY` from the + peer is honoured wherever it lands in a read: established streams finish, + later ones are refused, and no private h2 state is touched. Waiting for + send credit is bounded by `timeout` instead of a fixed five seconds; a + peer that stops reading gets `RST_STREAM(CANCEL)` and the application is + stopped rather than told the response succeeded. An application error + after the response headers went out resets the stream instead of sending a + second `HEADERS` block, which corrupted the HPACK table for every later + response. An empty final body chunk now ends the stream. On `gthread` and + `gevent`, frames read while waiting for credit are handled in order, an + idle connection is closed after `keepalive` and a body that stalls for + `timeout` is cancelled, so a peer can no longer pin a thread, and streams + the peer resets before they are served are dropped so `HEADERS+RST_STREAM` + floods cannot grow the worker. On the ASGI worker, `receive()` after the + body blocks until the peer goes away instead of spinning the event loop, a + reader paused under backpressure is resumed, and in-flight streams get the + disconnect grace period before they are cancelled. ## 26.2.0 - 2026-08-24 diff --git a/gunicorn/config.py b/gunicorn/config.py index d0a156057c..35c3a08199 100644 --- a/gunicorn/config.py +++ b/gunicorn/config.py @@ -868,6 +868,14 @@ class Timeout(Setting): For the non sync workers it just means that the worker process is still communicating and is not tied to the length of time required to handle a single request. + + On HTTP/2 connections it also bounds a single stream: a request body + that makes no progress for this long, or a response the peer stops + reading for this long, is cancelled with ``RST_STREAM``. 0 disables + that too. + + .. versionchanged:: 26.3.0 + Also bounds a stalled HTTP/2 stream. """ @@ -907,6 +915,12 @@ class Keepalive(Setting): .. note:: ``sync`` worker does not support persistent connections and will ignore this option. + + On the ``gthread`` and ``gevent`` workers an HTTP/2 connection with no + stream open is closed with ``GOAWAY`` after this many seconds. + + .. versionchanged:: 26.3.0 + Also applies to idle HTTP/2 connections. """ From 57451ae9a2357328a5890da3bc4800d2dd97b058 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 16:14:17 +0200 Subject: [PATCH 11/13] test: make the ASGI lock-race tests deterministic and regenerate the settings doc The tests took the write lock before the app was dispatched, which deadlocked on Python 3.10 and 3.11 scheduling; the app now signals before the lock is taken and is released with a second event. --- docs/content/reference/settings.md | 14 ++++++++++++++ tests/test_http2_h2c.py | 25 ++++++++++++++++--------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/docs/content/reference/settings.md b/docs/content/reference/settings.md index 8f7e246d3e..846f0e1844 100644 --- a/docs/content/reference/settings.md +++ b/docs/content/reference/settings.md @@ -1945,6 +1945,14 @@ For the non sync workers it just means that the worker process is still communicating and is not tied to the length of time required to handle a single request. +On HTTP/2 connections it also bounds a single stream: a request body +that makes no progress for this long, or a response the peer stops +reading for this long, is cancelled with ``RST_STREAM``. 0 disables +that too. + +!!! info "Changed in 26.3.0" + Also bounds a stalled HTTP/2 stream. + ### `graceful_timeout` **Command line:** `--graceful-timeout INT` @@ -1974,6 +1982,12 @@ set this to a higher value. ``sync`` worker does not support persistent connections and will ignore this option. +On the ``gthread`` and ``gevent`` workers an HTTP/2 connection with no +stream open is closed with ``GOAWAY`` after this many seconds. + +!!! info "Changed in 26.3.0" + Also applies to idle HTTP/2 connections. + ### `asgi_loop` **Command line:** `--asgi-loop STRING` diff --git a/tests/test_http2_h2c.py b/tests/test_http2_h2c.py index 696faf84dd..412b7ba066 100644 --- a/tests/test_http2_h2c.py +++ b/tests/test_http2_h2c.py @@ -1490,7 +1490,6 @@ def test_receive_after_the_stream_was_reset(self): import h2.errors got = [] - reset_seen = asyncio.Event() async def app(scope, receive, send): got.append((await receive())["type"]) @@ -1498,6 +1497,8 @@ async def app(scope, receive, send): got.append((await receive())["type"]) loop, worker, proto, transport, client = _asgi_h2_session(app) + # Created after the loop exists: before 3.12 an Event binds its loop here. + reset_seen = asyncio.Event() try: _post_headers(client, 1, "/reset") client.send_data(1, b"first", end_stream=False) @@ -1713,11 +1714,11 @@ class TestH2CASGIResponseSendIsAtomic: """ def _run(self, body): - started = asyncio.Event() async def app(scope, receive, send): await receive() started.set() + await go.wait() await send({"type": "http.response.start", "status": 200, "headers": []}) if body: @@ -1726,15 +1727,19 @@ async def app(scope, receive, send): await send({"type": "http.response.body", "body": b""}) loop, worker, proto, transport, client = _asgi_h2_session(app) + # Created after the loop exists: before 3.12 an Event binds its loop here. + started = asyncio.Event() + go = asyncio.Event() try: _post_headers(client, 1, "/atomic", end_stream=True) proto.data_received(client.data_to_send()) async def race(): - while not hasattr(proto, "_h2_conn"): - await asyncio.sleep(0.005) + # The app is dispatched (the loop has flushed and released + # the lock) before the lock is taken from under it. + await started.wait() async with proto._h2_conn._lock(): - await started.wait() + go.set() await asyncio.sleep(0.02) # app is now blocked on the lock client.close_connection() proto.data_received(client.data_to_send()) @@ -1760,11 +1765,11 @@ def test_reset_during_drain_is_not_an_app_error(self): """Every send after the peer's reset stays inert, trailers included.""" import h2.errors - started = asyncio.Event() async def app(scope, receive, send): await receive() started.set() + await go.wait() await send({"type": "http.response.start", "status": 200, "headers": []}) for chunk in (b"one", b"two", b"three"): @@ -1775,15 +1780,17 @@ async def app(scope, receive, send): "headers": [(b"x-t", b"1")]}) loop, worker, proto, transport, client = _asgi_h2_session(app) + # Created after the loop exists: before 3.12 an Event binds its loop here. + started = asyncio.Event() + go = asyncio.Event() try: _post_headers(client, 1, "/cancel", end_stream=True) proto.data_received(client.data_to_send()) async def race(): - while not hasattr(proto, "_h2_conn"): - await asyncio.sleep(0.005) + await started.wait() async with proto._h2_conn._lock(): - await started.wait() + go.set() await asyncio.sleep(0.02) client.reset_stream(1, error_code=h2.errors.ErrorCodes.CANCEL) proto.data_received(client.data_to_send()) From 0230d46159d15402f88b2e1b1365c6d56c2dc4cf Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 16:31:23 +0200 Subject: [PATCH 12/13] test: run h2spec against each HTTP/2 worker A docker suite runs the official h2spec image against gthread, gevent and asgi services, failing on any case outside a per-worker list of known gaps, with a CI job on HTTP/2 changes. --- .github/workflows/docker-integration.yml | 26 ++++++ .gitignore | 1 + tests/docker/README.md | 1 + tests/docker/h2spec/Dockerfile | 11 +++ tests/docker/h2spec/README.rst | 21 +++++ tests/docker/h2spec/__init__.py | 0 tests/docker/h2spec/asgi_app.py | 16 ++++ tests/docker/h2spec/conftest.py | 101 +++++++++++++++++++++++ tests/docker/h2spec/docker-compose.yml | 65 +++++++++++++++ tests/docker/h2spec/test_h2spec.py | 56 +++++++++++++ 10 files changed, 298 insertions(+) create mode 100644 tests/docker/h2spec/Dockerfile create mode 100644 tests/docker/h2spec/README.rst create mode 100644 tests/docker/h2spec/__init__.py create mode 100644 tests/docker/h2spec/asgi_app.py create mode 100644 tests/docker/h2spec/conftest.py create mode 100644 tests/docker/h2spec/docker-compose.yml create mode 100644 tests/docker/h2spec/test_h2spec.py diff --git a/.github/workflows/docker-integration.yml b/.github/workflows/docker-integration.yml index a7a86b10be..5732e33e59 100644 --- a/.github/workflows/docker-integration.yml +++ b/.github/workflows/docker-integration.yml @@ -6,11 +6,19 @@ on: paths: - 'gunicorn/uwsgi/**' - 'tests/docker/uwsgi/**' + - 'gunicorn/http2/**' + - 'gunicorn/asgi/**' + - 'gunicorn/workers/**' + - 'tests/docker/h2spec/**' - '.github/workflows/docker-integration.yml' pull_request: paths: - 'gunicorn/uwsgi/**' - 'tests/docker/uwsgi/**' + - 'gunicorn/http2/**' + - 'gunicorn/asgi/**' + - 'gunicorn/workers/**' + - 'tests/docker/h2spec/**' - '.github/workflows/docker-integration.yml' permissions: @@ -43,3 +51,21 @@ jobs: - name: Run uWSGI integration tests run: | pytest tests/docker/uwsgi/ -v --tb=short + + h2spec: + name: HTTP/2 conformance (h2spec) + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v7 + - name: Set up Python + uses: actions/setup-python@v7 + with: + python-version: "3.12" + - name: Install test dependencies + run: | + python -m pip install --upgrade pip + python -m pip install pytest + - name: Run h2spec against each HTTP/2 worker + run: | + pytest tests/docker/h2spec/ -v --tb=short diff --git a/.gitignore b/.gitignore index f855616824..4362dbe48a 100755 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ setuptools-* site/ docs/site/ tests/docker/*/results/ +tests/docker/h2spec/certs/ diff --git a/tests/docker/README.md b/tests/docker/README.md index 9ad5efefd7..0931fe0f35 100644 --- a/tests/docker/README.md +++ b/tests/docker/README.md @@ -46,6 +46,7 @@ to 8000 and collides with `asgi_compliance`. | `asgi_compliance` | 8000, 8080, 8443, 8444, 8445 | | `asgi_framework_compat` | 8001 to 8006 | | `http2` | 8443, 8444 | +| `h2spec` | 8451, 8452, 8453 | | `uwsgi` | 8080 | If a suite fails everywhere at once, check for a port collision before looking diff --git a/tests/docker/h2spec/Dockerfile b/tests/docker/h2spec/Dockerfile new file mode 100644 index 0000000000..5307b488c3 --- /dev/null +++ b/tests/docker/h2spec/Dockerfile @@ -0,0 +1,11 @@ +FROM python:3.14-slim + +RUN apt-get update && apt-get install -y --no-install-recommends gcc \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /app + +COPY . /gunicorn-src/ +RUN pip install --no-cache-dir "/gunicorn-src/[http2,gevent]" + +EXPOSE 8443 diff --git a/tests/docker/h2spec/README.rst b/tests/docker/h2spec/README.rst new file mode 100644 index 0000000000..9c57d8d8ea --- /dev/null +++ b/tests/docker/h2spec/README.rst @@ -0,0 +1,21 @@ +h2spec conformance +================== + +Runs `h2spec `_ (RFC 7540 and HPACK +conformance, 146 cases) against gunicorn on each HTTP/2 capable worker: +``gthread``, ``gevent`` and ``asgi``. h2spec runs from its official image +inside the compose network, so nothing needs to be installed on the host +besides Docker. + +Run it on its own or through the shared runner:: + + PYTEST=".venv/bin/python -m pytest" scripts/run_docker_tests.sh h2spec + +Each worker has a list of known failing cases in ``test_h2spec.py``. A case +failing outside that list fails the test; a listed case that starts passing +is reported as an expected failure so the list can be trimmed. + +Ports 8451, 8452 and 8453 are bound on the host for manual runs, for +example with a locally installed binary:: + + h2spec -h 127.0.0.1 -p 8451 -t -k diff --git a/tests/docker/h2spec/__init__.py b/tests/docker/h2spec/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/docker/h2spec/asgi_app.py b/tests/docker/h2spec/asgi_app.py new file mode 100644 index 0000000000..2099f1dc99 --- /dev/null +++ b/tests/docker/h2spec/asgi_app.py @@ -0,0 +1,16 @@ +"""Minimal ASGI application for h2spec conformance runs.""" + + +async def app(scope, receive, send): + if scope["type"] != "http": + return + while True: + message = await receive() + if not message.get("more_body", False): + break + await send({ + "type": "http.response.start", + "status": 200, + "headers": [(b"content-type", b"text/plain")], + }) + await send({"type": "http.response.body", "body": b"ok"}) diff --git a/tests/docker/h2spec/conftest.py b/tests/docker/h2spec/conftest.py new file mode 100644 index 0000000000..921f798712 --- /dev/null +++ b/tests/docker/h2spec/conftest.py @@ -0,0 +1,101 @@ +# -*- coding: utf-8 - +# +# This file is part of gunicorn released under the MIT license. +# See the NOTICE for more information. + +"""Fixtures for the h2spec conformance suite.""" + +import subprocess +from pathlib import Path + +import pytest + +DOCKER_DIR = Path(__file__).parent +CERTS_DIR = DOCKER_DIR / "certs" +COMPOSE_FILE = DOCKER_DIR / "docker-compose.yml" + +WORKERS = ("gthread", "gevent", "asgi") +PORTS = {"gthread": 8451, "gevent": 8452, "asgi": 8453} + + +def _compose(*args, **kwargs): + return subprocess.run( + ["docker", "compose", "-f", str(COMPOSE_FILE), *args], + cwd=DOCKER_DIR, **kwargs) + + +def _generate_cert(): + CERTS_DIR.mkdir(exist_ok=True) + crt, key = CERTS_DIR / "server.crt", CERTS_DIR / "server.key" + if crt.exists() and key.exists(): + check = subprocess.run( + ["openssl", "x509", "-checkend", "86400", "-noout", "-in", str(crt)], + capture_output=True) + if check.returncode == 0: + return + subprocess.run([ + "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", + "-keyout", str(key), "-out", str(crt), "-days", "1", + "-subj", "/CN=localhost", + ], check=True, capture_output=True) + + +def _wait_healthy(timeout=180): + import time + deadline = time.monotonic() + timeout + services = [f"gunicorn-{w}" for w in WORKERS] + while time.monotonic() < deadline: + result = _compose("ps", "--format", "{{.Service}} {{.Health}}", + capture_output=True, text=True) + healthy = {line.split()[0] for line in result.stdout.splitlines() + if line.endswith("healthy")} + if all(s in healthy for s in services): + return True + time.sleep(2) + return False + + +@pytest.fixture(scope="session") +def h2spec_services(): + """Bring up one gunicorn per worker class; tear everything down after.""" + for cmd in (["docker", "info"], ["docker", "compose", "version"]): + try: + subprocess.run(cmd, check=True, capture_output=True) + except (subprocess.CalledProcessError, FileNotFoundError): + pytest.skip("Docker is not available") + _generate_cert() + try: + _compose("build", check=True) + _compose("pull", "h2spec", check=True, capture_output=True) + _compose("up", "-d", check=True) + if not _wait_healthy(): + logs = _compose("logs", capture_output=True, text=True) + pytest.fail(f"gunicorn services did not become healthy:\n{logs.stdout}\n{logs.stderr}") + yield + finally: + _compose("down", "--remove-orphans", "-v", capture_output=True) + + +def run_h2spec(worker): + """Run h2spec against one worker inside the compose network. + + Returns (passed, failed, [failing case descriptions], full output). + """ + result = _compose( + "run", "--rm", "--no-deps", "h2spec", + "-h", f"gunicorn-{worker}", "-p", "8443", "-t", "-k", "-o", "10", + capture_output=True, text=True) + out = result.stdout + result.stderr + passed = failed = None + failures = [] + for line in out.splitlines(): + stripped = line.strip() + if stripped.startswith("×"): + failures.append(stripped[1:].strip()) + if " tests, " in stripped and " passed" in stripped: + parts = stripped.replace(",", "").split() + passed = int(parts[parts.index("passed") - 1]) + failed = int(parts[parts.index("failed") - 1]) + if passed is None: + pytest.fail(f"h2spec produced no summary for {worker}:\n{out}") + return passed, failed, sorted(set(failures)), out diff --git a/tests/docker/h2spec/docker-compose.yml b/tests/docker/h2spec/docker-compose.yml new file mode 100644 index 0000000000..8789458a13 --- /dev/null +++ b/tests/docker/h2spec/docker-compose.yml @@ -0,0 +1,65 @@ +# One gunicorn image, one service per HTTP/2 capable worker, and the +# official h2spec image run against each of them by the tests. +x-gunicorn: &gunicorn + image: gunicorn-h2spec:local + volumes: + - ./certs:/certs:ro + - ../http2/app.py:/app/app.py:ro + - ./asgi_app.py:/app/asgi_app.py:ro + healthcheck: + test: ["CMD", "python", "-c", "import ssl,socket; s=socket.socket(); s.settimeout(1); ctx=ssl.create_default_context(); ctx.check_hostname=False; ctx.verify_mode=ssl.CERT_NONE; ss=ctx.wrap_socket(s,server_hostname='localhost'); ss.connect(('localhost',8443)); ss.close()"] + interval: 2s + timeout: 5s + retries: 15 + start_period: 5s + +services: + gunicorn-gthread: + <<: *gunicorn + build: + context: ../../../ + dockerfile: tests/docker/h2spec/Dockerfile + ports: + - "8451:8443" + command: ["gunicorn", "app:app", "--bind", "0.0.0.0:8443", + "--worker-class", "gthread", "--threads", "4", + "--http-protocols", "h2,h1", + "--certfile", "/certs/server.crt", "--keyfile", "/certs/server.key", + "--log-level", "warning"] + + gunicorn-gevent: + <<: *gunicorn + depends_on: + gunicorn-gthread: + condition: service_started + ports: + - "8452:8443" + command: ["gunicorn", "app:app", "--bind", "0.0.0.0:8443", + "--worker-class", "gevent", + "--http-protocols", "h2,h1", + "--certfile", "/certs/server.crt", "--keyfile", "/certs/server.key", + "--log-level", "warning"] + + gunicorn-asgi: + <<: *gunicorn + depends_on: + gunicorn-gthread: + condition: service_started + ports: + - "8453:8443" + command: ["gunicorn", "asgi_app:app", "--bind", "0.0.0.0:8443", + "--worker-class", "asgi", + "--http-protocols", "h2,h1", + "--certfile", "/certs/server.crt", "--keyfile", "/certs/server.key", + "--log-level", "warning"] + + h2spec: + image: summerwind/h2spec:2.6.0 + profiles: ["tools"] + depends_on: + gunicorn-gthread: + condition: service_healthy + gunicorn-gevent: + condition: service_healthy + gunicorn-asgi: + condition: service_healthy diff --git a/tests/docker/h2spec/test_h2spec.py b/tests/docker/h2spec/test_h2spec.py new file mode 100644 index 0000000000..51a8f0b8da --- /dev/null +++ b/tests/docker/h2spec/test_h2spec.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 - +# +# This file is part of gunicorn released under the MIT license. +# See the NOTICE for more information. + +"""RFC 7540 and HPACK conformance via h2spec, per HTTP/2 capable worker. + +h2spec (https://github.com/summerwind/h2spec) drives 146 cases against +a live server. Every case must pass except the ones listed per worker +below, which are known gaps tracked for a fix; a case that starts +failing outside that list fails the test, and a listed case that starts +passing is reported so the list can shrink. +""" + +import warnings + +import pytest + +from .conftest import WORKERS, run_h2spec + +# Known gaps, keyed by the case description h2spec prints. Observed with +# the official image on Linux; a local macOS binary shows a few more +# timing-sensitive ones on the sync workers. +WINDOW_OVERFLOW_ON_STREAM = ( + "3: Sends multiple WINDOW_UPDATE frames increasing the flow control " + "window to above 2^31-1 on a stream") +KNOWN_FAILURES = { + # The sync path answers a stream window overflow with a connection + # error instead of RST_STREAM(FLOW_CONTROL_ERROR). + "gthread": {WINDOW_OVERFLOW_ON_STREAM}, + "gevent": { + WINDOW_OVERFLOW_ON_STREAM, + # The socket is closed with unread bytes pending, so the client + # sees a reset instead of a clean close; flaps run to run. + "1: Sends a GOAWAY frame", + "1: Sends an invalid PING frame for connection close", + "1: Sends a GOAWAY frame with unknown error code", + }, + "asgi": set(), +} + + +@pytest.mark.parametrize("worker", WORKERS) +def test_h2spec(worker, h2spec_services): + passed, failed, failures, output = run_h2spec(worker) + known = KNOWN_FAILURES[worker] + unexpected = [f for f in failures if f not in known] + fixed = [f for f in known if f not in failures] + + assert not unexpected, ( + f"{worker}: {failed} failed ({passed} passed); new failures:\n " + + "\n ".join(unexpected) + f"\n\n{output}") + assert passed + failed >= 140, f"{worker}: only {passed + failed} cases ran\n{output}" + if fixed: + # Some listed cases flap; say so without failing the run. + warnings.warn(f"{worker}: listed h2spec cases passed this time: {fixed}") From 3da7d9868ed42c13c2be29fcdc9dabb5e9bbdca4 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sun, 30 Aug 2026 16:36:26 +0200 Subject: [PATCH 13/13] ci: install pytest-cov for the h2spec job --- .github/workflows/docker-integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docker-integration.yml b/.github/workflows/docker-integration.yml index 5732e33e59..cf89f8461e 100644 --- a/.github/workflows/docker-integration.yml +++ b/.github/workflows/docker-integration.yml @@ -65,7 +65,7 @@ jobs: - name: Install test dependencies run: | python -m pip install --upgrade pip - python -m pip install pytest + python -m pip install pytest pytest-cov - name: Run h2spec against each HTTP/2 worker run: | pytest tests/docker/h2spec/ -v --tb=short