net/http: in Transport, don't error on non-chunked response with Trailer header

There are cases where HTTP message specifies the Trailer header
but not the Transfer-Encoding = chunked. The existing
implementation would return an error in those cases, without
returning also the message itself.
Instead, it would be preferable to let the library user decide when
the message is valid or not.
This change makes the fixTrailer() function not to return an error
and to keep the Trailer value in the Response.Header map but not
populate Response.Trailer.

Fixes #27197

Change-Id: Ic1e96791fde97f31dc5ecb8de05c8e4f49465c2c
Reviewed-on: https://go-review.googlesource.com/c/145398
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/request.go b/src/net/http/request.go
index 3669f17f66cd36facd2d0ed2f6c61ec2a8d34172..0bcdeae0df1f582cc57a7c0c5bbbe18346e8e616 100644 (file)
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -53,8 +53,9 @@ var (
        // available.
        ErrNotSupported = &ProtocolError{"feature not supported"}
 
-       // ErrUnexpectedTrailer is returned by the Transport when a server
-       // replies with a Trailer header, but without a chunked reply.
+       // Deprecated: ErrUnexpectedTrailer is no longer returned by
+       // anything in the net/http package. Callers should not
+       // compare errors against this variable.
        ErrUnexpectedTrailer = &ProtocolError{"trailer header without chunked transfer encoding"}
 
        // ErrMissingBoundary is returned by Request.MultipartReader when the

diff --git a/src/net/http/response_test.go b/src/net/http/response_test.go
index c28b0cba89e87feed71beeea3d2002951fbea94d..c46f13f7988334e7697c9bcc13d6cf815693aee1 100644 (file)
--- a/src/net/http/response_test.go
+++ b/src/net/http/response_test.go
@@ -157,6 +157,34 @@ var respTests = []respTest{
                "Body here\ncontinued",
        },
 
+       // Trailer header but no TransferEncoding
+       {
+               "HTTP/1.0 200 OK\r\n" +
+                       "Trailer: Content-MD5, Content-Sources\r\n" +
+                       "Content-Length: 10\r\n" +
+                       "Connection: close\r\n" +
+                       "\r\n" +
+                       "Body here\n",
+
+               Response{
+                       Status:     "200 OK",
+                       StatusCode: 200,
+                       Proto:      "HTTP/1.0",
+                       ProtoMajor: 1,
+                       ProtoMinor: 0,
+                       Request:    dummyReq("GET"),
+                       Header: Header{
+                               "Connection":     {"close"},
+                               "Content-Length": {"10"},
+                               "Trailer":        []string{"Content-MD5, Content-Sources"},
+                       },
+                       Close:         true,
+                       ContentLength: 10,
+               },
+
+               "Body here\n",
+       },
+
        // Chunked response with Content-Length.
        {
                "HTTP/1.1 200 OK\r\n" +

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index f0b43844dd92c31d266867806e97321723515f5c..3eb9f0da91298357531b270e5170c980d1ecf62d 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -740,6 +740,16 @@ func fixTrailer(header Header, te []string) (Header, error) {
        if !ok {
                return nil, nil
        }
+       if !chunked(te) {
+               // Trailer and no chunking:
+               // this is an invalid use case for trailer header.
+               // Nevertheless, no error will be returned and we
+               // let users decide if this is a valid HTTP message.
+               // The Trailer header will be kept in Response.Header
+               // but not populate Response.Trailer.
+               // See issue #27197.
+               return nil, nil
+       }
        header.Del("Trailer")
 
        trailer := make(Header)
@@ -763,10 +773,6 @@ func fixTrailer(header Header, te []string) (Header, error) {
        if len(trailer) == 0 {
                return nil, nil
        }
-       if !chunked(te) {
-               // Trailer and no chunking
-               return nil, ErrUnexpectedTrailer
-       }
        return trailer, nil
 }