Repository: favonia/cloudflare-ddns
Pull request #495 (2023-05-22 15:20:27)

fix(config): display a message when 1.0.0.1 also doesn't work

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/495?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) Report
> Merging [#495](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/495?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) (b70062a) into [main](https://app.codecov.io/gh/favonia/cloudflare-ddns/commit/d0db1be82e6f9df0e894f9e6ea001b83701dfd81?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) (d0db1be) will **decrease** coverage by `0.09%`.
> The diff coverage is `0.00%`.

```diff
@@            Coverage Diff             @@
##             main     #495      +/-   ##
==========================================
- Coverage   87.86%   87.78%   -0.09%     
==========================================
  Files          42       42              
  Lines        2159     2161       +2     
==========================================
  Hits         1897     1897              
- Misses        253      255       +2     
  Partials        9        9              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `87.78% <0.00%> (-0.09%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | Coverage Δ | |
|---|---|---|
| [internal/config/network\_probe.go](https://app.codecov.io/gh/favonia/cloudflare-ddns/pull/495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#diff-aW50ZXJuYWwvY29uZmlnL25ldHdvcmtfcHJvYmUuZ28=) | `75.67% <0.00%> (-4.33%)` | :arrow_down: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia)


Patch:
diff --git a/internal/config/network_probe.go b/internal/config/network_probe.go
index ba5a54d..40a9e0d 100644
--- a/internal/config/network_probe.go
+++ b/internal/config/network_probe.go
@@ -33,9 +33,10 @@ func ProbeURL(ctx context.Context, url string) bool {
 // ShouldWeUse1001 quickly checks 1.1.1.1 and 1.0.0.1 and notes whether 1.0.0.1 should be used.
 func (c *Config) ShouldWeUse1001(ctx context.Context, ppfmt pp.PP) bool {
 	c.Use1001 = false
-	if c.Provider[ipnet.IP4] == nil {
+	if c.Provider[ipnet.IP4] == nil || !c.Provider[ipnet.IP4].ShouldWeCheck1111() {
 		return true
 	}
+
 	if ppfmt.IsEnabledFor(pp.Info) {
 		ppfmt.Infof(pp.EmojiEnvVars, "Checking 1.1.1.1 . . .")
 		ppfmt = ppfmt.IncIndent()
@@ -44,11 +45,13 @@ func (c *Config) ShouldWeUse1001(ctx context.Context, ppfmt pp.PP) bool {
 	if ProbeURL(ctx, "https://1.1.1.1") {
 		ppfmt.Infof(pp.EmojiGood, "1.1.1.1 appears to be working")
 	} else {
-		ppfmt.Warningf(pp.EmojiError, "1.1.1.1 appears to be blocked or intercepted by your ISP or your router")
+		ppfmt.Warningf(pp.EmojiError, "1.1.1.1 appears to be blocked or hijacked by your ISP or your router")
 
 		if ProbeURL(ctx, "https://1.0.0.1") {
 			ppfmt.Warningf(pp.EmojiGood, "1.0.0.1 appears to be working and will be used instead of 1.1.1.1")
 			c.Use1001 = true
+		} else {
+			ppfmt.Warningf(pp.EmojiGood, "1.0.0.1 appears to be blocked or hijacked as well; sticking to 1.1.1.1")
 		}
 	}
 	return true
diff --git a/internal/mocks/mock_provider.go b/internal/mocks/mock_provider.go
index d0543c2..ac83e46 100644
--- a/internal/mocks/mock_provider.go
+++ b/internal/mocks/mock_provider.go
@@ -65,3 +65,17 @@ func (mr *MockProviderMockRecorder) Name() *gomock.Call {
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Name", reflect.TypeOf((*MockProvider)(nil).Name))
 }
+
+// ShouldWeCheck1111 mocks base method.
+func (m *MockProvider) ShouldWeCheck1111() bool {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ShouldWeCheck1111")
+	ret0, _ := ret[0].(bool)
+	return ret0
+}
+
+// ShouldWeCheck1111 indicates an expected call of ShouldWeCheck1111.
+func (mr *MockProviderMockRecorder) ShouldWeCheck1111() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldWeCheck1111", reflect.TypeOf((*MockProvider)(nil).ShouldWeCheck1111))
+}
diff --git a/internal/provider/base.go b/internal/provider/base.go
index 1856ecf..d2b34c8 100644
--- a/internal/provider/base.go
+++ b/internal/provider/base.go
@@ -13,8 +13,16 @@ import (
 
 // Provider is the abstraction of a protocol to detect public IP addresses.
 type Provider interface {
-	Name() string                                                                              // name of the protocol
-	GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, use1001 bool) (netip.Addr, bool) // get the IP
+	Name() string
+	// Name gives the name of the protocol
+
+	ShouldWeCheck1111() bool
+	// ShouldWeCheck1111() says whether the provider will connect to 1.1.1.1.
+	// Only when the IPv4 provider returns yes on this operation
+	// the hijacking detection of 1.1.1.1 will be performed.
+
+	GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, use1001 bool) (netip.Addr, bool)
+	// Actually get the IP
 }
 
 // Name gets the protocol name. It returns "none" for nil.
diff --git a/internal/provider/cloudflare_doh.go b/internal/provider/cloudflare_doh.go
index 8b6bedf..7ed993c 100644
--- a/internal/provider/cloudflare_doh.go
+++ b/internal/provider/cloudflare_doh.go
@@ -11,7 +11,8 @@ import (
 // If use1001 is true, 1.0.0.1 is used instead of 1.1.1.1.
 func NewCloudflareDOH() Provider {
 	return &protocol.DNSOverHTTPS{
-		ProviderName: "cloudflare.doh",
+		ProviderName:     "cloudflare.doh",
+		Is1111UsedForIP4: true,
 		Param: map[ipnet.Type]struct {
 			URL   protocol.Switch
 			Name  string
diff --git a/internal/provider/cloudflare_trace.go b/internal/provider/cloudflare_trace.go
index 16366eb..4bb035a 100644
--- a/internal/provider/cloudflare_trace.go
+++ b/internal/provider/cloudflare_trace.go
@@ -9,7 +9,8 @@ import (
 // If use1001 is true, 1.0.0.1 is used instead of 1.1.1.1.
 func NewCloudflareTrace() Provider {
 	return &protocol.Field{
-		ProviderName: "cloudflare.trace",
+		ProviderName:     "cloudflare.trace",
+		Is1111UsedforIP4: true,
 		Param: map[ipnet.Type]struct {
 			URL   protocol.Switch
 			Field string
diff --git a/internal/provider/ipify.go b/internal/provider/ipify.go
index 74402d1..ccde42b 100644
--- a/internal/provider/ipify.go
+++ b/internal/provider/ipify.go
@@ -8,7 +8,8 @@ import (
 // NewIpify creates a specialized HTTP provider that uses the ipify service.
 func NewIpify() Provider {
 	return &protocol.HTTP{
-		ProviderName: "ipify",
+		ProviderName:     "ipify",
+		Is1111UsedForIP4: false,
 		URL: map[ipnet.Type]protocol.Switch{
 			ipnet.IP4: protocol.Constant("https://api4.ipify.org"),
 			ipnet.IP6: protocol.Constant("https://api6.ipify.org"),
diff --git a/internal/provider/local_cloudflare.go b/internal/provider/local_cloudflare.go
index 90ffc7a..c4258ba 100644
--- a/internal/provider/local_cloudflare.go
+++ b/internal/provider/local_cloudflare.go
@@ -6,15 +6,14 @@ import (
 )
 
 // NewLocal creates a specialized Local provider that uses Cloudflare as the remote server.
-// (No actual UDP packets will be sent out.)
+// (No actual UDP packets will be sent to Cloudflare.)
 func NewLocal() Provider {
 	return &protocol.Local{
-		ProviderName: "local",
+		ProviderName:     "local",
+		Is1111UsedForIP4: false, // 1.0.0.1 is used in case 1.1.1.1 is hijacked by the router
 		RemoteUDPAddr: map[ipnet.Type]protocol.Switch{
-			ipnet.IP4: protocol.Switchable{
-				Use1111: "1.1.1.1:443",
-				Use1001: "1.0.0.1:443",
-			},
+			// 1.0.0.1 is used in case 1.1.1.1 is hijacked by the router
+			ipnet.IP4: protocol.Constant("1.0.0.1:443"),
 			ipnet.IP6: protocol.Constant("[2606:4700:4700::1111]:443"),
 		},
 	}
diff --git a/internal/provider/protocol/doh.go b/internal/provider/protocol/doh.go
index 940fa8c..7cb7ebe 100644
--- a/internal/provider/protocol/doh.go
+++ b/internal/provider/protocol/doh.go
@@ -168,8 +168,9 @@ func getIPFromDNS(ctx context.Context, ppfmt pp.PP,
 
 // DNSOverHTTPS represents a generic detection protocol using DNS over HTTPS.
 type DNSOverHTTPS struct {
-	ProviderName string // name of the protocol
-	Param        map[ipnet.Type]struct {
+	ProviderName     string // name of the protocol
+	Is1111UsedForIP4 bool   // whether 1.1.1.1 is used
+	Param            map[ipnet.Type]struct {
 		URL   Switch           // the DoH server
 		Name  string           // domain name to query
 		Class dnsmessage.Class // DNS class to query
@@ -196,3 +197,6 @@ func (p *DNSOverHTTPS) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type,
 
 	return ipNet.NormalizeDetectedIP(ppfmt, ip)
 }
+
+// ShouldWeCheck1111 returns whether we should check 1.1.1.1.
+func (p *DNSOverHTTPS) ShouldWeCheck1111() bool { return p.Is1111UsedForIP4 }
diff --git a/internal/provider/protocol/field.go b/internal/provider/protocol/field.go
index 23e6eb9..15d43e6 100644
--- a/internal/provider/protocol/field.go
+++ b/internal/provider/protocol/field.go
@@ -40,8 +40,9 @@ func getIPFromField(ctx context.Context, ppfmt pp.PP, url string, field string)
 
 // Field represents a generic detection protocol to parse an HTTP response.
 type Field struct {
-	ProviderName string // name of the detection protocol
-	Param        map[ipnet.Type]struct {
+	ProviderName     string // name of the detection protocol
+	Is1111UsedforIP4 bool
+	Param            map[ipnet.Type]struct {
 		URL   Switch // URL of the detection page
 		Field string // name of the field holding the IP address
 	}
@@ -67,3 +68,6 @@ func (p *Field) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, use100
 
 	return ipNet.NormalizeDetectedIP(ppfmt, ip)
 }
+
+// ShouldWeCheck1111 returns whether we should check 1.1.1.1.
+func (p *Field) ShouldWeCheck1111() bool { return p.Is1111UsedforIP4 }
diff --git a/internal/provider/protocol/http.go b/internal/provider/protocol/http.go
index 189d21e..043cd24 100644
--- a/internal/provider/protocol/http.go
+++ b/internal/provider/protocol/http.go
@@ -31,8 +31,9 @@ func getIPFromHTTP(ctx context.Context, ppfmt pp.PP, url string) (netip.Addr, bo
 
 // HTTP represents a generic detection protocol to use an HTTP response directly.
 type HTTP struct {
-	ProviderName string                // name of the protocol
-	URL          map[ipnet.Type]Switch // URL of the detection page
+	ProviderName     string                // name of the protocol
+	Is1111UsedForIP4 bool                  // whether 1.1.1.1 is used
+	URL              map[ipnet.Type]Switch // URL of the detection page
 }
 
 // Name of the detection protocol.
@@ -55,3 +56,6 @@ func (p *HTTP) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, use1001
 
 	return ipNet.NormalizeDetectedIP(ppfmt, ip)
 }
+
+// ShouldWeCheck1111 returns whether we should check 1.1.1.1.
+func (p *HTTP) ShouldWeCheck1111() bool { return p.Is1111UsedForIP4 }
diff --git a/internal/provider/protocol/local.go b/internal/provider/protocol/local.go
index dbebd5a..9468792 100644
--- a/internal/provider/protocol/local.go
+++ b/internal/provider/protocol/local.go
@@ -17,6 +17,9 @@ type Local struct {
 	// Name of the detection protocol.
 	ProviderName string
 
+	// Whether 1.1.1.1 is used for IPv4
+	Is1111UsedForIP4 bool
+
 	// The target IP address of the UDP packet to be sent.
 	RemoteUDPAddr map[ipnet.Type]Switch
 }
@@ -48,3 +51,6 @@ func (p *Local) GetIP(_ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type, use10
 
 	return ipNet.NormalizeDetectedIP(ppfmt, ip)
 }
+
+// ShouldWeCheck1111 returns whether we should check 1.1.1.1.
+func (p *Local) ShouldWeCheck1111() bool { return p.Is1111UsedForIP4 }


Test patch:
diff --git a/internal/provider/protocol/doh_test.go b/internal/provider/protocol/doh_test.go
index 252728d..49c9688 100644
--- a/internal/provider/protocol/doh_test.go
+++ b/internal/provider/protocol/doh_test.go
@@ -22,8 +22,9 @@ func TestDNSOverHTTPSName(t *testing.T) {
 	t.Parallel()
 
 	p := &protocol.DNSOverHTTPS{
-		ProviderName: "very secret name",
-		Param:        nil,
+		ProviderName:     "very secret name",
+		Is1111UsedForIP4: false,
+		Param:            nil,
 	}
 
 	require.Equal(t, "very secret name", p.Name())
@@ -570,7 +571,8 @@ func TestDNSOverHTTPSGetIP(t *testing.T) {
 			server := setupServer(t, tc.name, tc.class, tc.response, tc.header, tc.idShift, tc.answers)
 
 			provider := &protocol.DNSOverHTTPS{
-				ProviderName: "",
+				ProviderName:     "",
+				Is1111UsedForIP4: false,
 				Param: map[ipnet.Type]struct {
 					URL   protocol.Switch
 					Name  string
@@ -590,3 +592,19 @@ func TestDNSOverHTTPSGetIP(t *testing.T) {
 		})
 	}
 }
+
+func TestDOHShouldWeCheck1111(t *testing.T) {
+	t.Parallel()
+
+	require.True(t, (&protocol.DNSOverHTTPS{
+		ProviderName:     "",
+		Is1111UsedForIP4: true,
+		Param:            nil,
+	}).ShouldWeCheck1111())
+
+	require.False(t, (&protocol.DNSOverHTTPS{
+		ProviderName:     "",
+		Is1111UsedForIP4: false,
+		Param:            nil,
+	}).ShouldWeCheck1111())
+}
diff --git a/internal/provider/protocol/field_test.go b/internal/provider/protocol/field_test.go
index db72b62..38fb510 100644
--- a/internal/provider/protocol/field_test.go
+++ b/internal/provider/protocol/field_test.go
@@ -21,8 +21,9 @@ func TestFieldName(t *testing.T) {
 	t.Parallel()
 
 	p := &protocol.Field{
-		ProviderName: "very secret name",
-		Param:        nil,
+		ProviderName:     "very secret name",
+		Is1111UsedforIP4: false,
+		Param:            nil,
 	}
 
 	require.Equal(t, "very secret name", p.Name())
@@ -149,7 +150,8 @@ func TestFieldGetIP(t *testing.T) {
 				mockCtrl := gomock.NewController(t)
 
 				provider := &protocol.Field{
-					ProviderName: "secret name",
+					ProviderName:     "secret name",
+					Is1111UsedforIP4: false,
 					Param: map[ipnet.Type]struct {
 						URL   protocol.Switch
 						Field string
@@ -172,3 +174,19 @@ func TestFieldGetIP(t *testing.T) {
 		}
 	})
 }
+
+func TestFieldShouldWeCheck1111(t *testing.T) {
+	t.Parallel()
+
+	require.True(t, (&protocol.Field{
+		ProviderName:     "",
+		Is1111UsedforIP4: true,
+		Param:            nil,
+	}).ShouldWeCheck1111())
+
+	require.False(t, (&protocol.Field{
+		ProviderName:     "",
+		Is1111UsedforIP4: false,
+		Param:            nil,
+	}).ShouldWeCheck1111())
+}
diff --git a/internal/provider/protocol/http_test.go b/internal/provider/protocol/http_test.go
index 2181939..c248cff 100644
--- a/internal/provider/protocol/http_test.go
+++ b/internal/provider/protocol/http_test.go
@@ -21,8 +21,9 @@ func TestHTTPName(t *testing.T) {
 	t.Parallel()
 
 	p := &protocol.HTTP{
-		ProviderName: "very secret name",
-		URL:          nil,
+		ProviderName:     "very secret name",
+		Is1111UsedForIP4: false,
+		URL:              nil,
 	}
 
 	require.Equal(t, "very secret name", p.Name())
@@ -141,7 +142,8 @@ func TestHTTPGetIP(t *testing.T) {
 				mockCtrl := gomock.NewController(t)
 
 				provider := &protocol.HTTP{
-					ProviderName: "secret name",
+					ProviderName:     "secret name",
+					Is1111UsedForIP4: false,
 					URL: map[ipnet.Type]protocol.Switch{
 						tc.urlKey: protocol.Constant(tc.url),
 					},
@@ -162,3 +164,19 @@ func TestHTTPGetIP(t *testing.T) {
 		}
 	})
 }
+
+func TestHTTPShouldWeCheck1111(t *testing.T) {
+	t.Parallel()
+
+	require.True(t, (&protocol.HTTP{
+		ProviderName:     "",
+		Is1111UsedForIP4: true,
+		URL:              nil,
+	}).ShouldWeCheck1111())
+
+	require.False(t, (&protocol.HTTP{
+		ProviderName:     "",
+		Is1111UsedForIP4: false,
+		URL:              nil,
+	}).ShouldWeCheck1111())
+}
diff --git a/internal/provider/protocol/local_test.go b/internal/provider/protocol/local_test.go
index 1bd25db..a2bfa88 100644
--- a/internal/provider/protocol/local_test.go
+++ b/internal/provider/protocol/local_test.go
@@ -18,8 +18,9 @@ func TestLocalName(t *testing.T) {
 	t.Parallel()
 
 	p := &protocol.Local{
-		ProviderName:  "very secret name",
-		RemoteUDPAddr: nil,
+		ProviderName:     "very secret name",
+		Is1111UsedForIP4: false,
+		RemoteUDPAddr:    nil,
 	}
 
 	require.Equal(t, "very secret name", p.Name())
@@ -83,7 +84,8 @@ func TestLocalGetIP(t *testing.T) {
 			mockCtrl := gomock.NewController(t)
 
 			provider := &protocol.Local{
-				ProviderName: "",
+				ProviderName:     "",
+				Is1111UsedForIP4: false,
 				RemoteUDPAddr: map[ipnet.Type]protocol.Switch{
 					tc.addrKey: protocol.Constant(tc.addr),
 				},
@@ -99,3 +101,19 @@ func TestLocalGetIP(t *testing.T) {
 		})
 	}
 }
+
+func TestLocalShouldWeCheck1111(t *testing.T) {
+	t.Parallel()
+
+	require.True(t, (&protocol.Local{
+		ProviderName:     "",
+		Is1111UsedForIP4: true,
+		RemoteUDPAddr:    nil,
+	}).ShouldWeCheck1111())
+
+	require.False(t, (&protocol.Local{
+		ProviderName:     "",
+		Is1111UsedForIP4: false,
+		RemoteUDPAddr:    nil,
+	}).ShouldWeCheck1111())
+}