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 behind the early_body_buffer Cargo feature. 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 HTTP/1.x and HTTP/2 upstreams during the proxy phase. proxy_custom support is intentionally left for a Cloudflare-side follow-up.

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. With early_body_buffer disabled, no API or runtime path is added; when enabled, default None preserves existing streaming behavior.

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

Retries: the buffered body is retained and replayed across upstream retry attempts. Regression tests cover fixed-length and chunked bodies, oversize rejection, and single-attempt forwarding.

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

RUST_LOG=INFO cargo run --features openssl,early_body_buffer --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

+            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

+                    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

+        // 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

+    /// 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

+        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

+    /// - 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

+    /// 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.

QiiHeng3 · 2026-03-31T14:59:34Z

@CodyPubNub This is a great feature I've been eagerly waiting for！！！I'm building a Rust-based Kubernetes ingress/gateway on top of Pingora, and the lack of early request body access has been a major pain point for us.

CodyPubNub · 2026-05-04T16:19:02Z

Hi @johnhurt, I was wondering if there was any updates or feedback from your team to share about this change. Thank you!

johnhurt · 2026-05-04T16:29:45Z

Hey, yeah. Sorry for the delay. We have been discussing this internally, so we would like to pull this in. I should have come back and explained what I meant by "configurable". These kinds of features even if they are off by default still incur a cost in runtime (even if it's a minimal branch) and risk. That's why we ask contributors to make changes that touch the main proxy trait configurable by cargo feature to avoid the. Checkout the connection filter feature for an example.

CodyPubNub · 2026-05-04T17:18:18Z

Hey, yeah. Sorry for the delay. We have been discussing this internally, so we would like to pull this in. I should have come back and explained what I meant by "configurable". These kinds of features even if they are off by default still incur a cost in runtime (even if it's a minimal branch) and risk. That's why we ask contributors to make changes that touch the main proxy trait configurable by cargo feature to avoid the. Checkout the connection filter feature for an example.

I appreciate the feedback. I've added an early_body_buffer feature to gate the behavior introduced in this PR. Please let me know if there's anything else. Thank you!

CodyPubNub · 2026-06-17T16:58:23Z

Hi @johnhurt I was wondering if there were any updates to share on the team's thoughts toward this PR. It would be very helpful for this, or something very much like this to be available in the main branch. Thank you 🙏

jeremyhamning · 2026-07-20T12:51:06Z

Hello @johnhurt, I'm also wondering if this could be reviewed and merged soon?

Resolve conflict in pingora-proxy/src/lib.rs: - downstream_custom_message() now returns the DownstreamCustomMessageReader type alias introduced by main's custom-message retry fix (7c04f54), rather than the inlined boxed-Stream signature the branch forked from. - Add upstream_h1_upgrade_status_mismatch to the test-only new_h1_with_http_session() constructor. Main added this Session field, and because the constructor is gated behind early_body_buffer it compiled fine on default features while breaking the feature build — a silent conflict git could not flag.

- Early buffering drains downstream before retry buffering starts, so taking the session buffer leaves retries with no body to replay. - Clone the bounded buffer per attempt, share lazy selection across H1/H2, and remove the destructive take_buffered_body API. - Add self-contained regression coverage for fixed-length and chunked bodies.

CodyPubNub · 2026-07-27T18:32:50Z

Hi @drcaramelsyrup I was wondering if you or others on the team have had a chance to think about this PR. Thank you 🙏

drcaramelsyrup · 2026-07-28T01:54:30Z

Hey! As you might be able to tell we haven’t had all the bandwidth to do so recently. Thanks for the ping, I will take a look this week.

…

On Jul 27, 2026 at 11:33 AM, <Cody Carlsen ***@***.***)> wrote: CodyPubNub left a comment (cloudflare/pingora#816) (#816 (comment)) Hi @drcaramelsyrup (https://github.com/drcaramelsyrup) I was wondering if you or others on the team have had a chance to think about this PR. Thank you 🙏 — Reply to this email directly, view it on GitHub (#816?email_source=notifications&email_token=AAGLWY6WM4TZJZDAAVPKB6L5G6N6PA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBZGUZDOMRUGIY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5095272421), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAGLWYZKXNRMV4SBUL3G6J35G6N6PAVCNFSNUABFKJSXA33TNF2G64TZHM3DGNRYGUZTSOBYHNEXG43VMU5TGOJUHEZDOOBYHAZ2C5QC). Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS (https://github.com/notifications/mobile/ios/AAGLWY77YU6TJGBWTLCTH7L5G6N6PA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBZGUZDOMRUGIY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y) and Android (https://github.com/notifications/mobile/android/AAGLWYYZAUEVEESG4QPCD635G6N6PA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBZGUZDOMRUGIY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI). Download it today! You are receiving this because you were mentioned.Message ID: ***@***.***>

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

drcaramelsyrup

Directionally good with changes requested, I'm ok with deferred the "stream-if-exceeds-buffer" behavior for a followup.

I noticed we'll need to change proxy_custom as well but it's a patch we can also apply internally since I suspect you don't have a great way of testing that.

drcaramelsyrup · 2026-08-22T22:05:53Z

+    /// Creates a Session from an H1 HttpSession (for testing only).
+    #[cfg(all(test, feature = "early_body_buffer"))]
+    pub fn new_h1_with_http_session(
+        http_session: pingora_core::protocols::http::v1::server::HttpSession,
+    ) -> Self {
+        use pingora_cache::HttpCache;
+        use pingora_core::protocols::http::compression::ResponseCompressionCtx;
+        use pingora_core::protocols::http::ServerSession;
+
+        let shutdown_flag = Arc::new(AtomicBool::new(false));
+        Session {
+            downstream_session: Box::new(ServerSession::H1(http_session)),
+            cache: HttpCache::new(),
+            upstream_compression: ResponseCompressionCtx::new(0, false, false),
+            ignore_downstream_range: false,
+            upstream_headers_mutated_for_cache: false,
+            h1_upgrade_request_status: H1UpgradeRequestStatus::default(),
+            subrequest_ctx: None,
+            subrequest_spawner: None,
+            downstream_modules_ctx: HttpModuleCtx::empty(),
+            #[cfg(feature = "upstream_modules")]
+            upstream_modules_ctx: HttpModuleCtx::empty(),
+            upstream_body_bytes_received: 0,
+            downstream_task_seen_upgraded: false,
+            upstream_write_pending_time: Duration::ZERO,
+            shutdown_flag,
+            buffered_request_body: None,
+            body_buffered: false,
+        }
+    }


Suggested change

/// Creates a Session from an H1 HttpSession (for testing only).

#[cfg(all(test, feature = "early_body_buffer"))]

pub fn new_h1_with_http_session(

http_session: pingora_core::protocols::http::v1::server::HttpSession,

) -> Self {

use pingora_cache::HttpCache;

use pingora_core::protocols::http::compression::ResponseCompressionCtx;

use pingora_core::protocols::http::ServerSession;

let shutdown_flag = Arc::new(AtomicBool::new(false));

Session {

downstream_session: Box::new(ServerSession::H1(http_session)),

cache: HttpCache::new(),

upstream_compression: ResponseCompressionCtx::new(0, false, false),

ignore_downstream_range: false,

upstream_headers_mutated_for_cache: false,

h1_upgrade_request_status: H1UpgradeRequestStatus::default(),

subrequest_ctx: None,

subrequest_spawner: None,

downstream_modules_ctx: HttpModuleCtx::empty(),

#[cfg(feature = "upstream_modules")]

upstream_modules_ctx: HttpModuleCtx::empty(),

upstream_body_bytes_received: 0,

downstream_task_seen_upgraded: false,

upstream_write_pending_time: Duration::ZERO,

shutdown_flag,

buffered_request_body: None,

body_buffered: false,

}

}

/// Creates a Session from an H1 HttpSession (for testing only).

#[cfg(all(test, feature = "early_body_buffer"))]

pub fn new_h1_with_http_session(

http_session: pingora_core::protocols::http::v1::server::HttpSession,

) -> Self {

use pingora_core::protocols::http::ServerSession;

Self::new(

Box::new(ServerSession::H1(http_session)),

&HttpModules::new(),

#[cfg(feature = "upstream_modules")]

&HttpModules::new(),

Arc::new(AtomicBool::new(false)),

)

}

drcaramelsyrup · 2026-08-22T22:06:20Z

+
+    /// Handle each chunk of request body during early buffering.
+    ///
+    /// This is called during [`buffer_request_body_early()`] for each body chunk, **before**


Suggested change

/// This is called during [`buffer_request_body_early()`] for each body chunk, **before**

/// This is called while the body is buffered early (enabled by

/// [`Self::early_request_body_buffer_limit()`]) for each body chunk, **before**

buffer_request_body_early() is a private method on HttpProxy, so we get a doc warning.

drcaramelsyrup · 2026-08-22T22:07:17Z

    ReadingFinished,
+    /// body was pre-buffered before upstream connection, skip all downstream polling
+    #[cfg(feature = "early_body_buffer")]
+    PreBuffered,


IMO the variant is unnecessary and actually undesirable when we already have ReadingFinished. It results in an odd behavior diff for abort on close: i.e., we do want to continue polling after the body is finished to be able to detect EOF or error.

drcaramelsyrup · 2026-08-22T22:08:50Z

+        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());


Suggested change

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());

let content_length = header_value_content_length(

session

.downstream_session

.req_header()

.headers

.get(header::CONTENT_LENGTH),

);

drcaramelsyrup · 2026-08-22T22:17:25Z

+        // read body chunks until end of stream
+        loop {
+            let body_chunk: Option<Bytes> =
+                match session.downstream_session.read_body_or_idle(false).await {


Suggested change

match session.downstream_session.read_body_or_idle(false).await {

match session.downstream_session.read_request_body().await {

There's no real point in choosing read_body_or_idle here since this isn't part of a select!. Also, you'd just hang if the body is done.

drcaramelsyrup · 2026-08-22T22:28:11Z

+        if total_size > 0 {
+            let mut combined = bytes::BytesMut::with_capacity(total_size);
+            for part in body_parts {
+                combined.extend_from_slice(&part);
+            }
+            session.set_buffered_body(Some(combined.freeze()));
+        } else {
+            session.mark_body_buffered();
+        }


Suggested change

if total_size > 0 {

let mut combined = bytes::BytesMut::with_capacity(total_size);

for part in body_parts {

combined.extend_from_slice(&part);

}

session.set_buffered_body(Some(combined.freeze()));

} else {

session.mark_body_buffered();

}

if total_size == 0 {

session.mark_body_buffered();

} else if body_parts.len() == 1 {

// common case: a single chunk can be moved out without a second copy

session.set_buffered_body(body_parts.pop());

} else {

let mut combined = bytes::BytesMut::with_capacity(total_size);

for part in body_parts {

combined.extend_from_slice(&part);

}

session.set_buffered_body(Some(combined.freeze()));

}

optimization when you have a single chunk as the whole body

…buffering

- reuse Session initialization to prevent test-only drift - preserve downstream close detection after buffering - use shared Content-Length parsing and avoid empty-body hangs - avoid copying single-chunk bodies and fix public API docs

CodyPubNub · 2026-08-26T17:22:19Z

Directionally good with changes requested, I'm ok with deferred the "stream-if-exceeds-buffer" behavior for a followup.

I noticed we'll need to change proxy_custom as well but it's a patch we can also apply internally since I suspect you don't have a great way of testing that.

Thank you very much for taking the time to review this and provide feedback. I've addressed all of the inline comments and would be happy to take you up on your offer to implement and test proxy_custom support internally. I've left the "stream-if-exceeds-buffer" behavior for a follow-up.

stevehu · 2026-08-27T18:44:23Z

We are implementing raw-body HMAC webhook authentication in networknt/light-fabric and independently hit the lifecycle gap addressed by this PR.

Our ordering requirement is slightly different from the automatic early-buffering use case: handler-chain rate limiting and JWT/API-key factors must be able to reject a request before its body is buffered, while the application also enforces a profile-specific read timeout and an aggregate memory budget. The integration we need is therefore:

leave early_request_body_buffer_limit() at its default None;
run header-phase policy in request_filter();
consume and verify the bounded body there;
call Session::set_buffered_body(Some(verified_body)); and
rely on the normal request_body_filter(), H1/H2 upstream forwarding, and retry paths to use that application-supplied body.

The current implementation appears to support this cleanly: both H1 and H2 select the same retained buffered-body source, and the normal request-body filter is applied while forwarding it. If this is intended as a supported contract, it would let us remove our small Pingora fork patch without moving security checks ahead of the handler chain or duplicating Pingora's forwarding machinery.

I added an end-to-end regression for the application-managed path in CodyPubNub/pingora#1. It disables automatic buffering, reads in request_filter(), calls set_buffered_body(), forces an upstream retry, verifies exact bytes at the origin, and asserts that request_body_filter() runs for every forwarding attempt. The complete early-body/retry test file passes 5/5 locally.

Could the maintainers confirm that application-supplied buffering through set_buffered_body() is intended to remain supported alongside automatic early buffering?

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

CodyPubNub added 2 commits May 4, 2026 09:58

Merge branch 'main' into early-request-body-buffering

ba51f61

Gate early_body_buffer behind cargo feature

b2524b9

CodyPubNub mentioned this pull request Jun 8, 2026

Enhanced LLM reverse proxy based on Model Name routing #907

Closed

Merge branch 'main' into early-request-body-buffering

ae6d2d9

Merge branch 'main' into early-request-body-buffering

09c4360

CodyPubNub added 2 commits July 27, 2026 10:04

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

99f6d17

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

drcaramelsyrup requested changes Aug 26, 2026

View reviewed changes

CodyPubNub added 2 commits August 26, 2026 09:58

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

5916496

…buffering

Address early body buffering review feedback

1dd0166

- reuse Session initialization to prevent test-only drift - preserve downstream close detection after buffering - use shared Content-Length parsing and avoid empty-body hangs - avoid copying single-chunk bodies and fix public API docs

CodyPubNub requested a review from drcaramelsyrup August 26, 2026 17:22

stevehu mentioned this pull request Aug 27, 2026

test(proxy): cover application-managed body buffering CodyPubNub/pingora#1

Open

stevehu mentioned this pull request Aug 27, 2026

Upgrade Pingora after #816 release and remove local proxy patch networknt/light-fabric#355

Open

11 tasks

	/// This is called during [`buffer_request_body_early()`] for each body chunk, before
	/// This is called while the body is buffered early (enabled by
	/// [`Self::early_request_body_buffer_limit()`]) for each body chunk, before

	match session.downstream_session.read_body_or_idle(false).await {
	match session.downstream_session.read_request_body().await {

Uh oh!

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!

QiiHeng3 commented Mar 31, 2026

Uh oh!

CodyPubNub commented May 4, 2026

Uh oh!

johnhurt commented May 4, 2026

Uh oh!

CodyPubNub commented May 4, 2026

Uh oh!

CodyPubNub commented Jun 17, 2026

Uh oh!

jeremyhamning commented Jul 20, 2026

Uh oh!

CodyPubNub commented Jul 27, 2026

Uh oh!

drcaramelsyrup commented Jul 28, 2026 via email

Uh oh!

drcaramelsyrup 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!

CodyPubNub commented Aug 26, 2026

Uh oh!

stevehu commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

CodyPubNub commented Feb 16, 2026 •

edited

Loading