Skip to content

net: ParseIP returns nil instead of IP to a seemingly valid IPv6 #69752

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

Closed
ucirello opened this issue Oct 2, 2024 · 8 comments
Closed

net: ParseIP returns nil instead of IP to a seemingly valid IPv6 #69752

ucirello opened this issue Oct 2, 2024 · 8 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@ucirello
Copy link
Contributor

ucirello commented Oct 2, 2024

Go version

go1.23.2

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/USER/Library/Caches/go-build'
GOENV='/Users/USER/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/USER/go/pkg/mod/'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/USER/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/USER/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/USER/go/src/github.com/axiomhq/axiom/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/rb/xq45klk13kg60s3mqqr0ff7c0000gn/T/go-build1766105564=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I parsed an IP using this code:

package main

import (
	"fmt"
	"net"
)

func main() {
	fmt.Println(net.ParseIP("0:0:0:0:000000:ffff:127.5.8.7").To4())
	fmt.Printf("%#v", net.ParseIP("0:0:0:0:000000:ffff:127.5.8.7"))
}

What did you see happen?

https://go.dev/play/p/bJUjbkLm_bg (Go 1.23)

<nil>
net.IP(nil)

What did you expect to see?

https://go.dev/play/p/bJUjbkLm_bg?v=goprev (Go 1.22)

127.5.8.7
net.IP{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0x7f, 0x5, 0x8, 0x7}
@seankhliao
Copy link
Member

That's not valid, there's too many digits in 000000

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
@ucirello
Copy link
Contributor Author

ucirello commented Oct 2, 2024

According to https://datatracker.ietf.org/doc/html/rfc4291#section-2.2

  1. The preferred form is x:x:x:x:x:x:x:x, where the 'x's are one to
    four hexadecimal digits of the eight 16-bit pieces of the address.

Meaning that the example at hand is invalid.

I don't thinkg this behavior change is covered by Go 1 Compatibility Promise, correct?

@mknyszek
Copy link
Contributor

mknyszek commented Oct 2, 2024

https://go.dev/cl/461605 is the change that added this behavior, which references #57760.

So, this change does seem deliberate. I'm not sure what the right path forward is here.

CC @neild @ianlancetaylor who reviewed the CL.

@mknyszek mknyszek added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 2, 2024
@mknyszek mknyszek added this to the Backlog milestone Oct 2, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Oct 2, 2024

Reopening for now to await input from Damien and/or Ian.

@mknyszek mknyszek reopened this Oct 2, 2024
@ianlancetaylor
Copy link
Member

CC @mpetrunic

@neild
Copy link
Contributor

neild commented Oct 2, 2024

Unless I'm missing something, 0:0:0:0:000000:ffff:127.5.8.7 is not a valid IPv6 address. As @seankhliao says, there are too many zeros.

Go 1.22 and prior had a bug which caused us to parse this address anyway. (#57760)

The bug was fixed in Go 1.23.

This isn't a violation of the compatibility promise, since we reserve the right to fix bugs. net.ParseIP's documentation says that it returns nil if given an invalid input, so this is definitely a bug. (Unless we're all very confused and this address is actually valid.)

I don't think there's anything for us to do here.

@ucirello
Copy link
Contributor Author

ucirello commented Oct 3, 2024

Thank you @neild and @seankhliao !

I apologize for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants