fix: sanitize path names (#11)

Sanitize the path names when on windows, to handle any usages of
filepath when the s3 FS is embedded in another FS

Implements fclairamb/afero-s3#435
and resolves fclairamb/afero-s3#434

Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>

s3_file.go

@@ -217,6 +217,10 @@ func (f *File) Close() error {
 // It returns the number of bytes read and an error, if any.
 // EOF is signaled by a zero count with err set to io.EOF.
 func (f *File) Read(p []byte) (int, error) {
+	if f.streamRead == nil {
+		return 0, io.EOF
+	}
+
 	n, err := f.streamRead.Read(p)
 
 	if err == nil {

s3_fs.go

@@ -110,6 +110,7 @@ func (fs *Fs) Create(name string) (afero.File, error) {
 
 // Mkdir makes a directory in S3.
 func (fs *Fs) Mkdir(name string, perm os.FileMode) error {
+	name = sanitize(name)
 	file, err := fs.OpenFile(fmt.Sprintf("%s/", path.Clean(name)), os.O_CREATE, perm)
 	if err == nil {
 		err = file.Close()
@@ -129,6 +130,7 @@ func (fs *Fs) Open(name string) (afero.File, error) {
 
 // OpenFile opens a file.
 func (fs *Fs) OpenFile(name string, flag int, _ os.FileMode) (afero.File, error) {
+	name = sanitize(name)
 	file := NewFile(fs, name)
 
 	// Reading and writing is technically supported but can't lead to anything that makes sense
@@ -170,6 +172,7 @@ func (fs *Fs) OpenFile(name string, flag int, _ os.FileMode) (afero.File, error)
 
 // Remove a file
 func (fs *Fs) Remove(name string) error {
+	name = sanitize(name)
 	if _, err := fs.Stat(name); err != nil {
 		return err
 	}
@@ -187,6 +190,8 @@ func (fs *Fs) forceRemove(name string) error {
 
 // RemoveAll removes a path.
 func (fs *Fs) RemoveAll(name string) error {
+	name = sanitize(name)
+
 	s3dir := NewFile(fs, name)
 	fis, err := s3dir.Readdir(0)
 	if err != nil {
@@ -216,6 +221,9 @@ func (fs *Fs) RemoveAll(name string) error {
 // will copy the file to an object with the new name and then delete
 // the original.
 func (fs *Fs) Rename(oldname, newname string) error {
+	oldname = sanitize(oldname)
+	newname = sanitize(newname)
+
 	if oldname == newname {
 		return nil
 	}
@@ -237,6 +245,8 @@ func (fs *Fs) Rename(oldname, newname string) error {
 // Stat returns a FileInfo describing the named file.
 // If there is an error, it will be of type *os.PathError.
 func (fs *Fs) Stat(name string) (os.FileInfo, error) {
+	name = sanitize(name)
+
 	// Special case because you can not HeadObject on the bucket itself.
 	if name == "/" {
 		return NewFileInfo(name, true, 0, time.Unix(0, 0)), nil
@@ -293,6 +303,8 @@ func (fs *Fs) statDirectory(name string) (os.FileInfo, error) {
 
 // Chmod doesn't exists in S3 but could be implemented by analyzing ACLs
 func (fs *Fs) Chmod(name string, mode os.FileMode) error {
+	name = sanitize(name)
+
 	var acl string
 
 	otherRead := mode&(1<<2) != 0
@@ -355,3 +367,34 @@ func applyFileWriteProps(input *s3.PutObjectInput, p *UploadedFileProperties) {
 		input.ContentType = p.ContentType
 	}
 }
+
+// sanitize name to ensure it uses forward slash paths even on Windows systems.
+func sanitize(name string) string {
+	// special case, not sure what an empty value
+	// _SHOULD_ map to, so just return it.
+	// Clean would try to map it to "."
+	if strings.TrimSpace(name) == "" {
+		return ""
+	}
+
+	// safely clean-up the path
+	out := filepath.Clean(name)
+
+	// clean will remove trailing slashes, but we want to preserve it
+	// clean _does_ not strip the leading slash, so we have a special check
+	// for the `/` case.
+	if strings.HasSuffix(name, string(filepath.Separator)) && len(out) > 1 {
+		out += string(filepath.Separator)
+	}
+
+	// On Windows: remove the volume name if it exists,
+	// e.g. remove C: from C:\path\to\file,
+	// Other OSes: a no-op
+	prefix := filepath.VolumeName(out)
+	if prefix != "" {
+		out = strings.TrimPrefix(out, prefix)
+	}
+
+	// s3 requires forward slashes
+	return filepath.ToSlash(out)
+}

s3_test.go

@@ -900,3 +900,54 @@ func TestFileInfo(t *testing.T) {
 	fi := NewFileInfo("name", false, 1024, time.Now())
 	require.Nil(t, fi.Sys())
 }
+
+func TestSanitize(t *testing.T) {
+	cases := []struct {
+		name      string
+		value     string
+		expected  string
+		separator rune
+	}{
+		{
+			name:     "empty",
+			value:    "",
+			expected: "",
+		},
+		{
+			name:     "linux path is unchanged",
+			value:    "/path/to/file",
+			expected: "/path/to/file",
+		},
+		{
+			name:     "linux path to directory is unchanged",
+			value:    "/path/to/dir/",
+			expected: "/path/to/dir/",
+		},
+		{
+			name:     "handles linux root path",
+			value:    "/",
+			expected: "/",
+		},
+		// {
+		// 	name:     "handles windows root path",
+		// 	value:    "C:\\",
+		// 	expected: "/",
+		// },
+		// {
+		// 	name:     "handles windows path",
+		// 	value:    "C:\\foo\\bar",
+		// 	expected: "/foo/bar",
+		// },
+		// {
+		// 	name:     "handles windows path with spaces",
+		// 	value:    "C:\\foo\\bar baz",
+		// 	expected: "/foo/bar baz",
+		// },
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.expected, sanitize(tc.value))
+		})
+	}
+}