Unsupported-encoding passthrough forwards encoded body with content-encoding stripped (found during #661 triage)

[ranxianglei]

Found while triaging #661 (not the cause of that 405 — filing per policy so it isn't lost).

Problem

In the #619 "unsupported encoding" path, src/server.ts forwards the still-encoded body bytes, but the content-encoding header is stripped as hop-by-hop (UPSTREAM_HOP_HEADERS, src/server.ts:208-227). Upstream receives gzip/br/deflate bytes declared as none → 400-class failures.

Also the comment near src/server.ts:1003 ("content-encoding header stays intact") contradicts the actual behavior — headers in UPSTREAM_HOP_HEADERS are stripped on forward.

Expected

Either:

• forward the body decoded (decode → strip header — consistent), or
• forward the raw bytes with content-encoding preserved on this path.

Repro sketch

Upstream advertises Accept-Encoding: br; client sends content-encoding: gzip body through a path that bili does not decode (unsupported encoding per #619); observe upstream rejecting undeclared binary body.

Priority

Low-medium: only bites clients that send request bodies with encodings bili doesn't decode. Discovered by ework during #661 PR #664 triage; independently confirmed in code reading.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Confirmed in code — triage results before fixing:

Verification. buildForwardTarget() (src/server.ts:2786) copies inbound request headers while skipping every name in UPSTREAM_HOP_HEADERS, which includes content-encoding (src/server.ts:217). The #619 catch block (src/server.ts:998-1007) drops protocol so the still-encoded bytes go to the verbatim passthrough — and the comment at src/server.ts:1003 ("the content-encoding header stays intact") is factually wrong: the header is stripped at forward time. Upstream receives undeclared binary bytes → 400-class failures. Exactly as filed.

Layering. Root cause is one step up from the symptom: content-encoding is an end-to-end header (RFC 9110 §7.2), not hop-by-hop, but it was lumped into a single strip set shared by BOTH directions. Stripping it on the response side is correct (Node's fetch transparently decodes upstream bodies, so the marker must not reach the client); stripping it on the request side is only correct when bili itself decoded/rewrote the body — and in that case the header is already explicitly deleted at decode time (src/server.ts:997). So every request where the header survives to forward time has a byte-identical passthrough body and MUST keep it.

Scope note. The bug isn't limited to the #619 path: any verbatim passthrough with an encoded body loses the header — including unknown-path requests (e.g. POST /v1/embeddings with a gzip body), which skip decoding entirely (protocol === null from the start). Fixing the shared set fixes all of them.

Fix chosen. Of the two options in the issue, "forward decoded" is impossible on this path by definition (the encoding is precisely what bili can't decode). Implementing option 2 at the root: split the strip set by direction — request side no longer strips content-encoding (decoded/rebuilt bodies already had it removed at decode time, so processed traffic is unchanged), response side keeps stripping it. No per-path flag plumbing needed. Working now; PR link follows.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Fixed — PR #678: #678

As discussed in my triage comment, I went with option 2 (forward raw bytes with content-encoding preserved), implemented as a direction split of the strip set in src/server.ts:

• content-encoding removed from UPSTREAM_HOP_HEADERS — the request-forward path (buildForwardTarget) now passes it through, so both the Follow-up from #617: robust session identity + no-spurious-400 for header-less clients (e.g. ZCode) #619 undecodable-encoding passthrough and unknown-path requests (where no decode is ever attempted) reach upstream with their encoding declared.
• New RESPONSE_ONLY_STRIP_HEADERS = new Set(["content-encoding"]) applied at the two response-forward sites in forward() — client-facing behavior is byte-for-byte unchanged there (Node fetch already decoded the upstream body, so the marker must not leak downstream).

The split is safe because every processed request already drops the header at decode time (if (decoded.decoded) delete req.headers["content-encoding"]) before forwarding — only byte-identical passthrough bodies carry it to the forward, where the marker describes exactly the bytes being sent. This also makes the old #619 catch-block comment ("the content-encoding header stays intact") true again instead of contradictory.

Tests (tests/decode-fail-passthrough.test.ts): decode-failure passthrough now asserts the header reaches upstream; new unknown-path test with a valid gzip body; new response-side test pinning that upstream response encodings are still stripped (client gets plain JSON, no content-encoding). The codex-official valid-gzip decoded-path assertion (header absent upstream) stays green as a canary for the other direction.

Pre-flight: typecheck ✅ · 1272/1272 tests ✅ · build ✅