Merge pull request #130 from stevvooe/path-names-escaped

Prefix non-name path components

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)