Skip to content

Commit

Permalink
Merge pull request #130 from stevvooe/path-names-escaped
Browse files Browse the repository at this point in the history 
Prefix non-name path components
  • Loading branch information
@stevvooe
stevvooe committed Feb 3, 2015
2 parents 64cdd3e + 43b3697 commit d91e4bc
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 40 deletions.
40 changes: 20 additions & 20 deletions storage/paths.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const storagePathVersion = "v2"
// <root>/v2
// -> repositories/
// -><name>/
// -> manifests/
// -> _manifests/
// revisions
// -> <manifest digest path>
// -> link
Expand All @@ -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>
Expand Down Expand Up @@ -65,27 +65,27 @@ const storagePathVersion = "v2"
//
// Manifests:
//
// manifestRevisionPathSpec: <root>/v2/repositories/<name>/manifests/revisions/<algorithm>/<hex digest>/
// manifestRevisionLinkPathSpec: <root>/v2/repositories/<name>/manifests/revisions/<algorithm>/<hex digest>/link
// 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
// manifestRevisionPathSpec: <root>/v2/repositories/<name>/_manifests/revisions/<algorithm>/<hex digest>/
// manifestRevisionLinkPathSpec: <root>/v2/repositories/<name>/_manifests/revisions/<algorithm>/<hex digest>/link
// 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
//
// Tags:
//
// manifestTagsPathSpec: <root>/v2/repositories/<name>/manifests/tags/
// manifestTagPathSpec: <root>/v2/repositories/<name>/manifests/tags/<tag>/
// manifestTagCurrentPathSpec: <root>/v2/repositories/<name>/manifests/tags/<tag>/current/link
// manifestTagIndexPathSpec: <root>/v2/repositories/<name>/manifests/tags/<tag>/index/
// manifestTagIndexEntryPathSpec: <root>/v2/repositories/<name>/manifests/tags/<tag>/index/<algorithm>/<hex digest>/link
// manifestTagsPathSpec: <root>/v2/repositories/<name>/_manifests/tags/
// manifestTagPathSpec: <root>/v2/repositories/<name>/_manifests/tags/<tag>/
// manifestTagCurrentPathSpec: <root>/v2/repositories/<name>/_manifests/tags/<tag>/current/link
// manifestTagIndexPathSpec: <root>/v2/repositories/<name>/_manifests/tags/<tag>/index/
// 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
// uploadStartedAtPathSpec: <root>/v2/repositories/<name>/uploads/<uuid>/startedat
// uploadDataPathSpec: <root>/v2/repositories/<name>/_uploads/<uuid>/data
// uploadStartedAtPathSpec: <root>/v2/repositories/<name>/_uploads/<uuid>/startedat
//
// Blob Store:
//
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down
24 changes: 12 additions & 12 deletions storage/paths_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,71 +21,71 @@ 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{
name: "foo/bar",
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{
name: "foo/bar",
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{
Expand All @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions storagedriver/storagedriver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
25 changes: 23 additions & 2 deletions storagedriver/testsuites/testsuites.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit d91e4bc

Please sign in to comment.