From ceb2c7de44405da054be5e391c1ceeb4fb2c7da4 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Tue, 21 Jul 2015 17:10:36 -0700 Subject: [PATCH 1/2] Add additional test coverage for the regexp contained in RepositoryNameRegexp This was inspired by problems found with new regexps proposed in PR #690 Signed-off-by: Aaron Lehmann --- docs/api/v2/names_test.go | 142 ++++++++++++++++++++++++++++++++++++- docs/api/v2/routes_test.go | 21 ++++++ 2 files changed, 162 insertions(+), 1 deletion(-) diff --git a/docs/api/v2/names_test.go b/docs/api/v2/names_test.go index 51e0ba8b3..3a017037b 100644 --- a/docs/api/v2/names_test.go +++ b/docs/api/v2/names_test.go @@ -6,7 +6,7 @@ import ( "testing" ) -func TestRepositoryNameRegexp(t *testing.T) { +func TestRepositoryComponentNameRegexp(t *testing.T) { for _, testcase := range []struct { input string err error @@ -149,3 +149,143 @@ func TestRepositoryNameRegexp(t *testing.T) { } } } + +func TestRepositoryNameRegexp(t *testing.T) { + for _, testcase := range []struct { + input string + invalid bool + }{ + { + input: "short", + }, + { + input: "simple/name", + }, + { + input: "library/ubuntu", + }, + { + input: "docker/stevvooe/app", + }, + { + input: "aa/aa/aa/aa/aa/aa/aa/aa/aa/bb/bb/bb/bb/bb/bb", + }, + { + input: "aa/aa/bb/bb/bb", + }, + { + input: "a/a/a/b/b", + }, + { + input: "a/a/a/a/", + invalid: true, + }, + { + input: "a//a/a", + invalid: true, + }, + { + input: "a", + }, + { + input: "a/aa", + }, + { + input: "aa/a", + }, + { + input: "a/aa/a", + }, + { + input: "foo.com/", + invalid: true, + }, + { + // currently not allowed by the regex + input: "foo.com:8080/bar", + invalid: true, + }, + { + input: "foo.com/bar", + }, + { + input: "foo.com/bar/baz", + }, + { + input: "foo.com/bar/baz/quux", + }, + { + input: "blog.foo.com/bar/baz", + }, + { + input: "asdf", + }, + { + input: "asdf$$^/aa", + invalid: true, + }, + { + input: "aa-a/aa", + }, + { + input: "aa/aa", + }, + { + input: "a-a/a-a", + }, + { + input: "a-/a/a/a", + invalid: true, + }, + { + input: "-foo/bar", + invalid: true, + }, + { + input: "foo/bar-", + invalid: true, + }, + { + input: "foo-/bar", + invalid: true, + }, + { + input: "foo/-bar", + invalid: true, + }, + { + input: "_foo/bar", + invalid: true, + }, + { + input: "foo/bar_", + invalid: true, + }, + { + input: "____/____", + invalid: true, + }, + { + input: "_docker/_docker", + invalid: true, + }, + { + input: "docker_/docker_", + invalid: true, + }, + } { + failf := func(format string, v ...interface{}) { + t.Logf(strconv.Quote(testcase.input)+": "+format, v...) + t.Fail() + } + + matches := RepositoryNameRegexp.FindString(testcase.input) == testcase.input + if matches == testcase.invalid { + if testcase.invalid { + failf("expected invalid repository name %s", testcase.input) + } else { + failf("expected valid repository name %s", testcase.input) + } + } + } +} diff --git a/docs/api/v2/routes_test.go b/docs/api/v2/routes_test.go index 9fd29a4f5..b8d724dfe 100644 --- a/docs/api/v2/routes_test.go +++ b/docs/api/v2/routes_test.go @@ -66,6 +66,27 @@ func TestRouter(t *testing.T) { "name": "foo/bar", }, }, + { + RouteName: RouteNameTags, + RequestURI: "/v2/docker.com/foo/tags/list", + Vars: map[string]string{ + "name": "docker.com/foo", + }, + }, + { + RouteName: RouteNameTags, + RequestURI: "/v2/docker.com/foo/bar/tags/list", + Vars: map[string]string{ + "name": "docker.com/foo/bar", + }, + }, + { + RouteName: RouteNameTags, + RequestURI: "/v2/docker.com/foo/bar/baz/tags/list", + Vars: map[string]string{ + "name": "docker.com/foo/bar/baz", + }, + }, { RouteName: RouteNameBlob, RequestURI: "/v2/foo/bar/blobs/tarsum.dev+foo:abcdef0919234", From 683dc197782ea8f4ea2b5aaef624d6cbc4e637a4 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 22 Jul 2015 18:16:20 -0700 Subject: [PATCH 2/2] Unify the testcases for the two tests in names_test.go Signed-off-by: Aaron Lehmann --- docs/api/v2/names_test.go | 229 +++++++++++++------------------------- 1 file changed, 76 insertions(+), 153 deletions(-) diff --git a/docs/api/v2/names_test.go b/docs/api/v2/names_test.go index 3a017037b..89ab9c619 100644 --- a/docs/api/v2/names_test.go +++ b/docs/api/v2/names_test.go @@ -6,10 +6,18 @@ import ( "testing" ) -func TestRepositoryComponentNameRegexp(t *testing.T) { - for _, testcase := range []struct { +var ( + // regexpTestcases is a unified set of testcases for + // TestValidateRepositoryName and TestRepositoryNameRegexp. + // Some of them are valid inputs for one and not the other. + regexpTestcases = []struct { + // input is the repository name or name component testcase input string - err error + // err is the error expected from ValidateRepositoryName, or nil + err error + // invalid should be true if the testcase is *not* expected to + // match RepositoryNameRegexp + invalid bool }{ { input: "", @@ -37,12 +45,14 @@ func TestRepositoryComponentNameRegexp(t *testing.T) { input: "a/a/a/b/b", }, { - input: "a/a/a/a/", - err: ErrRepositoryNameComponentInvalid, + input: "a/a/a/a/", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "a//a/a", - err: ErrRepositoryNameComponentInvalid, + input: "a//a/a", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { input: "a", @@ -56,9 +66,27 @@ func TestRepositoryComponentNameRegexp(t *testing.T) { { input: "a/aa/a", }, + { + input: "foo.com/", + err: ErrRepositoryNameComponentInvalid, + invalid: true, + }, + { + // TODO: this testcase should be valid once we switch to + // the reference package. + input: "foo.com:8080/bar", + err: ErrRepositoryNameComponentInvalid, + invalid: true, + }, + { + input: "foo.com/bar", + }, { input: "foo.com/bar/baz", }, + { + input: "foo.com/bar/baz/quux", + }, { input: "blog.foo.com/bar/baz", }, @@ -66,8 +94,9 @@ func TestRepositoryComponentNameRegexp(t *testing.T) { input: "asdf", }, { - input: "asdf$$^/aa", - err: ErrRepositoryNameComponentInvalid, + input: "asdf$$^/aa", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { input: "aa-a/aa", @@ -79,8 +108,9 @@ func TestRepositoryComponentNameRegexp(t *testing.T) { input: "a-a/a-a", }, { - input: "a-/a/a/a", - err: ErrRepositoryNameComponentInvalid, + input: "a-/a/a/a", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { input: strings.Repeat("a", 255), @@ -90,42 +120,57 @@ func TestRepositoryComponentNameRegexp(t *testing.T) { err: ErrRepositoryNameLong, }, { - input: "-foo/bar", - err: ErrRepositoryNameComponentInvalid, + input: "-foo/bar", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "foo/bar-", - err: ErrRepositoryNameComponentInvalid, + input: "foo/bar-", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "foo-/bar", - err: ErrRepositoryNameComponentInvalid, + input: "foo-/bar", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "foo/-bar", - err: ErrRepositoryNameComponentInvalid, + input: "foo/-bar", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "_foo/bar", - err: ErrRepositoryNameComponentInvalid, + input: "_foo/bar", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "foo/bar_", - err: ErrRepositoryNameComponentInvalid, + input: "foo/bar_", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "____/____", - err: ErrRepositoryNameComponentInvalid, + input: "____/____", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "_docker/_docker", - err: ErrRepositoryNameComponentInvalid, + input: "_docker/_docker", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, { - input: "docker_/docker_", - err: ErrRepositoryNameComponentInvalid, + input: "docker_/docker_", + err: ErrRepositoryNameComponentInvalid, + invalid: true, }, - } { + } +) + +// TestValidateRepositoryName tests the ValidateRepositoryName function, +// which uses RepositoryNameComponentAnchoredRegexp for validation +func TestValidateRepositoryName(t *testing.T) { + for _, testcase := range regexpTestcases { failf := func(format string, v ...interface{}) { t.Logf(strconv.Quote(testcase.input)+": "+format, v...) t.Fail() @@ -151,129 +196,7 @@ func TestRepositoryComponentNameRegexp(t *testing.T) { } func TestRepositoryNameRegexp(t *testing.T) { - for _, testcase := range []struct { - input string - invalid bool - }{ - { - input: "short", - }, - { - input: "simple/name", - }, - { - input: "library/ubuntu", - }, - { - input: "docker/stevvooe/app", - }, - { - input: "aa/aa/aa/aa/aa/aa/aa/aa/aa/bb/bb/bb/bb/bb/bb", - }, - { - input: "aa/aa/bb/bb/bb", - }, - { - input: "a/a/a/b/b", - }, - { - input: "a/a/a/a/", - invalid: true, - }, - { - input: "a//a/a", - invalid: true, - }, - { - input: "a", - }, - { - input: "a/aa", - }, - { - input: "aa/a", - }, - { - input: "a/aa/a", - }, - { - input: "foo.com/", - invalid: true, - }, - { - // currently not allowed by the regex - input: "foo.com:8080/bar", - invalid: true, - }, - { - input: "foo.com/bar", - }, - { - input: "foo.com/bar/baz", - }, - { - input: "foo.com/bar/baz/quux", - }, - { - input: "blog.foo.com/bar/baz", - }, - { - input: "asdf", - }, - { - input: "asdf$$^/aa", - invalid: true, - }, - { - input: "aa-a/aa", - }, - { - input: "aa/aa", - }, - { - input: "a-a/a-a", - }, - { - input: "a-/a/a/a", - invalid: true, - }, - { - input: "-foo/bar", - invalid: true, - }, - { - input: "foo/bar-", - invalid: true, - }, - { - input: "foo-/bar", - invalid: true, - }, - { - input: "foo/-bar", - invalid: true, - }, - { - input: "_foo/bar", - invalid: true, - }, - { - input: "foo/bar_", - invalid: true, - }, - { - input: "____/____", - invalid: true, - }, - { - input: "_docker/_docker", - invalid: true, - }, - { - input: "docker_/docker_", - invalid: true, - }, - } { + for _, testcase := range regexpTestcases { failf := func(format string, v ...interface{}) { t.Logf(strconv.Quote(testcase.input)+": "+format, v...) t.Fail()