[net] internal/http3: validate pseudo-header counts in server

0 views
Skip to first unread message

Nicholas Husin (Gerrit)

unread,
Apr 28, 2026, 9:59:18 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

Nicholas Husin would like Damien Neil to review this change.

Commit message

internal/http3: validate pseudo-header counts in server

Following RFC 9114, CONNECT requests must have exactly one value for
:method, :scheme, and :path pseudo-headers. CONNECT requests, on the
other hand, should omit :scheme and :path, with only one :authority
pseudo-header.

For golang/go#70914
Change-Id: I7e240bc6997498b96c4f5cc0ecd3dbbf6a6a6964

Change diff

diff --git a/internal/http3/server.go b/internal/http3/server.go
index 57f6a34..aaf1e70 100644
--- a/internal/http3/server.go
+++ b/internal/http3/server.go
@@ -392,12 +392,16 @@
 		return nil, pseudoHeader{}, &streamError{errH3MessageError, "received other frames when expecting HEADERS"}
 	}
 	header := make(http.Header)
+	pHeaderCount := make(map[string]int)
 	var pHeader pseudoHeader
 	var dec qpackDecoder
 	if err := dec.decode(st, func(_ indexType, name, value string) error {
 		if !httpguts.ValidHeaderFieldValue(value) {
 			return &streamError{errH3MessageError, "invalid field value"}
 		}
+		if strings.HasPrefix(name, ":") {
+			pHeaderCount[name]++
+		}
 		switch name {
 		case ":method":
 			pHeader.method = value
@@ -423,6 +427,32 @@
 	if hasDisallowedConnectionHeader(header) {
 		return nil, pseudoHeader{}, &streamError{errH3MessageError, "invalid connection-related header"}
 	}
+
+	// "All HTTP/3 requests MUST include exactly one value for the :method,
+	// :scheme, and :path pseudo-header fields, unless the request is a CONNECT
+	// request"
+	//
+	// "A CONNECT request MUST be constructed as follows:
+	// - The :method pseudo-header field is set to "CONNECT"
+	// - The :scheme and :path pseudo-header fields are omitted
+	// - The :authority pseudo-header field contains the host and port to connect to"
+	invalidPHeaderErr := &streamError{errH3MessageError, "invalid pseudo-header"}
+	methodCount := pHeaderCount[":method"]
+	schemeCount := pHeaderCount[":scheme"]
+	pathCount := pHeaderCount[":path"]
+	authCount := pHeaderCount[":authority"]
+	if methodCount != 1 {
+		return nil, pseudoHeader{}, invalidPHeaderErr
+	}
+	if pHeader.method != "CONNECT" {
+		if schemeCount != 1 || pathCount != 1 {
+			return nil, pseudoHeader{}, invalidPHeaderErr
+		}
+	} else {
+		if schemeCount != 0 || pathCount != 0 || authCount != 1 {
+			return nil, pseudoHeader{}, invalidPHeaderErr
+		}
+	}
 	return header, pHeader, nil
 }
 
diff --git a/internal/http3/server_test.go b/internal/http3/server_test.go
index eb1989a..acb8025 100644
--- a/internal/http3/server_test.go
+++ b/internal/http3/server_test.go
@@ -235,6 +235,95 @@
 	})
 }
 
+func TestServerPseudoHeaderCount(t *testing.T) {
+	tests := []struct {
+		name      string
+		header    http.Header
+		wantError bool
+	}{
+		{
+			name: "missing method pseudo-header",
+			header: http.Header{
+				":scheme":    {"https"},
+				":path":      {"/"},
+				":authority": {"fake.tld:1234"},
+			},
+			wantError: true,
+		},
+		{
+			name: "valid pseudo-headers for non-CONNECT request",
+			header: http.Header{
+				":method": {"GET"},
+				":scheme": {"https"},
+				":path":   {"/"},
+			},
+			wantError: false,
+		},
+		{
+			name: "extraneous pseudo-headers for non-CONNECT request",
+			header: http.Header{
+				":method": {"GET", "GET"}, // Duplicate :method.
+				":scheme": {"https"},
+				":path":   {"/"},
+			},
+			wantError: true,
+		},
+		{
+			name: "missing pseudo-headers for non-CONNECT request",
+			header: http.Header{
+				":method": {"GET", "GET"},
+				":path":   {"/"},
+			},
+			wantError: true,
+		},
+		{
+			name: "valid pseudo-headers for CONNECT request",
+			header: http.Header{
+				":method":    {"CONNECT"},
+				":authority": {"fake.tld:1234"},
+			},
+			wantError: false,
+		},
+		{
+			name: "extraneous pseudo-headers for CONNECT request",
+			header: http.Header{
+				":method":    {"CONNECT"},
+				":authority": {"fake.tld:1234"},
+				":path":      {"/"}, // :path should be omitted.
+			},
+			wantError: true,
+		},
+		{
+			name: "missing pseudo-headers for CONNECT request",
+			header: http.Header{
+				":method": {"CONNECT"},
+			},
+			wantError: true,
+		},
+	}
+	for _, tt := range tests {
+		synctestSubtest(t, tt.name, func(t *testing.T) {
+			body := []byte("some data")
+			ts := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Write(body)
+			}))
+			tc := ts.connect()
+			tc.greet()
+
+			reqStream := tc.newStream(streamTypeRequest)
+			reqStream.writeHeaders(tt.header)
+
+			if tt.wantError {
+				reqStream.wantError(quic.StreamErrorCode(errH3MessageError))
+			} else {
+				reqStream.wantHeaders(nil)
+				reqStream.wantData(body)
+				reqStream.wantClosed("request is complete")
+			}
+		})
+	}
+}
+
 func TestServerInvalidHeader(t *testing.T) {
 	synctest.Test(t, func(t *testing.T) {
 		ts := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

Change information

Files:
  • M internal/http3/server.go
  • M internal/http3/server_test.go
Change size: M
Delta: 2 files changed, 119 insertions(+), 0 deletions(-)
Open in Gerrit

Related details

Attention is currently required from:
  • Damien Neil
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: net
Gerrit-Branch: master
Gerrit-Change-Id: I7e240bc6997498b96c4f5cc0ecd3dbbf6a6a6964
Gerrit-Change-Number: 771421
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
Gerrit-Attention: Damien Neil <dn...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Damien Neil (Gerrit)

unread,
Apr 28, 2026, 5:32:39 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 added 1 comment

File internal/http3/server.go
Line 395, Patchset 1 (Latest): pHeaderCount := make(map[string]int)
Damien Neil . unresolved

Let's not create a transient map on each request. I'm not sure if escape analysis avoids putting this on the heap or not, but it's probably not very efficient. And we don't care about the number of headers, just whether each is present.

It might be a little bit more code, but a collection of bools should be efficient: `hasMethod`, `hasScheme`, etc.

Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Husin
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: I7e240bc6997498b96c4f5cc0ecd3dbbf6a6a6964
    Gerrit-Change-Number: 771421
    Gerrit-PatchSet: 1
    Gerrit-Owner: Nicholas Husin <n...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
    Gerrit-Attention: Nicholas Husin <n...@golang.org>
    Gerrit-Comment-Date: Tue, 28 Apr 2026 21:32:36 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Nicholas Husin (Gerrit)

    unread,
    Apr 28, 2026, 5:47:52 PM (11 hours ago) Apr 28
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Nicholas Husin

    Nicholas Husin uploaded new patchset

    Nicholas Husin uploaded patch set #2 to this change.
    Following approvals got outdated and were removed:
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Nicholas Husin
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: newpatchset
      Gerrit-Project: net
      Gerrit-Branch: master
      Gerrit-Change-Id: I7e240bc6997498b96c4f5cc0ecd3dbbf6a6a6964
      Gerrit-Change-Number: 771421
      Gerrit-PatchSet: 2
      unsatisfied_requirement
      open
      diffy

      Nicholas Husin (Gerrit)

      unread,
      Apr 28, 2026, 5:49:22 PM (11 hours ago) Apr 28
      to goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Damien Neil, golang-co...@googlegroups.com
      Attention needed from Damien Neil

      Nicholas Husin added 1 comment

      File internal/http3/server.go
      Line 395, Patchset 1: pHeaderCount := make(map[string]int)
      Damien Neil . resolved

      Let's not create a transient map on each request. I'm not sure if escape analysis avoids putting this on the heap or not, but it's probably not very efficient. And we don't care about the number of headers, just whether each is present.

      It might be a little bit more code, but a collection of bools should be efficient: `hasMethod`, `hasScheme`, etc.

      Nicholas Husin

      Sounds good, done!

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Damien Neil
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: net
        Gerrit-Branch: master
        Gerrit-Change-Id: I7e240bc6997498b96c4f5cc0ecd3dbbf6a6a6964
        Gerrit-Change-Number: 771421
        Gerrit-PatchSet: 2
        Gerrit-Owner: Nicholas Husin <n...@golang.org>
        Gerrit-Reviewer: Damien Neil <dn...@google.com>
        Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
        Gerrit-Attention: Damien Neil <dn...@google.com>
        Gerrit-Comment-Date: Tue, 28 Apr 2026 21:49:19 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Damien Neil <dn...@google.com>
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy

        Damien Neil (Gerrit)

        unread,
        Apr 28, 2026, 7:58:59 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

        Code-Review+2
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Nicholas Husin
        Submit Requirements:
        • requirement satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: net
        Gerrit-Branch: master
        Gerrit-Change-Id: I7e240bc6997498b96c4f5cc0ecd3dbbf6a6a6964
        Gerrit-Change-Number: 771421
        Gerrit-PatchSet: 2
        Gerrit-Owner: Nicholas Husin <n...@golang.org>
        Gerrit-Reviewer: Damien Neil <dn...@google.com>
        Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
        Gerrit-Attention: Nicholas Husin <n...@golang.org>
        Gerrit-Comment-Date: Tue, 28 Apr 2026 23:58:55 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages