Skip to content

cache: fix background stale-while-revalidate subrequest failure for HTTP/2 downstream sessions - #1043

Open
stillnovice wants to merge 4 commits into
cloudflare:mainfrom
stillnovice:fix-h2-subrequest-cache-lock
Open

stillnovice wants to merge 4 commits into
cloudflare:mainfrom
stillnovice:fix-h2-subrequest-cache-lock

Conversation

@stillnovice

Copy link
Copy Markdown

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):

  1. SubrequestSpawner::spawn_background_subrequest serializes the downstream request as HTTP/1 text via to_h1_raw(). For an HTTP/2 session, pseudo_raw_h1_request_header() wrote HTTP/2 as the request-line version, and httparse rejects it with Error::Version.
  2. process_subrequest then hits the None => return branch, so cache_write_lock() is never taken — the write permit is dropped while still held. Debug builds panic with Dangling 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 serializes Version::HTTP_2 as HTTP/1.1. This is safe: its only production callers are the HTTP/1 client writer (which only ever carries an HTTP/1.x version) and pseudo_raw_h1_request_header(), whose output is consumed exclusively by HTTP/1 parsers (the dummy SessionV1 subrequest session).

Tests

  • test_h2_request_to_wire_uses_h1_version: unit regression test asserting an H2-versioned RequestHeader serializes with an HTTP/1.1 request 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 exact pseudo_raw_h1_request_header() path used by SubrequestSpawner.

Verified both tests fail with httparse::Error::Version when the fix is reverted, and pass with it; existing wire-format and pingora-proxy subrequest tests still pass; clippy clean.

Fixes #1033

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
@stillnovice

Copy link
Copy Markdown
Author

Note on CI: the pingora (nightly) job fails at cargo fmt --all -- --check on pingora-cache/src/key.rs:187 — a doc-comment style change (*/ → */) introduced by a recent nightly rustfmt. That file is untouched by this PR; the failure is pre-existing style drift against the latest nightly toolchain, not caused by these changes. Stable (1.97.1) and MSRV (1.85.0) jobs pass. Happy to include the one-line rustfmt fix here if maintainers prefer.

Nightly rustfmt now wants the block-comment terminator aligned with the
comment body (' */' instead of '*/'); unbreaks the nightly CI fmt check.
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.

cache: background stale-while-revalidate subrequest fails for HTTP/2 downstream sessions

1 participant