go icon indicating copy to clipboard operation
go copied to clipboard

net/http: ResponseController panic

Open komuw opened this issue 2 years ago • 4 comments

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

$ go version
go version go1.20 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//.cache/go-build"
GOENV="/home//.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home//go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home//go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home//Downloads/cool/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 -fdebug-prefix-map=/run/user/1000/go-build839916512=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run the program; https://go.dev/play/p/Alu2e-LdzqX?v=gotip and then use curl to access it curl -vkL https://127.0.0.1:56782/

What did you expect to see?

No panic, possibly an error

What did you see instead?

listening on  :56782
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x7471f5]

goroutine 22 [running]:
net/http.(*http2pipe).CloseWithError(...)
        /usr/local/go/src/net/http/h2_bundle.go:3718
net/http.(*http2stream).onReadTimeout(0xc00024e0b0)
        /usr/local/go/src/net/http/h2_bundle.go:5639 +0xd5
created by time.goFunc
        /usr/local/go/src/time/sleep.go:176 +0x48
exit status 2

komuw avatar Feb 02 '23 05:02 komuw

this line cause panic, add If st.Body == nil maybe a choice to fix this. but I am not very sure.

cuiweixie avatar Feb 02 '23 10:02 cuiweixie

cc @neild

seankhliao avatar Feb 02 '23 11:02 seankhliao

Change https://go.dev/cl/464936 mentions this issue: http2: check stream body is present on read timeout

gopherbot avatar Feb 02 '23 22:02 gopherbot

Change https://go.dev/cl/465035 mentions this issue: net/http: add ResponseController http2 request without body read deadline test

gopherbot avatar Feb 02 '23 22:02 gopherbot

Is there any further information that would be helpful for this issue? It can still be reproduced in Go 1.20.5, and prohibits the use of http.ResponseController.SetReadDeadline() in many real-world deployments (at least where TLS is enabled, since it's HTTP/2-specific).

lwithers avatar Jun 27 '23 16:06 lwithers

https://go.dev/cl/465035 is the reproducer. https://go.dev/cl/464936 is the fix.

AlexanderYastrebov avatar Jul 21 '23 21:07 AlexanderYastrebov

https://go.dev/cl/464936 is in, so at this point I think it's just waiting on someone to re-run the bundler to import x/net/http2 into net/http.

bcmills avatar Sep 28 '23 15:09 bcmills

I think it's just waiting on someone to re-run the bundler to import x/net/http2

http2 bundle was updated by https://go-review.googlesource.com/c/go/+/534295 I've rebased https://go-review.googlesource.com/c/go/+/465035

AlexanderYastrebov avatar Oct 12 '23 09:10 AlexanderYastrebov

If I have understood correctly, I think the updated h2 bundle is in Go 1.21.3 (via e175f27), but I am afraid that I still see the problem with the original reproducer (https://go.dev/play/p/Alu2e-LdzqX?v=gotip):

$ go version
go version go1.21.3 linux/amd64
$ go build
$ ./h2test &
listening on  :56782
$ curl https://localhost.example:56782/hello-world
curl: (18) HTTP/2 stream 1 was not closed cleanly before end of the underlying stream

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x602e61]

goroutine 18 [running]:
net/http.(*http2pipe).CloseWithError(...)
	/opt/go/src/net/http/h2_bundle.go:3791
net/http.(*http2stream).onReadTimeout(0xc00010ec80)
	/opt/go/src/net/http/h2_bundle.go:5727 +0x61
created by time.goFunc
	/opt/go/src/time/sleep.go:176 +0x2d

lwithers avatar Oct 12 '23 15:10 lwithers

@lwithers Could you please check tip, I think line numbers in your trace do not match:

https://github.com/golang/go/blob/e4f72f773666b8f13ed5d053abf87ca42c68cc16/src/net/http/h2_bundle.go#L3792 https://github.com/golang/go/blob/e4f72f773666b8f13ed5d053abf87ca42c68cc16/src/net/http/h2_bundle.go#L5721

AlexanderYastrebov avatar Oct 12 '23 15:10 AlexanderYastrebov

I cloned the Go repo and built at e4f72f7, and this did not exhibit the problem:

$ PATH=/home/lwithers/tmp/goroot/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games go version
go version devel go1.22-e4f72f7736 Thu Oct 12 14:39:39 2023 +0000 linux/amd64
$ PATH=/home/lwithers/tmp/goroot/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games go build
$ ./h2test &
listening on  :56782
$ curl https://localhost.internal.yoti.com:56782/hello-world
curl: (92) HTTP/2 stream 1 was not closed cleanly: INTERNAL_ERROR (err 2)

(the curl error is expected).

So tip is fine; it just appears to be Go 1.21.3 that still exhibits the problem.

lwithers avatar Oct 12 '23 16:10 lwithers

always on 1.21.4

reproducer: https://go.dev/play/p/Alu2e-LdzqX?v=gotip

[15:11:43]➜  go-http2 go version
go version go1.21.4 darwin/amd64
[15:12:28]➜  go-http2 go build
[15:12:35]➜  go-http2 ./go-http2
listening on  :56782
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x12055e1]

goroutine 21 [running]:
net/http.(*http2pipe).CloseWithError(...)
	/usr/local/go/src/net/http/h2_bundle.go:3791
net/http.(*http2stream).onReadTimeout(0xc000212140)
	/usr/local/go/src/net/http/h2_bundle.go:5727 +0x61
created by time.goFunc
	/usr/local/go/src/time/sleep.go:176 +0x2d

dcboy avatar Nov 27 '23 07:11 dcboy

always on 1.21.4

The fix was merged to master by updating h2_bundle within https://go-review.googlesource.com/c/go/+/534295 for https://github.com/golang/go/issues/63426 Then there were backports for https://github.com/golang/go/issues/63426 that did not include the fix for this issue

AlexanderYastrebov avatar Nov 27 '23 09:11 AlexanderYastrebov

I'm going to close this as fixed for 1.22 / tip. I don't believe this was ever backported to 1.21.

seankhliao avatar Jul 09 '24 19:07 seankhliao