From 215a91ae8f7c060361a11cd684f8060eaa616fa2 Mon Sep 17 00:00:00 2001
From: Yuxi Xie <xieyuxi@stripe.com>
Date: Thu, 9 Nov 2023 10:20:29 -0800
Subject: [PATCH] addressed comments

---
 https.go      | 30 +++++++++++++++++++-----------
 https_test.go |  4 ++--
 proxy.go      | 15 ++++++++-------
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/https.go b/https.go
index 5794e9bd..3908e580 100644
--- a/https.go
+++ b/https.go
@@ -118,7 +118,7 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request
 			host += ":80"
 		}
 
-		httpsProxy, err := httpsProxy(r.URL, proxy.HttpsProxyAddr)
+		httpsProxy, err := httpsProxyAddr(r.URL, proxy.HttpsProxyAddr)
 		if err != nil {
 			ctx.Warnf("Error configuring HTTPS proxy err=%q url=%q", err, r.URL.String())
 		}
@@ -398,14 +398,20 @@ func copyAndClose(ctx *ProxyCtx, dst, src *net.TCPConn) {
 	src.CloseRead()
 }
 
-func dialerFromEnv(proxy *ProxyHttpServer) func(network, addr string) (net.Conn, error) {
-	https_proxy := os.Getenv("HTTPS_PROXY")
+// dialerFromProxy gets the HttpsProxyAddr from proxy to create a dialer.
+// When the HttpsProxyAddr from proxy is empty, use the HTTPS_PROXY, https_proxy from environment variables.
+func dialerFromProxy(proxy *ProxyHttpServer) func(network, addr string) (net.Conn, error) {
+	https_proxy := proxy.HttpsProxyAddr
 	if https_proxy == "" {
-		https_proxy = os.Getenv("https_proxy")
-	}
-	if https_proxy == "" {
-		return nil
+		https_proxy = os.Getenv("HTTPS_PROXY")
+		if https_proxy == "" {
+			https_proxy = os.Getenv("https_proxy")
+		}
+		if https_proxy == "" {
+			return nil
+		}
 	}
+
 	return proxy.NewConnectDialToProxy(https_proxy)
 }
 
@@ -559,13 +565,15 @@ func (proxy *ProxyHttpServer) connectDialProxyWithContext(ctx *ProxyCtx, proxyHo
 	return c, nil
 }
 
-// httpsProxy allows goproxy to respect no_proxy env vars
+// httpsProxyAddr function uses the address in httpsProxy parameter. 
+// When the httpProxyAddr parameter is empty, uses the HTTPS_PROXY, https_proxy from environment variables.
+// httpsProxyAddr function allows goproxy to respect no_proxy env vars
 // https://github.com/stripe/goproxy/pull/5
-func httpsProxy(reqURL *url.URL, httpProxyAddr string) (string, error) {
+func httpsProxyAddr(reqURL *url.URL, httpsProxy string) (string, error) {
 	cfg := httpproxy.FromEnvironment()
 
-	if httpProxyAddr != "" {
-		cfg.HTTPSProxy = httpProxyAddr
+	if httpsProxy != "" {
+		cfg.HTTPSProxy = httpsProxy
 	}
 
 	// We only use this codepath for HTTPS CONNECT proxies so we shouldn't
diff --git a/https_test.go b/https_test.go
index ce960752..5e6c8833 100644
--- a/https_test.go
+++ b/https_test.go
@@ -26,7 +26,7 @@ var proxytests = map[string]struct {
 
 var envKeys = []string{"no_proxy", "http_proxy", "https_proxy", "NO_PROXY", "HTTP_PROXY", "HTTPS_PROXY"}
 
-func TestHttpsProxy(t *testing.T) {
+func TestHttpsProxyAddr(t *testing.T) {
 	for _, k := range envKeys {
 		v, ok := os.LookupEnv(k)
 		if ok {
@@ -52,7 +52,7 @@ func TestHttpsProxy(t *testing.T) {
 				t.Fatalf("bad test input URL %s: %v", spec.url, err)
 			}
 
-			actual, err := httpsProxy(url, spec.customHttpsProxy)
+			actual, err := httpsProxyAddr(url, spec.customHttpsProxy)
 			if err != nil {
 				t.Fatalf("unexpected error parsing proxy from env: %#v", err)
 			}
diff --git a/proxy.go b/proxy.go
index eb4cb2a2..c4d34385 100644
--- a/proxy.go
+++ b/proxy.go
@@ -231,23 +231,24 @@ func NewProxyHttpServer(opts ...ProxyHttpServerOptions) *ProxyHttpServer {
 		Tr: &http.Transport{TLSClientConfig: tlsClientSkipVerify, Proxy: http.ProxyFromEnvironment},
 	}
 
-	cfg := httpproxy.FromEnvironment()
+	// httpProxyCfg holds configuration for HTTP proxy settings. See FromEnvironment for details.
+	httpProxyCfg := httpproxy.FromEnvironment()
+
 	if appliedOpts.httpProxyAddr != "" {
 		proxy.HttpProxyAddr = appliedOpts.httpProxyAddr
-		cfg.HTTPProxy = appliedOpts.httpProxyAddr
+		httpProxyCfg.HTTPProxy = appliedOpts.httpProxyAddr
 	}
 
 	if appliedOpts.httpsProxyAddr != "" {
 		proxy.HttpsProxyAddr = appliedOpts.httpsProxyAddr
-		cfg.HTTPSProxy = appliedOpts.httpsProxyAddr
-		proxy.ConnectDial = proxy.NewConnectDialToProxy(appliedOpts.httpsProxyAddr)
-	} else {
-		proxy.ConnectDial = dialerFromEnv(&proxy)
+		httpProxyCfg.HTTPSProxy = appliedOpts.httpsProxyAddr
 	}
 
+	proxy.ConnectDial = dialerFromProxy(&proxy)
+	
 	if appliedOpts.httpProxyAddr != "" || appliedOpts.httpsProxyAddr != "" {
 		proxy.Tr.Proxy = func(req *http.Request) (*url.URL, error) {
-			return cfg.ProxyFunc()(req.URL)
+			return httpProxyCfg.ProxyFunc()(req.URL)
 		}
 	}