code review 6842053: net: consoldate literal target address into IP address ...

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.

[mikio, 2012-11-26 16:04:36]

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

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2012-11-26 16:07:26]

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#new...
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#newco...
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#newco...
src/pkg/net/udpsock.go:40: udpa, ok := ia.(*UDPAddr)
Same.

[mikio, 2012-11-26 16:27:51]

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#new...
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#newco...
src/pkg/net/tcpsock.go:42: tcpa, ok := ia.(*TCPAddr)
On 2012/11/26 16:07:26, rsc wrote:
> Same.

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#newco...
src/pkg/net/udpsock.go:40: udpa, ok := ia.(*UDPAddr)
On 2012/11/26 16:07:26, rsc wrote:
> Same.

Done.

[mikio, 2012-11-26 16:28:15]

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

Please take another look.

[rsc, 2012-11-26 17:09:37]

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#new...
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?

[mikio, 2012-11-27 00:50:44]

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#new...
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?


No, it doesn't. It would be captured by splitHostPort.

> 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?

Hm, will add net string check to exposed ResolveProtocolAddr APIs.

[mikio, 2012-11-27 00:50:59]

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

Please take another look.

[rsc, 2012-11-27 19:50:46]

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#ne...
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")},

[mikio, 2012-11-27 21:36:14]

*** 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

[bradfitz, 2012-11-28 02:58:11]

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


On Tue, Nov 27, 2012 at 1:36 PM, <mikioh.mikioh@gmail.com> wrote:

> *** Submitted as
>
http://code.google.com/p/go/**source/detail?r=589646797508<http://code.google...
>
>
> 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>
>
>
>
http://codereview.appspot.com/**6842053/<http://codereview.appspot.com/6842053/>
>

[mikio, 2012-11-28 03:15:30]

On Wed, Nov 28, 2012 at 11:58 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> 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.

[bradfitz, 2012-11-28 03:30:10]

On Tue, Nov 27, 2012 at 7:15 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:

> On Wed, Nov 28, 2012 at 11:58 AM, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
>
> > 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.

[mikio, 2012-11-28 04:16:21]

On Wed, Nov 28, 2012 at 12:30 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> 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.

[mikio, 2012-12-10 09:43:37]

I missed this activity at 6man-wg.

http://tools.ietf.org/html/draft-ietf-6man-uri-zoneid

[rsc, 2012-12-10 14:42:41]

Interesting. Should we make it an error (everywhere) to see a % if the
address is not in fe80::/10?

Russ

[mikio, 2012-12-10 22:33:27]

On Mon, Dec 10, 2012 at 11:42 PM, Russ Cox <rsc@golang.org> wrote:

> Interesting. Should we make it an error (everywhere) to see a % if the
> address is not in fe80::/10?

I think that's application's business, not API's.

At least net API should accept both link-local unicast/multicast addresses
for signaling/routing protocols. For net/url.... not sure. If backyard/devops
guys want to write their provisioning scripts in URI? hm...