net/http/httputil: ReverseProxy uses CloseNotifier before body consumption

[orivej]

Since 99fb191, one of Consul tests does not pass because CloseNotify for clientGone gets triggered in httputil.ReverseProxy.ServeHTTP when the client in fact is not gone.

How to reproduce:

go get -u -v github.com/hashicorp/consul
go test github.com/hashicorp/consul/api -run TestLock_MonitorRetry

The test should not fail, but does since aforementioned commit. If I comment out requestCanceler.CancelRequest(outreq) in net/http/httputil/reverseproxy.go, it passes again.

Downstream issue: hashicorp/consul#1524

[bradfitz]

Are you using HTTP/1.1 pipelining?

[bradfitz]

Nevermind, I see now that httputil.ReverseProxy doesn't obey the new tightened rules for CloseNotifier.

I can fix.

[pwaller]

@bradfitz I can't see mention of the tightened rules in the release notes or on the mailing list. I guess you're referring to this CL and this new rule?:

CloseNotify is undefined before Request.Body has been fully read.

For me, I get the RequestProxy giving http: proxy error: net/http: request canceled whenever there is a HTTP post; which I assume is this issue.

I guess the fix is straightforward and the call to CloseNotify() just needs hoisting inside RunOnFirstRead?

[starchou]

@pwaller I have the same issue . and I use POST,PUT,DELETE method ,they all output error :http: proxy error: net/http: request canceled .
@bradfitz can you fix it ?
3q

[starchou]

@pwaller I copy it and move code like that: https://gist.github.com/starchou/3fa257ea20fdf6babeab
it's work now.

[orivej]

This was discussed a little on the list: http://thread.gmane.org/gmane.comp.lang.go.general/170241

[gopherbot]

CL https://golang.org/cl/18144 mentions this issue.