code review 155420044: net/http: don't send implicit gzip Accept-Encoding on R...

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 Issue 8923

[bradfitz, 2014-10-15 15:27:03]

Hello adg@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[adg, 2014-10-15 15:37:59]

LGTM

[bradfitz, 2014-10-15 15:51:17]

*** Submitted as https://code.google.com/p/go/source/detail?r=8ad75982389b ***

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 Issue 8923

LGTM=adg
R=adg
CC=golang-codereviews
https://codereview.appspot.com/155420044

[gobot, 2014-10-16 03:34:54]

This changed caused perf changes on windows-amd64-perf:


json-1                    old          new      delta
cputime             190000000    177187500      -6.74
time                190151452    177179758      -6.82

json-2                    old          new      delta
cputime             189375000    180156250      -4.87
time                 95976543     90381493      -5.83

json-4                    old          new      delta
cputime             192343750    180859375      -5.97
time                 48829030     46371814      -5.03

json-8                    old          new      delta
cputime             202593750    191906250      -5.28
time                 26039480     24704277      -5.13

http://build.golang.org/perfdetail?commit=8ad75982389b5c458a3c3c265568df912fb...



—gobot

[bradfitz, 2014-10-16 08:45:39]

I really doubt it.  Can we disable these emails until they're accurate?


On Thu, Oct 16, 2014 at 5:34 AM, <gobot@golang.org> wrote:

> This changed caused perf changes on windows-amd64-perf:
>
>
> json-1                    old          new      delta
> cputime             190000000    177187500      -6.74
> time                190151452    177179758      -6.82
>
> json-2                    old          new      delta
> cputime             189375000    180156250      -4.87
> time                 95976543     90381493      -5.83
>
> json-4                    old          new      delta
> cputime             192343750    180859375      -5.97
> time                 48829030     46371814      -5.03
>
> json-8                    old          new      delta
> cputime             202593750    191906250      -5.28
> time                 26039480     24704277      -5.13
>
> http://build.golang.org/perfdetail?commit=8ad75982389b5c458a3c3c265568df
> 912fbcde4c&commit0=3266b7e742ba13bf2207d7e1447155653289629d&kind=builder&
> builder=windows-amd64-perf
>
>
>
> --gobot
>
>
> https://codereview.appspot.com/155420044/
>