msb-public
/
cloudreve
mirror of https://github.com/cloudreve/Cloudreve
1
0
Code Issues Projects Releases Wiki Activity

fix(dav): guard renamed move/copy against intermediate name collisions

Browse Source 
A MOVE or COPY to a renamed destination lands at dstFolder/srcName
before the rename step. When a *different* resource already occupies
that path (dstFolder/x.txt while moving x.txt -> y.txt), SetParent hits
UNIQUE(parent, name) mid-operation and surfaces as 405 — a residual of
#3537 that upstream's dst-delete fix does not cover.

performCopyMove now resolves dstFolder/srcName first: if it exists and
is not the source itself (same-folder rename), return 412. Same-name
operations skip the check entirely.

Authored By: TDvorak <info@tdvorak.dev>

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pull/3582/head
Tomas Dvorak 2 weeks ago
parent 84b29430f1
commit fb96eaebaa
2 changed files with 70 additions and 13 deletions
Show Stats Download Patch File Download Diff File

18
pkg/webdav/webdav.go
Unescape View File

@ -875,7 +875,7 @@ func handleCopyMove(c *gin.Context, user *ent.User, fm manager.FileManager) (sta
}
}
return performCopyMove(ctx, fm, srcUri, dstUri, dstFolderUri, true, overwrite, dstExists)
return performCopyMove(ctx, fm, srcUri, dstUri, dstFolderUri, true, overwrite, dstExists, hashid.EncodeUserID(hasher, user.ID))
}
release, ls, status, err := confirmLock(c, fm, user, srcTarget, dstTarget, srcUri, dstUri)
@ -893,13 +893,14 @@ func handleCopyMove(c *gin.Context, user *ent.User, fm manager.FileManager) (sta
return http.StatusBadRequest, errInvalidDepth
}
}
return performCopyMove(ctx, fm, srcUri, dstUri, dstFolderUri, false, overwrite, dstExists)
return performCopyMove(ctx, fm, srcUri, dstUri, dstFolderUri, false, overwrite, dstExists, hashid.EncodeUserID(hasher, user.ID))
}
type copyMoveOperations interface {
Delete(ctx context.Context, path []*fs.URI, opts ...fs.Option) error
MoveOrCopy(ctx context.Context, src []*fs.URI, dst *fs.URI, isCopy bool) error
Rename(ctx context.Context, path *fs.URI, newName string) (fs.File, error)
SharedAddressTranslation(ctx context.Context, path *fs.URI, opts ...fs.Option) (fs.File, *fs.URI, error)
}
func parseOverwrite(value string) (bool, error) {
@ -918,7 +919,20 @@ func performCopyMove(
fm copyMoveOperations,
srcUri, dstUri, dstFolderUri *fs.URI,
isCopy, overwrite, dstExists bool,
uid string,
) (int, error) {
if srcUri.Name() != dstUri.Name() {
// A renamed move/copy lands at dstFolder/srcName before the rename;
// a different resource already occupying that path would hit the
// UNIQUE(parent, name) constraint mid-operation.
_, intermediateUri, err := fm.SharedAddressTranslation(ctx, dstFolderUri.Join(srcUri.Name()))
if err == nil && !intermediateUri.IsSame(srcUri, uid) {
return http.StatusPreconditionFailed, errDestinationExists
} else if err != nil && !ent.IsNotFound(err) {
return purposeStatusCodeFromError(err), err
}
}
if dstExists {
if !overwrite {
return http.StatusPreconditionFailed, errDestinationExists

65
pkg/webdav/webdav_test.go
Unescape View File

@ -13,11 +13,14 @@ import (
)
type copyMoveOperationsStub struct {
calls []string
deleteErr error
moveErr error
renameErr error
moveIsCopy bool
calls []string
deleteErr error
moveErr error
renameErr error
moveIsCopy bool
translateURI *fs.URI
translateErr error
translateHits int
}
func (s *copyMoveOperationsStub) Delete(context.Context, []*fs.URI, ...fs.Option) error {
@ -36,6 +39,11 @@ func (s *copyMoveOperationsStub) Rename(context.Context, *fs.URI, string) (fs.Fi
return nil, s.renameErr
}
func (s *copyMoveOperationsStub) SharedAddressTranslation(context.Context, *fs.URI, ...fs.Option) (fs.File, *fs.URI, error) {
s.translateHits++
return nil, s.translateURI, s.translateErr
}
func TestParseOverwrite(t *testing.T) {
tests := []struct {
value string
@ -67,8 +75,8 @@ func TestPerformCopyMove(t *testing.T) {
dstFolder := dst.DirUri()
t.Run("overwrite disabled", func(t *testing.T) {
operations := &copyMoveOperationsStub{}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, false, false, true)
operations := &copyMoveOperationsStub{translateErr: &ent.NotFoundError{}}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, false, false, true, "u1")
if status != http.StatusPreconditionFailed || !errors.Is(err, errDestinationExists) {
t.Fatalf("unexpected result: status=%d err=%v", status, err)
}
@ -78,8 +86,8 @@ func TestPerformCopyMove(t *testing.T) {
})
t.Run("overwrite existing move", func(t *testing.T) {
operations := &copyMoveOperationsStub{}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, false, true, true)
operations := &copyMoveOperationsStub{translateErr: &ent.NotFoundError{}}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, false, true, true, "u1")
if err != nil || status != http.StatusNoContent {
t.Fatalf("unexpected result: status=%d err=%v", status, err)
}
@ -92,8 +100,8 @@ func TestPerformCopyMove(t *testing.T) {
})
t.Run("new copy", func(t *testing.T) {
operations := &copyMoveOperationsStub{}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, true, true, false)
operations := &copyMoveOperationsStub{translateErr: &ent.NotFoundError{}}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, true, true, false, "u1")
if err != nil || status != http.StatusCreated {
t.Fatalf("unexpected result: status=%d err=%v", status, err)
}
@ -104,6 +112,41 @@ func TestPerformCopyMove(t *testing.T) {
t.Fatal("copy was executed as move")
}
})
t.Run("intermediate src name occupied", func(t *testing.T) {
// dstFolder already holds a different file named src.Name(): the
// intermediate dstFolder/srcName state would collide mid-operation.
operations := &copyMoveOperationsStub{translateURI: mustWebDAVTestURI(t, "cloudreve://my/elsewhere")}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, false, true, true, "u1")
if status != http.StatusPreconditionFailed || !errors.Is(err, errDestinationExists) {
t.Fatalf("unexpected result: status=%d err=%v", status, err)
}
if len(operations.calls) != 0 {
t.Fatalf("unexpected operations: %v", operations.calls)
}
})
t.Run("intermediate is src itself", func(t *testing.T) {
// Same-folder rename: dstFolder/srcName resolves back to the source.
operations := &copyMoveOperationsStub{translateURI: src}
status, err := performCopyMove(context.Background(), operations, src, dst, dstFolder, false, true, true, "u1")
if err != nil || status != http.StatusNoContent {
t.Fatalf("unexpected result: status=%d err=%v", status, err)
}
})
t.Run("same-name move skips intermediate check", func(t *testing.T) {
sameNameDst := mustWebDAVTestURI(t, "cloudreve://my/other/source.temp")
sameNameFolder := sameNameDst.DirUri()
operations := &copyMoveOperationsStub{translateURI: mustWebDAVTestURI(t, "cloudreve://my/other/source.temp")}
status, err := performCopyMove(context.Background(), operations, src, sameNameDst, sameNameFolder, false, true, true, "u1")
if err != nil || status != http.StatusNoContent {
t.Fatalf("unexpected result: status=%d err=%v", status, err)
}
if operations.translateHits != 0 {
t.Fatalf("intermediate check ran for same-name move")
}
})
}
func mustWebDAVTestURI(t *testing.T, raw string) *fs.URI {

Write Preview
Loading…
Cancel
Save