diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 22105e7d..a5994ec9 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -51,12 +51,7 @@ const ( metaTmpMultipartDir = metaTmpDir + "/multipart" onameAttr = "user.objname" tagHdr = "X-Amz-Tagging" - dirObjKey = "user.dirisobject" -) - -var ( - newObjUID = 0 - newObjGID = 0 + emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" ) func New(rootdir string) (*Posix, error) { @@ -309,15 +304,6 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ return nil, fmt.Errorf("set etag attr: %w", err) } - if newObjUID != 0 || newObjGID != 0 { - err = os.Chown(objname, newObjUID, newObjGID) - if err != nil { - // cleanup object if returning error - os.Remove(objname) - return nil, fmt.Errorf("set object uid/gid: %w", err) - } - } - // cleanup tmp dirs os.RemoveAll(upiddir) // use Remove for objdir in case there are still other uploads @@ -438,12 +424,6 @@ func mkdirAll(path string, perm os.FileMode, bucket, object string) error { } return s3err.GetAPIError(s3err.ErrObjectParentIsFile) } - if newObjUID != 0 || newObjGID != 0 { - err = os.Chown(path, newObjUID, newObjGID) - if err != nil { - return fmt.Errorf("set parent ownership: %w", err) - } - } return nil } @@ -737,13 +717,10 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { xattr.Set(name, "user."+k, []byte(v)) } - // set our attribute that this dir was specifically put - xattr.Set(name, dirObjKey, nil) + // set etag attribute to signify this dir was specifically put + xattr.Set(name, "user.etag", []byte(emptyMD5)) - // TODO: what etag should be returned here - // and we should set etag xattr to identify dir was - // specifically uploaded - return "", nil + return emptyMD5, nil } // object is file @@ -781,13 +758,6 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { etag := hex.EncodeToString(dataSum[:]) xattr.Set(name, "user.etag", []byte(etag)) - if newObjUID != 0 || newObjGID != 0 { - err = os.Chown(name, newObjUID, newObjGID) - if err != nil { - return "", fmt.Errorf("set object uid/gid: %v", err) - } - } - return etag, nil } @@ -826,7 +796,7 @@ func (p *Posix) removeParents(bucket, object string) error { break } - _, err := xattr.Get(parent, dirObjKey) + _, err := xattr.Get(parent, "user.etag") if err == nil { break } @@ -1015,7 +985,20 @@ func (p *Posix) ListObjects(bucket, prefix, marker, delim string, maxkeys int) ( } fileSystem := os.DirFS(bucket) - results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys) + results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys, + func(path string) (bool, error) { + _, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + if isNoAttr(err) { + return false, nil + } + if err != nil { + return false, err + } + return true, nil + }, func(path string) (string, error) { + etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + return string(etag), err + }, []string{metaTmpDir}) if err != nil { return nil, fmt.Errorf("walk %v: %w", bucket, err) } @@ -1043,7 +1026,20 @@ func (p *Posix) ListObjectsV2(bucket, prefix, marker, delim string, maxkeys int) } fileSystem := os.DirFS(bucket) - results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys) + results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys, + func(path string) (bool, error) { + _, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + if isNoAttr(err) { + return false, nil + } + if err != nil { + return false, err + } + return true, nil + }, func(path string) (string, error) { + etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + return string(etag), err + }, []string{metaTmpDir}) if err != nil { return nil, fmt.Errorf("walk %v: %w", bucket, err) } diff --git a/backend/walk.go b/backend/walk.go index 50d11f23..bd6ed172 100644 --- a/backend/walk.go +++ b/backend/walk.go @@ -31,9 +31,12 @@ type WalkResults struct { NextMarker string } +type DirObjCheck func(path string) (bool, error) +type GetETag func(path string) (string, error) + // Walk walks the supplied fs.FS and returns results compatible with list // objects responses -func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResults, error) { +func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int, dirchk DirObjCheck, getetag GetETag, skipdirs []string) (WalkResults, error) { cpmap := make(map[string]struct{}) var objects []types.Object @@ -63,6 +66,10 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu return nil } + if contains(d.Name(), skipdirs) { + return fs.SkipDir + } + // If prefix is defined and the directory does not match prefix, // do not descend into the directory because nothing will // match this prefix. Make sure to append the / at the end of @@ -76,8 +83,35 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu return fs.SkipDir } - // TODO: special case handling if directory is empty - // and was "PUT" explicitly + // TODO: can we do better here rather than a second readdir + // per directory? + ents, err := fs.ReadDir(fileSystem, path) + if err != nil { + return fmt.Errorf("readdir %q: %w", path, err) + } + if len(ents) == 0 { + dirobj, err := dirchk(path) + if err != nil { + return fmt.Errorf("directory object check %q: %w", path, err) + } + if dirobj { + fi, err := d.Info() + if err != nil { + return fmt.Errorf("dir info %q: %w", path, err) + } + etag, err := getetag(path) + if err != nil { + return fmt.Errorf("get etag %q: %w", path, err) + } + path := path + "/" + objects = append(objects, types.Object{ + ETag: &etag, + Key: &path, + LastModified: GetTimePtr(fi.ModTime()), + }) + } + } + return nil } @@ -100,16 +134,22 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu if err != nil { return fmt.Errorf("get info for %v: %w", path, err) } + etag, err := getetag(path) + if err != nil { + return fmt.Errorf("get etag %q: %w", path, err) + } objects = append(objects, types.Object{ - ETag: new(string), + ETag: &etag, Key: &path, LastModified: GetTimePtr(fi.ModTime()), Size: fi.Size(), }) + if max > 0 && (len(objects)+len(cpmap)) == max { pastMax = true } + return nil } @@ -133,7 +173,7 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu // these are all rolled up into the common prefix. // Note: The delimeter can be anything, so we have to operate on // the full path without any assumptions on posix directory heirarchy - // here. Usually the delimeter with be "/", but thats not required. + // here. Usually the delimeter will be "/", but thats not required. suffix := strings.TrimPrefix(path, prefix) before, _, found := strings.Cut(suffix, delimiter) if !found { @@ -141,8 +181,12 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu if err != nil { return fmt.Errorf("get info for %v: %w", path, err) } + etag, err := getetag(path) + if err != nil { + return fmt.Errorf("get etag %q: %w", path, err) + } objects = append(objects, types.Object{ - ETag: new(string), + ETag: &etag, Key: &path, LastModified: GetTimePtr(fi.ModTime()), Size: fi.Size(), @@ -187,3 +231,12 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu NextMarker: newMarker, }, nil } + +func contains(a string, strs []string) bool { + for _, s := range strs { + if s == a { + return true + } + } + return false +} diff --git a/backend/walk_test.go b/backend/walk_test.go index 8cabde4b..35bdb687 100644 --- a/backend/walk_test.go +++ b/backend/walk_test.go @@ -26,31 +26,50 @@ import ( type walkTest struct { fsys fs.FS expected backend.WalkResults + dc backend.DirObjCheck } +func gettag(string) (string, error) { return "myetag", nil } + func TestWalk(t *testing.T) { - tests := []walkTest{{ - // test case from - // https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html - fsys: fstest.MapFS{ - "sample.jpg": {}, - "photos/2006/January/sample.jpg": {}, - "photos/2006/February/sample2.jpg": {}, - "photos/2006/February/sample3.jpg": {}, - "photos/2006/February/sample4.jpg": {}, + tests := []walkTest{ + { + // test case from + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html + fsys: fstest.MapFS{ + "sample.jpg": {}, + "photos/2006/January/sample.jpg": {}, + "photos/2006/February/sample2.jpg": {}, + "photos/2006/February/sample3.jpg": {}, + "photos/2006/February/sample4.jpg": {}, + }, + expected: backend.WalkResults{ + CommonPrefixes: []types.CommonPrefix{{ + Prefix: backend.GetStringPtr("photos/"), + }}, + Objects: []types.Object{{ + Key: backend.GetStringPtr("sample.jpg"), + }}, + }, + dc: func(string) (bool, error) { return false, nil }, }, - expected: backend.WalkResults{ - CommonPrefixes: []types.CommonPrefix{{ - Prefix: backend.GetStringPtr("photos/"), - }}, - Objects: []types.Object{{ - Key: backend.GetStringPtr("sample.jpg"), - }}, + { + // test case single dir/single file + fsys: fstest.MapFS{ + "test/file": {}, + }, + expected: backend.WalkResults{ + CommonPrefixes: []types.CommonPrefix{{ + Prefix: backend.GetStringPtr("test/"), + }}, + Objects: []types.Object{}, + }, + dc: func(string) (bool, error) { return true, nil }, }, - }} + } for _, tt := range tests { - res, err := backend.Walk(tt.fsys, "", "/", "", 1000) + res, err := backend.Walk(tt.fsys, "", "/", "", 1000, tt.dc, gettag, []string{}) if err != nil { t.Fatalf("walk: %v", err) } @@ -67,13 +86,16 @@ func compareResults(got, wanted backend.WalkResults, t *testing.T) { } if !compareObjects(got.Objects, wanted.Objects) { - t.Errorf("unexpected common prefix, got %v wanted %v", + t.Errorf("unexpected object, got %v wanted %v", printObjects(got.Objects), printObjects(wanted.Objects)) } } func compareCommonPrefix(a, b []types.CommonPrefix) bool { + if len(a) == 0 && len(b) == 0 { + return true + } if len(a) != len(b) { return false } @@ -108,6 +130,9 @@ func printCommonPrefixes(list []types.CommonPrefix) string { } func compareObjects(a, b []types.Object) bool { + if len(a) == 0 && len(b) == 0 { + return true + } if len(a) != len(b) { return false } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index e51c0f59..862478a2 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -151,9 +151,9 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { } func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { - dstBucket := ctx.Params("bucket") - dstKeyStart := ctx.Params("key") - dstKeyEnd := ctx.Params("*1") + bucket := ctx.Params("bucket") + keyStart := ctx.Params("key") + keyEnd := ctx.Params("*1") uploadId := ctx.Query("uploadId") partNumberStr := ctx.Query("partNumber") @@ -177,8 +177,12 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { grants := grantFullControl + grantRead + grantReadACP + granWrite + grantWriteACP - if dstKeyEnd != "" { - dstKeyStart = strings.Join([]string{dstKeyStart, dstKeyEnd}, "/") + if keyEnd != "" { + keyStart = strings.Join([]string{keyStart, keyEnd}, "/") + } + path := ctx.Path() + if path[len(path)-1:] == "/" && keyStart[len(keyStart)-1:] != "/" { + keyStart = keyStart + "/" } if partNumberStr != "" { @@ -198,8 +202,8 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { } res, err := c.be.UploadPartCopy(&s3.UploadPartCopyInput{ - Bucket: &dstBucket, - Key: &dstKeyStart, + Bucket: &bucket, + Key: &keyStart, PartNumber: int32(partNumber), UploadId: &uploadId, CopySource: ©Source, @@ -214,7 +218,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { if uploadId != "" { body := io.ReadSeeker(bytes.NewReader([]byte(ctx.Body()))) - res, err := c.be.UploadPart(dstBucket, dstKeyStart, uploadId, body) + res, err := c.be.UploadPart(bucket, keyStart, uploadId, body) return Responce(ctx, res, err) } @@ -224,8 +228,8 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { } err := c.be.PutObjectAcl(&s3.PutObjectAclInput{ - Bucket: &dstBucket, - Key: &dstKeyStart, + Bucket: &bucket, + Key: &keyStart, ACL: types.ObjectCannedACL(acl), GrantFullControl: &grantFullControl, GrantRead: &grantRead, @@ -240,7 +244,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { copySourceSplit := strings.Split(copySource, "/") srcBucket, srcObject := copySourceSplit[0], copySourceSplit[1:] - res, err := c.be.CopyObject(srcBucket, strings.Join(srcObject, "/"), dstBucket, dstKeyStart) + res, err := c.be.CopyObject(srcBucket, strings.Join(srcObject, "/"), bucket, keyStart) return Responce(ctx, res, err) } @@ -252,8 +256,8 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { metadata := utils.GetUserMetaData(&ctx.Request().Header) res, err := c.be.PutObject(&s3.PutObjectInput{ - Bucket: &dstBucket, - Key: &dstKeyStart, + Bucket: &bucket, + Key: &keyStart, ContentLength: contentLength, Metadata: metadata, Body: bytes.NewReader(ctx.Request().Body()),