net/http: split Trailers tests into two halves

The old test was in client_test.go but was a mix of four things:

- clients writing trailers
- servers reading trailers
- servers writing trailers
- clients reading trailers

It definitely wasn't just about clients.

This moves it into clientserver_test.go and separates it into two
halves:

- servers writing trailers + clients reading trailers
- clients writing trailers + servers reading trailers

Which still isn't ideal, but is much better, and easier to read.

Updates #13557

Change-Id: I8c3e58a1f974c1b10bb11ef9b588cfa0f73ff5d9
Reviewed-on: https://go-review.googlesource.com/17895
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Blake Mizerany <blake.mizerany@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index 3aa5b5d3ef7d5580f76b10c200aba3b0e011050a..9d3444c89af32d2a6b7827f1ec12bbbfcdd9f4e1 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -20,8 +20,6 @@ import (
        . "net/http"
        "net/http/httptest"
        "net/url"
-       "reflect"
-       "sort"
        "strconv"
        "strings"
        "sync"
@@ -1096,81 +1094,6 @@ func (f eofReaderFunc) Read(p []byte) (n int, err error) {
        return 0, io.EOF
 }
 
-func TestClientTrailers_h1(t *testing.T) { testClientTrailers(t, h1Mode) }
-func TestClientTrailers_h2(t *testing.T) {
-       t.Skip("skipping in http2 mode; golang.org/issue/13557")
-       testClientTrailers(t, h2Mode)
-}
-func testClientTrailers(t *testing.T, h2 bool) {
-       defer afterTest(t)
-       cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
-               w.Header().Set("Connection", "close")
-               w.Header().Set("Trailer", "Server-Trailer-A, Server-Trailer-B")
-               w.Header().Add("Trailer", "Server-Trailer-C")
-
-               var decl []string
-               for k := range r.Trailer {
-                       decl = append(decl, k)
-               }
-               sort.Strings(decl)
-
-               slurp, err := ioutil.ReadAll(r.Body)
-               if err != nil {
-                       t.Errorf("Server reading request body: %v", err)
-               }
-               if string(slurp) != "foo" {
-                       t.Errorf("Server read request body %q; want foo", slurp)
-               }
-               if r.Trailer == nil {
-                       io.WriteString(w, "nil Trailer")
-               } else {
-                       fmt.Fprintf(w, "decl: %v, vals: %s, %s",
-                               decl,
-                               r.Trailer.Get("Client-Trailer-A"),
-                               r.Trailer.Get("Client-Trailer-B"))
-               }
-
-               // How handlers set Trailers: declare it ahead of time
-               // with the Trailer header, and then mutate the
-               // Header() of those values later, after the response
-               // has been written (we wrote to w above).
-               w.Header().Set("Server-Trailer-A", "valuea")
-               w.Header().Set("Server-Trailer-C", "valuec") // skipping B
-       }))
-       defer cst.close()
-
-       var req *Request
-       req, _ = NewRequest("POST", cst.ts.URL, io.MultiReader(
-               eofReaderFunc(func() {
-                       req.Trailer["Client-Trailer-A"] = []string{"valuea"}
-               }),
-               strings.NewReader("foo"),
-               eofReaderFunc(func() {
-                       req.Trailer["Client-Trailer-B"] = []string{"valueb"}
-               }),
-       ))
-       req.Trailer = Header{
-               "Client-Trailer-A": nil, //  to be set later
-               "Client-Trailer-B": nil, //  to be set later
-       }
-       req.ContentLength = -1
-       res, err := cst.c.Do(req)
-       if err != nil {
-               t.Fatal(err)
-       }
-       if err := wantBody(res, err, "decl: [Client-Trailer-A Client-Trailer-B], vals: valuea, valueb"); err != nil {
-               t.Error(err)
-       }
-       want := Header{
-               "Server-Trailer-A": []string{"valuea"},
-               "Server-Trailer-B": nil,
-               "Server-Trailer-C": []string{"valuec"},
-       }
-       if !reflect.DeepEqual(res.Trailer, want) {
-               t.Errorf("Response trailers = %#v; want %#v", res.Trailer, want)
-       }
-}
-
 func TestReferer(t *testing.T) {
        tests := []struct {
                lastReq, newReq string // from -> to URLs

diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go
index e54091f3b8118225f514200429280bd968171996..09dbceb99d4d677fc16c883092d3443d87a94f02 100644 (file)
--- a/src/net/http/clientserver_test.go
+++ b/src/net/http/clientserver_test.go
@@ -18,6 +18,7 @@ import (
        "net/http/httptest"
        "os"
        "reflect"
+       "sort"
        "strings"
        "sync"
        "testing"
@@ -465,3 +466,128 @@ func testCancelRequestMidBody(t *testing.T, h2 bool) {
                t.Errorf("ReadAll error = %v; want %v", err, ExportErrRequestCanceled)
        }
 }
+
+// Tests that clients can send trailers to a server and that the server can read them.
+func TestTrailersClientToServer_h1(t *testing.T) { testTrailersClientToServer(t, h1Mode) }
+func TestTrailersClientToServer_h2(t *testing.T) {
+       t.Skip("skipping in http2 mode; golang.org/issue/13557")
+       testTrailersClientToServer(t, h2Mode)
+}
+
+func testTrailersClientToServer(t *testing.T, h2 bool) {
+       defer afterTest(t)
+       cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+               var decl []string
+               for k := range r.Trailer {
+                       decl = append(decl, k)
+               }
+               sort.Strings(decl)
+
+               slurp, err := ioutil.ReadAll(r.Body)
+               if err != nil {
+                       t.Errorf("Server reading request body: %v", err)
+               }
+               if string(slurp) != "foo" {
+                       t.Errorf("Server read request body %q; want foo", slurp)
+               }
+               if r.Trailer == nil {
+                       io.WriteString(w, "nil Trailer")
+               } else {
+                       fmt.Fprintf(w, "decl: %v, vals: %s, %s",
+                               decl,
+                               r.Trailer.Get("Client-Trailer-A"),
+                               r.Trailer.Get("Client-Trailer-B"))
+               }
+       }))
+       defer cst.close()
+
+       var req *Request
+       req, _ = NewRequest("POST", cst.ts.URL, io.MultiReader(
+               eofReaderFunc(func() {
+                       req.Trailer["Client-Trailer-A"] = []string{"valuea"}
+               }),
+               strings.NewReader("foo"),
+               eofReaderFunc(func() {
+                       req.Trailer["Client-Trailer-B"] = []string{"valueb"}
+               }),
+       ))
+       req.Trailer = Header{
+               "Client-Trailer-A": nil, //  to be set later
+               "Client-Trailer-B": nil, //  to be set later
+       }
+       req.ContentLength = -1
+       res, err := cst.c.Do(req)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if err := wantBody(res, err, "decl: [Client-Trailer-A Client-Trailer-B], vals: valuea, valueb"); err != nil {
+               t.Error(err)
+       }
+}
+
+// Tests that servers send trailers to a client and that the client can read them.
+func TestTrailersServerToClient_h1(t *testing.T) { testTrailersServerToClient(t, h1Mode, false) }
+func TestTrailersServerToClient_h2(t *testing.T) {
+       t.Skip("skipping in http2 mode; golang.org/issue/13557")
+       testTrailersServerToClient(t, h2Mode, false)
+}
+func TestTrailersServerToClient_Flush_h1(t *testing.T) { testTrailersServerToClient(t, h1Mode, true) }
+func TestTrailersServerToClient_Flush_h2(t *testing.T) {
+       t.Skip("skipping in http2 mode; golang.org/issue/13557")
+       testTrailersServerToClient(t, h2Mode, true)
+}
+
+func testTrailersServerToClient(t *testing.T, h2, flush bool) {
+       defer afterTest(t)
+       const body = "Some body"
+       cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+               w.Header().Set("Trailer", "Server-Trailer-A, Server-Trailer-B")
+               w.Header().Add("Trailer", "Server-Trailer-C")
+
+               io.WriteString(w, body)
+               if flush {
+                       w.(Flusher).Flush()
+               }
+
+               // How handlers set Trailers: declare it ahead of time
+               // with the Trailer header, and then mutate the
+               // Header() of those values later, after the response
+               // has been written (we wrote to w above).
+               w.Header().Set("Server-Trailer-A", "valuea")
+               w.Header().Set("Server-Trailer-C", "valuec") // skipping B
+               w.Header().Set("Server-Trailer-NotDeclared", "should be omitted")
+       }))
+       defer cst.close()
+
+       res, err := cst.c.Get(cst.ts.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       delete(res.Header, "Date") // irrelevant for test
+       if got, want := res.Header, (Header{
+               "Content-Type": {"text/plain; charset=utf-8"},
+       }); !reflect.DeepEqual(got, want) {
+               t.Errorf("Header = %v; want %v", got, want)
+       }
+
+       if got, want := res.Trailer, (Header{
+               "Server-Trailer-A": nil,
+               "Server-Trailer-B": nil,
+               "Server-Trailer-C": nil,
+       }); !reflect.DeepEqual(got, want) {
+               t.Errorf("Trailer before body read = %v; want %v", got, want)
+       }
+
+       if err := wantBody(res, nil, body); err != nil {
+               t.Fatal(err)
+       }
+
+       if got, want := res.Trailer, (Header{
+               "Server-Trailer-A": {"valuea"},
+               "Server-Trailer-B": nil,
+               "Server-Trailer-C": {"valuec"},
+       }); !reflect.DeepEqual(got, want) {
+               t.Errorf("Trailer after body read = %v; want %v", got, want)
+       }
+}

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index 38b6f67c42860cc5ab9768770d7b36e7db383a86..b452f33ad6caefc6aa3057381ddd3244fd6cd93e 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -577,21 +577,29 @@ func shouldClose(major, minor int, header Header, removeCloseHeader bool) bool {
 
 // Parse the trailer header
 func fixTrailer(header Header, te []string) (Header, error) {
-       raw := header.get("Trailer")
-       if raw == "" {
+       vv, ok := header["Trailer"]
+       if !ok {
                return nil, nil
        }
-
        header.Del("Trailer")
+
        trailer := make(Header)
-       keys := strings.Split(raw, ",")
-       for _, key := range keys {
-               key = CanonicalHeaderKey(strings.TrimSpace(key))
-               switch key {
-               case "Transfer-Encoding", "Trailer", "Content-Length":
-                       return nil, &badStringError{"bad trailer key", key}
-               }
-               trailer[key] = nil
+       var err error
+       for _, v := range vv {
+               foreachHeaderElement(v, func(key string) {
+                       key = CanonicalHeaderKey(key)
+                       switch key {
+                       case "Transfer-Encoding", "Trailer", "Content-Length":
+                               if err == nil {
+                                       err = &badStringError{"bad trailer key", key}
+                                       return
+                               }
+                       }
+                       trailer[key] = nil
+               })
+       }
+       if err != nil {
+               return nil, err
        }
        if len(trailer) == 0 {
                return nil, nil