Repository: quic-go/quic-go
Pull request #4639 (2024-08-23 05:38:58)

http3: reject pseudo header fields in trailers
@sudorandom Would you mind reviewing this PR?

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4639?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.84%. Comparing base [(`17fb3b9`)](https://app.codecov.io/gh/quic-go/quic-go/commit/17fb3b96ba07f820519560a00a82369f8ef6a368?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`8827dc8`)](https://app.codecov.io/gh/quic-go/quic-go/commit/8827dc8e9eb96c33fb0dc93b12eef325b386e503?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    #4639      +/-   ##
==========================================
- Coverage   84.93%   84.84%   -0.09%     
==========================================
  Files         150      147       -3     
  Lines       14864    14590     -274     
==========================================
- Hits        12624    12378     -246     
+ Misses       1715     1704      -11     
+ Partials      525      508      -17     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4639?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).


Patch:
diff --git a/http3/conn.go b/http3/conn.go
index 5c12b704..0f372b0d 100644
--- a/http3/conn.go
+++ b/http3/conn.go
@@ -116,7 +116,7 @@ func (c *connection) openRequestStream(
 	qstr := newStateTrackingStream(str, c, datagrams)
 	rsp := &http.Response{}
 	hstr := newStream(qstr, c, datagrams, func(r io.Reader, l uint64) error {
-		hdr, err := c.parseTrailer(r, l, maxHeaderBytes)
+		hdr, err := c.decodeTrailers(r, l, maxHeaderBytes)
 		if err != nil {
 			return err
 		}
@@ -126,7 +126,7 @@ func (c *connection) openRequestStream(
 	return newRequestStream(hstr, requestWriter, reqDone, c.decoder, disableCompression, maxHeaderBytes, rsp), nil
 }
 
-func (c *connection) parseTrailer(r io.Reader, l, maxHeaderBytes uint64) (http.Header, error) {
+func (c *connection) decodeTrailers(r io.Reader, l, maxHeaderBytes uint64) (http.Header, error) {
 	if l > maxHeaderBytes {
 		return nil, fmt.Errorf("HEADERS frame too large: %d bytes (max: %d)", l, maxHeaderBytes)
 	}
@@ -139,11 +139,7 @@ func (c *connection) parseTrailer(r io.Reader, l, maxHeaderBytes uint64) (http.H
 	if err != nil {
 		return nil, err
 	}
-	h := http.Header{}
-	for _, field := range fields {
-		h.Add(field.Name, field.Value)
-	}
-	return h, nil
+	return parseTrailers(fields)
 }
 
 func (c *connection) acceptStream(ctx context.Context) (quic.Stream, *datagrammer, error) {
diff --git a/http3/headers.go b/http3/headers.go
index fd50d04a..c51f48d0 100644
--- a/http3/headers.go
+++ b/http3/headers.go
@@ -101,6 +101,17 @@ func parseHeaders(headers []qpack.HeaderField, isRequest bool) (header, error) {
 	return hdr, nil
 }
 
+func parseTrailers(headers []qpack.HeaderField) (http.Header, error) {
+	h := make(http.Header, len(headers))
+	for _, field := range headers {
+		if field.IsPseudo() {
+			return nil, fmt.Errorf("http3: received pseudo header in trailer: %s", field.Name)
+		}
+		h.Add(field.Name, field.Value)
+	}
+	return h, nil
+}
+
 func requestFromHeaders(headerFields []qpack.HeaderField) (*http.Request, error) {
 	hdr, err := parseHeaders(headerFields, true)
 	if err != nil {


Test patch:
diff --git a/http3/headers_test.go b/http3/headers_test.go
index b1b52f5a..65fa3837 100644
--- a/http3/headers_test.go
+++ b/http3/headers_test.go
@@ -358,4 +358,21 @@ var _ = Describe("Response", func() {
 		err := updateResponseFromHeaders(&http.Response{}, headers)
 		Expect(err).To(MatchError("invalid response pseudo header: :method"))
 	})
+
+	It("parses trailers", func() {
+		headers := []qpack.HeaderField{
+			{Name: "content-length", Value: "42"},
+		}
+		hdr, err := parseTrailers(headers)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(hdr.Get("Content-Length")).To(Equal("42"))
+	})
+
+	It("parses trailers", func() {
+		headers := []qpack.HeaderField{
+			{Name: ":status", Value: "200"},
+		}
+		_, err := parseTrailers(headers)
+		Expect(err).To(MatchError("http3: received pseudo header in trailer: :status"))
+	})
 })