net: fix looking up port numbers starting with numbers.

LookupPort() correctly parses service names beginning with numerals by
implementing a new parser, mainly taken from strconv/atoi.go.

Also testes some previously undefined behaviours around port numbers
larger than 65535 that previously could lead to some tests fail with
EOPNOTSUPP (Operation Not Supported).

Fixes #14322

Change-Id: I1b90dbed434494723e261d84e73fe705e5c0507a
Reviewed-on: https://go-review.googlesource.com/19720
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>

diff --git a/src/net/lookup.go b/src/net/lookup.go
index 0d3ef79bab342e0ffd88779463a40be6a34f1f50..8f02787422be9063252cbf6a4962d092cc6f7f39 100644 (file)
--- a/src/net/lookup.go
+++ b/src/net/lookup.go
@@ -112,13 +112,8 @@ func lookupIPContext(ctx context.Context, host string) (addrs []IPAddr, err erro
 
 // LookupPort looks up the port for the given network and service.
 func LookupPort(network, service string) (port int, err error) {
-       if service == "" {
-               // Lock in the legacy behavior that an empty string
-               // means port 0. See Issue 13610.
-               return 0, nil
-       }
-       port, _, ok := dtoi(service, 0)
-       if !ok && port != big && port != -big {
+       port, needsLookup := parsePort(service)
+       if needsLookup {
                port, err = lookupPort(network, service)
                if err != nil {
                        return 0, err

diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go
index 85bcfef6e9ad4899161d039e33ce09752fdd07f9..6e54fdba76d902a1cf09511ba22eebd842d1c48b 100644 (file)
--- a/src/net/lookup_test.go
+++ b/src/net/lookup_test.go
@@ -627,6 +627,7 @@ var lookupPortTests = []struct {
        {"tcp", "65536", 0, false},
        {"udp", "-1", 0, false},
        {"udp", "65536", 0, false},
+       {"tcp", "123456789", 0, false},
 
        // Issue 13610: LookupPort("tcp", "")
        {"tcp", "", 0, true},
@@ -647,7 +648,7 @@ func TestLookupPort(t *testing.T) {
 
        for _, tt := range lookupPortTests {
                if port, err := LookupPort(tt.network, tt.name); port != tt.port || (err == nil) != tt.ok {
-                       t.Errorf("LookupPort(%q, %q) = %d, %v; want %d", tt.network, tt.name, port, err, tt.port)
+                       t.Errorf("LookupPort(%q, %q) = %d, %v; want %d, error=%t", tt.network, tt.name, port, err, tt.port, !tt.ok)
                }
        }
 }

diff --git a/src/net/port.go b/src/net/port.go
new file mode 100644 (file)
index 0000000..8e1321a
--- /dev/null
+++ b/src/net/port.go
@@ -0,0 +1,62 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package net
+
+// parsePort parses service as a decimal interger and returns the
+// corresponding value as port. It is the caller's responsibility to
+// parse service as a non-decimal integer when needsLookup is true.
+//
+// Some system resolvers will return a valid port number when given a number
+// over 65536 (see https://github.com/golang/go/issues/11715). Alas, the parser
+// can't bail early on numbers > 65536. Therefore reasonably large/small
+// numbers are parsed in full and rejected if invalid.
+func parsePort(service string) (port int, needsLookup bool) {
+       if service == "" {
+               // Lock in the legacy behavior that an empty string
+               // means port 0. See golang.org/issue/13610.
+               return 0, false
+       }
+       const (
+               max    = uint32(1<<32 - 1)
+               cutoff = uint32(1 << 30)
+       )
+       neg := false
+       if service[0] == '+' {
+               service = service[1:]
+       } else if service[0] == '-' {
+               neg = true
+               service = service[1:]
+       }
+       var n uint32
+       for _, d := range service {
+               if '0' <= d && d <= '9' {
+                       d -= '0'
+               } else {
+                       return 0, true
+               }
+               if n >= cutoff {
+                       n = max
+                       break
+               }
+               n *= 10
+               nn := n + uint32(d)
+               if nn < n || nn > max {
+                       n = max
+                       break
+               }
+               n = nn
+       }
+       if !neg && n >= cutoff {
+               port = int(cutoff - 1)
+       } else if neg && n > cutoff {
+               port = int(cutoff)
+       } else {
+               port = int(n)
+       }
+       if neg {
+               port = -port
+       }
+       return port, false
+}

diff --git a/src/net/port_test.go b/src/net/port_test.go
new file mode 100644 (file)
index 0000000..e0bdb42
--- /dev/null
+++ b/src/net/port_test.go
@@ -0,0 +1,52 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package net
+
+import "testing"
+
+var parsePortTests = []struct {
+       service     string
+       port        int
+       needsLookup bool
+}{
+       {"", 0, false},
+
+       // Decimal number literals
+       {"-1073741825", -1 << 30, false},
+       {"-1073741824", -1 << 30, false},
+       {"-1073741823", -(1<<30 - 1), false},
+       {"-123456789", -123456789, false},
+       {"-1", -1, false},
+       {"-0", 0, false},
+       {"0", 0, false},
+       {"+0", 0, false},
+       {"+1", 1, false},
+       {"65535", 65535, false},
+       {"65536", 65536, false},
+       {"123456789", 123456789, false},
+       {"1073741822", 1<<30 - 2, false},
+       {"1073741823", 1<<30 - 1, false},
+       {"1073741824", 1<<30 - 1, false},
+       {"1073741825", 1<<30 - 1, false},
+
+       // Others
+       {"abc", 0, true},
+       {"9pfs", 0, true},
+       {"123badport", 0, true},
+       {"bad123port", 0, true},
+       {"badport123", 0, true},
+       {"123456789badport", 0, true},
+       {"-2147483649badport", 0, true},
+       {"2147483649badport", 0, true},
+}
+
+func TestParsePort(t *testing.T) {
+       // The following test cases are cribbed from the strconv
+       for _, tt := range parsePortTests {
+               if port, needsLookup := parsePort(tt.service); port != tt.port || needsLookup != tt.needsLookup {
+                       t.Errorf("parsePort(%q) = %d, %t; want %d, %t", tt.service, port, needsLookup, tt.port, tt.needsLookup)
+               }
+       }
+}