Support early request body buffering before upstream peer selection by CodyPubNub · Pull Request #816 · cloudflare/pingora

CodyPubNub · 2026-02-16T19:42:59Z

Resolves #780

Adds opt-in early body buffering to ProxyHttp. When early_request_body_buffer_limit() returns Some(max_size), the full request body is read before request_filter runs. The buffered body is available via Session::get_buffered_body() for inspection and Session::set_buffered_body() for mutation, and is automatically forwarded to upstream during the proxy phase.

New trait methods:

early_request_body_buffer_limit() — opt in to buffering with a size limit (default None)
early_request_body_filter() — per-chunk callback during early buffering, before any header-phase filters run. Use for streaming processing (e.g., decompression) that doesn't depend on request_filter state. The normal request_body_filter() still runs during upstream forwarding.

Use cases:

Auth signature verification that needs the full body before making an auth decision
Content-based routing (e.g., routing GraphQL queries by operation name)
Body transformation before upstream selection
Streaming decompression during buffering via early_request_body_filter

Size limits are enforced in two layers: Content-Length check before reading (fail fast), and accumulated size check during streaming. Exceeding the limit returns HTTP 413. Default is None (no buffering), so existing code is unaffected.

HTTP/2 body detection: requests without Content-Length (valid in HTTP/2) are handled correctly — only Content-Length: 0 skips the body read.

Includes a body_routing example demonstrating all three patterns — stream, peek, and mutate:

RUST_LOG=INFO cargo run --features openssl --example body_routing

# Peek + mutate — body is inspected for routing then wrapped in an envelope:
curl -X POST 127.0.0.1:6193/post -H "Host: httpbin.org" -H "Content-Type: application/json" -d '{"route": "beta"}'

# Multi-chunk — early_request_body_filter fires per-chunk:
printf 'POST /post HTTP/1.1\r\nHost: httpbin.org\r\nTransfer-Encoding: chunked\r\n\r\na\r\n{"part":1}\r\na\r\n{"part":2}\r\n0\r\n\r\n' | nc 127.0.0.1 6193

# No buffering — GET requests pass through unchanged:
curl 127.0.0.1:6193/get -H "Host: httpbin.org"

Phase docs and mermaid charts updated to include the new phase.

Add opt-in request body buffering via request_body_buffer_limit() trait method. When implemented, the full request body is read and filtered before request_filter runs, making it available for auth signature verification and content-based routing decisions. Resolves cloudflare#780

…buffering # Conflicts: # pingora-proxy/src/lib.rs

CodyPubNub · 2026-03-12T17:00:30Z

Hi @johnhurt 👋 friendly ping on this. This implements the feature requested in #780 (which has the help wanted label). Happy to adjust the approach if the team has a different direction in mind. Would appreciate any initial feedback on whether this is something you'd consider merging.

johnhurt · 2026-03-13T17:31:29Z

Hey, thanks for your patience. I realize this ticket missed some of our triage steps. It's a big change, but it seems worthwhile. We will check it out to make sure the impact on our system wouldn't be too extreme.

One thing that will make this easier to review is to make this fully configurable so that we can opt out of this change.

CodyPubNub · 2026-03-13T17:36:37Z

Hey, thanks for your patience. I realize this ticket missed some of our triage steps. It's a big change, but it seems worthwhile. We will check it out to make sure the impact on our system wouldn't be too extreme.

One thing that will make this easier to review is to make this fully configurable so that we can opt out of this change.

Thanks for taking a look! The feature is fully opt-in, request_body_buffer_limit() returns None by default, and when it does, buffer_request_body_early() returns immediately with no side effects. No existing code paths are altered unless a user explicitly overrides that trait method to return Some(max_size). Happy to add additional gating if needed.

PiotrSikora

Hi @CodyPubNub,
this is completely unsolicited drive-by review (I have no relationship with the project, so my feedback might be different than that from maintainers), but I was recently looking at Pingora and was surprised by the lack of request body buffering before establishing connection to the upstream, so I'm also interested in solving this, albeit for a more generic use case.

PiotrSikora · 2026-03-17T03:59:00Z

pingora-proxy/src/lib.rs

+            match body_chunk {
+                Some(data) => {
+                    let is_body_done = session.downstream_session.is_body_done();
+
+                    // Call request_body_filter for each chunk
+                    let mut filter_data: Option<Bytes> = Some(data);
+                    session
+                        .downstream_modules_ctx
+                        .request_body_filter(&mut filter_data, is_body_done)
+                        .await?;
+                    self.inner
+                        .request_body_filter(session, &mut filter_data, is_body_done, ctx)
+                        .await?;
+
+                    // Accumulate the (possibly filtered) data
+                    if let Some(filtered) = filter_data {
+                        total_size += filtered.len();
+
+                        // Check size limit during accumulation (streaming protection)
+                        if total_size > max_size {
+                            return Error::e_explain(
+                                HTTPStatus(413),
+                                format!(
+                                    "Request body exceeded limit: {} > {} bytes",
+                                    total_size, max_size
+                                ),
+                            );
+                        }
+
+                        body_parts.push(filtered);
+                    }
+
+                    if is_body_done {
+                        break;
+                    }
+                }
+                None => {
+                    // End of body, call filter with end_of_stream=true
+                    let mut filter_data: Option<Bytes> = None;
+                    session
+                        .downstream_modules_ctx
+                        .request_body_filter(&mut filter_data, true)
+                        .await?;
+                    self.inner
+                        .request_body_filter(session, &mut filter_data, true, ctx)
+                        .await?;
+
+                    // Collect any final data from the filter
+                    if let Some(filtered) = filter_data {
+                        total_size += filtered.len();
+
+                        // Final size check
+                        if total_size > max_size {
+                            return Error::e_explain(
+                                HTTPStatus(413),
+                                format!(
+                                    "Request body exceeded limit: {} > {} bytes",
+                                    total_size, max_size
+                                ),
+                            );
+                        }
+
+                        body_parts.push(filtered);
+                    }
+                    break;
+                }


Both of those branches contain virtually the same code.

Could you de-duplicate this and use if let Some(data) = body_chunk { ... } where needed?

PiotrSikora · 2026-03-17T04:03:05Z

pingora-proxy/src/lib.rs

+                    session
+                        .downstream_modules_ctx
+                        .request_body_filter(&mut filter_data, is_body_done)
+                        .await?;
+                    self.inner
+                        .request_body_filter(session, &mut filter_data, is_body_done, ctx)
+                        .await?;


This results in wrong and error-prone ordering of callbacks, i.e. request body callbacks (HttpModule::request_body_filter and ProxyHttp::request_body_filter) are called before request headers callbacks (HttpModule::request_header_filter and ProxyHttp::request_filter).

The buffered request body is available in request_filter using get_buffered_body to perform any business logic based on the request body, so I'm not sure why you need to call those filters here. You should use this step only to pre-read and buffer the request body, and then push it through request_body_filter after request_filter is done.

Alternatively, you could add early_request_body_filter to avoid messing with the existing request flow.

PiotrSikora · 2026-03-17T04:22:23Z

pingora-proxy/src/lib.rs

+        // Get Content-Length if present (for early size check)
+        let content_length = session
+            .downstream_session
+            .req_header()
+            .headers
+            .get(header::CONTENT_LENGTH)
+            .and_then(|v| v.to_str().ok())
+            .and_then(|s| s.parse::<usize>().ok());
+
+        // Fail fast: check Content-Length before reading
+        if let Some(cl) = content_length {
+            if cl > max_size {
+                return Error::e_explain(
+                    HTTPStatus(413),
+                    format!(
+                        "Request body too large: Content-Length {} exceeds limit {} bytes",
+                        cl, max_size
+                    ),
+                );
+            }
+        }
+
+        // Check if there's a body to read (Content-Length > 0 or Transfer-Encoding)
+        let has_body = content_length.is_some_and(|len| len > 0)
+            || session
+                .downstream_session
+                .req_header()
+                .headers
+                .get(header::TRANSFER_ENCODING)
+                .is_some();
+
+        if !has_body {
+            // No body to buffer, mark as done
+            session.mark_body_buffered();
+            return Ok(());
+        }


This doesn't work with HTTP/2 requests that don't contain the Content-Length header.

PiotrSikora · 2026-03-17T04:55:38Z

pingora-proxy/src/proxy_trait.rs

+    /// Determine whether to buffer the entire request body before connecting to upstream.
+    ///
+    /// This is called after [`Self::early_request_filter()`] but before [`Self::request_filter()`]
+    /// and [`Self::upstream_peer()`]. The body is buffered in `Session::buffered_request_body`
+    /// and can be accessed via [`Session::get_buffered_body()`].
+    ///
+    /// # Returns
+    /// - `None`: Don't buffer, stream body to upstream (default)
+    /// - `Some(max_size)`: Buffer body with size limit, return 413 error if exceeded
+    ///
+    /// # Use Cases
+    /// - Auth signature verification (need full body before auth decision)
+    /// - Content-based routing decisions
+    /// - Body transformation before upstream selection
+    ///
+    /// # Size Limit Enforcement
+    /// When returning `Some(max_size)`:
+    /// - Content-Length header is checked first (fail fast before reading)
+    /// - Body size is checked during accumulation (streaming protection)
+    /// - If exceeded, returns HTTP 413 (Payload Too Large)


This approach works well for a few specific use cases, but generic proxies should allow buffering up to the buffer limit, and then resume reading and forward remaining data once the upstream is connected, without rejecting the requests with request body larger than the buffer limit.

This approach works well for a few specific use cases, but generic proxies should allow buffering up to the buffer limit, and then resume reading and forward remaining data once the upstream is connected, without rejecting the requests with request body larger than the buffer limit.

I agree buffer-then-stream is useful for generic proxies. Inspecting the head of a large upload without rejecting it has clear value.

The use case driving this PR is authorization: the auth decision depends on a signature computed over the complete body, so a partial buffer isn't sufficient. The full body has to be available before upstream_peer. Without a hard size limit that becomes an unbounded memory commitment per request, which is why request_body_buffer_limit returns a max size and rejects with 413 if exceeded.

I think buffer-then-stream is a different feature with different semantics (partial visibility, no 413, resume streaming after peer selection). I've scoped this PR to the full-buffer case, but buffer-then-stream would be a solid follow-up.

FWIW, my use case is similar to @CodyPubNub's. We need the full request body in order to make decisions about whether to allow the request to proceed.

(also unaffiliated with the project, just 👀 this PR because I want this feature)

I think buffer-then-stream is a different feature with different semantics (partial visibility, no 413, resume streaming after peer selection). I've scoped this PR to the full-buffer case, but buffer-then-stream would be a solid follow-up.

Right, the feature is different on a high-level, but both require buffering request body before it can be forwarded upstream, and once you have the generic capability, it's easy to support your use case (i.e. block forwarding until max_size bytes or complete request body).

@PiotrSikora

- Fix HTTP/2 body detection: replace has_body heuristic (Content-Length or Transfer-Encoding) with explicit Content-Length == 0 skip. H2 POST without Content-Length was incorrectly treated as bodyless. - Collapse two near-identical match arms (Some/None) into a unified flow using end_of_body flag, removing ~40 lines of duplication. Addresses review comments 1 and 3 from @PiotrSikora.

@PiotrSikora

- New trait method runs per-chunk during buffer_request_body_early(), before request_header_filter — avoids calling request_body_filter out of phase order. - Remove is_body_buffered() skip guards from proxy_h1/h2 — normal request_body_filter runs unguarded during upstream forwarding. - Update body_routing example to demonstrate the streaming callback. - Add early_request_body_filter to phase docs and mermaid charts. Addresses review comment 2 from @PiotrSikora.

CodyPubNub · 2026-03-17T07:05:40Z

Hi @CodyPubNub, this is completely unsolicited drive-by review (I have no relationship with the project, so my feedback might be different than that from maintainers), but I was recently looking at Pingora and was surprised by the lack of request body buffering before establishing connection to the upstream, so I'm also interested in solving this, albeit for a more generic use case.

Thanks for the thorough review, @PiotrSikora. I really appreciate you taking the time.

I've addressed comments 1-3:

Comment 1 (dedup read loop): collapsed the two Some/None match arms into a unified flow with an end_of_body flag.
Comment 2 (callback ordering): added early_request_body_filter() as a dedicated trait method. The early buffering loop calls this instead of request_body_filter, so the existing callback contract is preserved and request_body_filter runs normally during upstream forwarding with no skip guards. Updated phase docs and mermaid charts.
Comment 3 (HTTP/2 body detection): replaced the has_body heuristic with content_length == Some(0). Only skips the read when body is explicitly zero-length.

PiotrSikora

Thanks! This looks much better now.

PiotrSikora · 2026-03-17T16:14:45Z

pingora-proxy/src/proxy_h2.rs

+        let mut downstream_state = if body_was_buffered {
+            DownstreamStateMachine::PreBuffered
+        } else {
+            DownstreamStateMachine::new(session.as_mut().is_body_done())
+        };
+
+        // Use pre-buffered body if available, otherwise check for retry buffer
+        let buffer = if body_was_buffered {
+            pre_buffered_body
+        } else {
+            session.as_mut().get_retry_buffer()
+        };


Nit: You could return (downstream_state, buffer) tuple here (same in H1 proxy).

PiotrSikora · 2026-03-17T16:16:37Z

pingora-proxy/src/proxy_trait.rs

+    /// - Content-Length header is checked first (fail fast before reading)
+    /// - Body size is checked during accumulation (streaming protection)
+    /// - If exceeded, returns HTTP 413 (Payload Too Large)
+    fn request_body_buffer_limit(&self, _session: &Session, _ctx: &Self::CTX) -> Option<usize> {


Nit: early_request_body_buffer_limit

PiotrSikora · 2026-03-17T16:37:06Z

pingora-proxy/src/proxy_trait.rs

+    /// Determine whether to buffer the entire request body before connecting to upstream.
+    ///
+    /// This is called after [`Self::early_request_filter()`] but before [`Self::request_filter()`]
+    /// and [`Self::upstream_peer()`]. The body is buffered in `Session::buffered_request_body`
+    /// and can be accessed via [`Session::get_buffered_body()`].
+    ///
+    /// # Returns
+    /// - `None`: Don't buffer, stream body to upstream (default)
+    /// - `Some(max_size)`: Buffer body with size limit, return 413 error if exceeded
+    ///
+    /// # Use Cases
+    /// - Auth signature verification (need full body before auth decision)
+    /// - Content-based routing decisions
+    /// - Body transformation before upstream selection
+    ///
+    /// # Size Limit Enforcement
+    /// When returning `Some(max_size)`:
+    /// - Content-Length header is checked first (fail fast before reading)
+    /// - Body size is checked during accumulation (streaming protection)
+    /// - If exceeded, returns HTTP 413 (Payload Too Large)


I think buffer-then-stream is a different feature with different semantics (partial visibility, no 413, resume streaming after peer selection). I've scoped this PR to the full-buffer case, but buffer-then-stream would be a solid follow-up.

Right, the feature is different on a high-level, but both require buffering request body before it can be forwarded upstream, and once you have the generic capability, it's easy to support your use case (i.e. block forwarding until max_size bytes or complete request body).

@PiotrSikora

- Rename request_body_buffer_limit → early_request_body_buffer_limit for consistency with early_request_body_filter (comment 6) - Collapse downstream_state + buffer into tuple return in proxy_h1 and proxy_h2 (comment 5) - Align inline comments with existing Cloudflare style - Update body_routing example and phase docs Addresses review comments 5 and 6 from @PiotrSikora.

CodyPubNub added 4 commits February 16, 2026 11:21

Add body_routing example for early request body buffering

b50ad3f

Mark body read errors as downstream in buffer_request_body_early

42e909d

Simplify early body buffering read errors to use into_down()

56f9065

davidthing mentioned this pull request Feb 23, 2026

Support Early Request Body Access for Dynamic Upstream Peer Selection #780

Open

Merge remote-tracking branch 'upstream/main' into early-request-body-…

82642a1

…buffering # Conflicts: # pingora-proxy/src/lib.rs

johnhurt added the enhancement New feature or request label Mar 13, 2026

PiotrSikora reviewed Mar 17, 2026

View reviewed changes

CodyPubNub added 2 commits March 16, 2026 23:15

PiotrSikora reviewed Mar 17, 2026

View reviewed changes

Conversation

CodyPubNub commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CodyPubNub commented Mar 12, 2026

Uh oh!

johnhurt commented Mar 13, 2026

Uh oh!

CodyPubNub commented Mar 13, 2026

Uh oh!

PiotrSikora left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CodyPubNub commented Mar 17, 2026

Uh oh!

PiotrSikora left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CodyPubNub commented Feb 16, 2026 •

edited

Loading