feat: warn about probably invalid custom providers

README.markdown

@@ -352,18 +352,18 @@ _(Click to expand the following items.)_
 
 ⚠️ [oznu/cloudflare-ddns](https://github.com/oznu/docker-cloudflare-ddns) relies on the insecure DNS protocol to obtain public IP addresses; a malicious hacker could more easily forge DNS responses and trick it into updating your domain with any IP address. In comparison, we use only verified responses from Cloudflare, which makes the attack much more difficult. See the [design document](docs/DESIGN.markdown) for more information on security.
 
-| Old Parameter                          |     | Note                                                                               |
-| -------------------------------------- | --- | ---------------------------------------------------------------------------------- |
-| `API_KEY=key`                          | ✔️  | Use `CF_API_TOKEN=key`                                                             |
-| `API_KEY_FILE=file`                    | ✔️  | Use `CF_API_TOKEN_FILE=file`                                                       |
-| `ZONE=example.org` and `SUBDOMAIN=sub` | ✔️  | Use `DOMAINS=sub.example.org` directly                                             |
-| `PROXIED=true`                         | ✔️  | Same (`PROXIED=true`)                                                              |
-| `RRTYPE=A`                             | ✔️  | Both IPv4 and IPv6 are enabled by default; use `IP6_PROVIDER=none` to disable IPv6 |
-| `RRTYPE=AAAA`                          | ✔️  | Both IPv4 and IPv6 are enabled by default; use `IP4_PROVIDER=none` to disable IPv4 |
-| `DELETE_ON_STOP=true`                  | ✔️  | Same (`DELETE_ON_STOP=true`)                                                       |
-| `INTERFACE=iface`                      | ✔️  | Not required for `local` providers; we can handle multiple network interfaces      |
-| `CUSTOM_LOOKUP_CMD=cmd`                | ❌  | There are no shells in the minimal Docker image                                    |
-| `DNS_SERVER=server`                    | ❌  | Only Cloudflare is supported                                                       |
+| Old Parameter                          |     | Note                                                                                 |
+| -------------------------------------- | --- | ------------------------------------------------------------------------------------ |
+| `API_KEY=key`                          | ✔️  | Use `CF_API_TOKEN=key`                                                               |
+| `API_KEY_FILE=file`                    | ✔️  | Use `CF_API_TOKEN_FILE=file`                                                         |
+| `ZONE=example.org` and `SUBDOMAIN=sub` | ✔️  | Use `DOMAINS=sub.example.org` directly                                               |
+| `PROXIED=true`                         | ✔️  | Same (`PROXIED=true`)                                                                |
+| `RRTYPE=A`                             | ✔️  | Both IPv4 and IPv6 are enabled by default; use `IP6_PROVIDER=none` to disable IPv6   |
+| `RRTYPE=AAAA`                          | ✔️  | Both IPv4 and IPv6 are enabled by default; use `IP4_PROVIDER=none` to disable IPv4   |
+| `DELETE_ON_STOP=true`                  | ✔️  | Same (`DELETE_ON_STOP=true`)                                                         |
+| `INTERFACE=iface`                      | ✔️  | Not required for `local` providers; we can handle multiple network interfaces        |
+| `CUSTOM_LOOKUP_CMD=cmd`                | ❌  | There are no shells in the minimal Docker image                                      |
+| `DNS_SERVER=server`                    | ❌  | Only Cloudflare is supported, except the experimental `url:URL` provider via HTTP(S) |
 
 </details>
 

internal/config/env_provider.go

@@ -14,9 +14,11 @@ import (
 //
 //nolint:funlen
 func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provider) bool {
-	if val := Getenv(key); val == "" {
+	val := Getenv(key)
+
+	if val == "" {
 		// parsing of the deprecated parameter
-		switch valPolicy := Getenv(keyDeprecated); valPolicy {
+		switch valDeprecated := Getenv(keyDeprecated); valDeprecated {
 		case "":
 			ppfmt.Infof(pp.EmojiBullet, "Use default %s=%s", key, provider.Name(*field))
 			return true
@@ -32,15 +34,15 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 			ppfmt.Warningf(
 				pp.EmojiUserWarning,
 				`%s is deprecated; use %s=%s`,
-				keyDeprecated, key, valPolicy,
+				keyDeprecated, key, valDeprecated,
 			)
 			*field = provider.NewCloudflareTrace()
 			return true
 		case "cloudflare.doh":
 			ppfmt.Warningf(
 				pp.EmojiUserWarning,
 				`%s is deprecated; use %s=%s`,
-				keyDeprecated, key, valPolicy,
+				keyDeprecated, key, valDeprecated,
 			)
 			*field = provider.NewCloudflareDOH()
 			return true
@@ -56,7 +58,7 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 			ppfmt.Warningf(
 				pp.EmojiUserWarning,
 				`%s is deprecated; use %s=%s`,
-				keyDeprecated, key, valPolicy,
+				keyDeprecated, key, valDeprecated,
 			)
 			*field = provider.NewLocal()
 			return true
@@ -69,57 +71,61 @@ func ReadProvider(ppfmt pp.PP, key, keyDeprecated string, field *provider.Provid
 			*field = nil
 			return true
 		default:
-			ppfmt.Errorf(pp.EmojiUserError, "%s (%q) is not a valid provider", keyDeprecated, valPolicy)
-			return false
-		}
-	} else {
-		if Getenv(keyDeprecated) != "" {
-			ppfmt.Errorf(
-				pp.EmojiUserError,
-				`Cannot have both %s and %s set`,
-				key, keyDeprecated,
-			)
+			ppfmt.Errorf(pp.EmojiUserError, "%s (%q) is not a valid provider", keyDeprecated, valDeprecated)
 			return false
 		}
+	}
 
-		switch val {
-		case "cloudflare":
-			ppfmt.Errorf(
-				pp.EmojiUserError,
-				`%s=cloudflare is invalid; use %s=cloudflare.trace or %s=cloudflare.doh`,
-				key, key, key,
-			)
-			return false
-		case "cloudflare.trace":
-			*field = provider.NewCloudflareTrace()
-			return true
-		case "cloudflare.doh":
-			*field = provider.NewCloudflareDOH()
-			return true
-		case "ipify":
-			ppfmt.Warningf(
-				pp.EmojiUserWarning,
-				`%s=ipify is deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
-				key, key, key,
-			)
-			*field = provider.NewIpify()
-			return true
-		case "local":
-			*field = provider.NewLocal()
-			return true
-		case "none":
-			*field = nil
-			return true
-		default:
-			if strings.HasPrefix(val, "url:") {
-				url := strings.TrimSpace(strings.TrimPrefix(val, "url:"))
-				*field = provider.NewCustom(url)
-				return true
-			}
-			ppfmt.Errorf(pp.EmojiUserError, "%s (%q) is not a valid provider", key, val)
-			return false
+	if Getenv(keyDeprecated) != "" {
+		ppfmt.Errorf(
+			pp.EmojiUserError,
+			`Cannot have both %s and %s set`,
+			key, keyDeprecated,
+		)
+		return false
+	}
+
+	switch val {
+	case "cloudflare":
+		ppfmt.Errorf(
+			pp.EmojiUserError,
+			`%s=cloudflare is invalid; use %s=cloudflare.trace or %s=cloudflare.doh`,
+			key, key, key,
+		)
+		return false
+	case "cloudflare.trace":
+		*field = provider.NewCloudflareTrace()
+		return true
+	case "cloudflare.doh":
+		*field = provider.NewCloudflareDOH()
+		return true
+	case "ipify":
+		ppfmt.Warningf(
+			pp.EmojiUserWarning,
+			`%s=ipify is deprecated; use %s=cloudflare.trace or %s=cloudflare.doh`,
+			key, key, key,
+		)
+		*field = provider.NewIpify()
+		return true
+	case "local":
+		*field = provider.NewLocal()
+		return true
+	case "none":
+		*field = nil
+		return true
+	}
+
+	if strings.HasPrefix(val, "url:") {
+		url := strings.TrimSpace(strings.TrimPrefix(val, "url:"))
+		p, ok := provider.NewCustom(ppfmt, url)
+		if ok {
+			*field = p
 		}
+		return ok
 	}
+
+	ppfmt.Errorf(pp.EmojiUserError, "%s (%q) is not a valid provider", key, val)
+	return false
 }
 
 // ReadProviderMap reads the environment variables IP4_PROVIDER and IP6_PROVIDER,

internal/config/env_provider_test.go

@@ -24,7 +24,7 @@ func TestReadProvider(t *testing.T) {
 		trace  = provider.NewCloudflareTrace()
 		local  = provider.NewLocal()
 		ipify  = provider.NewIpify()
-		custom = provider.NewCustom("https://url.io")
+		custom = provider.MustNewCustom("https://url.io")
 	)
 
 	for name, tc := range map[string]struct {

internal/provider/custom.go

@@ -1,18 +1,55 @@
 package provider
 
 import (
+	"net/url"
+	"strings"
+
 	"github.com/favonia/cloudflare-ddns/internal/ipnet"
+	"github.com/favonia/cloudflare-ddns/internal/pp"
 	"github.com/favonia/cloudflare-ddns/internal/provider/protocol"
 )
 
 // NewCustom creates a HTTP provider.
-func NewCustom(url string) Provider {
+func NewCustom(ppfmt pp.PP, rawURL string) (Provider, bool) {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		ppfmt.Errorf(pp.EmojiUserError, "Failed to parse the custom provider (redacted)")
+		return nil, false
+	}
+
+	if !u.IsAbs() || u.Opaque != "" || u.Host == "" {
+		ppfmt.Errorf(pp.EmojiUserError, `The custom provider (redacted) does not look like a valid URL`)
+		return nil, false
+	}
+
+	switch u.Scheme {
+	case "http":
+		ppfmt.Warningf(pp.EmojiUserWarning, "The custom provider (redacted) uses HTTP; consider using HTTPS instead")
+
+	case "https":
+		// HTTPS is good!
+
+	default:
+		ppfmt.Errorf(pp.EmojiUserError, `The custom provider (redacted) must use HTTP or HTTPS`)
+		return nil, false
+	}
+
 	return &protocol.HTTP{
 		ProviderName:     "custom",
 		Is1111UsedForIP4: false,
 		URL: map[ipnet.Type]protocol.Switch{
-			ipnet.IP4: protocol.Constant(url),
-			ipnet.IP6: protocol.Constant(url),
+			ipnet.IP4: protocol.Constant(rawURL),
+			ipnet.IP6: protocol.Constant(rawURL),
 		},
+	}, true
+}
+
+// MustNewCustom creates a HTTP provider and panics if it fails.
+func MustNewCustom(rawURL string) Provider {
+	var buf strings.Builder
+	p, ok := NewCustom(pp.New(&buf), rawURL)
+	if !ok {
+		panic(buf.String())
 	}
+	return p
 }

internal/provider/custom_test.go

@@ -4,12 +4,69 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"go.uber.org/mock/gomock"
 
+	"github.com/favonia/cloudflare-ddns/internal/mocks"
+	"github.com/favonia/cloudflare-ddns/internal/pp"
 	"github.com/favonia/cloudflare-ddns/internal/provider"
 )
 
 func TestCustomName(t *testing.T) {
 	t.Parallel()
 
-	require.Equal(t, "custom", provider.Name(provider.NewCustom("")))
+	require.Equal(t, "custom", provider.Name(provider.MustNewCustom("https://1.1.1.1/")))
+}
+
+func TestNewCustom(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		input         string
+		ok            bool
+		prepareMockPP func(*mocks.MockPP)
+	}{
+		{"https://1.2.3.4", true, nil},
+		{
+			":::::", false,
+			func(m *mocks.MockPP) {
+				m.EXPECT().Errorf(pp.EmojiUserError, "Failed to parse the custom provider (redacted)")
+			},
+		},
+		{
+			"http://1.2.3.4", true,
+			func(m *mocks.MockPP) {
+				m.EXPECT().Warningf(pp.EmojiUserWarning, "The custom provider (redacted) uses HTTP; consider using HTTPS instead")
+			},
+		},
+		{
+			"ftp://1.2.3.4", false,
+			func(m *mocks.MockPP) {
+				m.EXPECT().Errorf(pp.EmojiUserError, `The custom provider (redacted) must use HTTP or HTTPS`)
+			},
+		},
+		{
+			"", false,
+			func(m *mocks.MockPP) {
+				m.EXPECT().Errorf(pp.EmojiUserError, `The custom provider (redacted) does not look like a valid URL`)
+			},
+		},
+	} {
+		tc := tc
+		t.Run(tc.input, func(t *testing.T) {
+			t.Parallel()
+
+			mockCtrl := gomock.NewController(t)
+			mockPP := mocks.NewMockPP(mockCtrl)
+			if tc.prepareMockPP != nil {
+				tc.prepareMockPP(mockPP)
+			}
+			p, ok := provider.NewCustom(mockPP, tc.input)
+			require.Equal(t, tc.ok, ok)
+			if ok {
+				require.NotNil(t, p)
+			} else {
+				require.Nil(t, p)
+			}
+		})
+	}
 }