From 31470054b1b56dfd509fc0c8c6bd33956c1b1eef Mon Sep 17 00:00:00 2001
From: Phil Estes <estesp@gmail.com>
Date: Thu, 11 Sep 2014 10:32:44 -0400
Subject: [PATCH] Validate tag names similar to repository name validation; add
 tests Addresses #7943

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
Upstream-commit: 46eb4140c915e05a893080b34b42e25b6a75de7b
Component: engine
---
 components/engine/graph/tags.go               |  9 +++++--
 .../integration-cli/docker_cli_tag_test.go    | 27 +++++++++++++------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/components/engine/graph/tags.go b/components/engine/graph/tags.go
index eee3be9f2f..79a2deaff3 100644
--- a/components/engine/graph/tags.go
+++ b/components/engine/graph/tags.go
@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strings"
 	"sync"
@@ -17,6 +18,10 @@ import (
 
 const DEFAULTTAG = "latest"
 
+var (
+	validTagName = regexp.MustCompile(`^[\w][\w.-]{1,29}$`)
+)
+
 type TagStore struct {
 	path         string
 	graph        *Graph
@@ -282,8 +287,8 @@ func validateTagName(name string) error {
 	if name == "" {
 		return fmt.Errorf("Tag name can't be empty")
 	}
-	if strings.Contains(name, "/") || strings.Contains(name, ":") {
-		return fmt.Errorf("Illegal tag name: %s", name)
+	if !validTagName.MatchString(name) {
+		return fmt.Errorf("Illegal tag name (%s): only [A-Za-z0-9_.-] are allowed, minimum 2, maximum 30 in length", name)
 	}
 	return nil
 }
diff --git a/components/engine/integration-cli/docker_cli_tag_test.go b/components/engine/integration-cli/docker_cli_tag_test.go
index 032927e221..bb9b0ac1e2 100644
--- a/components/engine/integration-cli/docker_cli_tag_test.go
+++ b/components/engine/integration-cli/docker_cli_tag_test.go
@@ -37,23 +37,34 @@ func TestTagUnprefixedRepoByID(t *testing.T) {
 	logDone("tag - busybox's image ID -> testfoobarbaz")
 }
 
-// ensure we don't allow the use of invalid tags; these tag operations should fail
+// ensure we don't allow the use of invalid repository names; these tag operations should fail
 func TestTagInvalidUnprefixedRepo(t *testing.T) {
-	// skip this until we start blocking bad tags
-	t.Skip()
 
-	invalidRepos := []string{"-foo", "fo$z$", "Foo@3cc", "Foo$3", "Foo*3", "Fo^3", "Foo!3", "F)xcz(", "fo", "f"}
+	invalidRepos := []string{"fo$z$", "Foo@3cc", "Foo$3", "Foo*3", "Fo^3", "Foo!3", "F)xcz(", "fo%asd"}
 
 	for _, repo := range invalidRepos {
 		tagCmd := exec.Command(dockerBinary, "tag", "busybox", repo)
 		_, _, err := runCommandWithOutput(tagCmd)
 		if err == nil {
-			t.Errorf("tag busybox %v should have failed", repo)
-			continue
+			t.Fatalf("tag busybox %v should have failed", repo)
 		}
-		logMessage := fmt.Sprintf("tag - busybox %v --> must fail", repo)
-		logDone(logMessage)
 	}
+	logDone("tag - busybox invalid repo names --> must fail")
+}
+
+// ensure we don't allow the use of invalid tags; these tag operations should fail
+func TestTagInvalidPrefixedRepo(t *testing.T) {
+
+	invalidTags := []string{"repo:fo$z$", "repo:Foo@3cc", "repo:Foo$3", "repo:Foo*3", "repo:Fo^3", "repo:Foo!3", "repo:%goodbye", "repo:#hashtagit", "repo:F)xcz(", "repo:f", "repo:fwaytoolongandwaymorethan30characterslong", "repo:-foo", "repo:.."}
+
+	for _, repotag := range invalidTags {
+		tagCmd := exec.Command(dockerBinary, "tag", "busybox", repotag)
+		_, _, err := runCommandWithOutput(tagCmd)
+		if err == nil {
+			t.Fatalf("tag busybox %v should have failed", repotag)
+		}
+	}
+	logDone("tag - busybox with invalid repo:tagnames --> must fail")
 }
 
 // ensure we allow the use of valid tags