code review 6431053: net: fix ResolveIPAddr for IPv6 address enclosed in squ... (issue 6431053)

[mikioh...@gmail.com]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com (cc: golan...@googlegroups.com),

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


Description:
net: fix ResolveIPAddr for IPv6 address enclosed in square brackets

Fixes issue 3837.

Please review this at http://codereview.appspot.com/6431053/

Affected files:
M src/pkg/net/ipraw_test.go
M src/pkg/net/iprawsock.go


Index: src/pkg/net/ipraw_test.go
===================================================================
--- a/src/pkg/net/ipraw_test.go
+++ b/src/pkg/net/ipraw_test.go
@@ -14,6 +14,27 @@
"time"
)

+var resolveIPAddrTests = []struct {
+ net string
+ addr string
+}{
+ {"ip", "0.0.0.0"},
+ {"ip", "[::]"},
+ {"ip", "::"},
+ {"ip4", "0.0.0.0"},
+ {"ip6", "[::]"},
+ {"ip6", "::"},
+}
+
+func TestResolveIPAddr(t *testing.T) {
+ for _, tt := range resolveIPAddrTests {
+ _, err := ResolveIPAddr(tt.net, tt.addr)
+ if err != nil {
+ t.Fatalf("ResolveIPAddr(%q, %q) failed: %v", tt.net, tt.addr, err)
+ }
+ }
+}
+
var icmpTests = []struct {
net string
laddr string
Index: src/pkg/net/iprawsock.go
===================================================================
--- a/src/pkg/net/iprawsock.go
+++ b/src/pkg/net/iprawsock.go
@@ -46,6 +46,10 @@
if net != "" && net[len(net)-1] == '6' {
filter = ipv6only
}
+ // Can put brackets around host ...
+ if len(host) > 0 && host[0] == '[' && host[len(host)-1] == ']' {
+ host = host[1 : len(host)-1]
+ }
// Not an IP address. Try as a DNS name.
addrs, err1 := LookupHost(host)
if err1 != nil {

[Andrew Gerrand]

Should this be in parseIPv6 instead?

Is [hostname] valid? Shouldn't ParseIP("[::]") work? Not clear to me that it does. 

[Mikio Hara]

Thank you for the comment. I realized that I put bracket remove lines
in wrong place, that should stay just before ParseIP in hostToIP.

On Sun, Jul 22, 2012 at 9:58 AM, Andrew Gerrand <a...@google.com> wrote:

> Should this be in parseIPv6 instead?

I guess, code itself says IP and IPAddr are different things. IP represents
a plain IP address and IPAddr represents something like IP address literal
in raw IP socket address, something like:

IP = IPv6Address
IPAddr = IPLiteralInRawIPSock ":" Port
IPLiteralInRawIPSock = "[" IPv6address / Name "]"

This looks similar to the relationship btw IPv6 address and literal IPv6
address in URL.

> Is [hostname] valid? Shouldn't ParseIP("[::]") work? Not clear to me that it
> does.

-- Mikio

[mikioh...@gmail.com]

Hello golan...@googlegroups.com, a...@google.com (cc:
golan...@googlegroups.com),

Please take another look.


http://codereview.appspot.com/6431053/

[a...@golang.org]

This looks OK to me, but I'd like someone else familiar with net to take
a look also.

http://codereview.appspot.com/6431053/

[r...@golang.org]

R=rsc

(sent by gocodereview)

[r...@golang.org]

NOT LGTM

[::] is no more an IP address than 127.0.0.1:. The syntax for adding a
port to an IPv6 address is [addr]:port, but the [] are not part of the
addr.

In addition to being consistent with the RFCs, this appears to be
consistent with other Unix programs. For example, on my Mac:

$ telnet '[::1]'
[::1]: nodename nor servname provided, or not known
$ telnet ::1
Trying ::1...
telnet: connect to address ::1: Connection refused
telnet: Unable to connect to remote host
$



http://codereview.appspot.com/6431053/

[Mikio Hara]

On Mon, Jul 30, 2012 at 9:05 AM, <r...@golang.org> wrote:

> In addition to being consistent with the RFCs, this appears to be
> consistent with other Unix programs. For example, on my Mac:
>
> $ telnet '[::1]'
> [::1]: nodename nor servname provided, or not known
> $ telnet ::1
> Trying ::1...
> telnet: connect to address ::1: Connection refused
> telnet: Unable to connect to remote host
> $

Makes sense, will abandon.

[mikioh...@gmail.com]

*** Abandoned ***

http://codereview.appspot.com/6431053/