fix(docker-reverse-proxy): fix response body handling in proxy and token acquisition#3134
Open
AdaAibaby wants to merge 2 commits into
Open
Conversation
…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).
Contributor
There was a problem hiding this comment.
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.
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/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.
ModifyResponseconsumes response body without restoring it (store.go)The
ModifyResponsecallback reads the entire response body viaio.ReadAllon401 Unauthorized responses for logging purposes, but never restores it. Since
httputil.ReverseProxyforwardsresp.Bodyto the client afterModifyResponsereturns, the client receives an empty 401 response bodywith no error details.
Fix: Close the consumed body, restore it with
io.NopCloser(bytes.NewReader(...)),and update
ContentLengthso the proxy writes the correctContent-Lengthheader.2. Potential panic + duplicate defer in
getToken(token.go)