[go] net/http: apply header timeout to server's unencrypted HTTP/2 check
3 views
Skip to first unread message
Damien Neil (Gerrit)
Jun 29, 2026, 7:03:50 PM (yesterday) Jun 29
to goph...@pubsubhelper.golang.org, Nicholas Husin, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
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. |
Damien Neil (Gerrit)
Jun 29, 2026, 7:37:28 PM (yesterday) Jun 29
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil uploaded new patchset![]( "Open in Gerrit")
Damien Neil uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
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. |
Damien Neil (Gerrit)
Jun 29, 2026, 7:50:08 PM (yesterday) Jun 29
to goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, golang-co...@googlegroups.com
Attention needed from Nicholas Husin
Damien Neil voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
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)
2:46 PM (8 hours ago) 2:46 PM
to Damien Neil, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin voted and added 2 comments![]( "Open in Gerrit")
Votes added by Nicholas Husin
| Code-Review | +2 |
2 comments
File src/net/http/server.go
Line 1006, Patchset 2 (Latest):
var (
wholeReqDeadline time.Time // or zero if none
)Nicholas Husin . unresolved
nit: Just `var wholeReqDeadline time.Time` now?
Line 1068, Patchset 2 (Latest):
// Adjust the read deadline if necessary.Nicholas Husin . unresolved
nit: "if necessary" is no longer accurate now I think?
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)
2:46 PM (8 hours ago) 2:46 PM
to Damien Neil, goph...@pubsubhelper.golang.org, Nicholas Husin, golang...@luci-project-accounts.iam.gserviceaccount.com, golang-co...@googlegroups.com
Attention needed from Damien Neil
Nicholas Husin voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
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)
5:09 PM (6 hours ago) 5:09 PM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil
Damien Neil uploaded new patchset![]( "Open in Gerrit")
Damien Neil uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
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)
5:10 PM (6 hours ago) 5:10 PM
to goph...@pubsubhelper.golang.org, Nicholas Husin, Nicholas Husin, golang...@luci-project-accounts.iam.gserviceaccount.com, golang-co...@googlegroups.com
Damien Neil voted and added 2 comments![]( "Open in Gerrit")
Votes added by Damien Neil
| Auto-Submit | +1 |
| Commit-Queue | +1 |
wholeReqDeadline time.Time // or zero if none
)Nicholas Husin . resolved
nit: Just `var wholeReqDeadline time.Time` now?
Damien Neil
Done
nit: "if necessary" is no longer accurate now I think?
Damien Neil
Done
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. |
Emmanuel Odeke (Gerrit)
6:04 PM (5 hours ago) 6:04 PM
to Damien Neil, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com
Attention needed from Damien Neil
Emmanuel Odeke added 4 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Emmanuel Odeke . resolved
Thank you Damien. I’ve left a few comments.
File src/net/http/serve_test.go
Line 860, Patchset 3 (Latest):
}Emmanuel Odeke . unresolved
1s timeout but the tests are in milliseconds and none of them including “header read expires” amount to more than the timeout, can we be explicit perhaps on the timeout?
Line 871, Patchset 3 (Latest):
t.Errorf("ReadAll from server: %v, want EOF", err)Emmanuel Odeke . unresolved
“want EOF” is very confusing, sure you want to read until the end of file but we have a common sentinel error EOF that isn’t relevant to this context. Perhaps: “ReadAll from server unexpectedly failed: %v”
Line 873, Patchset 3 (Latest):
if got, want := time.Since(start), srv.ReadHeaderTimeout; got != want {Emmanuel Odeke . unresolved
This seems like a fickle check that’ll flake, perhaps got < want? Is it because of synctest and fake time?
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)
6:40 PM (4 hours ago) 6:40 PM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil
Damien Neil uploaded new patchset![]( "Open in Gerrit")
Damien Neil uploaded patch set #4 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by golang...@luci-project-accounts.iam.gserviceaccount.com
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)
6:40 PM (4 hours ago) 6:40 PM
to goph...@pubsubhelper.golang.org, Emmanuel Odeke, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com
Attention needed from Emmanuel Odeke
Damien Neil voted and added 3 comments![]( "Open in Gerrit")
Votes added by Damien Neil
| Commit-Queue | +1 |
1s timeout but the tests are in milliseconds and none of them including “header read expires” amount to more than the timeout, can we be explicit perhaps on the timeout?
Damien Neil
The purpose of the sleeps in the tests is to demonstrate that they can't cause the timeout to extend past the defined 1s.
“want EOF” is very confusing, sure you want to read until the end of file but we have a common sentinel error EOF that isn’t relevant to this context. Perhaps: “ReadAll from server unexpectedly failed: %v”
Damien Neil
We should read an EOF from the server (which ReadAll reports as nil).
Line 873, Patchset 3:
if got, want := time.Since(start), srv.ReadHeaderTimeout; got != want {Emmanuel Odeke . resolved
This seems like a fickle check that’ll flake, perhaps got < want? Is it because of synctest and fake time?
Damien Neil
Not flaky. This runs in a synctest bubble with a fake clock; the timeout happens at precisely 1s or there's a bug.
Note that outside of a synctest bubble this would never pass, time.Since would always report that additional nanoseconds have passed since the timeout occurred.
Related details
Attention is currently required from:
- Emmanuel Odeke
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)
6:40 PM (4 hours ago) 6:40 PM
to goph...@pubsubhelper.golang.org, Emmanuel Odeke, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com
Attention needed from Emmanuel Odeke
Damien Neil voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Related details
Attention is currently required from:
- Emmanuel Odeke
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)
7:12 PM (4 hours ago) 7:12 PM
to Damien Neil, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, golang...@luci-project-accounts.iam.gserviceaccount.com, Emmanuel Odeke, Nicholas Husin, Nicholas Husin, golang-co...@googlegroups.com
Gopher Robot 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/server.go
Insertions: 1, Deletions: 4.
@@ -570,11 +570,30 @@
}
}
-// disableWriteContinue stops Request.Body.Read from sending an automatic 100-Continue.
-// If a 100-Continue is being written, it waits for it to complete before continuing.
-func (w *response) disableWriteContinue() {
+// disableWriteContinue stops Request.Body.Read from sending an automatic
+// 100 Continue. As the name implies, it is only useful when the request
+// expects a 100 Continue and the body is wrapped in an expectContinueReader;
+// otherwise, it is a no-op.
+// If a 100-Continue is being written, it waits for it to complete before
+// continuing. If skipDrain is true, it also prevents the server from draining
+// the request body and flags the connection to be closed after the reply, as
+// the client will never send the body.
+func (w *response) disableWriteContinue(skipDrain bool) {
+ ecr, ok := w.reqBody.(*expectContinueReader)
+ if !ok {
+ return
+ }
w.writeContinueMu.Lock()
- w.canWriteContinue.Store(false)
+ if w.canWriteContinue.Load() {
+ w.canWriteContinue.Store(false)
+ if skipDrain {
+ // Make sure that the connection will not be reused by sending
+ // "Connection: close" header in the response.
+ w.closeAfterReply = true
+ // Ensure that the body will not be drained in Close.
+ ecr.closed.Store(true)
+ }
+ }
w.writeContinueMu.Unlock()
}
@@ -983,7 +1002,12 @@
}
func (ecr *expectContinueReader) Close() error {
- ecr.closed.Store(true)
+ if ecr.resp.canWriteContinue.Load() {
+ ecr.resp.disableWriteContinue(true)
+ }
+ if ecr.closed.Swap(true) {
+ return nil
+ }
return ecr.readCloser.Close()
}
@@ -1003,10 +1027,8 @@
return nil, ErrHijacked
}
- var (
- wholeReqDeadline time.Time // or zero if none
- )
t0 := time.Now()
+ var wholeReqDeadline time.Time // or zero if none
if d := c.server.ReadTimeout; d > 0 {
wholeReqDeadline = t0.Add(d)
}
@@ -1065,7 +1087,6 @@
body.doEarlyClose = true
}
- // Adjust the read deadline if necessary.
c.rwc.SetReadDeadline(wholeReqDeadline)
w = &response{
@@ -1178,10 +1199,12 @@
}
checkWriteHeaderCode(code)
- if code < 101 || code > 199 {
- // Sending a 100 Continue or any non-1xx header disables the
- // automatically-sent 100 Continue from Request.Body.Read.
- w.disableWriteContinue()
+ // Sending a 100 Continue or any non-1XX header disables the
+ // automatically-sent 100 Continue from Request.Body.Read. If it is a final
+ // response (200 or higher), we skip draining the request body, which the
+ // client will never send.
+ if code == 100 || code >= 200 {
+ w.disableWriteContinue(code >= 200)
}
// Handle informational headers.
@@ -1656,7 +1679,7 @@
if w.canWriteContinue.Load() {
// Body reader wants to write 100 Continue but hasn't yet. Tell it not to.
- w.disableWriteContinue()
+ w.disableWriteContinue(true)
}
if !w.wroteHeader {
@@ -1694,6 +1717,10 @@
w.conn.r.abortPendingRead()
+ if w.canWriteContinue.Load() {
+ w.disableWriteContinue(true)
+ }
+
// Close the body (regardless of w.closeAfterReply) so we can
// re-use its bufio.Reader later safely.
w.reqBody.Close()
@@ -1924,7 +1951,7 @@
}
if inFlightResponse != nil {
inFlightResponse.cancelCtx()
- inFlightResponse.disableWriteContinue()
+ inFlightResponse.disableWriteContinue(true)
}
if !c.hijacked() {
if inFlightResponse != nil {
@@ -2091,6 +2118,7 @@
// Wrap the Body reader with one that replies on the connection
req.Body = &expectContinueReader{readCloser: req.Body, resp: w}
w.canWriteContinue.Store(true)
+ w.reqBody = req.Body
}
} else if req.Header.get("Expect") != "" {
w.sendExpectationFailed()
@@ -2258,7 +2286,7 @@
if w.handlerDone.Load() {
panic("net/http: Hijack called after ServeHTTP finished")
}
- w.disableWriteContinue()
+ w.disableWriteContinue(false)
if w.wroteHeader {
w.cw.flush()
}
```
Change information
Commit message:
net/http: apply header timeout to server's unencrypted HTTP/2 check
When a server is configured to support unencrypted HTTP/2,
it reads a few bytes from each new connection to see if they
contain the HTTP/2 client preface. This read was being done
with no timeout applied. Apply the header timeout (since this
is essentially reading the first headers from the connection).
Hoist the header timeout into the server serve loop so we can
use a single timeout to cover both the HTTP/2 preface and
the first HTTP/1 headers.
Thanks to Vsevolod Naumov and Ainar Garipov from AdGuard for reporting this issue.
Fixes #80205
Fixes CVE-2026-56853
Change-Id: I69943a79453967bc2f70da5b5ceeac936a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/795540
Reviewed-by: Nicholas Husin <n...@golang.org>
LUCI-TryBot-Result: golang...@luci-project-accounts.iam.gserviceaccount.com <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <hu...@google.com>
Auto-Submit: Damien Neil <dn...@google.com>
Files:
- M src/net/http/serve_test.go
- M src/net/http/server.go
Change size: M
Delta: 2 files changed, 68 insertions(+), 13 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Nicholas Husin, +2 by Nicholas Husin
TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages