From 8d7ed4d174124f75f321f2b145a7c843c768ca01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 4 Jul 2023 15:12:33 +0200 Subject: [PATCH] Prevent lockfile name clashes --- pkg/storage/utils/decomposedfs/lookup/lookup.go | 4 ++-- .../decomposedfs/metadata/messagepack_backend.go | 5 +++-- .../utils/decomposedfs/metadata/metadata.go | 4 ++++ .../utils/decomposedfs/metadata/xattrs_backend.go | 6 +++++- pkg/storage/utils/decomposedfs/tree/tree.go | 14 ++------------ .../utils/decomposedfs/upload/processing.go | 4 ++-- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/lookup/lookup.go b/pkg/storage/utils/decomposedfs/lookup/lookup.go index 816150efc6..273e094f9d 100644 --- a/pkg/storage/utils/decomposedfs/lookup/lookup.go +++ b/pkg/storage/utils/decomposedfs/lookup/lookup.go @@ -284,7 +284,7 @@ func refFromCS3(b []byte) (*provider.Reference, error) { func (lu *Lookup) CopyMetadata(ctx context.Context, src, target string, filter func(attributeName string) bool) (err error) { // Acquire a read log on the source node // write lock existing node before reading treesize or tree time - lock, err := lockedfile.OpenFile(lu.MetadataBackend().MetadataPath(src)+".lock", os.O_RDONLY|os.O_CREATE, 0600) + lock, err := lockedfile.OpenFile(lu.MetadataBackend().LockfilePath(src), os.O_RDONLY|os.O_CREATE, 0600) if err != nil { return err } @@ -312,7 +312,7 @@ func (lu *Lookup) CopyMetadataWithSourceLock(ctx context.Context, sourcePath, ta switch { case lockedSource == nil: return errors.New("no lock provided") - case lockedSource.File.Name() != lu.MetadataBackend().MetadataPath(sourcePath)+".lock": + case lockedSource.File.Name() != lu.MetadataBackend().LockfilePath(sourcePath): return errors.New("lockpath does not match filepath") } diff --git a/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go b/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go index 32a36a5c61..9cfa629c10 100644 --- a/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go +++ b/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go @@ -145,7 +145,7 @@ func (b MessagePackBackend) saveAttributes(ctx context.Context, path string, set span.End() }() - lockPath := b.lockFilePath(path) + lockPath := b.LockfilePath(path) metaPath := b.MetadataPath(path) if acquireLock { _, subspan := tracer.Start(ctx, "lockedfile.OpenFile") @@ -308,7 +308,8 @@ func (b MessagePackBackend) Rename(oldPath, newPath string) error { // MetadataPath returns the path of the file holding the metadata for the given path func (MessagePackBackend) MetadataPath(path string) string { return path + ".mpk" } -func (MessagePackBackend) lockFilePath(path string) string { return path + ".mpk.lock" } +// LockfilePath returns the path of the lock file +func (MessagePackBackend) LockfilePath(path string) string { return path + ".mpk.lock" } func (b MessagePackBackend) cacheKey(path string) string { // rootPath is guaranteed to have no trailing slash diff --git a/pkg/storage/utils/decomposedfs/metadata/metadata.go b/pkg/storage/utils/decomposedfs/metadata/metadata.go index c3e37ea074..243895b364 100644 --- a/pkg/storage/utils/decomposedfs/metadata/metadata.go +++ b/pkg/storage/utils/decomposedfs/metadata/metadata.go @@ -52,6 +52,7 @@ type Backend interface { Rename(oldPath, newPath string) error IsMetaFile(path string) bool MetadataPath(path string) string + LockfilePath(path string) string AllWithLockedSource(ctx context.Context, path string, source io.Reader) (map[string][]byte, error) } @@ -110,6 +111,9 @@ func (NullBackend) Rename(oldPath, newPath string) error { return errUnconfigure // MetadataPath returns the path of the file holding the metadata for the given path func (NullBackend) MetadataPath(path string) string { return "" } +// LockfilePath returns the path of the lock file +func (NullBackend) LockfilePath(path string) string { return "" } + // AllWithLockedSource reads all extended attributes from the given reader // The path argument is used for storing the data in the cache func (NullBackend) AllWithLockedSource(ctx context.Context, path string, source io.Reader) (map[string][]byte, error) { diff --git a/pkg/storage/utils/decomposedfs/metadata/xattrs_backend.go b/pkg/storage/utils/decomposedfs/metadata/xattrs_backend.go index 5a402f6817..92462f89d2 100644 --- a/pkg/storage/utils/decomposedfs/metadata/xattrs_backend.go +++ b/pkg/storage/utils/decomposedfs/metadata/xattrs_backend.go @@ -24,6 +24,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "github.com/cs3org/reva/v2/pkg/storage/utils/filelocks" "github.com/pkg/errors" @@ -156,7 +157,7 @@ func (XattrsBackend) Remove(ctx context.Context, filePath string, key string) (e } // IsMetaFile returns whether the given path represents a meta file -func (XattrsBackend) IsMetaFile(path string) bool { return false } +func (XattrsBackend) IsMetaFile(path string) bool { return strings.HasSuffix(path, ".meta.lock") } // Purge purges the data of a given path func (XattrsBackend) Purge(path string) error { return nil } @@ -167,6 +168,9 @@ func (XattrsBackend) Rename(oldPath, newPath string) error { return nil } // MetadataPath returns the path of the file holding the metadata for the given path func (XattrsBackend) MetadataPath(path string) string { return path } +// LockfilePath returns the path of the lock file +func (XattrsBackend) LockfilePath(path string) string { return path + ".meta.lock" } + func cleanupLockfile(f *lockedfile.File) { _ = f.Close() _ = os.Remove(f.Name()) diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 551668213e..2613033a1e 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -40,7 +40,6 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata/prefixes" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/options" - "github.com/cs3org/reva/v2/pkg/storage/utils/filelocks" "github.com/cs3org/reva/v2/pkg/utils" "github.com/google/uuid" "github.com/pkg/errors" @@ -750,17 +749,8 @@ func (t *Tree) Propagate(ctx context.Context, n *node.Node, sizeDiff int64) (err // lock parent before reading treesize or tree time _, subspan := tracer.Start(ctx, "lockedfile.OpenFile") - var parentFilename string - switch t.lookup.MetadataBackend().(type) { - case metadata.MessagePackBackend: - parentFilename = t.lookup.MetadataBackend().MetadataPath(n.ParentPath()) + ".lock" - f, err = lockedfile.OpenFile(parentFilename, os.O_RDWR|os.O_CREATE, 0600) - case metadata.XattrsBackend: - // we have to use dedicated lockfiles to lock directories - // this only works because the xattr backend also locks folders with separate lock files - parentFilename = n.ParentPath() + filelocks.LockFileSuffix - f, err = lockedfile.OpenFile(parentFilename, os.O_RDWR|os.O_CREATE, 0600) - } + parentFilename := t.lookup.MetadataBackend().LockfilePath(n.ParentPath()) + f, err = lockedfile.OpenFile(parentFilename, os.O_RDWR|os.O_CREATE, 0600) subspan.End() if err != nil { sublog.Error().Err(err). diff --git a/pkg/storage/utils/decomposedfs/upload/processing.go b/pkg/storage/utils/decomposedfs/upload/processing.go index 4138caebbb..08bba8f996 100644 --- a/pkg/storage/utils/decomposedfs/upload/processing.go +++ b/pkg/storage/utils/decomposedfs/upload/processing.go @@ -328,7 +328,7 @@ func initNewNode(upload *Upload, n *node.Node, fsize uint64) (*lockedfile.File, } // create and write lock new node metadata - f, err := lockedfile.OpenFile(upload.lu.MetadataBackend().MetadataPath(n.InternalPath())+".lock", os.O_RDWR|os.O_CREATE, 0600) + f, err := lockedfile.OpenFile(upload.lu.MetadataBackend().LockfilePath(n.InternalPath()), os.O_RDWR|os.O_CREATE, 0600) if err != nil { return nil, err } @@ -396,7 +396,7 @@ func updateExistingNode(upload *Upload, n *node.Node, spaceID string, fsize uint targetPath := n.InternalPath() // write lock existing node before reading treesize or tree time - f, err := lockedfile.OpenFile(upload.lu.MetadataBackend().MetadataPath(targetPath)+".lock", os.O_RDWR|os.O_CREATE, 0600) + f, err := lockedfile.OpenFile(upload.lu.MetadataBackend().LockfilePath(targetPath), os.O_RDWR|os.O_CREATE, 0600) if err != nil { return nil, err }