Skip to content

Commit 29f6996

Browse files
committed
Add ParseIPAsIPNet/ParseAddrAsPrefix to parse "ifaddr"-type CIDR strings
1 parent f39235b commit 29f6996

File tree

3 files changed

+116
-19
lines changed

3 files changed

+116
-19
lines changed

net/v2/ips_test.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ var badTestIPs = []testIP{
356356
// testCIDR represents a set of equivalent CIDR representations.
357357
type testCIDR struct {
358358
desc string
359+
ifaddr bool
359360
family IPFamily
360361
strings []string
361362
ipnets []*net.IPNet
@@ -379,8 +380,10 @@ type testCIDR struct {
379380
// - Each element of .prefixes should be identified as .family.
380381
//
381382
// Parsing tests (unless `skipParse: true`):
382-
// - Each element of .strings should parse to a value "equal" to .ipnets[0].
383-
// - Each element of .strings should parse to a value equal to .prefixes[0].
383+
// - Each element of .strings should parse to a value "equal" to .ipnets[0]
384+
// (via ParseIPNet if .ifaddr is false, or ParseIPAsIPNet if .ifaddr is true).
385+
// - Each element of .strings should parse to a value equal to .prefixes[0]
386+
// (via ParsePrefix if .ifaddr is false, or ParseAddrAsPrefix if .ifaddr is true).
384387
//
385388
// Conversion tests (unless `skipConvert: true`):
386389
// - Each element of .ipnets should convert to a value equal to .prefixes[0].
@@ -441,7 +444,8 @@ var goodTestCIDRs = []testCIDR{
441444
},
442445
},
443446
{
444-
desc: "IPv4 ifaddr (masked)",
447+
desc: "IPv4 ifaddr (masked)",
448+
ifaddr: false,
445449
// This tests that if you try to parse an "ifaddr-style" CIDR string with
446450
// ParseIPNet, it has the bits beyond the prefix length masked out.
447451
family: IPv4,
@@ -464,6 +468,7 @@ var goodTestCIDRs = []testCIDR{
464468
},
465469
{
466470
desc: "IPv4 ifaddr",
471+
ifaddr: true,
467472
family: IPv4,
468473
strings: []string{
469474
"1.2.3.4/24",
@@ -475,10 +480,6 @@ var goodTestCIDRs = []testCIDR{
475480
netip.PrefixFrom(netip.AddrFrom4([4]byte{1, 2, 3, 4}), 24),
476481
netip.MustParsePrefix("1.2.3.4/24"),
477482
},
478-
479-
// The *net.IPNet return value of ParseCIDRSloppy() masks out the lower
480-
// bits, so the parsed version won't compare equal to .ipnets[0]
481-
skipParse: true,
482483
},
483484
{
484485
desc: "IPv6",
@@ -532,7 +533,8 @@ var goodTestCIDRs = []testCIDR{
532533
},
533534
},
534535
{
535-
desc: "IPv6 ifaddr (masked)",
536+
desc: "IPv6 ifaddr (masked)",
537+
ifaddr: false,
536538
// This tests that if you try to parse an "ifaddr-style" CIDR string with
537539
// ParseIPNet, it value has the bits beyond the prefix length masked out.
538540
family: IPv6,
@@ -555,6 +557,7 @@ var goodTestCIDRs = []testCIDR{
555557
},
556558
{
557559
desc: "IPv6 ifaddr",
560+
ifaddr: true,
558561
family: IPv6,
559562
strings: []string{
560563
"2001:db8::1/64",
@@ -566,10 +569,6 @@ var goodTestCIDRs = []testCIDR{
566569
netip.PrefixFrom(netip.AddrFrom16([16]byte{0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}), 64),
567570
netip.MustParsePrefix("2001:db8::1/64"),
568571
},
569-
570-
// The *net.IPNet return value of ParseCIDRSloppy() masks out the lower
571-
// bits, so the parsed version won't compare equal to .ipnets[0]
572-
skipParse: true,
573572
},
574573
{
575574
desc: "IPv4-mapped IPv6",

net/v2/parse.go

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,25 +77,51 @@ func ParseAddr(ipStr string) (netip.Addr, error) {
7777
//
7878
// The return value is equivalent to the second return value from net.ParseCIDR. Note that
7979
// this means that if the CIDR string has bits set beyond the prefix length (e.g., the "5"
80-
// in "192.168.1.5/24"), those bits are simply discarded.
80+
// in "192.168.1.5/24"), those bits are simply discarded. Use ParseIPAsIPNet instead if
81+
// you want a *net.IPNet value containing the complete IP.
8182
//
8283
// Compare ParsePrefix, which returns a netip.Prefix but is otherwise identical.
8384
func ParseIPNet(cidrStr string) (*net.IPNet, error) {
85+
_, ipnet, err := parseIPNetInternal(cidrStr)
86+
return ipnet, err
87+
}
88+
89+
// ParseIPAsIPNet parses an IPv4 or IPv6 CIDR string representing an IP address and subnet
90+
// mask, to a *net.IPNet. This accepts both fully-valid CIDR values and
91+
// irregular/ambiguous forms, making it usable for both validated and non-validated input
92+
// strings. It should be used instead of net.ParseCIDR (which rejects some strings that we
93+
// need to accept for backward-compatibility) and the old netutilsv1.ParseCIDRSloppy.
94+
//
95+
// The return value combines the two return values of net.ParseCIDR; the returned
96+
// *net.IPNet's IP field will contain a net.IP corresponding to the full IP address from
97+
// the CIDR string, while the Mask field will contain a net.IPMask corresponding to the
98+
// CIDR's prefix length.
99+
//
100+
// Compare ParseAddrAsPrefix, which returns a netip.Prefix, but is otherwise identical.
101+
func ParseIPAsIPNet(cidrStr string) (*net.IPNet, error) {
102+
ip, ipnet, err := parseIPNetInternal(cidrStr)
103+
if err != nil {
104+
return nil, err
105+
}
106+
return &net.IPNet{IP: ip, Mask: ipnet.Mask}, nil
107+
}
108+
109+
func parseIPNetInternal(cidrStr string) (net.IP, *net.IPNet, error) {
84110
// Note: if we want to get rid of forkednet, we should be able to use some
85111
// invocation of regexp.ReplaceAllString to get rid of leading 0s in cidrStr.
86-
if _, ipnet, err := forkednet.ParseCIDR(cidrStr); err == nil {
87-
return ipnet, nil
112+
if ip, ipnet, err := forkednet.ParseCIDR(cidrStr); err == nil {
113+
return ip, ipnet, nil
88114
}
89115

90116
if cidrStr == "" {
91-
return nil, fmt.Errorf("expected a CIDR value")
117+
return nil, nil, fmt.Errorf("expected a CIDR value")
92118
}
93119
// NB: we use forkednet.ParseIP directly, not our own ParseIP, to avoid recursing
94120
// between ParseIPNet and ParseIP.
95121
if forkednet.ParseIP(cidrStr) != nil {
96-
return nil, fmt.Errorf("expected a CIDR value, but got IP address")
122+
return nil, nil, fmt.Errorf("expected a CIDR value, but got IP address")
97123
}
98-
return nil, fmt.Errorf("not a valid CIDR value")
124+
return nil, nil, fmt.Errorf("not a valid CIDR value")
99125
}
100126

101127
// ParsePrefix parses an IPv4 or IPv6 CIDR string representing a subnet or mask, to a
@@ -109,7 +135,8 @@ func ParseIPNet(cidrStr string) (*net.IPNet, error) {
109135
// a prefix for which `.Addr().Is4In6()` would return `true`.
110136
// - When given a CIDR string with bits set beyond the prefix length, like
111137
// `"192.168.1.5/24"`, it discards those extra bits (the equivalent of calling
112-
// .Masked() on the return value of netip.ParsePrefix).
138+
// .Masked() on the return value of netip.ParsePrefix). Use ParseAddrAsPrefix instead
139+
// if you want a netip.Prefix value containing the complete IP.
113140
//
114141
// Compare ParseIPNet, which returns a *net.IPNet but is otherwise identical.
115142
func ParsePrefix(cidrStr string) (netip.Prefix, error) {
@@ -119,3 +146,22 @@ func ParsePrefix(cidrStr string) (netip.Prefix, error) {
119146
ipnet, err := ParseIPNet(cidrStr)
120147
return PrefixFromIPNet(ipnet), err
121148
}
149+
150+
// ParseAddrAsPrefix parses an IPv4 or IPv6 CIDR string representing an IP address and
151+
// subnet mask, to a netip.Prefix. This accepts both fully-valid CIDR values and
152+
// irregular/ambiguous forms, making it usable for both validated and non-validated input
153+
// strings. As with ParsePrefix, this should be used rather than netip.ParsePrefix,
154+
// for backward-compatibility and better handling of ambiguous values.
155+
//
156+
// The return value is identical to the value returned from ParsePrefix except in the
157+
// case of CIDR strings with bits set beyond the prefix length, which are preserved by
158+
// ParseAddrAsPrefix but discarded by ParsePrefix.
159+
//
160+
// Compare ParseIPAsIPNet, which returns a *net.IPNet, but is otherwise identical.
161+
func ParseAddrAsPrefix(cidrStr string) (netip.Prefix, error) {
162+
// To ensure identical parsing, we use ParseIPAsIPNet and then convert. (If
163+
// ParseIPAsIPNet returns nil, PrefixFromIPNet will convert that to the
164+
// zero/invalid netip.Prefix, which is what we want.)
165+
ipnet, err := ParseIPAsIPNet(cidrStr)
166+
return PrefixFromIPNet(ipnet), err
167+
}

net/v2/parse_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,32 @@ func TestParseIPNet(t *testing.T) {
133133
if err != nil {
134134
t.Errorf("expected %q to parse, but got error %v", str, err)
135135
}
136+
ifaddr, err := ParseIPAsIPNet(str)
137+
if err != nil {
138+
t.Errorf("expected %q to parse via ParseIPAsIPNet, but got error %v", str, err)
139+
}
140+
141+
if tc.ifaddr {
142+
// The test case expects ParseIPNet and
143+
// ParseIPAsIPNet to parse to different values.
144+
if ipnet.String() == ifaddr.String() {
145+
t.Errorf("expected %q to parse differently with ParseIPNet and ParseIPAsIPNet but got %q for both", str, ipnet.String())
146+
}
147+
// In this case, it's the ParseIPAsIPNet value
148+
// that should re-stringify correctly. (ParseIPNet
149+
// will have discarded the trailing bits.)
150+
ipnet = ifaddr
151+
} else {
152+
// Some strings might parse to the same value and
153+
// others might parse to different values.
154+
// However, in all cases, the ParseIPAsIPNet value
155+
// should be the same as the ParseIPNet value
156+
// after masking it.
157+
if !ipnet.IP.Equal(ifaddr.IP.Mask(ifaddr.Mask)) {
158+
t.Errorf("expected %q to parse similarly with ParseIPNet and ParseIPAsIPNet but got IPs %q and %q->%q", str, ipnet.IP, ifaddr, ifaddr.IP.Mask(ifaddr.Mask))
159+
}
160+
}
161+
136162
if ipnet.String() != tc.ipnets[0].String() {
137163
t.Errorf("expected string %d %q to parse and re-stringify to %q but got %q", i+1, str, tc.ipnets[0].String(), ipnet.String())
138164
}
@@ -184,6 +210,32 @@ func TestParsePrefix(t *testing.T) {
184210
if err != nil {
185211
t.Errorf("expected %q to parse, but got error %v", str, err)
186212
}
213+
ifaddr, err := ParseAddrAsPrefix(str)
214+
if err != nil {
215+
t.Errorf("expected %q to parse via ParseAddrAsPrefix, but got error %v", str, err)
216+
}
217+
218+
if tc.ifaddr {
219+
// The test case expects ParsePrefix and
220+
// ParseAddrAsPrefix to parse to different values.
221+
if prefix == ifaddr {
222+
t.Errorf("expected %q to parse differently with ParsePrefix and ParseAddrAsPrefix but got %#v %q for both", str, prefix, prefix)
223+
}
224+
// In this case, it's the ParseAddrAsPrefix value
225+
// that should re-stringify correctly. (ParsePrefix
226+
// will have discarded the trailing bits.)
227+
prefix = ifaddr
228+
} else {
229+
// Some strings might parse to the same value and
230+
// others might parse to different values.
231+
// However, in all cases, the ParseAddrAsPrefix
232+
// value should be the same as the ParsePrefix
233+
// value after masking it.
234+
if prefix != ifaddr.Masked() {
235+
t.Errorf("expected %q to parse similarly with ParsePrefix and ParseAddrAsPrefix but got %q and %q->%q", str, prefix, ifaddr, ifaddr.Masked())
236+
}
237+
}
238+
187239
if prefix != tc.prefixes[0] {
188240
t.Errorf("expected string %d %q to parse equal to Prefix %#v %q but got %#v (%q)", i+1, str, tc.prefixes[0], tc.prefixes[0].String(), prefix, prefix.String())
189241
}

0 commit comments

Comments
 (0)