Merge pull request #7996 from estesp/7943-validate-tagnames

Validate tag names similar to repository name validation; add tests

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
@@ -284,8 +289,8 @@ func validateTagName(name string) error {
 	if name == "" {
 		return fmt.Errorf("Tag name can't be empty")
 	}
-	if strings.Contains(name, "/") || strings.Contains(name, ":") {
+	if !validTagName.MatchString(name) {
-		return fmt.Errorf("Illegal tag name: %s", name)
+		return fmt.Errorf("Illegal tag name (%s): only [A-Za-z0-9_.-] are allowed, minimum 2, maximum 30 in length", name)
 	}
 	return nil
 }

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)
+			t.Fatalf("tag busybox %v should have failed", repo)
-			continue
 		}
-		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