From 8a1889efeb0853fef93b3d5d9fbf84e206633d87 Mon Sep 17 00:00:00 2001 From: Brian Bland Date: Thu, 11 Dec 2014 14:11:47 -0800 Subject: [PATCH] Enforces a path format for storage drivers (#819) Requires all paths in the inmemory and filesystem drivers to begin with a slash, and then contain only valid path components (2+ alphanumeric characters with optional period, hyphen, and underscore separators) delimited by slashes. Also updates the storage driver test suites to construct paths of this format, and causes the suite to abort if files are not cleaned up after the test run. --- storagedriver/filesystem/driver.go | 34 +++++++++++++++++++ storagedriver/inmemory/driver.go | 34 +++++++++++++++++++ storagedriver/storagedriver.go | 21 ++++++++++++ storagedriver/testsuites/testsuites.go | 46 ++++++++++++++++++-------- 4 files changed, 122 insertions(+), 13 deletions(-) diff --git a/storagedriver/filesystem/driver.go b/storagedriver/filesystem/driver.go index 3e352125f..49a94a507 100644 --- a/storagedriver/filesystem/driver.go +++ b/storagedriver/filesystem/driver.go @@ -56,6 +56,10 @@ func New(rootDirectory string) *Driver { // GetContent retrieves the content stored at "path" as a []byte. func (d *Driver) GetContent(path string) ([]byte, error) { + if !storagedriver.PathRegexp.MatchString(path) { + return nil, storagedriver.InvalidPathError{Path: path} + } + rc, err := d.ReadStream(path, 0) if err != nil { return nil, err @@ -72,6 +76,10 @@ func (d *Driver) GetContent(path string) ([]byte, error) { // PutContent stores the []byte content at a location designated by "path". func (d *Driver) PutContent(subPath string, contents []byte) error { + if !storagedriver.PathRegexp.MatchString(subPath) { + return storagedriver.InvalidPathError{Path: subPath} + } + if _, err := d.WriteStream(subPath, 0, bytes.NewReader(contents)); err != nil { return err } @@ -82,6 +90,10 @@ func (d *Driver) PutContent(subPath string, contents []byte) error { // ReadStream retrieves an io.ReadCloser for the content stored at "path" with a // given byte offset. func (d *Driver) ReadStream(path string, offset int64) (io.ReadCloser, error) { + if !storagedriver.PathRegexp.MatchString(path) { + return nil, storagedriver.InvalidPathError{Path: path} + } + if offset < 0 { return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset} } @@ -110,6 +122,10 @@ func (d *Driver) ReadStream(path string, offset int64) (io.ReadCloser, error) { // WriteStream stores the contents of the provided io.Reader at a location // designated by the given path. func (d *Driver) WriteStream(subPath string, offset int64, reader io.Reader) (nn int64, err error) { + if !storagedriver.PathRegexp.MatchString(subPath) { + return 0, storagedriver.InvalidPathError{Path: subPath} + } + if offset < 0 { return 0, storagedriver.InvalidOffsetError{Path: subPath, Offset: offset} } @@ -150,6 +166,10 @@ func (d *Driver) WriteStream(subPath string, offset int64, reader io.Reader) (nn // Stat retrieves the FileInfo for the given path, including the current size // in bytes and the creation time. func (d *Driver) Stat(subPath string) (storagedriver.FileInfo, error) { + if !storagedriver.PathRegexp.MatchString(subPath) { + return nil, storagedriver.InvalidPathError{Path: subPath} + } + fullPath := d.fullPath(subPath) fi, err := os.Stat(fullPath) @@ -170,6 +190,10 @@ func (d *Driver) Stat(subPath string) (storagedriver.FileInfo, error) { // List returns a list of the objects that are direct descendants of the given // path. func (d *Driver) List(subPath string) ([]string, error) { + if !storagedriver.PathRegexp.MatchString(subPath) && subPath != "/" { + return nil, storagedriver.InvalidPathError{Path: subPath} + } + if subPath[len(subPath)-1] != '/' { subPath += "/" } @@ -196,6 +220,12 @@ func (d *Driver) List(subPath string) ([]string, error) { // Move moves an object stored at sourcePath to destPath, removing the original // object. func (d *Driver) Move(sourcePath string, destPath string) error { + if !storagedriver.PathRegexp.MatchString(sourcePath) { + return storagedriver.InvalidPathError{Path: sourcePath} + } else if !storagedriver.PathRegexp.MatchString(destPath) { + return storagedriver.InvalidPathError{Path: destPath} + } + source := d.fullPath(sourcePath) dest := d.fullPath(destPath) @@ -213,6 +243,10 @@ func (d *Driver) Move(sourcePath string, destPath string) error { // Delete recursively deletes all objects stored at "path" and its subpaths. func (d *Driver) Delete(subPath string) error { + if !storagedriver.PathRegexp.MatchString(subPath) { + return storagedriver.InvalidPathError{Path: subPath} + } + fullPath := d.fullPath(subPath) _, err := os.Stat(fullPath) diff --git a/storagedriver/inmemory/driver.go b/storagedriver/inmemory/driver.go index 841ce56c1..7481c4727 100644 --- a/storagedriver/inmemory/driver.go +++ b/storagedriver/inmemory/driver.go @@ -46,6 +46,10 @@ func New() *Driver { // GetContent retrieves the content stored at "path" as a []byte. func (d *Driver) GetContent(path string) ([]byte, error) { + if !storagedriver.PathRegexp.MatchString(path) { + return nil, storagedriver.InvalidPathError{Path: path} + } + d.mutex.RLock() defer d.mutex.RUnlock() @@ -60,6 +64,10 @@ func (d *Driver) GetContent(path string) ([]byte, error) { // PutContent stores the []byte content at a location designated by "path". func (d *Driver) PutContent(p string, contents []byte) error { + if !storagedriver.PathRegexp.MatchString(p) { + return storagedriver.InvalidPathError{Path: p} + } + d.mutex.Lock() defer d.mutex.Unlock() @@ -79,6 +87,10 @@ func (d *Driver) PutContent(p string, contents []byte) error { // ReadStream retrieves an io.ReadCloser for the content stored at "path" with a // given byte offset. func (d *Driver) ReadStream(path string, offset int64) (io.ReadCloser, error) { + if !storagedriver.PathRegexp.MatchString(path) { + return nil, storagedriver.InvalidPathError{Path: path} + } + d.mutex.RLock() defer d.mutex.RUnlock() @@ -103,6 +115,10 @@ func (d *Driver) ReadStream(path string, offset int64) (io.ReadCloser, error) { // WriteStream stores the contents of the provided io.ReadCloser at a location // designated by the given path. func (d *Driver) WriteStream(path string, offset int64, reader io.Reader) (nn int64, err error) { + if !storagedriver.PathRegexp.MatchString(path) { + return 0, storagedriver.InvalidPathError{Path: path} + } + d.mutex.Lock() defer d.mutex.Unlock() @@ -135,6 +151,10 @@ func (d *Driver) WriteStream(path string, offset int64, reader io.Reader) (nn in // Stat returns info about the provided path. func (d *Driver) Stat(path string) (storagedriver.FileInfo, error) { + if !storagedriver.PathRegexp.MatchString(path) { + return nil, storagedriver.InvalidPathError{Path: path} + } + d.mutex.RLock() defer d.mutex.RUnlock() @@ -161,6 +181,10 @@ func (d *Driver) Stat(path string) (storagedriver.FileInfo, error) { // List returns a list of the objects that are direct descendants of the given // path. func (d *Driver) List(path string) ([]string, error) { + if !storagedriver.PathRegexp.MatchString(path) && path != "/" { + return nil, storagedriver.InvalidPathError{Path: path} + } + normalized := normalize(path) found := d.root.find(normalized) @@ -188,6 +212,12 @@ func (d *Driver) List(path string) ([]string, error) { // Move moves an object stored at sourcePath to destPath, removing the original // object. func (d *Driver) Move(sourcePath string, destPath string) error { + if !storagedriver.PathRegexp.MatchString(sourcePath) { + return storagedriver.InvalidPathError{Path: sourcePath} + } else if !storagedriver.PathRegexp.MatchString(destPath) { + return storagedriver.InvalidPathError{Path: destPath} + } + d.mutex.Lock() defer d.mutex.Unlock() @@ -204,6 +234,10 @@ func (d *Driver) Move(sourcePath string, destPath string) error { // Delete recursively deletes all objects stored at "path" and its subpaths. func (d *Driver) Delete(path string) error { + if !storagedriver.PathRegexp.MatchString(path) { + return storagedriver.InvalidPathError{Path: path} + } + d.mutex.Lock() defer d.mutex.Unlock() diff --git a/storagedriver/storagedriver.go b/storagedriver/storagedriver.go index 339b465a7..f86e3d1eb 100644 --- a/storagedriver/storagedriver.go +++ b/storagedriver/storagedriver.go @@ -3,6 +3,7 @@ package storagedriver import ( "fmt" "io" + "regexp" "strconv" "strings" ) @@ -72,6 +73,17 @@ type StorageDriver interface { Delete(path string) error } +// PathComponentRegexp is the regular expression which each repository path +// component must match. +// A component of a repository path must be at least two characters, optionally +// separated by periods, dashes or underscores. +var PathComponentRegexp = regexp.MustCompile(`[a-z0-9]+([._-]?[a-z0-9])+`) + +// PathRegexp is the regular expression which each repository path must match. +// A repository path is absolute, beginning with a slash and containing a +// positive number of path components separated by slashes. +var PathRegexp = regexp.MustCompile(`^(/[a-z0-9]+([._-]?[a-z0-9])+)+$`) + // PathNotFoundError is returned when operating on a nonexistent path. type PathNotFoundError struct { Path string @@ -81,6 +93,15 @@ func (err PathNotFoundError) Error() string { return fmt.Sprintf("Path not found: %s", err.Path) } +// InvalidPathError is returned when the provided path is malformed. +type InvalidPathError struct { + Path string +} + +func (err InvalidPathError) Error() string { + return fmt.Sprintf("Invalid path: %s", err.Path) +} + // InvalidOffsetError is returned when attempting to read or write from an // invalid offset. type InvalidOffsetError struct { diff --git a/storagedriver/testsuites/testsuites.go b/storagedriver/testsuites/testsuites.go index 16c8cd0e9..8dcc29537 100644 --- a/storagedriver/testsuites/testsuites.go +++ b/storagedriver/testsuites/testsuites.go @@ -108,6 +108,16 @@ func (suite *DriverSuite) TearDownSuite(c *check.C) { } } +// TearDownTest tears down the gocheck test. +// This causes the suite to abort if any files are left around in the storage +// driver. +func (suite *DriverSuite) TearDownTest(c *check.C) { + files, _ := suite.StorageDriver.List("/") + if len(files) > 0 { + c.Fatalf("Storage driver did not clean up properly. Offending files: %#v", files) + } +} + // TestWriteRead1 tests a simple write-read workflow. func (suite *DriverSuite) TestWriteRead1(c *check.C) { filename := randomPath(32) @@ -337,7 +347,7 @@ func (suite *DriverSuite) TestContinueStreamAppend(c *check.C) { c.Assert(fi.Size(), check.Equals, int64(len(contentsChunk1))) if fi.Size() > chunkSize { - c.Fatalf("Offset too large, %d > %d", fi.Size(), chunkSize) + c.Errorf("Offset too large, %d > %d", fi.Size(), chunkSize) } nn, err = suite.StorageDriver.WriteStream(filename, fi.Size(), bytes.NewReader(contentsChunk2)) c.Assert(err, check.IsNil) @@ -349,7 +359,7 @@ func (suite *DriverSuite) TestContinueStreamAppend(c *check.C) { c.Assert(fi.Size(), check.Equals, 2*chunkSize) if fi.Size() > 2*chunkSize { - c.Fatalf("Offset too large, %d > %d", fi.Size(), 2*chunkSize) + c.Errorf("Offset too large, %d > %d", fi.Size(), 2*chunkSize) } nn, err = suite.StorageDriver.WriteStream(filename, fi.Size(), bytes.NewReader(fullContents[fi.Size():])) @@ -409,7 +419,7 @@ func (suite *DriverSuite) TestReadNonexistentStream(c *check.C) { // TestList checks the returned list of keys after populating a directory tree. func (suite *DriverSuite) TestList(c *check.C) { rootDirectory := "/" + randomFilename(int64(8+rand.Intn(8))) - defer suite.StorageDriver.Delete("/") + defer suite.StorageDriver.Delete(rootDirectory) parentDirectory := rootDirectory + "/" + randomFilename(int64(8+rand.Intn(8))) childFiles := make([]string, 50) @@ -625,11 +635,11 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { c.Assert(fi.IsDir(), check.Equals, false) if start.After(fi.ModTime()) { - c.Fatalf("modtime %s before file created (%v)", fi.ModTime(), start) + c.Errorf("modtime %s before file created (%v)", fi.ModTime(), start) } if fi.ModTime().After(expectedModTime) { - c.Fatalf("modtime %s after file created (%v)", fi.ModTime(), expectedModTime) + c.Errorf("modtime %s after file created (%v)", fi.ModTime(), expectedModTime) } // Call on directory @@ -643,11 +653,11 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { c.Assert(fi.IsDir(), check.Equals, true) if start.After(fi.ModTime()) { - c.Fatalf("modtime %s before file created (%v)", fi.ModTime(), start) + c.Errorf("modtime %s before file created (%v)", fi.ModTime(), start) } if fi.ModTime().After(expectedModTime) { - c.Fatalf("modtime %s after file created (%v)", fi.ModTime(), expectedModTime) + c.Errorf("modtime %s after file created (%v)", fi.ModTime(), expectedModTime) } } @@ -779,16 +789,19 @@ func (suite *DriverSuite) writeReadCompareStreams(c *check.C, filename string, c } var filenameChars = []byte("abcdefghijklmnopqrstuvwxyz0123456789") +var separatorChars = []byte("._-") func randomPath(length int64) string { - path := "" + path := "/" for int64(len(path)) < length { - chunkLength := rand.Int63n(length-int64(len(path))) + 1 + chunkLength := rand.Int63n(length-int64(len(path)+1)) + 2 chunk := randomFilename(chunkLength) path += chunk if length-int64(len(path)) == 1 { path += randomFilename(1) - } else if length-int64(len(path)) > 1 { + } else if length-int64(len(path)) == 2 { + path += randomFilename(2) + } else if length-int64(len(path)) > 2 { path += "/" } } @@ -797,8 +810,15 @@ func randomPath(length int64) string { func randomFilename(length int64) string { b := make([]byte, length) + wasSeparator := true for i := range b { - b[i] = filenameChars[rand.Intn(len(filenameChars))] + if !wasSeparator && i < len(b)-1 && rand.Intn(4) == 0 { + b[i] = separatorChars[rand.Intn(len(separatorChars))] + wasSeparator = true + } else { + b[i] = filenameChars[rand.Intn(len(filenameChars))] + wasSeparator = false + } } return string(b) } @@ -821,8 +841,8 @@ func firstPart(filePath string) string { if dir == "" && file == "" { return "/" } - if dir == "" { - return file + if dir == "/" || dir == "" { + return "/" + file } if file == "" { return dir