Kludex/uvicorn
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Drop is_valid_h2c_settings, validate via the real upgrade path instead

Browse Source 
The pre-flight validator was running every payload through a throwaway
H2Connection just to ask "would h2 accept this?" before doing the same
work on the real connection. That's redundant: hyper-h2 raises out of
`initiate_upgrade_connection` for malformed payloads on its own.

Make `H2Protocol.initiate_h2c_upgrade` return a bool. The HTTP/1.1
caller now attempts the upgrade up front and only commits the
`101 Switching Protocols` response if hyper-h2 accepts the SETTINGS
payload. When hyper-h2 rejects it the H1 protocol replies with 400
instead of leaving the wire in a half-broken state.
This commit is contained in:
Marcelo Trylesinski 2026-05-03 10:07:39 +02:00
parent 84f4842ee1
commit 841948dc5d
4 changed files with 82 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
tests/protocols/test_http.py
View File

@ -1181,13 +1181,6 @@ async def test_h2c_upgrade_disabled_with_http2_false(http_protocol_cls: type[HTT
pytest.param(h2c_upgrade_request(settings=None), id="missing_http2_settings_header"),
pytest.param(h2c_upgrade_request(connection=b"Upgrade"), id="missing_connection_token"),
pytest.param(h2c_upgrade_request(settings=b""), id="empty_http2_settings_value"),
pytest.param(h2c_upgrade_request(settings=b"!!not-base64!!"), id="non_base64_http2_settings"),
# `AAAA` decodes to 3 bytes, which is shorter than one SETTINGS entry (6 bytes), so
# SettingsFrame.parse_body rejects it before we commit to the upgrade.
pytest.param(h2c_upgrade_request(settings=b"AAAA"), id="invalid_settings_frame_body"),
# `AAIAAAAC` encodes ENABLE_PUSH=2, which is syntactically a valid SETTINGS body but
# an invalid value (must be 0 or 1). h2 raises InvalidSettingsValueError.
pytest.param(h2c_upgrade_request(settings=b"AAIAAAAC"), id="invalid_settings_value"),
pytest.param(
h2c_upgrade_request(extra_settings=b"AAMAAABkAAQBAAAAAAIAAAAA"),
id="duplicate_http2_settings_header",
@ -1204,12 +1197,14 @@ async def test_h2c_upgrade_disabled_with_http2_false(http_protocol_cls: type[HTT
),
],
)
async def test_h2c_upgrade_rejected_for_malformed_request(
async def test_h2c_upgrade_request_falls_back_to_http1(
http_protocol_cls: type[HTTPProtocol],
request_bytes: bytes,
caplog: pytest.LogCaptureFixture,
):
"""A malformed h2c upgrade must NOT switch protocols and must NOT send 101."""
"""When the upgrade request is structurally malformed (missing required
headers, has a body), the server falls back to plain HTTP/1.1 instead of
sending `101 Switching Protocols`."""
caplog.set_level(logging.WARNING, logger="uvicorn.error")
app = Response("Hello, world", media_type="text/plain")
protocol = get_connected_protocol(app, http_protocol_cls, http2=True)
@ -1222,6 +1217,34 @@ async def test_h2c_upgrade_rejected_for_malformed_request(
assert any("Ignoring h2c upgrade" in record.getMessage() for record in caplog.records)
@skip_if_no_h2
@pytest.mark.parametrize(
"request_bytes",
[
pytest.param(h2c_upgrade_request(settings=b"!!not-base64!!"), id="non_base64_http2_settings"),
# `AAAA` decodes to 3 bytes, which is shorter than one SETTINGS entry (6 bytes).
pytest.param(h2c_upgrade_request(settings=b"AAAA"), id="invalid_settings_frame_body"),
# `AAIAAAAC` encodes ENABLE_PUSH=2, which is a valid SETTINGS body shape but
# an invalid value (must be 0 or 1).
pytest.param(h2c_upgrade_request(settings=b"AAIAAAAC"), id="invalid_settings_value"),
],
)
async def test_h2c_upgrade_with_invalid_settings_returns_400(
http_protocol_cls: type[HTTPProtocol],
request_bytes: bytes,
):
"""When the HTTP2-Settings payload is well-framed enough to attempt the
upgrade but hyper-h2 rejects the SETTINGS contents, the server replies
with 400 instead of leaving the wire in a half-broken state."""
app = Response("Hello, world", media_type="text/plain")
protocol = get_connected_protocol(app, http_protocol_cls, http2=True)
protocol.data_received(request_bytes)
assert b"HTTP/1.1 400" in protocol.transport.buffer
assert b"HTTP/1.1 101 Switching Protocols" not in protocol.transport.buffer
assert protocol.transport.get_protocol() is None
async def test_header_upgrade_is_websocket_depend_not_installed(
caplog: pytest.LogCaptureFixture, http_protocol_cls: type[HTTPProtocol]
):

35
uvicorn/protocols/http/h11_impl.py
View File

@ -204,10 +204,11 @@ class H11Protocol(asyncio.Protocol):
def _get_h2c_settings(self, connection_tokens: list[bytes]) -> bytes | None:
# Per RFC 7540 section 3.2, an h2c upgrade requires the `HTTP2-Settings`
# connection-option and exactly one valid `HTTP2-Settings` header field whose value
# is a base64url-encoded SETTINGS frame payload. Requests that carry a body cannot
# be upgraded because we'd lose the body bytes when we hand stream 1 to h2.
# Returns the validated raw header value, or None if the upgrade is not valid.
# connection-option and exactly one `HTTP2-Settings` header field. Requests
# that carry a body cannot be upgraded because we'd lose the body bytes
# when we hand stream 1 to h2. The actual SETTINGS payload is validated
# later by `H2Protocol.initiate_h2c_upgrade`, which only commits the
# protocol switch if hyper-h2 accepts the payload.
if b"http2-settings" not in connection_tokens:
return None
seen: bytes | None = None
@ -220,11 +221,6 @@ class H11Protocol(asyncio.Protocol):
return None
elif name == b"transfer-encoding":
return None
if seen is None:
return None
assert self.h2_protocol_class is not None
if not self.h2_protocol_class.is_valid_h2c_settings(seen):
return None
return seen
def _get_upgrade_type(self) -> str | None:
@ -379,25 +375,17 @@ class H11Protocol(asyncio.Protocol):
def handle_h2c_upgrade(self, event: h11.Request) -> None:
"""Handle HTTP/2 cleartext (h2c) upgrade request."""
assert self.h2_protocol_class is not None
# `_get_upgrade_type` validates the HTTP2-Settings header before this is reached.
assert self._h2c_settings is not None
if self.logger.level <= TRACE_LOG_LEVEL: # pragma: full coverage
prefix = "%s:%d - " % self.client if self.client else ""
self.logger.log(TRACE_LOG_LEVEL, "%sUpgrading to HTTP/2 (h2c)", prefix)
self.connections.discard(self)
# Bytes the peer already sent after the upgrade request - typically
# the HTTP/2 client preface (`PRI * HTTP/2.0...`) and the first
# frames - need to be forwarded to the new protocol after the switch
# so the connection doesn't stall on lost initial frames.
trailing = self.conn.trailing_data[0]
# Send 101 Switching Protocols response
response = b"HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: h2c\r\n\r\n"
self.transport.write(response)
# Create and switch to H2Protocol
h2_protocol = self.h2_protocol_class(
config=self.config,
server_state=self.server_state,
@ -405,15 +393,22 @@ class H11Protocol(asyncio.Protocol):
_loop=self.loop,
)
# Initialize the H2 connection for upgrade
h2_protocol.initiate_h2c_upgrade(
# Try the upgrade first; only commit `101 Switching Protocols` if h2
# accepts the SETTINGS payload. Otherwise the wire would be left in
# a half-broken state with the peer expecting HTTP/2 and the server
# unable to speak it.
if not h2_protocol.initiate_h2c_upgrade(
self.transport,
event.method.decode("ascii"),
event.target.decode("ascii"),
self.headers,
self._h2c_settings,
)
):
self.send_400_response("Invalid HTTP2-Settings header for h2c upgrade")
return
self.connections.discard(self)
self.transport.write(b"HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: h2c\r\n\r\n")
self.transport.set_protocol(h2_protocol)
if trailing:
h2_protocol.data_received(trailing)

71
uvicorn/protocols/http/h2_impl.py
View File

@ -9,8 +9,6 @@ References:
from __future__ import annotations
import asyncio
import base64
import binascii
import contextlib
import contextvars
import logging
@ -76,15 +74,6 @@ class HTTP2Protocol(asyncio.Protocol):
_loop: Any = None,
) -> None: ...
@staticmethod
def is_valid_h2c_settings(value: bytes) -> bool:
"""Return True if `value` is a valid HTTP2-Settings header payload.
Used by HTTP/1.1 protocols to validate an h2c upgrade request before
committing to `101 Switching Protocols`.
"""
raise NotImplementedError
def initiate_h2c_upgrade(
self,
transport: asyncio.Transport,
@ -92,16 +81,15 @@ class HTTP2Protocol(asyncio.Protocol):
path: str,
headers: list[tuple[bytes, bytes]],
http2_settings: bytes,
) -> None:
) -> bool:
"""Initialize an HTTP/2 connection from an h2c upgrade request.
This method is called by HTTP/1.1 protocols when they receive an h2c upgrade request. The implementation should:
1. Set up the HTTP/2 connection state
2. Process the original request as stream 1
3. Start handling HTTP/2 frames
The caller must validate `http2_settings` via `is_valid_h2c_settings` before invoking this method.
Returns True when the upgrade succeeded and the caller should send the
`101 Switching Protocols` response, False when the HTTP2-Settings
payload was rejected by the HTTP/2 stack and the caller should keep
speaking HTTP/1.1.
"""
raise NotImplementedError
class H2Protocol(HTTP2Protocol):
@ -171,30 +159,6 @@ class H2Protocol(HTTP2Protocol):
# is closed.
self._unmatched_resets = 0
@staticmethod
def is_valid_h2c_settings(value: bytes) -> bool:
"""Return True if `value` is a valid base64url-encoded SETTINGS payload.
Validates the full payload (frame body + setting values) by running it
through h2's own upgrade machinery on a throwaway connection. h2 raises
`InvalidSettingsValueError` for things like `ENABLE_PUSH=2`, so this
catches every case h2 would reject after the upgrade was committed.
"""
try:
# Strict validation rejects characters outside the base64url alphabet
# that `urlsafe_b64decode` would otherwise silently strip
# (e.g. `AAMAAABk!!`).
base64.b64decode(value.replace(b"-", b"+").replace(b"_", b"/"), validate=True)
except (binascii.Error, ValueError):
return False
try:
H2Connection(config=H2Configuration(client_side=False, header_encoding=None)).initiate_upgrade_connection(
value
)
except (ProtocolError, HyperframeError, ValueError):
return False
return True
# Protocol interface
def connection_made(self, transport: asyncio.Transport) -> None: # type: ignore[override]
self.connections.add(self)
@ -220,24 +184,28 @@ class H2Protocol(HTTP2Protocol):
path: str,
headers: list[tuple[bytes, bytes]],
http2_settings: bytes,
) -> None:
) -> bool:
"""Initialize HTTP/2 connection for h2c upgrade.
This is called when upgrading from HTTP/1.1 to HTTP/2 cleartext.
The 101 Switching Protocols response has already been sent and
`http2_settings` is the validated raw HTTP2-Settings header value.
Returns True if hyper-h2 accepts the HTTP2-Settings payload (and the
caller should send `101 Switching Protocols`), False otherwise.
"""
self.connections.add(self)
# Try the upgrade up front: hyper-h2 raises if the SETTINGS payload is
# malformed or has out-of-spec values, and we want to discover that
# before the caller commits to `101 Switching Protocols`. ProtocolError
# and HyperframeError cover the documented failures; ValueError covers
# the base64 decode that h2 does internally.
try:
self.conn.initiate_upgrade_connection(http2_settings)
except (ProtocolError, HyperframeError, ValueError):
return False
self.connections.add(self)
self.transport = transport
self.flow = FlowControl(transport)
self.server = get_local_addr(transport)
self.client = get_remote_addr(transport)
self.scheme = "http"
# Initialize HTTP/2 connection for upgrade
# This handles the upgrade case where client sent HTTP/1.1 request with Upgrade: h2c
self.conn.initiate_upgrade_connection(http2_settings)
self.transport.write(self.conn.data_to_send())
if self.logger.level <= TRACE_LOG_LEVEL: # pragma: no cover
@ -247,6 +215,7 @@ class H2Protocol(HTTP2Protocol):
# The initial HTTP/1.1 request becomes stream 1 in HTTP/2
# We need to process it as if we received a RequestReceived event
self._handle_upgraded_request(method, path, headers)
return True
def _handle_upgraded_request(
self,

35
uvicorn/protocols/http/httptools_impl.py
View File

@ -206,10 +206,11 @@ class HttpToolsProtocol(asyncio.Protocol):
def _get_h2c_settings(self, connection_tokens: list[bytes]) -> bytes | None:
# Per RFC 7540 section 3.2, an h2c upgrade requires the `HTTP2-Settings`
# connection-option and exactly one valid `HTTP2-Settings` header field whose value
# is a base64url-encoded SETTINGS frame payload. Requests that carry a body cannot
# be upgraded because we'd lose the body bytes when we hand stream 1 to h2.
# Returns the validated raw header value, or None if the upgrade is not valid.
# connection-option and exactly one `HTTP2-Settings` header field. Requests
# that carry a body cannot be upgraded because we'd lose the body bytes
# when we hand stream 1 to h2. The actual SETTINGS payload is validated
# later by `H2Protocol.initiate_h2c_upgrade`, which only commits the
# protocol switch if hyper-h2 accepts the payload.
if b"http2-settings" not in connection_tokens:
return None
seen: bytes | None = None
@ -222,11 +223,6 @@ class HttpToolsProtocol(asyncio.Protocol):
return None
elif name == b"transfer-encoding":
return None
if seen is None:
return None
assert self.h2_protocol_class is not None
if not self.h2_protocol_class.is_valid_h2c_settings(seen):
return None
return seen
def _get_upgrade_type(self) -> str | None:
@ -300,19 +296,11 @@ class HttpToolsProtocol(asyncio.Protocol):
immediately after the Upgrade headers are not lost.
"""
assert self.h2_protocol_class is not None
# `_get_upgrade_type` validates the HTTP2-Settings header before this is reached.
assert self._h2c_settings is not None
if self.logger.level <= TRACE_LOG_LEVEL: # pragma: full coverage
prefix = "%s:%d - " % self.client if self.client else ""
self.logger.log(TRACE_LOG_LEVEL, "%sUpgrading to HTTP/2 (h2c)", prefix)
self.connections.discard(self)
# Send 101 Switching Protocols response
response = b"HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: h2c\r\n\r\n"
self.transport.write(response)
# Create and switch to H2Protocol
h2_protocol = self.h2_protocol_class(
config=self.config,
server_state=self.server_state,
@ -320,15 +308,22 @@ class HttpToolsProtocol(asyncio.Protocol):
_loop=self.loop,
)
# Initialize the H2 connection for upgrade
h2_protocol.initiate_h2c_upgrade(
# Try the upgrade first; only commit `101 Switching Protocols` if h2
# accepts the SETTINGS payload. Otherwise the wire would be left in
# a half-broken state with the peer expecting HTTP/2 and the server
# unable to speak it.
if not h2_protocol.initiate_h2c_upgrade(
self.transport,
self.scope["method"],
self.url.decode("ascii"),
list(self.scope["headers"]),
self._h2c_settings,
)
):
self.send_400_response("Invalid HTTP2-Settings header for h2c upgrade")
return
self.connections.discard(self)
self.transport.write(b"HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: h2c\r\n\r\n")
self.transport.set_protocol(h2_protocol)
if trailing_data:
h2_protocol.data_received(trailing_data)