Commit 42c31307 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

net/http: don't send implicit gzip Accept-Encoding on Range requests

The http package by default adds "Accept-Encoding: gzip" to outgoing
requests, unless it's a bad idea, or the user requested otherwise.
Only when the http package adds its own implicit Accept-Encoding header
does the http package also transparently un-gzip the response.

If the user requested part of a document (e.g. bytes 40 to 50), it appears
that Github/Varnish send:
        range(gzip(content), 40, 50)

And not:
        gzip(range(content, 40, 50))

The RFC 2616 set of replacements (with the purpose of
clarifying ambiguities since 1999) has an RFC about Range
requests (http://tools.ietf.org/html/rfc7233) but does not
mention the interaction with encodings.

Regardless of whether range(gzip(content)) or gzip(range(content)) is
correct, this change prevents the Go package from asking for gzip
in requests if we're also asking for Range, avoiding the issue.
If the user cared, they can do it themselves. But Go transparently
un-gzipping a fragment of gzip is never useful.

Fixes #8923

LGTM=adg
R=adg
CC=golang-codereviews
https://golang.org/cl/155420044
parent 9d51cd0f
......@@ -377,6 +377,34 @@ some body`,
"Body here\n",
},
// 206 Partial Content. golang.org/issue/8923
{
"HTTP/1.1 206 Partial Content\r\n" +
"Content-Type: text/plain; charset=utf-8\r\n" +
"Accept-Ranges: bytes\r\n" +
"Content-Range: bytes 0-5/1862\r\n" +
"Content-Length: 6\r\n\r\n" +
"foobar",
Response{
Status: "206 Partial Content",
StatusCode: 206,
Proto: "HTTP/1.1",
ProtoMajor: 1,
ProtoMinor: 1,
Request: dummyReq("GET"),
Header: Header{
"Accept-Ranges": []string{"bytes"},
"Content-Length": []string{"6"},
"Content-Type": []string{"text/plain; charset=utf-8"},
"Content-Range": []string{"bytes 0-5/1862"},
},
ContentLength: 6,
},
"foobar",
},
}
func TestReadResponse(t *testing.T) {
......
......@@ -1040,11 +1040,14 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err
}
// Ask for a compressed version if the caller didn't set their
// own value for Accept-Encoding. We only attempted to
// own value for Accept-Encoding. We only attempt to
// uncompress the gzip stream if we were the layer that
// requested it.
requestedGzip := false
if !pc.t.DisableCompression && req.Header.Get("Accept-Encoding") == "" && req.Method != "HEAD" {
if !pc.t.DisableCompression &&
req.Header.Get("Accept-Encoding") == "" &&
req.Header.Get("Range") == "" &&
req.Method != "HEAD" {
// Request gzip only, not deflate. Deflate is ambiguous and
// not as universally supported anyway.
// See: http://www.gzip.org/zlib/zlib_faq.html#faq38
......@@ -1053,6 +1056,10 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err
// due to a bug in nginx:
// http://trac.nginx.org/nginx/ticket/358
// http://golang.org/issue/5522
//
// We don't request gzip if the request is for a range, since
// auto-decoding a portion of a gzipped document will just fail
// anyway. See http://golang.org/issue/8923
requestedGzip = true
req.extraHeaders().Set("Accept-Encoding", "gzip")
}
......
......@@ -2216,6 +2216,39 @@ func TestTransportCloseIdleConnsThenReturn(t *testing.T) {
wantIdle("after final put", 1)
}
// This tests that an client requesting a content range won't also
// implicitly ask for gzip support. If they want that, they need to do it
// on their own.
// golang.org/issue/8923
func TestTransportRangeAndGzip(t *testing.T) {
defer afterTest(t)
reqc := make(chan *Request, 1)
ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
reqc <- r
}))
defer ts.Close()
req, _ := NewRequest("GET", ts.URL, nil)
req.Header.Set("Range", "bytes=7-11")
res, err := DefaultClient.Do(req)
if err != nil {
t.Fatal(err)
}
select {
case r := <-reqc:
if strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
t.Error("Transport advertised gzip support in the Accept header")
}
if r.Header.Get("Range") == "" {
t.Error("no Range in request")
}
case <-time.After(10 * time.Second):
t.Fatal("timeout")
}
res.Body.Close()
}
func wantBody(res *http.Response, err error, want string) error {
if err != nil {
return err
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment