From 690aabda0e1715ae0aed1e8b4a17b9b6e9651528 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 2 Nov 2026 10:28:48 -0800 Subject: [PATCH] filer: reuse IsEqualOrUnder for the rename subdirectory check (#12338) * filer: test the rename subdirectory check through CanRename Co-Authored-By: Claude Opus 5.5 * filer: reuse IsEqualOrUnder for the rename subdirectory check A trailing slash on the old name made the source path end in a slash, so a child of the source was no longer seen as under it. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- weed/filer/filer_rename.go | 9 +------ weed/filer/filer_rename_test.go | 43 ++++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/weed/filer/filer_rename.go b/weed/filer/filer_rename.go index 1a9c48fc59..7cfd35e88f 100644 --- a/weed/filer/filer_rename.go +++ b/weed/filer/filer_rename.go @@ -10,7 +10,7 @@ import ( func (f *Filer) CanRename(ctx context.Context, source, target util.FullPath, oldName string) error { sourcePath := source.Child(oldName) - if movesIntoItself(sourcePath, target) { + if util.IsEqualOrUnder(string(target), string(sourcePath)) { return fmt.Errorf("mv: can not move directory to a subdirectory of itself") } @@ -33,13 +33,6 @@ func (f *Filer) CanRename(ctx context.Context, source, target util.FullPath, old return nil } -// movesIntoItself reports whether target, the new parent directory, is -// sourcePath or lies under it. A sibling that only shares a name prefix, -// such as /a.d for /a, does not. -func movesIntoItself(sourcePath, target util.FullPath) bool { - return target == sourcePath || target.IsUnder(sourcePath) -} - func (f *Filer) DetectBucket(source util.FullPath) (bucket string) { if strings.HasPrefix(string(source), f.DirBucketsPath+"/") { bucketAndObjectKey := string(source)[len(f.DirBucketsPath)+1:] diff --git a/weed/filer/filer_rename_test.go b/weed/filer/filer_rename_test.go index 5df9429c14..b5447c7539 100644 --- a/weed/filer/filer_rename_test.go +++ b/weed/filer/filer_rename_test.go @@ -1,6 +1,9 @@ package filer import ( + "context" + "os" + "strings" "testing" "github.com/seaweedfs/seaweedfs/weed/util" @@ -30,25 +33,37 @@ func TestDetectBucket(t *testing.T) { } } -func TestMovesIntoItself(t *testing.T) { +func TestCanRenameIntoItself(t *testing.T) { + f, store := newTestFilerWithStubStore(t) + f.DirBucketsPath = "/buckets" + if err := store.InsertEntry(context.Background(), &Entry{FullPath: "/dir/a", Attr: Attr{Mode: os.ModeDir}}); err != nil { + t.Fatal(err) + } tests := []struct { - name string - sourcePath string - target string - want bool + name string + oldName string + target string + reject bool }{ - {"into itself", "/dir/a", "/dir/a", true}, - {"into a child", "/dir/a", "/dir/a/b", true}, - {"into a deeper descendant", "/dir/a", "/dir/a/b/c", true}, - {"into a sibling sharing the name prefix", "/dir/a", "/dir/a.d", false}, - {"into a descendant of that sibling", "/dir/a", "/dir/ab/c", false}, - {"into the parent", "/dir/a", "/dir", false}, - {"into an unrelated directory", "/dir/a", "/other", false}, + {"into itself", "a", "/dir/a", true}, + {"into a child", "a", "/dir/a/b", true}, + {"into a deeper descendant", "a", "/dir/a/b/c", true}, + {"into a child with a trailing slash", "a", "/dir/a/b/", true}, + {"old name with a trailing slash into a child", "a/", "/dir/a/b", true}, + {"into a sibling sharing the name prefix", "a", "/dir/a.d", false}, + {"into a descendant of that sibling", "a", "/dir/ab/c", false}, + {"into the parent", "a", "/dir", false}, + {"into an unrelated directory", "a", "/other", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := movesIntoItself(util.FullPath(tt.sourcePath), util.FullPath(tt.target)); got != tt.want { - t.Errorf("movesIntoItself(%q, %q) = %v, want %v", tt.sourcePath, tt.target, got, tt.want) + err := f.CanRename(context.Background(), "/dir", util.FullPath(tt.target), tt.oldName) + rejected := err != nil && strings.Contains(err.Error(), "subdirectory of itself") + if rejected != tt.reject { + t.Errorf("CanRename(/dir, %q, %q) = %v, want rejected=%v", tt.target, tt.oldName, err, tt.reject) + } + if !tt.reject && err != nil { + t.Errorf("CanRename(/dir, %q, %q) = %v, want nil", tt.target, tt.oldName, err) } }) }