fix(drivers/local): handle cross-device recycle bin deletes (#3304)
Beta Release builds / Beta Release Changelog (push) Failing after 56s
Beta Release builds / Beta Release (, , md5, !(*musl*|*windows-arm64*|*windows7-*|*android*|*freebsd*)) (push) Skipped
Beta Release builds / Beta Release (, , md5-android, false, android-*) (push) Skipped
Beta Release builds / Beta Release (, , md5-freebsd, false, freebsd-*) (push) Skipped
Beta Release builds / Beta Release (, , md5-windows-arm64, false, windows-arm64) (push) Skipped
Beta Release builds / Beta Release (, -tags=sqlite_cgo_compat, md5-windows7, false, windows7-*) (push) Skipped
Beta Release builds / Beta Release (-ldflags=-linkmode external -extldflags '-static -fpic', , md5-linux-musl, true, linux-!(arm*|mips|mips64|mipsle|mips64le|loong64)-musl*) (push) Skipped
Beta Release builds / Beta Release (-ldflags=-linkmode external -extldflags '-static -fpic', , md5-linux-musl-arm, true, linux-arm*-musl*) (push) Skipped
Beta Release builds / Beta Release (-ldflags=-linkmode external -extldflags '-static -fpic', , md5-linux-musl-mips, true, linux-(mips|mips64|mipsle|mips64le|loong64)-musl*) (push) Skipped
Sync to Gitee / Sync GitHub to Gitee (push) Failing after 36s
Beta Release (Docker) / Build Binaries for Docker Release (Beta) (push) Failing after 57m4s
Beta Release (Docker) / Release Docker image (Beta) (aio, INSTALL_FFMPEG=true INSTALL_ARIA2=true , aio, suffix=-aio,onlatest=true) (push) Skipped
Beta Release (Docker) / Release Docker image (Beta) (aria2, INSTALL_ARIA2=true, aria2, suffix=-aria2,onlatest=true) (push) Skipped
Beta Release (Docker) / Release Docker image (Beta) (base, , latest, ) (push) Skipped
Beta Release (Docker) / Release Docker image (Beta) (ffmpeg, INSTALL_FFMPEG=true, ffmpeg, suffix=-ffmpeg,onlatest=true) (push) Skipped

- 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 <lunelle+suyunmeng@oplist.org>
This commit is contained in:
SuyunjingandLunelleEverdawn authored and GitHub committed 2026-11-08 13:22:06 +08:00
1 parent e7b6fac89c
commit 25da9901f5
5 files changed
+217 -2

No files matched your search

+13 -1
View File
@@ -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
+136
View File
@@ -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)
}
}
+64
View File
@@ -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")
+1
View File
@@ -272,4 +272,5 @@ const (
DirectUploadContentTypeKey
SkipNoOverwriteKey
ThumbnailBaseURLKey
RemovePermanentlyKey
)
+3 -1
View File
@@ -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)
}