Refactor codebase

pkg/vpn/cache.go

@@ -4,37 +4,42 @@ import (
 	"encoding/json"
 	"io"
 	"os"
-	"path"
+	"path/filepath"
 	"time"
-
-	"github.com/rs/zerolog/log"
-	"github.com/spf13/afero"
 )
 
 const serverCachefile = "servers.json"
 
-func getCacheDir() string {
+func getCacheDir() (string, error) {
 	homeDir, err := os.UserHomeDir()
 	if err != nil {
-		log.Error().Msgf("Failed to get user's home directory: %s ", err)
-		return ""
+		return "", err
 	}
-	cacheDir := path.Join(homeDir, ".vpngate", "cache")
-	return cacheDir
+	cacheDir := filepath.Join(homeDir, ".vpngate", "cache")
+	return cacheDir, nil
 }
 
 func createCacheDir() error {
-	cacheDir := getCacheDir()
-	AppFs := afero.NewOsFs()
-	return AppFs.MkdirAll(cacheDir, 0o700)
+	cacheDir, err := getCacheDir()
+	if err != nil {
+		return err
+	}
+	return os.MkdirAll(cacheDir, 0o700)
 }
 
 func getVpnListCache() (*[]Server, error) {
-	cacheFile := path.Join(getCacheDir(), serverCachefile)
+	cacheDir, err := getCacheDir()
+	if err != nil {
+		return nil, err
+	}
+	cacheFile := filepath.Join(cacheDir, serverCachefile)
 	serversFile, err := os.Open(cacheFile)
 	if err != nil {
 		return nil, err
 	}
+	defer func() {
+		_ = serversFile.Close()
+	}()
 
 	byteValue, err := io.ReadAll(serversFile)
 	if err != nil {
@@ -44,7 +49,6 @@ func getVpnListCache() (*[]Server, error) {
 	var servers []Server
 
 	err = json.Unmarshal(byteValue, &servers)
-
 	if err != nil {
 		return nil, err
 	}
@@ -53,8 +57,7 @@ func getVpnListCache() (*[]Server, error) {
 }
 
 func writeVpnListToCache(servers []Server) error {
-	err := createCacheDir()
-	if err != nil {
+	if err := createCacheDir(); err != nil {
 		return err
 	}
 
@@ -63,20 +66,26 @@ func writeVpnListToCache(servers []Server) error {
 		return err
 	}
 
-	cacheFile := path.Join(getCacheDir(), serverCachefile)
+	cacheDir, err := getCacheDir()
+	if err != nil {
+		return err
+	}
+	cacheFile := filepath.Join(cacheDir, serverCachefile)
 
-	err = os.WriteFile(cacheFile, f, 0o644)
-
-	return err
+	return os.WriteFile(cacheFile, f, 0o644)
 }
 
 func vpnListCacheIsExpired() bool {
-	file, err := os.Stat(path.Join(getCacheDir(), serverCachefile))
+	cacheDir, err := getCacheDir()
+	if err != nil {
+		return true
+	}
+	file, err := os.Stat(filepath.Join(cacheDir, serverCachefile))
 	if err != nil {
 		return true
 	}
 
 	lastModified := file.ModTime()
 
-	return (time.Since(lastModified)) > time.Duration(24*time.Hour)
-}
+	return time.Since(lastModified) > 24*time.Hour
+}

pkg/vpn/client.go

@@ -1,28 +1,17 @@
 package vpn
 
 import (
-	"os"
 	"runtime"
 
 	"github.com/davegallant/vpngate/pkg/exec"
-	"github.com/juju/errors"
 )
 
 // Connect to a specified OpenVPN configuration
 func Connect(configPath string) error {
-	tmpLogFile, err := os.CreateTemp("", "vpngate-openvpn-log-")
-	if err != nil {
-		return errors.Annotate(err, "Unable to create a temporary log file")
-	}
-	defer func() {
-		_ = os.Remove(tmpLogFile.Name())
-	}()
-
 	executable := "openvpn"
 	if runtime.GOOS == "windows" {
 		executable = "C:\\Program Files\\OpenVPN\\bin\\openvpn.exe"
 	}
 
-	err = exec.Run(executable, ".", "--verb", "4", "--config", configPath, "--data-ciphers", "AES-128-CBC")
-	return err
-}
+	return exec.Run(executable, ".", "--verb", "4", "--config", configPath, "--data-ciphers", "AES-128-CBC")
+}

pkg/vpn/list.go

@@ -2,10 +2,12 @@ package vpn
 
 import (
 	"bytes"
+	"context"
 	"io"
+	"net"
 	"net/http"
 	"net/url"
-	"os"
+	"time"
 
 	"github.com/jszwec/csvutil"
 	"github.com/rs/zerolog/log"
@@ -16,10 +18,12 @@ import (
 )
 
 const (
-	vpnList = "https://www.vpngate.net/api/iphone/"
+	vpnList            = "https://www.vpngate.net/api/iphone/"
+	httpClientTimeout  = 30 * time.Second
+	dialTimeout        = 10 * time.Second
 )
 
-// Server holds in formation about a vpn relay server
+// Server holds information about a vpn relay server
 type Server struct {
 	HostName          string `csv:"#HostName"`
 	CountryLong       string `csv:"CountryLong"`
@@ -30,20 +34,14 @@ type Server struct {
 	Ping              string `csv:"Ping"`
 }
 
-func streamToBytes(stream io.Reader) []byte {
-	buf := new(bytes.Buffer)
-	_, err := buf.ReadFrom(stream)
-	if err != nil {
-		log.Error().Msg("Unable to stream bytes")
-	}
-	return buf.Bytes()
-}
-
-// parse csv
+// parseVpnList parses the VPN server list from CSV format
 func parseVpnList(r io.Reader) (*[]Server, error) {
 	var servers []Server
 
-	serverList := streamToBytes(r)
+	serverList, err := io.ReadAll(r)
+	if err != nil {
+		return nil, errors.Annotate(err, "Unable to read stream")
+	}
 
 	// Trim known invalid rows
 	serverList = bytes.TrimPrefix(serverList, []byte("*vpn_servers\r\n"))
@@ -57,86 +55,119 @@ func parseVpnList(r io.Reader) (*[]Server, error) {
 	return &servers, nil
 }
 
+// createHTTPClient creates an HTTP client with optional proxy configuration
+func createHTTPClient(httpProxy string, socks5Proxy string) (*http.Client, error) {
+	if httpProxy != "" {
+		proxyURL, err := url.Parse(httpProxy)
+		if err != nil {
+			return nil, errors.Annotatef(err, "Error parsing HTTP proxy: %s", httpProxy)
+		}
+		transport := &http.Transport{
+			Proxy: http.ProxyURL(proxyURL),
+		}
+		return &http.Client{
+			Transport: transport,
+			Timeout:   httpClientTimeout,
+		}, nil
+	}
+
+	if socks5Proxy != "" {
+		dialer, err := proxy.SOCKS5("tcp", socks5Proxy, nil, proxy.Direct)
+		if err != nil {
+			return nil, errors.Annotatef(err, "Error creating SOCKS5 dialer: %v", err)
+		}
+
+		// Create a DialContext function from the SOCKS5 dialer
+		dialContext := func(ctx context.Context, network, addr string) (net.Conn, error) {
+			// Check if context is already done
+			select {
+			case <-ctx.Done():
+				return nil, ctx.Err()
+			default:
+			}
+
+			// Use the dialer with a timeout
+			conn, err := dialer.Dial(network, addr)
+			if err != nil {
+				return nil, err
+			}
+
+			// Respect context cancellation after connection
+			go func() {
+				<-ctx.Done()
+				_ = conn.Close()
+			}()
+
+			return conn, nil
+		}
+
+		httpTransport := &http.Transport{
+			DialContext: dialContext,
+		}
+		return &http.Client{
+			Transport: httpTransport,
+			Timeout:   httpClientTimeout,
+		}, nil
+	}
+
+	return &http.Client{
+		Timeout: httpClientTimeout,
+		Transport: &http.Transport{
+			DialContext: (&net.Dialer{
+				Timeout: dialTimeout,
+			}).DialContext,
+		},
+	}, nil
+}
+
 // GetList returns a list of vpn servers
 func GetList(httpProxy string, socks5Proxy string) (*[]Server, error) {
 	cacheExpired := vpnListCacheIsExpired()
 
-	var servers *[]Server
-	var client *http.Client
-
+	// Try to use cached list if not expired
 	if !cacheExpired {
 		servers, err := getVpnListCache()
-
-		if err != nil {
-			log.Info().Msg("Unable to retrieve vpn list from cache")
-		} else {
+		if err == nil {
 			return servers, nil
 		}
-
+		log.Info().Msg("Unable to retrieve vpn list from cache")
 	} else {
 		log.Info().Msg("The vpn server list cache has expired")
 	}
 
 	log.Info().Msg("Fetching the latest server list")
 
-	if httpProxy != "" {
-		proxyURL, err := url.Parse(httpProxy)
-		if err != nil {
-			log.Error().Msgf("Error parsing proxy: %s", err)
-			os.Exit(1)
-		}
-		transport := &http.Transport{
-			Proxy: http.ProxyURL(proxyURL),
-		}
-
-		client = &http.Client{
-			Transport: transport,
-		}
-
-	} else if socks5Proxy != "" {
-		dialer, err := proxy.SOCKS5("tcp", socks5Proxy, nil, proxy.Direct)
-		if err != nil {
-			log.Error().Msgf("Error creating SOCKS5 dialer: %v", err)
-			os.Exit(1)
-		}
-
-		httpTransport := &http.Transport{
-			Dial: dialer.Dial,
-		}
-
-		client = &http.Client{
-			Transport: httpTransport,
-		}
-	} else {
-		client = &http.Client{}
+	client, err := createHTTPClient(httpProxy, socks5Proxy)
+	if err != nil {
+		return nil, err
 	}
 
-	var r *http.Response
+	var servers *[]Server
 
-	err := util.Retry(5, 1, func() error {
-		var err error
-		r, err = client.Get(vpnList)
+	err = util.Retry(5, 1, func() error {
+		resp, err := client.Get(vpnList)
 		if err != nil {
 			return err
 		}
 		defer func() {
-			_ = r.Body.Close()
+			_ = resp.Body.Close()
 		}()
 
-		if r.StatusCode != 200 {
-			return errors.Annotatef(err, "Unexpected status code when retrieving vpn list: %d", r.StatusCode)
+		if resp.StatusCode != http.StatusOK {
+			return errors.Annotatef(err, "Unexpected status code when retrieving vpn list: %d", resp.StatusCode)
 		}
 
-		servers, err = parseVpnList(r.Body)
-
+		parsedServers, err := parseVpnList(resp.Body)
 		if err != nil {
 			return err
 		}
 
-		err = writeVpnListToCache(*servers)
+		servers = parsedServers
 
-		if err != nil {
-			log.Warn().Msgf("Unable to write servers to cache: %s", err)
+		// Cache the servers for future use
+		cacheErr := writeVpnListToCache(*servers)
+		if cacheErr != nil {
+			log.Warn().Msgf("Unable to write servers to cache: %s", cacheErr)
 		}
 		return nil
 	})
@@ -146,4 +177,4 @@ func GetList(httpProxy string, socks5Proxy string) (*[]Server, error) {
 	}
 
 	return servers, nil
-}
+}