diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go index 1d6a987545..2e01a07f84 100644 --- a/src/net/http/transfer.go +++ b/src/net/http/transfer.go @@ -7,7 +7,6 @@ package http import ( "bufio" "bytes" - "compress/gzip" "errors" "fmt" "io" @@ -467,34 +466,6 @@ func suppressedHeaders(status int) []string { return nil } -// proxyingReadCloser is a composite type that accepts and proxies -// io.Read and io.Close calls to its respective Reader and Closer. -// -// It is composed of: -// a) a top-level reader e.g. the result of decompression -// b) a symbolic Closer e.g. the result of decompression, the -// original body and the connection itself. -type proxyingReadCloser struct { - io.Reader - io.Closer -} - -// multiCloser implements io.Closer and allows a bunch of io.Closer values -// to all be closed once. -// Example usage is with proxyingReadCloser if we are decompressing a response -// body on the fly and would like to close both *gzip.Reader and underlying body. -type multiCloser []io.Closer - -func (mc multiCloser) Close() error { - var err error - for _, c := range mc { - if err1 := c.Close(); err1 != nil && err == nil { - err = err1 - } - } - return err -} - // msg is *Request or *Response. func readTransfer(msg interface{}, r *bufio.Reader) (err error) { t := &transferReader{RequestMethod: "GET"} @@ -572,7 +543,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { // Prepare body reader. ContentLength < 0 means chunked encoding // or close connection when finished, since multipart is not supported yet switch { - case chunked(t.TransferEncoding) || implicitlyChunked(t.TransferEncoding): + case chunked(t.TransferEncoding): if noResponseBodyExpected(t.RequestMethod) || !bodyAllowedForStatus(t.StatusCode) { t.Body = NoBody } else { @@ -593,21 +564,6 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { } } - // Finally if "gzip" was one of the requested transfer-encodings, - // we'll unzip the concatenated body/payload of the request. - // TODO: As we support more transfer-encodings, extract - // this code and apply the un-codings in reverse. - if t.Body != NoBody && gzipped(t.TransferEncoding) { - zr, err := gzip.NewReader(t.Body) - if err != nil { - return fmt.Errorf("http: failed to gunzip body: %v", err) - } - t.Body = &proxyingReadCloser{ - Reader: zr, - Closer: multiCloser{zr, t.Body}, - } - } - // Unify output switch rr := msg.(type) { case *Request: @@ -627,41 +583,8 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { return nil } -// Checks whether chunked is the last part of the encodings stack -func chunked(te []string) bool { return len(te) > 0 && te[len(te)-1] == "chunked" } - -// implicitlyChunked is a helper to check for implicity of chunked, because -// RFC 7230 Section 3.3.1 says that the sender MUST apply chunked as the final -// payload body to ensure that the message is framed for both the request -// and the body. Since "identity" is incompatible with any other transformational -// encoding cannot co-exist, the presence of "identity" will cause implicitlyChunked -// to return false. -func implicitlyChunked(te []string) bool { - if len(te) == 0 { // No transfer-encodings passed in, so not implicitly chunked. - return false - } - for _, tei := range te { - if tei == "identity" { - return false - } - } - return true -} - -func isGzipTransferEncoding(tei string) bool { - // RFC 7230 4.2.3 requests that "x-gzip" SHOULD be considered the same as "gzip". - return tei == "gzip" || tei == "x-gzip" -} - -// Checks where either of "gzip" or "x-gzip" are contained in transfer encodings. -func gzipped(te []string) bool { - for _, tei := range te { - if isGzipTransferEncoding(tei) { - return true - } - } - return false -} +// Checks whether chunked is part of the encodings stack +func chunked(te []string) bool { return len(te) > 0 && te[0] == "chunked" } // Checks whether the encoding is explicitly "identity". func isIdentity(te []string) bool { return len(te) == 1 && te[0] == "identity" } @@ -697,47 +620,25 @@ func (t *transferReader) fixTransferEncoding() error { encodings := strings.Split(raw[0], ",") te := make([]string, 0, len(encodings)) - - // When adding new encodings, please maintain the invariant: - // if chunked encoding is present, it must always - // come last and it must be applied only once. - // See RFC 7230 Section 3.3.1 Transfer-Encoding. - for i, encoding := range encodings { + // TODO: Even though we only support "identity" and "chunked" + // encodings, the loop below is designed with foresight. One + // invariant that must be maintained is that, if present, + // chunked encoding must always come first. + for _, encoding := range encodings { encoding = strings.ToLower(strings.TrimSpace(encoding)) - + // "identity" encoding is not recorded if encoding == "identity" { - // "identity" should not be mixed with other transfer-encodings/compressions - // because it means "no compression, no transformation". - if len(encodings) != 1 { - return &badStringError{`"identity" when present must be the only transfer encoding`, strings.Join(encodings, ",")} - } - // "identity" is not recorded. break } - - switch { - case encoding == "chunked": - // "chunked" MUST ALWAYS be the last - // encoding as per the loop invariant. - // That is: - // Invalid: [chunked, gzip] - // Valid: [gzip, chunked] - if i+1 != len(encodings) { - return &badStringError{"chunked must be applied only once, as the last encoding", strings.Join(encodings, ",")} - } - // Supported otherwise. - - case isGzipTransferEncoding(encoding): - // Supported - - default: + if encoding != "chunked" { return &unsupportedTEError{fmt.Sprintf("unsupported transfer encoding: %q", encoding)} } - te = te[0 : len(te)+1] te[len(te)-1] = encoding } - + if len(te) > 1 { + return &badStringError{"too many transfer encodings", strings.Join(te, ",")} + } if len(te) > 0 { // RFC 7230 3.3.2 says "A sender MUST NOT send a // Content-Length header field in any message that diff --git a/src/net/http/transfer_test.go b/src/net/http/transfer_test.go index a8ce2d3709..65009ee8bf 100644 --- a/src/net/http/transfer_test.go +++ b/src/net/http/transfer_test.go @@ -7,7 +7,6 @@ package http import ( "bufio" "bytes" - "compress/gzip" "crypto/rand" "fmt" "io" @@ -62,6 +61,7 @@ func TestFinalChunkedBodyReadEOF(t *testing.T) { buf := make([]byte, len(want)) n, err := res.Body.Read(buf) if n != len(want) || err != io.EOF { + t.Logf("body = %#v", res.Body) t.Errorf("Read = %v, %v; want %d, EOF", n, err, len(want)) } if string(buf) != want { @@ -290,7 +290,7 @@ func TestFixTransferEncoding(t *testing.T) { }, { hdr: Header{"Transfer-Encoding": {"chunked, chunked", "identity", "chunked"}}, - wantErr: &badStringError{"chunked must be applied only once, as the last encoding", "chunked, chunked"}, + wantErr: &badStringError{"too many transfer encodings", "chunked,chunked"}, }, { hdr: Header{"Transfer-Encoding": {"chunked"}}, @@ -310,283 +310,3 @@ func TestFixTransferEncoding(t *testing.T) { } } } - -func gzipIt(s string) string { - buf := new(bytes.Buffer) - gw := gzip.NewWriter(buf) - gw.Write([]byte(s)) - gw.Close() - return buf.String() -} - -func TestUnitTestProxyingReadCloserClosesBody(t *testing.T) { - var checker closeChecker - buf := new(bytes.Buffer) - buf.WriteString("Hello, Gophers!") - prc := &proxyingReadCloser{ - Reader: buf, - Closer: &checker, - } - prc.Close() - - read, err := ioutil.ReadAll(prc) - if err != nil { - t.Fatalf("Read error: %v", err) - } - if g, w := string(read), "Hello, Gophers!"; g != w { - t.Errorf("Read mismatch: got %q want %q", g, w) - } - - if checker.closed != true { - t.Fatal("closeChecker.Close was never invoked") - } -} - -func TestGzipTransferEncoding_request(t *testing.T) { - helloWorldGzipped := gzipIt("Hello, World!") - - tests := []struct { - payload string - wantErr string - wantBody string - }{ - - { - // The case of "chunked" properly applied as the last encoding - // and a gzipped request payload that is streamed in 3 parts. - payload: `POST / HTTP/1.1 -Host: golang.org -Transfer-Encoding: gzip, chunked -Content-Type: text/html; charset=UTF-8 - -` + fmt.Sprintf("%02x\r\n%s\r\n%02x\r\n%s\r\n%02x\r\n%s\r\n0\r\n\r\n", - 3, helloWorldGzipped[:3], - 5, helloWorldGzipped[3:8], - len(helloWorldGzipped)-8, helloWorldGzipped[8:]), - wantBody: `Hello, World!`, - }, - - { - // The request specifies "Transfer-Encoding: chunked" so its body must be left untouched. - payload: `PUT / HTTP/1.1 -Host: golang.org -Transfer-Encoding: chunked -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + fmt.Sprintf("%0x\r\n%s\r\n0\r\n\r\n", len(helloWorldGzipped), helloWorldGzipped), - // We want that payload as it was sent. - wantBody: helloWorldGzipped, - }, - - { - // Valid request, the body doesn't have "Transfer-Encoding: chunked" but implicitly encoded - // for chunking as per the advisory from RFC 7230 3.3.1 which advises for cases where. - payload: `POST / HTTP/1.1 -Host: localhost -Transfer-Encoding: gzip -Content-Type: text/html; charset=UTF-8 - -` + fmt.Sprintf("%0x\r\n%s\r\n0\r\n\r\n", len(helloWorldGzipped), helloWorldGzipped), - wantBody: `Hello, World!`, - }, - - { - // Invalid request, the body isn't chunked nor is the connection terminated immediately - // hence invalid as per the advisory from RFC 7230 3.3.1 which advises for cases where - // a Transfer-Encoding that isn't finally chunked is provided. - payload: `PUT / HTTP/1.1 -Host: golang.org -Transfer-Encoding: gzip -Content-Length: 0 -Connection: close -Content-Type: text/html; charset=UTF-8 - -`, - wantErr: `EOF`, - }, - - { - // The case of chunked applied before another encoding. - payload: `PUT / HTTP/1.1 -Location: golang.org -Transfer-Encoding: chunked, gzip -Content-Length: 0 -Connection: close -Content-Type: text/html; charset=UTF-8 - -`, - wantErr: `chunked must be applied only once, as the last encoding "chunked, gzip"`, - }, - - { - // The case of chunked properly applied as the - // last encoding BUT with a bad "Content-Length". - payload: `POST / HTTP/1.1 -Host: golang.org -Transfer-Encoding: gzip, chunked -Content-Length: 10 -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + "0\r\n\r\n", - wantErr: "EOF", - }, - } - - for i, tt := range tests { - req, err := ReadRequest(bufio.NewReader(strings.NewReader(tt.payload))) - if tt.wantErr != "" { - if err == nil || !strings.Contains(err.Error(), tt.wantErr) { - t.Errorf("test %d. Error mismatch\nGot: %v\nWant: %s", i, err, tt.wantErr) - } - continue - } - - if err != nil { - t.Errorf("test %d. Unexpected ReadRequest error: %v\nPayload:\n%s", i, err, tt.payload) - continue - } - - got, err := ioutil.ReadAll(req.Body) - req.Body.Close() - if err != nil { - t.Errorf("test %d. Failed to read response body: %v", i, err) - } - if g, w := string(got), tt.wantBody; g != w { - t.Errorf("test %d. Request body mimsatch\nGot:\n%s\n\nWant:\n%s", i, g, w) - } - } -} - -func TestGzipTransferEncoding_response(t *testing.T) { - helloWorldGzipped := gzipIt("Hello, World!") - - tests := []struct { - payload string - wantErr string - wantBody string - }{ - - { - // The case of "chunked" properly applied as the last encoding - // and a gzipped payload that is streamed in 3 parts. - payload: `HTTP/1.1 302 Found -Location: https://golang.org/ -Transfer-Encoding: gzip, chunked -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + fmt.Sprintf("%02x\r\n%s\r\n%02x\r\n%s\r\n%02x\r\n%s\r\n0\r\n\r\n", - 3, helloWorldGzipped[:3], - 5, helloWorldGzipped[3:8], - len(helloWorldGzipped)-8, helloWorldGzipped[8:]), - wantBody: `Hello, World!`, - }, - - { - // The response specifies "Transfer-Encoding: chunked" so response body must be left untouched. - payload: `HTTP/1.1 302 Found -Location: https://golang.org/ -Transfer-Encoding: chunked -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + fmt.Sprintf("%0x\r\n%s\r\n0\r\n\r\n", len(helloWorldGzipped), helloWorldGzipped), - // We want that payload as it was sent. - wantBody: helloWorldGzipped, - }, - - { - // Valid response, the body doesn't have "Transfer-Encoding: chunked" but implicitly encoded - // for chunking as per the advisory from RFC 7230 3.3.1 which advises for cases where. - payload: `HTTP/1.1 302 Found -Location: https://golang.org/ -Transfer-Encoding: gzip -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + fmt.Sprintf("%0x\r\n%s\r\n0\r\n\r\n", len(helloWorldGzipped), helloWorldGzipped), - wantBody: `Hello, World!`, - }, - - { - // Invalid response, the body isn't chunked nor is the connection terminated immediately - // hence invalid as per the advisory from RFC 7230 3.3.1 which advises for cases where - // a Transfer-Encoding that isn't finally chunked is provided. - payload: `HTTP/1.1 302 Found -Location: https://golang.org/ -Transfer-Encoding: gzip -Content-Length: 0 -Connection: close -Content-Type: text/html; charset=UTF-8 - -`, - wantErr: `EOF`, - }, - - { - // The case of chunked applied before another encoding. - payload: `HTTP/1.1 302 Found -Location: https://golang.org/ -Transfer-Encoding: chunked, gzip -Content-Length: 0 -Connection: close -Content-Type: text/html; charset=UTF-8 - -`, - wantErr: `chunked must be applied only once, as the last encoding "chunked, gzip"`, - }, - - { - // The case of chunked properly applied as the - // last encoding BUT with a bad "Content-Length". - payload: `HTTP/1.1 302 Found -Location: https://golang.org/ -Transfer-Encoding: gzip, chunked -Content-Length: 10 -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + "0\r\n\r\n", - wantErr: "EOF", - }, - - { - // Including "identity" more than once. - payload: `HTTP/1.1 200 OK -Location: https://golang.org/ -Transfer-Encoding: identity, identity -Content-Length: 0 -Connection: close -Content-Type: text/html; charset=UTF-8 - -` + "0\r\n\r\n", - wantErr: `"identity" when present must be the only transfer encoding "identity, identity"`, - }, - } - - for i, tt := range tests { - res, err := ReadResponse(bufio.NewReader(strings.NewReader(tt.payload)), nil) - if tt.wantErr != "" { - if err == nil || !strings.Contains(err.Error(), tt.wantErr) { - t.Errorf("test %d. Error mismatch\nGot: %v\nWant: %s", i, err, tt.wantErr) - } - continue - } - - if err != nil { - t.Errorf("test %d. Unexpected ReadResponse error: %v\nPayload:\n%s", i, err, tt.payload) - continue - } - - got, err := ioutil.ReadAll(res.Body) - res.Body.Close() - if err != nil { - t.Errorf("test %d. Failed to read response body: %v", i, err) - } - if g, w := string(got), tt.wantBody; g != w { - t.Errorf("test %d. Response body mimsatch\nGot:\n%s\n\nWant:\n%s", i, g, w) - } - } -}