diff --git a/backend/meta/meta.go b/backend/meta/meta.go index a500f493..d43bccdb 100644 --- a/backend/meta/meta.go +++ b/backend/meta/meta.go @@ -44,4 +44,30 @@ type MetadataStorer interface { // within the given bucket. This must be called whenever the data // directory for an object is renamed so that metadata stays in sync. RenameObject(bucket, oldObject, newObject string) error + + // CommitMetadata atomically moves any per-upload temporary metadata written + // via StoreAttribute(f, bucket, object, …) to the final sidecar location + // for (bucket, object). It must be called immediately after the data file + // has been linked into the namespace (tmpfile.link). For backends that do + // not use a temporary staging area this is a no-op. + // + // token is the per-upload sidecar identifier computed by tmpfile.SidecarToken() + // before link() closed the temp file descriptor. + // + // dataPath is the filesystem path of the committed data file (e.g. + // filepath.Join(bucket, object) relative to the POSIX data root). The + // SideCar implementation uses it to verify that this upload is still the + // inode at the final path before each attribute rename. Other + // implementations may pass an empty string or ignore the parameter. + CommitMetadata(bucket, object, token, dataPath string) error + + // CleanupMetadata removes any per-upload temporary metadata staged by + // StoreAttribute(f, bucket, object, …) without promoting it to the final + // location. It must be called when this upload lost the concurrent link() + // race (didWinLink returned false) or when link() returned EEXIST, so that + // the temporary staging directory does not accumulate. For backends that do + // not use a temporary staging area this is a no-op. + // + // token is the per-upload sidecar identifier computed by tmpfile.SidecarToken(). + CleanupMetadata(bucket, token string) error } diff --git a/backend/meta/none.go b/backend/meta/none.go index 7f984caa..9b40f37b 100644 --- a/backend/meta/none.go +++ b/backend/meta/none.go @@ -53,6 +53,16 @@ func (NoMeta) DeleteAttributes(bucket, object string) error { return nil } +// CommitMetadata is a no-op because NoMeta does not store any metadata. +func (NoMeta) CommitMetadata(_, _, _, _ string) error { + return nil +} + +// CleanupMetadata is a no-op because NoMeta does not store any metadata. +func (NoMeta) CleanupMetadata(_, _ string) error { + return nil +} + // RenameObject is a no-op because NoMeta does not store metadata. func (NoMeta) RenameObject(_, _, _ string) error { return nil diff --git a/backend/meta/sidecar.go b/backend/meta/sidecar.go index 3056ae3e..936f50b8 100644 --- a/backend/meta/sidecar.go +++ b/backend/meta/sidecar.go @@ -20,6 +20,8 @@ import ( "io" "os" "path/filepath" + "strconv" + "strings" "syscall" "github.com/versity/versitygw/s3err" @@ -66,32 +68,86 @@ func (s SideCar) RetrieveAttribute(_ *os.File, bucket, object, attribute string) return value, nil } -// StoreAttribute stores the value of a specific attribute for an object or a bucket. -func (s SideCar) StoreAttribute(_ *os.File, bucket, object, attribute string, value []byte) error { - metadir := filepath.Join(s.dir, bucket, object, sidecarmeta) - if object == "" { - metadir = filepath.Join(s.dir, bucket, sidecarmeta) - } - err := os.MkdirAll(metadir, 0777) - if err != nil { - if errors.Is(err, syscall.ENOSPC) { - return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice) - } - return fmt.Errorf("failed to create metadata directory: %v", err) +// tmpSidecarID returns a string that uniquely identifies an in-flight upload +// for the purpose of naming its temporary sidecar directory. +// +// On Unix it uses the file's inode number, which is stable for the lifetime +// of the upload's data file regardless of fd-number reuse. This is critical +// on Linux where O_TMPFILE fds can be reused by other goroutines as soon as +// link() closes the fd. +// On platforms without POSIX inodes (e.g. Windows), fileIno returns 0 and +// the function falls back to the path-based identifier, which is always +// unique because CreateTemp generates uniquely-named files. +// +// IMPORTANT: the token format must stay in sync with tmpfile.SidecarToken() in +// backend/posix/otmpfile_common.go, which computes the same value from the +// captured inode after link(). Both functions must produce identical output for +// the staging and commit steps to find the same directory. +func tmpSidecarID(f *os.File) string { + if ino := fileIno(f); ino != 0 { + return fmt.Sprintf("%d.%d", os.Getpid(), ino) } + return fmt.Sprintf("%d.%s", os.Getpid(), filepath.Base(f.Name())) +} - attr := filepath.Join(metadir, attribute) - tempfile, err := os.CreateTemp(metadir, attribute) - if err != nil { +// StoreAttribute stores the value of a specific attribute for an object or a bucket. +// +// When f is non-nil and object is non-empty the attribute is written to a +// per-upload temporary sidecar directory instead of the final path. This +// prevents a race where concurrent uploads of the same object could have their +// checksum metadata and data file committed by different goroutines. Call +// CommitMetadata after the data file has been linked to atomically move the +// temp sidecar to the final location. +func (s SideCar) StoreAttribute(f *os.File, bucket, object, attribute string, value []byte) error { + var metadir string + if f != nil && object != "" { + metadir = filepath.Join(s.dir, bucket, ".sgwtmp."+tmpSidecarID(f), sidecarmeta) + } else { + metadir = filepath.Join(s.dir, bucket, object, sidecarmeta) + if object == "" { + metadir = filepath.Join(s.dir, bucket, sidecarmeta) + } + } + // mkdirAndCreateTemp atomically retries MkdirAll+CreateTemp when the + // directory is removed between the two calls. This can happen when a + // concurrent CommitMetadata goroutine calls RemoveAll on the same + // inode-named temp sidecar directory (possible via inode reuse after + // a previous upload's data file was overwritten and its inode freed). + const maxRetries = 5 + var ( + tempfile *os.File + lastErr error + ) + for range maxRetries { + if err := os.MkdirAll(metadir, 0777); err != nil { + if errors.Is(err, syscall.ENOSPC) { + return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice) + } + return fmt.Errorf("failed to create metadata directory: %v", err) + } + var err error + tempfile, err = os.CreateTemp(metadir, attribute) + if err == nil { + break + } if errors.Is(err, syscall.ENOSPC) { return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice) } - return fmt.Errorf("failed to create temporary file: %v", err) + if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("failed to create temporary file: %v", err) + } + // ENOENT: the directory was removed by a concurrent CommitMetadata + // (inode reuse race); retry with a fresh MkdirAll. + lastErr = err + } + if tempfile == nil { + return fmt.Errorf("failed to create temporary file in %v after %v retries: %w", metadir, maxRetries, lastErr) } defer os.Remove(tempfile.Name()) - _, err = tempfile.Write(value) - if err != nil { + attr := filepath.Join(metadir, attribute) + + if _, err := tempfile.Write(value); err != nil { tempfile.Close() if errors.Is(err, syscall.ENOSPC) { return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice) @@ -101,12 +157,11 @@ func (s SideCar) StoreAttribute(_ *os.File, bucket, object, attribute string, va // Close explicitly before rename to prevent error on Windows: // The process cannot access the file because it is being used by another process. - if err = tempfile.Close(); err != nil { + if err := tempfile.Close(); err != nil { return fmt.Errorf("failed to close temporary file: %v", err) } - err = os.Rename(tempfile.Name(), attr) - if err != nil { + if err := os.Rename(tempfile.Name(), attr); err != nil { return fmt.Errorf("failed to rename temporary file: %v", err) } return nil @@ -180,6 +235,106 @@ func (s SideCar) DeleteAttributes(bucket, object string) error { return nil } +// inoFromToken extracts the inode encoded in a SidecarToken string. +// Token format on Unix: "." where ino is a decimal uint64. +// Token format on Windows: "." where basename is non-numeric. +// Returns 0 if the suffix cannot be parsed as a uint64 (Windows fallback). +func inoFromToken(token string) uint64 { + dot := strings.Index(token, ".") + if dot < 0 { + return 0 + } + ino, err := strconv.ParseUint(token[dot+1:], 10, 64) + if err != nil { + return 0 + } + return ino +} + +// CommitMetadata moves the per-upload temporary sidecar attributes written by +// StoreAttribute to the final location for (bucket, object). It must be +// called after the data file has been linked into the namespace. +// +// token is the per-upload sidecar identifier returned by tmpfile.SidecarToken(), +// computed before link() closed the file descriptor. +// +// dataPath is the filesystem path of the committed data file (e.g. +// filepath.Join(bucket, object) from the POSIX backend). It is used to +// re-verify the inode before each attribute rename so that a goroutine whose +// inode was subsequently displaced by a later link() aborts instead of +// overwriting the correct winner's metadata. +// +// Each attribute file is moved individually (atomic rename). Before each +// rename the inode at dataPath is re-verified against the inode encoded in +// token. If the inode no longer matches (a later concurrent link() has +// installed a different upload's data file), this goroutine is no longer the +// winner: it aborts, removes its staged temp directory, and returns without +// error. The true winner's CommitMetadata will overwrite any partially +// committed attributes before it finishes, leaving only consistent metadata. +func (s SideCar) CommitMetadata(bucket, object, token, dataPath string) error { + if token == "" || object == "" { + return nil + } + + myIno := inoFromToken(token) + + tempDir := filepath.Join(s.dir, bucket, ".sgwtmp."+token) + tempMetaDir := filepath.Join(tempDir, sidecarmeta) + finalMetaDir := filepath.Join(s.dir, bucket, object, sidecarmeta) + + entries, err := os.ReadDir(tempMetaDir) + if errors.Is(err, os.ErrNotExist) { + // No metadata was staged for this upload; nothing to commit. + return nil + } + if err != nil { + return fmt.Errorf("read staged metadata: %w", err) + } + + if err := os.MkdirAll(finalMetaDir, 0777); err != nil { + if errors.Is(err, syscall.ENOSPC) { + return s3err.GetAPIError(s3err.ErrNoSpaceLeftOnDevice) + } + return fmt.Errorf("create final metadata dir: %w", err) + } + + for _, ent := range entries { + // Re-verify we are still the link() winner before each rename. + // If a concurrent goroutine has since won link() (replacing our inode + // at dataPath), abort now. Any attributes we already renamed will be + // overwritten by the true winner's CommitMetadata, which will run to + // completion because no later link() can displace its inode. + if myIno != 0 && pathIno(dataPath) != myIno { + os.RemoveAll(tempDir) + return nil + } + + src := filepath.Join(tempMetaDir, ent.Name()) + dst := filepath.Join(finalMetaDir, ent.Name()) + if err := os.Rename(src, dst); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("commit attribute %s: %w", ent.Name(), err) + } + } + + os.RemoveAll(tempDir) + return nil +} + +// CleanupMetadata removes the per-upload temporary sidecar directory staged by +// StoreAttribute without promoting it to the final location. It should be +// called when the upload lost the concurrent link() race or when link() +// returned EEXIST, to prevent orphaned .sgwtmp.* directories from accumulating. +func (s SideCar) CleanupMetadata(bucket, token string) error { + if token == "" { + return nil + } + tempDir := filepath.Join(s.dir, bucket, ".sgwtmp."+token) + if err := os.RemoveAll(tempDir); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("cleanup staged metadata: %w", err) + } + return nil +} + // RenameObject renames the sidecar metadata directory from oldObject to // newObject so that path-based lookups continue to work after the data // directory has been renamed. diff --git a/backend/meta/sidecar_unix.go b/backend/meta/sidecar_unix.go new file mode 100644 index 00000000..02ac295c --- /dev/null +++ b/backend/meta/sidecar_unix.go @@ -0,0 +1,52 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//go:build !windows + +package meta + +import ( + "os" + "syscall" +) + +// fileIno returns the inode number of the open file f. The inode is a +// stable, filesystem-unique identifier for the file's data, making it safe +// to use as part of a per-upload sidecar directory name. Returns 0 on error. +func fileIno(f *os.File) uint64 { + fi, err := f.Stat() + if err != nil { + return 0 + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0 + } + return st.Ino +} + +// pathIno returns the inode number of the file at path by calling Lstat. +// Used by CommitMetadata to verify the object path still holds this upload's +// inode before each attribute rename. Returns 0 on error. +func pathIno(path string) uint64 { + fi, err := os.Lstat(path) + if err != nil { + return 0 + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0 + } + return st.Ino +} diff --git a/backend/posix/parentdir_other.go b/backend/meta/sidecar_windows.go similarity index 57% rename from backend/posix/parentdir_other.go rename to backend/meta/sidecar_windows.go index c5b67954..f697fc2a 100644 --- a/backend/posix/parentdir_other.go +++ b/backend/meta/sidecar_windows.go @@ -12,14 +12,21 @@ // specific language governing permissions and limitations // under the License. -//go:build !windows +//go:build windows -package posix +package meta -import ( - "github.com/versity/versitygw/s3err" -) +import "os" -func handleParentDirError(_ string) error { - return s3err.GetAPIError(s3err.ErrObjectParentIsFile) +// fileIno returns 0 on Windows because Windows does not expose POSIX inodes +// via the standard syscall interface. tmpSidecarID falls back to the +// path-based identifier, which is always unique on Windows because +// CreateTemp generates uniquely-named files. +func fileIno(_ *os.File) uint64 { + return 0 +} + +// pathIno returns 0 on Windows; POSIX inodes are not available. +func pathIno(_ string) uint64 { + return 0 } diff --git a/backend/meta/xattr.go b/backend/meta/xattr.go index 7be71d7a..9e82d869 100644 --- a/backend/meta/xattr.go +++ b/backend/meta/xattr.go @@ -91,6 +91,18 @@ func (x XattrMeta) DeleteAttributes(bucket, object string) error { return nil } +// CommitMetadata is a no-op for xattr: extended attributes are stored on the +// inode and travel atomically with the data file through link(), so there is +// no temporary staging area to commit. +func (x XattrMeta) CommitMetadata(_, _, _, _ string) error { + return nil +} + +// CleanupMetadata is a no-op for xattr: there is no temporary staging area. +func (x XattrMeta) CleanupMetadata(_, _ string) error { + return nil +} + // RenameObject is a no-op for xattr because extended attributes are stored // on the inodes and follow the file/directory when it is renamed. func (x XattrMeta) RenameObject(_, _, _ string) error { diff --git a/backend/posix/platform_helpers_unix.go b/backend/posix/platform_helpers_unix.go new file mode 100644 index 00000000..6efe2bd9 --- /dev/null +++ b/backend/posix/platform_helpers_unix.go @@ -0,0 +1,67 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//go:build !windows + +package posix + +import ( + "os" + "path/filepath" + "syscall" + + "github.com/versity/versitygw/s3err" +) + +func handleParentDirError(_ string) error { + return s3err.GetAPIError(s3err.ErrObjectParentIsFile) +} + +// captureIno returns the inode number of the open file f by calling fstat. +// This is used to capture the inode of a temp upload file while its fd is +// still open, before link() closes it. The captured inode is stored in +// tmpfile.ino and used later by SidecarToken() and didWinLink(). +func captureIno(f *os.File) uint64 { + fi, err := f.Stat() + if err != nil { + return 0 + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0 + } + return st.Ino +} + +// didWinLink reports whether this upload's data file is the one currently +// installed at the final object path. It compares the inode captured just +// before link() closed the temp file against the inode of the live object. +// Only the winner should call CommitMetadata. +// +// If ino is 0 (not captured, or on an error path) this returns false so that +// metadata commit is skipped rather than potentially committing stale data. +func (tmp *tmpfile) didWinLink() bool { + if tmp.ino == 0 { + return false + } + fi, err := os.Lstat(filepath.Join(tmp.bucket, tmp.objname)) + if err != nil { + return false + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return false + } + return st.Ino == tmp.ino +} diff --git a/backend/posix/parentdir_windows.go b/backend/posix/platform_helpers_windows.go similarity index 72% rename from backend/posix/parentdir_windows.go rename to backend/posix/platform_helpers_windows.go index d9d96c2a..a730ef70 100644 --- a/backend/posix/parentdir_windows.go +++ b/backend/posix/platform_helpers_windows.go @@ -44,3 +44,17 @@ func handleParentDirError(name string) error { // Parent doesn't exist or is a directory, treat as ENOENT return nil } + +// captureIno returns 0 on Windows; POSIX inodes are not available. +// Windows uploads always use CreateTemp which produces uniquely-named files, +// so path-based sidecar tokens are safe there. +func captureIno(_ *os.File) uint64 { + return 0 +} + +// didWinLink always returns true on Windows. Windows uploads use CreateTemp +// which produces uniquely-named files, so there is no fd-reuse race and no +// concurrent winner determination is needed. +func (tmp *tmpfile) didWinLink() bool { + return true +} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index b8073463..0cd582fb 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -2266,6 +2266,15 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C return res, "", fmt.Errorf("link object in namespace: %w", err) } + if f.didWinLink() { + if err := p.meta.CommitMetadata(bucket, object, f.SidecarToken(), filepath.Join(bucket, object)); err != nil { + return res, "", fmt.Errorf("commit object metadata: %w", err) + } + } else { + // This upload lost the concurrent link() race; clean up its staged sidecar. + _ = p.meta.CleanupMetadata(bucket, f.SidecarToken()) + } + // cleanup tmp dirs os.RemoveAll(filepath.Join(bucket, objdir, activeUploadName)) // use Remove for objdir in case there are still other uploads @@ -3251,6 +3260,15 @@ func (p *Posix) UploadPartWithPostFunc(ctx context.Context, input *s3.UploadPart return nil, fmt.Errorf("link object in namespace: %w", err) } + if f.didWinLink() { + if err := p.meta.CommitMetadata(bucket, partPath, f.SidecarToken(), filepath.Join(bucket, partPath)); err != nil { + return nil, fmt.Errorf("commit part metadata: %w", err) + } + } else { + // This upload lost the concurrent link() race; clean up its staged sidecar. + _ = p.meta.CleanupMetadata(bucket, f.SidecarToken()) + } + return res, nil } @@ -3509,6 +3527,15 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) return s3response.CopyPartResult{}, fmt.Errorf("link object in namespace: %w", err) } + if f.didWinLink() { + if err := p.meta.CommitMetadata(*upi.Bucket, partPath, f.SidecarToken(), filepath.Join(*upi.Bucket, partPath)); err != nil { + return s3response.CopyPartResult{}, fmt.Errorf("commit part metadata: %w", err) + } + } else { + // This upload lost the concurrent link() race; clean up its staged sidecar. + _ = p.meta.CleanupMetadata(*upi.Bucket, f.SidecarToken()) + } + fi, err = os.Stat(filepath.Join(*upi.Bucket, partPath)) if err != nil { return s3response.CopyPartResult{}, fmt.Errorf("stat part path: %w", err) @@ -3929,6 +3956,9 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje err = f.link() if errors.Is(err, syscall.EEXIST) { + // Another upload installed its data file first; discard our staged + // sidecar metadata to prevent .sgwtmp.* dirs from accumulating. + _ = p.meta.CleanupMetadata(*po.Bucket, f.SidecarToken()) return s3response.PutObjectOutput{ ETag: etag, VersionID: versionID, @@ -3938,6 +3968,15 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrExistingObjectIsDirectory) } + if f.didWinLink() { + if err := p.meta.CommitMetadata(*po.Bucket, *po.Key, f.SidecarToken(), filepath.Join(*po.Bucket, *po.Key)); err != nil { + return s3response.PutObjectOutput{}, fmt.Errorf("commit object metadata: %w", err) + } + } else { + // This upload lost the concurrent link() race; clean up its staged sidecar. + _ = p.meta.CleanupMetadata(*po.Bucket, f.SidecarToken()) + } + // Set object tagging if tags != nil { err := p.PutObjectTagging(withCtxNoSlot(ctx), *po.Bucket, *po.Key, "", tags) diff --git a/backend/posix/otmpfile_common.go b/backend/posix/tmpfile_common.go similarity index 53% rename from backend/posix/otmpfile_common.go rename to backend/posix/tmpfile_common.go index a76876dd..713fe9b1 100644 --- a/backend/posix/otmpfile_common.go +++ b/backend/posix/tmpfile_common.go @@ -18,6 +18,7 @@ import ( "fmt" "math/rand" "os" + "path/filepath" "time" ) @@ -35,6 +36,28 @@ func (tmp *tmpfile) File() *os.File { return tmp.f } +// SidecarToken returns the per-upload identifier used to name the temporary +// sidecar directory staged by StoreAttribute. It must be called after +// link() so that tmp.ino has been set. +// +// On Unix the token is ".", which is unique for the lifetime of +// the upload's data file regardless of fd number reuse. +// On platforms where inodes are unavailable (e.g. Windows) it falls back to +// ".", which is unique because CreateTemp generates +// uniquely-named files. +// +// IMPORTANT: the token format must stay in sync with tmpSidecarID() in +// backend/meta/sidecar.go, which computes the same value from the live *os.File +// during StoreAttribute. Both functions must produce identical output for the +// staging and commit steps to find the same directory. +func (tmp *tmpfile) SidecarToken() string { + if tmp.ino != 0 { + return fmt.Sprintf("%d.%d", os.Getpid(), tmp.ino) + } + // Fallback: path-based token for platforms without POSIX inodes. + return fmt.Sprintf("%d.%s", os.Getpid(), filepath.Base(tmp.f.Name())) +} + func sleepWithJitter(backoffMs int) { if backoffMs <= 1 { time.Sleep(1 * time.Millisecond) diff --git a/backend/posix/with_otmpfile.go b/backend/posix/with_otmpfile.go index 6554f023..820294d7 100644 --- a/backend/posix/with_otmpfile.go +++ b/backend/posix/with_otmpfile.go @@ -13,7 +13,6 @@ // under the License. //go:build linux -// +build linux package posix @@ -47,6 +46,10 @@ type tmpfile struct { uid int gid int newDirPerm fs.FileMode + // ino holds the inode of the temp file captured just before link() renames + // it into place. Used by didWinLink() to determine whether this upload is + // the one currently installed at the final object path. + ino uint64 } var ( @@ -217,6 +220,10 @@ func (tmp *tmpfile) link() error { return tmp.fallbackLink() } + // Capture the inode via fstat before linkat so didWinLink() can later + // verify that this upload's file is the one at the final path. + tmp.ino = captureIno(tmp.f) + procdir, err := os.Open(procfddir) if err != nil { return fmt.Errorf("open proc dir: %w", err) @@ -270,6 +277,10 @@ func (tmp *tmpfile) link() error { func (tmp *tmpfile) fallbackLink() error { tempname := tmp.f.Name() + // Capture the inode before closing/renaming so didWinLink() can later + // verify that this upload's file is the one at the final path. + tmp.ino = captureIno(tmp.f) + // reset default file mode because CreateTemp uses 0600 tmp.f.Chmod(fs.FileMode(defaultFilePerm)) diff --git a/backend/posix/without_otmpfile.go b/backend/posix/without_otmpfile.go index 7b22a5f6..2d9e2c82 100644 --- a/backend/posix/without_otmpfile.go +++ b/backend/posix/without_otmpfile.go @@ -44,6 +44,10 @@ type tmpfile struct { uid int gid int doChown bool + // ino holds the inode of the temp file captured just before link() renames + // it into place. Used by didWinLink() to determine whether this upload is + // the one currently installed at the final object path. + ino uint64 } func (p *Posix) openTmpFile(dir, bucket, obj string, size int64, acct auth.Account, _ bool, _ bool) (*tmpfile, error) { @@ -98,6 +102,10 @@ func (tmp *tmpfile) link() error { objPath := filepath.Join(tmp.bucket, tmp.objname) + // Capture the inode before closing/renaming so didWinLink() can later + // verify that this upload's file is the one at the final path. + tmp.ino = captureIno(tmp.f) + // reset default file mode because CreateTemp uses 0600 tmp.f.Chmod(defaultFilePerm) diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 9dc93c73..7652930d 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -564,7 +564,7 @@ func TestCompleteMultipartUpload(ts *TestState) { } ts.Run(CompleteMultipartUpload_success) ts.Run(CompleteMultipartUpload_already_completed) - if !(ts.conf.azureTests || ts.conf.sidecarTests) { + if !(ts.conf.azureTests) { ts.Run(CompleteMultipartUpload_racey_success) ts.Run(CompleteMultipartUpload_racey_data_integrity) }