net/http/httputil: ReverseProxy should use ErrAbortHandler on read error while copying body #23643
Comments
|
ReverseProxy is streaming. It doesn't slurp up the entire response from the backend before replying to the client. So if the backend writes some bytes (which involves writing headers, triggering an implicit 200 OK if the backend is in Go), then the ReverseProxy will also copy that 200 and then begin copying (reading from backend, writing to client) the body. So if the read error occurs after the header's been sent, the header's already been sent. The best we can do is terminate the connection so the client knows there's an error (stream error in HTTP/2, unexpected EOF from Content-Length mismatch or premature chunking EOF without the zero chunk EOF) |
|
But I see we could do better here. ReverseProxy should use https://golang.org/pkg/net/http/#ErrAbortHandler in copyResponse if copyBuffer returns an error. That doesn't exactly address your issue, but it's the closest we can do without buffering everything, which I don't think we want to introduce as an option. |
bradfitz
changed the title
bradfitz
added
help wanted
NeedsFix
|
So |
|
Yup. |
|
Change https://golang.org/cl/91675 mentions this issue: |
|
Change https://golang.org/cl/122819 mentions this issue: |
… Server Prior to the fix to #23643, the ReverseProxy didn't panic with ErrAbortHandler when the copy to a client failed. During Go 1.11 beta testing, we found plenty of code using ReverseProxy in tests that were unprepared for a panic. Change the behavior to only panic when running under the http.Server that'll handle the panic. Updates #23643 Change-Id: Ic1fa8405fd54c858ce8c797cec79d006833a9f7d Reviewed-on: https://go-review.googlesource.com/122819 Reviewed-by: Ian Lance Taylor <iant@golang.org>
What version of Go are you using (
go version)?1.9.2
Does this issue reproduce with the latest release?
Yes, additionally, looking at the code, it doesn't seem to be different in master.
What operating system and processor architecture are you using (
go env)?OS independent (tested on Windows/Linux)
What did you do?
https://play.golang.org/p/dK-zOdvklm1
If there's an error reading the body of a request, the ReverseProxy returns 200. The above script uses http.MaxBytesReader to cause an error reading the body and then prints the status code returned by the proxy.
What did you expect to see?
I expect the proxy to return a 502 bad gateway error instead of 200.
It looks like the problem is that
*ReverseProxy.copyResponsedoesn't return the error from*ReverseProxy.copyBuffer. An error channel, or similar, will need to be used for the FlushInterval case, but I'd expect the error to make it back into ServeHTTP. However, since the headers are already written before the body is read, that would need to be changed.What did you see instead?
If you run the go playground above you get:
The text was updated successfully, but these errors were encountered: