net/http, x/net/http2: page fails to load with http2 server push due to underflow #63511

MagicalTux · 2023-10-12T08:11:12Z

What version of Go are you using (`go version`)?

$ go version
go version go1.21.3 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/magicaltux/.cache/go-build'
GOENV='/home/magicaltux/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/magicaltux/go/pkg/mod'
GONOPROXY='git.atonline.com'
GONOSUMDB='git.atonline.com'
GOOS='linux'
GOPATH='/home/magicaltux/go'
GOPRIVATE='git.atonline.com'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/pkg/main/dev-lang.go.dev.1.21.3.linux.amd64'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/pkg/main/dev-lang.go.dev.1.21.3.linux.amd64/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.3'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/magicaltux/projects/platform-fe/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1840746337=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Running a http server using http.Pusher to push a specific resource to the brower.

We updated in emergency to go1.21.3 following the http2 fast reset issue, this caused all the sites we use to fail to load on firefox and safari until we commented the use of http.Pusher.

I think this looks suspiciously a lot similar to #17777

What did you expect to see?

Page should work fine.

What did you see instead?

Only the initial page loads, and somehow firefox refuses to load the rest, either remaining blank or erroring with NS_BINDING_ABORTED. Firefox didn't provide any useful information to debug this issue, but not using http.Pusher fixed it.

The text was updated successfully, but these errors were encountered:

MagicalTux · 2023-10-12T09:20:24Z

I've looked at the code and in case the stream has been reset before runHandler was called the whole thing is simply ignored. I'm guessing this is the same issue as in #17777 where firefox/safari don't like HTTP servers that do not respond to RST.

See: 24ae2d9#diff-3ed0cda0a3f5adc3e3db797faf92c20e94be976c09023410295d04d9ae946d98R6151

seankhliao · 2023-10-12T11:03:18Z

cc @neild

mauri870 · 2023-10-15T23:13:55Z

Looks like there is an underflow in sc.curHandlers. It should always be incremented before calling runHandler otherwise when we decrement it in handlerDone we end up with an underflow. Seems like we missed one of these cases where we should increment it.

mauri870 · 2023-10-15T23:14:17Z

@gopherbot, please backport to Go 1.21.4

gopherbot · 2023-10-15T23:14:51Z

Backport issue(s) opened: #63560 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

mauri870 · 2023-10-15T23:19:48Z

@gopherbot, please backport to Go 1.20.11

After CL534295 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers. There is one place that calls runHandler without incrementing curHandlers. Fixes golang#63511

gopherbot · 2023-10-15T23:34:10Z

Change https://go.dev/cl/535575 mentions this issue: net/http: fix underflow when handling http2 done message

mauri870 · 2023-10-15T23:54:04Z

@MagicalTux Could you please update the issue title to include x/net/http2 alongside net/http? Thanks

After CL 534215 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers in handlerDone. There is one place that calls runHandler without incrementing curHandlers. Seems to only affect http.Pusher. For golang/go#63511

mauri870 · 2023-10-16T00:00:19Z

@gopherbot add NeedsFix, release-blocker

gopherbot · 2023-10-16T00:08:55Z

Change https://go.dev/cl/535595 mentions this issue: http2: fix underflow in http2 server push

mauri870 · 2023-10-19T15:29:08Z

kindly CC @neild

mauri870 · 2023-10-19T19:15:57Z

@seankhliao could you please update the issue title to smth akin to net/http, x/net/http2: page fails to load with http2 server push due to underflow. Thanks

After CL 534215 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers in handlerDone. The func startPush calls runHandler without incrementing curHandlers. Seems to only affect users of http.Pusher. For golang/go#63511 Change-Id: Ic537c27c9945c2c2d4306ddb04e9527b65cee320 GitHub-Last-Rev: 249fe55 GitHub-Pull-Request: #197 Reviewed-on: https://go-review.googlesource.com/c/net/+/535595 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com>

dr2chase · 2023-10-25T16:18:16Z

Also needs a 1.20 backport.

gopherbot · 2023-10-25T16:51:48Z

Change https://go.dev/cl/537715 mentions this issue: http2: add test for push promise accounting underflow

Verify that repeated requests resulting in a PUSH_PROMISE result all complete successfully, validating the fix in CL 535595. For golang/go#63511 Change-Id: I6bebdcfcecb6c53f076e4ac6873d61a150d1040e Reviewed-on: https://go-review.googlesource.com/c/net/+/537715 Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopherbot · 2023-10-26T22:26:17Z

Change https://go.dev/cl/538095 mentions this issue: [release-branch.go1.20] net/http: pull http2 underflow fix from x/net/http2

gopherbot · 2023-10-26T23:01:29Z

Change https://go.dev/cl/537996 mentions this issue: [release-branch.go1.21] net/http: pull http2 underflow fix from x/net/http2

gopherbot · 2023-10-26T23:23:35Z

Change https://go.dev/cl/537956 mentions this issue: [internal-branch.go1.20-vendor] http2: fix underflow in http2 server push

gopherbot · 2023-10-26T23:25:51Z

Change https://go.dev/cl/537957 mentions this issue: [internal-branch.go1.21-vendor] http2: fix underflow in http2 server push

…push After CL 534215 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers in handlerDone. The func startPush calls runHandler without incrementing curHandlers. Seems to only affect users of http.Pusher. For golang/go#63511 For golang/go#63740 Change-Id: Ic537c27c9945c2c2d4306ddb04e9527b65cee320 GitHub-Last-Rev: 249fe55 GitHub-Pull-Request: #197 Reviewed-on: https://go-review.googlesource.com/c/net/+/535595 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com> (cherry picked from commit 37479d6) Reviewed-on: https://go-review.googlesource.com/c/net/+/537956 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

…push After CL 534215 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers in handlerDone. The func startPush calls runHandler without incrementing curHandlers. Seems to only affect users of http.Pusher. For golang/go#63511 For golang/go#63560 Change-Id: Ic537c27c9945c2c2d4306ddb04e9527b65cee320 GitHub-Last-Rev: 249fe55 GitHub-Pull-Request: #197 Reviewed-on: https://go-review.googlesource.com/c/net/+/535595 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com> (cherry picked from commit 37479d6) Reviewed-on: https://go-review.googlesource.com/c/net/+/537957 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>

…/http2 After CL 534295 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers in handlerDone. Pull in a fix from x/net/http2: http2: fix underflow in http2 server push https://go-review.googlesource.com/c/net/+/535595 For #63511 Fixes #63740 Change-Id: I5c678ce7dcc53635f3ad5e4999857cb120dfc1ab GitHub-Last-Rev: 587ffa3 GitHub-Pull-Request: #63561 Reviewed-on: https://go-review.googlesource.com/c/go/+/535575 Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 0046c14) Reviewed-on: https://go-review.googlesource.com/c/go/+/538095 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>

…/http2 After CL 534295 was merged to fix a CVE it introduced an underflow when we try to decrement sc.curHandlers in handlerDone. Pull in a fix from x/net/http2: http2: fix underflow in http2 server push https://go-review.googlesource.com/c/net/+/535595 For #63511 Fixes #63560 Change-Id: I5c678ce7dcc53635f3ad5e4999857cb120dfc1ab GitHub-Last-Rev: 587ffa3 GitHub-Pull-Request: #63561 Reviewed-on: https://go-review.googlesource.com/c/go/+/535575 Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 0046c14) Reviewed-on: https://go-review.googlesource.com/c/go/+/537996 Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

seankhliao added the NeedsInvestigation label Oct 12, 2023

gopherbot mentioned this issue Oct 15, 2023

net/http: http2 page fails on firefox/safari if pushing resources [1.21 backport] #63560

Closed

mauri870 mentioned this issue Oct 15, 2023

net/http: pull http2 underflow fix from x/net/http2 #63561

Closed

mauri870 mentioned this issue Oct 15, 2023

http2: fix underflow in http2 server push golang/net#197

Closed

gopherbot added NeedsFix release-blocker and removed NeedsInvestigation labels Oct 16, 2023

dmitshur added this to the Go1.22 milestone Oct 16, 2023

MagicalTux changed the title ~~net/http: http2 page fails on firefox/safari if pushing resources~~ net/http x/net/http2: http2 page fails on firefox/safari if pushing resources Oct 18, 2023

dmitshur changed the title ~~net/http x/net/http2: http2 page fails on firefox/safari if pushing resources~~ net/http, x/net/http2: page fails to load with http2 server push due to underflow Oct 19, 2023

This comment was marked as duplicate.

Sign in to view

neild mentioned this issue Oct 25, 2023

net/http: http2 page fails on firefox/safari if pushing resources [1.20 backport] #63740

Closed

mauri870 self-assigned this Oct 26, 2023

gopherbot closed this as completed in 0046c14 Oct 26, 2023

golang locked and limited conversation to collaborators Oct 25, 2024

gopherbot added the FrozenDueToAge label Oct 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/http, x/net/http2: page fails to load with http2 server push due to underflow #63511

net/http, x/net/http2: page fails to load with http2 server push due to underflow #63511

MagicalTux commented Oct 12, 2023 •

edited

Loading

MagicalTux commented Oct 12, 2023 •

edited

Loading

seankhliao commented Oct 12, 2023

mauri870 commented Oct 15, 2023

mauri870 commented Oct 15, 2023

gopherbot commented Oct 15, 2023

mauri870 commented Oct 15, 2023

gopherbot commented Oct 15, 2023

mauri870 commented Oct 15, 2023

mauri870 commented Oct 16, 2023

gopherbot commented Oct 16, 2023

mauri870 commented Oct 19, 2023

mauri870 commented Oct 19, 2023

This comment was marked as duplicate.

dr2chase commented Oct 25, 2023

gopherbot commented Oct 25, 2023

gopherbot commented Oct 26, 2023

gopherbot commented Oct 26, 2023

gopherbot commented Oct 26, 2023

gopherbot commented Oct 26, 2023

net/http, x/net/http2: page fails to load with http2 server push due to underflow #63511

net/http, x/net/http2: page fails to load with http2 server push due to underflow #63511

Comments

MagicalTux commented Oct 12, 2023 • edited Loading

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

MagicalTux commented Oct 12, 2023 • edited Loading

seankhliao commented Oct 12, 2023

mauri870 commented Oct 15, 2023

mauri870 commented Oct 15, 2023

gopherbot commented Oct 15, 2023

mauri870 commented Oct 15, 2023

gopherbot commented Oct 15, 2023

mauri870 commented Oct 15, 2023

mauri870 commented Oct 16, 2023

gopherbot commented Oct 16, 2023

mauri870 commented Oct 19, 2023

mauri870 commented Oct 19, 2023

This comment was marked as duplicate.

dr2chase commented Oct 25, 2023

gopherbot commented Oct 25, 2023

gopherbot commented Oct 26, 2023

gopherbot commented Oct 26, 2023

gopherbot commented Oct 26, 2023

gopherbot commented Oct 26, 2023

MagicalTux commented Oct 12, 2023 •

edited

Loading

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

MagicalTux commented Oct 12, 2023 •

edited

Loading