http3: initialize trailer map with empty trailer entries when parsing the header (#4656)

* feat: pre-populate trailers on response with empty values

* fix: improve comment/func name
This commit is contained in:
Kevin McDonald
2024-09-07 08:59:07 +02:00
committed by GitHub
parent 4f48b2ce25
commit b92bf0c80d
5 changed files with 61 additions and 10 deletions

View File

@@ -585,7 +585,7 @@ var _ = Describe("Client", func() {
trailerBuf := &bytes.Buffer{}
enc := qpack.NewEncoder(trailerBuf)
Expect(enc.WriteField(qpack.HeaderField{Name: "Grpc-Status", Value: "0"})).To(Succeed())
Expect(enc.WriteField(qpack.HeaderField{Name: "This-Is-A-Trailer", Value: "0"})).To(Succeed())
Expect(enc.Close()).To(Succeed())
b := (&headersFrame{Length: uint64(trailerBuf.Len())}).Append(nil)
b = append(b, trailerBuf.Bytes()...)
@@ -603,7 +603,7 @@ var _ = Describe("Client", func() {
Expect(err).ToNot(HaveOccurred())
_, err = io.ReadAll(rsp.Body)
Expect(err).ToNot(HaveOccurred())
Expect(rsp.Trailer).To(Equal(http.Header{"Grpc-Status": []string{"0"}}))
Expect(rsp.Trailer).To(Equal(http.Header{"This-Is-A-Trailer": []string{"0"}}))
Expect(rsp.Proto).To(Equal("HTTP/3.0"))
Expect(rsp.ProtoMajor).To(Equal(3))
Expect(rsp.StatusCode).To(Equal(418))
@@ -637,7 +637,7 @@ var _ = Describe("Client", func() {
{
trailerBuf := &bytes.Buffer{}
enc := qpack.NewEncoder(trailerBuf)
Expect(enc.WriteField(qpack.HeaderField{Name: "Grpc-Status", Value: "0"})).To(Succeed())
Expect(enc.WriteField(qpack.HeaderField{Name: "This-Is-A-Trailer", Value: "0"})).To(Succeed())
Expect(enc.Close()).To(Succeed())
b := (&headersFrame{Length: uint64(trailerBuf.Len())}).Append(nil)
b = append(b, trailerBuf.Bytes()...)
@@ -647,7 +647,7 @@ var _ = Describe("Client", func() {
{
trailerBuf := &bytes.Buffer{}
enc := qpack.NewEncoder(trailerBuf)
Expect(enc.WriteField(qpack.HeaderField{Name: "Grpc-Status", Value: "1"})).To(Succeed())
Expect(enc.WriteField(qpack.HeaderField{Name: "This-Is-A-Trailer", Value: "1"})).To(Succeed())
Expect(enc.Close()).To(Succeed())
b := (&headersFrame{Length: uint64(trailerBuf.Len())}).Append(nil)
b = append(b, trailerBuf.Bytes()...)
@@ -666,7 +666,7 @@ var _ = Describe("Client", func() {
Expect(err).ToNot(HaveOccurred())
_, err = io.ReadAll(rsp.Body)
Expect(err).To(MatchError(errors.New("additional HEADERS frame received after trailers")))
Expect(rsp.Trailer).To(Equal(http.Header{"Grpc-Status": []string{"0"}}))
Expect(rsp.Trailer).To(Equal(http.Header{"This-Is-A-Trailer": []string{"0"}}))
Expect(rsp.Proto).To(Equal("HTTP/3.0"))
Expect(rsp.ProtoMajor).To(Equal(3))
Expect(rsp.StatusCode).To(Equal(418))
@@ -679,7 +679,7 @@ var _ = Describe("Client", func() {
{
trailerBuf := &bytes.Buffer{}
enc := qpack.NewEncoder(trailerBuf)
Expect(enc.WriteField(qpack.HeaderField{Name: "Grpc-Status", Value: "0"})).To(Succeed())
Expect(enc.WriteField(qpack.HeaderField{Name: "This-Is-A-Trailer", Value: "0"})).To(Succeed())
Expect(enc.Close()).To(Succeed())
b := (&headersFrame{Length: uint64(trailerBuf.Len())}).Append(nil)
b = append(b, trailerBuf.Bytes()...)
@@ -706,7 +706,7 @@ var _ = Describe("Client", func() {
Expect(err).ToNot(HaveOccurred())
_, err = io.ReadAll(rsp.Body)
Expect(err).To(MatchError(errors.New("DATA frame received after trailers")))
Expect(rsp.Trailer).To(Equal(http.Header{"Grpc-Status": []string{"0"}}))
Expect(rsp.Trailer).To(Equal(http.Header{"This-Is-A-Trailer": []string{"0"}}))
Expect(rsp.Proto).To(Equal("HTTP/3.0"))
Expect(rsp.ProtoMajor).To(Equal(3))
Expect(rsp.StatusCode).To(Equal(418))

View File

@@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"net/http"
"net/textproto"
"net/url"
"strconv"
"strings"
@@ -222,6 +223,7 @@ func updateResponseFromHeaders(rsp *http.Response, headerFields []qpack.HeaderFi
rsp.Proto = "HTTP/3.0"
rsp.ProtoMajor = 3
rsp.Header = hdr.Headers
processTrailers(rsp)
rsp.ContentLength = hdr.ContentLength
status, err := strconv.Atoi(hdr.Status)
@@ -232,3 +234,27 @@ func updateResponseFromHeaders(rsp *http.Response, headerFields []qpack.HeaderFi
rsp.Status = hdr.Status + " " + http.StatusText(status)
return nil
}
// processTrailers initializes the rsp.Trailer map, and adds keys for every announced header value.
// The Trailer header is removed from the http.Response.Header map.
// It handles both duplicate as well as comma-separated values for the Trailer header.
// For example:
//
// Trailer: Trailer1, Trailer2
// Trailer: Trailer3
//
// Will result in a http.Response.Trailer map containing the keys "Trailer1", "Trailer2", "Trailer3".
func processTrailers(rsp *http.Response) {
rawTrailers, ok := rsp.Header["Trailer"]
if !ok {
return
}
rsp.Trailer = make(http.Header)
for _, rawVal := range rawTrailers {
for _, val := range strings.Split(rawVal, ",") {
rsp.Trailer[http.CanonicalHeaderKey(textproto.TrimString(val))] = nil
}
}
delete(rsp.Header, "Trailer")
}

View File

@@ -335,6 +335,23 @@ var _ = Describe("Response", func() {
Expect(rsp.Status).To(Equal("200 OK"))
})
It("parses trailer", func() {
headers := []qpack.HeaderField{
{Name: ":status", Value: "200"},
{Name: "trailer", Value: "Trailer1, Trailer2"},
{Name: "trailer", Value: "TRAILER3"},
}
rsp := &http.Response{}
err := updateResponseFromHeaders(rsp, headers)
Expect(err).NotTo(HaveOccurred())
Expect(rsp.Header).To(HaveLen(0))
Expect(rsp.Trailer).To(Equal(http.Header(map[string][]string{
"Trailer1": nil,
"Trailer2": nil,
"Trailer3": nil,
})))
})
It("rejects pseudo header fields after regular header fields", func() {
headers := []qpack.HeaderField{
{Name: "content-length", Value: "42"},

View File

@@ -307,10 +307,10 @@ func (w *responseWriter) writeTrailers() error {
var b bytes.Buffer
enc := qpack.NewEncoder(&b)
for trailer := range w.trailers {
trailerName := strings.ToLower(strings.TrimPrefix(trailer, http.TrailerPrefix))
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 {
if err := enc.WriteField(qpack.HeaderField{Name: trailerName, Value: val}); err != nil {
return err
}
}