[net] internal/http3: reject response headers with invalid characters in RoundTrip
0 views
Skip to first unread message
Nicholas Husin (Gerrit)
Apr 28, 2026, 9:59:17 AM (19 hours ago) Apr 28
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin has uploaded the change for review![]( "Open in Gerrit")
Nicholas Husin would like Damien Neil to review this change.
Commit message
internal/http3: reject response headers with invalid characters in RoundTrip
Similar to our server implementation, RoundTrip should reject response
headers which contains invalid characters (e.g. uppercase characters and
control characters) when doing QPACK decoding.
For golang/go#70914
Change-Id: Ifa4065b709a30e72ce8a3fa4c64110b36a6a6964
Change diff
diff --git a/internal/http3/qpack.go b/internal/http3/qpack.go
index 64ce99a..8286191 100644
--- a/internal/http3/qpack.go
+++ b/internal/http3/qpack.go
@@ -8,6 +8,7 @@
"errors"
"io"
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/http2/hpack"
)
@@ -330,3 +331,26 @@
}
return b
}
+
+// validWireHeaderFieldName reports whether v is a valid header field
+// name (key). See httpguts.ValidHeaderName for the base rules.
+//
+// Further, http3 says:
+// "A request or response containing uppercase characters in field names MUST
+// be treated as malformed."
+//
+// This function does not validate whether a pseudo-header field name is valid.
+func validWireHeaderFieldName(v string) bool {
+ if len(v) == 0 {
+ return false
+ }
+ for _, r := range v {
+ if !httpguts.IsTokenRune(r) {
+ return false
+ }
+ if 'A' <= r && r <= 'Z' {
+ return false
+ }
+ }
+ return true
+}
diff --git a/internal/http3/roundtrip.go b/internal/http3/roundtrip.go
index 29e7016..edf5f13 100644
--- a/internal/http3/roundtrip.go
+++ b/internal/http3/roundtrip.go
@@ -345,6 +345,9 @@
// Issue #71374: Consider tracking the never-indexed status of headers
// with the N bit set in their QPACK encoding.
err = cc.dec.decode(st, func(_ indexType, name, value string) error {
+ if !httpguts.ValidHeaderFieldValue(value) {
+ return &streamError{errH3MessageError, "invalid field value"}
+ }
switch {
case name == ":status":
if haveStatus {
@@ -372,6 +375,9 @@
cookie += "; " + value
}
default:
+ if !validWireHeaderFieldName(name) {
+ return &streamError{errH3MessageError, "invalid field name"}
+ }
if h == nil {
h = make(http.Header)
}
diff --git a/internal/http3/roundtrip_test.go b/internal/http3/roundtrip_test.go
index 8d32221..6c8b997 100644
--- a/internal/http3/roundtrip_test.go
+++ b/internal/http3/roundtrip_test.go
@@ -185,22 +185,49 @@
}{{
name: "duplicate :status",
respHeader: http.Header{
- ":status": []string{"200", "204"},
+ ":status": {"200", "204"},
},
}, {
name: "unparsable :status",
respHeader: http.Header{
- ":status": []string{"frogpants"},
+ ":status": {"frogpants"},
},
}, {
name: "undefined pseudo-header",
respHeader: http.Header{
- ":status": []string{"200"},
- ":unknown": []string{"x"},
+ ":status": {"200"},
+ ":unknown": {"x"},
},
}, {
name: "no :status",
respHeader: http.Header{},
+ }, {
+ name: "header name with control character",
+ respHeader: http.Header{
+ ":status": {"200"},
+ "name\nevilinjection": {"Value"},
+ },
+ }, {
+ name: "header name with uppercase character",
+ respHeader: http.Header{
+ ":status": {"200"},
+ "nAme": {"Value"},
+ },
+ }, {
+ name: "pseudo-header name with control character",
+ respHeader: http.Header{":status\nevilinjection": {"200"}},
+ }, {
+ name: "pseudo-header name with uppercase character",
+ respHeader: http.Header{":stAtus": {"200"}},
+ }, {
+ name: "header value with control character",
+ respHeader: http.Header{
+ ":status": {"200"},
+ "name": {"Value\nEvilInjection"},
+ },
+ }, {
+ name: "pseudo-header value with control character",
+ respHeader: http.Header{":status": {"200\nEvilInjection"}},
}} {
synctestSubtest(t, test.name, func(t *testing.T) {
tc := newTestClientConn(t)
@@ -210,7 +237,7 @@
rt := tc.roundTrip(req)
st := tc.wantStream(streamTypeRequest)
st.wantHeaders(nil)
- st.writeHeaders(test.respHeader)
+ st.writeHeadersRaw(test.respHeader)
rt.wantError("malformed response")
})
}
@@ -501,7 +528,7 @@
st.wantHeaders(nil)
st.writeHeaders(http.Header{
":status": {"200"},
- "Content-Length": {"0"},
+ "content-length": {"0"},
})
rt.wantStatus(200)
st.wantClosed("request is complete")
@@ -514,7 +541,7 @@
st.wantHeaders(nil)
st.writeHeaders(http.Header{
":status": {"200"},
- "Content-Length": {"1000"},
+ "content-length": {"1000"},
})
rt.wantStatus(200)
st.wantClosed("request is complete")
@@ -602,15 +629,15 @@
st.wantHeaders(nil)
st.writeHeaders(http.Header{
":status": {"200"},
- "Trailer": {"Server-Trailer-A, Server-Trailer-B", "server-trailer-c"}, // Should be canonicalized.
+ "trailer": {"Server-Trailer-A, Server-Trailer-B", "server-trailer-c"}, // Should be canonicalized.
})
body := []byte("body from server")
st.writeData(body)
st.writeHeaders(http.Header{
- "Server-Trailer-A": {"valuea"},
+ "server-trailer-a": {"valuea"},
// Note that Server-Trailer-B is skipped.
- "Server-Trailer-C": {"valuec"},
- "Undeclared-Trailer": {"undeclared"}, // Should be ignored.
+ "server-trailer-c": {"valuec"},
+ "undeclared-trailer": {"undeclared"}, // Should be ignored.
})
rt.wantStatus(200)
@@ -646,14 +673,14 @@
st.wantHeaders(nil)
st.writeHeaders(http.Header{
":status": {"200"},
- "Content-Length": {"0"},
- "Trailer": {"Server-Trailer-A, Server-Trailer-B", "server-trailer-c"}, // Should be canonicalized.
+ "content-length": {"0"},
+ "trailer": {"Server-Trailer-A, Server-Trailer-B", "server-trailer-c"}, // Should be canonicalized.
})
st.writeHeaders(http.Header{
- "Server-Trailer-A": {"valuea"},
+ "server-trailer-a": {"valuea"},
// Note that Server-Trailer-B is skipped.
- "Server-Trailer-C": {"valuec"},
- "Undeclared-Trailer": {"undeclared"}, // Should be ignored.
+ "server-trailer-c": {"valuec"},
+ "undeclared-trailer": {"undeclared"}, // Should be ignored.
})
rt.wantStatus(200)
diff --git a/internal/http3/server.go b/internal/http3/server.go
index aaf1e70..f6a5ece 100644
--- a/internal/http3/server.go
+++ b/internal/http3/server.go
@@ -324,29 +324,6 @@
}
}
-// validWireHeaderFieldName reports whether v is a valid header field
-// name (key). See httpguts.ValidHeaderName for the base rules.
-//
-// Further, http3 says:
-// "A request or response containing uppercase characters in field names MUST
-// be treated as malformed."
-//
-// This function does not validate whether a pseudo-header field name is valid.
-func validWireHeaderFieldName(v string) bool {
- if len(v) == 0 {
- return false
- }
- for _, r := range v {
- if !httpguts.IsTokenRune(r) {
- return false
- }
- if 'A' <= r && r <= 'Z' {
- return false
- }
- }
- return true
-}
-
// hasDisallowedConnectionHeader reports whether h contains connnection headers
// that are not allowed in HTTP/3:
//
Change information
Files:
- M internal/http3/qpack.go
- M internal/http3/roundtrip.go
- M internal/http3/roundtrip_test.go
- M internal/http3/server.go
Change size: M
Delta: 4 files changed, 73 insertions(+), 39 deletions(-)
Related details
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Apr 28, 2026, 5:42:38 PM (12 hours ago) Apr 28
to Nicholas Husin, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Damien Neil
| Code-Review | +2 |
1 comment
File internal/http3/qpack.go
Line 336, Patchset 1 (Latest):
// name (key). See httpguts.ValidHeaderName for the base rules.Damien Neil . unresolved
`ValidHeaderFieldName` I think.
Related details
Attention is currently required from:
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
Apr 28, 2026, 5:47:54 PM (11 hours ago) Apr 28
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil and Nicholas Husin
Nicholas Husin uploaded new patchset![]( "Open in Gerrit")
Nicholas Husin uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- Code-Review: +2 by Damien Neil
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
Attention is currently required from:
- Damien Neil
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
Apr 28, 2026, 5:50:09 PM (11 hours ago) Apr 28
to goph...@pubsubhelper.golang.org, Damien Neil, golang...@luci-project-accounts.iam.gserviceaccount.com, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin added 1 comment![]( "Open in Gerrit")
File internal/http3/qpack.go
Line 336, Patchset 1:
// name (key). See httpguts.ValidHeaderName for the base rules.Damien Neil . resolved
Nicholas Husin`ValidHeaderFieldName` I think.
Oops, thanks. Also fixed it in the HTTP/2 version, from which I yanked this function.
Related details
Attention is currently required from:
- Damien Neil
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Apr 28, 2026, 7:59:16 PM (9 hours ago) Apr 28
to Nicholas Husin, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil voted Code-Review+2![]( "Open in Gerrit")
Attention is currently required from:
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages