cloudflare / cloudflare/pingora

H1 upstream: chunked terminator 0\r\n\r\n written twice when an H2 downstream ends the request body with an empty DATA frame (END_STREAM), poisoning upstream keep-alive connections

Open
#935 2 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
27.4k
Forks
1.7k
Avg merge
6h 22m
Merged PRs (30d)
3

Description

# Summary

When proxying **HTTP/2 downstream → HTTP/1.1 upstream**, if the client finishes the request body with an **empty DATA frame carrying END_STREAM** (very common: Cloudflare's *HTTP/2 to Origin* does this, so does `curl -T -` and most streaming clients that don't know body length up front), pingora writes the chunked terminating sequence `0\r\n\r\n` to the upstream **twice**.

Strict HTTP/1.1 parsers on the upstream (e.g. uvicorn/h11, which rejects with `400 Invalid HTTP request received.`) parse the duplicate terminator as the start of a bogus pipelined request. The upstream connection is then desynchronized: the stray error response is delivered to whichever request reuses the pooled keep-alive connection next, so **unrelated requests intermittently receive 400s**, and the connection pool churns (`Upstream body is already finished. Nothing to write` / `Data received on idle client connection, close it` warnings flood the logs).

Verified on `pingora-proxy 0.6.0`; by code inspection the same gap is still present on current `main`.

# Mechanism

1. `proxy_h1.rs` — the downstream read path deliberately forwards a final empty chunk when `end_of_body` is true (the guard only skips empty chunks *mid-stream*):

```rust
/* It is normal to get 0 bytes because of multi-chunk ...
* Don't write 0 bytes to the network since it will be
* treated as the terminating chunk */
if !upstream_end_of_body && data.as_ref().is_some_and(|d| d.is_empty()) {
return Ok(false);
}
```

2. `proxy_h1.rs::send_body_to1` — the empty body slice is passed to `write_body`, then `finish_body` runs because `end == true`:

```rust
HttpTask::Body(data, end) => {
body_done = end;
if let Some(d) = data {
let m = client_session.write_body(&d).await; // <-- d is empty
...
}
}
...
if body_done {
match client_session.finish_body().await { // <-- terminator again
```

3. `pingora-core/src/protocols/http/v1/body.rs::do_write_chunked_body` has no empty-buffer guard, so a 0-byte write **is** the terminator:

```rust
let chunk_size = buf.len(); // 0
let chuck_size_buf = format!("{:X}\r\n", chunk_size); // "0\r\n"
let mut output_buf = Bytes::from(chuck_size_buf).chain(buf).chain(&b"\r\n"[..]);
// wire: 0\r\n\r\n == chunked terminator
self.body_mode = BM::ChunkedEncoding(written + chunk_size); // stays ChunkedEncoding
```

Because `body_mode` stays `ChunkedEncoding`, the subsequent `finish_body()` → `do_finish_chunked_body()` writes `LAST_CHUNK` (`0\r\n\r\n`) a second time.

# Reproduction

Any `ProxyHttp` service with an h2-enabled TLS listener and a plain-HTTP/1.1 upstream. Point the upstream at a byte-capture sink:

```bash
nc -l 9999 > captured.txt # upstream: capture raw bytes

# client: HTTP/2, body streamed from stdin => no content-length,
# curl ends the stream with an empty DATA frame + END_STREAM
printf '%s' '{"hello":"world"}' | \
curl -sk --http2 -X POST -T - https://localhost:8443/test \
-H 'Content-Type: application/json'
```

Captured upstream bytes (pingora 0.6.0):

```
POST /test HTTP/1.1
user-agent: curl/8.7.1
accept: */*
Content-Type: application/json
Transfer-Encoding: chunked
Host: localhost:8443

11
{"hello":"world"}
0

0

```

Note the **two** `0\r\n\r\n` sequences. The same request sent as HTTP/1.1 chunked (`-H 'Transfer-Encoding: chunked'`, no `-T`) produces a single terminator — the difference is the trailing empty DATA frame that only the h2 path delivers as `Body(Some(empty), end=true)`.

Real-world trigger: Cloudflare *HTTP/2 to Origin* streams POST bodies this way, so every such request through a pingora-based origin proxy corrupts the upstream connection.

# Suggested fix

Either (or both):

- `send_body_to1`: skip the write for empty data — `finish_body()` alone terminates correctly:

```rust
if let Some(d) = data {
if !d.is_empty() {
let m = client_session.write_body(&d).await;
...
}
}
```

- `BodyWriter::do_write_chunked_body`: return early on an empty buffer so a 0-byte application write can never emit the protocol terminator.

# Workaround for users

Drop the final empty chunk in `ProxyHttp::request_body_filter`:

```rust
async fn request_body_filter(
&self,
_session: &mut Session,
body: &mut Option,
end_of_stream: bool,
_ctx: &mut Self::CTX,
) -> Result<()> {
// Only when end_of_stream: an empty mid-stream chunk turned into None
// would falsely signal end-of-body in proxy_h1.
if end_of_stream && body.as_ref().is_some_and(|b| b.is_empty()) {
*body = None;
}
Ok(())
}
```

# Environment

- pingora / pingora-proxy / pingora-core 0.6.0 (boringssl feature), Linux x86_64
- Downstream: HTTP/2 over TLS (ALPN h2), e.g. Cloudflare HTTP/2-to-Origin or `curl --http2`
- Upstream: HTTP/1.1 cleartext (gunicorn + uvicorn worker; reproduced with raw `nc` byte capture)

Contributor guide

Open the contributing guide

Research direction

Start with the HTTP/2-to-HTTP/1.1 path in proxy_h1.rs and then inspect pingora-core/src/protocols/http/v1/body.rs::do_write_chunked_body. Run the provided curl and nc reproduction to capture the upstream bytes. Done means the final empty DATA frame produces only one 0\r\n\r\n terminator while normal streaming remains correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.