From d001242206155298644132d00715df30f7eeacec Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 14 Nov 2022 10:35:21 -0500 Subject: [PATCH] Refactor IsIPv*() tests Merge the IsIPv4String and IsIPv6String tests together and add more cases. Merge the IsIPv4 and IsIPv6 tests together and add more cases. Merge all the CIDR tests together, both string and binary, since there are no cases there that we care about testing that can't be expressed as a string to be input to ParseCIDR(). --- net/ipfamily_test.go | 525 +++++++++++++++++++++---------------------- 1 file changed, 261 insertions(+), 264 deletions(-) diff --git a/net/ipfamily_test.go b/net/ipfamily_test.go index 8069d439..2ec7a2a0 100644 --- a/net/ipfamily_test.go +++ b/net/ipfamily_test.go @@ -17,6 +17,7 @@ limitations under the License. package net import ( + "fmt" "net" "testing" ) @@ -227,400 +228,396 @@ func TestDualStackCIDRs(t *testing.T) { } } -func TestIsIPv6String(t *testing.T) { +func TestIsIPvXString(t *testing.T) { testCases := []struct { - ip string - expectIPv6 bool + desc string + ip string + family IPFamily }{ { - ip: "127.0.0.1", - expectIPv6: false, + desc: "IPv4 1", + ip: "127.0.0.1", + family: IPv4, }, { - ip: "192.168.0.0", - expectIPv6: false, + desc: "IPv4 2", + ip: "192.168.0.0", + family: IPv4, }, { - ip: "1.2.3.4", - expectIPv6: false, + desc: "IPv4 3", + ip: "1.2.3.4", + family: IPv4, }, { - ip: "bad ip", - expectIPv6: false, + desc: "IPv4 with leading 0s is accepted", + ip: "001.002.003.004", + family: IPv4, }, { - ip: "::1", - expectIPv6: true, + desc: "IPv4 encoded as IPv6 is IPv4", + ip: "::FFFF:1.2.3.4", + family: IPv4, }, { - ip: "fd00::600d:f00d", - expectIPv6: true, + desc: "IPv6 1", + ip: "::1", + family: IPv6, }, { - ip: "2001:db8::5", - expectIPv6: true, - }, - } - for i := range testCases { - isIPv6 := IsIPv6String(testCases[i].ip) - if isIPv6 != testCases[i].expectIPv6 { - t.Errorf("[%d] Expect ipv6 %v, got %v", i+1, testCases[i].expectIPv6, isIPv6) - } - } -} - -func TestIsIPv6(t *testing.T) { - testCases := []struct { - ip net.IP - expectIPv6 bool - }{ - { - ip: net.IPv4zero, - expectIPv6: false, + desc: "IPv6 2", + ip: "fd00::600d:f00d", + family: IPv6, }, { - ip: net.IPv4bcast, - expectIPv6: false, + desc: "IPv6 3", + ip: "2001:db8::5", + family: IPv6, }, { - ip: ParseIPSloppy("127.0.0.1"), - expectIPv6: false, + desc: "IPv4 with out-of-range octets is not accepted", + ip: "1.2.3.400", + family: IPFamilyUnknown, }, { - ip: ParseIPSloppy("10.20.40.40"), - expectIPv6: false, + desc: "IPv6 with out-of-range segment is not accepted", + ip: "2001:db8::10005", + family: IPFamilyUnknown, }, { - ip: ParseIPSloppy("172.17.3.0"), - expectIPv6: false, + desc: "IPv4 with empty octet is not accepted", + ip: "1.2..4", + family: IPFamilyUnknown, }, { - ip: nil, - expectIPv6: false, + desc: "IPv6 with multiple empty segments is not accepted", + ip: "2001::db8::5", + family: IPFamilyUnknown, }, { - ip: net.IPv6loopback, - expectIPv6: true, + desc: "IPv4 CIDR is not accepted", + ip: "1.2.3.4/32", + family: IPFamilyUnknown, }, { - ip: net.IPv6zero, - expectIPv6: true, + desc: "IPv6 CIDR is not accepted", + ip: "2001:db8::/64", + family: IPFamilyUnknown, }, { - ip: ParseIPSloppy("fd00::600d:f00d"), - expectIPv6: true, + desc: "IPv4:port is not accepted", + ip: "1.2.3.4:80", + family: IPFamilyUnknown, }, { - ip: ParseIPSloppy("2001:db8::5"), - expectIPv6: true, + desc: "[IPv6] with brackets is not accepted", + ip: "[2001:db8::5]", + family: IPFamilyUnknown, }, - } - for i := range testCases { - isIPv6 := IsIPv6(testCases[i].ip) - if isIPv6 != testCases[i].expectIPv6 { - t.Errorf("[%d] Expect ipv6 %v, got %v", i+1, testCases[i].expectIPv6, isIPv6) - } - } -} - -func TestIsIPv6CIDRString(t *testing.T) { - testCases := []struct { - desc string - cidr string - expectResult bool - }{ { - desc: "ipv4 CIDR 1", - cidr: "10.0.0.0/8", - expectResult: false, + desc: "[IPv6]:port is not accepted", + ip: "[2001:db8::5]:80", + family: IPFamilyUnknown, }, { - desc: "ipv4 CIDR 2", - cidr: "192.168.0.0/16", - expectResult: false, + desc: "IPv6%zone is not accepted", + ip: "fe80::1234%eth0", + family: IPFamilyUnknown, }, { - desc: "ipv6 CIDR 1", - cidr: "::/1", - expectResult: true, + desc: "IPv4 with leading whitespace is not accepted", + ip: " 1.2.3.4", + family: IPFamilyUnknown, }, { - desc: "ipv6 CIDR 2", - cidr: "2000::/10", - expectResult: true, + desc: "IPv4 with trailing whitespace is not accepted", + ip: "1.2.3.4 ", + family: IPFamilyUnknown, }, { - desc: "ipv6 CIDR 3", - cidr: "2001:db8::/32", - expectResult: true, + desc: "IPv6 with leading whitespace is not accepted", + ip: " 2001:db8::5", + family: IPFamilyUnknown, }, { - desc: "bad CIDR", - cidr: "foo", - expectResult: false, + desc: "IPv6 with trailing whitespace is not accepted", + ip: " 2001:db8::5", + family: IPFamilyUnknown, }, { - desc: "IP, not CIDR", - cidr: "2001:db8::", - expectResult: false, + desc: "random unparseable string", + ip: "bad ip", + family: IPFamilyUnknown, }, } - for _, tc := range testCases { - res := IsIPv6CIDRString(tc.cidr) - if res != tc.expectResult { - t.Errorf("%v: want IsIPv6CIDRString=%v, got %v", tc.desc, tc.expectResult, res) - } + t.Run(tc.desc, func(t *testing.T) { + isIPv4 := IsIPv4String(tc.ip) + isIPv6 := IsIPv6String(tc.ip) + + if isIPv4 != (tc.family == IPv4) { + t.Errorf("Expect %q ipv4 %v, got %v", tc.ip, tc.family == IPv4, isIPv6) + } + if isIPv6 != (tc.family == IPv6) { + t.Errorf("Expect %q ipv6 %v, got %v", tc.ip, tc.family == IPv6, isIPv6) + } + }) } } -func TestIsIPv6CIDR(t *testing.T) { - testCases := []struct { - desc string - cidr string - expectResult bool - }{ - { - desc: "ipv4 CIDR 1", - cidr: "10.0.0.0/8", - expectResult: false, - }, - { - desc: "ipv4 CIDR 2", - cidr: "192.168.0.0/16", - expectResult: false, - }, - { - desc: "ipv6 CIDR 1", - cidr: "::/1", - expectResult: true, - }, - { - desc: "ipv6 CIDR 2", - cidr: "2000::/10", - expectResult: true, - }, - { - desc: "ipv6 CIDR 3", - cidr: "2001:db8::/32", - expectResult: true, - }, - { - desc: "bad CIDR", - cidr: "foo", - expectResult: false, - }, - { - desc: "IP, not CIDR", - cidr: "2001:db8::", - expectResult: false, - }, +// This is specifically for testing "the kinds of net.IP values returned by net.ParseCIDR()" +func mustParseCIDRIP(cidrstr string) net.IP { + ip, _, err := net.ParseCIDR(cidrstr) + if ip == nil { + panic(fmt.Sprintf("bad test case: %q should have been parseable: %v", cidrstr, err)) } + return ip +} - for _, tc := range testCases { - _, cidr, _ := ParseCIDRSloppy(tc.cidr) - res := IsIPv6CIDR(cidr) - if res != tc.expectResult { - t.Errorf("%v: want IsIPv6CIDR=%v, got %v", tc.desc, tc.expectResult, res) - } +// This is specifically for testing "the kinds of net.IP values returned by net.ParseCIDR()" +func mustParseCIDRBase(cidrstr string) net.IP { + _, cidr, err := net.ParseCIDR(cidrstr) + if cidr == nil { + panic(fmt.Sprintf("bad test case: %q should have been parseable: %v", cidrstr, err)) + } + return cidr.IP +} + +// This is specifically for testing "the kinds of net.IP values returned by net.ParseCIDR()" +func mustParseCIDRMask(cidrstr string) net.IPMask { + _, cidr, err := net.ParseCIDR(cidrstr) + if cidr == nil { + panic(fmt.Sprintf("bad test case: %q should have been parseable: %v", cidrstr, err)) } + return cidr.Mask } -func TestIsIPv4String(t *testing.T) { +func TestIsIPvX(t *testing.T) { testCases := []struct { - ip string - expectIPv4 bool + desc string + ip net.IP + family IPFamily }{ { - ip: "127.0.0.1", - expectIPv4: true, + desc: "IPv4 all-zeros", + ip: net.IPv4zero, + family: IPv4, }, { - ip: "192.168.0.0", - expectIPv4: true, + desc: "IPv6 all-zeros", + ip: net.IPv6zero, + family: IPv6, }, { - ip: "1.2.3.4", - expectIPv4: true, + desc: "IPv4 broadcast", + ip: net.IPv4bcast, + family: IPv4, }, { - ip: "bad ip", - expectIPv4: false, + desc: "IPv4 loopback", + ip: net.ParseIP("127.0.0.1"), + family: IPv4, }, { - ip: "::1", - expectIPv4: false, + desc: "IPv6 loopback", + ip: net.IPv6loopback, + family: IPv6, }, { - ip: "fd00::600d:f00d", - expectIPv4: false, + desc: "IPv4 1", + ip: net.ParseIP("10.20.40.40"), + family: IPv4, }, { - ip: "2001:db8::5", - expectIPv4: false, + desc: "IPv4 2", + ip: net.ParseIP("172.17.3.0"), + family: IPv4, }, - } - for i := range testCases { - isIPv4 := IsIPv4String(testCases[i].ip) - if isIPv4 != testCases[i].expectIPv4 { - t.Errorf("[%d] Expect ipv4 %v, got %v", i+1, testCases[i].expectIPv4, isIPv4) - } - } -} - -func TestIsIPv4(t *testing.T) { - testCases := []struct { - ip net.IP - expectIPv4 bool - }{ { - ip: net.IPv4zero, - expectIPv4: true, + desc: "IPv4 encoded as IPv6 is IPv4", + ip: net.ParseIP("::FFFF:1.2.3.4"), + family: IPv4, }, { - ip: net.IPv4bcast, - expectIPv4: true, + desc: "IPv6 1", + ip: net.ParseIP("fd00::600d:f00d"), + family: IPv6, }, { - ip: ParseIPSloppy("127.0.0.1"), - expectIPv4: true, + desc: "IPv6 2", + ip: net.ParseIP("2001:db8::5"), + family: IPv6, }, { - ip: ParseIPSloppy("10.20.40.40"), - expectIPv4: true, + desc: "IP from IPv4 CIDR is IPv4", + ip: mustParseCIDRIP("192.168.1.1/24"), + family: IPv4, }, { - ip: ParseIPSloppy("172.17.3.0"), - expectIPv4: true, + desc: "CIDR base IP from IPv4 CIDR is IPv4", + ip: mustParseCIDRBase("192.168.1.1/24"), + family: IPv4, }, { - ip: nil, - expectIPv4: false, + desc: "CIDR mask from IPv4 CIDR is IPv4", + ip: net.IP(mustParseCIDRMask("192.168.1.1/24")), + family: IPv4, }, { - ip: net.IPv6loopback, - expectIPv4: false, + desc: "IP from IPv6 CIDR is IPv6", + ip: mustParseCIDRIP("2001:db8::5/64"), + family: IPv6, }, { - ip: net.IPv6zero, - expectIPv4: false, + desc: "CIDR base IP from IPv6 CIDR is IPv6", + ip: mustParseCIDRBase("2001:db8::5/64"), + family: IPv6, }, { - ip: ParseIPSloppy("fd00::600d:f00d"), - expectIPv4: false, + desc: "CIDR mask from IPv6 CIDR is IPv6", + ip: net.IP(mustParseCIDRMask("2001:db8::5/64")), + family: IPv6, }, { - ip: ParseIPSloppy("2001:db8::5"), - expectIPv4: false, + desc: "nil is accepted, but is neither IPv4 nor IPv6", + ip: nil, + family: IPFamilyUnknown, }, } - for i := range testCases { - isIPv4 := IsIPv4(testCases[i].ip) - if isIPv4 != testCases[i].expectIPv4 { - t.Errorf("[%d] Expect ipv4 %v, got %v", i+1, testCases[i].expectIPv4, isIPv4) - } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + isIPv4 := IsIPv4(tc.ip) + isIPv6 := IsIPv6(tc.ip) + + if isIPv4 != (tc.family == IPv4) { + t.Errorf("Expect ipv4 %v, got %v", tc.family == IPv4, isIPv6) + } + if isIPv6 != (tc.family == IPv6) { + t.Errorf("Expect ipv6 %v, got %v", tc.family == IPv6, isIPv6) + } + }) } } -func TestIsIPv4CIDRString(t *testing.T) { +func TestIsIPvXCIDR(t *testing.T) { testCases := []struct { - desc string - cidr string - expectResult bool + desc string + cidr string + family IPFamily }{ { - desc: "ipv4 CIDR 1", - cidr: "10.0.0.0/8", - expectResult: true, + desc: "IPv4 CIDR 1", + cidr: "10.0.0.0/8", + family: IPv4, }, { - desc: "ipv4 CIDR 2", - cidr: "192.168.0.0/16", - expectResult: true, + desc: "IPv4 CIDR 2", + cidr: "192.168.0.0/16", + family: IPv4, }, { - desc: "ipv6 CIDR 1", - cidr: "::/1", - expectResult: false, + desc: "IPv6 CIDR 1", + cidr: "::/1", + family: IPv6, }, { - desc: "ipv6 CIDR 2", - cidr: "2000::/10", - expectResult: false, + desc: "IPv6 CIDR 2", + cidr: "2000::/10", + family: IPv6, }, { - desc: "ipv6 CIDR 3", - cidr: "2001:db8::/32", - expectResult: false, + desc: "IPv6 CIDR 3", + cidr: "2001:db8::/32", + family: IPv6, }, { - desc: "bad CIDR", - cidr: "foo", - expectResult: false, + desc: "bad CIDR", + cidr: "foo", + family: IPFamilyUnknown, }, { - desc: "IP, not CIDR", - cidr: "192.168.0.0", - expectResult: false, + desc: "IPv4 with out-of-range octets is not accepted", + cidr: "1.2.3.400/32", + family: IPFamilyUnknown, + }, + { + desc: "IPv6 with out-of-range segment is not accepted", + cidr: "2001:db8::10005/64", + family: IPFamilyUnknown, }, - } - - for _, tc := range testCases { - res := IsIPv4CIDRString(tc.cidr) - if res != tc.expectResult { - t.Errorf("%v: want IsIPv4CIDRString=%v, got %v", tc.desc, tc.expectResult, res) - } - } -} - -func TestIsIPv4CIDR(t *testing.T) { - testCases := []struct { - desc string - cidr string - expectResult bool - }{ { - desc: "ipv4 CIDR 1", - cidr: "10.0.0.0/8", - expectResult: true, + desc: "IPv4 with out-of-range mask length is not accepted", + cidr: "1.2.3.4/64", + family: IPFamilyUnknown, }, { - desc: "ipv4 CIDR 2", - cidr: "192.168.0.0/16", - expectResult: true, + desc: "IPv6 with out-of-range mask length is not accepted", + cidr: "2001:db8::5/192", + family: IPFamilyUnknown, }, { - desc: "ipv6 CIDR 1", - cidr: "::/1", - expectResult: false, + desc: "IPv4 with empty octet is not accepted", + cidr: "1.2..4/32", + family: IPFamilyUnknown, }, { - desc: "ipv6 CIDR 2", - cidr: "2000::/10", - expectResult: false, + desc: "IPv6 with multiple empty segments is not accepted", + cidr: "2001::db8::5/64", + family: IPFamilyUnknown, }, { - desc: "ipv6 CIDR 3", - cidr: "2001:db8::/32", - expectResult: false, + desc: "IPv4 IP is not CIDR", + cidr: "192.168.0.0", + family: IPFamilyUnknown, }, { - desc: "bad CIDR", - cidr: "foo", - expectResult: false, + desc: "IPv6 IP is not CIDR", + cidr: "2001:db8::", + family: IPFamilyUnknown, }, { - desc: "IP, not CIDR", - cidr: "192.168.0.0", - expectResult: false, + desc: "IPv4 CIDR with leading whitespace is not accepted", + cidr: " 1.2.3.4/32", + family: IPFamilyUnknown, + }, + { + desc: "IPv4 CIDR with trailing whitespace is not accepted", + cidr: "1.2.3.4/32 ", + family: IPFamilyUnknown, + }, + { + desc: "IPv6 CIDR with leading whitespace is not accepted", + cidr: " 2001:db8::5/64", + family: IPFamilyUnknown, + }, + { + desc: "IPv6 CIDR with trailing whitespace is not accepted", + cidr: " 2001:db8::5/64", + family: IPFamilyUnknown, }, } for _, tc := range testCases { - _, cidr, _ := ParseCIDRSloppy(tc.cidr) - res := IsIPv4CIDR(cidr) - if res != tc.expectResult { - t.Errorf("%v: want IsIPv4CIDR=%v, got %v", tc.desc, tc.expectResult, res) - } + t.Run(tc.desc, func(t *testing.T) { + isIPv4 := IsIPv4CIDRString(tc.cidr) + isIPv6 := IsIPv6CIDRString(tc.cidr) + + if isIPv4 != (tc.family == IPv4) { + t.Errorf("Expect %q ipv4 %v, got %v", tc.cidr, tc.family == IPv4, isIPv6) + } + if isIPv6 != (tc.family == IPv6) { + t.Errorf("Expect %q ipv6 %v, got %v", tc.cidr, tc.family == IPv6, isIPv6) + } + + _, parsed, _ := ParseCIDRSloppy(tc.cidr) + isIPv4Parsed := IsIPv4CIDR(parsed) + isIPv6Parsed := IsIPv6CIDR(parsed) + if isIPv4Parsed != isIPv4 { + t.Errorf("%q gives different results for IsIPv4CIDR (%v) and IsIPv4CIDRString (%v)", tc.cidr, isIPv4Parsed, isIPv4) + } + if isIPv6Parsed != isIPv6 { + t.Errorf("%q gives different results for IsIPv6CIDR (%v) and IsIPv6CIDRString (%v)", tc.cidr, isIPv6Parsed, isIPv6) + } + }) } }