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

@@ -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)

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()
 

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 {

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 == "" {
+		if dir == "/" || dir == "" {
-			return file
+			return "/" + file
 		}
 		if file == "" {
 			return dir