From ae9ce822ff4015fbbe7aa4303e6f3c160f2c53af Mon Sep 17 00:00:00 2001 From: Matt Layher Date: Tue, 29 Mar 2022 10:42:58 -0400 Subject: [PATCH] net/netip: return an error from ParsePrefix with IPv6 zone input net.ParseCIDR already rejects input in the form of 2001:db8::%a/32, but netip.ParsePrefix previously accepted the input and silently dropped the zone. Make the two consistent by always returning an error if an IPv6 zone is present in CIDR input for ParsePrefix. Fixes #51899. Change-Id: Iee7d8d4a5161e0b54a4ee1bd68b02c1a287ff399 Reviewed-on: https://go-review.googlesource.com/c/go/+/396299 Trust: Matt Layher Run-TryBot: Matt Layher TryBot-Result: Gopher Robot Reviewed-by: Damien Neil Trust: Damien Neil --- src/net/netip/netip.go | 7 +++++ src/net/netip/netip_pkg_test.go | 6 ++-- src/net/netip/netip_test.go | 49 ++++++++++----------------------- 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/src/net/netip/netip.go b/src/net/netip/netip.go index 1cf75fb5a1..893eeea7f0 100644 --- a/src/net/netip/netip.go +++ b/src/net/netip/netip.go @@ -1290,6 +1290,8 @@ func (p Prefix) IsSingleIP() bool { return p.bits != 0 && int(p.bits) == p.ip.Bi // ParsePrefix parses s as an IP address prefix. // The string can be in the form "192.168.1.0/24" or "2001:db8::/32", // the CIDR notation defined in RFC 4632 and RFC 4291. +// IPv6 zones are not permitted in prefixes, and an error will be returned if a +// zone is present. // // Note that masked address bits are not zeroed. Use Masked for that. func ParsePrefix(s string) (Prefix, error) { @@ -1301,6 +1303,11 @@ func ParsePrefix(s string) (Prefix, error) { if err != nil { return Prefix{}, errors.New("netip.ParsePrefix(" + strconv.Quote(s) + "): " + err.Error()) } + // IPv6 zones are not allowed: https://go.dev/issue/51899 + if ip.Is6() && ip.z != z6noz { + return Prefix{}, errors.New("netip.ParsePrefix(" + strconv.Quote(s) + "): IPv6 zones cannot be present in a prefix") + } + bitsStr := s[i+1:] bits, err := strconv.Atoi(bitsStr) if err != nil { diff --git a/src/net/netip/netip_pkg_test.go b/src/net/netip/netip_pkg_test.go index f5cd9ee86d..677f523e6d 100644 --- a/src/net/netip/netip_pkg_test.go +++ b/src/net/netip/netip_pkg_test.go @@ -160,9 +160,9 @@ func TestPrefixContains(t *testing.T) { {mustPrefix("::1/127"), mustIP("::2"), false}, {mustPrefix("::1/128"), mustIP("::1"), true}, {mustPrefix("::1/127"), mustIP("::2"), false}, - // zones support - {mustPrefix("::1%a/128"), mustIP("::1"), true}, // prefix zones are stripped... - {mustPrefix("::1%a/128"), mustIP("::1%a"), false}, // but ip zones are not + // Zones ignored: https://go.dev/issue/51899 + {Prefix{mustIP("1.2.3.4").WithZone("a"), 32}, mustIP("1.2.3.4"), true}, + {Prefix{mustIP("::1").WithZone("a"), 128}, mustIP("::1"), true}, // invalid IP {mustPrefix("::1/0"), Addr{}, false}, {mustPrefix("1.2.3.4/0"), Addr{}, false}, diff --git a/src/net/netip/netip_test.go b/src/net/netip/netip_test.go index a72390fd5b..c2811c4703 100644 --- a/src/net/netip/netip_test.go +++ b/src/net/netip/netip_test.go @@ -421,8 +421,8 @@ func TestPrefixMarshalTextString(t *testing.T) { {mustPrefix("1.2.3.4/24"), "1.2.3.4/24"}, {mustPrefix("fd7a:115c:a1e0:ab12:4843:cd96:626b:430b/118"), "fd7a:115c:a1e0:ab12:4843:cd96:626b:430b/118"}, {mustPrefix("::ffff:c000:0280/96"), "::ffff:192.0.2.128/96"}, - {mustPrefix("::ffff:c000:0280%eth0/37"), "::ffff:192.0.2.128/37"}, // Zone should be stripped {mustPrefix("::ffff:192.168.140.255/8"), "::ffff:192.168.140.255/8"}, + {PrefixFrom(mustIP("::ffff:c000:0280").WithZone("eth0"), 37), "::ffff:192.0.2.128/37"}, // Zone should be stripped } for i, tt := range tests { if got := tt.in.String(); got != tt.want { @@ -448,7 +448,7 @@ func TestPrefixMarshalUnmarshalBinary(t *testing.T) { {mustPrefix("1.2.3.4/24"), 4 + 1}, {mustPrefix("fd7a:115c:a1e0:ab12:4843:cd96:626b:430b/118"), 16 + 1}, {mustPrefix("::ffff:c000:0280/96"), 16 + 1}, - {mustPrefix("::ffff:c000:0280%eth0/37"), 16 + 1}, // Zone should be stripped + {PrefixFrom(mustIP("::ffff:c000:0280").WithZone("eth0"), 37), 16 + 1}, // Zone should be stripped } tests = append(tests, testCase{PrefixFrom(tests[0].prefix.Addr(), 33), tests[0].wantSize}, @@ -901,25 +901,25 @@ func TestPrefixMasking(t *testing.T) { { ip: mustIP(fmt.Sprintf("2001:db8::1%s", zone)), bits: 32, - p: mustPrefix(fmt.Sprintf("2001:db8::%s/32", zone)), + p: mustPrefix("2001:db8::/32"), ok: true, }, { ip: mustIP(fmt.Sprintf("fe80::dead:beef:dead:beef%s", zone)), bits: 96, - p: mustPrefix(fmt.Sprintf("fe80::dead:beef:0:0%s/96", zone)), + p: mustPrefix("fe80::dead:beef:0:0/96"), ok: true, }, { ip: mustIP(fmt.Sprintf("aaaa::%s", zone)), bits: 4, - p: mustPrefix(fmt.Sprintf("a000::%s/4", zone)), + p: mustPrefix("a000::/4"), ok: true, }, { ip: mustIP(fmt.Sprintf("::%s", zone)), bits: 63, - p: mustPrefix(fmt.Sprintf("::%s/63", zone)), + p: mustPrefix("::/63"), ok: true, }, } @@ -1047,26 +1047,6 @@ func TestPrefixMarshalUnmarshal(t *testing.T) { } } -func TestPrefixMarshalUnmarshalZone(t *testing.T) { - orig := `"fe80::1cc0:3e8c:119f:c2e1%ens18/128"` - unzoned := `"fe80::1cc0:3e8c:119f:c2e1/128"` - - var p Prefix - if err := json.Unmarshal([]byte(orig), &p); err != nil { - t.Fatalf("failed to unmarshal: %v", err) - } - - pb, err := json.Marshal(p) - if err != nil { - t.Fatalf("failed to marshal: %v", err) - } - - back := string(pb) - if back != unzoned { - t.Errorf("Marshal = %q; want %q", back, unzoned) - } -} - func TestPrefixUnmarshalTextNonZero(t *testing.T) { ip := mustPrefix("fe80::/64") if err := ip.UnmarshalText([]byte("xxx")); err == nil { @@ -1222,14 +1202,6 @@ func TestPrefix(t *testing.T) { contains: mustIPs("2001:db8::1"), notContains: mustIPs("fe80::1"), }, - { - prefix: "::%0/00/80", - ip: mustIP("::"), - bits: 80, - str: "::/80", - contains: mustIPs("::"), - notContains: mustIPs("ff::%0/00", "ff::%1/23", "::%0/00", "::%1/23"), - }, } for _, test := range tests { t.Run(test.prefix, func(t *testing.T) { @@ -1348,6 +1320,15 @@ func TestParsePrefixError(t *testing.T) { prefix: "2001::/129", errstr: "out of range", }, + // Zones are not allowed: https://go.dev/issue/51899 + { + prefix: "1.1.1.0%a/24", + errstr: "unexpected character", + }, + { + prefix: "2001:db8::%a/32", + errstr: "zones cannot be present", + }, } for _, test := range tests { t.Run(test.prefix, func(t *testing.T) { -- 2.48.1