From a650f0f85457ff4677494331f461fb8099062361 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 17 Nov 2014 11:28:32 -0800 Subject: [PATCH] Clarify repository naming constraints for registry API After discussion, it was found that one of the proposed regular expressions incorrectly limited separator delimited compoonents to two characters. The desired restriction is to have repository name components limited to two characters minimum. This changeset accomplishes this by wrapping the regular expressions in a validation function, returning detailed feedback on the validation error. With this change, the repository name regular expressions are no longer enough to respond with 404s on invalid repo names. Changes to the router will need to be added to support this. --- common/names.go | 61 +++++++++++++++++++++++++++++++++++++++- common/names_test.go | 67 +++++++++++++++++++++++++++++++------------- 2 files changed, 108 insertions(+), 20 deletions(-) diff --git a/common/names.go b/common/names.go index c16cbaad8..43f7247aa 100644 --- a/common/names.go +++ b/common/names.go @@ -1,13 +1,25 @@ package common import ( + "fmt" "regexp" + "strings" +) + +const ( + RepositoryNameComponentMinLength = 2 + RepositoryNameComponentMaxLength = 30 + + RepositoryNameMinComponents = 2 + RepositoryNameMaxComponents = 5 + RepositoryNameTotalLengthMax = 255 ) // RepositoryNameComponentRegexp restricts registtry path components names to // start with at least two letters or numbers, with following parts able to // separated by one period, dash or underscore. -var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]{2,}(?:[._-][a-z0-9]+)*`) +var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]+(?:[._-][a-z0-9]+)*`) +var RepositoryNameComponentAnchoredRegexp = regexp.MustCompile(`^` + RepositoryNameComponentRegexp.String() + `$`) // TODO(stevvooe): RepositoryName needs to be limited to some fixed length. // Looking path prefixes and s3 limitation of 1024, this should likely be @@ -21,3 +33,50 @@ var RepositoryNameRegexp = regexp.MustCompile(`(?:` + RepositoryNameComponentReg var TagNameRegexp = regexp.MustCompile(`[\w][\w.-]{0,127}`) // TODO(stevvooe): Contribute these exports back to core, so they are shared. + +var ( + ErrRepositoryNameComponentShort = fmt.Errorf("respository name component must be %v or more characters", RepositoryNameComponentMinLength) + ErrRepositoryNameComponentLong = fmt.Errorf("respository name component must be %v characters or less", RepositoryNameComponentMaxLength) + + ErrRepositoryNameMissingComponents = fmt.Errorf("repository name must have at least %v components", RepositoryNameMinComponents) + ErrRepositoryNameTooManyComponents = fmt.Errorf("repository name %v or less components", RepositoryNameMaxComponents) + + ErrRepositoryNameLong = fmt.Errorf("repository name must not be more than %v characters", RepositoryNameTotalLengthMax) + ErrRepositoryNameComponentInvalid = fmt.Errorf("repository name component must match %q", RepositoryNameComponentRegexp.String()) +) + +// ValidateRespositoryName ensures the repository name is valid for use in the +// registry. This function accepts a superset of what might be accepted by +// docker core or docker hub. If the name does not pass validation, an error, +// describing the conditions, is returned. +func ValidateRespositoryName(name string) error { + if len(name) > RepositoryNameTotalLengthMax { + return ErrRepositoryNameLong + } + + components := strings.Split(name, "/") + + if len(components) < RepositoryNameMinComponents { + return ErrRepositoryNameMissingComponents + } + + if len(components) > RepositoryNameMaxComponents { + return ErrRepositoryNameTooManyComponents + } + + for _, component := range components { + if len(component) < RepositoryNameComponentMinLength { + return ErrRepositoryNameComponentShort + } + + if len(component) > RepositoryNameComponentMaxLength { + return ErrRepositoryNameComponentLong + } + + if !RepositoryNameComponentAnchoredRegexp.MatchString(component) { + return ErrRepositoryNameComponentInvalid + } + } + + return nil +} diff --git a/common/names_test.go b/common/names_test.go index 17655984f..e88257bde 100644 --- a/common/names_test.go +++ b/common/names_test.go @@ -7,56 +7,85 @@ import ( func TestRepositoryNameRegexp(t *testing.T) { for _, testcase := range []struct { input string - valid bool + err error }{ { input: "simple/name", - valid: true, }, { input: "library/ubuntu", - valid: true, }, { input: "docker/stevvooe/app", - valid: true, }, { input: "aa/aa/aa/aa/aa/aa/aa/aa/aa/bb/bb/bb/bb/bb/bb", - valid: true, + err: ErrRepositoryNameTooManyComponents, }, { - input: "a/a/a/a/a/a/b/b/b/b", - valid: false, + input: "aa/aa/bb/bb/bb", + }, + { + input: "a/a/a/b/b", + err: ErrRepositoryNameComponentShort, }, { input: "a/a/a/a/", - valid: false, + err: ErrRepositoryNameComponentShort, }, { input: "foo.com/bar/baz", - valid: true, }, { input: "blog.foo.com/bar/baz", - valid: true, }, { input: "asdf", - valid: false, + err: ErrRepositoryNameMissingComponents, }, { - input: "asdf$$^/", - valid: false, + input: "asdf$$^/aa", + err: ErrRepositoryNameComponentInvalid, + }, + { + input: "aa-a/aa", + }, + { + input: "aa/aa", + }, + { + input: "a-a/a-a", + }, + { + input: "a", + err: ErrRepositoryNameMissingComponents, + }, + { + input: "a-/a/a/a", + err: ErrRepositoryNameComponentInvalid, }, } { - if RepositoryNameRegexp.MatchString(testcase.input) != testcase.valid { - status := "invalid" - if testcase.valid { - status = "valid" - } - t.Fatalf("expected %q to be %s repository name", testcase.input, status) + failf := func(format string, v ...interface{}) { + t.Logf(testcase.input+": "+format, v...) + t.Fail() + } + + if err := ValidateRespositoryName(testcase.input); err != testcase.err { + if testcase.err != nil { + if err != nil { + failf("unexpected error for invalid repository: got %v, expected %v", err, testcase.err) + } else { + failf("expected invalid repository: %v", testcase.err) + } + } else { + if err != nil { + // Wrong error returned. + failf("unexpected error validating repository name: %v, expected %v", err, testcase.err) + } else { + failf("unexpected error validating repository name: %v", err) + } + } } } }