From 375f3cc1365dd0f22553e85c973a538cb7453f58 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 12 Nov 2014 16:39:35 -0800 Subject: [PATCH 1/3] Define common regexps used across registry application This commit adds regular expression definitions for several string identifiers used througout the registry. The repository name regex supports up to five path path components and restricts repeated periods, dashes and underscores. The tag regex simply validates the length of the tag and that printable characters are required. Though we define a new package common, these definition should land in docker core. --- common/names.go | 19 +++++++++++ common/names_test.go | 62 +++++++++++++++++++++++++++++++++ common/tarsum.go | 70 ++++++++++++++++++++++++++++++++++++++ common/tarsum_test.go | 79 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 230 insertions(+) create mode 100644 common/names.go create mode 100644 common/names_test.go create mode 100644 common/tarsum.go create mode 100644 common/tarsum_test.go diff --git a/common/names.go b/common/names.go new file mode 100644 index 0000000000..ce25e48729 --- /dev/null +++ b/common/names.go @@ -0,0 +1,19 @@ +package common + +import ( + "regexp" +) + +// 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](?:[a-z0-9]+[._-]?)*[a-z0-9]`) + +// RepositoryNameRegexp builds on RepositoryNameComponentRegexp to allow 2 to +// 5 path components, separated by a forward slash. +var RepositoryNameRegexp = regexp.MustCompile(`(?:` + RepositoryNameComponentRegexp.String() + `/){1,4}` + RepositoryNameComponentRegexp.String()) + +// TagNameRegexp matches valid tag names. From docker/docker:graph/tags.go. +var TagNameRegexp = regexp.MustCompile(`[\w][\w.-]{0,127}`) + +// TODO(stevvooe): Contribute these exports back to core, so they are shared. diff --git a/common/names_test.go b/common/names_test.go new file mode 100644 index 0000000000..17655984fc --- /dev/null +++ b/common/names_test.go @@ -0,0 +1,62 @@ +package common + +import ( + "testing" +) + +func TestRepositoryNameRegexp(t *testing.T) { + for _, testcase := range []struct { + input string + valid bool + }{ + { + 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, + }, + { + input: "a/a/a/a/a/a/b/b/b/b", + valid: false, + }, + { + input: "a/a/a/a/", + valid: false, + }, + { + input: "foo.com/bar/baz", + valid: true, + }, + { + input: "blog.foo.com/bar/baz", + valid: true, + }, + { + input: "asdf", + valid: false, + }, + { + input: "asdf$$^/", + valid: false, + }, + } { + 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) + } + } +} diff --git a/common/tarsum.go b/common/tarsum.go new file mode 100644 index 0000000000..5a6e7d2159 --- /dev/null +++ b/common/tarsum.go @@ -0,0 +1,70 @@ +package common + +import ( + "fmt" + + "regexp" +) + +// TarSumRegexp defines a reguler expression to match tarsum identifiers. +var TarsumRegexp = regexp.MustCompile("tarsum(?:.[a-z0-9]+)?\\+[a-zA-Z0-9]+:[A-Fa-f0-9]+") + +// TarsumRegexpCapturing defines a reguler expression to match tarsum identifiers with +// capture groups corresponding to each component. +var TarsumRegexpCapturing = regexp.MustCompile("(tarsum)(.([a-z0-9]+))?\\+([a-zA-Z0-9]+):([A-Fa-f0-9]+)") + +// TarSumInfo contains information about a parsed tarsum. +type TarSumInfo struct { + // Version contains the version of the tarsum. + Version string + + // Algorithm contains the algorithm for the final digest + Algorithm string + + // Digest contains the hex-encoded digest. + Digest string +} + +type InvalidTarSumError struct { + TarSum string +} + +func (e InvalidTarSumError) Error() string { + return fmt.Sprintf("invalid tarsum: %q", e.TarSum) +} + +// ParseTarSum parses a tarsum string into its components of interest. For +// example, this method may receive the tarsum in the following format: +// +// tarsum.v1+sha256:220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e +// +// The function will return the following: +// +// TarSumInfo{ +// Version: "v1", +// Algorithm: "sha256", +// Digest: "220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e", +// } +// +func ParseTarSum(tarSum string) (tsi TarSumInfo, err error) { + components := TarsumRegexpCapturing.FindStringSubmatch(tarSum) + + if len(components) != 1+TarsumRegexpCapturing.NumSubexp() { + return TarSumInfo{}, InvalidTarSumError{TarSum: tarSum} + } + + return TarSumInfo{ + Version: components[3], + Algorithm: components[4], + Digest: components[5], + }, nil +} + +// String returns the valid, string representation of the tarsum info. +func (tsi TarSumInfo) String() string { + if tsi.Version == "" { + return fmt.Sprintf("tarsum+%s:%s", tsi.Algorithm, tsi.Digest) + } + + return fmt.Sprintf("tarsum.%s+%s:%s", tsi.Version, tsi.Algorithm, tsi.Digest) +} diff --git a/common/tarsum_test.go b/common/tarsum_test.go new file mode 100644 index 0000000000..e860c9cdf1 --- /dev/null +++ b/common/tarsum_test.go @@ -0,0 +1,79 @@ +package common + +import ( + "reflect" + "testing" +) + +func TestParseTarSumComponents(t *testing.T) { + for _, testcase := range []struct { + input string + expected TarSumInfo + err error + }{ + { + input: "tarsum.v1+sha256:220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e", + expected: TarSumInfo{ + Version: "v1", + Algorithm: "sha256", + Digest: "220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e", + }, + }, + { + input: "", + err: InvalidTarSumError{}, + }, + { + input: "purejunk", + err: InvalidTarSumError{TarSum: "purejunk"}, + }, + { + input: "tarsum.v23+test:12341234123412341effefefe", + expected: TarSumInfo{ + Version: "v23", + Algorithm: "test", + Digest: "12341234123412341effefefe", + }, + }, + + // The following test cases are ported from docker core + { + // Version 0 tarsum + input: "tarsum+sha256:e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b", + expected: TarSumInfo{ + Algorithm: "sha256", + Digest: "e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b", + }, + }, + { + // Dev version tarsum + input: "tarsum.dev+sha256:e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b", + expected: TarSumInfo{ + Version: "dev", + Algorithm: "sha256", + Digest: "e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b", + }, + }, + } { + tsi, err := ParseTarSum(testcase.input) + if err != nil { + if testcase.err != nil && err == testcase.err { + continue // passes + } + + t.Fatalf("unexpected error parsing tarsum: %v", err) + } + + if testcase.err != nil { + t.Fatalf("expected error not encountered on %q: %v", testcase.input, testcase.err) + } + + if !reflect.DeepEqual(tsi, testcase.expected) { + t.Fatalf("expected tarsum info: %v != %v", tsi, testcase.expected) + } + + if testcase.input != tsi.String() { + t.Fatalf("input should equal output: %q != %q", tsi.String(), testcase.input) + } + } +} From 145c89bb94e5bf1770097f31fecdb407067b628c Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 12 Nov 2014 16:59:50 -0800 Subject: [PATCH 2/3] Disambiguate routing for multi-level repository names To be able to support multi-level repository names, the API has been adjusted to disabiguate routes tagged image manifest routes and tag list routes. With this effort, the regular expressions have been defined in a single place to reduce repitition and ensure that validation is consistent across the registry. The router was also refactored to remove the use of subrouters, simplifying the route definition code. This also reduces the number of regular expression match checks during the routing process. --- app_test.go | 6 +-- routes.go | 38 ++++++---------- routes_test.go | 121 ++++++++++++++++++++++++++++++------------------- 3 files changed, 90 insertions(+), 75 deletions(-) diff --git a/app_test.go b/app_test.go index 43b001ec66..e0fa727fe3 100644 --- a/app_test.go +++ b/app_test.go @@ -87,21 +87,21 @@ func TestAppDispatcher(t *testing.T) { endpoint: routeNameLayer, vars: []string{ "name", "foo/bar", - "tarsum", "thetarsum", + "tarsum", "tarsum.v1+bogus:abcdef0123456789", }, }, { endpoint: routeNameLayerUpload, vars: []string{ "name", "foo/bar", - "tarsum", "thetarsum", + "tarsum", "tarsum.v1+bogus:abcdef0123456789", }, }, { endpoint: routeNameLayerUploadResume, vars: []string{ "name", "foo/bar", - "tarsum", "thetarsum", + "tarsum", "tarsum.v1+bogus:abcdef0123456789", "uuid", "theuuid", }, }, diff --git a/routes.go b/routes.go index d49426966c..8da7c3e2ca 100644 --- a/routes.go +++ b/routes.go @@ -1,12 +1,11 @@ package registry import ( + "github.com/docker/docker-registry/common" "github.com/gorilla/mux" ) const ( - routeNameRoot = "root" - routeNameName = "name" routeNameImageManifest = "image-manifest" routeNameTags = "tags" routeNameLayer = "layer" @@ -25,47 +24,36 @@ var allEndpoints = []string{ // v2APIRouter builds a gorilla router with named routes for the various API // methods. We may export this for use by the client. func v2APIRouter() *mux.Router { - router := mux.NewRouter() - - rootRouter := router. - PathPrefix("/v2"). - Name(routeNameRoot). - Subrouter() - - // All routes are subordinate to named routes - namedRouter := rootRouter. - PathPrefix("/{name:[A-Za-z0-9-_]+/[A-Za-z0-9-_]+}"). // TODO(stevvooe): Verify this format with core - Name(routeNameName). - Subrouter(). + router := mux.NewRouter(). StrictSlash(true) // GET /v2//image/ Image Manifest Fetch the image manifest identified by name and tag. // PUT /v2//image/ Image Manifest Upload the image manifest identified by name and tag. // DELETE /v2//image/ Image Manifest Delete the image identified by name and tag. - namedRouter. - Path("/image/{tag:[A-Za-z0-9-_]+}"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/image/{tag:" + common.TagNameRegexp.String() + "}"). Name(routeNameImageManifest) - // GET /v2//tags Tags Fetch the tags under the repository identified by name. - namedRouter. - Path("/tags"). + // GET /v2//tags/list Tags Fetch the tags under the repository identified by name. + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/tags/list"). Name(routeNameTags) // GET /v2//layer/ Layer Fetch the layer identified by tarsum. - namedRouter. - Path("/layer/{tarsum}"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}"). Name(routeNameLayer) // POST /v2//layer//upload/ Layer Upload Initiate an upload of the layer identified by tarsum. Requires length and a checksum parameter. - namedRouter. - Path("/layer/{tarsum}/upload/"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}/upload/"). Name(routeNameLayerUpload) // GET /v2//layer//upload/ Layer Upload Get the status of the upload identified by tarsum and uuid. // PUT /v2//layer//upload/ Layer Upload Upload all or a chunk of the upload identified by tarsum and uuid. // DELETE /v2//layer//upload/ Layer Upload Cancel the upload identified by layer and uuid - namedRouter. - Path("/layer/{tarsum}/upload/{uuid}"). + router. + Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}/upload/{uuid}"). Name(routeNameLayerUploadResume) return router diff --git a/routes_test.go b/routes_test.go index e3ef371a7f..8907684a90 100644 --- a/routes_test.go +++ b/routes_test.go @@ -10,9 +10,10 @@ import ( "github.com/gorilla/mux" ) -type routeInfo struct { +type routeTestCase struct { RequestURI string Vars map[string]string + RouteName string } // TestRouter registers a test handler with all the routes and ensures that @@ -26,14 +27,15 @@ func TestRouter(t *testing.T) { router := v2APIRouter() testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - routeInfo := routeInfo{ + testCase := routeTestCase{ RequestURI: r.RequestURI, Vars: mux.Vars(r), + RouteName: mux.CurrentRoute(r).GetName(), } enc := json.NewEncoder(w) - if err := enc.Encode(routeInfo); err != nil { + if err := enc.Encode(testCase); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -42,64 +44,81 @@ func TestRouter(t *testing.T) { // Startup test server server := httptest.NewServer(router) - for _, testcase := range []struct { - routeName string - expectedRouteInfo routeInfo - }{ + for _, testcase := range []routeTestCase{ { - routeName: routeNameImageManifest, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/image/tag", - Vars: map[string]string{ - "name": "foo/bar", - "tag": "tag", - }, + RouteName: routeNameImageManifest, + RequestURI: "/v2/foo/bar/image/tag", + Vars: map[string]string{ + "name": "foo/bar", + "tag": "tag", }, }, { - routeName: routeNameTags, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/tags", - Vars: map[string]string{ - "name": "foo/bar", - }, + RouteName: routeNameTags, + RequestURI: "/v2/foo/bar/tags/list", + Vars: map[string]string{ + "name": "foo/bar", }, }, { - routeName: routeNameLayer, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/layer/tarsum", - Vars: map[string]string{ - "name": "foo/bar", - "tarsum": "tarsum", - }, + RouteName: routeNameLayer, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", }, }, { - routeName: routeNameLayerUpload, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/layer/tarsum/upload/", - Vars: map[string]string{ - "name": "foo/bar", - "tarsum": "tarsum", - }, + RouteName: routeNameLayerUpload, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", }, }, { - routeName: routeNameLayerUploadResume, - expectedRouteInfo: routeInfo{ - RequestURI: "/v2/foo/bar/layer/tarsum/upload/uuid", - Vars: map[string]string{ - "name": "foo/bar", - "tarsum": "tarsum", - "uuid": "uuid", - }, + RouteName: routeNameLayerUploadResume, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/uuid", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", + "uuid": "uuid", + }, + }, + { + RouteName: routeNameLayerUploadResume, + RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/D95306FA-FAD3-4E36-8D41-CF1C93EF8286", + Vars: map[string]string{ + "name": "foo/bar", + "tarsum": "tarsum.dev+foo:abcdef0919234", + "uuid": "D95306FA-FAD3-4E36-8D41-CF1C93EF8286", + }, + }, + + { + // Check ambiguity: ensure we can distinguish between tags for + // "foo/bar/image/image" and image for "foo/bar/image" with tag + // "tags" + RouteName: routeNameImageManifest, + RequestURI: "/v2/foo/bar/image/image/tags", + Vars: map[string]string{ + "name": "foo/bar/image", + "tag": "tags", + }, + }, + { + // This case presents an ambiguity between foo/bar with tag="tags" + // and list tags for "foo/bar/image" + RouteName: routeNameTags, + RequestURI: "/v2/foo/bar/image/tags/list", + Vars: map[string]string{ + "name": "foo/bar/image", }, }, } { // Register the endpoint - router.GetRoute(testcase.routeName).Handler(testHandler) - u := server.URL + testcase.expectedRouteInfo.RequestURI + router.GetRoute(testcase.RouteName).Handler(testHandler) + u := server.URL + testcase.RequestURI resp, err := http.Get(u) @@ -107,15 +126,23 @@ func TestRouter(t *testing.T) { t.Fatalf("error issuing get request: %v", err) } + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status for %s: %v %v", u, resp.Status, resp.StatusCode) + } + dec := json.NewDecoder(resp.Body) - var actualRouteInfo routeInfo + var actualRouteInfo routeTestCase if err := dec.Decode(&actualRouteInfo); err != nil { t.Fatalf("error reading json response: %v", err) } - if !reflect.DeepEqual(actualRouteInfo, testcase.expectedRouteInfo) { - t.Fatalf("actual does not equal expected: %v != %v", actualRouteInfo, testcase.expectedRouteInfo) + if actualRouteInfo.RouteName != testcase.RouteName { + t.Fatalf("incorrect route %q matched, expected %q", actualRouteInfo.RouteName, testcase.RouteName) + } + + if !reflect.DeepEqual(actualRouteInfo, testcase) { + t.Fatalf("actual does not equal expected: %#v != %#v", actualRouteInfo, testcase) } } From 15c651b73216daaef24d0b09641511d04cde7970 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 12 Nov 2014 18:06:54 -0800 Subject: [PATCH 3/3] Simplify repository name component regexp --- common/names.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/names.go b/common/names.go index ce25e48729..d856467bfd 100644 --- a/common/names.go +++ b/common/names.go @@ -7,7 +7,7 @@ import ( // 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](?:[a-z0-9]+[._-]?)*[a-z0-9]`) +var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]{2,}(?:[._-][a-z0-9]+)*`) // RepositoryNameRegexp builds on RepositoryNameComponentRegexp to allow 2 to // 5 path components, separated by a forward slash.