Skip to content

Fix unframed tunnel response hangs - #34

Merged
bjhaid merged 1 commit into
brexhq:mainfrom
pquerna:fix/close-unframed-tunnel-responses
Jun 2, 2026
Merged

bjhaid merged 1 commit into
brexhq:mainfrom
pquerna:fix/close-unframed-tunnel-responses

Conversation

@pquerna

@pquerna pquerna commented May 29, 2026

Copy link
Copy Markdown
Contributor

Fix Unframed Tunnel Response Hangs

Summary

Fix an HTTP/1.1 tunnel hang for unknown-length upstream responses.

When CrabTrap MITMs an HTTPS CONNECT tunnel, some upstream responses arrive
without Content-Length and without Transfer-Encoding. In that case, EOF is
the only valid response-body delimiter. CrabTrap was writing the response body
to the client but keeping the tunnel open, causing clients like curl to wait
indefinitely after receiving the full body.

This changes tunnel keep-alive behavior so unframed, unknown-length responses
close the tunnel after the response is written.

Reproduction

This hung behind CrabTrap before the fix:

curl -fsSL \
  --proxy 'http://gat_local_ct100_dev:@192.168.32.100:8080' \
  --cacert /opt/egress-gateway/crabtrap/certs/ca.crt \
  https://api.github.com/repos/openai/codex/releases/latest \
  -o /tmp/codex-release.json

Verbose output showed the response body was fully received, but curl waited for
EOF:

< HTTP/1.1 200 OK
* no chunk, no close, no size. Assume close to signal end
100 278k  0 278k ...

Concrete URL:

https://api.github.com/repos/openai/codex/releases/latest

This was observed while running the Codex installer from:

https://chatgpt.com/codex/install.sh

Fix

If a tunnel response has:

  • a response body
  • ContentLength < 0
  • no TransferEncoding
  • no explicit close handling already present

then CrabTrap no longer keeps the HTTP/1.1 tunnel alive. Closing the tunnel
gives the client the EOF delimiter it is waiting for.

Test

Added a focused test covering unknown-length, unframed tunnel responses:

TestUnknownLengthUnframedResponseDoesNotKeepTunnelAlive

Also manually verified the Codex release metadata and release tarball download
complete through CrabTrap after the fix.

@greptile-apps

greptile-apps Bot commented May 29, 2026

Copy link
Copy Markdown

Greptile Summary

Fixes HTTP/1.1 CONNECT tunnel hangs when upstream responses have no Content-Length and no Transfer-Encoding. Per RFC 7230 §3.3.3, EOF is the only valid body delimiter for such responses, so the tunnel must close after writing the response. The fix adds two checks to shouldKeepAlive: respect resp.Close from Go's HTTP transport, and detect unframed responses (body present, unknown length, no transfer encoding) to close the tunnel.

  • Adds resp.Close check to shouldKeepAlive in handler.go — aligns with Go's net/http signal that the server wants the connection closed
  • Adds unframed response detection (ContentLength < 0 && len(TransferEncoding) == 0) to prevent keep-alive on EOF-delimited responses
  • New test TestUnknownLengthUnframedResponseDoesNotKeepTunnelAlive validates the fix

Confidence Score: 5/5

This PR is safe to merge — it's a targeted, correct fix for a well-understood HTTP/1.1 protocol issue.

The change is minimal (9 lines of logic + test), correctly implements RFC 7230 §3.3.3 semantics, only affects the specific edge case of unframed responses, and includes a focused test. The conditions are well-guarded — normal keep-alive for framed responses (Content-Length or Transfer-Encoding present) is unaffected.

No files require special attention.

Important Files Changed

Filename Overview
internal/proxy/handler.go Adds two early-return checks to shouldKeepAlive: resp.Close and unframed response detection. Both are correct per HTTP/1.1 semantics and well-scoped.
internal/proxy/streaming_test.go Adds a focused test validating that shouldKeepAlive returns false for unframed responses.

Reviews (2): Last reviewed commit: "Fix unframed tunnel response hangs" | Re-trigger Greptile

@bjhaid bjhaid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks!

@bjhaid
bjhaid merged commit 83e8ddf into brexhq:main Jun 2, 2026
4 checks passed
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