http2: return unexpected eof on empty response with non-zero content …

…length Fixes golang/go#46071 Change-Id: I8b8262ba8e28a129f6aacfca43b7b8d9e605a0ce GitHub-Last-Rev: 11b92cc8ba6e1d08716aac816d33b659563a893f GitHub-Pull-Request: golang/net#114 Reviewed-on: https://go-review.googlesource.com/c/net/+/354141 Reviewed-by: Damien Neil <[email protected]> Trust: Damien Neil <[email protected]> Trust: Alexander Rakoczy <[email protected]> Run-TryBot: Alexander Rakoczy <[email protected]> TryBot-Result: Go Bot <[email protected]>
dteh · Oct 13, 2021 · f78f23f · f78f23f
1 parent e52279c
commit f78f23f
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 13 deletions.
diff --git a/transport.go b/transport.go
@@ -2217,28 +2217,33 @@ func (rl *clientConnReadLoop) handleResponse(cs *clientStream, f *MetaHeadersFra
 		return nil, nil
 	}
 
-	streamEnded := f.StreamEnded()
-	isHead := cs.req.Method == "HEAD"
-	if !streamEnded || isHead {
-		res.ContentLength = -1
-		if clens := res.Header["Content-Length"]; len(clens) == 1 {
-			if cl, err := strconv.ParseUint(clens[0], 10, 63); err == nil {
-				res.ContentLength = int64(cl)
-			} else {
-				// TODO: care? unlike http/1, it won't mess up our framing, so it's
-				// more safe smuggling-wise to ignore.
-			}
-		} else if len(clens) > 1 {
+	res.ContentLength = -1
+	if clens := res.Header["Content-Length"]; len(clens) == 1 {
+		if cl, err := strconv.ParseUint(clens[0], 10, 63); err == nil {
+			res.ContentLength = int64(cl)
+		} else {
 			// TODO: care? unlike http/1, it won't mess up our framing, so it's
 			// more safe smuggling-wise to ignore.
 		}
+	} else if len(clens) > 1 {
+		// TODO: care? unlike http/1, it won't mess up our framing, so it's
+		// more safe smuggling-wise to ignore.
 	}
 
-	if streamEnded || isHead {
+	if cs.req.Method == "HEAD" {
 		res.Body = noBody
 		return res, nil
 	}
 
+	if f.StreamEnded() {
+		if res.ContentLength > 0 {
+			res.Body = missingBody{}
+		} else {
+			res.Body = noBody
+		}
+		return res, nil
+	}
+
 	cs.bufPipe.setBuffer(&dataBuffer{expected: res.ContentLength})
 	cs.bytesRemain = res.ContentLength
 	res.Body = transportResponseBody{cs}
@@ -2786,6 +2791,11 @@ func (t *Transport) logf(format string, args ...interface{}) {
 
 var noBody io.ReadCloser = ioutil.NopCloser(bytes.NewReader(nil))
 
+type missingBody struct{}
+
+func (missingBody) Close() error             { return nil }
+func (missingBody) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF }
+
 func strSliceContains(ss []string, s string) bool {
 	for _, v := range ss {
 		if v == s {

diff --git a/transport_test.go b/transport_test.go
@@ -5623,3 +5623,56 @@ func TestTransportTimeoutServerHangs(t *testing.T) {
 	}
 	ct.run()
 }
+
+func TestTransportContentLengthWithoutBody(t *testing.T) {
+	contentLength := ""
+	st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Length", contentLength)
+	}, optOnlyServer)
+	defer st.Close()
+	tr := &Transport{TLSClientConfig: tlsConfigInsecure}
+	defer tr.CloseIdleConnections()
+
+	for _, test := range []struct {
+		name              string
+		contentLength     string
+		wantBody          string
+		wantErr           error
+		wantContentLength int64
+	}{
+		{
+			name:              "non-zero content length",
+			contentLength:     "42",
+			wantErr:           io.ErrUnexpectedEOF,
+			wantContentLength: 42,
+		},
+		{
+			name:              "zero content length",
+			contentLength:     "0",
+			wantErr:           nil,
+			wantContentLength: 0,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			contentLength = test.contentLength
+
+			req, _ := http.NewRequest("GET", st.ts.URL, nil)
+			res, err := tr.RoundTrip(req)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer res.Body.Close()
+			body, err := io.ReadAll(res.Body)
+
+			if err != test.wantErr {
+				t.Errorf("Expected error %v, got: %v", test.wantErr, err)
+			}
+			if len(body) > 0 {
+				t.Errorf("Expected empty body, got: %v", body)
+			}
+			if res.ContentLength != test.wantContentLength {
+				t.Errorf("Expected content length %d, got: %d", test.wantContentLength, res.ContentLength)
+			}
+		})
+	}
+}