From bb590205bbfbbb4384209f7ca4396f979dceeb01 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Tue, 29 Sep 2026 13:41:18 +0400 Subject: [PATCH] fix: keep the last modified time of promoted object versions in posix Deleting the current version of an object by `versionId` makes the newest remaining version current again. Posix copies that version from the versioning directory into a new temporary file and links it in place of the object, but never restored the file's modification time, so `HeadObject`, `ListObjectVersions` and `ListObjectsV2` reported the delete time as the promoted version's `LastModified`. This affected an older version, a version under a deleted delete marker and the null version alike, while AWS keeps the original `LastModified` in all three cases. Directory objects were not affected, as `deleteDirObjectLatestVersion` already restores the directory's mtime. The temporary file now carries an optional modification time, set with `setModTime` and applied by `link()` right before the file is published: through `/proc/self/fd` before `linkat` for an `O_TMPFILE` file, and after the file is closed and before the rename on the `CreateTemp` fallback and on non-Linux platforms. Both the promotion in `DeleteObject` and `createObjVersion` use it. `createObjVersion` used to call `os.Chtimes` on the temporary file while it was still open for writing, and Windows resets the last write time when a handle the file was written through is closed. --- backend/posix/otmpfile_common.go | 17 ++++ backend/posix/posix.go | 10 +-- backend/posix/with_otmpfile.go | 14 +++ backend/posix/without_otmpfile.go | 9 ++ tests/integration/group-tests.go | 2 + tests/integration/versioning.go | 143 ++++++++++++++++++++++++++++++ 6 files changed, 190 insertions(+), 5 deletions(-) diff --git a/backend/posix/otmpfile_common.go b/backend/posix/otmpfile_common.go index 3f80243e..0530d9b0 100644 --- a/backend/posix/otmpfile_common.go +++ b/backend/posix/otmpfile_common.go @@ -96,6 +96,23 @@ func (tmp *tmpfile) File() *os.File { return tmp.f } +// setModTime makes link() publish the file with the modification time t +// instead of the time it was last written +func (tmp *tmpfile) setModTime(t time.Time) { + tmp.modTime = t +} + +// applyModTime sets the modification time requested with setModTime on the +// file at path. Windows updates the last write time when a handle the file +// was written through is closed, so it has to be set after that handle is +// closed. +func (tmp *tmpfile) applyModTime(path string) error { + if tmp.modTime.IsZero() { + return nil + } + return os.Chtimes(path, time.Now(), tmp.modTime) +} + func sleepWithJitter(backoffMs int) { if backoffMs <= 1 { time.Sleep(1 * time.Millisecond) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 80d57e8f..6cc994f0 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1496,11 +1496,8 @@ func (p *Posix) createObjVersion(bucket, key string, size int64, acc auth.Accoun } } - // Restore original mtime after copy - err = os.Chtimes(f.File().Name(), time.Now(), originalMTime) - if err != nil { - return versionPath, err - } + // the version keeps the last modified time of the object + f.setModTime(originalMTime) versionPath = filepath.Join(versionBucketPath, versioningKey) @@ -5328,6 +5325,9 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( return nil, fmt.Errorf("close obj version: %w", err) } + // the restored version keeps its last modified time + f.setModTime(srcObjVersion.ModTime()) + if err := f.link(); err != nil { return nil, fmt.Errorf("link tmp file: %w", err) } diff --git a/backend/posix/with_otmpfile.go b/backend/posix/with_otmpfile.go index 7afa25e6..b76610b8 100644 --- a/backend/posix/with_otmpfile.go +++ b/backend/posix/with_otmpfile.go @@ -50,6 +50,9 @@ type tmpfile struct { gid int newDirPerm fs.FileMode newFilePerm fs.FileMode + // modTime is the modification time the file is published with, + // the time it was last written if zero + modTime time.Time } // openTmpFile opens a temporary file in dir (a filesystem path) that link() @@ -286,6 +289,12 @@ func (tmp *tmpfile) link() error { return tmp.fallbackLink() } + // the unnamed file has no path until it's linked + err = tmp.applyModTime(filepath.Join(procfddir, tmp.procFDName)) + if err != nil { + return fmt.Errorf("set tmpfile modification time: %w", err) + } + procdir, err := os.Open(procfddir) if err != nil { return fmt.Errorf("open proc dir: %w", err) @@ -351,6 +360,11 @@ func (tmp *tmpfile) fallbackLink() error { return fmt.Errorf("close tmpfile: %w", err) } + err = tmp.applyModTime(tempname) + if err != nil { + return fmt.Errorf("set tmpfile modification time: %w", err) + } + objPath := filepath.Join(tmp.bucket, tmp.objname) dir := filepath.Dir(objPath) err = os.Rename(tempname, objPath) diff --git a/backend/posix/without_otmpfile.go b/backend/posix/without_otmpfile.go index bab2b4e9..c5713825 100644 --- a/backend/posix/without_otmpfile.go +++ b/backend/posix/without_otmpfile.go @@ -24,6 +24,7 @@ import ( "os" "path/filepath" "syscall" + "time" "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" @@ -49,6 +50,9 @@ type tmpfile struct { uid int gid int doChown bool + // modTime is the modification time the file is published with, + // the time it was last written if zero + modTime time.Time } // openTmpFile opens a temporary file in dir (a filesystem path) that link() @@ -121,6 +125,11 @@ func (tmp *tmpfile) link() error { return fmt.Errorf("close tmpfile: %w", err) } + err = tmp.applyModTime(tempname) + if err != nil { + return fmt.Errorf("set tmpfile modification time: %w", err) + } + backoffMs := initialBackoffMs for { err = backend.MoveFile(tempname, objPath, tmp.newFilePerm) diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index b5b09633..8ed9c9b2 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -2057,6 +2057,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_DeleteObject_latest_version_with_null_version) ts.Run(Versioning_DeleteObject_latest_version_null_version_order) ts.Run(Versioning_DeleteObject_promoted_version_attributes) + ts.Run(Versioning_DeleteObject_promoted_version_last_modified) ts.Run(Versioning_DeleteObject_non_existing_object) ts.Run(Versioning_DeleteObject_implicit_dir) ts.Run(Versioning_DeleteObject_trailing_slash_counterpart) @@ -3659,6 +3660,7 @@ func GetIntTests() IntTests { "Versioning_DeleteObject_latest_version_with_null_version": Versioning_DeleteObject_latest_version_with_null_version, "Versioning_DeleteObject_latest_version_null_version_order": Versioning_DeleteObject_latest_version_null_version_order, "Versioning_DeleteObject_promoted_version_attributes": Versioning_DeleteObject_promoted_version_attributes, + "Versioning_DeleteObject_promoted_version_last_modified": Versioning_DeleteObject_promoted_version_last_modified, "Versioning_DeleteObject_dir_object_latest_version": Versioning_DeleteObject_dir_object_latest_version, "Versioning_DeleteObject_non_existing_object": Versioning_DeleteObject_non_existing_object, "Versioning_DeleteObject_implicit_dir": Versioning_DeleteObject_implicit_dir, diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index b14054b7..9d7d8bd5 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -2818,6 +2818,149 @@ func Versioning_DeleteObject_promoted_version_attributes(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +// Versioning_DeleteObject_promoted_version_last_modified deletes the current +// version of objects, which makes an older version the current one: the +// version put before it, the version under a delete marker or the null +// version. The promoted version keeps its last modified time. +func Versioning_DeleteObject_promoted_version_last_modified(s *S3Conf) error { + testName := "Versioning_DeleteObject_promoted_version_last_modified" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + cases := []struct { + obj string + // the version to promote is put while versioning is suspended + null bool + // the version to promote is hidden by a delete marker + delMarker bool + // the last modified time of the version to promote + lastModified *time.Time + }{ + {obj: "my-obj"}, + {obj: "my-dir/"}, + {obj: "my-dm-obj", delMarker: true}, + {obj: "my-dm-dir/", delMarker: true}, + {obj: "my-null-obj", null: true}, + {obj: "my-null-dir/", null: true}, + } + + put := func(obj string) (*string, error) { + r, err := putObjectWithData(objDataLen(obj, 100), &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return nil, err + } + return r.res.VersionId, nil + } + + // the versions to promote are put first, the null versions while + // versioning is suspended + for _, status := range []types.BucketVersioningStatus{ + types.BucketVersioningStatusSuspended, + types.BucketVersioningStatusEnabled, + } { + err := putBucketVersioningStatus(s3client, bucket, status) + if err != nil { + return err + } + for i, c := range cases { + if c.null != (status == types.BucketVersioningStatusSuspended) { + continue + } + if _, err := put(c.obj); err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &c.obj, + }) + cancel() + if err != nil { + return err + } + if res.LastModified == nil { + return fmt.Errorf("%v: expected non nil LastModified", c.obj) + } + cases[i].lastModified = res.LastModified + } + } + + // the versions are promoted in a later second than they were put + time.Sleep(time.Second) + + for _, c := range cases { + var latest *string + if c.delMarker { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &c.obj, + }) + cancel() + if err != nil { + return err + } + latest = out.VersionId + } else { + versionId, err := put(c.obj) + if err != nil { + return err + } + latest = versionId + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &c.obj, + VersionId: latest, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &c.obj, + }) + cancel() + if err != nil { + return err + } + if res.LastModified == nil || !res.LastModified.Equal(*c.lastModified) { + return fmt.Errorf("%v: expected the promoted version LastModified to be %v, instead got %v", + c.obj, *c.lastModified, res.LastModified) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + versions, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{ + Bucket: &bucket, + Prefix: &c.obj, + }) + cancel() + if err != nil { + return err + } + + if len(versions.Versions) != 1 { + return fmt.Errorf("%v: expected 1 object version, instead got %v", + c.obj, len(versions.Versions)) + } + v := versions.Versions[0] + if v.LastModified == nil || !v.LastModified.Equal(*c.lastModified) { + return fmt.Errorf("%v: expected the listed version LastModified to be %v, instead got %v", + c.obj, *c.lastModified, v.LastModified) + } + } + + return nil + }, withVersioning(types.BucketVersioningStatusEnabled)) +} + func Versioning_DeleteObject_non_existing_object(s *S3Conf) error { testName := "Versioning_DeleteObject_non_existing_object" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {