Skip to content

fix(docker-reverse-proxy): fix response body handling in proxy and token acquisition#3134

Open
AdaAibaby wants to merge 2 commits into
e2b-dev:mainfrom
AdaAibaby:fix/docker-reverse-proxy-response-body-handling
Open

fix(docker-reverse-proxy): fix response body handling in proxy and token acquisition#3134
AdaAibaby wants to merge 2 commits into
e2b-dev:mainfrom
AdaAibaby:fix/docker-reverse-proxy-response-body-handling

Conversation

@AdaAibaby

@AdaAibaby AdaAibaby commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

/cc @jakubno @dobrac @ValentaTomas Would appreciate your review on this change, thanks!

Fix two response body handling issues in the docker-reverse-proxy package.

1. ModifyResponse consumes response body without restoring it (store.go)

The ModifyResponse callback reads the entire response body via io.ReadAll on
401 Unauthorized responses for logging purposes, but never restores it. Since
httputil.ReverseProxy forwards resp.Body to the client after
ModifyResponse returns, the client receives an empty 401 response body
with no error details.

Fix: Close the consumed body, restore it with io.NopCloser(bytes.NewReader(...)),
and update ContentLength so the proxy writes the correct Content-Length header.

2. Potential panic + duplicate defer in getToken (token.go)

body := make([]byte, resp.ContentLength)  // panics if ContentLength == -1

…ken acquisition

store.go: ModifyResponse consumed resp.Body via io.ReadAll on 401
responses but never restored it, causing the reverse proxy to forward
an empty body to the client. Restore the body with io.NopCloser after
logging.

token.go: getToken used make([]byte, resp.ContentLength) which panics
when ContentLength is -1 (chunked/unknown). Replace with io.ReadAll.
Also remove a duplicate defer resp.Body.Close() (already deferred at
line 139).

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

Ignoring the error from io.ReadAll in ModifyResponse can lead to silent response truncation and should be handled properly. Modifying resp.ContentLength without updating the Content-Length header in resp.Header causes a mismatch that can lead to protocol violations or client hangs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/docker-reverse-proxy/internal/handlers/store.go Outdated
Comment thread packages/docker-reverse-proxy/internal/handlers/store.go
…ength header

Address review feedback:
- Handle io.ReadAll error in ModifyResponse to avoid silent truncation
- Sync Content-Length header with resp.ContentLength after body restore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants