Repository: quic-go/quic-go
Pull request #4664 (2024-09-07 03:32:49)

update qpack to v0.5.1
This release now doesn't reject N=1 values in Literal Field Line with Name Reference, see https://github.com/quic-go/qpack/pull/52.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4664?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 [(`4be70e0`)](https://app.codecov.io/gh/quic-go/quic-go/commit/4be70e0146b2a227b7960e69d2ed44c8fc7a8297?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`420babe`)](https://app.codecov.io/gh/quic-go/quic-go/commit/420babe986fc3e4ed8224a810776590d43a31638?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    #4664      +/-   ##
==========================================
+ Coverage   84.85%   84.87%   +0.02%     
==========================================
  Files         147      147              
  Lines       14577    14577              
==========================================
+ Hits        12368    12371       +3     
+ Misses       1702     1699       -3     
  Partials      507      507              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4664?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/.github/workflows/build-interop-docker.yml b/.github/workflows/build-interop-docker.yml
index 99b80516..209e9b4f 100644
--- a/.github/workflows/build-interop-docker.yml
+++ b/.github/workflows/build-interop-docker.yml
@@ -5,7 +5,11 @@ on:
       - master
     tags:
       - 'v*'
-  
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   interop:
     runs-on: ${{ fromJSON(vars['DOCKER_RUNNER_UBUNTU'] || '"ubuntu-latest"') }}
diff --git a/connection.go b/connection.go
index f4a5ca93..70c47333 100644
--- a/connection.go
+++ b/connection.go
@@ -864,7 +864,9 @@ func (s *connection) handlePacketImpl(rp receivedPacket) bool {
 			if counter > 0 {
 				p.buffer.Split()
 			}
-			processed = s.handleShortHeaderPacket(p)
+			if wasProcessed := s.handleShortHeaderPacket(p); wasProcessed {
+				processed = true
+			}
 			break
 		}
 	}
diff --git a/go.mod b/go.mod
index 22009b8d..17db49c1 100644
--- a/go.mod
+++ b/go.mod
@@ -7,7 +7,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.9.5
 	github.com/onsi/gomega v1.27.6
 	github.com/prometheus/client_golang v1.19.1
-	github.com/quic-go/qpack v0.5.0
+	github.com/quic-go/qpack v0.5.1
 	go.uber.org/mock v0.4.0
 	golang.org/x/crypto v0.26.0
 	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
diff --git a/go.sum b/go.sum
index cde63ad3..25fe9d11 100644
--- a/go.sum
+++ b/go.sum
@@ -100,8 +100,8 @@ github.com/prometheus/common v0.48.0/go.mod h1:0/KsvlIEfPQCQ5I2iNSAWKPZziNCvRs5E
 github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k6Bo=
 github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo=
-github.com/quic-go/qpack v0.5.0 h1:jldbr38Ef/swDfxtvNvvUIYNg5LNm3Oa9W+IZvCm4q0=
-github.com/quic-go/qpack v0.5.0/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg=
+github.com/quic-go/qpack v0.5.1 h1:giqksBPnT/HDtZ6VhtFKgoLOWmlyo9Ei6u9PqzIMbhI=
+github.com/quic-go/qpack v0.5.1/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg=
 github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ=
 github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog=
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
diff --git a/http3/response_writer.go b/http3/response_writer.go
index 71f6551e..14ba1ba8 100644
--- a/http3/response_writer.go
+++ b/http3/response_writer.go
@@ -5,11 +5,13 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
+	"net/textproto"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/quic-go/qpack"
+	"golang.org/x/net/http/httpguts"
 )
 
 // The HTTPStreamer allows taking over a HTTP/3 stream. The interface is implemented the http.Response.Body.
@@ -28,10 +30,11 @@ const maxSmallResponseSize = 4096
 type responseWriter struct {
 	str *stream
 
-	conn   Connection
-	header http.Header
-	buf    []byte
-	status int // status code passed to WriteHeader
+	conn     Connection
+	header   http.Header
+	trailers map[string]struct{}
+	buf      []byte
+	status   int // status code passed to WriteHeader
 
 	// for responses smaller than maxSmallResponseSize, we buffer calls to Write,
 	// and automatically add the Content-Length header
@@ -42,6 +45,7 @@ type responseWriter struct {
 	headerComplete bool  // set once WriteHeader is called with a status code >= 200
 	headerWritten  bool  // set once the response header has been serialized to the stream
 	isHead         bool
+	trailerWritten bool // set once the response trailers has been serialized to the stream
 
 	hijacked bool // set on HTTPStream is called
 
@@ -198,7 +202,26 @@ func (w *responseWriter) writeHeader(status int) error {
 		return err
 	}
 
+	// Handle trailer fields
+	if vals, ok := w.header["Trailer"]; ok {
+		for _, val := range vals {
+			for _, trailer := range strings.Split(val, ",") {
+				// We need to convert to the canonical header key value here because this will be called when using
+				// headers.Add or headers.Set.
+				trailer = textproto.CanonicalMIMEHeaderKey(strings.TrimSpace(trailer))
+				w.declareTrailer(trailer)
+			}
+		}
+	}
+
 	for k, v := range w.header {
+		if _, excluded := w.trailers[k]; excluded {
+			continue
+		}
+		// Ignore "Trailer:" prefixed headers
+		if strings.HasPrefix(k, http.TrailerPrefix) {
+			continue
+		}
 		for index := range v {
 			if err := enc.WriteField(qpack.HeaderField{Name: strings.ToLower(k), Value: v[index]}); err != nil {
 				return err
@@ -222,6 +245,15 @@ func (w *responseWriter) FlushError() error {
 	return err
 }
 
+func (w *responseWriter) flushTrailers() {
+	if w.trailerWritten {
+		return
+	}
+	if err := w.writeTrailers(); err != nil {
+		w.logger.Debug("could not write trailers", "error", err)
+	}
+}
+
 func (w *responseWriter) Flush() {
 	if err := w.FlushError(); err != nil {
 		if w.logger != nil {
@@ -230,6 +262,69 @@ func (w *responseWriter) Flush() {
 	}
 }
 
+// declareTrailer adds a trailer to the trailer list, while also validating that the trailer has a
+// valid name.
+func (w *responseWriter) declareTrailer(k string) {
+	if !httpguts.ValidTrailerHeader(k) {
+		// Forbidden by RFC 9110, section 6.5.1.
+		w.logger.Debug("ignoring invalid trailer", slog.String("header", k))
+		return
+	}
+	if w.trailers == nil {
+		w.trailers = make(map[string]struct{})
+	}
+	w.trailers[k] = struct{}{}
+}
+
+// hasNonEmptyTrailers checks to see if there are any trailers with an actual
+// value set. This is possible by adding trailers to the "Trailers" header
+// but never actually setting those names as trailers in the course of handling
+// the request. In that case, this check may save us some allocations.
+func (w *responseWriter) hasNonEmptyTrailers() bool {
+	for trailer := range w.trailers {
+		if _, ok := w.header[trailer]; ok {
+			return true
+		}
+	}
+	return false
+}
+
+// writeTrailers will write trailers to the stream if there are any.
+func (w *responseWriter) writeTrailers() error {
+	// promote headers added via "Trailer:" convention as trailers, these can be added after
+	// streaming the status/headers have been written.
+	for k := range w.header {
+		// Handle "Trailer:" prefix
+		if strings.HasPrefix(k, http.TrailerPrefix) {
+			w.declareTrailer(k)
+		}
+	}
+
+	if !w.hasNonEmptyTrailers() {
+		return nil
+	}
+
+	var b bytes.Buffer
+	enc := qpack.NewEncoder(&b)
+	for trailer := range w.trailers {
+		if vals, ok := w.header[trailer]; ok {
+			name := strings.TrimPrefix(trailer, http.TrailerPrefix)
+			for _, val := range vals {
+				if err := enc.WriteField(qpack.HeaderField{Name: strings.ToLower(name), Value: val}); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	buf := make([]byte, 0, frameHeaderLen+b.Len())
+	buf = (&headersFrame{Length: uint64(b.Len())}).Append(buf)
+	buf = append(buf, b.Bytes()...)
+	_, err := w.str.writeUnframed(buf)
+	w.trailerWritten = true
+	return err
+}
+
 func (w *responseWriter) HTTPStream() Stream {
 	w.hijacked = true
 	w.Flush()
diff --git a/http3/server.go b/http3/server.go
index d3e43673..9f285b6e 100644
--- a/http3/server.go
+++ b/http3/server.go
@@ -625,6 +625,7 @@ func (s *Server) handleRequest(conn *connection, str quic.Stream, datagrams *dat
 			}
 		}
 		r.Flush()
+		r.flushTrailers()
 	}
 
 	// abort the stream when there is a panic


Test patch:
diff --git a/http3/response_writer_test.go b/http3/response_writer_test.go
index 3394043a..94437c77 100644
--- a/http3/response_writer_test.go
+++ b/http3/response_writer_test.go
@@ -31,6 +31,7 @@ var _ = Describe("Response Writer", func() {
 
 	decodeHeader := func(str io.Reader) map[string][]string {
 		rw.Flush()
+		rw.flushTrailers()
 		fields := make(map[string][]string)
 		decoder := qpack.NewDecoder(nil)
 
@@ -184,4 +185,65 @@ var _ = Describe("Response Writer", func() {
 		Expect(func() { rw.WriteHeader(99) }).To(Panic())
 		Expect(func() { rw.WriteHeader(1000) }).To(Panic())
 	})
+
+	It("write announced trailer", func() {
+		rw.Header().Add("Trailer", "Key")
+		rw.WriteHeader(http.StatusTeapot)
+		n, err := rw.Write([]byte("foobar"))
+		Expect(n).To(Equal(6))
+		Expect(err).ToNot(HaveOccurred())
+		rw.Header().Set("Key", "Value")
+
+		// writeTrailers needs to be called after writing the full body
+		Expect(rw.writeTrailers()).ToNot(HaveOccurred())
+
+		fields := decodeHeader(strBuf)
+		Expect(fields).To(HaveKeyWithValue(":status", []string{"418"}))
+		Expect(fields).To(HaveKeyWithValue("trailer", []string{"Key"}))
+		Expect(getData(strBuf)).To(Equal([]byte("foobar")))
+
+		fields = decodeHeader(strBuf)
+		Expect(fields).To(HaveKeyWithValue("key", []string{"Value"}))
+	})
+
+	It("ignore non-announced trailer (without trailer prefix)", func() {
+		rw.Header().Set("Trailer", "Key")
+		rw.WriteHeader(200)
+		rw.Write([]byte("foobar"))
+		rw.Header().Set("UnknownKey", "Value")
+		rw.Header().Set("Key", "Value")
+
+		// Needs to call writeTrailers to simulate the end of the handler
+		Expect(rw.writeTrailers()).ToNot(HaveOccurred())
+		headers := decodeHeader(strBuf)
+		Expect(headers).To(HaveKeyWithValue(":status", []string{"200"}))
+		Expect(headers).To(HaveKeyWithValue("trailer", []string{"Key"}))
+
+		Expect(getData(strBuf)).To(Equal([]byte("foobar")))
+
+		trailers := decodeHeader(strBuf)
+		Expect(trailers).To(HaveKeyWithValue("key", []string{"Value"}))
+		Expect(trailers).To(Not(HaveKeyWithValue("unknownkey", []string{"Value"})))
+	})
+
+	It("write non-announced trailer (with trailer prefix)", func() {
+		rw.Header().Set("Trailer", "Key")
+		rw.WriteHeader(200)
+		rw.Write([]byte("foobar"))
+		rw.Header().Set("Key", "Value")
+		rw.Header().Set(http.TrailerPrefix+"Key2", "Value")
+		rw.Flush()
+
+		// Needs to call writeTrailers to simulate the end of the handler
+		Expect(rw.writeTrailers()).ToNot(HaveOccurred())
+		headers := decodeHeader(strBuf)
+		Expect(headers).To(HaveKeyWithValue(":status", []string{"200"}))
+		Expect(headers).To(HaveKeyWithValue("trailer", []string{"Key"}))
+
+		Expect(getData(strBuf)).To(Equal([]byte("foobar")))
+
+		trailers := decodeHeader(strBuf)
+		Expect(trailers).To(HaveKeyWithValue("key", []string{"Value"}))
+		Expect(trailers).To(HaveKeyWithValue("key2", []string{"Value"}))
+	})
 })
diff --git a/integrationtests/gomodvendor/go.mod b/integrationtests/gomodvendor/go.mod
index ebc9de9b..3411b837 100644
--- a/integrationtests/gomodvendor/go.mod
+++ b/integrationtests/gomodvendor/go.mod
@@ -11,7 +11,7 @@ require (
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 // indirect
 	github.com/onsi/ginkgo/v2 v2.9.5 // indirect
-	github.com/quic-go/qpack v0.5.0 // indirect
+	github.com/quic-go/qpack v0.5.1 // indirect
 	go.uber.org/mock v0.4.0 // indirect
 	golang.org/x/crypto v0.26.0 // indirect
 	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
diff --git a/integrationtests/gomodvendor/go.sum b/integrationtests/gomodvendor/go.sum
index 4a26ee49..8d125919 100644
--- a/integrationtests/gomodvendor/go.sum
+++ b/integrationtests/gomodvendor/go.sum
@@ -21,8 +21,8 @@ github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE=
 github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/quic-go/qpack v0.5.0 h1:jldbr38Ef/swDfxtvNvvUIYNg5LNm3Oa9W+IZvCm4q0=
-github.com/quic-go/qpack v0.5.0/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg=
+github.com/quic-go/qpack v0.5.1 h1:giqksBPnT/HDtZ6VhtFKgoLOWmlyo9Ei6u9PqzIMbhI=
+github.com/quic-go/qpack v0.5.1/go.mod h1:+PC4XFrEskIVkcLzpEkbLqq1uCoxPhQuvK5rH1ZgaEg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
diff --git a/integrationtests/self/http_test.go b/integrationtests/self/http_test.go
index cef97ba2..2242e0c4 100644
--- a/integrationtests/self/http_test.go
+++ b/integrationtests/self/http_test.go
@@ -1019,4 +1019,45 @@ var _ = Describe("HTTP tests", func() {
 			Expect(num0RTTPackets.Load()).To(BeNumerically(">", 0))
 		})
 	})
+
+	It("sends and receives trailers", func() {
+		mux.HandleFunc("/trailers", func(w http.ResponseWriter, r *http.Request) {
+			defer GinkgoRecover()
+			w.Header().Set("Trailer", "AtEnd1, AtEnd2")
+			w.Header().Add("Trailer", "LAST")
+			w.Header().Set("Content-Type", "text/plain; charset=utf-8") // normal header
+			w.WriteHeader(http.StatusOK)
+			w.Header().Set("AtEnd1", "value 1")
+			io.WriteString(w, "This HTTP response has both headers before this text and trailers at the end.\n")
+			w.(http.Flusher).Flush()
+			w.Header().Set("AtEnd2", "value 2")
+			io.WriteString(w, "More text\n")
+			w.(http.Flusher).Flush()
+			w.Header().Set("LAST", "value 3")
+			w.Header().Set(http.TrailerPrefix+"Unannounced", "Surprise!")
+			w.Header().Set("Late-Header", "No surprise!")
+		})
+
+		resp, err := client.Get(fmt.Sprintf("https://localhost:%d/trailers", port))
+		Expect(err).ToNot(HaveOccurred())
+		Expect(resp.StatusCode).To(Equal(200))
+		Expect(resp.Header.Values("Trailer")).To(Equal([]string{"AtEnd1, AtEnd2", "LAST"}))
+		Expect(resp.Header).To(Not(HaveKey("Atend1")))
+		Expect(resp.Header).To(Not(HaveKey("Atend2")))
+		Expect(resp.Header).To(Not(HaveKey("Last")))
+		Expect(resp.Header).To(Not(HaveKey("Late-Header")))
+
+		body, err := io.ReadAll(gbytes.TimeoutReader(resp.Body, 3*time.Second))
+		Expect(err).ToNot(HaveOccurred())
+		Expect(string(body)).To(Equal("This HTTP response has both headers before this text and trailers at the end.\nMore text\n"))
+		for k := range resp.Header {
+			Expect(k).To(Not(HavePrefix(http.TrailerPrefix)))
+		}
+		Expect(resp.Trailer).To(Equal(http.Header(map[string][]string{
+			"Atend1":      {"value 1"},
+			"Atend2":      {"value 2"},
+			"Last":        {"value 3"},
+			"Unannounced": {"Surprise!"},
+		})))
+	})
 })