Skip to content

fix(proxy): preserve streamed request bodies - #764

Closed
beyondlong wants to merge 1 commit into
projectdiscovery:devfrom
beyondlong:fix/post-body-stream
Closed

beyondlong wants to merge 1 commit into
projectdiscovery:devfrom
beyondlong:fix/post-body-stream

Conversation

@beyondlong

Copy link
Copy Markdown

Proposed changes

Fixes #749.

ModifyRequest forced ContentLength to -1, while the asynchronous logger and HTTP transport both consumed the same live request body. The reader that lost that race received an empty body; in the reported case, the upstream server saw an empty chunked POST.

This change preserves the request's original content length and gives the asynchronous logger an independent streaming reader backed by io.Pipe. The upstream transport remains the source reader, so the request body is copied to logging as it is forwarded. If the logging reader closes, forwarding continues. This also avoids buffering attacker-controlled bodies in memory, which was the security issue identified on the earlier implementation in #750.

Regression coverage verifies:

  • a POST body and its original Content-Length reach a real httptest upstream without forced chunked encoding;
  • the forwarding and logging readers receive identical bodies;
  • unknown content lengths remain unknown;
  • an early logging-reader close does not break forwarding.

Verification:

GOTOOLCHAIN=go1.25.0+auto go test -race ./...
GOTOOLCHAIN=go1.25.0+auto go vet ./...
GOTOOLCHAIN=go1.25.0+auto go build ./...
golangci-lint run --timeout 5m  # 0 issues

Disclosure: the implementation and tests were prepared with Codex assistance and reviewed and verified locally by the contributor.

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (not applicable; no user-facing behavior or option was added)

@dwisiswant0 dwisiswant0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This patch separates the initial request reader, but io.PipeWriter.Write blocks forwarding until the logger consumes each write, and LogResponse still queues the live response and original request. Removing the forced unknown length also leaves match-and-replace requests with potentially stale framing metadata.

I'm superseding this patch with #765, which addresses body ownership across both request and response logging.

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.

2 participants