Skip to content

fix(proxy): read chunked HTTP request bodies on L7 forward path - #1687

Open
Frankie-Xu wants to merge 3 commits into
nolabs-ai:mainfrom
Frankie-Xu:fix/1635-chunked-request-body
Open

fix(proxy): read chunked HTTP request bodies on L7 forward path#1687
Frankie-Xu wants to merge 3 commits into
nolabs-ai:mainfrom
Frankie-Xu:fix/1635-chunked-request-body

Conversation

@Frankie-Xu

Copy link
Copy Markdown
Contributor

Summary

  • Decode Transfer-Encoding: chunked request bodies in read_request_body so L7 plaintext HTTP forward no longer hangs when clients omit Content-Length (e.g. Go net/http streaming readers).
  • Strip Transfer-Encoding from forwarded headers and re-frame upstream requests with a computed Content-Length.
  • Fail closed on malformed chunk framing (400 Bad Request / 413 Payload Too Large) instead of silently deadlocking.

Closes #1635

Approach

  1. reverse.rs: When headers declare chunked encoding, read and decode the body from the client stream (bounded by existing MAX_REQUEST_BODY = 16 MiB), using the same line-bounded reading pattern as tls_intercept/http1.rs::relay_chunked on the response side.
  2. filter_headers / filter_headers_multi: Strip Transfer-Encoding alongside Content-Length so upstream never receives Transfer-Encoding: chunked with zero chunks.
  3. server.rs forward path: Read body before building upstream headers; strip body-framing headers in strip_proxy_headers / strip_and_redeem_proxy_headers; append Content-Length after decode.

CONNECT blind-tunnel path is unchanged.

Test plan

  • cargo test -p nono-proxy — 493 unit/integration tests pass
  • cargo clippy -p nono-proxy -- -D warnings -D clippy::unwrap_used
  • make fmt-check
  • New unit tests:
    • chunked body decode (normal + buffered prefix)
    • empty chunked body (0\r\n\r\n)
    • malformed chunk → 400, no hang
    • Content-Length regression
    • has_chunked_transfer_encoding + header stripping
  • Manual: Linux curl chunked POST through proxy allow-list (issue repro steps)

Conflict note (#1683)

PR #1683 (fix(proxy): decode Basic auth for basic_auth phantom validation) also touches reverse.rs but is still open / not merged. This branch is based on current origin/main (post v0.74.0). Overlap is limited to unrelated areas of reverse.rs; rebasing after #1683 merges may require a trivial conflict resolution.

Agent Compliance Check

  • I am not prohibited from contributing under this policy
  • An issue already exists (chunked http request body Bug issue #1635)
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code (chunked line-reading pattern from tls_intercept/http1.rs::relay_chunked)
  • I did not use forbidden patterns such as unwrap/expect (production code)
  • I used NonoError/ProxyError where required
  • I validated and canonicalized all relevant paths (N/A — no new path grants)
  • This PR matches the approved or disclosed issue scope

Contributor disclosure: This PR was prepared by an AI coding agent (Cursor).

Made with Cursor

@Frankie-Xu

Copy link
Copy Markdown
Contributor Author

CI: please run / approve checks when available. cargo test -p nono-proxy (493 tests) and cargo clippy -p nono-proxy pass locally on macOS; full make ci hit a SIGKILL on nono-cli tests in this environment (likely OOM), unrelated to this change.

@github-actions github-actions Bot added bug Something isn't working nono-proxy size/medium labels Aug 19, 2026
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +400
Lines removed -41
Total changed 441
Classification Large (> 300 lines)

Affected crates

  • crates/nono-proxydownstream consumers depend on this crate. API or behaviour changes will affect external callers; treat any breaking change with extra scrutiny.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@nogent-nolabs-ai nogent-nolabs-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nogent code review

No blocking security issues; 2 correctness and stability concerns identified.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

Comment thread crates/nono-proxy/src/server.rs Outdated
Comment thread crates/nono-proxy/src/reverse.rs
@Frankie-Xu
Frankie-Xu marked this pull request as draft August 21, 2026 10:56
@Frankie-Xu

Copy link
Copy Markdown
Contributor Author

Converting to draft so only #1682 and #1683 stay ready. I'll mark this ready later, one PR at a time. No action needed from reviewers now.

@Frankie-Xu
Frankie-Xu force-pushed the fix/1635-chunked-request-body branch from fddffa3 to 988dd22 Compare August 31, 2026 18:12
@Frankie-Xu
Frankie-Xu marked this pull request as ready for review August 31, 2026 18:12

@nogent-nolabs-ai nogent-nolabs-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nogent code review

1 minor suggestion regarding header stripping consistency.

Findings (not tied to a changed line):

  • 🔒 [LOW · security] crates/nono-proxy/src/tls_intercept/http1.rs:87 — In filter_websocket_request, the list of stripped headers matches on content-length but does not include transfer-encoding. For consistency and defense-in-depth against potential request manipulation on upgrade paths, add transfer-encoding to the matches! macro in filter_websocket_request.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

Comment thread crates/nono-proxy/src/reverse.rs
Frankie-Xu and others added 3 commits September 2, 2026 00:30
Decode Transfer-Encoding: chunked request bodies in read_request_body
so the L7 plaintext forward path no longer hangs when clients send
chunked bodies without Content-Length. Strip Transfer-Encoding from
forwarded headers and re-frame with Content-Length upstream.

Closes nolabs-ai#1635

Signed-off-by: Frankie-Xu Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Always re-add Content-Length after stripping CL/chunked framing,
including empty bodies, and cap chunked request trailer lines to avoid
DoS from unbounded trailer loops.

Signed-off-by: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@Frankie-Xu
Frankie-Xu force-pushed the fix/1635-chunked-request-body branch from 988dd22 to ea9798f Compare September 1, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working nono-proxy size/large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chunked http request body Bug issue

1 participant