Repository navigation
cache: fix background stale-while-revalidate subrequest failure for HTTP/2 downstream sessions - #1043
Open
stillnovice wants to merge 4 commits into
Open
stillnovice wants to merge 4 commits into
stillnovice wants to merge 4 commits into
Conversation
http_req_header_to_wire serialized an H2 request with an `HTTP/2` request-line version, which httparse rejects (Error::Version). Background stale-while-revalidate subrequests spawned from H2 downstream sessions therefore failed to parse, so process_subrequest returned early and the cache write permit was dropped while still held (debug: panic 'Dangling cache lock started!'; release: origin never revalidated and every subsequent stale request repeated the failure). Serialize H2 requests as HTTP/1.1 instead: the wire form is only consumed by HTTP/1 parsers (the H1 client writer and the subrequest dummy session). Adds a unit regression test and an end-to-end H2 test exercising the exact pseudo_raw_h1_request_header path used by SubrequestSpawner. Fixes cloudflare#1033
… parser - test_h2_request_to_wire_preserves_headers: method, target, and headers survive H2 -> H1 wire serialization; only the version changes - extend test_pseudo_raw_h1_request_header_is_valid_h1 to also parse the pseudo-raw bytes through pingora's HTTP/1 server session, the exact parse path a background subrequest takes
Author
|
Note on CI: the |
Nightly rustfmt now wants the block-comment terminator aligned with the
comment body (' */' instead of '*/'); unbreaks the nightly CI fmt check.
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
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.
Problem
When a proxy enables stale-while-revalidate, an HTTP/2 downstream request holding the cache write lock spawns a background subrequest that always fails (reported in #1033):
SubrequestSpawner::spawn_background_subrequestserializes the downstream request as HTTP/1 text viato_h1_raw(). For an HTTP/2 session,pseudo_raw_h1_request_header()wroteHTTP/2as the request-line version, andhttparserejects it withError::Version.process_subrequestthen hits theNone => returnbranch, socache_write_lock()is never taken — the write permit is dropped while still held. Debug builds panic withDangling cache lock started!; release builds release the permit as dangling, the origin never revalidates, and every subsequent stale request on the key repeats the failure.Fix
http_req_header_to_wire()now serializesVersion::HTTP_2asHTTP/1.1. This is safe: its only production callers are the HTTP/1 client writer (which only ever carries an HTTP/1.x version) andpseudo_raw_h1_request_header(), whose output is consumed exclusively by HTTP/1 parsers (the dummySessionV1subrequest session).Tests
test_h2_request_to_wire_uses_h1_version: unit regression test asserting an H2-versionedRequestHeaderserializes with anHTTP/1.1request line and parses via httparse.test_pseudo_raw_h1_request_header_is_valid_h1: end-to-end H2 test (real h2 handshake over a duplex stream) exercising the exactpseudo_raw_h1_request_header()path used bySubrequestSpawner.Verified both tests fail with
httparse::Error::Versionwhen the fix is reverted, and pass with it; existing wire-format and pingora-proxy subrequest tests still pass; clippy clean.Fixes #1033