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 <stephen.day@docker.com>

storage/paths.go

@@ -18,7 +18,7 @@ const storagePathVersion = "v2"
 //		<root>/v2
 //			-> repositories/
 // 				-><name>/
-// 					-> manifests/
+// 					-> _manifests/
 // 						revisions
 //							-> <manifest digest path>
 //								-> link
@@ -28,9 +28,9 @@ const storagePathVersion = "v2"
 //							-> current/link
 // 							-> index
 //								-> <algorithm>/<hex digest>/link
-// 					-> layers/
+// 					-> _layers/
 // 						<layer links to blob store>
-// 					-> uploads/<uuid>
+// 					-> _uploads/<uuid>
 // 						data
 // 						startedat
 //			-> blob/<algorithm>
@@ -65,27 +65,27 @@ const storagePathVersion = "v2"
 //
 //	Manifests:
 //
-// 	manifestRevisionPathSpec:      <root>/v2/repositories/<name>/manifests/revisions/<algorithm>/<hex digest>/
+// 	manifestRevisionPathSpec:      <root>/v2/repositories/<name>/_manifests/revisions/<algorithm>/<hex digest>/
-// 	manifestRevisionLinkPathSpec:  <root>/v2/repositories/<name>/manifests/revisions/<algorithm>/<hex digest>/link
+// 	manifestRevisionLinkPathSpec:  <root>/v2/repositories/<name>/_manifests/revisions/<algorithm>/<hex digest>/link
-// 	manifestSignaturesPathSpec:    <root>/v2/repositories/<name>/manifests/revisions/<algorithm>/<hex digest>/signatures/
+// 	manifestSignaturesPathSpec:    <root>/v2/repositories/<name>/_manifests/revisions/<algorithm>/<hex digest>/signatures/
-// 	manifestSignatureLinkPathSpec: <root>/v2/repositories/<name>/manifests/revisions/<algorithm>/<hex digest>/signatures/<algorithm>/<hex digest>/link
+// 	manifestSignatureLinkPathSpec: <root>/v2/repositories/<name>/_manifests/revisions/<algorithm>/<hex digest>/signatures/<algorithm>/<hex digest>/link
 //
 //	Tags:
 //
-// 	manifestTagsPathSpec:          <root>/v2/repositories/<name>/manifests/tags/
+// 	manifestTagsPathSpec:          <root>/v2/repositories/<name>/_manifests/tags/
-// 	manifestTagPathSpec:           <root>/v2/repositories/<name>/manifests/tags/<tag>/
+// 	manifestTagPathSpec:           <root>/v2/repositories/<name>/_manifests/tags/<tag>/
-// 	manifestTagCurrentPathSpec:    <root>/v2/repositories/<name>/manifests/tags/<tag>/current/link
+// 	manifestTagCurrentPathSpec:    <root>/v2/repositories/<name>/_manifests/tags/<tag>/current/link
-// 	manifestTagIndexPathSpec:      <root>/v2/repositories/<name>/manifests/tags/<tag>/index/
+// 	manifestTagIndexPathSpec:      <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/
-// 	manifestTagIndexEntryPathSpec: <root>/v2/repositories/<name>/manifests/tags/<tag>/index/<algorithm>/<hex digest>/link
+// 	manifestTagIndexEntryPathSpec: <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/<algorithm>/<hex digest>/link
 //
 // 	Layers:
 //
-// 	layerLinkPathSpec:             <root>/v2/repositories/<name>/layers/tarsum/<tarsum version>/<tarsum hash alg>/<tarsum hash>/link
+// 	layerLinkPathSpec:             <root>/v2/repositories/<name>/_layers/tarsum/<tarsum version>/<tarsum hash alg>/<tarsum hash>/link
 //
 //	Uploads:
 //
-// 	uploadDataPathSpec:             <root>/v2/repositories/<name>/uploads/<uuid>/data
+// 	uploadDataPathSpec:             <root>/v2/repositories/<name>/_uploads/<uuid>/data
-// 	uploadStartedAtPathSpec:        <root>/v2/repositories/<name>/uploads/<uuid>/startedat
+// 	uploadStartedAtPathSpec:        <root>/v2/repositories/<name>/_uploads/<uuid>/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)

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)

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.
+// PathRegexp is the regular expression which each file path must match. A
-// A file path is absolute, beginning with a slash and containing a
+// file path is absolute, beginning with a slash and containing a positive
-// positive number of path components separated by slashes, where each component
+// number of path components separated by slashes, where each component is
-// is restricted to lowercase alphanumeric characters, optionally separated by
+// restricted to lowercase alphanumeric characters or a period, underscore, or
-// a period, underscore, or hyphen.
+// hyphen.
-var PathRegexp = regexp.MustCompile(`^(/[a-z0-9]+([._-][a-z0-9]+)*)+$`)
+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")

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)