Repository: quic-go/quic-go
Pull request #4645 (2024-08-27 12:59:46)

http3: set ContentLength to -1 if no Content-Length header is set
This applies to both the `http.Response` and the `http.Request`.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4645?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 84.87%. Comparing base [(`920bfb4`)](https://app.codecov.io/gh/quic-go/quic-go/commit/920bfb46af9db2ab23a94e54701b7a310ecc4928?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`0f5e90f`)](https://app.codecov.io/gh/quic-go/quic-go/commit/0f5e90fa73e23294ecca33f67dadae4925cf2c65?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4645      +/-   ##
==========================================
- Coverage   84.91%   84.87%   -0.04%     
==========================================
  Files         150      147       -3     
  Lines       14868    14587     -281     
==========================================
- Hits        12624    12380     -244     
+ Misses       1718     1701      -17     
+ Partials      526      506      -20     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4645?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

marten-seemann: Added some more tests. This should be ready to merge once CI passes.

Patch:
diff --git a/http3/headers.go b/http3/headers.go
index 7c67342e..3f17f222 100644
--- a/http3/headers.go
+++ b/http3/headers.go
@@ -22,7 +22,7 @@ type header struct {
 	Status    string
 	// for Extended connect
 	Protocol string
-	// parsed and deduplicated
+	// parsed and deduplicated. -1 if no Content-Length header is sent
 	ContentLength int64
 	// all non-pseudo headers
 	Headers http.Header
@@ -91,6 +91,7 @@ func parseHeaders(headers []qpack.HeaderField, isRequest bool) (header, error) {
 			}
 		}
 	}
+	hdr.ContentLength = -1
 	if len(contentLengthStr) > 0 {
 		// use ParseUint instead of ParseInt, so that parsing fails on negative values
 		cl, err := strconv.ParseUint(contentLengthStr, 10, 63)
diff --git a/http3/http_stream.go b/http3/http_stream.go
index af46226b..47add370 100644
--- a/http3/http_stream.go
+++ b/http3/http_stream.go
@@ -238,11 +238,7 @@ func (s *requestStream) ReadResponse() (*http.Response, error) {
 
 	// Check that the server doesn't send more data in DATA frames than indicated by the Content-Length header (if set).
 	// See section 4.1.2 of RFC 9114.
-	contentLength := int64(-1)
-	if _, ok := res.Header["Content-Length"]; ok && res.ContentLength >= 0 {
-		contentLength = res.ContentLength
-	}
-	respBody := newResponseBody(s.stream, contentLength, s.reqDone)
+	respBody := newResponseBody(s.stream, res.ContentLength, s.reqDone)
 
 	// Rules for when to set Content-Length are defined in https://tools.ietf.org/html/rfc7230#section-3.3.2.
 	isInformational := res.StatusCode >= 100 && res.StatusCode < 200


Test patch:
diff --git a/http3/headers_test.go b/http3/headers_test.go
index e9bf7a76..5bc41e31 100644
--- a/http3/headers_test.go
+++ b/http3/headers_test.go
@@ -33,6 +33,17 @@ var _ = Describe("Request", func() {
 		Expect(req.RequestURI).To(Equal("/foo"))
 	})
 
+	It("sets the ContentLength to -1", func() {
+		headers := []qpack.HeaderField{
+			{Name: ":path", Value: "/foo"},
+			{Name: ":authority", Value: "quic.clemente.io"},
+			{Name: ":method", Value: "GET"},
+		}
+		req, err := requestFromHeaders(headers)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(req.ContentLength).To(BeEquivalentTo(-1))
+	})
+
 	It("rejects upper-case fields", func() {
 		headers := []qpack.HeaderField{
 			{Name: ":path", Value: "/foo"},
@@ -343,7 +354,6 @@ var _ = Describe("Response", func() {
 	It("rejects invalid status codes", func() {
 		headers := []qpack.HeaderField{
 			{Name: ":status", Value: "foobar"},
-			{Name: "content-length", Value: "42"},
 		}
 		err := updateResponseFromHeaders(&http.Response{}, headers)
 		Expect(err).To(HaveOccurred())
diff --git a/integrationtests/self/http_test.go b/integrationtests/self/http_test.go
index cd97896b..cf978685 100644
--- a/integrationtests/self/http_test.go
+++ b/integrationtests/self/http_test.go
@@ -289,6 +289,7 @@ var _ = Describe("HTTP tests", func() {
 		Expect(resp.StatusCode).To(Equal(200))
 		body, err := io.ReadAll(gbytes.TimeoutReader(resp.Body, 20*time.Second))
 		Expect(err).ToNot(HaveOccurred())
+		Expect(resp.ContentLength).To(BeEquivalentTo(-1))
 		Expect(body).To(Equal(PRDataLong))
 	})