fix(ipnet): tighten the checking of IP addresses

internal/ipnet/ipnet.go

@@ -66,6 +66,7 @@ func (t Type) UDPNetwork() string {
 
 // Matches checks whether an IP belongs to it.
 func (t Type) Matches(ip netip.Addr) bool {
+	ip = ip.Unmap()
 	switch t {
 	case IP4:
 		return ip.Is4()
@@ -79,9 +80,9 @@ func (t Type) Matches(ip netip.Addr) bool {
 // NormalizeDetectedIP normalizes an IP into an IPv4 or IPv6 address.
 func (t Type) NormalizeDetectedIP(ppfmt pp.PP, ip netip.Addr) (netip.Addr, bool) {
 	if !ip.IsValid() {
-		ppfmt.Noticef(
-			pp.EmojiImpossible,
-			`Detected IP address is not valid`,
+		ppfmt.Noticef(pp.EmojiImpossible,
+			`Detected IP address is not valid; this should not happen and please report it at %s`,
+			pp.IssueReportingURL,
 		)
 		return netip.Addr{}, false
 	}
@@ -116,20 +117,42 @@ func (t Type) NormalizeDetectedIP(ppfmt pp.PP, ip netip.Addr) (netip.Addr, bool)
 	}
 
 	if ip.IsUnspecified() {
-		ppfmt.Noticef(
-			pp.EmojiImpossible,
-			`Detected IP address %s is an unspecified %s address`,
-			ip.String(),
-			t.Describe(),
+		ppfmt.Noticef(pp.EmojiError,
+			`Detected %s address %s is an unspecified address`,
+			t.Describe(), ip.String(),
+		)
+		return netip.Addr{}, false
+	}
+
+	if ip.IsLoopback() {
+		ppfmt.Noticef(pp.EmojiError,
+			`Detected %s address %s is a loopback address`,
+			t.Describe(), ip.String(),
+		)
+		return netip.Addr{}, false
+	}
+
+	if ip.IsInterfaceLocalMulticast() || ip.IsLinkLocalMulticast() || ip.IsLinkLocalUnicast() {
+		ppfmt.Noticef(pp.EmojiError,
+			`Detected %s address %s is an interface-local or link-local address`,
+			t.Describe(), ip.String(),
+		)
+		return netip.Addr{}, false
+	}
+
+	if ip.IsMulticast() {
+		ppfmt.Noticef(pp.EmojiError,
+			`Detected %s address %s is a multicast address`,
+			t.Describe(), ip.String(),
 		)
 		return netip.Addr{}, false
 	}
 
 	if !ip.IsGlobalUnicast() {
 		ppfmt.Noticef(
-			pp.EmojiUserWarning,
-			`Detected IP address %s does not look like a global unicast IP address.`,
-			ip.String(),
+			pp.EmojiWarning,
+			`Detected %s address %s does not look like a global unicast address`,
+			t.Describe(), ip.String(),
 		)
 	}
 

internal/ipnet/ipnet_test.go

@@ -134,7 +134,9 @@ func TestNormalizeDetectedIP(t *testing.T) {
 			netip.Addr{},
 			false,
 			func(m *mocks.MockPP) {
-				m.EXPECT().Noticef(pp.EmojiImpossible, "Detected IP address is not valid")
+				m.EXPECT().Noticef(pp.EmojiImpossible,
+					`Detected IP address is not valid; this should not happen and please report it at %s`,
+					pp.IssueReportingURL)
 			},
 		},
 		"100-10.10.10.10": {
@@ -148,8 +150,8 @@ func TestNormalizeDetectedIP(t *testing.T) {
 			netip.Addr{},
 			false,
 			func(m *mocks.MockPP) {
-				m.EXPECT().Noticef(pp.EmojiImpossible,
-					"Detected IP address %s is an unspecified %s address", "0.0.0.0", "IPv4")
+				m.EXPECT().Noticef(pp.EmojiError,
+					"Detected %s address %s is an unspecified address", "IPv4", "0.0.0.0")
 			},
 		},
 	} {

internal/provider/happy_eyeballs.go

@@ -105,7 +105,7 @@ func (p happyEyeballs) GetIP(ctx context.Context, ppfmt pp.PP, ipNet ipnet.Type)
 				queuedPP[r.method].Flush()
 
 				if r.method == MethodAlternative {
-					ppfmt.Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
+					ppfmt.Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
 				}
 
 				return r.ip, r.method, r.ok

internal/provider/happy_eyeballs_test.go

@@ -86,7 +86,7 @@ func TestHappyEyeballs(t *testing.T) {
 						return ip1001, true
 					}).Times(2)
 				ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
-				ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
+				ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
 				p.EXPECT().HasAlternative(ipNet).Return(true)
 				ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
 			},
@@ -150,7 +150,7 @@ func TestHappyEyeballs(t *testing.T) {
 						return ip1001, true
 					}).Times(2)
 				ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
-				ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
+				ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
 				p.EXPECT().HasAlternative(ipNet).Return(true)
 				ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
 			},
@@ -192,7 +192,7 @@ func TestHappyEyeballs(t *testing.T) {
 						return ip1001, true
 					}).Times(2)
 				ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
-				ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on.")
+				ppfmt.EXPECT().Infof(pp.EmojiNow, "The server 1.0.0.1 responded before 1.1.1.1 does and will be used from now on")
 				p.EXPECT().HasAlternative(ipNet).Return(true)
 				ppfmt.EXPECT().Infof(pp.EmojiGood, "Got 1.0.0.1!")
 			},

internal/provider/protocol/local_test.go

@@ -28,71 +28,56 @@ func TestLocalName(t *testing.T) {
 func TestLocalGetIP(t *testing.T) {
 	t.Parallel()
 
-	ip4Loopback := netip.MustParseAddr("127.0.0.1")
-	ip6Loopback := netip.MustParseAddr("::1")
 	invalidIP := netip.Addr{}
 
 	for name, tc := range map[string]struct {
 		addrKey       ipnet.Type
 		addr          string
 		ipNet         ipnet.Type
-		expected      gomock.Matcher
 		ok            bool
+		expected      netip.Addr
 		prepareMockPP func(*mocks.MockPP)
 	}{
-		"4": {
-			ipnet.IP4, "127.0.0.1:80", ipnet.IP4, gomock.Eq(ip4Loopback), true,
+		"loopback/4": {
+			ipnet.IP4, "127.0.0.1:80", ipnet.IP4,
+			false, invalidIP,
 			func(m *mocks.MockPP) {
-				m.EXPECT().Noticef(pp.EmojiUserWarning,
-					"Detected IP address %s does not look like a global unicast IP address.", "127.0.0.1")
+				m.EXPECT().Noticef(pp.EmojiError,
+					"Detected %s address %s is a loopback address", "IPv4", "127.0.0.1")
 			},
 		},
-		"6": {
+		"loopback/6": {
 			ipnet.IP6, "[::1]:80", ipnet.IP6,
-			gomock.AnyOf(
-				ip6Loopback,
-				gomock.Cond(func(x any) bool {
-					a, ok := x.(netip.Addr)
-					if !ok {
-						return false
-					}
-					return a.IsLinkLocalUnicast()
-				})),
-			true,
+			false, invalidIP,
 			func(m *mocks.MockPP) {
-				m.EXPECT().Noticef(pp.EmojiUserWarning,
-					"Detected IP address %s does not look like a global unicast IP address.",
-					gomock.AnyOf(
-						"::1",
-						gomock.Cond(func(x any) bool {
-							s, ok := x.(string)
-							if !ok {
-								return false
-							}
-							return netip.MustParseAddr(s).IsLinkLocalUnicast()
-						})))
+				m.EXPECT().Noticef(pp.EmojiError,
+					"Detected %s address %s is a loopback address", "IPv6", "::1")
 			},
 		},
-		"4-nil1": {
-			ipnet.IP4, "", ipnet.IP4, gomock.Eq(invalidIP), false,
+		"empty/4": {
+			ipnet.IP4, "", ipnet.IP4,
+			false, invalidIP,
 			func(m *mocks.MockPP) {
 				m.EXPECT().Noticef(pp.EmojiError, "Failed to detect a local %s address: %v", "IPv4", gomock.Any())
 			},
 		},
-		"6-nil1": {
-			ipnet.IP6, "", ipnet.IP6, gomock.Eq(invalidIP), false,
+		"empty/6": {
+			ipnet.IP6, "", ipnet.IP6,
+			false, invalidIP,
 			func(m *mocks.MockPP) {
 				m.EXPECT().Noticef(pp.EmojiError, "Failed to detect a local %s address: %v", "IPv6", gomock.Any())
 			},
 		},
-		"4-nil2": {
-			ipnet.IP4, "127.0.0.1:80", ipnet.IP6, gomock.Eq(invalidIP), false,
+		"mismatch/6": {
+			ipnet.IP4, "127.0.0.1:80", ipnet.IP6,
+			false, invalidIP,
 			func(m *mocks.MockPP) {
 				m.EXPECT().Noticef(pp.EmojiImpossible, "Unhandled IP network: %s", "IPv6")
 			},
 		},
-		"6-nil2": {
-			ipnet.IP6, "::1:80", ipnet.IP4, gomock.Eq(invalidIP), false,
+		"mismatch/4": {
+			ipnet.IP6, "::1:80", ipnet.IP4,
+			false, invalidIP,
 			func(m *mocks.MockPP) {
 				m.EXPECT().Noticef(pp.EmojiImpossible, "Unhandled IP network: %s", "IPv4")
 			},
@@ -114,7 +99,7 @@ func TestLocalGetIP(t *testing.T) {
 				tc.prepareMockPP(mockPP)
 			}
 			ip, method, ok := provider.GetIP(context.Background(), mockPP, tc.ipNet)
-			require.True(t, tc.expected.Matches(ip))
+			require.Equal(t, tc.expected, ip)
 			require.NotEqual(t, protocol.MethodAlternative, method)
 			require.Equal(t, tc.ok, ok)
 		})