]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: add Response.Uncompressed bool
authorBrad Fitzpatrick <bradfitz@golang.org>
Sat, 16 Apr 2016 16:35:32 +0000 (09:35 -0700)
committerBrad Fitzpatrick <bradfitz@golang.org>
Sun, 1 May 2016 06:29:57 +0000 (06:29 +0000)
The Transport's automatic gzip uncompression lost information in the
process (the compressed Content-Length, if known). Normally that's
okay, but it's not okay for reverse proxies which have to be able to
generate a valid HTTP response from the Transport's provided
*Response.

Reverse proxies should normally be disabling compression anyway and
just piping the compressed pipes though and not wasting CPU cycles
decompressing them. So also document that on the new Uncompressed
field.

Then, using the new field, fix Response.Write to not inject a bogus
"Connection: close" header when it doesn't see a transfer encoding or
content-length.

Updates #15366 (the http2 side remains, once this is submitted)

Change-Id: I476f40aa14cfa7aa7b3bf99021bebba4639f9640
Reviewed-on: https://go-review.googlesource.com/22671
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/http/clientserver_test.go
src/net/http/httputil/dump.go
src/net/http/response.go
src/net/http/response_test.go
src/net/http/transport.go

index f7213823650131a92ab67188549ba01ee5feb1da..9c3949fc3970f8125da57a391b1fbc0bbe245026 100644 (file)
@@ -17,6 +17,7 @@ import (
        "net"
        . "net/http"
        "net/http/httptest"
+       "net/http/httputil"
        "net/url"
        "os"
        "reflect"
@@ -147,10 +148,11 @@ type reqFunc func(c *Client, url string) (*Response, error)
 // h12Compare is a test that compares HTTP/1 and HTTP/2 behavior
 // against each other.
 type h12Compare struct {
-       Handler       func(ResponseWriter, *Request)    // required
-       ReqFunc       reqFunc                           // optional
-       CheckResponse func(proto string, res *Response) // optional
-       Opts          []interface{}
+       Handler            func(ResponseWriter, *Request)    // required
+       ReqFunc            reqFunc                           // optional
+       CheckResponse      func(proto string, res *Response) // optional
+       EarlyCheckResponse func(proto string, res *Response) // optional; pre-normalize
+       Opts               []interface{}
 }
 
 func (tt h12Compare) reqFunc() reqFunc {
@@ -176,6 +178,12 @@ func (tt h12Compare) run(t *testing.T) {
                t.Errorf("HTTP/2 request: %v", err)
                return
        }
+
+       if fn := tt.EarlyCheckResponse; fn != nil {
+               fn("HTTP/1.1", res1)
+               fn("HTTP/2.0", res2)
+       }
+
        tt.normalizeRes(t, res1, "HTTP/1.1")
        tt.normalizeRes(t, res2, "HTTP/2.0")
        res1body, res2body := res1.Body, res2.Body
@@ -220,6 +228,12 @@ func (tt h12Compare) normalizeRes(t *testing.T, res *Response, wantProto string)
                t.Errorf("got %q response; want %q", res.Proto, wantProto)
        }
        slurp, err := ioutil.ReadAll(res.Body)
+
+       // TODO(bradfitz): short-term hack. Fix the
+       // http2 side of golang.org/issue/15366 once
+       // the http1 part is submitted.
+       res.Uncompressed = false
+
        res.Body.Close()
        res.Body = slurpResult{
                ReadCloser: ioutil.NopCloser(bytes.NewReader(slurp)),
@@ -1151,6 +1165,41 @@ func testInterruptWithPanic(t *testing.T, h2 bool) {
        }
 }
 
+// Issue 15366
+func TestH12_AutoGzipWithDumpResponse(t *testing.T) {
+       h12Compare{
+               Handler: func(w ResponseWriter, r *Request) {
+                       h := w.Header()
+                       h.Set("Content-Encoding", "gzip")
+                       h.Set("Content-Length", "23")
+                       h.Set("Connection", "keep-alive")
+                       io.WriteString(w, "\x1f\x8b\b\x00\x00\x00\x00\x00\x00\x00s\xf3\xf7\a\x00\xab'\xd4\x1a\x03\x00\x00\x00")
+               },
+               EarlyCheckResponse: func(proto string, res *Response) {
+                       if proto == "HTTP/2.0" {
+                               // TODO(bradfitz): Fix the http2 side
+                               // of golang.org/issue/15366 once the
+                               // http1 part is submitted.
+                               return
+                       }
+                       if !res.Uncompressed {
+                               t.Errorf("%s: expected Uncompressed to be set", proto)
+                       }
+                       dump, err := httputil.DumpResponse(res, true)
+                       if err != nil {
+                               t.Errorf("%s: DumpResponse: %v", proto, err)
+                               return
+                       }
+                       if strings.Contains(string(dump), "Connection: close") {
+                               t.Errorf("%s: should not see \"Connection: close\" in dump; got:\n%s", proto, dump)
+                       }
+                       if !strings.Contains(string(dump), "FOO") {
+                               t.Errorf("%s: should see \"FOO\" in response; got:\n%s", proto, dump)
+                       }
+               },
+       }.run(t)
+}
+
 type noteCloseConn struct {
        net.Conn
        closeFunc func()
index 692ab62c9b718f26dc8847744c5355294610978f..15116816328693db674f02a85cd446188b91b720 100644 (file)
@@ -183,7 +183,8 @@ var reqWriteExcludeHeaderDump = map[string]bool{
 //
 // The documentation for http.Request.Write details which fields
 // of req are included in the dump.
-func DumpRequest(req *http.Request, body bool) (dump []byte, err error) {
+func DumpRequest(req *http.Request, body bool) ([]byte, error) {
+       var err error
        save := req.Body
        if !body || req.Body == nil {
                req.Body = nil
@@ -231,7 +232,7 @@ func DumpRequest(req *http.Request, body bool) (dump []byte, err error) {
 
        err = req.Header.WriteSubset(&b, reqWriteExcludeHeaderDump)
        if err != nil {
-               return
+               return nil, err
        }
 
        io.WriteString(&b, "\r\n")
@@ -250,10 +251,9 @@ func DumpRequest(req *http.Request, body bool) (dump []byte, err error) {
 
        req.Body = save
        if err != nil {
-               return
+               return nil, err
        }
-       dump = b.Bytes()
-       return
+       return b.Bytes(), nil
 }
 
 // errNoBody is a sentinel error value used by failureToReadBody so we
@@ -273,8 +273,9 @@ func (failureToReadBody) Close() error             { return nil }
 var emptyBody = ioutil.NopCloser(strings.NewReader(""))
 
 // DumpResponse is like DumpRequest but dumps a response.
-func DumpResponse(resp *http.Response, body bool) (dump []byte, err error) {
+func DumpResponse(resp *http.Response, body bool) ([]byte, error) {
        var b bytes.Buffer
+       var err error
        save := resp.Body
        savecl := resp.ContentLength
 
index 91d4ffb7ec79e9c35449a6df3092d165dddcbf79..0164a09c6a0d2a76bff4a2e834bd7e53dda274ad 100644 (file)
@@ -73,6 +73,15 @@ type Response struct {
        // ReadResponse nor Response.Write ever closes a connection.
        Close bool
 
+       // Uncompressed reports whether the response was sent compressed but
+       // was decompressed by the http package. When true, reading from
+       // Body yields the uncompressed content instead of the compressed
+       // content actually set from the server, ContentLength is set to -1,
+       // and the "Content-Length" and "Content-Encoding" fields are deleted
+       // from the responseHeader. To get the original response from
+       // the server, set Transport.DisableCompression to true.
+       Uncompressed bool
+
        // Trailer maps trailer keys to values in the same
        // format as Header.
        //
@@ -268,7 +277,7 @@ func (r *Response) Write(w io.Writer) error {
        // content-length, the only way to do that is the old HTTP/1.0
        // way, by noting the EOF with a connection close, so we need
        // to set Close.
-       if r1.ContentLength == -1 && !r1.Close && r1.ProtoAtLeast(1, 1) && !chunked(r1.TransferEncoding) {
+       if r1.ContentLength == -1 && !r1.Close && r1.ProtoAtLeast(1, 1) && !chunked(r1.TransferEncoding) && !r1.Uncompressed {
                r1.Close = true
        }
 
index 2591e3ac81258e0d186fe87cd5f8bd1c60b93afa..126da9273557c43cefa7391c3d3e9e865502aba8 100644 (file)
@@ -506,6 +506,32 @@ some body`,
 
                "Body here\n",
        },
+
+       {
+               "HTTP/1.1 200 OK\r\n" +
+                       "Content-Encoding: gzip\r\n" +
+                       "Content-Length: 23\r\n" +
+                       "Connection: keep-alive\r\n" +
+                       "Keep-Alive: timeout=7200\r\n\r\n" +
+                       "\x1f\x8b\b\x00\x00\x00\x00\x00\x00\x00s\xf3\xf7\a\x00\xab'\xd4\x1a\x03\x00\x00\x00",
+               Response{
+                       Status:     "200 OK",
+                       StatusCode: 200,
+                       Proto:      "HTTP/1.1",
+                       ProtoMajor: 1,
+                       ProtoMinor: 1,
+                       Request:    dummyReq("GET"),
+                       Header: Header{
+                               "Content-Length":   {"23"},
+                               "Content-Encoding": {"gzip"},
+                               "Connection":       {"keep-alive"},
+                               "Keep-Alive":       {"timeout=7200"},
+                       },
+                       Close:         false,
+                       ContentLength: 23,
+               },
+               "\x1f\x8b\b\x00\x00\x00\x00\x00\x00\x00s\xf3\xf7\a\x00\xab'\xd4\x1a\x03\x00\x00\x00",
+       },
 }
 
 // tests successful calls to ReadResponse, and inspects the returned Response.
index f9cbd06a79b8794370dda9f82bd1fe79ed365202..0f11676de6f4231424694c7d0f21eb66de340d1c 100644 (file)
@@ -1392,6 +1392,7 @@ func (pc *persistConn) readLoop() {
                        resp.Header.Del("Content-Encoding")
                        resp.Header.Del("Content-Length")
                        resp.ContentLength = -1
+                       resp.Uncompressed = true
                }
 
                select {