From 43b36970f589904544125d0fa7e26fe77aca0e13 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 2 Feb 2015 13:17:33 -0800 Subject: [PATCH] Prefix non-name path components To address the possibility of confusing registry name components with repository paths, path components that abut user provided repository names are escaped with a prefixed underscore. This works because repository name components are no allowed to start with underscores. The requirements on backend driver path names have been relaxed greatly to support this use case. Signed-off-by: Stephen J Day --- storage/paths.go | 40 +++++++++++++------------- storage/paths_test.go | 24 ++++++++-------- storagedriver/storagedriver.go | 12 ++++---- storagedriver/testsuites/testsuites.go | 25 ++++++++++++++-- 4 files changed, 61 insertions(+), 40 deletions(-) diff --git a/storage/paths.go b/storage/paths.go index f393a62a..9380dc65 100644 --- a/storage/paths.go +++ b/storage/paths.go @@ -18,7 +18,7 @@ const storagePathVersion = "v2" // /v2 // -> repositories/ // ->/ -// -> manifests/ +// -> _manifests/ // revisions // -> // -> link @@ -28,9 +28,9 @@ const storagePathVersion = "v2" // -> current/link // -> index // -> //link -// -> layers/ +// -> _layers/ // -// -> uploads/ +// -> _uploads/ // data // startedat // -> blob/ @@ -65,27 +65,27 @@ const storagePathVersion = "v2" // // Manifests: // -// manifestRevisionPathSpec: /v2/repositories//manifests/revisions/// -// manifestRevisionLinkPathSpec: /v2/repositories//manifests/revisions///link -// manifestSignaturesPathSpec: /v2/repositories//manifests/revisions///signatures/ -// manifestSignatureLinkPathSpec: /v2/repositories//manifests/revisions///signatures///link +// manifestRevisionPathSpec: /v2/repositories//_manifests/revisions/// +// manifestRevisionLinkPathSpec: /v2/repositories//_manifests/revisions///link +// manifestSignaturesPathSpec: /v2/repositories//_manifests/revisions///signatures/ +// manifestSignatureLinkPathSpec: /v2/repositories//_manifests/revisions///signatures///link // // Tags: // -// manifestTagsPathSpec: /v2/repositories//manifests/tags/ -// manifestTagPathSpec: /v2/repositories//manifests/tags// -// manifestTagCurrentPathSpec: /v2/repositories//manifests/tags//current/link -// manifestTagIndexPathSpec: /v2/repositories//manifests/tags//index/ -// manifestTagIndexEntryPathSpec: /v2/repositories//manifests/tags//index///link +// manifestTagsPathSpec: /v2/repositories//_manifests/tags/ +// manifestTagPathSpec: /v2/repositories//_manifests/tags// +// manifestTagCurrentPathSpec: /v2/repositories//_manifests/tags//current/link +// manifestTagIndexPathSpec: /v2/repositories//_manifests/tags//index/ +// manifestTagIndexEntryPathSpec: /v2/repositories//_manifests/tags//index///link // // Layers: // -// layerLinkPathSpec: /v2/repositories//layers/tarsum////link +// layerLinkPathSpec: /v2/repositories//_layers/tarsum////link // // Uploads: // -// uploadDataPathSpec: /v2/repositories//uploads//data -// uploadStartedAtPathSpec: /v2/repositories//uploads//startedat +// uploadDataPathSpec: /v2/repositories//_uploads//data +// uploadStartedAtPathSpec: /v2/repositories//_uploads//startedat // // Blob Store: // @@ -130,7 +130,7 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) { return "", err } - return path.Join(append(append(repoPrefix, v.name, "manifests", "revisions"), components...)...), nil + return path.Join(append(append(repoPrefix, v.name, "_manifests", "revisions"), components...)...), nil case manifestRevisionLinkPathSpec: root, err := pm.path(manifestRevisionPathSpec{ name: v.name, @@ -169,7 +169,7 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) { return path.Join(root, path.Join(append(signatureComponents, "link")...)), nil case manifestTagsPathSpec: - return path.Join(append(repoPrefix, v.name, "manifests", "tags")...), nil + return path.Join(append(repoPrefix, v.name, "_manifests", "tags")...), nil case manifestTagPathSpec: root, err := pm.path(manifestTagsPathSpec{ name: v.name, @@ -226,7 +226,7 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) { return "", fmt.Errorf("unsupported content digest: %v", v.digest) } - layerLinkPathComponents := append(repoPrefix, v.name, "layers") + layerLinkPathComponents := append(repoPrefix, v.name, "_layers") return path.Join(path.Join(append(layerLinkPathComponents, components...)...), "link"), nil case blobDataPathSpec: @@ -240,9 +240,9 @@ func (pm *pathMapper) path(spec pathSpec) (string, error) { return path.Join(append(blobPathPrefix, components...)...), nil case uploadDataPathSpec: - return path.Join(append(repoPrefix, v.name, "uploads", v.uuid, "data")...), nil + return path.Join(append(repoPrefix, v.name, "_uploads", v.uuid, "data")...), nil case uploadStartedAtPathSpec: - return path.Join(append(repoPrefix, v.name, "uploads", v.uuid, "startedat")...), nil + return path.Join(append(repoPrefix, v.name, "_uploads", v.uuid, "startedat")...), nil default: // TODO(sday): This is an internal error. Ensure it doesn't escape (panic?). return "", fmt.Errorf("unknown path spec: %#v", v) diff --git a/storage/paths_test.go b/storage/paths_test.go index 94e4a497..79410e75 100644 --- a/storage/paths_test.go +++ b/storage/paths_test.go @@ -21,14 +21,14 @@ func TestPathMapper(t *testing.T) { name: "foo/bar", revision: "sha256:abcdef0123456789", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/revisions/sha256/abcdef0123456789", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789", }, { spec: manifestRevisionLinkPathSpec{ name: "foo/bar", revision: "sha256:abcdef0123456789", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/revisions/sha256/abcdef0123456789/link", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789/link", }, { spec: manifestSignatureLinkPathSpec{ @@ -36,41 +36,41 @@ func TestPathMapper(t *testing.T) { revision: "sha256:abcdef0123456789", signature: "sha256:abcdef0123456789", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/revisions/sha256/abcdef0123456789/signatures/sha256/abcdef0123456789/link", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789/signatures/sha256/abcdef0123456789/link", }, { spec: manifestSignaturesPathSpec{ name: "foo/bar", revision: "sha256:abcdef0123456789", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/revisions/sha256/abcdef0123456789/signatures", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/revisions/sha256/abcdef0123456789/signatures", }, { spec: manifestTagsPathSpec{ name: "foo/bar", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/tags", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags", }, { spec: manifestTagPathSpec{ name: "foo/bar", tag: "thetag", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/tags/thetag", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags/thetag", }, { spec: manifestTagCurrentPathSpec{ name: "foo/bar", tag: "thetag", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/tags/thetag/current/link", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags/thetag/current/link", }, { spec: manifestTagIndexPathSpec{ name: "foo/bar", tag: "thetag", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/tags/thetag/index", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags/thetag/index", }, { spec: manifestTagIndexEntryPathSpec{ @@ -78,14 +78,14 @@ func TestPathMapper(t *testing.T) { tag: "thetag", revision: "sha256:abcdef0123456789", }, - expected: "/pathmapper-test/repositories/foo/bar/manifests/tags/thetag/index/sha256/abcdef0123456789/link", + expected: "/pathmapper-test/repositories/foo/bar/_manifests/tags/thetag/index/sha256/abcdef0123456789/link", }, { spec: layerLinkPathSpec{ name: "foo/bar", digest: "tarsum.v1+test:abcdef", }, - expected: "/pathmapper-test/repositories/foo/bar/layers/tarsum/v1/test/abcdef/link", + expected: "/pathmapper-test/repositories/foo/bar/_layers/tarsum/v1/test/abcdef/link", }, { spec: blobDataPathSpec{ @@ -105,14 +105,14 @@ func TestPathMapper(t *testing.T) { name: "foo/bar", uuid: "asdf-asdf-asdf-adsf", }, - expected: "/pathmapper-test/repositories/foo/bar/uploads/asdf-asdf-asdf-adsf/data", + expected: "/pathmapper-test/repositories/foo/bar/_uploads/asdf-asdf-asdf-adsf/data", }, { spec: uploadStartedAtPathSpec{ name: "foo/bar", uuid: "asdf-asdf-asdf-adsf", }, - expected: "/pathmapper-test/repositories/foo/bar/uploads/asdf-asdf-asdf-adsf/startedat", + expected: "/pathmapper-test/repositories/foo/bar/_uploads/asdf-asdf-asdf-adsf/startedat", }, } { p, err := pm.path(testcase.spec) diff --git a/storagedriver/storagedriver.go b/storagedriver/storagedriver.go index d050d68f..c94ef8d6 100644 --- a/storagedriver/storagedriver.go +++ b/storagedriver/storagedriver.go @@ -78,12 +78,12 @@ type StorageDriver interface { URLFor(path string, options map[string]interface{}) (string, error) } -// PathRegexp is the regular expression which each file path must match. -// A file path is absolute, beginning with a slash and containing a -// positive number of path components separated by slashes, where each component -// is restricted to lowercase alphanumeric characters, optionally separated by -// a period, underscore, or hyphen. -var PathRegexp = regexp.MustCompile(`^(/[a-z0-9]+([._-][a-z0-9]+)*)+$`) +// PathRegexp is the regular expression which each file path must match. A +// file path is absolute, beginning with a slash and containing a positive +// number of path components separated by slashes, where each component is +// restricted to lowercase alphanumeric characters or a period, underscore, or +// hyphen. +var PathRegexp = regexp.MustCompile(`^(/[a-z0-9._-]+)+$`) // UnsupportedMethodErr may be returned in the case where a StorageDriver implementation does not support an optional method. var ErrUnsupportedMethod = errors.New("Unsupported method") diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index 2b181471..324024e1 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -123,7 +123,21 @@ func (suite *DriverSuite) TearDownTest(c *check.C) { // storage driver. func (suite *DriverSuite) TestValidPaths(c *check.C) { contents := randomContents(64) - validFiles := []string{"/a", "/2", "/aa", "/a.a", "/0-9/abcdefg", "/abcdefg/z.75", "/abc/1.2.3.4.5-6_zyx/123.z/4", "/docker/docker-registry"} + validFiles := []string{ + "/a", + "/2", + "/aa", + "/a.a", + "/0-9/abcdefg", + "/abcdefg/z.75", + "/abc/1.2.3.4.5-6_zyx/123.z/4", + "/docker/docker-registry", + "/123.abc", + "/abc./abc", + "/.abc", + "/a--b", + "/a-.b", + "/_.abc"} for _, filename := range validFiles { err := suite.StorageDriver.PutContent(filename, contents) @@ -140,7 +154,14 @@ func (suite *DriverSuite) TestValidPaths(c *check.C) { // storage driver. func (suite *DriverSuite) TestInvalidPaths(c *check.C) { contents := randomContents(64) - invalidFiles := []string{"", "/", "abc", "123.abc", "/abc./abc", "/.abc", "/a--b", "/a-.b", "/_.abc", "//bcd", "/abc_123/", "/Docker/docker-registry"} + invalidFiles := []string{ + "", + "/", + "abc", + "123.abc", + "//bcd", + "/abc_123/", + "/Docker/docker-registry"} for _, filename := range invalidFiles { err := suite.StorageDriver.PutContent(filename, contents)