Conversation
…eface with 400 Address review feedback on the h2c fallback path: - Rename --h2c to --http2-prior-knowledge: the flag only ever meant the RFC 9113 prior-knowledge form, so the name now says exactly that. - Only sniff the connection preface when the peer is trusted per forwarded_allow_ips (same list, same semantics as forwarded headers, unix socket peers included). Untrusted peers are served HTTP/1.1 exactly as if the flag were off - no sniff, no preface timeout. - A trusted peer that sends anything other than a valid preface (an HTTP/1.x request, a malformed or stalled preface) now gets an unconditional 400 via the new InvalidH2CPreface error instead of a silent HTTP/1.1 fallback. On a prior-knowledge port the trusted proxy is expected to speak HTTP/2; downgrading would only hide misconfiguration and invite odd interactions with keepalive and worker timeouts. The 101-upgrade dance remains deliberately unimplemented. Tests updated: trusted-peer rejection paths, untrusted-peer passthrough, wildcard allow list, unix socket peer. Full suite passes.
Moves preface matching, the trust check and the blocking preface read into gunicorn/http2/negotiation.py so gthread, gevent and the ASGI worker decide the same way. Only gthread is wired up here. The preface timeout becomes a budget for the whole preface rather than for each read, so a client sending one byte at a time cannot hold a connection, and a thread pool slot, for as long as the preface is long. http2_prior_knowledge becomes http2_cleartext, taking off, prior-knowledge, upgrade or both, because upgrade is coming and enabling one mechanism must not enable the other.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #3663 by @fluffy-dev, whose two commits are kept as they are. Closes
part of #3489.
The negotiation logic moves to a shared module so the three workers decide the
same way. Only gthread is wired up here; gevent and ASGI come next, and they read
their bytes very differently, which is exactly why the decision has to live in
one place and not be written three times.
Two things change from #3663.
The preface timeout was applied to each read instead of to the preface as a
whole. A client sending one byte at a time stayed under it forever and kept a
thread pool slot busy, which is what the slow-client protection in #3518 exists
to prevent. It is now a deadline for the whole preface.
http2_prior_knowledgebecomeshttp2_cleartext, which takesoff,prior-knowledge,upgradeorboth.Upgrade: h2cis coming later, andturning one mechanism on must not turn the other on. A boolean cannot say that.
Breaking changes
None for anyone: the setting it renames was never released.
--http2-prior-knowledgebecomes--http2-cleartext prior-knowledge. In aconfig file,
http2_cleartext = Trueis still understood and meansprior-knowledge, so the boolean form does not break silently.