git » go-net » commit 7394c11

diff --git a/http2/transport.go b/http2/transport.go
index 3cefc22..ccb885f 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -622,15 +622,18 @@ func bodyAndLength(req *http.Request) (body io.Reader, contentLen int64) {
 	// We have a body but a zero content length. Test to see if
 	// it's actually zero or just unset.
 	var buf [1]byte
-	n, rerr := io.ReadFull(body, buf[:])
+	n, rerr := body.Read(buf[:])
 	if rerr != nil && rerr != io.EOF {
 		return errorReader{rerr}, -1
 	}
 	if n == 1 {
 		// Oh, guess there is data in this Body Reader after all.
 		// The ContentLength field just wasn't set.
-		// Stich the Body back together again, re-attaching our
+		// Stitch the Body back together again, re-attaching our
 		// consumed byte.
+		if rerr == io.EOF {
+			return bytes.NewReader(buf[:]), 1
+		}
 		return io.MultiReader(bytes.NewReader(buf[:]), body), -1
 	}
 	// Body is actually zero bytes.
@@ -901,10 +904,11 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (
 			err = cc.fr.WriteData(cs.ID, sentEnd, data)
 			if err == nil {
 				// TODO(bradfitz): this flush is for latency, not bandwidth.
-				// Most requests won't need this. Make this opt-in or opt-out?
-				// Use some heuristic on the body type? Nagel-like timers?
-				// Based on 'n'? Only last chunk of this for loop, unless flow control
-				// tokens are low? For now, always:
+				// Most requests won't need this. Make this opt-in or
+				// opt-out?  Use some heuristic on the body type? Nagel-like
+				// timers?  Based on 'n'? Only last chunk of this for loop,
+				// unless flow control tokens are low? For now, always.
+				// If we change this, see comment below.
 				err = cc.bw.Flush()
 			}
 			cc.wmu.Unlock()
@@ -914,8 +918,15 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (
 		}
 	}
 
+	if sentEnd {
+		// Already sent END_STREAM (which implies we have no
+		// trailers) and flushed, because currently all
+		// WriteData frames above get a flush. So we're done.
+		return nil
+	}
+
 	var trls []byte
-	if !sentEnd && hasTrailers {
+	if hasTrailers {
 		cc.mu.Lock()
 		defer cc.mu.Unlock()
 		trls = cc.encodeTrailers(req)
@@ -924,8 +935,8 @@ func (cs *clientStream) writeRequestBody(body io.Reader, bodyCloser io.Closer) (
 	cc.wmu.Lock()
 	defer cc.wmu.Unlock()
 
-	// Avoid forgetting to send an END_STREAM if the encoded
-	// trailers are 0 bytes. Both results produce and END_STREAM.
+	// Two ways to send END_STREAM: either with trailers, or
+	// with an empty DATA frame.
 	if len(trls) > 0 {
 		err = cc.writeHeaders(cs.ID, true, trls)
 	} else {
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 @@ func TestTransportReturnsErrorOnBadResponseHeaders(t *testing.T) {
 	}
 	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()
+	}
+}