Merge pull request #7475 from erikh/fix_hostname_parsing

fix parsing of hostnames when we actually want IP addresses.

nat/nat.go

@@ -5,6 +5,7 @@ package nat
 
 import (
 	"fmt"
+	"net"
 	"strconv"
 	"strings"
 
@@ -68,6 +69,10 @@ func SplitProtoPort(rawPort string) (string, string) {
 		return "", ""
 	}
 	if l == 1 {
+		if rawPort == "" {
+			return "", "" // ""/tcp is not valid, ever
+		}
+
 		return "tcp", rawPort
 	}
 	return parts[1], parts[0]
@@ -114,6 +119,9 @@ func ParsePortSpecs(ports []string) (map[Port]struct{}, map[Port][]PortBinding,
 			hostPort      = parts["hostPort"]
 		)
 
+		if rawIp != "" && net.ParseIP(rawIp) == nil {
+			return nil, nil, fmt.Errorf("Invalid ip address: %s", rawIp)
+		}
 		if containerPort == "" {
 			return nil, nil, fmt.Errorf("No port specified: %s<empty>", rawPort)
 		}
@@ -123,6 +131,7 @@ func ParsePortSpecs(ports []string) (map[Port]struct{}, map[Port][]PortBinding,
 		if _, err := strconv.ParseUint(hostPort, 10, 16); hostPort != "" && err != nil {
 			return nil, nil, fmt.Errorf("Invalid hostPort: %s", hostPort)
 		}
+
 		if !validateProto(proto) {
 			return nil, nil, fmt.Errorf("Invalid proto: %s", proto)
 		}

nat/nat_test.go

@@ -0,0 +1,189 @@
+package nat
+
+import (
+	"testing"
+)
+
+func TestParsePort(t *testing.T) {
+	var (
+		p   int
+		err error
+	)
+
+	p, err = ParsePort("1234")
+
+	if err != nil || p != 1234 {
+		t.Fatal("Parsing '1234' did not succeed")
+	}
+
+	// FIXME currently this is a valid port. I don't think it should be.
+	// I'm leaving this test commented out until we make a decision.
+	// - erikh
+
+	/*
+		p, err = ParsePort("0123")
+
+		if err != nil {
+		    t.Fatal("Successfully parsed port '0123' to '123'")
+		}
+	*/
+
+	p, err = ParsePort("asdf")
+
+	if err == nil || p != 0 {
+		t.Fatal("Parsing port 'asdf' succeeded")
+	}
+
+	p, err = ParsePort("1asdf")
+
+	if err == nil || p != 0 {
+		t.Fatal("Parsing port '1asdf' succeeded")
+	}
+}
+
+func TestPort(t *testing.T) {
+	p := NewPort("tcp", "1234")
+
+	if string(p) != "1234/tcp" {
+		t.Fatal("tcp, 1234 did not result in the string 1234/tcp")
+	}
+
+	if p.Proto() != "tcp" {
+		t.Fatal("protocol was not tcp")
+	}
+
+	if p.Port() != "1234" {
+		t.Fatal("port string value was not 1234")
+	}
+
+	if p.Int() != 1234 {
+		t.Fatal("port int value was not 1234")
+	}
+}
+
+func TestSplitProtoPort(t *testing.T) {
+	var (
+		proto string
+		port  string
+	)
+
+	proto, port = SplitProtoPort("1234/tcp")
+
+	if proto != "tcp" || port != "1234" {
+		t.Fatal("Could not split 1234/tcp properly")
+	}
+
+	proto, port = SplitProtoPort("")
+
+	if proto != "" || port != "" {
+		t.Fatal("parsing an empty string yielded surprising results")
+	}
+
+	proto, port = SplitProtoPort("1234")
+
+	if proto != "tcp" || port != "1234" {
+		t.Fatal("tcp is not the default protocol for portspec '1234'")
+	}
+}
+
+func TestParsePortSpecs(t *testing.T) {
+	var (
+		portMap    map[Port]struct{}
+		bindingMap map[Port][]PortBinding
+		err        error
+	)
+
+	portMap, bindingMap, err = ParsePortSpecs([]string{"1234/tcp", "2345/udp"})
+
+	if err != nil {
+		t.Fatalf("Error while processing ParsePortSpecs: %s", err.Error())
+	}
+
+	if _, ok := portMap[Port("1234/tcp")]; !ok {
+		t.Fatal("1234/tcp was not parsed properly")
+	}
+
+	if _, ok := portMap[Port("2345/udp")]; !ok {
+		t.Fatal("2345/udp was not parsed properly")
+	}
+
+	for portspec, bindings := range bindingMap {
+		if len(bindings) != 1 {
+			t.Fatalf("%s should have exactly one binding", portspec)
+		}
+
+		if bindings[0].HostIp != "" {
+			t.Fatalf("HostIp should not be set for %s", portspec)
+		}
+
+		if bindings[0].HostPort != "" {
+			t.Fatalf("HostPort should not be set for %s", portspec)
+		}
+	}
+
+	portMap, bindingMap, err = ParsePortSpecs([]string{"1234:1234/tcp", "2345:2345/udp"})
+
+	if err != nil {
+		t.Fatalf("Error while processing ParsePortSpecs: %s", err.Error())
+	}
+
+	if _, ok := portMap[Port("1234/tcp")]; !ok {
+		t.Fatal("1234/tcp was not parsed properly")
+	}
+
+	if _, ok := portMap[Port("2345/udp")]; !ok {
+		t.Fatal("2345/udp was not parsed properly")
+	}
+
+	for portspec, bindings := range bindingMap {
+		_, port := SplitProtoPort(string(portspec))
+
+		if len(bindings) != 1 {
+			t.Fatalf("%s should have exactly one binding", portspec)
+		}
+
+		if bindings[0].HostIp != "" {
+			t.Fatalf("HostIp should not be set for %s", portspec)
+		}
+
+		if bindings[0].HostPort != port {
+			t.Fatalf("HostPort should be %s for %s", port, portspec)
+		}
+	}
+
+	portMap, bindingMap, err = ParsePortSpecs([]string{"0.0.0.0:1234:1234/tcp", "0.0.0.0:2345:2345/udp"})
+
+	if err != nil {
+		t.Fatalf("Error while processing ParsePortSpecs: %s", err.Error())
+	}
+
+	if _, ok := portMap[Port("1234/tcp")]; !ok {
+		t.Fatal("1234/tcp was not parsed properly")
+	}
+
+	if _, ok := portMap[Port("2345/udp")]; !ok {
+		t.Fatal("2345/udp was not parsed properly")
+	}
+
+	for portspec, bindings := range bindingMap {
+		_, port := SplitProtoPort(string(portspec))
+
+		if len(bindings) != 1 {
+			t.Fatalf("%s should have exactly one binding", portspec)
+		}
+
+		if bindings[0].HostIp != "0.0.0.0" {
+			t.Fatalf("HostIp is not 0.0.0.0 for %s", portspec)
+		}
+
+		if bindings[0].HostPort != port {
+			t.Fatalf("HostPort should be %s for %s", port, portspec)
+		}
+	}
+
+	_, _, err = ParsePortSpecs([]string{"localhost:1234:1234/tcp"})
+
+	if err == nil {
+		t.Fatal("Received no error while trying to parse a hostname instead of ip")
+	}
+}