From f1c7a5df73e835cfb556aa3aff5314550ea0f327 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 22 Apr 2025 18:54:31 +0800 Subject: [PATCH] http3: migrate the headers tests away from Ginkgo (#5068) --- http3/headers.go | 9 +- http3/headers_test.go | 832 +++++++++++++++++++++--------------------- http3/transport.go | 8 + 3 files changed, 424 insertions(+), 425 deletions(-) diff --git a/http3/headers.go b/http3/headers.go index 27af9500..db9afc9f 100644 --- a/http3/headers.go +++ b/http3/headers.go @@ -211,13 +211,6 @@ func requestFromHeaders(headerFields []qpack.HeaderField) (*http.Request, error) }, nil } -func hostnameFromURL(url *url.URL) string { - if url != nil { - return url.Host - } - return "" -} - // updateResponseFromHeaders sets up http.Response as an HTTP/3 response, // using the decoded qpack header filed. // It is only called for the HTTP header (and not the HTTP trailer). @@ -228,7 +221,7 @@ func updateResponseFromHeaders(rsp *http.Response, headerFields []qpack.HeaderFi return err } if hdr.Status == "" { - return errors.New("missing status field") + return errors.New("missing :status field") } rsp.Proto = "HTTP/3.0" rsp.ProtoMajor = 3 diff --git a/http3/headers_test.go b/http3/headers_test.go index 3f23cc3b..ef5ac278 100644 --- a/http3/headers_test.go +++ b/http3/headers_test.go @@ -3,454 +3,452 @@ package http3 import ( "fmt" "net/http" - "net/url" + "testing" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" "github.com/quic-go/qpack" + "github.com/stretchr/testify/require" ) -var _ = Describe("Request", func() { - It("populates requests", func() { +func TestRequestHeaderParsing(t *testing.T) { + t.Run("regular path", func(t *testing.T) { + testRequestHeaderParsing(t, "/foo") + }) + // see https://github.com/quic-go/quic-go/pull/1898 + t.Run("path starting with //", func(t *testing.T) { + testRequestHeaderParsing(t, "//foo") + }) +} + +func testRequestHeaderParsing(t *testing.T, path string) { + headers := []qpack.HeaderField{ + {Name: ":path", Value: path}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, + {Name: "content-length", Value: "42"}, + } + req, err := requestFromHeaders(headers) + require.NoError(t, err) + require.Equal(t, http.MethodGet, req.Method) + require.Equal(t, path, req.URL.Path) + require.Equal(t, "", req.URL.Host) + require.Equal(t, "HTTP/3.0", req.Proto) + require.Equal(t, 3, req.ProtoMajor) + require.Zero(t, req.ProtoMinor) + require.Equal(t, int64(42), req.ContentLength) + require.Equal(t, 1, len(req.Header)) + require.Equal(t, "42", req.Header.Get("Content-Length")) + require.Nil(t, req.Body) + require.Equal(t, "quic-go.net", req.Host) + require.Equal(t, path, req.RequestURI) +} + +func TestRequestHeadersContentLength(t *testing.T) { + t.Run("no content length", func(t *testing.T) { headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "content-length", Value: "42"}, + {Name: ":path", Value: "/"}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, } req, err := requestFromHeaders(headers) - Expect(err).NotTo(HaveOccurred()) - Expect(req.Method).To(Equal("GET")) - Expect(req.URL.Path).To(Equal("/foo")) - Expect(req.URL.Host).To(BeEmpty()) - Expect(req.Proto).To(Equal("HTTP/3.0")) - Expect(req.ProtoMajor).To(Equal(3)) - Expect(req.ProtoMinor).To(BeZero()) - Expect(req.ContentLength).To(Equal(int64(42))) - Expect(req.Header).To(HaveLen(1)) - Expect(req.Header.Get("Content-Length")).To(Equal("42")) - Expect(req.Body).To(BeNil()) - Expect(req.Host).To(Equal("quic.clemente.io")) - Expect(req.RequestURI).To(Equal("/foo")) + require.NoError(t, err) + require.Equal(t, int64(-1), req.ContentLength) }) - It("sets the ContentLength to -1", func() { + t.Run("multiple content lengths", func(t *testing.T) { 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"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "Content-Length", Value: "42"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("header field is not lower-case: Content-Length")) - }) - - It("rejects unknown pseudo headers", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: ":foo", Value: "bar"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("unknown pseudo header: :foo")) - }) - - It("rejects invalid field names", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "@", Value: "42"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(`invalid header field name: "@"`)) - }) - - It("rejects invalid field values", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "content", Value: "\n"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(`invalid header field value for content: "\n"`)) - }) - - It("rejects pseudo header fields after regular header fields", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: "content-length", Value: "42"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("received pseudo header :authority after a regular header field")) - }) - - It("rejects negative Content-Length values", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "content-length", Value: "-42"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("invalid content length")) - }) - - It("rejects multiple Content-Length headers, if they differ", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "content-length", Value: "42"}, - {Name: "content-length", Value: "1337"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("contradicting content lengths (42 and 1337)")) - }) - - It("deduplicates multiple Content-Length headers, if they're the same", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, + {Name: ":path", Value: "/"}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, {Name: "content-length", Value: "42"}, {Name: "content-length", Value: "42"}, } req, err := requestFromHeaders(headers) - Expect(err).ToNot(HaveOccurred()) - Expect(req.ContentLength).To(Equal(int64(42))) - Expect(req.Header.Get("Content-Length")).To(Equal("42")) + require.NoError(t, err) + require.Equal(t, "42", req.Header.Get("Content-Length")) }) +} - It("rejects pseudo header fields defined for responses", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: ":status", Value: "404"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("invalid request pseudo header: :status")) - }) - - It("parses path with leading double slashes", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "//foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - } - req, err := requestFromHeaders(headers) - Expect(err).NotTo(HaveOccurred()) - Expect(req.Header).To(BeEmpty()) - Expect(req.Body).To(BeNil()) - Expect(req.URL.Path).To(Equal("//foo")) - Expect(req.URL.Host).To(BeEmpty()) - Expect(req.Host).To(Equal("quic.clemente.io")) - Expect(req.RequestURI).To(Equal("//foo")) - }) - - It("concatenates the cookie headers", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "cookie", Value: "cookie1=foobar1"}, - {Name: "cookie", Value: "cookie2=foobar2"}, - } - req, err := requestFromHeaders(headers) - Expect(err).NotTo(HaveOccurred()) - Expect(req.Header).To(Equal(http.Header{ - "Cookie": []string{"cookie1=foobar1; cookie2=foobar2"}, - })) - }) - - It("handles Other headers", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: "cache-control", Value: "max-age=0"}, - {Name: "duplicate-header", Value: "1"}, - {Name: "duplicate-header", Value: "2"}, - } - req, err := requestFromHeaders(headers) - Expect(err).NotTo(HaveOccurred()) - Expect(req.Header).To(Equal(http.Header{ - "Cache-Control": []string{"max-age=0"}, - "Duplicate-Header": []string{"1", "2"}, - })) - }) - - It("errors with missing path", func() { - headers := []qpack.HeaderField{ - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(":path, :authority and :method must not be empty")) - }) - - It("errors with missing method", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(":path, :authority and :method must not be empty")) - }) - - It("errors with missing authority", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":method", Value: "GET"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(":path, :authority and :method must not be empty")) - }) - - It("errors with invalid protocol", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: ":protocol", Value: "connect-udp"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(":protocol must be empty")) - }) - - It("errors with duplicate pseudo header in request", func() { - headers := []qpack.HeaderField{ - {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: "GET"}, - {Name: ":scheme", Value: "https"}, - {Name: ":method", Value: "POST"}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("duplicate pseudo header: :method")) - }) - - Context("regular HTTP CONNECT", func() { - It("handles CONNECT method", func() { - headers := []qpack.HeaderField{ - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: http.MethodConnect}, +func TestRequestHeadersContentLengthValidation(t *testing.T) { + for _, tc := range []struct { + name string + headers []qpack.HeaderField + err string + errContains string + }{ + { + name: "negative content length", + headers: []qpack.HeaderField{ + {Name: "content-length", Value: "-42"}, + }, + errContains: "invalid content length", + }, + { + name: "multiple differing content lengths", + headers: []qpack.HeaderField{ + {Name: "content-length", Value: "42"}, + {Name: "content-length", Value: "1337"}, + }, + err: "contradicting content lengths (42 and 1337)", + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := requestFromHeaders(tc.headers) + if tc.errContains != "" { + require.ErrorContains(t, err, tc.errContains) } - req, err := requestFromHeaders(headers) - Expect(err).NotTo(HaveOccurred()) - Expect(req.Method).To(Equal(http.MethodConnect)) - Expect(req.Proto).To(Equal("HTTP/3.0")) - Expect(req.RequestURI).To(Equal("quic.clemente.io")) - }) - - It("errors with missing authority in CONNECT method", func() { - headers := []qpack.HeaderField{ - {Name: ":method", Value: http.MethodConnect}, + if tc.err != "" { + require.EqualError(t, err, tc.err) } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(":path must be empty and :authority must not be empty")) }) + } +} - It("errors with extra path in CONNECT method", func() { - headers := []qpack.HeaderField{ +func TestRequestHeadersValidation(t *testing.T) { + for _, tc := range []struct { + name string + headers []qpack.HeaderField + err string + }{ + { + name: "upper-case field name", + headers: []qpack.HeaderField{ + {Name: "Content-Length", Value: "42"}, + }, + err: "header field is not lower-case: Content-Length", + }, + { + name: "unknown pseudo header", + headers: []qpack.HeaderField{ + {Name: ":foo", Value: "bar"}, + }, + err: "unknown pseudo header: :foo", + }, + { + name: "pseudo header after regular header", + headers: []qpack.HeaderField{ {Name: ":path", Value: "/foo"}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":method", Value: http.MethodConnect}, - } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError(":path must be empty and :authority must not be empty")) - }) - }) - - Context("Extended CONNECT", func() { - It("handles Extended CONNECT method", func() { - headers := []qpack.HeaderField{ - {Name: ":protocol", Value: "webtransport"}, - {Name: ":scheme", Value: "ftp"}, - {Name: ":method", Value: http.MethodConnect}, - {Name: ":authority", Value: "quic.clemente.io"}, - {Name: ":path", Value: "/foo?val=1337"}, - } - req, err := requestFromHeaders(headers) - Expect(err).NotTo(HaveOccurred()) - Expect(req.Method).To(Equal(http.MethodConnect)) - Expect(req.Proto).To(Equal("webtransport")) - Expect(req.URL.String()).To(Equal("ftp://quic.clemente.io/foo?val=1337")) - Expect(req.URL.Query().Get("val")).To(Equal("1337")) - }) - - It("errors with missing scheme", func() { - headers := []qpack.HeaderField{ - {Name: ":protocol", Value: "webtransport"}, - {Name: ":method", Value: http.MethodConnect}, - {Name: ":authority", Value: "quic.clemente.io"}, + {Name: "content-length", Value: "42"}, + {Name: ":authority", Value: "quic-go.net"}, + }, + err: "received pseudo header :authority after a regular header field", + }, + { + name: "invalid field name", + headers: []qpack.HeaderField{ + {Name: "@", Value: "42"}, + }, + err: `invalid header field name: "@"`, + }, + { + name: "invalid field value", + headers: []qpack.HeaderField{ + {Name: "content", Value: "\n"}, + }, + err: `invalid header field value for content: "\n"`, + }, + { + name: ":status header field", // :status is a response pseudo header + headers: []qpack.HeaderField{ + {Name: ":status", Value: "404"}, + }, + err: "invalid request pseudo header: :status", + }, + { + name: "missing :path", + headers: []qpack.HeaderField{ + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, + }, + err: ":path, :authority and :method must not be empty", + }, + { + name: "missing :authority", + headers: []qpack.HeaderField{ {Name: ":path", Value: "/foo"}, + {Name: ":method", Value: http.MethodGet}, + }, + err: ":path, :authority and :method must not be empty", + }, + { + name: "missing :method", + headers: []qpack.HeaderField{ + {Name: ":path", Value: "/foo"}, + {Name: ":authority", Value: "quic-go.net"}, + }, + err: ":path, :authority and :method must not be empty", + }, + { + name: "duplicate :path", + headers: []qpack.HeaderField{ + {Name: ":path", Value: "/foo"}, + {Name: ":path", Value: "/foo"}, + }, + err: "duplicate pseudo header: :path", + }, + { + name: "duplicate :authority", + headers: []qpack.HeaderField{ + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":authority", Value: "quic-go.net"}, + }, + err: "duplicate pseudo header: :authority", + }, + { + name: "duplicate :method", + headers: []qpack.HeaderField{ + {Name: ":method", Value: http.MethodGet}, + {Name: ":method", Value: http.MethodGet}, + }, + err: "duplicate pseudo header: :method", + }, + { + name: "invalid :protocol", + headers: []qpack.HeaderField{ + {Name: ":path", Value: "/foo"}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, + {Name: ":protocol", Value: "connect-udp"}, + }, + err: ":protocol must be empty", + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := requestFromHeaders(tc.headers) + require.EqualError(t, err, tc.err) + }) + } +} + +func TestCookieHeader(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":path", Value: "/foo"}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, + {Name: "cookie", Value: "cookie1=foobar1"}, + {Name: "cookie", Value: "cookie2=foobar2"}, + } + req, err := requestFromHeaders(headers) + require.NoError(t, err) + require.Equal(t, http.Header{ + "Cookie": []string{"cookie1=foobar1; cookie2=foobar2"}, + }, req.Header) +} + +func TestHeadersConcatenation(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":path", Value: "/foo"}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodGet}, + {Name: "cache-control", Value: "max-age=0"}, + {Name: "duplicate-header", Value: "1"}, + {Name: "duplicate-header", Value: "2"}, + } + req, err := requestFromHeaders(headers) + require.NoError(t, err) + require.Equal(t, http.Header{ + "Cache-Control": []string{"max-age=0"}, + "Duplicate-Header": []string{"1", "2"}, + }, req.Header) +} + +func TestRequestHeadersConnect(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":method", Value: http.MethodConnect}, + } + req, err := requestFromHeaders(headers) + require.NoError(t, err) + require.Equal(t, http.MethodConnect, req.Method) + require.Equal(t, "HTTP/3.0", req.Proto) + require.Equal(t, "quic-go.net", req.RequestURI) +} + +func TestRequestHeadersConnectValidation(t *testing.T) { + for _, tc := range []struct { + name string + headers []qpack.HeaderField + err string + }{ + { + name: "missing :authority", + headers: []qpack.HeaderField{ + {Name: ":method", Value: http.MethodConnect}, + }, + err: ":path must be empty and :authority must not be empty", + }, + { + name: ":path set", + headers: []qpack.HeaderField{ + {Name: ":path", Value: "/foo"}, + {Name: ":method", Value: http.MethodConnect}, + }, + err: ":path must be empty and :authority must not be empty", + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := requestFromHeaders(tc.headers) + require.EqualError(t, err, tc.err) + }) + } +} + +func TestRequestHeadersExtendedConnect(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":protocol", Value: "webtransport"}, + {Name: ":scheme", Value: "ftp"}, + {Name: ":method", Value: http.MethodConnect}, + {Name: ":authority", Value: "quic-go.net"}, + {Name: ":path", Value: "/foo?val=1337"}, + } + req, err := requestFromHeaders(headers) + require.NoError(t, err) + require.Equal(t, http.MethodConnect, req.Method) + require.Equal(t, "webtransport", req.Proto) + require.Equal(t, "ftp://quic-go.net/foo?val=1337", req.URL.String()) + require.Equal(t, "1337", req.URL.Query().Get("val")) +} + +func TestRequestHeadersExtendedConnectRequestValidation(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":protocol", Value: "webtransport"}, + {Name: ":method", Value: http.MethodConnect}, + {Name: ":authority", Value: "quic.clemente.io"}, + {Name: ":path", Value: "/foo"}, + } + _, err := requestFromHeaders(headers) + require.EqualError(t, err, "extended CONNECT: :scheme, :path and :authority must not be empty") +} + +func TestResponseHeaderParsing(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":status", Value: "200"}, + {Name: "content-length", Value: "42"}, + } + rsp := &http.Response{} + require.NoError(t, updateResponseFromHeaders(rsp, headers)) + require.Equal(t, "HTTP/3.0", rsp.Proto) + require.Equal(t, 3, rsp.ProtoMajor) + require.Zero(t, rsp.ProtoMinor) + require.Equal(t, int64(42), rsp.ContentLength) + require.Equal(t, 1, len(rsp.Header)) + require.Equal(t, "42", rsp.Header.Get("Content-Length")) + require.Nil(t, rsp.Body) + require.Equal(t, 200, rsp.StatusCode) + require.Equal(t, "200 OK", rsp.Status) +} + +func TestResponseHeaderParsingValidation(t *testing.T) { + for _, tc := range []struct { + name string + headers []qpack.HeaderField + err string + errContains string + }{ + { + name: "missing :status", + headers: []qpack.HeaderField{ + {Name: "content-length", Value: "42"}, + }, + err: "missing :status field", + }, + { + name: "invalid status code", + headers: []qpack.HeaderField{ + {Name: ":status", Value: "foobar"}, + }, + errContains: "invalid status code", + }, + { + name: ":method header field", // :method is a request pseudo header + headers: []qpack.HeaderField{ + {Name: ":method", Value: http.MethodGet}, + }, + err: "invalid response pseudo header: :method", + }, + { + name: "duplicate :status", + headers: []qpack.HeaderField{ + {Name: ":status", Value: "200"}, + {Name: ":status", Value: "404"}, + }, + err: "duplicate pseudo header: :status", + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := updateResponseFromHeaders(&http.Response{}, tc.headers) + if tc.errContains != "" { + require.ErrorContains(t, err, tc.errContains) + } + if tc.err != "" { + require.EqualError(t, err, tc.err) } - _, err := requestFromHeaders(headers) - Expect(err).To(MatchError("extended CONNECT: :scheme, :path and :authority must not be empty")) }) - }) + } - Context("extracting the hostname from a request", func() { - var url *url.URL - - BeforeEach(func() { - var err error - url, err = url.Parse("https://quic.clemente.io:1337") - Expect(err).ToNot(HaveOccurred()) - }) - - It("uses URL.Host", func() { - Expect(hostnameFromURL(url)).To(Equal("quic.clemente.io:1337")) - }) - - It("returns an empty hostname if nothing is set", func() { - Expect(hostnameFromURL(nil)).To(BeEmpty()) - }) - }) -}) - -var _ = Describe("Response", func() { - It("populates responses", func() { - headers := []qpack.HeaderField{ - {Name: ":status", Value: "200"}, - {Name: "content-length", Value: "42"}, - } - rsp := &http.Response{} - err := updateResponseFromHeaders(rsp, headers) - Expect(err).NotTo(HaveOccurred()) - Expect(rsp.Proto).To(Equal("HTTP/3.0")) - Expect(rsp.ProtoMajor).To(Equal(3)) - Expect(rsp.ProtoMinor).To(BeZero()) - Expect(rsp.ContentLength).To(Equal(int64(42))) - Expect(rsp.Header).To(HaveLen(1)) - Expect(rsp.Header.Get("Content-Length")).To(Equal("42")) - Expect(rsp.Body).To(BeNil()) - Expect(rsp.StatusCode).To(BeEquivalentTo(200)) - 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"}, - {Name: ":status", Value: "200"}, - } - err := updateResponseFromHeaders(&http.Response{}, headers) - Expect(err).To(MatchError("received pseudo header :status after a regular header field")) - }) - - It("rejects response with no status field", func() { - headers := []qpack.HeaderField{ - {Name: "content-length", Value: "42"}, - } - err := updateResponseFromHeaders(&http.Response{}, headers) - Expect(err).To(MatchError("missing status field")) - }) - - It("rejects invalid status codes", func() { - headers := []qpack.HeaderField{ - {Name: ":status", Value: "foobar"}, - } - err := updateResponseFromHeaders(&http.Response{}, headers) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("invalid status code")) - }) - - It("rejects pseudo header fields defined for requests", func() { - headers := []qpack.HeaderField{ - {Name: ":status", Value: "404"}, - {Name: ":method", Value: "GET"}, - } - err := updateResponseFromHeaders(&http.Response{}, headers) - Expect(err).To(MatchError("invalid response pseudo header: :method")) - }) - - DescribeTable("rejecting invalid header fields", - func(invalidField string) { + for _, tc := range []struct { + name string + invalidField string + }{ + {name: "connection", invalidField: "connection"}, + {name: "keep-alive", invalidField: "keep-alive"}, + {name: "proxy-connection", invalidField: "proxy-connection"}, + {name: "transfer-encoding", invalidField: "transfer-encoding"}, + {name: "upgrade", invalidField: "upgrade"}, + } { + t.Run("invalid field: "+tc.name, func(t *testing.T) { headers := []qpack.HeaderField{ {Name: ":status", Value: "404"}, - {Name: invalidField, Value: "some-value"}, + {Name: tc.invalidField, Value: "some-value"}, } err := updateResponseFromHeaders(&http.Response{}, headers) - Expect(err).To(MatchError(fmt.Sprintf("invalid header field name: %q", invalidField))) - }, - Entry("connection", "connection"), - Entry("keep-alive", "keep-alive"), - Entry("proxy-connection", "proxy-connection"), - Entry("transfer-encoding", "transfer-encoding"), - Entry("upgrade", "upgrade"), - ) + require.EqualError(t, err, fmt.Sprintf("invalid header field name: %q", tc.invalidField)) + }) + } +} - It("rejects the TE header field, unless it is set to trailers", func() { - headers := []qpack.HeaderField{ - {Name: ":status", Value: "404"}, - {Name: "te", Value: "trailers"}, - } - Expect(updateResponseFromHeaders(&http.Response{}, headers)).To(Succeed()) - headers = []qpack.HeaderField{ - {Name: ":status", Value: "404"}, - {Name: "te", Value: "not-trailers"}, - } - Expect(updateResponseFromHeaders(&http.Response{}, headers)).To(MatchError("invalid TE header field value: \"not-trailers\"")) - }) +func TestResponseTrailerFields(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":status", Value: "200"}, + {Name: "trailer", Value: "Trailer1, Trailer2"}, + {Name: "trailer", Value: "TRAILER3"}, + } + var rsp http.Response + require.NoError(t, updateResponseFromHeaders(&rsp, headers)) + require.Equal(t, 0, len(rsp.Header)) + require.Equal(t, http.Header(map[string][]string{ + "Trailer1": nil, + "Trailer2": nil, + "Trailer3": nil, + }), rsp.Trailer) +} - 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")) - }) +func TestResponseTrailerParsingTE(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":status", Value: "404"}, + {Name: "te", Value: "trailers"}, + } + require.NoError(t, updateResponseFromHeaders(&http.Response{}, headers)) + headers = []qpack.HeaderField{ + {Name: ":status", Value: "404"}, + {Name: "te", Value: "not-trailers"}, + } + require.EqualError(t, + updateResponseFromHeaders(&http.Response{}, headers), + `invalid TE header field value: "not-trailers"`) +} - 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")) +func TestResponseTrailerParsing(t *testing.T) { + trailerHdr, err := parseTrailers([]qpack.HeaderField{ + {Name: "content-length", Value: "42"}, }) + require.NoError(t, err) + require.Equal(t, "42", trailerHdr.Get("Content-Length")) +} - It("errors with duplicate status header in response", func() { - headers := []qpack.HeaderField{ - {Name: ":status", Value: "200"}, - {Name: ":status", Value: "400"}, - } - err := updateResponseFromHeaders(&http.Response{}, headers) - Expect(err).To(MatchError("duplicate pseudo header: :status")) - }) -}) +func TestResponseTrailerParsingValidation(t *testing.T) { + headers := []qpack.HeaderField{ + {Name: ":status", Value: "200"}, + } + _, err := parseTrailers(headers) + require.EqualError(t, err, "http3: received pseudo header in trailer: :status") +} diff --git a/http3/transport.go b/http3/transport.go index bdedb524..dd73a8bb 100644 --- a/http3/transport.go +++ b/http3/transport.go @@ -10,6 +10,7 @@ import ( "net" "net/http" "net/http/httptrace" + "net/url" "strings" "sync" "sync/atomic" @@ -428,6 +429,13 @@ func (t *Transport) Close() error { return nil } +func hostnameFromURL(url *url.URL) string { + if url != nil { + return url.Host + } + return "" +} + func validMethod(method string) bool { /* Method = "OPTIONS" ; Section 9.2