Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/netip: ParseAddr gives different values with same input #50111

Closed
capnspacehook opened this issue Dec 11, 2021 · 6 comments · Fixed by ferrmin/go#241
Closed

net/netip: ParseAddr gives different values with same input #50111

capnspacehook opened this issue Dec 11, 2021 · 6 comments · Fixed by ferrmin/go#241

Comments

@capnspacehook
Copy link

@capnspacehook capnspacehook commented Dec 11, 2021

What version of Go are you using (go version)?

$ go version
go version devel go1.18-766f89b5c6 Fri Dec 10 19:26:50 2021 +0000 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/capnspacehook/.cache/go-build"
GOENV="/home/capnspacehook/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/capnspacehook/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/capnspacehook/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/capnspacehook/Documents/git/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/capnspacehook/Documents/git/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.18-766f89b5c6 Fri Dec 10 19:26:50 2021 +0000"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/capnspacehook/Documents/git/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3691310336=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tested that netip.ParseAddr would produce the same value twice when given the same input:

https://go.dev/play/p/ZpqXiudtmA_P?v=gotip

What did you expect to see?

true
netip.Addr{addr:netip.uint128{hi:0x0, lo:0xffff00000000}, z:(*intern.Value)(0xc0000b4048)}, netip.Addr{addr:netip.uint128{hi:0x0, lo:0xffff00000000}, z:(*intern.Value)(0xc0000b4048)}

What did you see instead?

false
netip.Addr{addr:netip.uint128{hi:0x0, lo:0xffff00000000}, z:(*intern.Value)(0xc0000b4048)}, netip.Addr{addr:netip.uint128{hi:0x0, lo:0xffff00000000}, z:(*intern.Value)(0xc0000b4030)}

I found this bug and a few others while working on #49367.

@capnspacehook capnspacehook changed the title affected/package: net/netip: ParseAddr gives different values with same input Dec 11, 2021
@bradfitz bradfitz self-assigned this Dec 12, 2021
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 12, 2021

Expanded example a bit: https://go.dev/play/p/3uzuOmxzN1c?v=gotip

/cc @josharian @danderson @mdlayher

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 12, 2021

And https://go.dev/play/p/owhCTtsY5Bn?v=gotip ... it stringifies without the zone, despite the Zone method returning "0".

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 12, 2021

Ah! The 4-in-6 special case to stringify it with the dotted quad form omits the zone check:

func (ip Addr) String() string {
        switch ip.z {
        case z0:
                return "invalid IP"
        case z4:
                return ip.string4()
        default:
                if ip.Is4In6() {
                        // TODO(bradfitz): this could alloc less.                                                                                     
                        return "::ffff:" + ip.Unmap().String()
                }
                return ip.string6()
        }
}

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 12, 2021

Change https://golang.org/cl/371094 mentions this issue: net/netip: fix formatting of IPv4-in-6 address with zone

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 12, 2021

I assume this is okay-for-after-beta1 too, @golang/release, but will leave that to y'all to tag.

@bradfitz bradfitz added this to the Go1.18 milestone Dec 12, 2021
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 12, 2021

Thanks, @capnspacehook!

capnspacehook pushed a commit to capnspacehook/go that referenced this issue Dec 12, 2021
Weird, but don't drop the zone when stringifying.

Fixes golang#50111

Change-Id: I5fbccdfedcdc77a77ee6bafc8d82b8ec8ec7220c
Reviewed-on: https://go-review.googlesource.com/c/go/+/371094
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Trust: Matt Layher <mdlayher@gmail.com>
Trust: Ian Lance Taylor <iant@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants