[go] net/http: Ensure that CONNECT proxied requests respects MaxResponseHeaderBytes
7 views
Skip to first unread message
Nicholas Husin (Gerrit)
Aug 25, 2025, 9:16:44 AM8/25/25
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Nicholas Husin has uploaded the change for review![]( "Open in Gerrit")
Commit message
net/http: Ensure that CONNECT proxied requests respects MaxResponseHeaderBytes
Currently, CONNECT proxied requests uses an unlimited Reader. As a
result, a malicious or misbehaving proxy server can send an unlimited
number of bytes to a client; causing the client to indefinitely receive bytes
until it runs out of memory.
To prevent this, we now use a LimitedReader that limits the number of
bytes according to MaxResponseHeaderBytes in Transport. If
MaxResponseHeaderBytes is not provided, we use the default value of 10
MB that has historically been used (see golang/go#26315).
Fixes golang/go#74633
Change-Id: I0b03bb354139dbc64318874402f7f29cc0fb42ce
Change diff
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index b860eb9..572c16a 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -325,6 +325,13 @@
return 4 << 10
}
+func (t *Transport) maxHeaderResponseSize() int64 {
+ if t.MaxResponseHeaderBytes > 0 {
+ return t.MaxResponseHeaderBytes
+ }
+ return 10 << 20 // conservative default; same as http2
+}
+
// Clone returns a deep copy of t's exported fields.
func (t *Transport) Clone() *Transport {
t.nextProtoOnce.Do(t.onceSetNextProtoDefaults)
@@ -1871,7 +1878,7 @@
}
// Okay to use and discard buffered reader here, because
// TLS server will not speak until spoken to.
- br := bufio.NewReader(conn)
+ br := bufio.NewReader(&io.LimitedReader{R: conn, N: t.maxHeaderResponseSize()})
resp, err = ReadResponse(br, connectReq)
}()
select {
@@ -2108,10 +2115,7 @@
}
func (pc *persistConn) maxHeaderResponseSize() int64 {
- if v := pc.t.MaxResponseHeaderBytes; v != 0 {
- return v
- }
- return 10 << 20 // conservative default; same as http2
+ return pc.t.maxHeaderResponseSize()
}
func (pc *persistConn) Read(p []byte) (n int, err error) {
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 9762f05..f7b6d58 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -1550,6 +1550,68 @@
}
}
+// Issue 74633: verify that a client will not indefinitely read a response from
+// a proxy server that writes an infinite byte of stream, rather than
+// responding with 200 OK.
+func TestProxyWithInfiniteHeader(t *testing.T) {
+ defer afterTest(t)
+
+ ln := newLocalListener(t)
+ defer ln.Close()
+ cancelc := make(chan struct{})
+ defer close(cancelc)
+
+ // Simulate a malicious / misbehaving proxy that writes an unlimited number
+ // of bytes rather than responding with 200 OK.
+ go func() {
+ c, err := ln.Accept()
+ if err != nil {
+ t.Errorf("Accept: %v", err)
+ return
+ }
+ defer c.Close()
+ // Read the CONNECT request
+ br := bufio.NewReader(c)
+ cr, err := ReadRequest(br)
+ if err != nil {
+ t.Errorf("proxy server failed to read CONNECT request")
+ return
+ }
+ if cr.Method != "CONNECT" {
+ t.Errorf("unexpected method %q", cr.Method)
+ return
+ }
+
+ // Keep writing bytes until the test exits.
+ for {
+ select {
+ case <-cancelc:
+ return
+ default:
+ c.Write([]byte("infinite stream of bytes"))
+ }
+ }
+ }()
+
+ c := &Client{
+ Transport: &Transport{
+ Proxy: func(*Request) (*url.URL, error) {
+ return url.Parse("http://" + ln.Addr().String())
+ },
+ // Limit MaxResponseHeaderBytes so the test returns quicker.
+ MaxResponseHeaderBytes: 1024,
+ },
+ }
+ req, err := NewRequest("GET", "https://golang.fake.tld/", nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ _, err = c.Do(req)
+ if err == nil {
+ t.Errorf("unexpected Get success")
+ }
+}
+
func TestOnProxyConnectResponse(t *testing.T) {
var tcases = []struct {
Change information
Files:
- M src/net/http/transport.go
- M src/net/http/transport_test.go
Change size: M
Delta: 2 files changed, 71 insertions(+), 5 deletions(-)
Related details
Attention set is empty
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. |
Gopher Robot (Gerrit)
Aug 25, 2025, 9:19:07 AM8/25/25
to Nicholas Husin, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Message from Gopher Robot![]( "Open in Gerrit")
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Related details
Attention set is empty
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)
Aug 25, 2025, 9:21:54 AM8/25/25
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Nicholas Husin uploaded new patchset![]( "Open in Gerrit")
Nicholas Husin uploaded patch set #2 to this change.
Related details
Attention set is empty
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)
Aug 26, 2025, 7:57:34 PM8/26/25
to Nicholas Husin, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil voted and added 2 comments![]( "Open in Gerrit")
Votes added by Damien Neil
| Code-Review | +2 |
| Commit-Queue | +1 |
2 comments
Patchset-level comments
Commit Message
Line 19, Patchset 2 (Latest):
Fixes golang/go#74633Damien Neil . unresolved
Just: "Fixes #74633"
The "golang/$REPONAME" is only necessary for repos other than the main go one.
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)
Aug 26, 2025, 10:05:08 PM8/26/25
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 #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
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)
Aug 26, 2025, 10:09:18 PM8/26/25
to goph...@pubsubhelper.golang.org, Go LUCI, Damien Neil, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin added 2 comments![]( "Open in Gerrit")
Patchset-level comments
Damien Neil . resolved
Nice.
Nicholas Husin
Thanks!
I think I need you to trigger TryBot again unfortunately. My newly added test was livelocking in WASM environment because the infinite byte writes was hogging the single thread. Should be fixed now with `runtime.Gosched()`.
Commit Message
Just: "Fixes #74633"
The "golang/$REPONAME" is only necessary for repos other than the main go one.
Nicholas Husin
Done, thanks!
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. |
Nicholas Husin (Gerrit)
Aug 26, 2025, 10:26:42 PM8/26/25
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin uploaded new patchset![]( "Open in Gerrit")
Nicholas Husin uploaded patch set #4 to this change.
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. |
Markus Kusano (Gerrit)
Aug 27, 2025, 3:27:57 PM8/27/25
to Nicholas Husin, goph...@pubsubhelper.golang.org, Go LUCI, Damien Neil, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil and Nicholas Husin
Markus Kusano voted Commit-Queue+1![]( "Open in Gerrit")
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. |
Markus Kusano (Gerrit)
Aug 27, 2025, 4:40:22 PM8/27/25
to Nicholas Husin, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Damien Neil, Gopher Robot, golang-co...@googlegroups.com
Markus Kusano submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/net/http/transport_test.go
Insertions: 4, Deletions: 0.
@@ -1584,6 +1584,10 @@
// Keep writing bytes until the test exits.
for {
+ // runtime.Gosched() is needed here. Otherwise, this test might
+ // livelock in environments like WASM, where the one single thread
+ // we have could be hogged by the infinite loop of writing bytes.
+ runtime.Gosched()
select {
case <-cancelc:
return
```
Change information
Commit message:
net/http: Ensure that CONNECT proxied requests respect MaxResponseHeaderBytes
Currently, CONNECT proxied requests use an unlimited Reader. As a
result, a malicious or misbehaving proxy server can send an unlimited
number of bytes to a client; causing the client to indefinitely receive bytes
until it runs out of memory.
To prevent this, we now use a LimitedReader that limits the number of
bytes according to MaxResponseHeaderBytes in Transport. If
MaxResponseHeaderBytes is not provided, we use the default value of 10
MB that has historically been used (see #26315).
Fixes #74633
Change-Id: I0b03bb354139dbc64318874402f7f29cc0fb42ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/698915
Reviewed-by: Damien Neil <dn...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Files:
- M src/net/http/transport.go
- M src/net/http/transport_test.go
Change size: M
Delta: 2 files changed, 75 insertions(+), 5 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Damien Neil
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages