mirror of
https://github.com/golang/go
synced 2024-09-04 23:44:16 +00:00
net/http/httputil: close incoming ReverseProxy request body
Reading from an incoming request body after the request handler aborts with a panic can cause a panic, becuse http.Server does not (contrary to its documentation) close the request body in this case. Always close the incoming request body in ReverseProxy.ServeHTTP to ensure that any in-flight outgoing requests using the body do not read from it. Updates #46866 Fixes CVE-2021-36221 Change-Id: I310df269200ad8732c5d9f1a2b00de68725831df Reviewed-on: https://go-review.googlesource.com/c/go/+/333191 Trust: Damien Neil <dneil@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
This commit is contained in:
parent
70fd4e47d7
commit
b7a85e0003
|
@ -235,6 +235,15 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
|
||||||
if req.ContentLength == 0 {
|
if req.ContentLength == 0 {
|
||||||
outreq.Body = nil // Issue 16036: nil Body for http.Transport retries
|
outreq.Body = nil // Issue 16036: nil Body for http.Transport retries
|
||||||
}
|
}
|
||||||
|
if outreq.Body != nil {
|
||||||
|
// Reading from the request body after returning from a handler is not
|
||||||
|
// allowed, and the RoundTrip goroutine that reads the Body can outlive
|
||||||
|
// this handler. This can lead to a crash if the handler panics (see
|
||||||
|
// Issue 46866). Although calling Close doesn't guarantee there isn't
|
||||||
|
// any Read in flight after the handle returns, in practice it's safe to
|
||||||
|
// read after closing it.
|
||||||
|
defer outreq.Body.Close()
|
||||||
|
}
|
||||||
if outreq.Header == nil {
|
if outreq.Header == nil {
|
||||||
outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate
|
outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate
|
||||||
}
|
}
|
||||||
|
|
|
@ -1122,6 +1122,45 @@ func TestReverseProxy_PanicBodyError(t *testing.T) {
|
||||||
rproxy.ServeHTTP(httptest.NewRecorder(), req)
|
rproxy.ServeHTTP(httptest.NewRecorder(), req)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Issue #46866: panic without closing incoming request body causes a panic
|
||||||
|
func TestReverseProxy_PanicClosesIncomingBody(t *testing.T) {
|
||||||
|
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
out := "this call was relayed by the reverse proxy"
|
||||||
|
// Coerce a wrong content length to induce io.ErrUnexpectedEOF
|
||||||
|
w.Header().Set("Content-Length", fmt.Sprintf("%d", len(out)*2))
|
||||||
|
fmt.Fprintln(w, out)
|
||||||
|
}))
|
||||||
|
defer backend.Close()
|
||||||
|
backendURL, err := url.Parse(backend.URL)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
proxyHandler := NewSingleHostReverseProxy(backendURL)
|
||||||
|
proxyHandler.ErrorLog = log.New(io.Discard, "", 0) // quiet for tests
|
||||||
|
frontend := httptest.NewServer(proxyHandler)
|
||||||
|
defer frontend.Close()
|
||||||
|
frontendClient := frontend.Client()
|
||||||
|
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
for i := 0; i < 2; i++ {
|
||||||
|
wg.Add(1)
|
||||||
|
go func() {
|
||||||
|
defer wg.Done()
|
||||||
|
for j := 0; j < 10; j++ {
|
||||||
|
const reqLen = 6 * 1024 * 1024
|
||||||
|
req, _ := http.NewRequest("POST", frontend.URL, &io.LimitedReader{R: neverEnding('x'), N: reqLen})
|
||||||
|
req.ContentLength = reqLen
|
||||||
|
resp, _ := frontendClient.Transport.RoundTrip(req)
|
||||||
|
if resp != nil {
|
||||||
|
io.Copy(io.Discard, resp.Body)
|
||||||
|
resp.Body.Close()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
}
|
||||||
|
wg.Wait()
|
||||||
|
}
|
||||||
|
|
||||||
func TestSelectFlushInterval(t *testing.T) {
|
func TestSelectFlushInterval(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
|
|
Loading…
Reference in a new issue