mirror of
https://github.com/golang/go
synced 2024-11-22 20:14:40 -07: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 {
|
||||
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 {
|
||||
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)
|
||||
}
|
||||
|
||||
// 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) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
Loading…
Reference in New Issue
Block a user