diff --git a/s3_file.go b/s3_file.go index 5a25409..dccb18f 100644 --- a/s3_file.go +++ b/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 { diff --git a/s3_fs.go b/s3_fs.go index cbf68f3..50298f5 100644 --- a/s3_fs.go +++ b/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) +} diff --git a/s3_test.go b/s3_test.go index 0bb3ccd..83280be 100644 --- a/s3_test.go +++ b/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)) + }) + } +}