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

* fix(config): display a message when 1.0.0.1 also doesn't work * feat(provider): check 1.1.1.1 only when we might actually use 1.1.1.1 * fix(provider): fix the local IP provider * test(provider): improve coverage * style(config): minor style tweak
favonia · May 22, 2023 · 5f5602d · 5f5602d
1 parent d0db1be
commit 5f5602d
Show file tree

Hide file tree

Showing 15 changed files with 148 additions and 31 deletions.
diff --git 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/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.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/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.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/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.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 }
diff --git 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())
+}