fix: don't emit the chunked terminator for zero-length body writes

[songhieu]

Fixes #935.

What

In ChunkedEncoding mode, BodyWriter encoded a zero-length application write as 0\r\n\r\n — which on the wire is the chunked terminator. finish() then wrote the terminator again, so the peer received the end-of-body sequence twice and parsed the second one as the start of a new message, desyncing the (keep-alive) connection.

This PR makes zero-length writes a no-op in chunked mode, in both write paths:

• do_write_chunked_body (async path): return Ok(Some(0)) without touching the stream.
• poll_write_chunked_body_task (cancel-safe task path): complete the task as Done(0) without writing.

finish() remains the only place that emits the terminator.

Why it matters in practice

proxy_h1.rs forwards the final downstream body chunk to the upstream even when it is empty (the mid-stream guard is deliberately !upstream_end_of_body && ...). An HTTP/2 downstream that ends the request body with an empty DATA frame carrying END_STREAM — Cloudflare's HTTP/2 to Origin does this for streamed POST bodies, as does curl -T - — therefore produces Body(Some(empty), end=true), and the H1 upstream receives:

Transfer-Encoding: chunked

11
{"hello":"world"}
0

0

Strict upstream parsers (e.g. uvicorn/h11) reject the duplicate terminator as a malformed pipelined request (400 Invalid HTTP request received.) and the poisoned pooled connection then serves that stale error to unrelated proxied requests. Full write-up with reproduction and captured wire bytes in #935.

Tests

• write_body_chunked_ignores_empty_chunk — async path: data chunk, empty write (asserts no wire bytes via the mock's exact-write expectations), single terminator on finish().
• write_body_task_chunked_ignores_empty_chunk — task path: same assertions through send_body_task / write_current_body_task.

cargo test -p pingora-core --lib protocols::http::v1::body passes.

[seonghobae]

Downstream consumer verification from ContextualWisdomLab/pingora-gateway#51: this repair remains a concrete release prerequisite before CWL can admit downstream HTTP/2 while keeping the characterized upstream path on HTTP/1.1.

Fresh public state: protected cloudflare/pingora@main is 09696b51bc59315353d96686355861604d0bb48c; current PR head is e40ed4cceb0c0ed8c05cc39eb01a8c73dea5497a. Exact comparison is 1 commit ahead / 0 behind with merge base exactly current protected main, and the effective diff is only pingora-core/src/protocols/http/v1/body.rs. The retained repair makes zero-length writes a no-op in both chunked write paths while leaving finish() as the terminator owner.

The restacked exact head is now also hosted GREEN: build run 33818852141 completed success for nightly, Rust 1.97.1, and Rust 1.85.0 jobs (fmt/check/test/doc-test/clippy/audit/machete as applicable), and Semgrep run 33818852056 completed success. The contributor additionally reported cargo test -p pingora-core --lib at 544 passed on the rebased tree. There is still no submitted PR review or maintainer integration, so this remains mutable supplier evidence rather than downstream dependency/release authority.

For CWL acceptance we will exercise the actual H2-downstream → H1-upstream wire path, including an empty DATA+END_STREAM request-body tail, verify exactly one chunk terminator, then reuse the same H1 keep-alive connection for an independent follow-up request. We will not work around the RED by switching upstream ALPN to H2 or pinning this mutable contributor head.

If the intended fix remains BodyWriter-level zero-length-write suppression, retaining both async and cancel-safe task-path regressions through maintainer review/integration gives downstream consumers the invariant needed for the later immutable-supplier bump and unchanged wire GREEN.

[songhieu]

Rebased onto current main (09696b5, commit is now e40ed4c). The change itself is unchanged: zero-length writes are a no-op in both chunked write paths (do_write_chunked_body and the cancel-safe poll_write_chunked_body_task), and finish() remains the only place that emits the terminator. Both regression tests (write_body_chunked_ignores_empty_chunk, write_body_task_chunked_ignores_empty_chunk) are retained.

I also confirmed the bug is still present on current main (neither chunked write path guards against a zero-length chunk), so the fix is still needed. cargo test -p pingora-core --lib passes locally on the rebased tree (544 passed).

@seonghobae thanks for the note; the branch is now directly consumable on top of main.

@drcaramelsyrup this is the PR for #935, ready for review whenever you have a moment.