webdav: overwrite on MOVE when the Overwrite header is missing (#12232)

* webdav: overwrite on MOVE when the Overwrite header is missing

* webdav: defer a MOVE destination's delete into the rename

x/net removes an existing destination before Rename, so a refused or
failed move — a CanRename rejection, a vanished parent, a transient
filer error — loses the target entirely. For an overwriting MOVE onto
an existing destination, run the request against a filesystem that
skips the destination RemoveAll and lets the filer's rename replace the
target inside one transaction instead; a file vs. collection clash
still takes the delete + rename path. Rename itself now tolerates an
existing target, which the atomic filer rename already handles.

* webdav: only default Overwrite when the header has no values

An Overwrite header sent with an empty first value makes Header.Get
return "", which the defaulting treated as absent and rewrote to T —
overriding a client's explicit F in a later value. Check the value list
instead so a present-but-empty header is left alone.

* webdav: move a collection onto a non-empty one via delete + rename

deferredDeleteFS skips the destination RemoveAll so the filer rename can
replace the target in one transaction, but the filer only replaces an
empty collection: moveSelfEntry deletes the target non-recursively and
refuses ErrNonEmptyFolder. Detect a non-empty collection destination and
take the depth-infinity delete + rename path moveFiles would have run.

* webdav: park a clashing MOVE target aside until the rename lands

A refused rename (cross-collection move, bucket target) after deleting
the destination lost its contents; the target now renames to a hidden
sibling first and is restored when the winning rename fails.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* webdav: restore a parked MOVE target under a detached context

A client disconnect cancels the request context mid-move, which also
canceled the restore and stranded the destination under its hidden
sibling name; the restore and parked-entry removal now run under
context.WithoutCancel with a bounded deadline.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* webdav: bound the parked MOVE target's restore after the move

The detached restore deadline started before the source rename, so a
long move could exhaust it; each detached context now starts where its
work begins. A failed parked-entry removal retries briefly and its
warning names the parked sibling for recovery.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* webdav: restore a parked MOVE target without replacing a new claimant

A client can create the destination name while the old target is parked, so the restore rename must fail instead of replacing that new entry.

* webdav: propagate RemoveAll's context to the filer delete

The parked-target cleanup runs under a bounded detached context, but the delete RPC discarded it; passing ctx through makes the deadline effective.

---------

Co-authored-by: user.email <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2026-10-29 22:24:43 -07:00
1 parent 74f727d987
commit 0a4668fcfa
2 files changed
+466 -12

No files matched your search

+145 -12
View File
@@ -96,12 +96,142 @@ type copyMoveGuardHandler struct {
next http.Handler
}
// deferredDeleteFS defers a destination's delete to the rename that follows
// it: x/net removes an existing MOVE destination before Rename, leaving it
// lost when the rename cannot complete. The filer's rename replaces the
// target in one transaction, so the delete is skipped here; a file vs.
// collection clash cannot merge in place and moves the target aside, which
// the refused rename then restores.
type deferredDeleteFS struct {
webdav.FileSystem
dst string
}
func (f deferredDeleteFS) RemoveAll(ctx context.Context, name string) error {
if strings.TrimRight(name, "/") == strings.TrimRight(f.dst, "/") {
return nil
}
return f.FileSystem.RemoveAll(ctx, name)
}
func (f deferredDeleteFS) Rename(ctx context.Context, oldName, newName string) error {
// a file vs. collection clash cannot merge in place, and a collection
// only replaces an empty one — those targets are moved aside rather
// than deleted outright, as moveFiles would have done, so a refused
// rename leaves the destination's contents to restore
src, srcErr := f.FileSystem.Stat(ctx, oldName)
dst, dstErr := f.FileSystem.Stat(ctx, newName)
if srcErr == nil && dstErr == nil {
clashing := src.IsDir() != dst.IsDir()
if !clashing && dst.IsDir() {
nonEmpty, err := nonEmptyDir(ctx, f.FileSystem, newName)
if err != nil {
return err
}
clashing = nonEmpty
}
if clashing {
return renameOverTarget(ctx, f.FileSystem, oldName, newName)
}
}
return f.FileSystem.Rename(ctx, oldName, newName)
}
// renameOverTarget parks a clashing rename target under a hidden sibling
// name so a refused rename can put it back; the parked entry is only
// removed once the move has landed.
func renameOverTarget(ctx context.Context, fs webdav.FileSystem, oldName, newName string) error {
dir, base := path.Split(strings.TrimRight(newName, "/"))
spare := fmt.Sprintf("%s.%s.webdav-moved-%x", dir, base, uint32(util.RandomInt32()))
if err := fs.Rename(ctx, newName, spare); err != nil {
return err
}
if err := fs.Rename(ctx, oldName, newName); err != nil {
// a client disconnect cancels ctx mid-move; restoration gets its
// own deadline, started after the move so its duration is not eaten
restoreCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Minute)
defer cancel()
// newName may have been claimed while the target was parked; the
// restore must refuse to replace it rather than clobber new data
var backErr error
if nrf, ok := fs.(interface {
rename(context.Context, string, string, bool) error
}); ok {
backErr = nrf.rename(restoreCtx, spare, newName, true)
} else {
backErr = fs.Rename(restoreCtx, spare, newName)
}
if backErr != nil {
glog.WarningfCtx(restoreCtx, "restore moved-aside %s to %s: %v", spare, newName, backErr)
}
return err
}
cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Minute)
defer cancel()
var removeErr error
for attempt := 0; attempt < 3; attempt++ {
if attempt > 0 {
time.Sleep(2 * time.Second)
}
if removeErr = fs.RemoveAll(cleanupCtx, spare); removeErr == nil {
return nil
}
}
// the move is done, so it still reports success; the warning names the
// parked sibling for recovery instead of leaving it hidden
glog.WarningfCtx(cleanupCtx, "moved-aside %s remains parked after failed removes: %v", spare, removeErr)
return nil
}
// nonEmptyDir reports whether a collection holds entries: the rename below
// replaces the target's own entry, which a non-empty collection refuses.
func nonEmptyDir(ctx context.Context, fs webdav.FileSystem, name string) (bool, error) {
dir, err := fs.OpenFile(ctx, name, os.O_RDONLY, 0)
if err != nil {
return false, err
}
defer dir.Close()
entries, err := dir.Readdir(1)
if err == io.EOF {
return false, nil
}
return len(entries) > 0, err
}
// moveHandler runs an overwriting MOVE onto an existing destination against
// deferredDeleteFS, so the target survives a refused or failed rename.
func (h copyMoveGuardHandler) moveHandler(r *http.Request) http.Handler {
next, ok := h.next.(*webdav.Handler)
if !ok || r.Header.Get("Overwrite") == "F" {
return h.next
}
u, err := url.Parse(r.Header.Get("Destination"))
if err != nil || u.Host != "" && u.Host != r.Host {
return h.next
}
if _, err := h.fs.Stat(r.Context(), u.Path); err != nil {
return h.next
}
inner := *next
inner.FileSystem = deferredDeleteFS{FileSystem: inner.FileSystem, dst: u.Path}
return &inner
}
func (h copyMoveGuardHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// RFC 4918 10.6: a missing Overwrite header means T, but webdav.Handler
// only overwrites on MOVE when it is set (golang/go#66059)
if r.Method == "MOVE" && len(r.Header.Values("Overwrite")) == 0 {
r.Header.Set("Overwrite", "T")
}
next := h.next
if r.Method == "MOVE" {
next = h.moveHandler(r)
}
if r.Method == "MOVE" || r.Method == "COPY" {
// a foreign-host Destination is another server's problem: the webdav
// handler answers 502, so the local checks below do not apply
if u, err := url.Parse(r.Header.Get("Destination")); err == nil && u.Host != "" && u.Host != r.Host {
h.next.ServeHTTP(w, r)
next.ServeHTTP(w, r)
return
}
if u, err := url.Parse(r.Header.Get("Destination")); err == nil {
@@ -155,7 +285,7 @@ func (h copyMoveGuardHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
}
}
}
h.next.ServeHTTP(w, r)
next.ServeHTTP(w, r)
}
// adapted from https://github.com/mattn/davfs/blob/master/plugin/mysql/mysql.go
@@ -417,7 +547,7 @@ func (fs *WebDavFileSystem) removeAll(ctx context.Context, fullFilePath string,
}
dir, name := fullpath.DirAndName()
return filer_pb.Remove(context.Background(), fs, dir, name, true, recursive, false, false, []int32{fs.signature})
return filer_pb.Remove(ctx, fs, dir, name, true, recursive, false, false, []int32{fs.signature})
}
@@ -429,6 +559,10 @@ func (fs *WebDavFileSystem) RemoveAll(ctx context.Context, name string) error {
}
func (fs *WebDavFileSystem) Rename(ctx context.Context, oldName, newName string) error {
return fs.rename(ctx, oldName, newName, false)
}
func (fs *WebDavFileSystem) rename(ctx context.Context, oldName, newName string, failIfExists bool) error {
glog.V(2).Infof("WebDavFileSystem.Rename %v to %v", oldName, newName)
@@ -449,11 +583,9 @@ func (fs *WebDavFileSystem) Rename(ctx context.Context, oldName, newName string)
return os.ErrInvalid
}
_, err = fs.stat(ctx, newName)
if err == nil {
return os.ErrExist
}
if err != os.ErrNotExist {
// an existing target is replaced inside the rename's transaction; a
// failed lookup aborts the move here
if _, err = fs.stat(ctx, newName); err != nil && err != os.ErrNotExist {
return err
}
@@ -467,10 +599,11 @@ func (fs *WebDavFileSystem) Rename(ctx context.Context, oldName, newName string)
return fs.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
request := &filer_pb.AtomicRenameEntryRequest{
OldDirectory: oldDir,
OldName: oldBaseName,
NewDirectory: newDir,
NewName: newBaseName,
OldDirectory: oldDir,
OldName: oldBaseName,
NewDirectory: newDir,
NewName: newBaseName,
FailIfTargetExists: failIfExists,
}
_, err := client.AtomicRenameEntry(ctx, request)
+321
View File
@@ -504,6 +504,327 @@ func (f *stubDavFs) Stat(_ context.Context, name string) (os.FileInfo, error) {
}
// A PUT onto a folder must not delete the folder to make room for the file.
// Without an Overwrite header a MOVE replaces an existing destination, as
// COPY already does.
func TestCopyMoveGuardHandler_MoveWithoutOverwriteHeader(t *testing.T) {
ctx := context.Background()
fs := webdav.NewMemFS()
for name, data := range map[string]string{"/a.txt": "a", "/b.txt": "b"} {
f, err := fs.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE, 0644)
if err != nil {
t.Fatal(err)
}
if _, err := f.Write([]byte(data)); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
}
h := copyMoveGuardHandler{
fs: fs,
next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()},
}
req := httptest.NewRequest("MOVE", "http://x/a.txt", nil)
req.Header.Set("Destination", "http://x/b.txt")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusNoContent, w.Code)
r, err := fs.OpenFile(ctx, "/b.txt", os.O_RDONLY, 0)
if assert.NoError(t, err) {
data, err := io.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, "a", string(data))
}
_, err = fs.Stat(ctx, "/a.txt")
assert.True(t, os.IsNotExist(err))
}
// An Overwrite header with an empty first value is present, not missing, so
// the default must not overwrite it — a later explicit F forbids the move.
func TestCopyMoveGuardHandler_MoveEmptyOverwriteValue(t *testing.T) {
ctx := context.Background()
fs := webdav.NewMemFS()
for name, data := range map[string]string{"/a.txt": "a", "/b.txt": "b"} {
f, err := fs.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE, 0644)
if err != nil {
t.Fatal(err)
}
if _, err := f.Write([]byte(data)); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
}
h := copyMoveGuardHandler{
fs: fs,
next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()},
}
req := httptest.NewRequest("MOVE", "http://x/a.txt", nil)
req.Header.Set("Destination", "http://x/b.txt")
req.Header["Overwrite"] = []string{"", "F"}
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusPreconditionFailed, w.Code)
r, err := fs.OpenFile(ctx, "/b.txt", os.O_RDONLY, 0)
if assert.NoError(t, err) {
data, err := io.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, "b", string(data))
}
}
// An overwriting MOVE of a collection onto a non-empty collection removes the
// destination: the deferred rename only merges over an empty one, so this
// case parks the target aside and deletes it once the move has landed.
func TestCopyMoveGuardHandler_MoveDirOntoNonEmptyDir(t *testing.T) {
ctx := context.Background()
fs := webdav.NewMemFS()
put := func(name, data string) {
f, err := fs.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE, 0644)
if err != nil {
t.Fatal(err)
}
if _, err := f.Write([]byte(data)); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
}
if err := fs.Mkdir(ctx, "/src", 0755); err != nil {
t.Fatal(err)
}
put("/src/s.txt", "s")
if err := fs.Mkdir(ctx, "/dst", 0755); err != nil {
t.Fatal(err)
}
put("/dst/d.txt", "d")
h := copyMoveGuardHandler{
fs: fs,
next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()},
}
req := httptest.NewRequest("MOVE", "http://x/src", nil)
req.Header.Set("Destination", "http://x/dst")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusNoContent, w.Code)
_, err := fs.Stat(ctx, "/dst/d.txt")
assert.True(t, os.IsNotExist(err))
if r, err := fs.OpenFile(ctx, "/dst/s.txt", os.O_RDONLY, 0); assert.NoError(t, err) {
data, err := io.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, "s", string(data))
}
_, err = fs.Stat(ctx, "/src")
assert.True(t, os.IsNotExist(err))
}
// An overwriting MOVE of a collection onto an empty collection keeps the
// deferred path: the destination survives if the rename itself fails.
func TestCopyMoveGuardHandler_MoveDirOntoEmptyDirKeepsDestinationOnFailure(t *testing.T) {
ctx := context.Background()
mem := webdav.NewMemFS()
if err := mem.Mkdir(ctx, "/src", 0755); err != nil {
t.Fatal(err)
}
if err := mem.Mkdir(ctx, "/dst", 0755); err != nil {
t.Fatal(err)
}
fs := failRenameFS{mem}
h := copyMoveGuardHandler{
fs: fs,
next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()},
}
req := httptest.NewRequest("MOVE", "http://x/src", nil)
req.Header.Set("Destination", "http://x/dst")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusForbidden, w.Code)
if fi, err := mem.Stat(ctx, "/dst"); assert.NoError(t, err) {
assert.True(t, fi.IsDir())
}
}
type failRenameFS struct {
webdav.FileSystem
}
func (f failRenameFS) Rename(context.Context, string, string) error {
return fmt.Errorf("rename refused")
}
type failPairRenameFS struct {
webdav.FileSystem
oldName, newName string
}
func (f failPairRenameFS) Rename(ctx context.Context, oldName, newName string) error {
if oldName == f.oldName && newName == f.newName {
return fmt.Errorf("rename refused")
}
return f.FileSystem.Rename(ctx, oldName, newName)
}
// A MOVE of a collection onto a non-empty collection whose rename is refused
// (cross-collection moves, bucket targets) must bring the destination back:
// it is only parked aside while the winning rename runs.
func TestCopyMoveGuardHandler_MoveDirOntoNonEmptyDirRestoresOnRenameFailure(t *testing.T) {
ctx := context.Background()
mem := webdav.NewMemFS()
put := func(name, data string) {
f, err := mem.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE, 0644)
if err != nil {
t.Fatal(err)
}
if _, err := f.Write([]byte(data)); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
}
if err := mem.Mkdir(ctx, "/src", 0755); err != nil {
t.Fatal(err)
}
put("/src/s.txt", "s")
if err := mem.Mkdir(ctx, "/dst", 0755); err != nil {
t.Fatal(err)
}
put("/dst/d.txt", "d")
fs := failPairRenameFS{mem, "/src", "/dst"}
h := copyMoveGuardHandler{
fs: fs,
next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()},
}
req := httptest.NewRequest("MOVE", "http://x/src", nil)
req.Header.Set("Destination", "http://x/dst")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusForbidden, w.Code)
if r, err := mem.OpenFile(ctx, "/dst/d.txt", os.O_RDONLY, 0); assert.NoError(t, err) {
data, err := io.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, "d", string(data))
r.Close()
}
if r, err := mem.OpenFile(ctx, "/src/s.txt", os.O_RDONLY, 0); assert.NoError(t, err) {
r.Close()
}
root, err := mem.OpenFile(ctx, "/", os.O_RDONLY, 0)
if assert.NoError(t, err) {
entries, err := root.Readdir(-1)
assert.NoError(t, err)
names := make([]string, 0, len(entries))
for _, e := range entries {
names = append(names, e.Name())
}
assert.ElementsMatch(t, []string{"src", "dst"}, names)
}
}
type cancelingRenameFS struct {
webdav.FileSystem
cancel context.CancelFunc
oldName, newName string
}
func (f cancelingRenameFS) Rename(ctx context.Context, oldName, newName string) error {
if oldName == f.oldName && newName == f.newName {
f.cancel()
}
if err := ctx.Err(); err != nil {
return err
}
return f.FileSystem.Rename(ctx, oldName, newName)
}
// A client disconnect cancels the request context mid-move; restoring the
// parked destination runs under a detached context, so the destination is
// not left under its hidden name.
func TestDeferredDeleteFS_RestoreSurvivesCanceledContext(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
mem := webdav.NewMemFS()
put := func(name, data string) {
f, err := mem.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE, 0644)
if err != nil {
t.Fatal(err)
}
if _, err := f.Write([]byte(data)); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
}
if err := mem.Mkdir(ctx, "/src", 0755); err != nil {
t.Fatal(err)
}
put("/src/s.txt", "s")
if err := mem.Mkdir(ctx, "/dst", 0755); err != nil {
t.Fatal(err)
}
put("/dst/d.txt", "d")
fs := deferredDeleteFS{
FileSystem: cancelingRenameFS{mem, cancel, "/src", "/dst"},
dst: "/dst",
}
err := fs.Rename(ctx, "/src", "/dst")
assert.ErrorIs(t, err, context.Canceled)
if r, err := mem.OpenFile(context.Background(), "/dst/d.txt", os.O_RDONLY, 0); assert.NoError(t, err) {
data, err := io.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, "d", string(data))
r.Close()
}
}
// A MOVE whose rename fails must leave the existing destination in place:
// the delete is deferred into the rename, which replaces atomically.
func TestCopyMoveGuardHandler_MoveKeepsDestinationOnRenameFailure(t *testing.T) {
ctx := context.Background()
mem := webdav.NewMemFS()
for name, data := range map[string]string{"/a.txt": "a", "/b.txt": "b"} {
f, err := mem.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE, 0644)
if err != nil {
t.Fatal(err)
}
if _, err := f.Write([]byte(data)); err != nil {
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
}
fs := failRenameFS{mem}
h := copyMoveGuardHandler{
fs: fs,
next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()},
}
req := httptest.NewRequest("MOVE", "http://x/a.txt", nil)
req.Header.Set("Destination", "http://x/b.txt")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
assert.Equal(t, http.StatusForbidden, w.Code)
r, err := mem.OpenFile(ctx, "/b.txt", os.O_RDONLY, 0)
if assert.NoError(t, err) {
data, err := io.ReadAll(r)
assert.NoError(t, err)
assert.Equal(t, "b", string(data))
}
}
func TestWebDavFileSystem_OpenFile_DoesNotReplaceFolder(t *testing.T) {
filer := &deleteRecordingFiler{existing: &filer_pb.Entry{
Name: "sub",