http2: fix protocol violation regression when writing certain request bodies
The mod_h2 workaround CL (git rev 28d1bd4,
https://golang.org/cl/25362) introduced a regression where the
Transport could write two DATA frames, both with END_STREAM, if the
Request.Body returned (non-0, io.EOF). strings.Reader, bytes.Reader
are the most common Reader types used with HTTP requests and they only
return (0, io.EOF) so we got generally lucky and things seemed to
work, but other Readers do return (non-0, io.EOF), like the HTTP
transport/server Readers. This is why we broke the HTTP proxy code,
when proxying to HTTP/2.
Updates golang/go#16788 (fixes after it's bundled into std)
Change-Id: I42684017039eacfc27ee53e9c11431f713fdaca4
Reviewed-on: https://go-review.googlesource.com/27406
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 9ab4149..1bc5c4b 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -2427,3 +2427,44 @@
}
ct.run()
}
+
+// byteAndEOFReader returns is in an io.Reader which reads one byte
+// (the underlying byte) and io.EOF at once in its Read call.
+type byteAndEOFReader byte
+
+func (b byteAndEOFReader) Read(p []byte) (n int, err error) {
+ if len(p) == 0 {
+ panic("unexpected useless call")
+ }
+ p[0] = byte(b)
+ return 1, io.EOF
+}
+
+// Issue 16788: the Transport had a regression where it started
+// sending a spurious DATA frame with a duplicate END_STREAM bit after
+// the request body writer goroutine had already read an EOF from the
+// Request.Body and included the END_STREAM on a data-carrying DATA
+// frame.
+//
+// Notably, to trigger this, the requests need to use a Request.Body
+// which returns (non-0, io.EOF) and also needs to set the ContentLength
+// explicitly.
+func TestTransportBodyDoubleEndStream(t *testing.T) {
+ st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
+ // Nothing.
+ }, optOnlyServer)
+ defer st.Close()
+
+ tr := &Transport{TLSClientConfig: tlsConfigInsecure}
+ defer tr.CloseIdleConnections()
+
+ for i := 0; i < 2; i++ {
+ req, _ := http.NewRequest("POST", st.ts.URL, byteAndEOFReader('a'))
+ req.ContentLength = 1
+ res, err := tr.RoundTrip(req)
+ if err != nil {
+ t.Fatalf("failure on req %d: %v", i+1, err)
+ }
+ defer res.Body.Close()
+ }
+}
