From 25da9901f50fecc6cb417193680f0f3cfe2ffcdd Mon Sep 17 00:00:00 2001 From: Suyunjing Date: Sun, 8 Nov 2026 13:22:06 +0800 Subject: [PATCH] fix(drivers/local): handle cross-device recycle bin deletes (#3304) - Fall back from cross-device rename to copy-then-remove while preserving relative symlinks, read-only directory support, file and directory modification times, and existing permissions. - Keep the source and incomplete recycle-bin copy when a named-pipe copy does not create a destination entry. - Preserve the complete recycle-bin copy when source removal fails. - Skip the recycle bin when transfer tasks remove sources after a successful cross-storage move, avoiding duplicate move and recycle-bin copies. - Add targeted coverage for relative symlinks, read-only directories, modification times, and permanent removal with a recycle bin configured. Fixes #1856 Co-authored-by: LunelleEverdawn --- drivers/local/driver.go | 14 +++- drivers/local/remove_test.go | 136 ++++++++++++++++++++++++++++++++ drivers/local/util.go | 64 +++++++++++++++ internal/conf/const.go | 1 + internal/task_group/transfer.go | 4 +- 5 files changed, 217 insertions(+), 2 deletions(-) diff --git a/drivers/local/driver.go b/drivers/local/driver.go index 3c851d98..2c39c9e3 100644 --- a/drivers/local/driver.go +++ b/drivers/local/driver.go @@ -361,7 +361,7 @@ func (d *Local) Copy(_ context.Context, srcObj, dstDir model.Obj) error { func (d *Local) Remove(ctx context.Context, obj model.Obj) error { var err error - if utils.SliceContains([]string{"", "delete permanently"}, d.RecycleBinPath) { + if utils.SliceContains([]string{"", "delete permanently"}, d.RecycleBinPath) || ctx.Value(conf.RemovePermanentlyKey) != nil { if obj.IsDir() { err = os.RemoveAll(obj.GetPath()) } else { @@ -388,6 +388,18 @@ func (d *Local) Remove(ctx context.Context, obj model.Obj) error { dstPath = filepath.Join(recycleBinPath, objName+"_"+time.Now().Format("20060102150405")) } err = os.Rename(objPath, dstPath) + if isCrossDeviceError(err) { + err = d.copyForMove(objPath, dstPath) + if err != nil { + _ = os.RemoveAll(dstPath) + } else { + if obj.IsDir() { + err = os.RemoveAll(objPath) + } else { + err = os.Remove(objPath) + } + } + } } if err != nil { return err diff --git a/drivers/local/remove_test.go b/drivers/local/remove_test.go index 86ec5108..b40993c5 100644 --- a/drivers/local/remove_test.go +++ b/drivers/local/remove_test.go @@ -4,8 +4,11 @@ import ( "context" "os" "path/filepath" + "runtime" "testing" + "time" + "github.com/OpenListTeam/OpenList/v4/internal/conf" "github.com/OpenListTeam/OpenList/v4/internal/model" ) @@ -42,3 +45,136 @@ func TestRemoveDeletesThumbCache(t *testing.T) { t.Fatalf("expected thumb cache to be removed, got %v", err) } } + +func TestCopyForMovePreservesRelativeSymlink(t *testing.T) { + root := t.TempDir() + srcPath := filepath.Join(root, "source") + dstPath := filepath.Join(root, "recycle", "source") + if err := os.Mkdir(srcPath, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(srcPath, "data"), []byte("data"), 0o666); err != nil { + t.Fatal(err) + } + if err := os.Symlink("data", filepath.Join(srcPath, "link")); err != nil { + t.Skipf("symlinks are unavailable: %v", err) + } + + driver := &Local{mkdirPerm: 0o755} + if err := driver.copyForMove(srcPath, dstPath); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(srcPath); err != nil { + t.Fatal(err) + } + + content, err := os.ReadFile(filepath.Join(dstPath, "link")) + if err != nil { + t.Fatal(err) + } + if string(content) != "data" { + t.Fatalf("expected data through moved relative symlink, got %q", content) + } +} + +func TestCopyForMoveHandlesReadOnlyDirectory(t *testing.T) { + root := t.TempDir() + srcPath := filepath.Join(root, "source") + dstPath := filepath.Join(root, "recycle", "source") + if err := os.Mkdir(srcPath, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(srcPath, "data"), []byte("data"), 0o666); err != nil { + t.Fatal(err) + } + if err := os.Chmod(srcPath, 0o555); err != nil { + t.Fatal(err) + } + defer os.Chmod(srcPath, 0o755) + + driver := &Local{mkdirPerm: 0o755} + if err := driver.copyForMove(srcPath, dstPath); err != nil { + t.Fatal(err) + } + + content, err := os.ReadFile(filepath.Join(dstPath, "data")) + if err != nil { + t.Fatal(err) + } + if string(content) != "data" { + t.Fatalf("expected copied file content, got %q", content) + } + info, err := os.Stat(dstPath) + if err != nil { + t.Fatal(err) + } + if runtime.GOOS != "windows" && info.Mode().Perm() != 0o555 { + t.Fatalf("expected destination mode 0555, got %o", info.Mode().Perm()) + } +} + +func TestCopyForMovePreservesModTime(t *testing.T) { + root := t.TempDir() + srcPath := filepath.Join(root, "source") + dstPath := filepath.Join(root, "recycle", "source") + filePath := filepath.Join(srcPath, "data") + if err := os.Mkdir(srcPath, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filePath, []byte("data"), 0o666); err != nil { + t.Fatal(err) + } + fileTime := time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC) + dirTime := time.Date(2021, 6, 7, 8, 9, 10, 0, time.UTC) + if err := os.Chtimes(filePath, fileTime, fileTime); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(srcPath, dirTime, dirTime); err != nil { + t.Fatal(err) + } + + driver := &Local{mkdirPerm: 0o755} + if err := driver.copyForMove(srcPath, dstPath); err != nil { + t.Fatal(err) + } + + for path, want := range map[string]time.Time{ + filepath.Join(dstPath, "data"): fileTime, + dstPath: dirTime, + } { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !info.ModTime().Equal(want) { + t.Fatalf("expected %s mod time %v, got %v", path, want, info.ModTime()) + } + } +} + +func TestRemovePermanentlySkipsRecycleBin(t *testing.T) { + root := t.TempDir() + storageRoot := filepath.Join(root, "storage") + recycleBin := filepath.Join(root, "recycle") + if err := os.Mkdir(storageRoot, 0o755); err != nil { + t.Fatal(err) + } + filePath := filepath.Join(storageRoot, "data") + if err := os.WriteFile(filePath, []byte("data"), 0o666); err != nil { + t.Fatal(err) + } + + driver := &Local{Addition: Addition{RecycleBinPath: recycleBin}} + driver.RootFolderPath = storageRoot + ctx := context.WithValue(context.Background(), conf.RemovePermanentlyKey, struct{}{}) + if err := driver.Remove(ctx, &model.Object{Path: filePath, Name: "data"}); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(filePath); !os.IsNotExist(err) { + t.Fatalf("expected source to be removed, got %v", err) + } + if _, err := os.Stat(filepath.Join(recycleBin, "data")); !os.IsNotExist(err) { + t.Fatalf("expected no recycle bin copy, got %v", err) + } +} diff --git a/drivers/local/util.go b/drivers/local/util.go index b3ccc1fd..17180143 100644 --- a/drivers/local/util.go +++ b/drivers/local/util.go @@ -22,6 +22,7 @@ import ( "github.com/OpenListTeam/OpenList/v4/internal/model" "github.com/OpenListTeam/OpenList/v4/pkg/utils" "github.com/disintegration/imaging" + log "github.com/sirupsen/logrus" ffmpeg "github.com/u2takey/ffmpeg-go" ) @@ -444,6 +445,69 @@ func (m *DirectoryMap) DeleteDirNode(dirname string) error { return nil } +func (d *Local) copyForMove(srcPath, dstPath string) error { + info, err := os.Lstat(srcPath) + if err != nil { + return err + } + if info.Mode()&os.ModeDevice != 0 { + return errors.New("cannot copy a device") + } + if info.Mode()&os.ModeSymlink != 0 { + return d.copySymlinkForMove(srcPath, dstPath) + } + if info.Mode()&os.ModeNamedPipe != 0 { + if err = copyNamedPipe(dstPath, info.Mode(), os.FileMode(d.mkdirPerm)); err != nil { + return err + } + // Some platforms skip named pipes, so keep the source when nothing was created. + _, err = os.Lstat(dstPath) + return err + } + if !info.IsDir() { + if err = utils.CopyFile(srcPath, dstPath); err != nil { + return err + } + preserveModTime(dstPath, info) + return nil + } + + if err = os.MkdirAll(dstPath, os.FileMode(d.mkdirPerm)); err != nil { + return err + } + files, err := readDir(srcPath) + if err != nil { + return err + } + for _, file := range files { + if err = d.copyForMove(filepath.Join(srcPath, file.Name()), filepath.Join(dstPath, file.Name())); err != nil { + return err + } + } + if err = os.Chmod(dstPath, info.Mode()); err != nil { + return err + } + preserveModTime(dstPath, info) + return nil +} + +func preserveModTime(dstPath string, info os.FileInfo) { + if err := os.Chtimes(dstPath, info.ModTime(), info.ModTime()); err != nil { + log.Errorf("[local] failed to change time of %s: %s", dstPath, err) + } +} + +func (d *Local) copySymlinkForMove(srcPath, dstPath string) error { + linkOrig, err := os.Readlink(srcPath) + if err != nil { + return err + } + if err = os.MkdirAll(filepath.Dir(dstPath), os.FileMode(d.mkdirPerm)); err != nil { + return err + } + return os.Symlink(linkOrig, dstPath) +} + func (d *Local) tryCopy(srcPath, dstPath string, info os.FileInfo) error { if info.Mode()&os.ModeDevice != 0 { return errors.New("cannot copy a device") diff --git a/internal/conf/const.go b/internal/conf/const.go index 9dda0ffa..9d4949af 100644 --- a/internal/conf/const.go +++ b/internal/conf/const.go @@ -272,4 +272,5 @@ const ( DirectUploadContentTypeKey SkipNoOverwriteKey ThumbnailBaseURLKey + RemovePermanentlyKey ) diff --git a/internal/task_group/transfer.go b/internal/task_group/transfer.go index 0e9661b4..9996aede 100644 --- a/internal/task_group/transfer.go +++ b/internal/task_group/transfer.go @@ -64,7 +64,9 @@ func HookAndRemove(ctx context.Context, dstPath string, payloads ...any) { log.Error(errors.WithMessage(err, "failed get src storage")) continue } - err = verifyAndRemove(ctx, srcStorage, dstStorage, srcActualPath, dstActualPath) + // The destination already holds the moved objects, so skip recycle bins. + removeCtx := context.WithValue(ctx, conf.RemovePermanentlyKey, struct{}{}) + err = verifyAndRemove(removeCtx, srcStorage, dstStorage, srcActualPath, dstActualPath) if err != nil { log.Error(err) }