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

Projects
None yet
5 participants
@gopherbot
Copy link

commented Oct 8, 2014

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

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2014

Comment 1:

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

@gopherbot gopherbot added new labels Oct 8, 2014

@bradfitz bradfitz removed the new label Dec 18, 2014

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@dsnet

This comment has been minimized.

Copy link
Member

commented Apr 2, 2016

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

Close?

@bradfitz

This comment has been minimized.

Copy link
Member

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 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.
You can’t perform that action at this time.