From c4e65df262168c8a058af03bee2aed9dcdb47bb7 Mon Sep 17 00:00:00 2001 From: Eric Drechsel Date: Sat, 16 Apr 2016 17:45:18 -0700 Subject: [PATCH 1/4] Proxy: Add a failing test which replicates #763 2 websocket connections are made instead of one --- caddyhttp/proxy/proxy_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index 866cde958..1535cf7fd 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -102,7 +102,8 @@ func TestReverseProxyInsecureSkipVerify(t *testing.T) { func TestWebSocketReverseProxyServeHTTPHandler(t *testing.T) { // No-op websocket backend simply allows the WS connection to be // accepted then it will be immediately closed. Perfect for testing. - wsNop := httptest.NewServer(websocket.Handler(func(ws *websocket.Conn) {})) + var connCount int + wsNop := httptest.NewServer(websocket.Handler(func(ws *websocket.Conn) { connCount++ })) defer wsNop.Close() // Get proxy to use for the test @@ -135,6 +136,9 @@ func TestWebSocketReverseProxyServeHTTPHandler(t *testing.T) { if !bytes.Equal(actual, expected) { t.Errorf("Expected backend to accept response:\n'%s'\nActually got:\n'%s'", expected, actual) } + if connCount != 1 { + t.Errorf("Expected 1 websocket connection, got %d", connCount) + } } func TestWebSocketReverseProxyFromWSClient(t *testing.T) { From d534a2139f56aab3bed82473120c2d1069c5b3bd Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Mon, 1 Aug 2016 19:11:31 -0700 Subject: [PATCH 2/4] Proxy: When connecting to websocket backend, reuse the connection isntead of starting a new one. --- caddyhttp/proxy/reverseproxy.go | 94 ++++++++++++++++++++++++++++++--- 1 file changed, 87 insertions(+), 7 deletions(-) diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go index eadd7e3ad..9d7087c24 100644 --- a/caddyhttp/proxy/reverseproxy.go +++ b/caddyhttp/proxy/reverseproxy.go @@ -183,9 +183,80 @@ var hopHeaders = []string{ type respUpdateFn func(resp *http.Response) +type hijackedConn struct { + net.Conn + hj *connHijackerTransport +} + +func (c *hijackedConn) Read(b []byte) (n int, err error) { + n, err = c.Conn.Read(b) + c.hj.Replay = append(c.hj.Replay, b[:n]...) + return +} + +func (c *hijackedConn) Close() error { + return nil +} + +type connHijackerTransport struct { + *http.Transport + Conn net.Conn + Replay []byte +} + +func newConnHijackerTransport(base http.RoundTripper) *connHijackerTransport { + transport := &http.Transport{ + Proxy: http.ProxyFromEnvironment, + Dial: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).Dial, + TLSHandshakeTimeout: 10 * time.Second, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + if base != nil { + if baseTransport, ok := base.(*http.Transport); ok { + transport.Proxy = baseTransport.Proxy + transport.TLSClientConfig = baseTransport.TLSClientConfig + transport.TLSHandshakeTimeout = baseTransport.TLSHandshakeTimeout + transport.Dial = baseTransport.Dial + transport.DialTLS = baseTransport.DialTLS + transport.DisableKeepAlives = true + } + } + hjTransport := &connHijackerTransport{transport, nil, bufferPool.Get().([]byte)[:0]} + oldDial := transport.Dial + oldDialTLS := transport.DialTLS + if oldDial == nil { + oldDial = (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).Dial + } + hjTransport.Dial = func(network, addr string) (net.Conn, error) { + c, err := oldDial(network, addr) + hjTransport.Conn = c + return &hijackedConn{c, hjTransport}, err + } + if oldDialTLS != nil { + hjTransport.DialTLS = func(network, addr string) (net.Conn, error) { + c, err := oldDialTLS(network, addr) + hjTransport.Conn = c + return &hijackedConn{c, hjTransport}, err + } + } + return hjTransport +} + +func requestIsWebsocket(req *http.Request) bool { + return !(strings.ToLower(req.Header.Get("Upgrade")) != "websocket" || !strings.Contains(strings.ToLower(req.Header.Get("Connection")), "upgrade")) +} + func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, outreq *http.Request, respUpdateFn respUpdateFn) error { transport := p.Transport - if transport == nil { + if requestIsWebsocket(outreq) { + transport = newConnHijackerTransport(transport) + } else if transport == nil { transport = http.DefaultTransport } @@ -216,13 +287,22 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, outreq *http.Request, r } defer conn.Close() - backendConn, err := net.Dial("tcp", outreq.URL.Host) - if err != nil { - return err - } - defer backendConn.Close() + var backendConn net.Conn + if hj, ok := transport.(*connHijackerTransport); ok { + backendConn = hj.Conn + if _, err := conn.Write(hj.Replay); err != nil { + return err + } + bufferPool.Put(hj.Replay) + } else { + backendConn, err = net.Dial("tcp", outreq.URL.Host) + if err != nil { + return err + } + defer backendConn.Close() - outreq.Write(backendConn) + outreq.Write(backendConn) + } go func() { io.Copy(backendConn, conn) // write tcp stream to backend. From f4cdf53761ff7ea6279122172ae4539153929414 Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Tue, 2 Aug 2016 12:31:17 -0700 Subject: [PATCH 3/4] Proxy: Fix transport defn; cleanup connection. --- caddyhttp/proxy/reverseproxy.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go index 9d7087c24..7feb90076 100644 --- a/caddyhttp/proxy/reverseproxy.go +++ b/caddyhttp/proxy/reverseproxy.go @@ -212,7 +212,7 @@ func newConnHijackerTransport(base http.RoundTripper) *connHijackerTransport { KeepAlive: 30 * time.Second, }).Dial, TLSHandshakeTimeout: 10 * time.Second, - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + DisableKeepAlives: true, } if base != nil { if baseTransport, ok := base.(*http.Transport); ok { @@ -299,10 +299,9 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, outreq *http.Request, r if err != nil { return err } - defer backendConn.Close() - outreq.Write(backendConn) } + defer backendConn.Close() go func() { io.Copy(backendConn, conn) // write tcp stream to backend. From 6e9439d22e21b5dec2f25fe2831f71f84a805671 Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Tue, 2 Aug 2016 12:39:15 -0700 Subject: [PATCH 4/4] Proxy: Fix data race in test. --- caddyhttp/proxy/proxy_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index 1535cf7fd..469b4c1f6 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -15,6 +15,7 @@ import ( "path/filepath" "runtime" "strings" + "sync/atomic" "testing" "time" @@ -102,8 +103,8 @@ func TestReverseProxyInsecureSkipVerify(t *testing.T) { func TestWebSocketReverseProxyServeHTTPHandler(t *testing.T) { // No-op websocket backend simply allows the WS connection to be // accepted then it will be immediately closed. Perfect for testing. - var connCount int - wsNop := httptest.NewServer(websocket.Handler(func(ws *websocket.Conn) { connCount++ })) + var connCount int32 + wsNop := httptest.NewServer(websocket.Handler(func(ws *websocket.Conn) { atomic.AddInt32(&connCount, 1) })) defer wsNop.Close() // Get proxy to use for the test @@ -136,7 +137,7 @@ func TestWebSocketReverseProxyServeHTTPHandler(t *testing.T) { if !bytes.Equal(actual, expected) { t.Errorf("Expected backend to accept response:\n'%s'\nActually got:\n'%s'", expected, actual) } - if connCount != 1 { + if atomic.LoadInt32(&connCount) != 1 { t.Errorf("Expected 1 websocket connection, got %d", connCount) } }