From 13808ccc216713fe7ceef0045f0d98e689421cbf Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Fri, 5 Dec 2025 13:36:33 +0200 Subject: [PATCH 1/3] refactor(api): rename applyProxy to NewProxyTransport - NewProxyTransport creates a cloned transport which does the required proxy handling in dial etc. - Remove use of 'customTransport' --- internal/api/api.go | 42 +++++++++++++++++++++--------------------- internal/api/proxy.go | 14 ++++---------- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 4dfad40d3b..7231263e6b 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -87,24 +87,10 @@ type ClientOpts struct { ProxyPath string } -// NewClient creates a new API client. -func NewClient(opts ClientOpts) Client { - if opts.Out == nil { - panic("unexpected nil out option") - } - - flags := opts.Flags - if flags == nil { - flags = defaultFlags() - } - - httpClient := http.DefaultClient - +func buildTransport(opts ClientOpts, flags *Flags) *http.Transport { transport := http.DefaultTransport.(*http.Transport).Clone() - customTransport := false if flags.insecureSkipVerify != nil && *flags.insecureSkipVerify { - customTransport = true transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} } @@ -112,14 +98,28 @@ func NewClient(opts ClientOpts) Client { transport.TLSClientConfig = &tls.Config{} } - if applyProxy(transport, opts.ProxyURL, opts.ProxyPath) { - customTransport = true + if opts.ProxyURL != nil || opts.ProxyPath != "" { + transport = NewProxyTransport(transport, opts.ProxyURL, opts.ProxyPath) } - if customTransport { - httpClient = &http.Client{ - Transport: transport, - } + return transport +} + +// NewClient creates a new API client. +func NewClient(opts ClientOpts) Client { + if opts.Out == nil { + panic("unexpected nil out option") + } + + flags := opts.Flags + if flags == nil { + flags = defaultFlags() + } + + transport := buildTransport(opts, flags) + + httpClient := &http.Client{ + Transport: transport, } return &client{ diff --git a/internal/api/proxy.go b/internal/api/proxy.go index 3cf8673829..d98c065ed0 100644 --- a/internal/api/proxy.go +++ b/internal/api/proxy.go @@ -11,10 +11,9 @@ import ( "net/url" ) -func applyProxy(transport *http.Transport, proxyURL *url.URL, proxyPath string) (applied bool) { - if proxyURL == nil && proxyPath == "" { - return false - } +func NewProxyTransport(base *http.Transport, proxyURL *url.URL, proxyPath string) *http.Transport { + // Clone so that we don't change the original transport + transport := base.Clone() handshakeTLS := func(ctx context.Context, conn net.Conn, addr string) (net.Conn, error) { // Extract the hostname (without the port) for TLS SNI @@ -34,8 +33,6 @@ func applyProxy(transport *http.Transport, proxyURL *url.URL, proxyPath string) return tlsConn, nil } - proxyApplied := false - if proxyPath != "" { dial := func(ctx context.Context, _, _ string) (net.Conn, error) { d := net.Dialer{} @@ -52,13 +49,11 @@ func applyProxy(transport *http.Transport, proxyURL *url.URL, proxyPath string) transport.DialTLSContext = dialTLS // clear out any system proxy settings transport.Proxy = nil - proxyApplied = true } else if proxyURL != nil { switch proxyURL.Scheme { case "socks5", "socks5h": // SOCKS proxies work out of the box - no need to manually dial transport.Proxy = http.ProxyURL(proxyURL) - proxyApplied = true case "http", "https": dial := func(ctx context.Context, network, addr string) (net.Conn, error) { // Dial the proxy @@ -130,9 +125,8 @@ func applyProxy(transport *http.Transport, proxyURL *url.URL, proxyPath string) transport.DialTLSContext = dialTLS // clear out any system proxy settings transport.Proxy = nil - proxyApplied = true } } - return proxyApplied + return transport } From fab6e434164b352b0ce7a1913e84b468aadcc617 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Fri, 5 Dec 2025 15:31:28 +0200 Subject: [PATCH 2/3] add test --- internal/api/api_test.go | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 0aaf3ec14a..c5f91dc908 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -1,3 +1,40 @@ package api -// TODO: implement a super basic GraphQL server that can return canned results. +import ( + "net/url" + "testing" +) + +func TestBuildTransport(t *testing.T) { + boolPtr := func(b bool) *bool { return &b } + + t.Run("insecure skip verify", func(t *testing.T) { + transport := buildTransport(ClientOpts{}, &Flags{insecureSkipVerify: boolPtr(true)}) + if !transport.TLSClientConfig.InsecureSkipVerify { + t.Error("expected InsecureSkipVerify to be true") + } + }) + + t.Run("unix socket proxy clears Proxy", func(t *testing.T) { + transport := buildTransport(ClientOpts{ProxyPath: "/tmp/test.sock"}, defaultFlags()) + if transport.Proxy != nil { + t.Error("expected Proxy to be nil") + } + }) + + // http.DefaultTransport.Dial / DialTLS is already set and we can't compare two funcs + // so our best effort here is to just check Proxy is nil / not nill based on the ProxyURL + t.Run("http proxy clears Proxy", func(t *testing.T) { + transport := buildTransport(ClientOpts{ProxyURL: &url.URL{Scheme: "http", Host: "proxy:8080"}}, defaultFlags()) + if transport.Proxy != nil { + t.Error("expected Proxy to be nil") + } + }) + + t.Run("socks5 proxy sets Proxy", func(t *testing.T) { + transport := buildTransport(ClientOpts{ProxyURL: &url.URL{Scheme: "socks5", Host: "proxy:1080"}}, defaultFlags()) + if transport.Proxy == nil { + t.Error("expected Proxy to be set") + } + }) +} From 5ebfac60c55f343c48108b6ed30faf406d5e8e38 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Mon, 8 Dec 2025 11:28:18 +0200 Subject: [PATCH 3/3] review comments - add doc string - unexport - delete low value tests --- internal/api/api.go | 2 +- internal/api/api_test.go | 40 ---------------------------------------- internal/api/proxy.go | 29 ++++++++++++++++------------- 3 files changed, 17 insertions(+), 54 deletions(-) delete mode 100644 internal/api/api_test.go diff --git a/internal/api/api.go b/internal/api/api.go index 7231263e6b..e0bc234558 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -99,7 +99,7 @@ func buildTransport(opts ClientOpts, flags *Flags) *http.Transport { } if opts.ProxyURL != nil || opts.ProxyPath != "" { - transport = NewProxyTransport(transport, opts.ProxyURL, opts.ProxyPath) + transport = withProxyTransport(transport, opts.ProxyURL, opts.ProxyPath) } return transport diff --git a/internal/api/api_test.go b/internal/api/api_test.go deleted file mode 100644 index c5f91dc908..0000000000 --- a/internal/api/api_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package api - -import ( - "net/url" - "testing" -) - -func TestBuildTransport(t *testing.T) { - boolPtr := func(b bool) *bool { return &b } - - t.Run("insecure skip verify", func(t *testing.T) { - transport := buildTransport(ClientOpts{}, &Flags{insecureSkipVerify: boolPtr(true)}) - if !transport.TLSClientConfig.InsecureSkipVerify { - t.Error("expected InsecureSkipVerify to be true") - } - }) - - t.Run("unix socket proxy clears Proxy", func(t *testing.T) { - transport := buildTransport(ClientOpts{ProxyPath: "/tmp/test.sock"}, defaultFlags()) - if transport.Proxy != nil { - t.Error("expected Proxy to be nil") - } - }) - - // http.DefaultTransport.Dial / DialTLS is already set and we can't compare two funcs - // so our best effort here is to just check Proxy is nil / not nill based on the ProxyURL - t.Run("http proxy clears Proxy", func(t *testing.T) { - transport := buildTransport(ClientOpts{ProxyURL: &url.URL{Scheme: "http", Host: "proxy:8080"}}, defaultFlags()) - if transport.Proxy != nil { - t.Error("expected Proxy to be nil") - } - }) - - t.Run("socks5 proxy sets Proxy", func(t *testing.T) { - transport := buildTransport(ClientOpts{ProxyURL: &url.URL{Scheme: "socks5", Host: "proxy:1080"}}, defaultFlags()) - if transport.Proxy == nil { - t.Error("expected Proxy to be set") - } - }) -} diff --git a/internal/api/proxy.go b/internal/api/proxy.go index d98c065ed0..9589b9beb5 100644 --- a/internal/api/proxy.go +++ b/internal/api/proxy.go @@ -11,10 +11,13 @@ import ( "net/url" ) -func NewProxyTransport(base *http.Transport, proxyURL *url.URL, proxyPath string) *http.Transport { - // Clone so that we don't change the original transport - transport := base.Clone() - +// withProxyTransport modifies the given transport to handle proxying of unix, socks5 and http connections. +// +// Note: baseTransport is considered to be a clone created with transport.Clone() +// +// - If a the proxyPath is not empty, a unix socket proxy is created. +// - Otherwise, the proxyURL is used to determine if we should proxy socks5 / http connections +func withProxyTransport(baseTransport *http.Transport, proxyURL *url.URL, proxyPath string) *http.Transport { handshakeTLS := func(ctx context.Context, conn net.Conn, addr string) (net.Conn, error) { // Extract the hostname (without the port) for TLS SNI host, _, err := net.SplitHostPort(addr) @@ -25,7 +28,7 @@ func NewProxyTransport(base *http.Transport, proxyURL *url.URL, proxyPath string ServerName: host, // Pull InsecureSkipVerify from the target host transport // so that insecure-skip-verify flag settings are honored for the proxy server - InsecureSkipVerify: transport.TLSClientConfig.InsecureSkipVerify, + InsecureSkipVerify: baseTransport.TLSClientConfig.InsecureSkipVerify, }) if err := tlsConn.HandshakeContext(ctx); err != nil { return nil, err @@ -45,15 +48,15 @@ func NewProxyTransport(base *http.Transport, proxyURL *url.URL, proxyPath string } return handshakeTLS(ctx, conn, addr) } - transport.DialContext = dial - transport.DialTLSContext = dialTLS + baseTransport.DialContext = dial + baseTransport.DialTLSContext = dialTLS // clear out any system proxy settings - transport.Proxy = nil + baseTransport.Proxy = nil } else if proxyURL != nil { switch proxyURL.Scheme { case "socks5", "socks5h": // SOCKS proxies work out of the box - no need to manually dial - transport.Proxy = http.ProxyURL(proxyURL) + baseTransport.Proxy = http.ProxyURL(proxyURL) case "http", "https": dial := func(ctx context.Context, network, addr string) (net.Conn, error) { // Dial the proxy @@ -121,12 +124,12 @@ func NewProxyTransport(base *http.Transport, proxyURL *url.URL, proxyPath string } return handshakeTLS(ctx, conn, addr) } - transport.DialContext = dial - transport.DialTLSContext = dialTLS + baseTransport.DialContext = dial + baseTransport.DialTLSContext = dialTLS // clear out any system proxy settings - transport.Proxy = nil + baseTransport.Proxy = nil } } - return transport + return baseTransport }