Commit eab57b27 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

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/122819Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
parent 5fbfda6a
......@@ -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) {
......
......@@ -29,6 +29,7 @@ import (
const fakeHopHeader = "X-Fake-Hop-Header-For-Test"
func init() {
inOurTests = true
hopHeaders = append(hopHeaders, fakeHopHeader)
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment