nat: ParsePortSpec: validate port before proto

commit f388222 made an optimization
to handle validating the proto before parsing port-numbers, but
as a result changed the error returned if the port was missing.

This patch moves the check for empty container ports to the start
to keep the old behavior. The error-message(s) can probably be
improved a bit, but this keeps the existing output.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Author: thaJeztah

nat/nat.go

@@ -173,6 +173,10 @@ func splitParts(rawport string) (hostIP, hostPort, containerPort string) {
 func ParsePortSpec(rawPort string) ([]PortMapping, error) {
 	ip, hostPort, containerPort := splitParts(rawPort)
 	proto, containerPort := SplitProtoPort(containerPort)
+	if containerPort == "" {
+		return nil, fmt.Errorf("no port specified: %s<empty>", rawPort)
+	}
+
 	proto = strings.ToLower(proto)
 	if err := validateProto(proto); err != nil {
 		return nil, err
@@ -189,9 +193,6 @@ func ParsePortSpec(rawPort string) ([]PortMapping, error) {
 	if ip != "" && net.ParseIP(ip) == nil {
 		return nil, errors.New("invalid IP address: " + ip)
 	}
-	if containerPort == "" {
-		return nil, fmt.Errorf("no port specified: %s<empty>", rawPort)
-	}
 
 	startPort, endPort, err := ParsePortRange(containerPort)
 	if err != nil {

nat/nat_test.go

@@ -240,6 +240,37 @@ func TestSplitProtoPort(t *testing.T) {
 		})
 	}
 }
+func TestParsePortSpecEmptyContainerPort(t *testing.T) {
+	tests := []struct {
+		name     string
+		spec     string
+		expError string
+	}{
+		{
+			name:     "empty spec",
+			spec:     "",
+			expError: `no port specified: <empty>`,
+		},
+		{
+			name:     "empty container port",
+			spec:     `0.0.0.0:1234-1235:/tcp`,
+			expError: `no port specified: 0.0.0.0:1234-1235:/tcp<empty>`,
+		},
+		{
+			name:     "empty container port and proto",
+			spec:     `0.0.0.0:1234-1235:`,
+			expError: `no port specified: 0.0.0.0:1234-1235:<empty>`,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := ParsePortSpec(tc.spec)
+			if err == nil || err.Error() != tc.expError {
+				t.Fatalf("expected %v, got: %v", tc.expError, err)
+			}
+		})
+	}
+}
 
 func TestParsePortSpecFull(t *testing.T) {
 	portMappings, err := ParsePortSpec("0.0.0.0:1234-1235:3333-3334/tcp")