net/http: run more tests in http2 mode

Failing ones are marked skipped.

Fixes #13543 (was just a test issue)
Updates #13555 (to be fixed later)
Updates #13556 (to be fixed later)
Updates #13557 (to be fixed later)
Fixes bug in golang.org/cl/17428 (http1 now uses HTTP status 431, not 413)

Change-Id: I8f7431fee35f2fc081cfe2c232ae75a00800a60b
Reviewed-on: https://go-review.googlesource.com/17683Reviewed-by: Blake Mizerany <blake.mizerany@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Burcu Dogan <jbd@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/http/client_test.go

@@ -83,8 +83,8 @@ func TestClient(t *testing.T) {
 	}
 }
 
-func TestClientHead_h1(t *testing.T) { testClientHead(t, false) }
-func TestClientHead_h2(t *testing.T) { testClientHead(t, true) }
+func TestClientHead_h1(t *testing.T) { testClientHead(t, h1Mode) }
+func TestClientHead_h2(t *testing.T) { testClientHead(t, h2Mode) }
 
 func testClientHead(t *testing.T, h2 bool) {
 	defer afterTest(t)
@@ -496,8 +496,8 @@ func (j *RecordingJar) logf(format string, args ...interface{}) {
 	fmt.Fprintf(&j.log, format, args...)
 }
 
-func TestStreamingGet_h1(t *testing.T) { testStreamingGet(t, false) }
-func TestStreamingGet_h2(t *testing.T) { testStreamingGet(t, true) }
+func TestStreamingGet_h1(t *testing.T) { testStreamingGet(t, h1Mode) }
+func TestStreamingGet_h2(t *testing.T) { testStreamingGet(t, h2Mode) }
 
 func testStreamingGet(t *testing.T, h2 bool) {
 	defer afterTest(t)
@@ -772,11 +772,11 @@ func TestHTTPSClientDetectsHTTPServer(t *testing.T) {
 
 // Verify Response.ContentLength is populated. https://golang.org/issue/4126
 func TestClientHeadContentLength_h1(t *testing.T) {
-	testClientHeadContentLength(t, false)
+	testClientHeadContentLength(t, h1Mode)
 }
 
 func TestClientHeadContentLength_h2(t *testing.T) {
-	testClientHeadContentLength(t, true)
+	testClientHeadContentLength(t, h2Mode)
 }
 
 func testClientHeadContentLength(t *testing.T, h2 bool) {
@@ -1037,14 +1037,8 @@ func TestClientTimeout_Headers(t *testing.T) {
 	}
 }
 
-func TestClientRedirectEatsBody_h1(t *testing.T) {
-	testClientRedirectEatsBody(t, false)
-}
-
-func TestClientRedirectEatsBody_h2(t *testing.T) {
-	testClientRedirectEatsBody(t, true)
-}
-
+func TestClientRedirectEatsBody_h1(t *testing.T) { testClientRedirectEatsBody(t, h1Mode) }
+func TestClientRedirectEatsBody_h2(t *testing.T) { testClientRedirectEatsBody(t, h2Mode) }
 func testClientRedirectEatsBody(t *testing.T, h2 bool) {
 	defer afterTest(t)
 	saw := make(chan string, 2)
@@ -1093,9 +1087,14 @@ func (f eofReaderFunc) Read(p []byte) (n int, err error) {
 	return 0, io.EOF
 }
 
-func TestClientTrailers(t *testing.T) {
+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)
-	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+	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")
@@ -1129,10 +1128,10 @@ func TestClientTrailers(t *testing.T) {
 		w.Header().Set("Server-Trailer-A", "valuea")
 		w.Header().Set("Server-Trailer-C", "valuec") // skipping B
 	}))
-	defer ts.Close()
+	defer cst.close()
 
 	var req *Request
-	req, _ = NewRequest("POST", ts.URL, io.MultiReader(
+	req, _ = NewRequest("POST", cst.ts.URL, io.MultiReader(
 		eofReaderFunc(func() {
 			req.Trailer["Client-Trailer-A"] = []string{"valuea"}
 		}),
@@ -1146,7 +1145,7 @@ func TestClientTrailers(t *testing.T) {
 		"Client-Trailer-B": nil, //  to be set later
 	}
 	req.ContentLength = -1
-	res, err := DefaultClient.Do(req)
+	res, err := cst.c.Do(req)
 	if err != nil {
 		t.Fatal(err)
 	}

src/net/http/clientserver_test.go

@@ -37,6 +37,11 @@ func (t *clientServerTest) close() {
 	t.ts.Close()
 }
 
+const (
+	h1Mode = false
+	h2Mode = true
+)
+
 func newClientServerTest(t *testing.T, h2 bool, h Handler) *clientServerTest {
 	cst := &clientServerTest{
 		t:  t,
@@ -87,8 +92,8 @@ func TestNewClientServerTest(t *testing.T) {
 	}
 }
 
-func TestChunkedResponseHeaders_h1(t *testing.T) { testChunkedResponseHeaders(t, false) }
-func TestChunkedResponseHeaders_h2(t *testing.T) { testChunkedResponseHeaders(t, true) }
+func TestChunkedResponseHeaders_h1(t *testing.T) { testChunkedResponseHeaders(t, h1Mode) }
+func TestChunkedResponseHeaders_h2(t *testing.T) { testChunkedResponseHeaders(t, h2Mode) }
 
 func testChunkedResponseHeaders(t *testing.T, h2 bool) {
 	defer afterTest(t)
@@ -354,8 +359,6 @@ func TestH12_HandlerWritesTooMuch(t *testing.T) {
 	}.run(t)
 }
 
-// TODO: TestH12_Trailers
-
 // Verify that both our HTTP/1 and HTTP/2 request and auto-decompress gzip.
 // Some hosts send gzip even if you don't ask for it; see golang.org/issue/13298
 func TestH12_AutoGzip(t *testing.T) {
@@ -375,8 +378,8 @@ func TestH12_AutoGzip(t *testing.T) {
 // Test304Responses verifies that 304s don't declare that they're
 // chunking in their response headers and aren't allowed to produce
 // output.
-func Test304Responses_h1(t *testing.T) { test304Responses(t, false) }
-func Test304Responses_h2(t *testing.T) { test304Responses(t, true) }
+func Test304Responses_h1(t *testing.T) { test304Responses(t, h1Mode) }
+func Test304Responses_h2(t *testing.T) { test304Responses(t, h2Mode) }
 
 func test304Responses(t *testing.T, h2 bool) {
 	defer afterTest(t)

src/net/http/fs_test.go

@@ -477,14 +477,27 @@ func TestServeFileFromCWD(t *testing.T) {
 	}
 }
 
-func TestServeFileWithContentEncoding(t *testing.T) {
+// Tests that ServeFile doesn't add a Content-Length if a Content-Encoding is
+// specified.
+func TestServeFileWithContentEncoding_h1(t *testing.T) { testServeFileWithContentEncoding(t, h1Mode) }
+func TestServeFileWithContentEncoding_h2(t *testing.T) { testServeFileWithContentEncoding(t, h2Mode) }
+func testServeFileWithContentEncoding(t *testing.T, h2 bool) {
 	defer afterTest(t)
-	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
 		w.Header().Set("Content-Encoding", "foo")
 		ServeFile(w, r, "testdata/file")
+
+		// Because the testdata is so small, it would fit in
+		// both the h1 and h2 Server's write buffers. For h1,
+		// sendfile is used, though, forcing a header flush at
+		// the io.Copy. http2 doesn't do a header flush so
+		// buffers all 11 bytes and then adds its own
+		// Content-Length. To prevent the Server's
+		// Content-Length and test ServeFile only, flush here.
+		w.(Flusher).Flush()
 	}))
-	defer ts.Close()
-	resp, err := Get(ts.URL)
+	defer cst.close()
+	resp, err := cst.c.Get(cst.ts.URL)
 	if err != nil {
 		t.Fatal(err)
 	}

src/net/http/request_test.go

@@ -176,9 +176,8 @@ func TestParseMultipartForm(t *testing.T) {
 	}
 }
 
-func TestRedirect_h1(t *testing.T) { testRedirect(t, false) }
-func TestRedirect_h2(t *testing.T) { testRedirect(t, true) }
-
+func TestRedirect_h1(t *testing.T) { testRedirect(t, h1Mode) }
+func TestRedirect_h2(t *testing.T) { testRedirect(t, h2Mode) }
 func testRedirect(t *testing.T, h2 bool) {
 	defer afterTest(t)
 	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {

src/net/http/server.go

@@ -1341,7 +1341,7 @@ func (c *conn) serve() {
 				// responding to them and hanging up
 				// while they're still writing their
 				// request.  Undefined behavior.
-				io.WriteString(c.rwc, "HTTP/1.1 413 Request Entity Too Large\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\n413 Request Entity Too Large")
+				io.WriteString(c.rwc, "HTTP/1.1 431 Request Header Fields Too Large\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\n431 Request Header Fields Too Large")
 				c.closeWriteAndWait()
 				break
 			} else if err == io.EOF {

src/net/http/sniff_test.go

@@ -51,8 +51,8 @@ func TestDetectContentType(t *testing.T) {
 	}
 }
 
-func TestServerContentType_h1(t *testing.T) { testServerContentType(t, false) }
-func TestServerContentType_h2(t *testing.T) { testServerContentType(t, true) }
+func TestServerContentType_h1(t *testing.T) { testServerContentType(t, h1Mode) }
+func TestServerContentType_h2(t *testing.T) { testServerContentType(t, h2Mode) }
 
 func testServerContentType(t *testing.T, h2 bool) {
 	defer afterTest(t)
@@ -87,9 +87,8 @@ func testServerContentType(t *testing.T, h2 bool) {
 
 // Issue 5953: shouldn't sniff if the handler set a Content-Type header,
 // even if it's the empty string.
-func TestServerIssue5953_h1(t *testing.T) { testServerIssue5953(t, false) }
-func TestServerIssue5953_h2(t *testing.T) { testServerIssue5953(t, true) }
-
+func TestServerIssue5953_h1(t *testing.T) { testServerIssue5953(t, h1Mode) }
+func TestServerIssue5953_h2(t *testing.T) { testServerIssue5953(t, h2Mode) }
 func testServerIssue5953(t *testing.T, h2 bool) {
 	defer afterTest(t)
 	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
@@ -111,9 +110,8 @@ func testServerIssue5953(t *testing.T, h2 bool) {
 	resp.Body.Close()
 }
 
-func TestContentTypeWithCopy_h1(t *testing.T) { testContentTypeWithCopy(t, false) }
-func TestContentTypeWithCopy_h2(t *testing.T) { testContentTypeWithCopy(t, true) }
-
+func TestContentTypeWithCopy_h1(t *testing.T) { testContentTypeWithCopy(t, h1Mode) }
+func TestContentTypeWithCopy_h2(t *testing.T) { testContentTypeWithCopy(t, h2Mode) }
 func testContentTypeWithCopy(t *testing.T, h2 bool) {
 	defer afterTest(t)
 
@@ -148,9 +146,8 @@ func testContentTypeWithCopy(t *testing.T, h2 bool) {
 	resp.Body.Close()
 }
 
-func TestSniffWriteSize_h1(t *testing.T) { testSniffWriteSize(t, false) }
-func TestSniffWriteSize_h2(t *testing.T) { testSniffWriteSize(t, true) }
-
+func TestSniffWriteSize_h1(t *testing.T) { testSniffWriteSize(t, h1Mode) }
+func TestSniffWriteSize_h2(t *testing.T) { testSniffWriteSize(t, h2Mode) }
 func testSniffWriteSize(t *testing.T, h2 bool) {
 	defer afterTest(t)
 	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {