Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

De-emphasize IPv4-mapped IPv6 handling, relegate it to stdlib compat. #108

Closed
wants to merge 1 commit into from
+54 −61
Closed

De-emphasize IPv4-mapped IPv6 handling, relegate it to stdlib compat. #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions inlining_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ func TestInlining(t *testing.T) {
"uint128.and",
"uint128.or",
"uint128.xor",
"uint128.not",
"appendDecimal",
"appendHex",
} {
Expand Down
66 changes: 31 additions & 35 deletions netaddr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ var (

// IPv6LinkLocalAllNodes returns the IPv6 link-local all nodes multicast
// address ff02::1.
func IPv6LinkLocalAllNodes() IP { return IPv6Raw([16]byte{0: 0xff, 1: 0x02, 15: 0x01}) }
func IPv6LinkLocalAllNodes() IP { return IPFrom16([16]byte{0: 0xff, 1: 0x02, 15: 0x01}) }

// IPv6Unspecified returns the IPv6 unspecified address ::.
func IPv6Unspecified() IP { return IP{z: z6noz} }
Expand All @@ -90,10 +90,8 @@ func IPv4(a, b, c, d uint8) IP {
}
}

// IPv6Raw returns the IPv6 address given by the bytes in addr,
// without an implicit Unmap call to unmap any v6-mapped IPv4
// address.
func IPv6Raw(addr [16]byte) IP {
// IPFrom16 returns the IPv6 address given by the bytes in addr.
func IPFrom16(addr [16]byte) IP {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates the "new behavior, new name" adage. It's better to break existing callers than silently break them with a change in behavior.

If this will only return IPv6 now, maybe a better name is just IPv6 to match https://godoc.org/inet.af/netaddr#IPv4.

return IP{
addr: uint128{
binary.BigEndian.Uint64(addr[:8]),
Expand All @@ -103,7 +101,7 @@ func IPv6Raw(addr [16]byte) IP {
}
}

// ipv6Slice is like IPv6Raw, but operates on a 16-byte slice. Assumes
// ipv6Slice is like IPFrom16, but operates on a 16-byte slice. Assumes
// slice is 16 bytes, caller must enforce this.
func ipv6Slice(addr []byte) IP {
return IP{
Expand All @@ -115,14 +113,6 @@ func ipv6Slice(addr []byte) IP {
}
}

// IPFrom16 returns the IP address given by the bytes in addr,
// unmapping any v6-mapped IPv4 address.
//
// It is equivalent to calling IPv6Raw(addr).Unmap().
func IPFrom16(addr [16]byte) IP {
return IPv6Raw(addr).Unmap()
}

// ParseIP parses s as an IP address, returning the result. The string
// s can be in dotted decimal ("192.0.2.1"), IPv6 ("2001:db8::68"),
// or IPv6 with a scoped addressing zone ("fe80::1cc0:3e8c:119f:c2e1%ens18").
Expand Down Expand Up @@ -337,7 +327,7 @@ func parseIPv6(in string) (IP, error) {
// Ellipsis must represent at least one 0 group.
return IP{}, parseIPError{in: in, msg: "the :: must expand to at least one field of zeros"}
}
return IPv6Raw(ip).WithZone(zone), nil
return IPFrom16(ip).WithZone(zone), nil
}

// FromStdIP returns an IP from the standard library's IP type.
Expand All @@ -352,9 +342,7 @@ func parseIPv6(in string) (IP, error) {
// FromStdIPRaw.
func FromStdIP(std net.IP) (ip IP, ok bool) {
ret, ok := FromStdIPRaw(std)
if ret.Is4in6() {
ret.z = z4
}
ret = ret.Unmap()
return ret, ok
}

Expand Down Expand Up @@ -485,7 +473,7 @@ func (ip IP) IPAddr() *net.IPAddr {

// Is4 reports whether ip is an IPv4 address.
//
// It returns false for IP4-mapped IPv6 addresses. See IP.Unmap.
// It returns false for IPv4-mapped IPv6 addresses. See IP.Unmap.
func (ip IP) Is4() bool {
return ip.z == z4
}
Expand Down Expand Up @@ -513,6 +501,17 @@ func (ip IP) Unmap() IP {
return ip
}

// Map returns ip mapped into IPv6 space.
//
// That is, if ip is an IPv4 address, it returns an IPv4-mapped IPv6
// address. Otherwise it returns ip, regardless of its type.
func (ip IP) Map() IP {
if ip.Is4() {
ip.z = z6noz
}
return ip
}

// WithZone returns an IP that's the same as ip but with the provided
// zone. If zone is empty, the zone is removed. If ip is an IPv4
// address it's returned unchanged.
Expand Down Expand Up @@ -1160,22 +1159,14 @@ func (p IPPrefix) lastIP() IP {
if !p.Valid() {
return IP{}
}
a16 := p.IP.As16()
var off uint8
var bits uint8 = 128
if p.IP.Is4() {
off = 12
bits = 32
}
for b := p.Bits; b < bits; b++ {
byteNum, bitInByte := b/8, 7-(b%8)
a16[off+byteNum] |= 1 << uint(bitInByte)
}
if p.IP.Is4() {
return IPFrom16(a16)
m := ^mask4(p.Bits)
p.IP.addr.lo |= uint64(m)
} else {
return IPv6Raw(a16) // doesn't unmap
m := mask6(p.Bits).not()
p.IP.addr = p.IP.addr.or(m)
}
return p.IP
}

// IPRange represents an inclusive range of IP addresses
Expand Down Expand Up @@ -1289,6 +1280,10 @@ func (u uint128) or(m uint128) uint128 {
return uint128{u.hi | m.hi, u.lo | m.lo}
}

func (u uint128) not() uint128 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other three above have docs, even if internal

return uint128{^u.hi, ^u.lo}
}

func u64CommonPrefixLen(a, b uint64) uint8 {
for i := uint8(0); i < 64; i++ {
if a == b {
Expand Down Expand Up @@ -1434,10 +1429,11 @@ func subOne(a []byte, i int) bool {
// ipFrom16Match returns an IP address from a with address family
// matching ip.
func ipFrom16Match(ip IP, a [16]byte) IP {
if ip.Is6() {
return IPv6Raw(a) // doesn't unwrap
ret := IPFrom16(a)
if ip.Is4() {
ret = ret.Unmap()
}
return IPFrom16(a)
return ret
}

func (ip IP) withInternedZone(z *intern.Value) IP {
Expand Down
46 changes: 21 additions & 25 deletions netaddr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,12 @@ func TestFromStdIP(t *testing.T) {
name: "v6",
fn: FromStdIP,
std: net.ParseIP("::1"),
want: IPv6Raw([...]byte{15: 1}),
want: IPFrom16([...]byte{15: 1}),
},
{
name: "4in6-unmap",
fn: FromStdIP,
std: net.ParseIP("1.2.3.4"),
std: net.ParseIP("1.2.3.4").To16(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change shouldn't be doing anything, right? I'd prefer you keep the old line alone and add a second test, even if it's currently redundant. It'd be interesting if it one of the cases failed in a future Go release.

want: IPv4(1, 2, 3, 4),
},
{
Expand All @@ -355,7 +355,7 @@ func TestFromStdIP(t *testing.T) {
name: "4in6-raw",
fn: FromStdIPRaw,
std: net.ParseIP("1.2.3.4"),
want: IPv6Raw([...]byte{10: 0xff, 11: 0xff, 12: 1, 13: 2, 14: 3, 15: 4}),
want: IPFrom16([...]byte{10: 0xff, 11: 0xff, 12: 1, 13: 2, 14: 3, 15: 4}),
},
{
name: "bad-raw",
Expand All @@ -373,41 +373,27 @@ func TestFromStdIP(t *testing.T) {
}
}

func TestIPFrom16AndIPv6Raw(t *testing.T) {
func TestIPFrom16(t *testing.T) {
tests := []struct {
name string
fn func([16]byte) IP
in [16]byte
want IP
}{
{
name: "v6-raw",
fn: IPv6Raw,
in: [...]byte{15: 1},
want: IP{z: z6noz, addr: uint128{0, 1}},
},
{
name: "v6-from16",
fn: IPFrom16,
name: "v6",
in: [...]byte{15: 1},
want: IP{z: z6noz, addr: uint128{0, 1}},
},
{
name: "v4-raw",
fn: IPv6Raw,
name: "v4",
in: [...]byte{10: 0xff, 11: 0xff, 12: 1, 13: 2, 14: 3, 15: 4},
want: IP{z: z6noz, addr: uint128{0, 0xffff01020304}},
},
{
name: "v4-from16",
fn: IPFrom16,
in: [...]byte{10: 0xff, 11: 0xff, 12: 1, 13: 2, 14: 3, 15: 4},
want: IPv4(1, 2, 3, 4),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := tt.fn(tt.in)
got := IPFrom16(tt.in)
if got != tt.want {
t.Errorf("got %#v; want %#v", got, tt.want)
}
Expand Down Expand Up @@ -908,6 +894,17 @@ func TestIs4In6(t *testing.T) {
if u != tt.wantUnmap {
t.Errorf("Unmap(%q) = %v; want %v", tt.ip, u, tt.wantUnmap)
}

// If Unmap did something, Map should do the inverse.
if tt.ip.Is4in6() {
m := u.Map()
// Unmapping strips v6 zone, so don't expect it back when we
// re-map.
want := tt.ip.WithZone("")
if m != want {
t.Errorf("Map(Unmap(%q)) = %v; want %v", tt.ip, m, want)
}
}
}
}

Expand Down Expand Up @@ -1623,7 +1620,7 @@ func TestAs4(t *testing.T) {
want: [4]byte{1, 2, 3, 4},
},
{
ip: IPv6Raw(mustIP("1.2.3.4").As16()), // IPv4-in-IPv6
ip: mustIP("::ffff:1.2.3.4"), // IPv4-in-IPv6
want: [4]byte{1, 2, 3, 4},
},
{
Expand Down Expand Up @@ -1710,7 +1707,7 @@ func TestIPPrefixOverlaps(t *testing.T) {
{pfx("0100::0/8"), pfx("::1/128"), false},

// v6-mapped v4 should not overlap with IPv4.
{IPPrefix{IP: IPv6Raw(mustIP("1.2.0.0").As16()), Bits: 16}, pfx("1.2.3.0/24"), false},
{IPPrefix{IP: mustIP("::ffff:1.2.0.0"), Bits: 16}, pfx("1.2.3.0/24"), false},

// Invalid prefixes
{IPPrefix{IP: mustIP("1.2.3.4"), Bits: 33}, pfx("1.2.3.0/24"), false},
Expand Down Expand Up @@ -2753,7 +2750,7 @@ func TestIPv6Accessor(t *testing.T) {
for i := range a {
a[i] = uint8(i) + 1
}
ip := IPv6Raw(a)
ip := IPFrom16(a)
for i := range a {
if got, want := ip.v6(uint8(i)), uint8(i)+1; got != want {
t.Errorf("v6(%v) = %v; want %v", i, got, want)
Expand Down Expand Up @@ -2839,7 +2836,6 @@ func TestNoAllocs(t *testing.T) {

// IP constructors
test("IPv4", func() { sinkIP = IPv4(1, 2, 3, 4) })
test("IPv6", func() { sinkIP = IPv6Raw([16]byte{}) })
test("IPFrom16", func() { sinkIP = IPFrom16([16]byte{15: 1}) })
test("ParseIP/4", func() { sinkIP = panicIP(ParseIP("1.2.3.4")) })
test("ParseIP/6", func() { sinkIP = panicIP(ParseIP("::1")) })
Expand Down
2 changes: 1 addition & 1 deletion slow_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func parseIPSlow(s string) (IP, error) {
ret[i*2+1] = b
}

return IPv6Raw(ret).WithZone(zone), nil
return IPFrom16(ret).WithZone(zone), nil
}

// normalizeIPv6Slow expands s, which is assumed to be an IPv6
Expand Down