diff options
author | Brad Fitzpatrick <bradfitz@golang.org> | 2018-07-09 22:29:00 +0000 |
---|---|---|
committer | Brad Fitzpatrick <bradfitz@golang.org> | 2018-07-09 23:40:02 +0000 |
commit | eab57b27f5460a7e6b87fff95ce2948b7812ce05 (patch) | |
tree | 4441cdf84997d01472a6ca9de3c2d71dbfca25cf /src/net/http/httputil/reverseproxy.go | |
parent | 5fbfda6a833f3bbc3d714459b5194f7fef1e0b43 (diff) | |
download | go-eab57b27f5460a7e6b87fff95ce2948b7812ce05.tar.gz go-eab57b27f5460a7e6b87fff95ce2948b7812ce05.zip |
net/http/httputil: don't panic in ReverseProxy unless running under a 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>
Diffstat (limited to 'src/net/http/httputil/reverseproxy.go')
-rw-r--r-- | src/net/http/httputil/reverseproxy.go | 28 |
1 files changed, 27 insertions, 1 deletions
diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index 2eda6b70d0..6f0a2418b3 100644 --- a/src/net/http/httputil/reverseproxy.go +++ b/src/net/http/httputil/reverseproxy.go @@ -253,7 +253,11 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { defer res.Body.Close() // Since we're streaming the response, if we run into an error all we can do // is abort the request. Issue 23643: ReverseProxy should use ErrAbortHandler - // on read error while copying body + // on read error while copying body. + if !shouldPanicOnCopyError(req) { + p.logf("suppressing panic for copyResponse error in test; copy error: %v", err) + return + } panic(http.ErrAbortHandler) } res.Body.Close() // close now, instead of defer, to populate res.Trailer @@ -271,6 +275,28 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { } } +var inOurTests bool // whether we're in our own tests + +// shouldPanicOnCopyError reports whether the reverse proxy should +// panic with http.ErrAbortHandler. This is the right thing to do by +// default, but Go 1.10 and earlier did not, so existing unit tests +// weren't expecting panics. Only panic in our own tests, or when +// running under the HTTP server. +func shouldPanicOnCopyError(req *http.Request) bool { + if inOurTests { + // Our tests know to handle this panic. + return true + } + if req.Context().Value(http.ServerContextKey) != nil { + // We seem to be running under an HTTP server, so + // it'll recover the panic. + return true + } + // Otherwise act like Go 1.10 and earlier to not break + // existing tests. + return false +} + // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h. // See RFC 7230, section 6.1 func removeConnectionHeaders(h http.Header) { |