Revert "Revert "Sanitize path names" (#458)"

This reverts commit 0df1df1.

.github/workflows/build.yml

@@ -37,7 +37,7 @@ jobs:
         if: matrix.lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.45
+          version: v1.43
 
       # Install Go
       - name: Setup go

s3_file.go

@@ -75,9 +75,9 @@ func (f *File) Readdir(n int) ([]os.FileInfo, error) {
 	if name != "" && !strings.HasSuffix(name, "/") {
 		name += "/"
 	}
-	output, err := f.fs.s3API.ListObjectsV2(&s3.ListObjectsV2Input{
+	output, err := f.fs.S3API.ListObjectsV2(&s3.ListObjectsV2Input{
 		ContinuationToken: f.readdirContinuationToken,
-		Bucket:            aws.String(f.fs.bucket),
+		Bucket:            aws.String(f.fs.Bucket),
 		Prefix:            aws.String(name),
 		Delimiter:         aws.String("/"),
 		MaxKeys:           aws.Int64(int64(n)),
@@ -213,6 +213,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 {
@@ -304,12 +308,12 @@ func (f *File) openWriteStream() error {
 	f.streamWriteCloseErr = make(chan error)
 	f.streamWrite = writer
 
-	uploader := s3manager.NewUploader(f.fs.session)
+	uploader := s3manager.NewUploader(f.fs.Session)
 	uploader.Concurrency = 1
 
 	go func() {
 		input := &s3manager.UploadInput{
-			Bucket: aws.String(f.fs.bucket),
+			Bucket: aws.String(f.fs.Bucket),
 			Key:    aws.String(f.name),
 			Body:   reader,
 		}
@@ -347,8 +351,8 @@ func (f *File) openReadStream(startAt int64) error {
 		streamRange = aws.String(fmt.Sprintf("bytes=%d-%d", startAt, f.cachedInfo.Size()))
 	}
 
-	resp, err := f.fs.s3API.GetObject(&s3.GetObjectInput{
-		Bucket: aws.String(f.fs.bucket),
+	resp, err := f.fs.S3API.GetObject(&s3.GetObjectInput{
+		Bucket: aws.String(f.fs.Bucket),
 		Key:    aws.String(f.name),
 		Range:  streamRange,
 	})

s3_fs.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
@@ -23,9 +24,10 @@ import (
 // Fs is an FS object backed by S3.
 type Fs struct {
 	FileProps *UploadedFileProperties // FileProps define the file properties we want to set for all new files
-	session   *session.Session        // Session config
-	s3API     *s3.S3
-	bucket    string // Bucket name
+	Session   *session.Session        // Session config
+	S3API     *s3.S3
+	Bucket    string // Bucket name
+	RawMode   bool   // Controls path sanitation.
 }
 
 // UploadedFileProperties defines all the set properties applied to future files
@@ -39,9 +41,9 @@ type UploadedFileProperties struct {
 func NewFs(bucket string, session *session.Session) *Fs {
 	s3Api := s3.New(session)
 	return &Fs{
-		bucket:  bucket,
-		session: session,
-		s3API:   s3Api,
+		Bucket:  bucket,
+		Session: session,
+		S3API:   s3Api,
 	}
 }
 
@@ -64,7 +66,7 @@ func (Fs) Name() string { return "s3" }
 func (fs Fs) Create(name string) (afero.File, error) {
 	{ // It's faster to trigger an explicit empty put object than opening a file for write, closing it and re-opening it
 		req := &s3.PutObjectInput{
-			Bucket: aws.String(fs.bucket),
+			Bucket: aws.String(fs.Bucket),
 			Key:    aws.String(name),
 			Body:   bytes.NewReader([]byte{}),
 		}
@@ -78,7 +80,7 @@ func (fs Fs) Create(name string) (afero.File, error) {
 			req.ContentType = aws.String(mime.TypeByExtension(filepath.Ext(name)))
 		}
 
-		_, errPut := fs.s3API.PutObject(req)
+		_, errPut := fs.S3API.PutObject(req)
 		if errPut != nil {
 			return nil, errPut
 		}
@@ -92,14 +94,15 @@ func (fs Fs) Create(name string) (afero.File, error) {
 	// Create(), like all of S3, is eventually consistent.
 	// To protect against unexpected behavior, have this method
 	// wait until S3 reports the object exists.
-	return file, fs.s3API.WaitUntilObjectExists(&s3.HeadObjectInput{
-		Bucket: aws.String(fs.bucket),
+	return file, fs.S3API.WaitUntilObjectExists(&s3.HeadObjectInput{
+		Bucket: aws.String(fs.Bucket),
 		Key:    aws.String(name),
 	})
 }
 
 // Mkdir makes a directory in S3.
 func (fs Fs) Mkdir(name string, perm os.FileMode) error {
+	name = fs.sanitize(name)
 	file, err := fs.OpenFile(fmt.Sprintf("%s/", path.Clean(name)), os.O_CREATE, perm)
 	if err == nil {
 		err = file.Close()
@@ -114,11 +117,13 @@ func (fs Fs) MkdirAll(path string, perm os.FileMode) error {
 
 // Open a file for reading.
 func (fs *Fs) Open(name string) (afero.File, error) {
+	name = fs.sanitize(name)
 	return fs.OpenFile(name, os.O_RDONLY, 0777)
 }
 
 // OpenFile opens a file.
 func (fs *Fs) OpenFile(name string, flag int, _ os.FileMode) (afero.File, error) {
+	name = fs.sanitize(name)
 	file := NewFile(fs, name)
 
 	// Reading and writing is technically supported but can't lead to anything that makes sense
@@ -160,6 +165,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 = fs.sanitize(name)
 	if _, err := fs.Stat(name); err != nil {
 		return err
 	}
@@ -168,15 +174,16 @@ func (fs Fs) Remove(name string) error {
 
 // forceRemove doesn't error if a file does not exist.
 func (fs Fs) forceRemove(name string) error {
-	_, err := fs.s3API.DeleteObject(&s3.DeleteObjectInput{
-		Bucket: aws.String(fs.bucket),
+	_, err := fs.S3API.DeleteObject(&s3.DeleteObjectInput{
+		Bucket: aws.String(fs.Bucket),
 		Key:    aws.String(name),
 	})
 	return err
 }
 
 // RemoveAll removes a path.
 func (fs *Fs) RemoveAll(name string) error {
+	name = fs.sanitize(name)
 	s3dir := NewFile(fs, name)
 	fis, err := s3dir.Readdir(0)
 	if err != nil {
@@ -206,19 +213,22 @@ 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 = fs.sanitize(newname)
+	newname = fs.sanitize(oldname)
+
 	if oldname == newname {
 		return nil
 	}
-	_, err := fs.s3API.CopyObject(&s3.CopyObjectInput{
-		Bucket:     aws.String(fs.bucket),
-		CopySource: aws.String(fs.bucket + oldname),
+	_, err := fs.S3API.CopyObject(&s3.CopyObjectInput{
+		Bucket:     aws.String(fs.Bucket),
+		CopySource: aws.String(fs.Bucket + oldname),
 		Key:        aws.String(newname),
 	})
 	if err != nil {
 		return err
 	}
-	_, err = fs.s3API.DeleteObject(&s3.DeleteObjectInput{
-		Bucket: aws.String(fs.bucket),
+	_, err = fs.S3API.DeleteObject(&s3.DeleteObjectInput{
+		Bucket: aws.String(fs.Bucket),
 		Key:    aws.String(oldname),
 	})
 	return err
@@ -227,8 +237,9 @@ 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) {
-	out, err := fs.s3API.HeadObject(&s3.HeadObjectInput{
-		Bucket: aws.String(fs.bucket),
+	name = fs.sanitize(name)
+	out, err := fs.S3API.HeadObject(&s3.HeadObjectInput{
+		Bucket: aws.String(fs.Bucket),
 		Key:    aws.String(name),
 	})
 	if err != nil {
@@ -260,8 +271,8 @@ func (fs Fs) Stat(name string) (os.FileInfo, error) {
 
 func (fs Fs) statDirectory(name string) (os.FileInfo, error) {
 	nameClean := path.Clean(name)
-	out, err := fs.s3API.ListObjectsV2(&s3.ListObjectsV2Input{
-		Bucket:  aws.String(fs.bucket),
+	out, err := fs.S3API.ListObjectsV2(&s3.ListObjectsV2Input{
+		Bucket:  aws.String(fs.Bucket),
 		Prefix:  aws.String(strings.TrimPrefix(nameClean, "/")),
 		MaxKeys: aws.Int64(1),
 	})
@@ -284,6 +295,7 @@ 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 = fs.sanitize(name)
 	var acl string
 
 	otherRead := mode&(1<<2) != 0
@@ -298,8 +310,8 @@ func (fs Fs) Chmod(name string, mode os.FileMode) error {
 		acl = "private"
 	}
 
-	_, err := fs.s3API.PutObjectAcl(&s3.PutObjectAclInput{
-		Bucket: aws.String(fs.bucket),
+	_, err := fs.S3API.PutObjectAcl(&s3.PutObjectAclInput{
+		Bucket: aws.String(fs.Bucket),
 		Key:    aws.String(name),
 		ACL:    aws.String(acl),
 	})
@@ -317,6 +329,14 @@ func (Fs) Chtimes(string, time.Time, time.Time) error {
 	return ErrNotSupported
 }
 
+// sanitize name if not in RawMode.
+func (fs Fs) sanitize(name string) string {
+	if fs.RawMode {
+		return name
+	}
+	return sanitize(name)
+}
+
 // I couldn't find a way to make this code cleaner. It's basically a big copy-paste on two
 // very similar structures.
 func applyFileCreateProps(req *s3.PutObjectInput, p *UploadedFileProperties) {
@@ -346,3 +366,22 @@ func applyFileWriteProps(req *s3manager.UploadInput, p *UploadedFileProperties)
 		req.ContentType = p.ContentType
 	}
 }
+
+// volumePrefixRegex matches the windows volume identifier eg "C:".
+var volumePrefixRegex = regexp.MustCompile(`^[[:alpha:]]:`)
+
+// sanitize name to ensure it uses forward slash paths even on Windows
+// systems.
+func sanitize(name string) string {
+	if strings.TrimSpace(name) == "" {
+		return name
+	}
+	name = volumePrefixRegex.ReplaceAllString(name, "")
+	name = strings.ReplaceAll(name, "\\", "/")
+	hasTrailingSlash := strings.HasSuffix(name, "/")
+	name = path.Clean(name)
+	if hasTrailingSlash {
+		name += "/"
+	}
+	return name
+}

s3_test.go

@@ -485,7 +485,7 @@ func TestBadConnection(t *testing.T) {
 	fs := __getS3Fs(t)
 
 	// Let's mess-up the config
-	fs.session.Config.Endpoint = aws.String("http://broken")
+	fs.Session.Config.Endpoint = aws.String("http://broken")
 
 	t.Run("Read", func(t *testing.T) {
 		// We will fail here because we are checking if the file exists and its type
@@ -674,8 +674,8 @@ func TestContentType(t *testing.T) {
 
 		// And we check the resulting content-type
 		for fileName, mimeType := range fileToMime {
-			resp, err := fs.s3API.GetObject(&s3.GetObjectInput{
-				Bucket: aws.String(fs.bucket),
+			resp, err := fs.S3API.GetObject(&s3.GetObjectInput{
+				Bucket: aws.String(fs.Bucket),
 				Key:    aws.String(fileName),
 			})
 			req.NoError(err)
@@ -687,8 +687,8 @@ func TestContentType(t *testing.T) {
 		_, err := fs.Create("create.png")
 		req.NoError(err)
 
-		resp, err := fs.s3API.GetObject(&s3.GetObjectInput{
-			Bucket: aws.String(fs.bucket),
+		resp, err := fs.S3API.GetObject(&s3.GetObjectInput{
+			Bucket: aws.String(fs.Bucket),
 			Key:    aws.String("create.png"),
 		})
 		req.NoError(err)
@@ -704,8 +704,8 @@ func TestContentType(t *testing.T) {
 		testCreateFile(t, fs, "custom-write", "content")
 
 		for _, name := range []string{"custom-create", "custom-write"} {
-			resp, err := fs.s3API.GetObject(&s3.GetObjectInput{
-				Bucket: aws.String(fs.bucket),
+			resp, err := fs.S3API.GetObject(&s3.GetObjectInput{
+				Bucket: aws.String(fs.Bucket),
 				Key:    aws.String(name),
 			})
 			req.NoError(err)
@@ -732,8 +732,8 @@ func TestFileProps(t *testing.T) {
 		testCreateFile(t, fs, "write", "content")
 
 		for _, name := range []string{"create", "write"} {
-			resp, err := fs.s3API.GetObject(&s3.GetObjectInput{
-				Bucket: aws.String(fs.bucket),
+			resp, err := fs.S3API.GetObject(&s3.GetObjectInput{
+				Bucket: aws.String(fs.Bucket),
 				Key:    aws.String(name),
 			})
 			req.NoError(err)