Skip to content

fix: keep content-encoding when forwarding undecoded passthrough bodies - #678

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-10_content-encoding-passthrough
Sep 11, 2026
Merged

fix: keep content-encoding when forwarding undecoded passthrough bodies#678
ranxianglei merged 1 commit into
masterfrom
2026-09-10_content-encoding-passthrough

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem (#677)

content-encoding is an end-to-end header (RFC 9110 §7.2), not hop-by-hop, but it lived in UPSTREAM_HOP_HEADERS — a single strip set shared by both forwarding directions. That is correct for responses (Node's fetch transparently decodes upstream bodies, so the marker must not reach the client) but wrong for requests on any path where bili forwards bytes it did not decode:

The comment near the #619 catch block ("the content-encoding header stays intact") also contradicted actual behavior; after this change it is true again.

Fix

Split the strip set by direction:

  • content-encoding removed from UPSTREAM_HOP_HEADERS (request side now forwards it);
  • new RESPONSE_ONLY_STRIP_HEADERS = new Set(["content-encoding"]) applied at the two response-forward sites (forward(), normal + debug log copy), preserving today's client-facing behavior exactly.

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, and there the marker describes exactly the bytes being sent.

Tests (tests/decode-fail-passthrough.test.ts)

  • decode-failure passthrough test now asserts content-encoding: gzip reaches the upstream;
  • new: unknown-path passthrough keeps content-encoding on a valid gzip body;
  • new: response-side strip still works (upstream gzip response → client gets plain JSON, no content-encoding header) — pins the direction asymmetry;
  • canary: tests/codex-official.test.ts valid-gzip decoded path still asserts the header is absent upstream (unchanged behavior).

Pre-flight

  • npm run typecheck
  • npm test — 1272/1272 ✅
  • npm run build

Fixes #677

@github-actions

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-09-10_content-encoding-passthrough (76f02f5)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-678

Each push to this PR publishes a new version under the pr-678 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr678.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Reviewing PR #678 now — single commit touching src/server.ts + tests/decode-fail-passthrough.test.ts (+129/-6), mergeable against master. I'll verify the direction-split logic in both forwarding paths, run typecheck/test/build locally, and check both compression modes are unaffected. Full review conclusion to follow.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review of PR #678 — verified independently against the code and re-ran the full pre-flight locally on branch 2026-09-10_content-encoding-passthrough (76f02f5).

Diff cleanliness (first gate). Exactly 2 files, +129/−6 (src/server.ts, tests/decode-fail-passthrough.test.ts); every line serves the stated purpose. ✅

Request side — verified safe.

  • buildForwardTarget (src/server.ts:2840-2846) now forwards content-encoding; processed requests can't leak it because it's deleted at decode time (src/server.ts:1006, if (decoded.decoded) delete req.headers["content-encoding"]) before any forward.
  • Passthrough bodies are byte-identical end-to-end: forward() receives the raw bodyBuffer on the unprocessed path (call site src/server.ts:1456), wireBody starts from it (src/server.ts:3207) and the billion-context: 400 Bad Request when codex sends role: "developer" messages — request a request-body compat transform hook #552 role rewrite only fires for string bodies — so the forwarded marker describes exactly the bytes sent.
  • content-length remains in the hop set, so undici recomputes it from the actual buffer — no length mismatch on encoded passthrough.
  • Edge cases checked in src/content-encoding.ts: multi-value encodings where any coding is unsupported fall back to verbatim relay keeping the original header (upstream decodes in reverse per RFC 7231 — correct); an explicit identity value is now forwarded instead of stripped — RFC-harmless, only affects that pathological case.

Response side — strip sites are exhaustive. There are exactly two client-facing upstream-header-forward sites, both in forward() and both patched (src/server.ts:3459 normal, :3466 debug-log copy). The #552 role-retry response rebuild (src/server.ts:3393) flows through the same strip loop, so it's covered. MITM TLS-terminated traffic routes into the same handle()/forward() (mitm emits connectionsrc/server.ts:370); blind CONNECT tunnels are raw byte pipes with no header handling. Client-facing behavior is byte-for-byte unchanged.

Both compression modes considered. The change sits at the HTTP wire boundary, orthogonal to plugin/proxy mode: processed requests (both modes) drop the header at decode; passthrough is protocol === null in both modes. No mode-specific interaction found.

Other. The old #619 catch-block comment ("the content-encoding header stays intact", src/server.ts:1009-1013) is now factually true instead of contradictory. Test coverage pins all three directions: encoded passthrough keeps the header, unknown-path passthrough keeps it, response-side strip still works, and the codex-official valid-gzip decoded path stays green as the opposite-direction canary.

Local re-run (not just trusting the PR's claim): npm run typecheck ✅ · npm test 1272/1272 ✅ · npm run build ✅ · targeted: decode-fail-passthrough.test.ts 4/4 ✅, codex-official.test.ts 2/2 ✅.

No issues found that needed fixing on the branch. Root cause correctly identified (end-to-end header lumped into a direction-shared strip set), fixed at the root rather than per-path, minimal diff. Ready to merge.

中文摘要:修复了 content-encoding(端到端头)被误放进双向共用的 hop-by-hop 剥离集、导致未解码透传请求体(#619 不可解码编码、未知路径)以"无编码声明的二进制字节"发给上游引发 400 的问题;按方向拆分剥离集(请求侧保留、响应侧仍剥离),已逐路径核实安全性并本地复跑 typecheck/1272 测试/build 全绿,可以合并。

@ranxianglei
ranxianglei merged commit 30ecbc1 into master Sep 11, 2026
6 checks passed
@ranxianglei ranxianglei mentioned this pull request Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant