Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: http.Client connection reuse behaviour differs when receiving from HTTP response body with Content-Encoding: gzip vs. uncompressed #8910

Closed
gopherbot opened this issue Oct 8, 2014 · 3 comments

Comments

@gopherbot
Copy link

by akrennmair:

What does 'go version' print?

go version go1.3.3 darwin/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. go run server.go (see attached files)
2. go run client.go 
3. use tcpdump to observe that one connection is reused
4. go run client.go -compressed
5. use tcpdump to observe that for each HTTP request, a new connection is opened and
then closed again.

What happened?

In client.go, the resp.Body.Read() read the same content from the HTTP response body but
returned different values (err == io.EOF for uncompressed vs. err == nil for
compressed). In the case of the compressed HTTP response body, connection reuse did not
work properly.

What should have happened instead?

The resp.Body.Read() should have behaved the same, no matter whether the underlying HTTP
response is Content-Encoding: gzip or not, and connection reuse should have worked in
both cases.

Please provide any additional information below.

What I could gather from reading the source, http.noteEOFReader doesn't properly pick up
the EOF when the underlying io.Reader is a gzip.Reader, which in turn makes
http.persistConn think the subsequent Close() is an early close, which makes it not
reuse the connection for subsequent requests. I stumbled upon the issue when using
json.NewDecoder(resp.Body).Decode(...) and noticing connections not being reused. As a
workaround, I used ioutil.ReadAll() and json.Unmarshal(), and then boiled down the issue
to the attached files, with some advice and pointing in the right direction from Dave
Cheney on Twitter.

Attachments:

  1. server.go (709 bytes)
  2. client.go (623 bytes)
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-none.

@dsnet
Copy link
Member

dsnet commented Apr 2, 2016

@bradfitz This sounds awfully similar to what we just fixed ;)

Close?

@bradfitz
Copy link
Contributor

bradfitz commented Apr 2, 2016

Wow, this bug has been open forever. I don't remember seeing it at all. :-(

But yes, this was fixed for google/go-github#317 in 18072ad (https://golang.org/cl/21291) and a separate way (either alone fixes it) in c27efce (https://golang.org/cl/21302)

@bradfitz bradfitz closed this as completed Apr 2, 2016
@golang golang locked and limited conversation to collaborators Apr 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants