Merge upstream cloudflare/pingora main - #27
Merged
Conversation
The default Buf::chunks_vectored impl only populates one IoSlice with self.chunk(). For WriteBuf::Chained, that collapses a multi-piece chunked-encoding frame (size header + payload + trailing CRLF) into a single-slice vectored write, so tokio's write_all_buf ends up issuing one poll_write_vectored per inner piece instead of one writev covering the whole frame. Forward chunks_vectored through WriteBuf to the inner Buf so the full chain is exposed to the writer. bytes::buf::Chain already implements chunks_vectored correctly (recursing into both sides), so the forward is sufficient; no other Buf impls in pingora-core need the same fix. Tighten the chunked_write_uses_vectored_path test (previously documented the defect in a comment) to assert exactly one poll_write_vectored call for the canonical 3-piece frame, and update chunked_write_resumes_on_ short_write to exercise the vectored short-write path. Expand the poll-based body writer test module with additional coverage: - chunked_write_non_vectored_writer: pins the existing non-vectored poll_write-per-piece behavior so a future refactor cannot silently break the non-vectored path. - chunked_write_resumes_after_pending: chunked counterpart to the existing content-length Pending-then-resume test, covering the cancel-safe per-poll re-creation of tokio's WriteAllBuf future. - chunked_write_two_frames_in_sequence: drives two consecutive send_body_task calls to catch state-machine leaks between frames. - content_length_write_uses_vectored_path: exercises the Simple arm of the chunks_vectored override under vectored I/O.
Missing entries can now be admitted through incremental weight updates, so use the same nonzero weight floor as normal admission. Existing entries should not shrink when a later max-weight hint is below the current tracked weight. The hint only limits growth while the tracked weight is still below it.
Stop readers from waiting on cache locks forever when each retry still finds a miss, a stale object that cannot be served, or an object close to expiry. These requests now bypass cache after the retry budget instead of joining the same ineffective loop again. Add no-cache reasons for the retry limit and ineffective retry cases, and avoid remembering those keys as uncacheable.
Make a previously-hardcoded limit configurable. This also removes the ineffective-retry path because it was ... ineffective.
Downstream sessions that do not report incomplete body errors, or upstream request mutations that create inconsistent framing, can reach this state. Prevent reuse of the partially written HTTP/1 upstream connection as defense in depth.
Cache miss finalization failures now fail only cache admission, not the downstream response. The client already has a valid upstream response. A late storage failure should leave us with a miss, not a broken request.
Note: this is defense-in-depth, not a fix for a default-configuration issue. The standard HTTP/1.1 request parser already rejects these bytes in the request target, so a normally-parsed request can never carry them into this path. It is only reachable when request headers are built from an alternative, non-parser source that does not apply the same validation to untrusted input.
Apply the same `arrayvec` element-by-element array init pattern that `pingora_lru::Lru<T, N>::with_capacity_and_watermark` already uses, to both `ConcurrentHashTable<V, N>` and `ConcurrentLruCache<V, N>` in `pingora-cache/src/hashtable.rs`. The stdlib only auto-derives `Default` for arrays up to N=32, so the prior `[T; N]: Default` bounds on these impl blocks silently capped practical shard counts at 32. `pingora-lru` had the same problem and had already solved it with `arrayvec::ArrayVec::<_, N>::new()` plus `push()` in a loop; this change brings the same fix to the sibling `pingora-cache` types so all three sharded structures (`Lru`, `ConcurrentHashTable`, `ConcurrentLruCache`) support identical N values under the same construction idiom. Behavior is unchanged for existing N <= 32 callers. Adds `arrayvec` as a direct dependency of `pingora-cache` (it's already used transitively through `pingora-lru`).
The arrayvec refactor means ConcurrentLruCache::new no longer needs [LruShard<V>; N]: Default, so: - Remove the now-dead LruShard::Default impl, which produced a semantically wrong unbounded LruCache and was never reached after the refactor. - Drop the stale where [LruShard<()>; N_SHARDS]: Default bounds (and the obsolete explanatory comment) from the Predictor impls. Those bounds silently capped Predictor at <=32 shards, partially defeating the purpose of lifting the Default bound. Also drop the now-unused LruShard import. Add a test constructing a Predictor with 64 shards to confirm shard counts above 32 now work.
A stale refresh can rewrite the primary variant metadata with a new created timestamp. Vary lookups use the primary metadata as the floor for secondary variants, so that can make an older sibling variant look invalid and fall through to a miss. Stamp provenance on new metadata, carry it across stale refreshes and revalidation, and reset it when the Vary family changes. Add focused coverage for the metadata transitions and the secondary lookup filter.
Add configurable HTTP upstream request-header policy with standards-oriented defaults and explicit legacy compatibility modes. Strip hop-by-hop and Connection-nominated fields by default, reject sensitive nominations, normalize supported WebSocket upgrades, and prevent inconsistent 101 tunnel transitions. Finalize HTTP/1 request framing after application upstream filters: preserve explicit outgoing framing and synthesize chunked encoding for remaining non-empty unframed bodies. Apply related sanitization to HTTP/2-bound requests and add integration coverage for framing, upgrades, protected nominations, and compatibility settings.
…hods Includes-commit: 2bf0208 Replicated-from: cloudflare#890
…hints Fixes cloudflare#287 --- fix: resolve cloudflare#287 — [Doc] Provide simple examples with doc hints Fixes cloudflare#287 Includes-commit: 3301261 Includes-commit: bedd6ae Replicated-from: cloudflare#898 Signed-off-by: ChinhLee <76194645+chinhkrb113@users.noreply.github.com>
Fixes cloudflare#295 Includes-commit: c5ab359 Replicated-from: cloudflare#897 Signed-off-by: ChinhLee <76194645+chinhkrb113@users.noreply.github.com>
Allow the h2 proxy test to pass whether the response body DATA frame is observed before the downstream stream reset or the stream reset arrives first. Both orderings are valid for this error path, and newer h2 behavior can surface RST_STREAM(CANCEL) before the buffered body chunk is delivered. Also keep the h2 drain-loop cleanup explicit by using clearer expect messages and releasing flow-control capacity while draining the response.
Adapt the change to the released crates.io boring 5.x packages while preserving the open source feature defaults.
Set the GitHub Actions MSRV lane to Rust 1.85.
Once a queued downstream proxy-task write reports an error during cache fill, the proxy loop already marks the downstream state errored and propagates that first error to the downstream session. Continuing to enter the queued-task branch after that point only polls the same failed downstream path unnecessarily while cache fill continues.
openssl 0.10.77 deprecated `Asn1StringRef::as_utf8`, which truncates at the first interior NUL byte. Decode subject-name ASN.1 strings with `as_slice()` + `String::from_utf8_lossy` instead. This API is not deprecated, is available on both the openssl and boringssl backends, and no longer truncates at interior NUL bytes.
Store accepted HTTP/2 sessions inline in H2Accept. This removes the heap allocation and immediate deallocation on the accepted-stream path while preserving rejected-stream and connection-close outcomes.
This introduces foundations in a separate crate for users who would like to use https://github.com/cloudflare/foundations, which provides a common set of utilities for telemetry and exports useful metrics out of the box.
Add grouped selectors with asynchronous bounded rebuilds and shared active health across backend views.
Downstream HTTP/1 switches to upgraded connection state after it writes a successful final 101 response for an upgrade request. The upstream/custom task stream is typed separately, so normal HTTP body tasks after that point indicate a protocol-state mismatch. Track downstream 101 state before batched or queued task writes. Record the upstream H1 upgrade request predicate and include it with the live downstream predicate in returned errors when rejecting mismatched 101 responses or invalid post-upgrade tasks. Reject 101 responses when downstream and upstream H1 upgrade request predicates differ, reject normal Header, Body, or Trailer tasks after downstream 101, and reject UpgradedBody before downstream 101. Allow explicit UpgradedBody after the 101. Keep Done valid after the 101: run response_done_filter as before, and write any returned bytes as UpgradedBody so they are not HTTP-framed. Attribute raw upstream 101 mismatches as upstream errors and local filter/module/task-stream invariant violations as internal errors, so fail_to_proxy, stale serving, and cache error classification do not blame the wrong side. Add regression coverage for batched and queued task writes, response_filter-created and upstream-created 101 mismatches, downstream module-created 101s, and body/trailer/upgraded-body/done edge cases.
…write is blocked.
## Summary
Replace `pub type DirectiveKey = String` with a proper enum that has variants for all 14 known RFC 9111 Cache-Control directives plus an `Unknown(String)` fallback for extension directives.
This eliminates heap allocations for known directive keys during parsing and manipulation. The parser maps lowercased tokens to enum variants at parse time; unknown directives still allocate via the `Unknown` variant.
## Motivation
`DirectiveKey` is used as the key type in `DirectiveMap = IndexMap<DirectiveKey, Option<DirectiveValue>>`, which is the core data structure of `CacheControl`. Every parsed Cache-Control directive currently allocates a `String` for its key via `token.to_lowercase()`, even though the vast majority of directives are static RFC-defined names (`max-age`, `s-maxage`, `no-cache`, etc.).
## Public API
### New type
```rust
pub enum DirectiveKey {
MaxAge, SMaxAge, NoCache, NoStore, Private, Public,
MustRevalidate, ProxyRevalidate, MustUnderstand, NoTransform,
Immutable, StaleWhileRevalidate, StaleIfError, OnlyIfCached,
Unknown(String),
}
```
### New methods/impls
- `DirectiveKey::as_str()` — returns the wire-format name (`&'static str` for known variants)
- `DirectiveKey::from_lowercase(&str)` — construct from a lowercase string (zero alloc for known directives)
- `DirectiveKey::from_lowercase_owned(String)` — same, but takes ownership to avoid re-allocation for unknown directives
- `CacheControl::has_directive(&DirectiveKey)` — enum-based key lookup
- `Display`, `PartialEq<str>`, `PartialEq<&str>` impls
### Backward compatibility
- `has_key(&str)` preserved as a convenience wrapper
- All named methods (`max_age()`, `public()`, etc.) continue to work unchanged
- `PartialEq<&str>` allows existing `== "directive-name"` comparisons to compile
## Testing
- All 144 existing tests pass unchanged
- `cargo clippy` clean
- `cargo fmt` clean
Match hyper's content-length parsing: identical duplicate and comma-combined identical Content-Length values are reconciled to a single value, while conflicting or unparseable values are treated as an unrecoverable framing error. Apply the same rule uniformly to HTTP/1 request and response reads and HTTP/2 request and response reads, so ambiguous framing is rejected at the origin/downstream boundary instead of being forwarded (and possibly re-interpreted when a request or response is downgraded to HTTP/1). HTTP/2 rejects the offending stream with PROTOCOL_ERROR; HTTP/1 fails the connection. Conflicting or duplicate Content-Length is always rejected. The allow_h1_response_invalid_content_length option now only tolerates a single, otherwise-invalid value (treated as close-delimited); it never tolerates conflicting framing. Resolve the framing length through a single range-checked helper so a value exceeding usize is rejected rather than silently truncated, and route body-writer framing through the same path. Bound the number of consecutive malformed HTTP/2 streams a single accept call will reset before tearing the connection down, so a client cannot hold a connection in an unbounded reset loop. Connection teardown is logged at warn; per-stream/per-message framing errors are logged at debug to avoid log floods under adversarial load. Add unit and integration tests for the reconcile/reject paths, the single-value tolerance option, the single-token helper, and HTTP/2 request rejection.
Includes-commit: 75df529 Replicated-from: cloudflare#908 # Conflicts: # pingora-core/src/listeners/tls/rustls/mod.rs # Conflicts: # pingora-core/src/connectors/l4.rs
Transfer pipelined request suffixes through header and body parsing as owned BytesMut buffers so each request can be split off without copying the remaining queue. Preserve cancellation safety for partial chunk headers and add regression tests and Criterion coverage.
…st-body write is blocked." This reverts commit 2278e5b1b9a7f86d98196a608d44ff47cc076c37.
Callers that run before ResponseCompressionCtx::response_header_filter have no way to ask whether that filter is about to replace the response body with its decompressed form, so they have to reimplement the ctx's internal decision logic, which then silently drifts whenever that logic changes. For example, a caller framing a byte range needs to know that the body it measured is about to be swapped out, invalidating both the offsets and the length it computed. Add ResponseCompressionCtx::will_decompress(), which reuses decide_action and the same per-algorithm decompress_enable flags the filter uses, so it cannot disagree with it. It returns false in the body phase rather than panicking, since it is a read-only query. Add Algorithm::can_decompress(), which reports whether a decompressor is implemented at all. This matters because decompression can be enabled for every algorithm including Algorithm::Other, and a multi-coding value such as "Content-Encoding: gzip, br" parses to Other, so decide_action returns Decompress(Other) even though nothing is actually decompressed. Both derive from a single private decompressor_impl() whose match is exhaustive, so the set of decompressible algorithms is stated once and a newly added algorithm fails to compile until it is considered. No behaviour change: decompressor() now goes through the same helper and returns exactly what it did before.
Track custom upstream response bytes while bypassing a previously oversized response. Clear the predictor only after a successful response completes within the configured cache size limit.
CacheKey hashed namespace and primary as an unframed byte concatenation, allowing distinct component pairs to produce the same key. Remove the misleading namespace field and constructor argument. Callers that need multiple components must encode their boundaries in the primary bytes themselves. Document migration behavior and cover both legacy raw concatenation and length-prefixed framing. Keys previously created with a non-empty namespace will change unless callers preserve the exact legacy concatenation, so adopting framed keys creates a cold cache.
During a zero downtime upgrade, unclaimed inherited fds were left open and transferred to later generations. With SO_REUSEPORT these sockets can black-hole connections because they remain active without an acceptor. Track complete service listen-address sets and close inherited fds not claimed after services are registered. If any service cannot report a complete set, skip cleanup. Adds coverage for both bootstrap paths and custom services.
Remove obsolete namespace arguments from the custom response cache predictor tests while preserving their primary keys and user tags.
pingora-foundations depends on darling 0.23.0, which requires rustc 1.88+, so cargo check fails under the 1.85.0 matrix entry now that the crate is part of the workspace. Exclude it from that toolchain only; it's still checked under the other toolchains in the matrix.
Keep the CI matrix's stable entry current.
Syncs 76 upstream commits. Conflict resolutions: - .github/workflows/build.yml: take upstream's latest-stable bump (1.91.1 -> 1.97.1); keep the fork's note explaining why MSRV is pinned at 1.85.0 (edition 2024, required by proxy-protocol >=0.5.3). - listeners/tls/boringssl_openssl/mod.rs: union the imports. The fork needs GetSocketDigest (ClientHello peer-addr extraction), upstream needs ServerConf (handshake-offload threadpool config). - pingora-http/src/lib.rs: upstream independently adopted the same authority-form/asterisk-form raw_path() fix the fork carried, so adopt upstream's combinator form verbatim to avoid re-conflicting. Behavior is identical; test_authority_form_raw_path still passes. Same for two test assertions that differed only by message string. Also fixes a silent semantic conflict git merged without markers: upstream's "Add downstream TLS handshake offload" changed handshake_with_callback()'s third parameter from SharedTlsAcceptCallbacks to &(dyn TlsAccept + Send + Sync), but the fork-only tls_handshake_with_client_hello() call site was not updated and no longer compiled under --features openssl/boringssl. Pass cb.as_ref() to match. Verified: cargo fmt --check; cargo check -p pingora-core for each of openssl/boringssl/rustls/s2n; CI's exact clippy --all-targets --all --deny=warnings; pingora-http tests 25/25.
h2 0.3.27 arrives through the same dial9 worker-s3 dependency path the existing rustls-webpki exemptions cover: aws-sdk-s3-transfer-manager -> aws-config -> aws-smithy-runtime -> aws-smithy-http-client, which still builds on hyper 0.14. The latest published aws-smithy-http-client (1.4.0) has no hyper 1.x release, so the advisory's ">= 0.4.16" remedy is not reachable by any version bump on our side. The vulnerable copy is used only as an HTTP/2 client talking to S3; pingora's own serving path resolves the workspace h2 (0.4.19) and is unaffected. Advisory postdates the last upstream sync, so this failure is present on main independently of the merge.
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
14 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.
Syncs 76 commits from upstream
cloudflare/pingorainto the fork, superseding #26 (which could not be merged from the upstream branch directly due to conflicts).Conflict resolutions
.github/workflows/build.yml— took upstream's latest-stable bump (1.91.1→1.97.1); kept the comment explaining why MSRV stays pinned at1.85.0(edition 2024, required byproxy-protocol >= 0.5.3).pingora-core/src/listeners/tls/boringssl_openssl/mod.rs— union of the imports.GetSocketDigestis needed by the ClientHello peer-address path;ServerConfis needed by upstream's new handshake-offload threadpool config. Both verified in use.pingora-http/src/lib.rs— upstream independently arrived at the same authority-form / asterisk-formraw_path()fix this fork already carried, expressed with combinators instead of an if-let chain. Adopted upstream's form verbatim so the function stops re-conflicting on every sync; behavior is identical andtest_authority_form_raw_pathstill passes. Two test assertions that differed only by an assert message were likewise taken from upstream.Semantic conflict git merged without markers
Upstream's "Add downstream TLS handshake offload" changed
handshake_with_callback's third parameter fromSharedTlsAcceptCallbacksto&(dyn TlsAccept + Send + Sync). It updated its own call site, buttls_handshake_with_client_hellodoes not exist upstream, so its call site was left untouched and the merged tree did not compile under--features opensslor--features boringssl. Resolved by passingcb.as_ref(), matching the adjacent call site.Note for future syncs:
pingora-corehasdefault = [], socargo check --workspacecompiles none of the TLS backends and passes over exactly this class of breakage. The per-feature checks below are the ones that catch it.Verification
cargo fmt --all -- --check— cleancargo check -p pingora-core --all-targetsfor each ofopenssl,boringssl,rustls,s2n— cleancargo clippy --all-targets --all -- --allow=unknown-lints --deny=warnings(CI's exact invocation) — exit 0pingora-httpunit tests — 25/25pingora-coreunit tests — 590/591The single failure is
connectors::l4::tests::test_conn_error_addr_not_avail, which is environmental rather than a regression: it binds192.0.2.2:0(TEST-NET-1) expectingEADDRNOTAVAILand gets a different errno under a sandboxed network namespace. It fails identically on the pre-merge base commit.