From 5ca44e7c2f4a3df26be9858fbfb3ca761856efc1 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Wed, 4 Oct 2023 15:52:03 -0700 Subject: [PATCH 1/2] fix: prevent directory type object uploads containing data Since objects with trailing "/" are mapped to directories in the posix filesystem, they must not contain data since there is no place to store that data. This checks both PutObject and CreateMultipartUpload for invalid directory object types containing data. --- backend/posix/posix.go | 13 +++++++++++++ s3err/s3err.go | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index cebfa45..ae2a715 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -223,6 +223,12 @@ func (p *Posix) CreateMultipartUpload(_ context.Context, mpu *s3.CreateMultipart return nil, fmt.Errorf("stat bucket: %w", err) } + if strings.HasSuffix(*mpu.Key, "/") { + // directory objects can't be uploaded with mutlipart uploads + // because posix directories can't contain data + return nil, s3err.GetAPIError(s3err.ErrDirectoryObjectContainsData) + } + // generate random uuid for upload id uploadID := uuid.New().String() // hash object name for multipart container @@ -960,6 +966,13 @@ func (p *Posix) PutObject(ctx context.Context, po *s3.PutObjectInput) (string, e if strings.HasSuffix(*po.Key, "/") { // object is directory + if po.ContentLength != 0 { + // posix directories can't contain data, send error + // if reuests has a data payload associated with a + // directory object + return "", s3err.GetAPIError(s3err.ErrDirectoryObjectContainsData) + } + err = mkdirAll(name, os.FileMode(0755), *po.Bucket, *po.Key) if err != nil { return "", err diff --git a/s3err/s3err.go b/s3err/s3err.go index 82e791f..838ee0f 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -115,6 +115,7 @@ const ( // Non-AWS errors ErrExistingObjectIsDirectory ErrObjectParentIsFile + ErrDirectoryObjectContainsData ) var errorCodeResponse = map[ErrorCode]APIError{ @@ -408,6 +409,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "Object parent already exists as a file.", HTTPStatusCode: http.StatusConflict, }, + ErrDirectoryObjectContainsData: { + Code: "DirectoryObjectContainsData", + Description: "Directory object contains data payload.", + HTTPStatusCode: http.StatusBadRequest, + }, } // GetAPIError provides API Error for input API error code. From 23281774aad17ee8a8c7588000d9a097829ee373 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 7 Oct 2023 15:57:31 -0700 Subject: [PATCH 2/2] fix: allow posix GET of 0-len directory type object --- backend/common.go | 4 ++-- backend/posix/posix.go | 51 +++++++++++++++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/backend/common.go b/backend/common.go index aa11d14..b4ebf91 100644 --- a/backend/common.go +++ b/backend/common.go @@ -61,9 +61,9 @@ var ( // ParseRange parses input range header and returns startoffset, length, and // error. If no endoffset specified, then length is set to -1. -func ParseRange(file fs.FileInfo, acceptRange string) (int64, int64, error) { +func ParseRange(fi fs.FileInfo, acceptRange string) (int64, int64, error) { if acceptRange == "" { - return 0, file.Size(), nil + return 0, fi.Size(), nil } rangeKv := strings.Split(acceptRange, "=") diff --git a/backend/posix/posix.go b/backend/posix/posix.go index ae2a715..91f7782 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1127,10 +1127,6 @@ func (p *Posix) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput) func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput, writer io.Writer) (*s3.GetObjectOutput, error) { bucket := *input.Bucket - object := *input.Key - acceptRange := *input.Range - var contentRange string - _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) @@ -1139,6 +1135,7 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput, writer io return nil, fmt.Errorf("stat bucket: %w", err) } + object := *input.Key objPath := filepath.Join(bucket, object) fi, err := os.Stat(objPath) if errors.Is(err, fs.ErrNotExist) { @@ -1148,21 +1145,59 @@ func (p *Posix) GetObject(_ context.Context, input *s3.GetObjectInput, writer io return nil, fmt.Errorf("stat object: %w", err) } + acceptRange := *input.Range startOffset, length, err := backend.ParseRange(fi, acceptRange) if err != nil { return nil, err } - if length == -1 { - length = fi.Size() - startOffset + 1 + objSize := fi.Size() + if fi.IsDir() { + // directory objects are always 0 len + objSize = 0 + length = 0 } - if startOffset+length > fi.Size()+1 { + if length == -1 { + length = objSize - startOffset + 1 + } + + if startOffset+length > objSize+1 { return nil, s3err.GetAPIError(s3err.ErrInvalidRange) } + var contentRange string if acceptRange != "" { - contentRange = fmt.Sprintf("bytes %v-%v/%v", startOffset, startOffset+length-1, fi.Size()) + contentRange = fmt.Sprintf("bytes %v-%v/%v", startOffset, startOffset+length-1, objSize) + } + + if fi.IsDir() { + userMetaData := make(map[string]string) + + contentType, contentEncoding := loadUserMetaData(objPath, userMetaData) + + b, err := xattr.Get(objPath, etagkey) + etag := string(b) + if err != nil { + etag = "" + } + + tags, err := p.getXattrTags(bucket, object) + if err != nil { + return nil, fmt.Errorf("get object tags: %w", err) + } + + return &s3.GetObjectOutput{ + AcceptRanges: &acceptRange, + ContentLength: length, + ContentEncoding: &contentEncoding, + ContentType: &contentType, + ETag: &etag, + LastModified: backend.GetTimePtr(fi.ModTime()), + Metadata: userMetaData, + TagCount: int32(len(tags)), + ContentRange: &contentRange, + }, nil } f, err := os.Open(objPath)