http2: make Transport treat http.NoBody like it were nil

Updates golang/go#18891

Change-Id: I866862d805dc1757b27817ddb30cf22dc48ac3ff
Reviewed-on: https://go-review.googlesource.com/45993
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

http2/go18.go

@@ -52,3 +52,5 @@ func reqGetBody(req *http.Request) func() (io.ReadCloser, error) {
 func reqBodyIsNoBody(body io.ReadCloser) bool {
 	return body == http.NoBody
 }
+
+func go18httpNoBody() io.ReadCloser { return http.NoBody } // for tests only

http2/not_go18.go

@@ -25,3 +25,5 @@ func reqGetBody(req *http.Request) func() (io.ReadCloser, error) {
 }
 
 func reqBodyIsNoBody(io.ReadCloser) bool { return false }
+
+func go18httpNoBody() io.ReadCloser { return nil } // for tests only

http2/transport.go

@@ -694,7 +694,7 @@ func checkConnHeaders(req *http.Request) error {
 // req.ContentLength, where 0 actually means zero (not unknown) and -1
 // means unknown.
 func actualContentLength(req *http.Request) int64 {
-	if req.Body == nil {
+	if req.Body == nil || reqBodyIsNoBody(req.Body) {
 		return 0
 	}
 	if req.ContentLength != 0 {
@@ -725,8 +725,8 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
 	}
 
 	body := req.Body
-	hasBody := body != nil
 	contentLen := actualContentLength(req)
+	hasBody := contentLen != 0
 
 	// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
 	var requestedGzip bool

http2/transport_test.go

@@ -417,6 +417,11 @@ func TestActualContentLength(t *testing.T) {
 			req:  &http.Request{Body: panicReader{}, ContentLength: 5},
 			want: 5,
 		},
+		// http.NoBody means 0, not -1.
+		3: {
+			req:  &http.Request{Body: go18httpNoBody()},
+			want: 0,
+		},
 	}
 	for i, tt := range tests {
 		got := actualContentLength(tt.req)
@@ -2969,3 +2974,42 @@ func TestTransportAllocationsAfterResponseBodyClose(t *testing.T) {
 		t.Errorf("Handler Write err = %v; want errStreamClosed", gotErr)
 	}
 }
+
+// Issue 18891: make sure Request.Body == NoBody means no DATA frame
+// is ever sent, even if empty.
+func TestTransportNoBodyMeansNoDATA(t *testing.T) {
+	ct := newClientTester(t)
+
+	unblockClient := make(chan bool)
+
+	ct.client = func() error {
+		req, _ := http.NewRequest("GET", "https://dummy.tld/", go18httpNoBody())
+		ct.tr.RoundTrip(req)
+		<-unblockClient
+		return nil
+	}
+	ct.server = func() error {
+		defer close(unblockClient)
+		defer ct.cc.(*net.TCPConn).Close()
+		ct.greet()
+
+		for {
+			f, err := ct.fr.ReadFrame()
+			if err != nil {
+				return fmt.Errorf("ReadFrame while waiting for Headers: %v", err)
+			}
+			switch f := f.(type) {
+			default:
+				return fmt.Errorf("Got %T; want HeadersFrame", f)
+			case *WindowUpdateFrame, *SettingsFrame:
+				continue
+			case *HeadersFrame:
+				if !f.StreamEnded() {
+					return fmt.Errorf("got headers frame without END_STREAM")
+				}
+				return nil
+			}
+		}
+	}
+	ct.run()
+}