I've found 2 issues around Flush() ing on http.ResponseWriter after the http.Handler has finished. The first problem is it causes panic. The later one is that Write()ing on the http.ResponseWriter is redirected to the response of another request. I'd like to report them.

My version and env

go version go1.8 darwin/amd64

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8m/15jd53qj3sngxm2y8gsqm6252h4p5m/T/go-build435976512=/tmp/go-build -gno-record-gcc-switches -fno-common"

Issue 1(panic)

I can reproduce it on playground.

https://play.golang.org/p/63aDsPEvZ1

Flush() to the ResponseWriter after the http.ResponseWriter from separated goroutine causes a panic. I think it's because the http.response.w is reset with Reset(nil) to put it to the buffer pool(ref: go/server.go at master · golang/go ) in order to reuse it. And bufio.Writer.Flush() with nil underlying-buffer causes panic (ref: #9657 )).

&{Status:200 OK StatusCode:200 Proto:HTTP/1.1 ProtoMajor:1 ProtoMinor:1 Header:map[Content-Length:[1] Date:[Tue, 10 Nov 2009 23:00:00 GMT] Content-Type:[text/plain; charset=utf-8]] Body:0x10732780 ContentLength:1 TransferEncoding:[] Close:false Uncompressed:false Trailer:map[] Request:0x10760360 TLS:<nil>}
sleeping...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0xffffffff addr=0x0 pc=0x131262]

goroutine 12 [running]:
bufio.(*Writer).Flush(0x0, 0x2dede0, 0x0, 0x3e0b00)
	/usr/local/go/src/bufio/bufio.go:560 +0x22
net/http.(*chunkWriter).flush(0x1076052c, 0x1073e700)
	/usr/local/go/src/net/http/server.go:374 +0x60
net/http.(*response).Flush(0x10760510, 0x3e2c20)
	/usr/local/go/src/net/http/server.go:1592 +0x60
main.main.func1.1(0x3e2c20, 0x10760510)
	/tmp/sandbox037200695/main.go:21 +0x140
created by main.main.func1
	/tmp/sandbox037200695/main.go:22 +0xc0

I think it's not very good manner writing to ResponseWriter after the handler has been finished. But we don't have ways to know if the handler has finished exactly (before the bufio.Writer.Reset(nil)).

Also you can use repeated Write() insted of Flush() because bufio.Writer.Write() potentially invoke its Flush() internally.

Issue 2(writing to the response of another request)

To make matters worse, because of the reuse of http.response.w, we can write to the response of another request unexpectedly (or expectedly).

https://play.golang.org/p/5z6VbtwqnR

body from /1:
body from /2:THIS IS DATA WRITTEN FOR /1

workaround to handle this issue

I think we can avoid this issue like below.

fixed version of first problem. https://play.golang.org/p/jfn1cG0006
fixed version of later problem. https://play.golang.org/p/7hmjZDuE7a

type safeResponseWriter struct {
	handlerDone int64
	http.ResponseWriter
}

func (w *safeResponseWriter) Write(p []byte) (int, error) {
	if atomic.LoadInt64(&w.handlerDone) == 0 {
		return w.ResponseWriter.Write(p)
	}
	return 0, errors.New("handler done")
}

func (w *safeResponseWriter) Flush() {
	if atomic.LoadInt64(&w.handlerDone) == 0 {
		w.ResponseWriter.(http.Flusher).Flush()
	}
}

func (w *safeResponseWriter) Done() {
	atomic.StoreInt64(&w.handlerDone, 1)
}

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	ww := &safeResponseWriter{ResponseWriter: w}
	defer ww.Done()
	w = ww
	
	// ...
}

IMHO, though the first issue(panic) might not be a big problem, the later one should be fixed (in net/http package). I think it might be a kind of vulnerability because third party HTTP middleware could hijack any other HTTP handler running on the same HTTP.Server.

How do you think about this?