internal/http3: add server support for "Trailer:" magic prefix Similar to our HTTP/1 and HTTP/2 implementations, allow HTTP/3 server handler to send trailer headers that have not been declared prior to writing response body by using the "Trailer:" magic prefix. To support this, also modify transport to accept all trailers, even those that have not been declared. This is consistent with the behavior of our HTTP/1 and HTTP/2 implementations. Note the following, unfortunately somewhat convoluted, behavior: 1. Transport will never send undeclared trailers. 2. Transport will accept undeclared trailers. 3. Server will never accept undeclared trailers. 4. Server will never send undeclared trailers, unless they use the "Trailer:" magic prefix. For golang/go#70914 Change-Id: Ifa5e026eb6ce68af9a8364845e4f2def6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/net/+/785860 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Viacheslav Danilin <viacheslavdanilin@gmail.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <husin@google.com>
diff --git a/internal/http3/body.go b/internal/http3/body.go index 169417c..e7e1042 100644 --- a/internal/http3/body.go +++ b/internal/http3/body.go
@@ -127,9 +127,12 @@ send100Continue func() // A map where the key represents the trailer header names we expect. If // there is a HEADERS frame after reading DATA frames to EOF, the value of - // the headers will be written here, provided that the name of the header - // exists in the map already. - trailer http.Header + // the headers will be written here. Keys in the map are assumed to be + // canonicalized. + // If filterTrailer is true, headers that are not already in the map will + // be ignored; otherwise, all headers will be added to the map. + trailer http.Header + filterTrailer bool } func (r *bodyReader) Read(p []byte) (n int, err error) { @@ -188,8 +191,16 @@ } var dec qpackDecoder if err := dec.decode(r.st, func(_ indexType, name, value string) error { + if r.trailer == nil { + return nil + } + if !validWireHeaderFieldName(name) || !httpguts.ValidHeaderFieldValue(name) { + return nil + } name = textproto.CanonicalMIMEHeaderKey(textproto.TrimString(name)) - if _, ok := r.trailer[name]; ok { + if !r.filterTrailer { + r.trailer.Add(name, value) + } else if _, ok := r.trailer[name]; ok { r.trailer.Add(name, value) } return nil
diff --git a/internal/http3/roundtrip_test.go b/internal/http3/roundtrip_test.go index 6c8b997..48f0a72 100644 --- a/internal/http3/roundtrip_test.go +++ b/internal/http3/roundtrip_test.go
@@ -557,13 +557,15 @@ req, _ = http.NewRequest("POST", "https://example.tld/", io.MultiReader( testReader{readFunc: func(_ []byte) (int, error) { req.Trailer["Client-Trailer-A"] = []string{"valuea"} - req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} // Should be ignored. + // Transport should not send undeclared trailer. + req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} return 0, io.EOF }}, strings.NewReader("a body"), testReader{readFunc: func(_ []byte) (int, error) { req.Trailer["Client-Trailer-B"] = []string{"valueb"} - req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} // Should be ignored. + // Transport should not send undeclared trailer. + req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} return 0, io.EOF }}, )) @@ -592,12 +594,14 @@ req, _ = http.NewRequest("POST", "https://example.tld/", io.MultiReader( testReader{readFunc: func(_ []byte) (int, error) { req.Trailer["Client-Trailer-A"] = []string{"valuea"} - req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} // Should be ignored. + // Transport should not send undeclared trailer. + req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} return 0, io.EOF }}, testReader{readFunc: func(_ []byte) (int, error) { req.Trailer["Client-Trailer-B"] = []string{"valueb"} - req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} // Should be ignored. + // Transport should not send undeclared trailer. + req.Trailer["Undeclared-Trailer"] = []string{"undeclared"} return 0, io.EOF }}, )) @@ -637,7 +641,7 @@ "server-trailer-a": {"valuea"}, // Note that Server-Trailer-B is skipped. "server-trailer-c": {"valuec"}, - "undeclared-trailer": {"undeclared"}, // Should be ignored. + "undeclared-trailer": {"undeclared"}, }) rt.wantStatus(200) @@ -655,6 +659,8 @@ "Server-Trailer-A": {"valuea"}, "Server-Trailer-B": nil, "Server-Trailer-C": {"valuec"}, + // Transport should accept undeclared trailers. + "Undeclared-Trailer": {"undeclared"}, }) st.wantClosed("request is complete") }) @@ -680,7 +686,7 @@ "server-trailer-a": {"valuea"}, // Note that Server-Trailer-B is skipped. "server-trailer-c": {"valuec"}, - "undeclared-trailer": {"undeclared"}, // Should be ignored. + "undeclared-trailer": {"undeclared"}, }) rt.wantStatus(200) @@ -698,6 +704,8 @@ "Server-Trailer-A": {"valuea"}, "Server-Trailer-B": nil, "Server-Trailer-C": {"valuec"}, + // Transport should accept undeclared trailers. + "Undeclared-Trailer": {"undeclared"}, }) st.wantClosed("request is complete") })
diff --git a/internal/http3/server.go b/internal/http3/server.go index cb3a5d7..72095ba 100644 --- a/internal/http3/server.go +++ b/internal/http3/server.go
@@ -11,6 +11,7 @@ "io" "maps" "net/http" + "net/textproto" "slices" "strconv" "strings" @@ -503,9 +504,10 @@ } if contentLength != 0 || len(reqInfo.Trailer) != 0 { body = &bodyReader{ - st: st, - remain: contentLength, - trailer: reqInfo.Trailer, + st: st, + remain: contentLength, + trailer: reqInfo.Trailer, + filterTrailer: true, } } else { body = http.NoBody @@ -578,6 +580,12 @@ return true } +// trailerPrefix is a magic prefix for [responseWriter.Header] map keys that, +// if present, signals that the map entry is actually for the response +// trailers, and not the response headers. See [net/http.TrailerPrefix] for +// details. +const trailerPrefix = "Trailer:" + type responseWriter struct { st *stream bw *bodyWriter @@ -608,6 +616,12 @@ delete(rw.trailer, name) } } + for name, vals := range rw.headers { + if name, found := strings.CutPrefix(name, trailerPrefix); found { + name = textproto.CanonicalMIMEHeaderKey(textproto.TrimString(name)) + rw.trailer[name] = vals + } + } if len(rw.trailer) > 0 { rw.bw.trailer = rw.trailer }
diff --git a/internal/http3/server_test.go b/internal/http3/server_test.go index dd1efb2..f407cbf 100644 --- a/internal/http3/server_test.go +++ b/internal/http3/server_test.go
@@ -798,9 +798,10 @@ })) reqStream.writeData(body) reqStream.writeHeaders(http.Header{ - "Client-Trailer-A": {"valuea"}, - "Client-Trailer-B": {"valueb"}, - "Undeclared-Trailer": {"undeclared"}, // Undeclared trailer should be ignored. + "Client-Trailer-A": {"valuea"}, + "Client-Trailer-B": {"valueb"}, + // Server should not accept undeclared trailers. + "Undeclared-Trailer": {"undeclared"}, }) reqStream.wantHeaders(nil) reqStream.wantClosed("request is complete") @@ -838,9 +839,10 @@ "content-length": {"0"}, })) reqStream.writeHeaders(http.Header{ - "Client-Trailer-A": {"valuea"}, - "Client-Trailer-B": {"valueb"}, - "Undeclared-Trailer": {"undeclared"}, // Undeclared trailer should be ignored. + "Client-Trailer-A": {"valuea"}, + "Client-Trailer-B": {"valueb"}, + // Server should not accept undeclared trailers. + "Undeclared-Trailer": {"undeclared"}, }) reqStream.wantHeaders(nil) reqStream.wantClosed("request is complete") @@ -858,7 +860,10 @@ w.Header().Set("server-trailer-a", "valuea") // Trailer header will be canonicalized. w.Header().Set("Server-Trailer-C", "valuec") // skipping B + // Server should not send undeclared trailers, unless it has the + // magic "Trailer:" prefix. w.Header().Set("Server-Trailer-Not-Declared", "should be omitted") + w.Header().Set("Trailer:Undeclared-Trailer-Exception", "should be sent") })) tc := ts.connect() tc.greet() @@ -871,8 +876,9 @@ }) reqStream.wantData(body) reqStream.wantSomeHeaders(http.Header{ - "Server-Trailer-A": {"valuea"}, - "Server-Trailer-C": {"valuec"}, + "Server-Trailer-A": {"valuea"}, + "Server-Trailer-C": {"valuec"}, + "Undeclared-Trailer-Exception": {"should be sent"}, }) reqStream.wantClosed("request is complete") }) @@ -888,7 +894,10 @@ w.Header().Set("server-trailer-a", "valuea") // Trailer header will be canonicalized. w.Header().Set("Server-Trailer-C", "valuec") // skipping B + // Server should not send undeclared trailers without "Trailer:" + // prefix. w.Header().Set("Server-Trailer-Not-Declared", "should be omitted") + w.Header().Set("Trailer:undeclared-trailer-exception", "should be sent") })) tc := ts.connect() tc.greet() @@ -900,8 +909,9 @@ "Trailer": {"Server-Trailer-A, Server-Trailer-B, Server-Trailer-C"}, }) reqStream.wantSomeHeaders(http.Header{ - "Server-Trailer-A": {"valuea"}, - "Server-Trailer-C": {"valuec"}, + "Server-Trailer-A": {"valuea"}, + "Server-Trailer-C": {"valuec"}, + "Undeclared-Trailer-Exception": {"should be sent"}, }) reqStream.wantClosed("request is complete") })