Skip to content

Add support for HTTP/2#13039

Open
Moist-Cat wants to merge 13 commits into
aio-libs:masterfrom
Moist-Cat:master
Open

Add support for HTTP/2#13039
Moist-Cat wants to merge 13 commits into
aio-libs:masterfrom
Moist-Cat:master

Conversation

@Moist-Cat

@Moist-Cat Moist-Cat commented Jul 2, 2026

Copy link
Copy Markdown

What do these changes do?

Add HTTP/2 client support.

Why

Faster I/O bound operations (e.g., many requests to the same host) via multiplexing (handling several streams/requests inside a single connection).

How

  1. Use the environment variable AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS=1 to allow h2 negotiation via ALPN during the TLS handshake.
  2. ResponseHandler was substituted by a wrapper that conditionally switches protocols depending on the negotiated protocol.
  3. I forced unconditional connection reuse for HTTP/2 connections since pooling is now unnecessary. This doesn't affect HTTP/1.1 connections. To make this possible, however, I had to use a Semaphore to avoid race conditions.

This opening many HTTP/1.1 connections in parallel is now slower since it's done sequentially. That said, to know if connections can be pooled or not it's only necessary to wait until the first connection is done. Once it's know whether the host supports HTTP/2 or not, the rest of the requests can be done in parallel so it's possible to mitigate this performance hit substantially.

Backward compatibility

Opt-in via AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS=1.

Testing

%95 coverage, benchmarks (%50 latency reduction for 99 requests, see below), and integration tests against real servers (~100).

Dependencies

hpack

Is it a substantial burden for the maintainers to support this?

Yes.

Related issue number

refs #5999

The implementation is self-contained and the changes to the current codebase are minimal and backwards compatible (that said, I make use of some black magic to be able to conditionally switch protocols).

Missing features (to the date):

  • Proxies
  • Chunking
  • CONTINUATION frames for very large headers
  • h2c (cleartext) not supported.
  • Ensure all the high-level configuration/parameters work (or make sense for) with HTTP/2 as well

Moist-Cat and others added 2 commits July 2, 2026 19:38
    This implementation is backwards compatible, functional, but still
incomplete.
Comment thread aiohttp/http2/connection.py Fixed
Comment thread aiohttp/http2/connection.py Fixed
Comment thread aiohttp/http2/connection.py Fixed
Comment thread aiohttp/http2/response.py Fixed
Comment thread aiohttp/http_protocol.py
self._handler: Optional[asyncio.Protocol] = None

# ---- Transport callbacks forwarded to the real handler ----
def connection_made(self, transport: asyncio.BaseTransport) -> None:
Comment thread tests/http2/test_http2.py Fixed
Comment thread tests/http2/test_http2.py Fixed
Comment thread tests/http2/test_http2.py Fixed
Comment thread tests/http2/test_http2.py Fixed
Comment thread tests/http2/test_http2.py Fixed
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.54281% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.92%. Comparing base (e3774b4) to head (2ce9b41).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
aiohttp/http2/connection.py 94.78% 9 Missing and 8 partials ⚠️
tests/http2/test_http2.py 98.74% 7 Missing and 3 partials ⚠️
aiohttp/connector.py 80.00% 1 Missing and 1 partial ⚠️
aiohttp/http2/response.py 96.49% 1 Missing and 1 partial ⚠️
aiohttp/http_protocol.py 93.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13039      +/-   ##
==========================================
- Coverage   98.96%   98.92%   -0.04%     
==========================================
  Files         131      138       +7     
  Lines       48156    49484    +1328     
  Branches     2499     2593      +94     
==========================================
+ Hits        47656    48951    +1295     
- Misses        376      395      +19     
- Partials      124      138      +14     
Flag Coverage Δ
Autobahn 22.38% <28.51%> (+0.16%) ⬆️
CI-GHA 98.87% <97.54%> (-0.04%) ⬇️
OS-Linux 98.64% <97.54%> (-0.03%) ⬇️
OS-Windows 97.06% <97.46%> (+0.01%) ⬆️
OS-macOS 97.94% <97.54%> (-0.02%) ⬇️
Py-3.10 98.12% <97.54%> (-0.02%) ⬇️
Py-3.11 98.39% <97.54%> (-0.03%) ⬇️
Py-3.12 98.47% <97.54%> (-0.03%) ⬇️
Py-3.13 98.45% <97.54%> (-0.03%) ⬇️
Py-3.14 98.47% <97.54%> (-0.03%) ⬇️
Py-3.14t 97.58% <97.54%> (-0.01%) ⬇️
Py-pypy-3.11 97.45% <97.54%> (+<0.01%) ⬆️
VM-macos 97.94% <97.54%> (-0.02%) ⬇️
VM-ubuntu 98.64% <97.54%> (-0.03%) ⬇️
VM-windows 97.06% <97.46%> (+0.01%) ⬆️
cython-coverage 38.05% <34.43%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 11.59%

❌ 2 regressed benchmarks
✅ 81 untouched benchmarks
⏩ 83 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_one_thousand_round_trip_websocket_text_messages 16.8 ms 19 ms -11.68%
test_one_thousand_round_trip_websocket_binary_messages[tcp-small] 16.4 ms 18.5 ms -11.51%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing Moist-Cat:master (2ce9b41) with master (e3774b4)

Open in CodSpeed

Footnotes

  1. 83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Comment thread tests/http2/test_http2.py Fixed
Comment thread docs/conf.py

try:
import sphinxcontrib.spelling # noqa
import sphinxcontrib.spelling
Moist-Cat and others added 2 commits July 4, 2026 21:52
    It was necessary to add a semaphore to ensure the requests connect
sequentially to the hosts and reuse connections when necessary. HTTP/2
uses a single connection per host.
Comment thread tests/http2/test_http2_integration.py Fixed
Comment thread tests/http2/test_http2_integration.py Fixed
Comment thread tests/http2/test_http2_integration.py Fixed
Comment thread tests/http2/test_http2_integration.py Fixed
Comment thread tests/http2/test_http2_integration.py Fixed
@Moist-Cat

Moist-Cat commented Jul 5, 2026

Copy link
Copy Markdown
Author

I ran tests against remote servers (httpbin.org) to verify HTTP/2 indeed reduces latency.

HTTP/2 Performance Test Results

System Specs:

  • CPU: Intel(R) Core(TM) i5-8350U CPU @ 1.70GHz (8 cores)
  • Memory: 7892016 KB
  • Python: 3.14.2
  • aiohttp: 4.0.0a2.dev0

Test Configuration:

  • Concurrency per batch: 99
  • Number of batches: 30
  • Total requests per version: 2970

Batch Mean Latency (seconds)

Version Mean Std Dev P50 P95 P99
HTTP/1.1 1.3928 0.9116 1.2004 2.9433 4.5932
HTTP/2 0.4821 0.1588 0.4502 0.6966 1.0516

Individual Request Latency Distribution

Version Mean P50 P95 P99
HTTP/1.1 1.3928 0.9819 3.8822 7.1815
HTTP/2 0.4821 0.4517 0.7857 1.3151

Statistical Analysis

  • Welch’s t‑test on batch means:
    t = 5.390, p = 0.000007
  • Cohen’s d: 1.392
  • Assumption: Measurement errors (batch means) are approximately normally distributed (reasonable with 30 batches by the Central Limit Theorem).

A simple bar chart with the means (results vary because they are from a second test):
bar_chart

We lose efficiency in CPU bound tasks (see #13039 (comment)) but I/O bound tasks are significantly faster. This is specially true for batch requests that require multiple TCP connections to the same host.

@Moist-Cat

Copy link
Copy Markdown
Author

I would like to know if the trade-offs (I/O vs CPU) are acceptable before writing the docs.

@Moist-Cat Moist-Cat marked this pull request as ready for review July 6, 2026 00:29
@aiolibsbot

Copy link
Copy Markdown
Contributor

@Moist-Cat: I ran tests against remote servers (httpbin.org) to verify HTTP/2 indeed reduces latency.

HTTP/1.1 regression not inherent to h2. Caused by global Semaphore(1) wrapping every connector.connect() in _connect_and_send_request. Serializes all connection setup, h1 included — hence the ~8% CodSpeed hit on non-h2 benchmarks. Scope the semaphore to first-connect-per-unknown-host under the flag; h1 parallelism returns.

@aiolibsbot

Copy link
Copy Markdown
Contributor

@Moist-Cat: I would like to know if the trade-offs (I/O vs CPU) are acceptable before writing the docs.

Bigger blocker than the CPU/IO trade-off. h2 path returns Http2Response, not ClientResponse. Breaks .json(), .text(), cookies, raise_for_status, redirects, middleware. Hold the docs. Resolve response integration, the connector-slot leak, and the semaphore serialization first. The default-path CPU cost is the semaphore — removable, not intrinsic.

@aiolibsbot

Copy link
Copy Markdown
Contributor

PR Review — Add support for HTTP/2

Ambitious, self-contained h2 implementation — but not mergeable yet: it doesn't integrate with the ClientResponse API and it regresses the default HTTP/1.1 path.

Strengths worth calling out:

  • A genuinely complete RFC 7540 core in http2/connection.py: frame parse/dispatch, HPACK via hpack, stream state machine, SETTINGS/PING/GOAWAY/WINDOW_UPDATE handling, and connection-level + stream-level flow control.
  • Correctly gated behind AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS with ALPN-based negotiation, and honest about missing pieces (proxies, chunking, CONTINUATION, h2c).
  • Real measurement effort — CodSpeed run plus httpbin latency benchmarks with a t-test.

Blocking issues:

  • h2 path returns Http2Response, not ClientResponse.json()/.text()/cookies/raise_for_status/redirects/middleware all bypassed.
  • Global Semaphore(1) serializes every connection setup, HTTP/1.1 included — this is the ~8% CodSpeed regression on non-h2 benchmarks.
  • Manual _conns insertion + conn._protocol = None leaks connector limit slots (double-booked in _acquired and _conns).
  • os.getenv(..., False) truthiness bug: =0 / =false still enables h2.
  • Library-level logger.setLevel(DEBUG) + eager per-frame f-string logging on the hot path.
  • Parser robustness gaps (padding underflow, no MAX_FRAME_SIZE cap) and a shared flow-control Event racing across streams; plus many CodeQL dead-import/variable hits and missing docs/changelog fragment.

Recommendation: keep the h2 core, but rework the integration so it produces a ClientResponse and stops serializing/leaking on the shared connect path before this leaves draft.


🔴 Blocking

1. HTTP/2 path returns Http2Response, not ClientResponse — breaks the public API contract
aiohttp/client.py:253-264

_connect_and_send_request is annotated -> ClientResponse, but the h2 branch returns whatever conn.protocol.send(...) yields, which is an Http2Response (see aiohttp/http2/connection.py::Http2Protocol.send). Http2Response is a standalone object, not a ClientResponse subclass.

Why it matters: every consumer of the response silently gets a different object. None of the documented client surface works on the h2 path:

  • resp.status, await resp.text(), await resp.json(), resp.content streaming
  • resp.cookies / cookie-jar updates, raise_for_status, resp.history
  • redirect following, async with session.get(...) as resp, and the client middleware chain

This isn't a missing-feature footnote — it means the feature doesn't integrate with aiohttp at all; it's a parallel client bolted onto the connect path. This is the central blocker for the PR.

How to fix: adapt the h2 result into a real ClientResponse (populate status/headers/content from the stream and feed the body through the normal StreamReader), so the rest of client.py, the response API, and middleware are unchanged. Until the return type is genuinely a ClientResponse, the opt-in path can't ship.

resp = await conn.protocol.send(  # type: ignore[attr-defined]
    req.method, req.url, list(req.headers.items()), body,
)
conn._protocol = None
2. Global Semaphore(1) serializes ALL connection establishment, including HTTP/1.1
aiohttp/connector.py:329

self.sem = asyncio.Semaphore(1) combined with async with connector.sem: around connector.connect() in client.py serializes every connection setup for the whole connector — not just h2, and not just when the experimental flag is set.

Why it matters: this is a regression on the default, non-opt-in path that every aiohttp user hits. With a semaphore of 1, N concurrent requests that each need a fresh connection now establish their TLS handshakes strictly one-at-a-time. That is exactly what the CodSpeed report shows — ~8% slower across test_one_hundred_simple_get_requests* even though those benchmarks never enable h2. The PR's own "CPU-bound tasks are slower" caveat is really this serialization.

How to fix: the semaphore should not gate HTTP/1.1 at all. Only serialize the first connection to an as-yet-unknown host so ALPN can be discovered, then let subsequent connects run in parallel — and only when AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS is set. A single process-wide Semaphore(1) on the hot path can't merge.

self.sem = asyncio.Semaphore(1)  # gates every connect(), h1 included

🟡 Important

1. os.getenv truthiness check enables h2 for any value, including "0" and "false"
aiohttp/connector.py:868-869

if os.getenv("AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS", False): tests the string returned by getenv. Any non-empty value is truthy, so AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS=0 and ...=false both enable h2 — the opposite of what a user setting =0 intends.

Why it matters: an operator explicitly disabling the flag with =0 silently turns it on. Opt-in gates that can't be turned off are a footgun.

How to fix: compare explicitly, e.g. os.getenv("AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS") == "1" (or accept a small set of truthy tokens). Also note the default False should be None/omitted for consistency, though it doesn't change behavior here.

if os.getenv("AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS", False):
    protocols += ["h2"]
2. Manual _conns insertion + conn._protocol=None leaks connector accounting slots
aiohttp/client.py:254-270

The h2 branch mutates connector internals directly: connector._conns[conn._key] = deque([(conn.protocol, time.monotonic())]) and then conn._protocol = None.

connector.connect() already added this protocol to connector._acquired (see connector.py:693 and the create path). Normally the Connection object releases it via _release. By nulling conn._protocol, the Connection no longer references the protocol, so it is never removed from _acquired — while simultaneously being present in _conns as "available".

Why it matters:

  • The protocol is double-booked: counted as acquired and pooled. Every h2 host permanently consumes a limit / limit_per_host slot that is never released, so a long-lived session progressively exhausts the connection budget and eventually blocks new connects.
  • Keepalive cleanup, _cleanup_closed, and close() all assume connections move through the normal acquire/release path; hand-poking _conns sidesteps all of it.

How to fix: route h2 connection reuse through the connector's real release path (_release / a dedicated multiplexed-connection mode) rather than assigning to _conns and orphaning the acquired slot.

connector._conns[conn._key] = deque([(conn.protocol, time.monotonic())])
...
conn._protocol = None
3. Module sets logger level to DEBUG at import; frame send/recv logs eagerly on the hot path
aiohttp/http2/connection.py:44

logger.setLevel(logging.DEBUG) runs at import time. Libraries must not configure their own log levels — that is the application's decision. This forces DEBUG on aiohttp.http2.connection regardless of how the embedding app configured logging.

Compounding it, _send_frame builds an f-string on every frame sent:

logger.debug(f"-> FRAME type={frame_type.name:>15} ...")

f-strings are formatted eagerly before logger.debug is called, so the cost is paid per frame even when DEBUG output is discarded — and here it isn't discarded because the level was forced to DEBUG.

Why it matters: on a multiplexed connection this is per-DATA-frame overhead plus a flood of log lines into every user's logs. It hurts exactly the throughput the PR is trying to win.

How to fix: remove the module-level setLevel; use lazy logging (logger.debug("-> FRAME type=%s ...", frame_type.name)) and guard hot-path logging with if logger.isEnabledFor(logging.DEBUG).

logger = logging.getLogger("aiohttp.http2.connection")
logger.setLevel(logging.DEBUG)
4. Incoming frame handling: no frame-size cap, padding underflow, and a shared flow-control Event across streams
aiohttp/http2/connection.py:209-215

A few robustness gaps in the wire-parsing / multiplexing core that the feature depends on:

  • Padding underflow in _handle_data_frame: data = payload[pos : len(payload) - pad_length]. If a server sends pad_length >= len(payload) (which RFC 9113 §6.1 says MUST be a connection error), the end index goes negative and the body is silently corrupted rather than rejected.
  • No frame-size enforcement: data_received accepts any 24-bit length (up to 16 MB) and buffers until complete, without validating against the advertised MAX_FRAME_SIZE. A hostile/buggy peer can force large allocations.
  • Single _flow_control_updated Event shared by all streams: in send_data, every writer clears/sets the same event. With real multiplexing (the whole point of h2), one stream's set() can wake writers on unrelated streams whose windows are still zero, and a clear() can drop a wakeup another stream needed — a classic lost-wakeup race under concurrency.

Why it matters: these are correctness/robustness holes in the part of the code that must be bulletproof — an untrusted server is on the other end.

How to fix: validate pad_length < len(payload) and treat violations as protocol errors; enforce MAX_FRAME_SIZE on inbound frames; give each stream its own flow-control wait (per-stream event/future) or use a condition keyed by stream. Also clear the CodeQL-flagged dead imports/variables while here.

data = payload[pos : len(payload) - pad_length]  # negative end if pad_length too big

Checklist

  • Public API changes stay backward-compatible — critical #1
  • No performance regression on the default path — critical #2, warning #3
  • Resource/accounting cleanup on all paths — warning #2
  • Input validation at system boundaries (untrusted server frames) — warning #4
  • Opt-in flag behaves correctly — warning #1
  • No library-level logging configuration — warning #3
  • Docs + changelog fragment updated

To rebase specific severity levels, mention me: @aiolibsbot rebase critical (fixes 🔴 only), @aiolibsbot rebase important (fixes 🔴 + 🟡), or just @aiolibsbot rebase for all.


Silent Failure Analysis

🟠 **HIGH** — silent hang on error path
aiohttp/http2/connection.py:230-240

Risk: On HPACK decode failure the stream is reset on the wire but its response_future is never completed, so the caller awaiting stream.response_future in Http2Protocol.send hangs forever; additionally HPACK is a connection-global state that is now corrupted yet the connection keeps running.

try:
    headers = self.hpack_decoder.decode(payload)
except Exception as exc:
    logger.error(f"HPACK decode error: {exc}")
    self._send_rst_stream(stream_id, 1)  # PROTOCOL_ERROR
    return

Fix: Set the stream's response_future exception before returning and tear down the whole connection with GOAWAY (not just RST_STREAM) since the HPACK context is unrecoverable.

🟠 **HIGH** — protocol frame silently ignored
aiohttp/http2/connection.py:195-205

Risk: CONTINUATION frames carry the remainder of a header block that didn't fit in the HEADERS frame; routing them to a no-op handler silently drops those headers, leaving the stream permanently incomplete and its response_future unresolved (indefinite hang).

elif frame_type in {FrameType.PRIORITY, FrameType.PUSH_PROMISE, FrameType.CONTINUATION}:
    logger.warning("%d frame ignored (not implemented)", frame_type)
    self._handle_priority_frame(stream_id, payload)

Fix: Buffer and process CONTINUATION frames as part of the header block, and for genuinely-unsupported frames fail the affected stream/connection with an explicit error rather than dropping it.

🟡 **MEDIUM** — misleading truthiness gate
aiohttp/connector.py:868

Risk: os.getenv returns the raw string, so setting the variable to "0" or "false" (intending to disable) is truthy and silently enables experimental HTTP/2, the opposite of the operator's intent.

protocols = ["http/1.1"]
if os.getenv("AIOHTTP_ENABLE_EXPERIMENTAL_PROTOCOLS", False):
    protocols += ["h2"]

Fix: Parse the value explicitly, e.g. os.getenv(...) in ("1", "true", "yes"), so falsy string values disable the feature.

🟡 **MEDIUM** — unguarded set_exception aborts cleanup loop
aiohttp/http2/connection.py:145-155

Risk: Unlike the streams loop above it, this loop does not check fut.done(); a single already-cancelled/resolved pending future raises InvalidStateError, aborting the loop so the remaining queued requests are never notified and hang forever.

for fut in self._pending_streams:
    fut.set_exception(ConnectionError("Connection lost"))
self.streams.clear()

Fix: Guard with if not fut.done(): before set_exception, and clear self._pending_streams after failing them.

🟡 **MEDIUM** — malformed frame crashes callback, streams left hanging
aiohttp/http2/connection.py:95-110

Risk: Handlers do unchecked struct.unpack on attacker-controlled payloads (e.g. RST_STREAM/WINDOW_UPDATE with wrong length); the resulting exception escapes data_received, which asyncio only logs, leaving every in-flight stream's response_future unresolved and the connection wedged.

def data_received(self, data: bytes) -> None:
    ...
    self._dispatch_frame(frame_type_val, flags, stream_id, payload)

Fix: Wrap frame dispatch in a try/except that sends GOAWAY and fails all outstanding stream futures on a fatal parse error instead of letting the exception silently escape the transport callback.


Automated review by Kōan (Claude) HEAD=538164e 5 min 59s

@Moist-Cat

Copy link
Copy Markdown
Author

Either inheriting from or using ClientResponse directly appears to be the most architecturally sound approach (even though inheritance in this case constitutes a violation of the Liskov substitution principle), however this class is deeply coupled with HTTP/1.1. For example, the _startmethod callsprotocol.read()fromconnection. This is incompatible with h2because the protocol handles many streams, not just the one associated to the response andraise_for_statusdepends onreasonwhich doesn't exist inh2. A better solution is to create a "doppelganger" class that mimics the public interface, which is precisely what Http2Response` is. The public interface is the same so the high-level functionalities that rely on these (e.g., session cookies, redirects) keep work regardless of the underlying protocol. In other words, the API is backwards compatible as far as I tested.

Regarding the Semaphore, I believe simply allowing parallel connections when the flag is not set would be the best approach here since the general solution (i.e., verifying if the host supports h2) requires tracking the hosts in TCPConnector which doesn't seem trivial at glance. Improving performance can be done in another PR after the protocol is integrated and working.

To deal with limit for h2, it's important to decide whether to count streams (current behaviour) or TCP connections (calling _release after every successful connection while keeping the same protocol in the _acquired set).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants