diff --git a/netutil/ip_test.go b/netutil/ip_test.go index 800e0e7..c00fc67 100644 --- a/netutil/ip_test.go +++ b/netutil/ip_test.go @@ -244,17 +244,8 @@ var ( ipNetSink *net.IPNet ) -// flushSinks should be used in Cleanup functions to get rid of values of global -// variables. -func flushSinks() { - errSink = nil - ipNetSink = nil -} - func BenchmarkParseSubnet(b *testing.B) { b.Run("good_cidr", func(b *testing.B) { - b.Cleanup(flushSinks) - b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { @@ -266,8 +257,6 @@ func BenchmarkParseSubnet(b *testing.B) { }) b.Run("good_ip", func(b *testing.B) { - b.Cleanup(flushSinks) - b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { @@ -279,8 +268,6 @@ func BenchmarkParseSubnet(b *testing.B) { }) b.Run("bad_cidr", func(b *testing.B) { - b.Cleanup(flushSinks) - b.ReportAllocs() for i := 0; i < b.N; i++ { _, errSink = netutil.ParseSubnet("1.2.3.4//567") @@ -290,8 +277,6 @@ func BenchmarkParseSubnet(b *testing.B) { }) b.Run("bad_ip", func(b *testing.B) { - b.Cleanup(flushSinks) - b.ReportAllocs() for i := 0; i < b.N; i++ { _, errSink = netutil.ParseSubnet("1.2.3.4.5") diff --git a/netutil/reversed.go b/netutil/reversed.go index c8a46df..e6cf1ed 100644 --- a/netutil/reversed.go +++ b/netutil/reversed.go @@ -197,20 +197,22 @@ func IPToReversedAddr(ip net.IP) (arpa string, err error) { } // ipv4NetFromReversed parses an IPv4 reverse network. It assumes that arpa is -// a valid domain name and not a single address IPv4 network. +// a valid domain name and is not a doman name with a full IPv4 address. func ipv4NetFromReversed(arpa string) (subnet *net.IPNet, err error) { - var b uint64 + var octet64 uint64 + var octetIdx int + ip := make(net.IP, net.IPv4len) l := 0 - for arpa != "" { - dotIdx := strings.LastIndexByte(arpa, '.') + for addr := arpa; addr != ""; addr = addr[:octetIdx-1] { + octetIdx = strings.LastIndexByte(addr, '.') + 1 // Don't check for out of range since the domain is validated to have no // empty labels. - b, err = strconv.ParseUint(arpa[dotIdx+1:], 10, 8) + octet64, err = strconv.ParseUint(addr[octetIdx:], 10, 8) if err != nil { return nil, err - } else if b != 0 && arpa[dotIdx+1] == '0' { + } else if octet64 != 0 && addr[octetIdx] == '0' { // Octets of an ARPA domain name shouldn't contain leading zero // except an octet itself equals zero. // @@ -218,21 +220,20 @@ func ipv4NetFromReversed(arpa string) (subnet *net.IPNet, err error) { return nil, &AddrError{ Err: errors.Error("leading zero is forbidden at this position"), Kind: AddrKindLabel, - Addr: arpa[dotIdx+1:], + Addr: addr[octetIdx:], } } - ip[l] = byte(b) + ip[l] = byte(octet64) l++ - if dotIdx == -1 { + if octetIdx == 0 { + // Prevent slicing with negative indices. break } - - arpa = arpa[:dotIdx] } - if b == 0 { + if octet64 == 0 { return nil, ErrNotAReversedSubnet } @@ -243,22 +244,24 @@ func ipv4NetFromReversed(arpa string) (subnet *net.IPNet, err error) { } // ipv6NetFromReversed parses an IPv6 reverse network. It assumes that arpa is -// a valid domain name and not a single address IPv6 network. +// a valid domain name and is not a doman name with a full IPv6 address. func ipv6NetFromReversed(arpa string) (subnet *net.IPNet, err error) { - nibIdx := len(arpa) - len(arpaV6Suffix) + len(".") - len("0.") - if nibIdx%2 != 0 { + const nibbleLen = len("0.") + + nibbleIdx := len(arpa) - len(arpaV6Suffix) + len(".") - nibbleLen + if nibbleIdx%2 != 0 { return nil, ErrNotAReversedSubnet } + var b byte ip := make(net.IP, net.IPv6len) l := 0 - var b byte - for ; nibIdx >= 0; nibIdx -= len("0.") { - if arpa[nibIdx+1] != '.' { + for ; nibbleIdx >= 0; nibbleIdx -= nibbleLen { + if arpa[nibbleIdx+1] != '.' { return nil, ErrNotAReversedSubnet } - c := arpa[nibIdx] + c := arpa[nibbleIdx] b = fromHexByte(c) if b == 0xff { return nil, &RuneError{ @@ -277,7 +280,7 @@ func ipv6NetFromReversed(arpa string) (subnet *net.IPNet, err error) { l++ } - if ip[l/2] == 0 { + if b == 0 { return nil, ErrNotAReversedSubnet } @@ -292,7 +295,9 @@ func ipv6NetFromReversed(arpa string) (subnet *net.IPNet, err error) { func subnetFromReversedV4(arpa string) (subnet *net.IPNet, err error) { arpa = arpa[:len(arpa)-len(arpaV4Suffix)] - if dots := strings.Count(arpa, "."); dots == 3 { + if dots := strings.Count(arpa, "."); dots > 3 { + return nil, ErrNotAReversedSubnet + } else if dots == 3 { var ip net.IP ip, err = ParseIPv4(arpa) if err != nil { @@ -302,8 +307,6 @@ func subnetFromReversedV4(arpa string) (subnet *net.IPNet, err error) { reverseIP(ip) return SingleIPSubnet(ip), nil - } else if dots > 3 { - return nil, ErrNotAReversedSubnet } return ipv4NetFromReversed(arpa) diff --git a/netutil/reversed_example_test.go b/netutil/reversed_example_test.go index 6a57586..8c2b039 100644 --- a/netutil/reversed_example_test.go +++ b/netutil/reversed_example_test.go @@ -60,3 +60,41 @@ func ExampleIPToReversedAddr_ipv6() { // // 4.3.2.1.d.c.b.a.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa } + +func ExampleSubnetFromReversedAddr() { + subnet, err := netutil.SubnetFromReversedAddr("10.in-addr.arpa") + if err != nil { + panic(err) + } + + fmt.Println(subnet) + + // Output: + // + // 10.0.0.0/8 +} + +func ExampleSubnetFromReversedAddr_ipv6() { + a := `3.2.1.d.c.b.a.ip6.arpa` + subnet, err := netutil.SubnetFromReversedAddr(a) + if err != nil { + panic(err) + } + + fmt.Println(subnet) + + // Output: + // + // abcd:1230::/28 +} + +func ExampleSubnetFromReversedAddr_domain() { + a := `in-addr.arpa` + _, err := netutil.SubnetFromReversedAddr(a) + + fmt.Println(err) + + // Output: + // + // bad arpa domain name "in-addr.arpa": not a reversed ip network +} diff --git a/netutil/reversed_test.go b/netutil/reversed_test.go index e3a6b2a..59c1abb 100644 --- a/netutil/reversed_test.go +++ b/netutil/reversed_test.go @@ -14,14 +14,15 @@ import ( ) const ( - ipv4RevGood = `4.3.2.1.in-addr.arpa` + ipv4Suffix = `.in-addr.arpa` + ipv4RevGood = `4.3.2.1` + ipv4Suffix ipv4RevGoodUp = `4.3.2.1.In-Addr.Arpa` - ipv4NetRevGood = `10.in-addr.arpa` + ipv4NetRevGood = `10` + ipv4Suffix - ipv4RevGoodUnspecified = `0.0.0.0.in-addr.arpa` + ipv4RevGoodUnspecified = `0.0.0.0` + ipv4Suffix - ipv4Missing = `.0.0.127.in-addr.arpa` - ipv4Char = `1.0.z.127.in-addr.arpa` + ipv4Missing = `.0.0.127` + ipv4Suffix + ipv4Char = `1.0.z.127` + ipv4Suffix ) const ( @@ -255,6 +256,15 @@ func TestIPToReversedAddr(t *testing.T) { } } +// newIPNet returns an IP network to use in test cases. It doesn't validate +// anything. +func newIPNet(ip net.IP, ones int) (n *net.IPNet) { + return &net.IPNet{ + IP: ip, + Mask: net.CIDRMask(ones, len(ip)*8), + } +} + func TestSubnetFromReversedAddr(t *testing.T) { t.Parallel() @@ -265,47 +275,32 @@ func TestSubnetFromReversedAddr(t *testing.T) { in string name string }{{ - wantErrAs: nil, - want: &net.IPNet{ - IP: testIPv4, - Mask: net.CIDRMask(netutil.IPv4BitLen, netutil.IPv4BitLen), - }, + want: newIPNet(testIPv4, netutil.IPv4BitLen), + wantErrAs: nil, wantErrMsg: "", in: ipv4RevGood, name: "good_ipv4_single_addr", }, { - wantErrAs: nil, - want: &net.IPNet{ - IP: testIPv4, - Mask: net.CIDRMask(netutil.IPv4BitLen, netutil.IPv4BitLen), - }, + want: newIPNet(testIPv4, netutil.IPv4BitLen), + wantErrAs: nil, wantErrMsg: "", in: ipv4RevGood + ".", name: "good_ipv4_single_addr_fqdn", }, { - wantErrAs: nil, - want: &net.IPNet{ - IP: testIPv4, - Mask: net.CIDRMask(netutil.IPv4BitLen, netutil.IPv4BitLen), - }, + want: newIPNet(testIPv4, netutil.IPv4BitLen), + wantErrAs: nil, wantErrMsg: "", in: ipv4RevGoodUp, name: "good_ipv4_single_addr_case", }, { - wantErrAs: nil, - want: &net.IPNet{ - IP: testIPv4ZeroTail, - Mask: net.CIDRMask(netutil.IPv4BitLen, netutil.IPv4BitLen), - }, + want: newIPNet(testIPv4ZeroTail, netutil.IPv4BitLen), + wantErrAs: nil, wantErrMsg: "", in: `0.0.0.` + ipv4NetRevGood, name: "good_ipv4_single_addr_leading_zero", }, { - wantErrAs: nil, - want: &net.IPNet{ - IP: testIPv4ZeroTail, - Mask: net.CIDRMask(8, netutil.IPv4BitLen), - }, + want: newIPNet(testIPv4ZeroTail, 8), + wantErrAs: nil, wantErrMsg: "", in: ipv4NetRevGood, name: "good_ipv4_subnet", @@ -352,37 +347,25 @@ func TestSubnetFromReversedAddr(t *testing.T) { in: `5.` + ipv4RevGood, name: "bad_ipv4_too_long", }, { - wantErrAs: nil, - want: &net.IPNet{ - IP: testIPv6, - Mask: net.CIDRMask(netutil.IPv6BitLen, netutil.IPv6BitLen), - }, + want: newIPNet(testIPv6, netutil.IPv6BitLen), + wantErrAs: nil, wantErrMsg: "", in: ipv6RevGood, name: "good_ipv6_single_addr", }, { - want: &net.IPNet{ - IP: testIPv6, - Mask: net.CIDRMask(netutil.IPv6BitLen, netutil.IPv6BitLen), - }, + want: newIPNet(testIPv6, netutil.IPv6BitLen), wantErrAs: nil, wantErrMsg: "", in: ipv6RevGood + ".", name: "good_ipv6_single_addr_fqdn", }, { - want: &net.IPNet{ - IP: testIPv6, - Mask: net.CIDRMask(netutil.IPv6BitLen, netutil.IPv6BitLen), - }, + want: newIPNet(testIPv6, netutil.IPv6BitLen), wantErrAs: nil, wantErrMsg: "", in: ipv6RevGoodUp, name: "good_ipv6_single_addr_case", }, { - want: &net.IPNet{ - IP: testIPv6ZeroTail, - Mask: net.CIDRMask(4*17, netutil.IPv6BitLen), - }, + want: newIPNet(testIPv6ZeroTail, 4*17), wantErrAs: nil, wantErrMsg: "", in: ipv6NetRevGood, @@ -471,6 +454,20 @@ func TestSubnetFromReversedAddr(t *testing.T) { `not a reversed ip network`, in: testIPv4.String(), name: "not_a_reversed_subnet", + }, { + want: nil, + wantErrAs: new(errors.Error), + wantErrMsg: `bad arpa domain name "` + ipv4Suffix[1:] + `": ` + + `not a reversed ip network`, + in: ipv4Suffix[1:], + name: "root_arpa_v4", + }, { + want: nil, + wantErrAs: new(errors.Error), + wantErrMsg: `bad arpa domain name "` + ipv6Suffix[1:] + `": ` + + `not a reversed ip network`, + in: ipv6Suffix[1:], + name: "root_arpa_v6", }} for _, tc := range testCases { @@ -516,8 +513,6 @@ func BenchmarkSubnetFromReversedAddr(b *testing.B) { for _, bc := range benchCases { b.Run(bc.name, func(b *testing.B) { - b.Cleanup(flushSinks) - b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ {