net/http: ResponseController may cause infinite loop

[weixiecui]

Go version

go version devel go1.23-d42cd452dc Tue Feb 20 06:04:31 2024 +0000 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xiecui/Library/Caches/go-build'
GOENV='/Users/xiecui/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE='*.futunn.com,*.oa.com'
GOMODCACHE='/Users/xiecui/code/go/gopath/pkg/mod'
GONOPROXY='*.futunn.com,*.oa.com'
GONOSUMDB='on'
GOOS='darwin'
GOPATH='/Users/xiecui/code/go/gopath'
GOPRIVATE='*.futunn.com,*.oa.com'
GOPROXY='https://goproxy.cn,direct'
GOROOT='/Users/xiecui/code/go/src/go'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/xiecui/code/go/src/go/pkg/tool/darwin_arm64'
GOVCS='*:all'
GOVERSION='devel go1.23-d42cd452dc Tue Feb 20 06:04:31 2024 +0000'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/xiecui/code/go/src/go/src/go.mod'
GOWORK='/Users/xiecui/code/go/src/go/src/go.work'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/d2/82q9pj7x33gc7m0xkpkx3p3w0000gn/T/go-build2147947390=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

package main

import "net/http"

type endless struct {
}

func (e endless) Header() http.Header {
	//TODO implement me
	panic("implement me")
}

func (e endless) Write(bytes []byte) (int, error) {
	//TODO implement me
	panic("implement me")
}

func (e endless) WriteHeader(statusCode int) {
	//TODO implement me
	panic("implement me")
}

func (e endless) Unwrap() http.ResponseWriter {
	return e
}

func main() {
	rw := http.NewResponseController(endless{})
	rw.Flush() // infinite loop here
}

What did you see happen?

infinite loop

What did you expect to see?

the program end normally

[weixiecui]

// Flush flushes buffered data to the client.
func (c *ResponseController) Flush() error {
	rw := c.rw
	for {
		switch t := rw.(type) {
		case interface{ FlushError() error }:
			return t.FlushError()
		case Flusher:
			t.Flush()
			return nil
		case rwUnwrapper:
			rw = t.Unwrap()
		default:
			return errNotSupported()
		}
	}
}

I think we can add a loop limit like 1000 in the above code.

[cuiweixie]

may be change the code to:

// Flush flushes buffered data to the client.
func (c *ResponseController) Flush() error {
	rw := c.rw
	i := 0
	for {
		switch t := rw.(type) {
		case interface{ FlushError() error }:
			return t.FlushError()
		case Flusher:
			t.Flush()
			return nil
		case rwUnwrapper:
			rw = t.Unwrap()
		default:
			return errNotSupported()
		}
		if i >= 1000 {
			return errors.New("wrap too deep")
		}
		i++
	}
}

[seankhliao]

cc @neild

[neild]

Don't do that?

That seems like a clear bug in the Unwrap implementation, and one which will become instantly apparent the first time it is used.