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>

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

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)
 		}
 	}
 }

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
+}

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)
+		}
+	}
+}