[golang-dev] code review 6842053: net: consoldate literal target address into IP address ... (issue 6842053)

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/iprawsock.go
File src/pkg/net/iprawsock.go (right):

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/iprawsock.go#newcode40
src/pkg/net/iprawsock.go:40: return nil, &ParseError{"IP address", net +
" " + addr}
Why does this need a special error? What is wrong with err?
It sounds like maybe this is working around the network being wrong? If
so maybe parseDialNetwork should be told what kind of network to expect?

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/tcpsock.go
File src/pkg/net/tcpsock.go (right):

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/tcpsock.go#newcode42
src/pkg/net/tcpsock.go:42: tcpa, ok := ia.(*TCPAddr)
Same.

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/udpsock.go
File src/pkg/net/udpsock.go (right):

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/udpsock.go#newcode40
src/pkg/net/udpsock.go:40: udpa, ok := ia.(*UDPAddr)
Same.

https://codereview.appspot.com/6842053/

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/iprawsock.go
File src/pkg/net/iprawsock.go (right):

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/iprawsock.go#newcode40
src/pkg/net/iprawsock.go:40: return nil, &ParseError{"IP address", net +
" " + addr}
yup, that makes unnecessary confusion.
will drop.

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/tcpsock.go
File src/pkg/net/tcpsock.go (right):

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/tcpsock.go#newcode42
src/pkg/net/tcpsock.go:42: tcpa, ok := ia.(*TCPAddr)
Done.

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/udpsock.go
File src/pkg/net/udpsock.go (right):

https://codereview.appspot.com/6842053/diff/6001/src/pkg/net/udpsock.go#newcode40
src/pkg/net/udpsock.go:40: udpa, ok := ia.(*UDPAddr)
Done.

https://codereview.appspot.com/6842053/

Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.


http://codereview.appspot.com/6842053/

https://codereview.appspot.com/6842053/diff/6002/src/pkg/net/iprawsock.go
File src/pkg/net/iprawsock.go (right):

https://codereview.appspot.com/6842053/diff/6002/src/pkg/net/iprawsock.go#newcode38
src/pkg/net/iprawsock.go:38: return a.(*IPAddr), nil
I'm confused. Now if I say ResolveIPAddr("tcp", "1.2.3.4") doesn't this
panic? I still want an error. Can we pass resolveInternetAddr something
like "ip" to tell it what prefix the network must have. Or pass it to
parseDialNetwork?

https://codereview.appspot.com/6842053/

Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.


http://codereview.appspot.com/6842053/

https://codereview.appspot.com/6842053/diff/6002/src/pkg/net/iprawsock.go
File src/pkg/net/iprawsock.go (right):

https://codereview.appspot.com/6842053/diff/6002/src/pkg/net/iprawsock.go#newcode38
src/pkg/net/iprawsock.go:38: return a.(*IPAddr), nil
this panic?

No, it doesn't. It would be captured by splitHostPort.
Hm, will add net string check to exposed ResolveProtocolAddr APIs.

https://codereview.appspot.com/6842053/

LGTM

Thanks. Now it all makes sense to me. :-)



https://codereview.appspot.com/6842053/diff/6004/src/pkg/net/ipraw_test.go
File src/pkg/net/ipraw_test.go (right):

https://codereview.appspot.com/6842053/diff/6004/src/pkg/net/ipraw_test.go#newcode33
src/pkg/net/ipraw_test.go:33: {"l2tp:gre", "127.0.0.1", nil,
UnknownNetworkError("l2tp:gre")},
Please add

{"tcp", "1.2.3.4:123", nil, UnknownNetworkError("tcp")},

https://codereview.appspot.com/6842053/

*** Submitted as
http://code.google.com/p/go/source/detail?r=589646797508 ***

net: consoldate literal target address into IP address functions

This CL continues with introducing IPv6 scoped addressing capability
into the net package.

Update issue 4234.

R=rsc
CC=golang-dev
http://codereview.appspot.com/6842053


http://codereview.appspot.com/6842053/

This CL also makes net.SplitHostPort now take a superset of valid input
than it used to?

If your superset means a literal IPv6 address w/ scoped addressing
zone, not yet.
An upcoming CL will support it.

I was concerned about code like:

if _, _, err := net.SplitHostPort(inputFromUser); err != nil {
fail
}
// use inputFromUser, trusting its format.

... and Go 1.1 then accepting a larger class of input with % and such past
such a check.

I don't know if that's a valid fear or not.

I guess such codes come from web stuff. As discussed, failed to make
a consensus in IETF, and is described in RFC 3986, generic syntax of
URI does not support IPv6 scope zone identifiers for host part in URI.

Seems a bit tough to bridge btw "host" of net package and host in "URI"
of net/http, net/uri packages. Hm, perhaps it might better to leave
net.SplitHostPort for now. Let me sleep on it. Thanks.

I agree with Brad that we shouldn't change the behavior of
SplitHostPort in Go 1.1
(and with Mikio that it hasn't changed yet).
I sent out https://codereview.appspot.com/6854119 to keep us honest.

;)