From 96fec4b9691f49d5f36d82afcdced01568361473 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Thu, 28 Apr 2022 11:34:36 -0700 Subject: [PATCH] net/tshttpproxy: synology: pick proxy by scheme This updates the fix from #4562 to pick the proxy based on the request scheme. Updates #4395, #2605, #4562 Signed-off-by: James Tucker --- net/tshttpproxy/tshttpproxy_synology.go | 70 ++++---- net/tshttpproxy/tshttpproxy_synology_test.go | 177 ++++++++++++++----- 2 files changed, 175 insertions(+), 72 deletions(-) diff --git a/net/tshttpproxy/tshttpproxy_synology.go b/net/tshttpproxy/tshttpproxy_synology.go index 7e99c3c52..bf33abf78 100644 --- a/net/tshttpproxy/tshttpproxy_synology.go +++ b/net/tshttpproxy/tshttpproxy_synology.go @@ -33,8 +33,9 @@ var ( var cache struct { sync.Mutex - proxy *url.URL - updated time.Time + httpProxy *url.URL + httpsProxy *url.URL + updated time.Time } func synologyProxyFromConfigCached(req *http.Request) (*url.URL, error) { @@ -45,34 +46,36 @@ func synologyProxyFromConfigCached(req *http.Request) (*url.URL, error) { cache.Lock() defer cache.Unlock() + var err error modtime := mtime(synologyProxyConfigPath) - if cache.updated == modtime { - return cache.proxy, nil + if modtime != cache.updated { + cache.httpProxy, cache.httpsProxy, err = synologyProxiesFromConfig() + cache.updated = modtime } - val, err := synologyProxyFromConfig(req) - cache.proxy = val - - cache.updated = modtime - - return val, err + if req.URL.Scheme == "https" { + return cache.httpsProxy, err + } + return cache.httpProxy, err } -func synologyProxyFromConfig(req *http.Request) (*url.URL, error) { +func synologyProxiesFromConfig() (*url.URL, *url.URL, error) { r, err := openSynologyProxyConf() if err != nil { if os.IsNotExist(err) { - return nil, nil + return nil, nil, nil } - return nil, err + return nil, nil, err } defer r.Close() return parseSynologyConfig(r) } -func parseSynologyConfig(r io.Reader) (*url.URL, error) { +// parseSynologyConfig parses the Synology proxy configuration, and returns any +// http proxy, and any https proxy respectively, or an error if parsing fails. +func parseSynologyConfig(r io.Reader) (*url.URL, *url.URL, error) { cfg := map[string]string{} if err := lineread.Reader(r, func(line []byte) error { @@ -89,36 +92,43 @@ func parseSynologyConfig(r io.Reader) (*url.URL, error) { cfg[string(key)] = string(value) return nil }); err != nil { - return nil, err + return nil, nil, err } if cfg["proxy_enabled"] != "yes" { - return nil, nil + return nil, nil, nil } - proxyURL := &url.URL{ - Scheme: "http", // regardless of proxy type - } + httpProxyURL := new(url.URL) + httpsProxyURL := new(url.URL) if cfg["auth_enabled"] == "yes" { - proxyURL.User = url.UserPassword(cfg["proxy_user"], cfg["proxy_pwd"]) + httpProxyURL.User = url.UserPassword(cfg["proxy_user"], cfg["proxy_pwd"]) + httpsProxyURL.User = url.UserPassword(cfg["proxy_user"], cfg["proxy_pwd"]) } - host, port := cfg["https_host"], cfg["https_port"] - if host == "" { - host, port = cfg["http_host"], cfg["http_port"] - } + // As far as we are aware, synology does not support tls proxies. + httpProxyURL.Scheme = "http" + httpsProxyURL.Scheme = "http" + httpsProxyURL = addHostPort(httpsProxyURL, cfg["https_host"], cfg["https_port"]) + httpProxyURL = addHostPort(httpProxyURL, cfg["http_host"], cfg["http_port"]) + + return httpProxyURL, httpsProxyURL, nil +} + +// addHostPort adds to u the given host and port and returns the updated url, or +// if host is empty, it returns nil. +func addHostPort(u *url.URL, host, port string) *url.URL { if host == "" { - return nil, nil + return nil } - if port != "" { - proxyURL.Host = net.JoinHostPort(host, port) + if port == "" { + u.Host = host } else { - proxyURL.Host = host + u.Host = net.JoinHostPort(host, port) } - - return proxyURL, nil + return u } // mtime stat's path and returns its modification time. If path does not exist, diff --git a/net/tshttpproxy/tshttpproxy_synology_test.go b/net/tshttpproxy/tshttpproxy_synology_test.go index 12a7a231e..ee10aaff9 100644 --- a/net/tshttpproxy/tshttpproxy_synology_test.go +++ b/net/tshttpproxy/tshttpproxy_synology_test.go @@ -22,7 +22,7 @@ import ( ) func TestSynologyProxyFromConfigCached(t *testing.T) { - req, err := http.NewRequest("GET", "https://example.org/", nil) + req, err := http.NewRequest("GET", "http://example.org/", nil) if err != nil { t.Fatal(err) } @@ -37,7 +37,8 @@ func TestSynologyProxyFromConfigCached(t *testing.T) { } cache.updated = time.Time{} - cache.proxy = nil + cache.httpProxy = nil + cache.httpsProxy = nil if val, err := synologyProxyFromConfigCached(req); val != nil || err != nil { t.Fatalf("got %s, %v; want nil, nil", val, err) @@ -46,19 +47,25 @@ func TestSynologyProxyFromConfigCached(t *testing.T) { if got, want := cache.updated, time.Unix(0, 0); got != want { t.Fatalf("got %s, want %s", got, want) } - if cache.proxy != nil { - t.Fatalf("got %s, want nil", cache.proxy) + if cache.httpProxy != nil { + t.Fatalf("got %s, want nil", cache.httpProxy) + } + if cache.httpsProxy != nil { + t.Fatalf("got %s, want nil", cache.httpsProxy) } }) t.Run("config file updated", func(t *testing.T) { cache.updated = time.Now() - cache.proxy = nil + cache.httpProxy = nil + cache.httpsProxy = nil if err := ioutil.WriteFile(synologyProxyConfigPath, []byte(` proxy_enabled=yes http_host=10.0.0.55 http_port=80 +https_host=10.0.0.66 +https_port=443 `), 0600); err != nil { t.Fatal(err) } @@ -67,6 +74,14 @@ http_port=80 if err != nil { t.Fatal(err) } + + if cache.httpProxy == nil { + t.Fatal("http proxy was not cached") + } + if cache.httpsProxy == nil { + t.Fatal("https proxy was not cached") + } + if want := urlMustParse("http://10.0.0.55:80"); val.String() != want.String() { t.Fatalf("got %s; want %s", val, want) } @@ -74,7 +89,8 @@ http_port=80 t.Run("config file removed", func(t *testing.T) { cache.updated = time.Now() - cache.proxy = urlMustParse("http://127.0.0.1/") + cache.httpProxy = urlMustParse("http://127.0.0.1/") + cache.httpsProxy = urlMustParse("http://127.0.0.1/") if err := os.Remove(synologyProxyConfigPath); err != nil && !os.IsNotExist(err) { t.Fatal(err) @@ -87,13 +103,62 @@ http_port=80 if val != nil { t.Fatalf("got %s; want nil", val) } - if cache.proxy != nil { - t.Fatalf("got %s, want nil", cache.proxy) + if cache.httpProxy != nil { + t.Fatalf("got %s, want nil", cache.httpProxy) + } + if cache.httpsProxy != nil { + t.Fatalf("got %s, want nil", cache.httpsProxy) + } + }) + + t.Run("picks proxy from request scheme", func(t *testing.T) { + cache.updated = time.Now() + cache.httpProxy = nil + cache.httpsProxy = nil + + if err := ioutil.WriteFile(synologyProxyConfigPath, []byte(` +proxy_enabled=yes +http_host=10.0.0.55 +http_port=80 +https_host=10.0.0.66 +https_port=443 + `), 0600); err != nil { + t.Fatal(err) + } + + httpReq, err := http.NewRequest("GET", "http://example.com", nil) + if err != nil { + t.Fatal(err) + } + val, err := synologyProxyFromConfigCached(httpReq) + if err != nil { + t.Fatal(err) + } + if val == nil { + t.Fatalf("got nil, want an http URL") + } + if got, want := val.String(), "http://10.0.0.55:80"; got != want { + t.Fatalf("got %q, want %q", got, want) + } + + httpsReq, err := http.NewRequest("GET", "https://example.com", nil) + if err != nil { + t.Fatal(err) + } + val, err = synologyProxyFromConfigCached(httpsReq) + if err != nil { + t.Fatal(err) + } + if val == nil { + t.Fatalf("got nil, want an http URL") + } + if got, want := val.String(), "http://10.0.0.66:443"; got != want { + t.Fatalf("got %q, want %q", got, want) } }) } -func TestSynologyProxyFromConfig(t *testing.T) { +func TestSynologyProxiesFromConfig(t *testing.T) { var ( openReader io.ReadCloser openErr error @@ -104,11 +169,6 @@ func TestSynologyProxyFromConfig(t *testing.T) { } defer func() { openSynologyProxyConf = origOpen }() - req, err := http.NewRequest("GET", "https://example.com/", nil) - if err != nil { - t.Fatal(err) - } - t.Run("with config", func(t *testing.T) { mc := &mustCloser{Reader: strings.NewReader(` proxy_user=foo @@ -125,13 +185,21 @@ http_port=80 defer mc.check(t) openReader = mc - proxyURL, err := synologyProxyFromConfig(req) + httpProxy, httpsProxy, err := synologyProxiesFromConfig() if got, want := err, openErr; got != want { t.Fatalf("got %s, want %s", got, want) } - if got, want := proxyURL, urlMustParse("http://foo:bar@10.0.0.66:8443"); got.String() != want.String() { + if got, want := httpsProxy, urlMustParse("http://foo:bar@10.0.0.66:8443"); got.String() != want.String() { + t.Fatalf("got %s, want %s", got, want) + } + + if got, want := err, openErr; got != want { + t.Fatalf("got %s, want %s", got, want) + } + + if got, want := httpProxy, urlMustParse("http://foo:bar@10.0.0.55:80"); got.String() != want.String() { t.Fatalf("got %s, want %s", got, want) } @@ -141,12 +209,15 @@ http_port=80 openReader = nil openErr = os.ErrNotExist - proxyURL, err := synologyProxyFromConfig(req) + httpProxy, httpsProxy, err := synologyProxiesFromConfig() if err != nil { t.Fatalf("expected no error, got %s", err) } - if proxyURL != nil { - t.Fatalf("expected no url, got %s", proxyURL) + if httpProxy != nil { + t.Fatalf("expected no url, got %s", httpProxy) + } + if httpsProxy != nil { + t.Fatalf("expected no url, got %s", httpsProxy) } }) @@ -154,12 +225,15 @@ http_port=80 openReader = nil openErr = errors.New("example error") - proxyURL, err := synologyProxyFromConfig(req) + httpProxy, httpsProxy, err := synologyProxiesFromConfig() if err != openErr { t.Fatalf("expected %s, got %s", openErr, err) } - if proxyURL != nil { - t.Fatalf("expected no url, got %s", proxyURL) + if httpProxy != nil { + t.Fatalf("expected no url, got %s", httpProxy) + } + if httpsProxy != nil { + t.Fatalf("expected no url, got %s", httpsProxy) } }) @@ -167,9 +241,10 @@ http_port=80 func TestParseSynologyConfig(t *testing.T) { cases := map[string]struct { - input string - url *url.URL - err error + input string + httpProxy *url.URL + httpsProxy *url.URL + err error }{ "populated": { input: ` @@ -184,8 +259,9 @@ https_port=8443 http_host=10.0.0.55 http_port=80 `, - url: urlMustParse("http://foo:bar@10.0.0.66:8443"), - err: nil, + httpProxy: urlMustParse("http://foo:bar@10.0.0.55:80"), + httpsProxy: urlMustParse("http://foo:bar@10.0.0.66:8443"), + err: nil, }, "no-auth": { input: ` @@ -200,10 +276,11 @@ https_port=8443 http_host=10.0.0.55 http_port=80 `, - url: urlMustParse("http://10.0.0.66:8443"), - err: nil, + httpProxy: urlMustParse("http://10.0.0.55:80"), + httpsProxy: urlMustParse("http://10.0.0.66:8443"), + err: nil, }, - "http": { + "http-only": { input: ` proxy_user=foo proxy_pwd=bar @@ -216,8 +293,9 @@ https_port=8443 http_host=10.0.0.55 http_port=80 `, - url: urlMustParse("http://foo:bar@10.0.0.55:80"), - err: nil, + httpProxy: urlMustParse("http://foo:bar@10.0.0.55:80"), + httpsProxy: nil, + err: nil, }, "empty": { input: ` @@ -232,14 +310,15 @@ https_port= http_host= http_port= `, - url: nil, - err: nil, + httpProxy: nil, + httpsProxy: nil, + err: nil, }, } for name, example := range cases { t.Run(name, func(t *testing.T) { - url, err := parseSynologyConfig(strings.NewReader(example.input)) + httpProxy, httpsProxy, err := parseSynologyConfig(strings.NewReader(example.input)) if err != example.err { t.Fatal(err) } @@ -247,18 +326,32 @@ http_port= return } - if url == nil && example.url == nil { - return + if example.httpProxy == nil && httpProxy != nil { + t.Fatalf("got %s, want nil", httpProxy) } - if example.url == nil { - if url != nil { - t.Fatalf("got %s, want nil", url) + if example.httpProxy != nil { + if httpProxy == nil { + t.Fatalf("got nil, want %s", example.httpProxy) } + + if got, want := example.httpProxy.String(), httpProxy.String(); got != want { + t.Fatalf("got %s, want %s", got, want) + } + } + + if example.httpsProxy == nil && httpsProxy != nil { + t.Fatalf("got %s, want nil", httpProxy) } - if got, want := url.String(), example.url.String(); got != want { - t.Fatalf("got %s, want %s", got, want) + if example.httpsProxy != nil { + if httpsProxy == nil { + t.Fatalf("got nil, want %s", example.httpsProxy) + } + + if got, want := example.httpsProxy.String(), httpsProxy.String(); got != want { + t.Fatalf("got %s, want %s", got, want) + } } }) }