From 136f159d53aed184119ccc603c5f84977adbe1f0 Mon Sep 17 00:00:00 2001
From: Ludovic Fernandez <ldez@users.noreply.github.com>
Date: Sat, 28 Nov 2020 17:54:26 +0100
Subject: [PATCH] pdns: fix URL request creation. (#1312)

---
 providers/dns/pdns/client.go      |  22 +++---
 providers/dns/pdns/client_test.go | 114 ++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 12 deletions(-)
 create mode 100644 providers/dns/pdns/client_test.go

diff --git a/providers/dns/pdns/client.go b/providers/dns/pdns/client.go
index 924e7650..2f685640 100644
--- a/providers/dns/pdns/client.go
+++ b/providers/dns/pdns/client.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"path"
 	"strconv"
 	"strings"
 
@@ -123,7 +124,7 @@ func (d *DNSProvider) findTxtRecord(fqdn string) (*rrSet, error) {
 	}
 
 	for _, set := range zone.RRSets {
-		if (set.Name == dns01.UnFqdn(fqdn) || set.Name == fqdn) && set.Type == "TXT" {
+		if set.Type == "TXT" && (set.Name == dns01.UnFqdn(fqdn) || set.Name == fqdn) {
 			return &set, nil
 		}
 	}
@@ -196,21 +197,18 @@ func (d *DNSProvider) sendRequest(method, uri string, body io.Reader) (json.RawM
 }
 
 func (d *DNSProvider) makeRequest(method, uri string, body io.Reader) (*http.Request, error) {
-	path := ""
-	if d.config.Host.Path != "/" {
-		path = d.config.Host.Path
+	p := path.Join("/", uri)
+
+	if p != "/api" && d.apiVersion > 0 && !strings.HasPrefix(p, "/api/v") {
+		p = path.Join("/api", "v"+strconv.Itoa(d.apiVersion), p)
 	}
 
-	if !strings.HasPrefix(uri, "/") {
-		uri = "/" + uri
+	u, err := d.config.Host.Parse(path.Join(d.config.Host.Path, p))
+	if err != nil {
+		return nil, err
 	}
 
-	if d.apiVersion > 0 && !strings.HasPrefix(uri, "/api/v") {
-		uri = "/api/v" + strconv.Itoa(d.apiVersion) + uri
-	}
-
-	u := d.config.Host.Scheme + "://" + d.config.Host.Host + path + uri
-	req, err := http.NewRequest(method, u, body)
+	req, err := http.NewRequest(method, u.String(), body)
 	if err != nil {
 		return nil, err
 	}
diff --git a/providers/dns/pdns/client_test.go b/providers/dns/pdns/client_test.go
new file mode 100644
index 00000000..cb8befb9
--- /dev/null
+++ b/providers/dns/pdns/client_test.go
@@ -0,0 +1,114 @@
+package pdns
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDNSProvider_makeRequest(t *testing.T) {
+	testCases := []struct {
+		desc       string
+		apiVersion int
+		baseURL    string
+		uri        string
+		expected   string
+	}{
+		{
+			desc:       "host with path",
+			apiVersion: 1,
+			baseURL:    "https://example.com/test",
+			uri:        "/foo",
+			expected:   "https://example.com/test/api/v1/foo",
+		},
+		{
+			desc:       "host with path + trailing slash",
+			apiVersion: 1,
+			baseURL:    "https://example.com/test/",
+			uri:        "/foo",
+			expected:   "https://example.com/test/api/v1/foo",
+		},
+		{
+			desc:       "no URI",
+			apiVersion: 1,
+			baseURL:    "https://example.com/test",
+			uri:        "",
+			expected:   "https://example.com/test/api/v1",
+		},
+		{
+			desc:       "host without path",
+			apiVersion: 1,
+			baseURL:    "https://example.com",
+			uri:        "/foo",
+			expected:   "https://example.com/api/v1/foo",
+		},
+		{
+			desc:       "api",
+			apiVersion: 1,
+			baseURL:    "https://example.com",
+			uri:        "/api",
+			expected:   "https://example.com/api",
+		},
+		{
+			desc:       "API version 0, host with path",
+			apiVersion: 0,
+			baseURL:    "https://example.com/test",
+			uri:        "/foo",
+			expected:   "https://example.com/test/foo",
+		},
+		{
+			desc:       "API version 0, host with path + trailing slash",
+			apiVersion: 0,
+			baseURL:    "https://example.com/test/",
+			uri:        "/foo",
+			expected:   "https://example.com/test/foo",
+		},
+		{
+			desc:       "API version 0, no URI",
+			apiVersion: 0,
+			baseURL:    "https://example.com/test",
+			uri:        "",
+			expected:   "https://example.com/test",
+		},
+		{
+			desc:       "API version 0, host without path",
+			apiVersion: 0,
+			baseURL:    "https://example.com",
+			uri:        "/foo",
+			expected:   "https://example.com/foo",
+		},
+		{
+			desc:       "API version 0, api",
+			apiVersion: 0,
+			baseURL:    "https://example.com",
+			uri:        "/api",
+			expected:   "https://example.com/api",
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			host, err := url.Parse(test.baseURL)
+			require.NoError(t, err)
+
+			config := &Config{Host: host, APIKey: "secret"}
+
+			p := &DNSProvider{
+				config:     config,
+				apiVersion: test.apiVersion,
+			}
+
+			req, err := p.makeRequest(http.MethodGet, test.uri, nil)
+			require.NoError(t, err)
+
+			assert.Equal(t, test.expected, req.URL.String())
+			assert.Equal(t, "secret", req.Header.Get("X-API-Key"))
+		})
+	}
+}