From db4cd8ee2d4c86ebfc0403ca216b31d6f1b36eec Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Mon, 1 Aug 2016 15:47:31 -0700 Subject: [PATCH 1/2] Proxy: Add keepalive directive to proxy to set MaxIdleConnsPerHost on transport. Fixes #938 --- caddyhttp/proxy/proxy.go | 2 +- caddyhttp/proxy/proxy_test.go | 8 +++--- caddyhttp/proxy/reverseproxy.go | 49 +++++++++++++++++++++++---------- caddyhttp/proxy/upstream.go | 14 ++++++++-- 4 files changed, 52 insertions(+), 21 deletions(-) diff --git a/caddyhttp/proxy/proxy.go b/caddyhttp/proxy/proxy.go index 6917bcd64..ed4383dcd 100644 --- a/caddyhttp/proxy/proxy.go +++ b/caddyhttp/proxy/proxy.go @@ -108,7 +108,7 @@ func (p Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { if nameURL, err := url.Parse(host.Name); err == nil { outreq.Host = nameURL.Host if proxy == nil { - proxy = NewSingleHostReverseProxy(nameURL, host.WithoutPathPrefix) + proxy = NewSingleHostReverseProxy(nameURL, host.WithoutPathPrefix, 0) } // use upstream credentials by default diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index de9fa53ba..866cde958 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -716,11 +716,11 @@ func newFakeUpstream(name string, insecure bool) *fakeUpstream { from: "/", host: &UpstreamHost{ Name: name, - ReverseProxy: NewSingleHostReverseProxy(uri, ""), + ReverseProxy: NewSingleHostReverseProxy(uri, "", 0), }, } if insecure { - u.host.ReverseProxy.Transport = InsecureTransport + u.host.ReverseProxy.UseInsecureTransport() } return u } @@ -744,7 +744,7 @@ func (u *fakeUpstream) Select() *UpstreamHost { } u.host = &UpstreamHost{ Name: u.name, - ReverseProxy: NewSingleHostReverseProxy(uri, u.without), + ReverseProxy: NewSingleHostReverseProxy(uri, u.without, 0), } } return u.host @@ -785,7 +785,7 @@ func (u *fakeWsUpstream) Select() *UpstreamHost { uri, _ := url.Parse(u.name) return &UpstreamHost{ Name: u.name, - ReverseProxy: NewSingleHostReverseProxy(uri, u.without), + ReverseProxy: NewSingleHostReverseProxy(uri, u.without, 0), UpstreamHeaders: http.Header{ "Connection": {"{>Connection}"}, "Upgrade": {"{>Upgrade}"}}, diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go index e6f759dd5..eadd7e3ad 100644 --- a/caddyhttp/proxy/reverseproxy.go +++ b/caddyhttp/proxy/reverseproxy.go @@ -83,7 +83,7 @@ func socketDial(hostName string) func(network, addr string) (conn net.Conn, err // the target request will be for /base/dir. // Without logic: target's path is "/", incoming is "/api/messages", // without is "/api", then the target request will be for /messages. -func NewSingleHostReverseProxy(target *url.URL, without string) *ReverseProxy { +func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) *ReverseProxy { targetQuery := target.RawQuery director := func(req *http.Request) { if target.Scheme == "unix" { @@ -122,10 +122,44 @@ func NewSingleHostReverseProxy(target *url.URL, without string) *ReverseProxy { rp.Transport = &http.Transport{ Dial: socketDial(target.String()), } + } else if keepalive != 0 { + rp.Transport = &http.Transport{ + Proxy: http.ProxyFromEnvironment, + Dial: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).Dial, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } + if keepalive < 0 { + rp.Transport.(*http.Transport).DisableKeepAlives = true + } else { + rp.Transport.(*http.Transport).MaxIdleConnsPerHost = keepalive + } } return rp } +// InsecureTransport is used to facilitate HTTPS proxying +// when it is OK for upstream to be using a bad certificate, +// since this transport skips verification. +func (rp *ReverseProxy) UseInsecureTransport() { + if rp.Transport == nil { + rp.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}, + } + } else if transport, ok := rp.Transport.(*http.Transport); ok { + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + } +} + func copyHeader(dst, src http.Header) { for k, vv := range src { for _, v := range vv { @@ -147,19 +181,6 @@ var hopHeaders = []string{ "Upgrade", } -// InsecureTransport is used to facilitate HTTPS proxying -// when it is OK for upstream to be using a bad certificate, -// since this transport skips verification. -var InsecureTransport http.RoundTripper = &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}, -} - type respUpdateFn func(resp *http.Response) func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, outreq *http.Request, respUpdateFn respUpdateFn) error { diff --git a/caddyhttp/proxy/upstream.go b/caddyhttp/proxy/upstream.go index e5a40ce25..fcbf15d96 100644 --- a/caddyhttp/proxy/upstream.go +++ b/caddyhttp/proxy/upstream.go @@ -25,6 +25,7 @@ type staticUpstream struct { downstreamHeaders http.Header Hosts HostPool Policy Policy + KeepAlive int insecureSkipVerify bool FailTimeout time.Duration @@ -154,9 +155,9 @@ func (u *staticUpstream) NewHost(host string) (*UpstreamHost, error) { return nil, err } - uh.ReverseProxy = NewSingleHostReverseProxy(baseURL, uh.WithoutPathPrefix) + uh.ReverseProxy = NewSingleHostReverseProxy(baseURL, uh.WithoutPathPrefix, u.KeepAlive) if u.insecureSkipVerify { - uh.ReverseProxy.Transport = InsecureTransport + uh.ReverseProxy.UseInsecureTransport() } return uh, nil @@ -312,6 +313,15 @@ func parseBlock(c *caddyfile.Dispenser, u *staticUpstream) error { u.IgnoredSubPaths = ignoredPaths case "insecure_skip_verify": u.insecureSkipVerify = true + case "keepalive": + if !c.NextArg() { + return c.ArgErr() + } + n, err := strconv.Atoi(c.Val()) + if err != nil { + return err + } + u.KeepAlive = n default: return c.Errf("unknown property '%s'", c.Val()) } From 5b5e36529544209b0c9a3e389333809f47c30535 Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Fri, 5 Aug 2016 15:41:32 -0700 Subject: [PATCH 2/2] Instead of treating 0 is a default value, use http.DefaultMaxIdleConnsPerHost --- caddyhttp/proxy/proxy.go | 2 +- caddyhttp/proxy/proxy_test.go | 6 +++--- caddyhttp/proxy/reverseproxy.go | 7 +++++-- caddyhttp/proxy/upstream.go | 4 ++++ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/caddyhttp/proxy/proxy.go b/caddyhttp/proxy/proxy.go index ed4383dcd..d1d695413 100644 --- a/caddyhttp/proxy/proxy.go +++ b/caddyhttp/proxy/proxy.go @@ -108,7 +108,7 @@ func (p Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { if nameURL, err := url.Parse(host.Name); err == nil { outreq.Host = nameURL.Host if proxy == nil { - proxy = NewSingleHostReverseProxy(nameURL, host.WithoutPathPrefix, 0) + proxy = NewSingleHostReverseProxy(nameURL, host.WithoutPathPrefix, http.DefaultMaxIdleConnsPerHost) } // use upstream credentials by default diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index 866cde958..9b94e6ec7 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -716,7 +716,7 @@ func newFakeUpstream(name string, insecure bool) *fakeUpstream { from: "/", host: &UpstreamHost{ Name: name, - ReverseProxy: NewSingleHostReverseProxy(uri, "", 0), + ReverseProxy: NewSingleHostReverseProxy(uri, "", http.DefaultMaxIdleConnsPerHost), }, } if insecure { @@ -744,7 +744,7 @@ func (u *fakeUpstream) Select() *UpstreamHost { } u.host = &UpstreamHost{ Name: u.name, - ReverseProxy: NewSingleHostReverseProxy(uri, u.without, 0), + ReverseProxy: NewSingleHostReverseProxy(uri, u.without, http.DefaultMaxIdleConnsPerHost), } } return u.host @@ -785,7 +785,7 @@ func (u *fakeWsUpstream) Select() *UpstreamHost { uri, _ := url.Parse(u.name) return &UpstreamHost{ Name: u.name, - ReverseProxy: NewSingleHostReverseProxy(uri, u.without, 0), + ReverseProxy: NewSingleHostReverseProxy(uri, u.without, http.DefaultMaxIdleConnsPerHost), UpstreamHeaders: http.Header{ "Connection": {"{>Connection}"}, "Upgrade": {"{>Upgrade}"}}, diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go index eadd7e3ad..30ac39915 100644 --- a/caddyhttp/proxy/reverseproxy.go +++ b/caddyhttp/proxy/reverseproxy.go @@ -122,7 +122,10 @@ func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) * rp.Transport = &http.Transport{ Dial: socketDial(target.String()), } - } else if keepalive != 0 { + } else if keepalive != http.DefaultMaxIdleConnsPerHost { + // if keepalive is equal to the default, + // just use default transport, to avoid creating + // a brand new transport rp.Transport = &http.Transport{ Proxy: http.ProxyFromEnvironment, Dial: (&net.Dialer{ @@ -132,7 +135,7 @@ func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) * TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, } - if keepalive < 0 { + if keepalive == 0 { rp.Transport.(*http.Transport).DisableKeepAlives = true } else { rp.Transport.(*http.Transport).MaxIdleConnsPerHost = keepalive diff --git a/caddyhttp/proxy/upstream.go b/caddyhttp/proxy/upstream.go index fcbf15d96..36620995f 100644 --- a/caddyhttp/proxy/upstream.go +++ b/caddyhttp/proxy/upstream.go @@ -55,6 +55,7 @@ func NewStaticUpstreams(c caddyfile.Dispenser) ([]Upstream, error) { FailTimeout: 10 * time.Second, MaxFails: 1, MaxConns: 0, + KeepAlive: http.DefaultMaxIdleConnsPerHost, } if !c.Args(&upstream.from) { @@ -321,6 +322,9 @@ func parseBlock(c *caddyfile.Dispenser, u *staticUpstream) error { if err != nil { return err } + if n < 0 { + return c.ArgErr() + } u.KeepAlive = n default: return c.Errf("unknown property '%s'", c.Val())