From 0a5dcdd8587c55c948da7ba9a94ca3bf8f39d821 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 8 Jan 2025 10:01:53 -0800
Subject: [PATCH] http2: disable extended CONNECT by default

Browsers interpret a server advertising extended CONNECT support as
indicating the server supports WebSockets-over-HTTP/2.
However, WebSocket-over-HTTP/2 requires support from both the HTTP
implementation and the WebSocket implementation, and existing
Go WebSocket packages don't support HTTP/2.

Disable extended CONNECT support by default, since advertising it
is a non-backwards-compatible change.

For golang/go#71128
For golang/go#49918

Change-Id: Ie7d3ee2cd48124836a00bad320752e78719ffc46
Reviewed-on: https://go-review.googlesource.com/c/net/+/641475
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 http2/http2.go          | 22 +++++++++++++++-------
 http2/http2_test.go     |  9 +++++++++
 http2/transport_test.go |  4 ++--
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/http2/http2.go b/http2/http2.go
index c7601c909..a898b5087 100644
--- a/http2/http2.go
+++ b/http2/http2.go
@@ -34,11 +34,19 @@ import (
 )
 
 var (
-	VerboseLogs                    bool
-	logFrameWrites                 bool
-	logFrameReads                  bool
-	inTests                        bool
-	disableExtendedConnectProtocol bool
+	VerboseLogs    bool
+	logFrameWrites bool
+	logFrameReads  bool
+	inTests        bool
+
+	// Enabling extended CONNECT by causes browsers to attempt to use
+	// WebSockets-over-HTTP/2. This results in problems when the server's websocket
+	// package doesn't support extended CONNECT.
+	//
+	// Disable extended CONNECT by default for now.
+	//
+	// Issue #71128.
+	disableExtendedConnectProtocol = true
 )
 
 func init() {
@@ -51,8 +59,8 @@ func init() {
 		logFrameWrites = true
 		logFrameReads = true
 	}
-	if strings.Contains(e, "http2xconnect=0") {
-		disableExtendedConnectProtocol = true
+	if strings.Contains(e, "http2xconnect=1") {
+		disableExtendedConnectProtocol = false
 	}
 }
 
diff --git a/http2/http2_test.go b/http2/http2_test.go
index b1e71f153..c7774133a 100644
--- a/http2/http2_test.go
+++ b/http2/http2_test.go
@@ -284,6 +284,15 @@ func TestNoUnicodeStrings(t *testing.T) {
 	}
 }
 
+// setForTest sets *p = v, and restores its original value in t.Cleanup.
+func setForTest[T any](t *testing.T, p *T, v T) {
+	orig := *p
+	t.Cleanup(func() {
+		*p = orig
+	})
+	*p = v
+}
+
 // must returns v if err is nil, or panics otherwise.
 func must[T any](v T, err error) T {
 	if err != nil {
diff --git a/http2/transport_test.go b/http2/transport_test.go
index b2e6fc59d..2cc4ad248 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -5856,7 +5856,7 @@ func TestTransportTLSNextProtoConnImmediateFailureUnused(t *testing.T) {
 }
 
 func TestExtendedConnectClientWithServerSupport(t *testing.T) {
-	disableExtendedConnectProtocol = false
+	setForTest(t, &disableExtendedConnectProtocol, false)
 	ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
 		if r.Header.Get(":protocol") != "extended-connect" {
 			t.Fatalf("unexpected :protocol header received")
@@ -5892,7 +5892,7 @@ func TestExtendedConnectClientWithServerSupport(t *testing.T) {
 }
 
 func TestExtendedConnectClientWithoutServerSupport(t *testing.T) {
-	disableExtendedConnectProtocol = true
+	setForTest(t, &disableExtendedConnectProtocol, true)
 	ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
 		io.Copy(w, r.Body)
 	})