Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http,x/net/http2: http2serverConn WriteTimeout cannot set negative values #65785

Closed
eudore opened this issue Feb 19, 2024 · 4 comments
Closed
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@eudore
Copy link

eudore commented Feb 19, 2024

Go version

go version go1.21.3 linux/amd64

Output of go env in your module/workspace:

GO111MODULE='off'
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/root/go:/data/web/golang'
GOPRIVATE=''
GOPROXY='https://goproxy.cn'
GOROOT='/usr/local/go1.21'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go1.21/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.3'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOMOD=''
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3116255717=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Set http.Server WriteTimeout to negative value.

Comment in http.Server: // A zero or negative value means there will be no timeout.

package main

import (
	"fmt"
	"net"
	"net/http"
	"time"
)

func main() {
	srv := &http.Server{
		WriteTimeout: -1 * time.Second,
	}

	ln, err := net.Listen("tcp", ":8088")
	if err != nil {
		panic(err)
	}
	go srv.Serve(ln)

	ln, err = net.Listen("tcp", ":8089")
	if err != nil {
		panic(err)
	}
	go runClient()

	err = srv.ServeTLS(ln, "/etc/nginx/openssl/eudore.cn/www.eudore.cn.pem", "/etc/nginx/openssl/eudore.cn/www.eudore.cn.key")
	if err != nil {
		panic(err)
	}
}

func runClient() {
	time.Sleep(time.Second)
	fmt.Println(http.Get("http://www.eudore.cn:8088"))
	fmt.Println(http.Get("https://www.eudore.cn:8089"))
}

What did you see happen?

Using golang h2 client request shows error.

&{404 Not Found 404 HTTP/1.1 1 1 map[Content-Length:[19] Content-Type:[text/plain; charset=utf-8] Date:[Mon, 19 Feb 2024 07:08:39 GMT] X-Content-Type-Options:[nosniff]] 0xc0001820c0 19 [] false false map[] 0xc0000b6360 <nil>} <nil>
<nil> Get "https://www.eudore.cn:8089": stream error: stream ID 1; INTERNAL_ERROR; received from peer

If you use the current version of chrome to access, ERR_HTTP2_PROTOCOL_ERROR is displayed.

The webpage at https://www.eudore.cn:8089/ might be temporarily down or it may have moved permanently to a new web address.
ERR_HTTP2_PROTOCOL_ERROR

What did you expect to see?

h2 server handles negative WriteTimeouts normally.

&{404 Not Found 404 HTTP/1.1 1 1 map[Content-Length:[19] Content-Type:[text/plain; charset=utf-8] Date:[Mon, 19 Feb 2024 07:08:22 GMT] X-Content-Type-Options:[nosniff]] 0xc00007c600 19 [] false false map[] 0xc00008c360 <nil>} <nil>
&{404 Not Found 404 HTTP/2.0 2 0 map[Content-Length:[19] Content-Type:[text/plain; charset=utf-8] Date:[Mon, 19 Feb 2024 07:08:22 GMT] X-Content-Type-Options:[nosniff]] {0xc0002b2180} 19 [] false false map[] 0xc00008c7e0 0xc0000844d0} <nil>

In the current go version, http.Server and http.http2serverConn set WriteTimeout judgment conditions differently.

func (c *conn) readRequest(ctx context.Context) (w *response, err error) {
	...
	if d := c.server.WriteTimeout; d > 0 {
		defer func() {
			c.rwc.SetWriteDeadline(time.Now().Add(d))
		}()
	}
	...
}

func (sc *http2serverConn) newStream(id, pusherID uint32, state http2streamState) *http2stream {
	...
	if sc.hs.WriteTimeout != 0 {
		st.writeDeadline = time.AfterFunc(sc.hs.WriteTimeout, st.onWriteTimeout)
	}
	...
}
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 19, 2024
@seankhliao seankhliao changed the title net/http: http2serverConn WriteTimeout cannot set negative values net/http,x/net/http2: http2serverConn WriteTimeout cannot set negative values Feb 19, 2024
@seankhliao
Copy link
Member

cc @neild

@panjf2000
Copy link
Member

According to #39177, this needs a fix.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/565195 mentions this issue: http2: only set up positive deadlines

@panjf2000 panjf2000 added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 19, 2024
@panjf2000 panjf2000 added this to the Backlog milestone Feb 21, 2024
@panjf2000 panjf2000 added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Feb 24, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/569819 mentions this issue: net/http: update bundled x/net/http2

gopherbot pushed a commit that referenced this issue Mar 19, 2024
For #65785 #65927

Change-Id: I21791d4e22ae3039144f6b105ac439877f8b01bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/569819
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: David Chase <[email protected]>
Auto-Submit: Emmanuel Odeke <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Commit-Queue: Emmanuel Odeke <[email protected]>
Reviewed-by: Emmanuel Odeke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants