Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-applying Sanitize path names #460

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Re-applying Sanitize path names #460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions s3_file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
})
Expand Down
85 changes: 62 additions & 23 deletions s3_fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"path"
"path/filepath"
"regexp"
"strings"
"time"

Expand All @@ -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
Expand All @@ -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,
}
}

Expand All @@ -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{}),
}
Expand All @@ -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
}
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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),
})
Expand All @@ -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
Expand All @@ -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),
})
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
18 changes: 9 additions & 9 deletions s3_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down