proxy: use a new context for the outgoing request (caddyserver#1358)

* proxy: use a new context for the outgoing request fix issue caddyserver#1345 Signed-off-by: Tw <[email protected]> * proxy: add test for canceling the request Signed-off-by: Tw <[email protected]>
overcome · Jan 24, 2017 · 38c7664 · 38c7664
1 parent 696b46f
commit 38c7664
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 0 deletions.
diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go
@@ -3,6 +3,7 @@ package proxy
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"crypto/tls"
 	"fmt"
 	"io"
@@ -11,6 +12,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/http/httptrace"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -1017,6 +1019,47 @@ func TestReverseProxyLargeBody(t *testing.T) {
 	}
 }
 
+func TestCancelRequest(t *testing.T) {
+	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("Hello, client"))
+	}))
+	defer backend.Close()
+
+	// set up proxy
+	p := &Proxy{
+		Next:      httpserver.EmptyNext, // prevents panic in some cases when test fails
+		Upstreams: []Upstream{newFakeUpstream(backend.URL, false)},
+	}
+
+	// setup request with cancel ctx
+	req := httptest.NewRequest("GET", "/", nil)
+	ctx, cancel := context.WithCancel(req.Context())
+	defer cancel()
+	req = req.WithContext(ctx)
+
+	// add GotConn hook to cancel the request
+	gotC := make(chan struct{})
+	defer close(gotC)
+	trace := &httptrace.ClientTrace{
+		GotConn: func(connInfo httptrace.GotConnInfo) {
+			gotC <- struct{}{}
+		},
+	}
+	req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace))
+
+	// wait for canceling the request
+	go func() {
+		<-gotC
+		cancel()
+	}()
+
+	status, err := p.ServeHTTP(httptest.NewRecorder(), req)
+	if status != 0 || err != nil {
+		t.Errorf("expect proxy handle normally, but not, status:%d, err:%q",
+			status, err)
+	}
+}
+
 type noopReader struct {
 	len uint64
 	pos uint64

diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go
@@ -12,6 +12,7 @@
 package proxy
 
 import (
+	"context"
 	"crypto/tls"
 	"io"
 	"net"
@@ -206,6 +207,14 @@ func (rp *ReverseProxy) ServeHTTP(rw http.ResponseWriter, outreq *http.Request,
 
 	rp.Director(outreq)
 
+	// Original incoming server request may be canceled by the
+	// user or by std lib(e.g. too many idle connections).
+	// Now we issue the new outgoing client request which
+	// doesn't depend on the original one. (issue 1345)
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	outreq = outreq.WithContext(ctx)
+
 	res, err := transport.RoundTrip(outreq)
 	if err != nil {
 		return err