fix(privatevpn): updater now scraps webpage instead of very outdated zip file

internal/provider/privatevpn/provider.go

@@ -1,6 +1,8 @@
 package privatevpn
 
 import (
+	"net/http"
+
 	"github.com/qdm12/gluetun/internal/constants/providers"
 	"github.com/qdm12/gluetun/internal/provider/common"
 	"github.com/qdm12/gluetun/internal/provider/privatevpn/updater"
@@ -13,13 +15,13 @@ type Provider struct {
 	common.Fetcher
 }
 
-func New(storage common.Storage, unzipper common.Unzipper, updaterWarner common.Warner,
+func New(storage common.Storage, client *http.Client, updaterWarner common.Warner,
 	parallelResolver common.ParallelResolver,
 ) *Provider {
 	return &Provider{
 		storage:    storage,
 		connPicker: utils.NewConnectionPicker(),
-		Fetcher:    updater.New(unzipper, updaterWarner, parallelResolver),
+		Fetcher:    updater.New(client, updaterWarner, parallelResolver),
 	}
 }
 

internal/provider/privatevpn/updater/countries.go

@@ -1,15 +0,0 @@
-package updater
-
-import "strings"
-
-func codeToCountry(countryCode string, countryCodes map[string]string) (
-	country string, warning string,
-) {
-	countryCode = strings.ToLower(countryCode)
-	country, ok := countryCodes[countryCode]
-	if !ok {
-		warning = "unknown country code: " + countryCode
-		country = countryCode
-	}
-	return country, warning
-}

internal/provider/privatevpn/updater/filename.go

@@ -1,48 +0,0 @@
-package updater
-
-import (
-	"fmt"
-	"regexp"
-	"strings"
-)
-
-var trailingNumber = regexp.MustCompile(` [0-9]+$`)
-
-func parseFilename(fileName string) (
-	countryCode, city string, err error,
-) {
-	fileName = strings.ReplaceAll(fileName, " ", "") // remove spaces
-
-	const prefix = "PrivateVPN-"
-	if !strings.HasPrefix(fileName, prefix) {
-		return "", "", fmt.Errorf("bad prefix in file name %s", fileName)
-	}
-	s := strings.TrimPrefix(fileName, prefix)
-
-	const tcpSuffix = "-TUN-443.ovpn"
-	const udpSuffix = "-TUN-1194.ovpn"
-	switch {
-	case strings.HasSuffix(fileName, tcpSuffix):
-		s = strings.TrimSuffix(s, tcpSuffix)
-	case strings.HasSuffix(fileName, udpSuffix):
-		s = strings.TrimSuffix(s, udpSuffix)
-	default:
-		return "", "", fmt.Errorf("bad suffix in file name %s", fileName)
-	}
-
-	s = trailingNumber.ReplaceAllString(s, "")
-
-	parts := strings.Split(s, "-")
-	const minParts = 2
-	if len(parts) < minParts {
-		return "", "", fmt.Errorf("not enough parts in file name %s", fileName)
-	}
-	countryCode, city = parts[0], parts[1]
-
-	countryCode = strings.ToLower(countryCode)
-	if countryCode == "co" && strings.HasPrefix(city, "Bogot") {
-		city = "Bogota"
-	}
-
-	return countryCode, city, nil
-}

internal/provider/privatevpn/updater/hosttoserver.go

@@ -1,54 +0,0 @@
-package updater
-
-import (
-	"net/netip"
-
-	"github.com/qdm12/gluetun/internal/constants/vpn"
-	"github.com/qdm12/gluetun/internal/models"
-)
-
-type hostToServer map[string]models.Server
-
-// TODO check if server supports TCP and UDP.
-func (hts hostToServer) add(host, country, city string) {
-	server, ok := hts[host]
-	if ok {
-		return
-	}
-	server.VPN = vpn.OpenVPN
-	server.Hostname = host
-	server.Country = country
-	server.City = city
-	server.UDP = true
-	server.TCP = true
-	hts[host] = server
-}
-
-func (hts hostToServer) toHostsSlice() (hosts []string) {
-	hosts = make([]string, 0, len(hts))
-	for host := range hts {
-		hosts = append(hosts, host)
-	}
-	return hosts
-}
-
-func (hts hostToServer) adaptWithIPs(hostToIPs map[string][]netip.Addr) {
-	for host, IPs := range hostToIPs {
-		server := hts[host]
-		server.IPs = IPs
-		hts[host] = server
-	}
-	for host, server := range hts {
-		if len(server.IPs) == 0 {
-			delete(hts, host)
-		}
-	}
-}
-
-func (hts hostToServer) toServersSlice() (servers []models.Server) {
-	servers = make([]models.Server, 0, len(hts))
-	for _, server := range hts {
-		servers = append(servers, server)
-	}
-	return servers
-}

internal/provider/privatevpn/updater/servers.go

@@ -3,86 +3,34 @@ package updater
 import (
 	"context"
 	"fmt"
+	"net/netip"
 	"sort"
-	"strings"
 
-	"github.com/qdm12/gluetun/internal/constants"
-	"github.com/qdm12/gluetun/internal/constants/vpn"
 	"github.com/qdm12/gluetun/internal/models"
 	"github.com/qdm12/gluetun/internal/provider/common"
-	"github.com/qdm12/gluetun/internal/updater/openvpn"
 )
 
 func (u *Updater) FetchServers(ctx context.Context, minServers int) (
 	servers []models.Server, err error,
 ) {
-	const url = "https://privatevpn.com/client/PrivateVPN-TUN.zip"
-	contents, err := u.unzipper.FetchAndExtract(ctx, url)
+	servers, warnings, err := fetchServersFromWebsite(ctx, u.client)
 	if err != nil {
 		return nil, err
-	} else if len(contents) < minServers {
+	}
+	for _, warning := range warnings {
+		u.warner.Warn(warning)
+	}
+
+	if len(servers) < minServers {
 		return nil, fmt.Errorf("%w: %d and expected at least %d",
-			common.ErrNotEnoughServers, len(contents), minServers)
+			common.ErrNotEnoughServers, len(servers), minServers)
 	}
 
-	countryCodes := constants.CountryCodes()
-
-	hts := make(hostToServer)
-	noHostnameServers := make([]models.Server, 0, 1) // there is only one for now
-
-	for fileName, content := range contents {
-		if !strings.HasSuffix(fileName, ".ovpn") {
-			continue // not an OpenVPN file
-		}
-
-		countryCode, city, err := parseFilename(fileName)
-		if err != nil {
-			// treat error as warning and go to next file
-			u.warner.Warn(err.Error() + " in " + fileName)
-			continue
-		}
-
-		country, warning := codeToCountry(countryCode, countryCodes)
-		if warning != "" {
-			u.warner.Warn(warning)
-		}
-
-		host, warning, err := openvpn.ExtractHost(content)
-		if warning != "" {
-			u.warner.Warn(warning)
-		}
-		if err == nil { // found host
-			hts.add(host, country, city)
-			continue
-		}
-
-		ips, extractIPErr := openvpn.ExtractIPs(content)
-		if warning != "" {
-			u.warner.Warn(warning)
-		}
-		if extractIPErr != nil {
-			// treat extract host error as warning and go to next file
-			u.warner.Warn(extractIPErr.Error() + " in " + fileName)
-			continue
-		}
-		server := models.Server{
-			VPN:     vpn.OpenVPN,
-			Country: country,
-			City:    city,
-			IPs:     ips,
-			UDP:     true,
-			TCP:     true,
-		}
-		noHostnameServers = append(noHostnameServers, server)
+	hosts := make([]string, len(servers))
+	for i := range servers {
+		hosts[i] = servers[i].Hostname
 	}
 
-	if len(noHostnameServers)+len(hts) < minServers {
-		return nil, fmt.Errorf("%w: %d and expected at least %d",
-			common.ErrNotEnoughServers, len(servers)+len(hts), minServers)
-	}
-
-	hosts := hts.toHostsSlice()
-
 	resolveSettings := parallelResolverSettings(hosts)
 	hostToIPs, warnings, err := u.parallelResolver.Resolve(ctx, resolveSettings)
 	for _, warning := range warnings {
@@ -92,17 +40,33 @@ func (u *Updater) FetchServers(ctx context.Context, minServers int) (
 		return nil, err
 	}
 
-	if len(noHostnameServers)+len(hostToIPs) < minServers {
+	servers = applyIPsToServers(servers, hostToIPs)
+
+	if len(servers) < minServers {
 		return nil, fmt.Errorf("%w: %d and expected at least %d",
 			common.ErrNotEnoughServers, len(servers), minServers)
 	}
 
-	hts.adaptWithIPs(hostToIPs)
-
-	servers = hts.toServersSlice()
-	servers = append(servers, noHostnameServers...)
-
 	sort.Sort(models.SortableServers(servers))
 
 	return servers, nil
 }
+
+func applyIPsToServers(servers []models.Server, hostToIPs map[string][]netip.Addr) (
+	result []models.Server,
+) {
+	result = make([]models.Server, 0, len(servers))
+	for _, server := range servers {
+		if len(server.Hostname) > 0 {
+			if ips, ok := hostToIPs[server.Hostname]; ok {
+				server.IPs = ips
+				result = append(result, server)
+			}
+			// Servers with unresolved hostnames are dropped silently
+		} else {
+			// Servers without hostnames (shouldn't happen with the new approach)
+			result = append(result, server)
+		}
+	}
+	return result
+}

internal/provider/privatevpn/updater/updater.go

@@ -1,20 +1,22 @@
 package updater
 
 import (
+	"net/http"
+
 	"github.com/qdm12/gluetun/internal/provider/common"
 )
 
 type Updater struct {
-	unzipper         common.Unzipper
+	client           *http.Client
 	parallelResolver common.ParallelResolver
 	warner           common.Warner
 }
 
-func New(unzipper common.Unzipper, warner common.Warner,
+func New(client *http.Client, warner common.Warner,
 	parallelResolver common.ParallelResolver,
 ) *Updater {
 	return &Updater{
-		unzipper:         unzipper,
+		client:           client,
 		parallelResolver: parallelResolver,
 		warner:           warner,
 	}

internal/provider/privatevpn/updater/website.go

@@ -0,0 +1,143 @@
+package updater
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+
+	"github.com/qdm12/gluetun/internal/constants/vpn"
+	"github.com/qdm12/gluetun/internal/models"
+	htmlutils "github.com/qdm12/gluetun/internal/updater/html"
+	"golang.org/x/net/html"
+)
+
+func fetchServersFromWebsite(ctx context.Context, client *http.Client) (
+	servers []models.Server, warnings []string, err error,
+) {
+	const url = "https://help.privatevpn.com/en/articles/302378-privatevpn-server-list"
+	rootNode, err := htmlutils.Fetch(ctx, client, url)
+	if err != nil {
+		return nil, nil, fmt.Errorf("fetching HTML code: %w", err)
+	}
+
+	servers, warnings, err = parseServerListTable(rootNode)
+	if err != nil {
+		return nil, warnings, fmt.Errorf("parsing HTML code: %w", err)
+	}
+
+	return servers, warnings, nil
+}
+
+func parseServerListTable(rootNode *html.Node) (
+	servers []models.Server, warnings []string, err error,
+) {
+	// Find the article_body div which contains the server list table
+	// This avoids picking up the footer contact table which also exists on the page
+	articleBodyNode := htmlutils.BFS(rootNode, func(node *html.Node) bool {
+		return htmlutils.HasClassStrings(node, "article_body")
+	})
+	if articleBodyNode == nil {
+		return nil, nil, htmlutils.WrapError(errors.New("article body not found"), rootNode)
+	}
+
+	// Find the table within the article body
+	tableNode := htmlutils.BFS(articleBodyNode, htmlutils.MatchData("table"))
+	if tableNode == nil {
+		return nil, nil, htmlutils.WrapError(errors.New("server list table not found in article body"), articleBodyNode)
+	}
+
+	tbodyNode := htmlutils.DirectChild(tableNode, htmlutils.MatchData("tbody"))
+	if tbodyNode == nil {
+		return nil, nil, htmlutils.WrapError(errors.New("table body not found"), tableNode)
+	}
+
+	// Iterate through each row in the table body
+	skipHeader := true
+	for trNode := tbodyNode.FirstChild; trNode != nil; trNode = trNode.NextSibling {
+		if trNode.Data != "tr" {
+			continue
+		}
+
+		if skipHeader {
+			skipHeader = false
+			continue
+		}
+
+		server, warning := parseServerRow(trNode)
+		if warning != "" {
+			warnings = append(warnings, warning)
+			continue
+		}
+		servers = append(servers, server)
+	}
+
+	return servers, warnings, nil
+}
+
+func parseServerRow(trNode *html.Node) (server models.Server, warning string) {
+	// Get all td cells in this row
+	var tds []*html.Node
+	for tdNode := trNode.FirstChild; tdNode != nil; tdNode = tdNode.NextSibling {
+		if tdNode.Data == "td" {
+			tds = append(tds, tdNode)
+		}
+	}
+
+	const expectedCellCount = 2
+	if len(tds) != expectedCellCount {
+		return models.Server{}, htmlutils.WrapWarning("expected 2 cells in row", trNode)
+	}
+
+	// First cell: Location (format: "Country - City" or just "Country")
+	location := extractTextFromCell(tds[0])
+	if location == "" {
+		return models.Server{}, htmlutils.WrapWarning("empty location cell", trNode)
+	}
+
+	// Second cell: Server Address (hostname ending in .pvdata.host)
+	hostname := extractTextFromCell(tds[1])
+	if hostname == "" {
+		return models.Server{}, htmlutils.WrapWarning("empty server address cell", trNode)
+	}
+	hostname = strings.TrimSpace(hostname)
+
+	country, city := parseLocation(location)
+
+	return models.Server{
+		VPN:      vpn.OpenVPN,
+		TCP:      true, // port 443
+		UDP:      true, // port 1194
+		Country:  country,
+		City:     city,
+		Hostname: hostname,
+	}, ""
+}
+
+func parseLocation(location string) (country, city string) {
+	const separator = " - "
+	parts := strings.SplitN(location, separator, 2) //nolint:mnd
+	country = strings.TrimSpace(parts[0])
+
+	if len(parts) > 1 {
+		city = strings.TrimSpace(parts[1])
+	}
+
+	return country, city
+}
+
+func extractTextFromCell(tdNode *html.Node) string {
+	var sb strings.Builder
+	extractText(tdNode, &sb)
+	return strings.TrimSpace(sb.String())
+}
+
+func extractText(node *html.Node, sb *strings.Builder) {
+	if node.Type == html.TextNode {
+		sb.WriteString(node.Data)
+	}
+	for child := node.FirstChild; child != nil; child = child.NextSibling {
+		extractText(child, sb)
+	}
+}

internal/provider/privatevpn/updater/website_test.go

@@ -0,0 +1,250 @@
+package updater
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	htmlutils "github.com/qdm12/gluetun/internal/updater/html"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/net/html"
+)
+
+func Test_parseServerListTable(t *testing.T) {
+	t.Parallel()
+
+	rootNode := parseTestHTML(t, "testdata/index.html")
+
+	servers, warnings, err := parseServerListTable(rootNode)
+	require.NoError(t, err)
+	assert.Empty(t, warnings)
+
+	// Verify we got a reasonable number of servers (expected 70+)
+	assert.Greater(t, len(servers), 50)
+}
+
+func Test_parseServerListTable_hasValidServers(t *testing.T) {
+	t.Parallel()
+
+	rootNode := parseTestHTML(t, "testdata/index.html")
+
+	servers, _, err := parseServerListTable(rootNode)
+	require.NoError(t, err)
+
+	testCases := map[string]struct {
+		findByHostname string
+		wantCountry    string
+		wantCity       string
+	}{
+		"australia_sydney": {
+			findByHostname: "au-syd.pvdata.host",
+			wantCountry:    "Australia",
+			wantCity:       "Sydney",
+		},
+		"netherlands_amsterdam": {
+			findByHostname: "nl-ams.pvdata.host",
+			wantCountry:    "Netherlands",
+			wantCity:       "Amsterdam",
+		},
+		"germany_frankfurt": {
+			findByHostname: "de-fra.pvdata.host",
+			wantCountry:    "Germany",
+			wantCity:       "Frankfurt",
+		},
+		"united_kingdom_london": {
+			findByHostname: "uk-lon.pvdata.host",
+			wantCountry:    "United Kingdom",
+			wantCity:       "London",
+		},
+		"us_new_york": {
+			findByHostname: "us-nyc.pvdata.host",
+			wantCountry:    "United States of America",
+			wantCity:       "New York",
+		},
+		"france_paris": {
+			findByHostname: "fr-par.pvdata.host",
+			wantCountry:    "France",
+			wantCity:       "Paris",
+		},
+		"italy_milan": {
+			findByHostname: "it-mil.pvdata.host",
+			wantCountry:    "Italy",
+			wantCity:       "Milan",
+		},
+		"poland_torun": {
+			findByHostname: "pl-tor.pvdata.host",
+			wantCountry:    "Poland",
+			wantCity:       "Torun",
+		},
+		"singapore_no_city_format": {
+			findByHostname: "sg-sin.pvdata.host",
+			wantCountry:    "Singapore",
+			wantCity:       "",
+		},
+		"japan_tokyo": {
+			findByHostname: "jp-tok.pvdata.host",
+			wantCountry:    "Japan",
+			wantCity:       "Tokyo",
+		},
+		"united_ae_dubai": {
+			findByHostname: "ae-dub.pvdata.host",
+			wantCountry:    "United Arab Emirates",
+			wantCity:       "Dubai",
+		},
+	}
+
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			found := false
+			for _, server := range servers {
+				if server.Hostname == testCase.findByHostname {
+					found = true
+					assert.Equal(t, testCase.wantCountry, server.Country, "country mismatch")
+					assert.Equal(t, testCase.wantCity, server.City, "city mismatch")
+					break
+				}
+			}
+			assert.True(t, found, "server with hostname %q not found", testCase.findByHostname)
+		})
+	}
+}
+
+func Test_parseServerListTable_noDeadServers(t *testing.T) {
+	t.Parallel()
+
+	rootNode := parseTestHTML(t, "testdata/index.html")
+
+	servers, _, err := parseServerListTable(rootNode)
+	require.NoError(t, err)
+
+	// These are the dead hostnames that were in the old 2019 zip file
+	deadHostnames := []string{
+		"au-syd2.pvdata.host",
+		"nl-ams2.pvdata.host",
+		"uk-lon3.pvdata.host",
+		"uk-lon5.pvdata.host",
+		"uk-lon6.pvdata.host",
+		"de-fra2.pvdata.host",
+		"it-mil2.pvdata.host",
+		"fr-par3.pvdata.host",
+		"pl-war.pvdata.host",
+		"us-nyc4.pvdata.host",
+	}
+
+	for _, deadHostname := range deadHostnames {
+		t.Run("no_"+deadHostname, func(t *testing.T) {
+			t.Parallel()
+
+			for _, server := range servers {
+				assert.NotEqual(t, deadHostname, server.Hostname,
+					"dead hostname %q should not be in the server list", deadHostname)
+			}
+		})
+	}
+}
+
+func Test_parseLocation(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		location    string
+		wantCountry string
+		wantCity    string
+	}{
+		"country_and_city": {
+			location:    "Germany - Frankfurt",
+			wantCountry: "Germany",
+			wantCity:    "Frankfurt",
+		},
+		"country_only": {
+			location:    "Singapore",
+			wantCountry: "Singapore",
+			wantCity:    "",
+		},
+		"long_country_name": {
+			location:    "United States of America - New York",
+			wantCountry: "United States of America",
+			wantCity:    "New York",
+		},
+		"city_with_dash": {
+			location:    "United States of America - New York City",
+			wantCountry: "United States of America",
+			wantCity:    "New York City",
+		},
+		"hong_kong": {
+			location:    "Hong Kong",
+			wantCountry: "Hong Kong",
+			wantCity:    "",
+		},
+	}
+
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			country, city := parseLocation(testCase.location)
+			assert.Equal(t, testCase.wantCountry, country)
+			assert.Equal(t, testCase.wantCity, city)
+		})
+	}
+}
+
+func Test_extractTextFromCell(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		htmlInput string
+		wantText  string
+	}{
+		"simple_text": {
+			htmlInput: "<table><tr><td><div><p>Germany - Frankfurt</p></div></td></tr></table>",
+			wantText:  "Germany - Frankfurt",
+		},
+		"nested_divs": {
+			htmlInput: "<table><tr><td>" +
+				"<div class=\"intercom-interblocks-paragraph\">" +
+				"<p>hostname.pvdata.host</p>" +
+				"</div></td></tr></table>",
+			wantText: "hostname.pvdata.host",
+		},
+		"whitespace_handling": {
+			htmlInput: "<table><tr><td>   United Kingdom - London   </td></tr></table>",
+			wantText:  "United Kingdom - London",
+		},
+	}
+
+	for name, testCase := range testCases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			node, err := html.Parse(strings.NewReader(testCase.htmlInput))
+			require.NoError(t, err)
+
+			// Find the td node
+			tdNode := findTDNode(node)
+			require.NotNil(t, tdNode, "td node not found")
+
+			text := extractTextFromCell(tdNode)
+			assert.Equal(t, testCase.wantText, text)
+		})
+	}
+}
+
+func parseTestHTML(t *testing.T, filepath string) *html.Node {
+	t.Helper()
+
+	content, err := os.ReadFile(filepath)
+	require.NoError(t, err)
+
+	rootNode, err := html.Parse(strings.NewReader(string(content)))
+	require.NoError(t, err)
+
+	return rootNode
+}
+
+func findTDNode(node *html.Node) *html.Node {
+	return htmlutils.BFS(node, htmlutils.MatchData("td"))
+}

internal/provider/providers.go

@@ -68,7 +68,7 @@ func NewProviders(storage Storage, timeNow func() time.Time,
 		providers.Nordvpn:               nordvpn.New(storage, client, updaterWarner),
 		providers.Privado:               privado.New(storage, client, updaterWarner),
 		providers.PrivateInternetAccess: privateinternetaccess.New(storage, timeNow, client),
-		providers.Privatevpn:            privatevpn.New(storage, unzipper, updaterWarner, parallelResolver),
+		providers.Privatevpn:            privatevpn.New(storage, client, updaterWarner, parallelResolver),
 		providers.Protonvpn:             protonvpn.New(storage, client, updaterWarner, *credentials.ProtonEmail, *credentials.ProtonPassword),
 		providers.Purevpn:               purevpn.New(storage, ipFetcher, unzipper, updaterWarner, parallelResolver),
 		providers.SlickVPN:              slickvpn.New(storage, client, updaterWarner, parallelResolver),