webdav: refuse MKCOL and PUT under a missing folder with 409 (#12122)

* webdav: refuse MKCOL and PUT under a missing folder with 409 RFC 4918 9.3 and 9.7.1 say a MKCOL or PUT whose parent collection does not exist must fail with 409 Conflict. The filer creates missing parents, so both succeeded and made the intermediate folders. Check the parent in Mkdir and in OpenFile with O_CREATE and return os.ErrNotExist, which x/net/webdav turns into 409. * webdav: check the PUT parent before an overwrite deletes the target OpenFile deleted an existing destination before the new parent lookup, so a PUT whose parent check failed left the old file gone. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: report a missing rename source as not-exist Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: refuse MOVE to a missing destination folder AtomicRenameEntry grows any missing parents itself, so a MOVE to /missing/file created /missing; check the destination parent first. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: answer 409 for COPY and MOVE under a missing collection The webdav handler reports a failed directory Mkdir or Rename as 403, so the destination parent is checked at the request layer instead. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: defer foreign-host MOVE and COPY destinations to the 502 handler * webdav: only check the destination parent for absolute paths * webdav: cover foreign-host and relative MOVE destinations in the guard test --------- Co-authored-by: user.email <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-27 23:40:41 +01:00 · 2026-10-27 01:34:11 -07:00
1 parent 5c14e566bc
commit a5036f568f
@@ -98,6 +98,12 @@ type copyMoveGuardHandler struct {

 func (h copyMoveGuardHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	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)
+			return
+		}
 		if u, err := url.Parse(r.Header.Get("Destination")); err == nil {
 			src := strings.Trim(path.Clean(r.URL.Path), "/")
 			dst := strings.Trim(path.Clean(u.Path), "/")
@@ -129,6 +135,25 @@ func (h copyMoveGuardHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 				r.Header.Set("Destination", u.String())
 			}
 		}
+		// the destination's parent collection must exist; Mkdir and Rename
+		// errors surface as 403 below, not the 409 a missing parent requires
+		if u, err := url.Parse(r.Header.Get("Destination")); err == nil {
+			if dst := path.Clean(u.Path); strings.HasPrefix(dst, "/") && dst != "/" {
+				if parent := path.Dir(dst); parent != "/" {
+					if pfi, err := h.fs.Stat(r.Context(), parent); err != nil {
+						if os.IsNotExist(err) {
+							w.WriteHeader(http.StatusConflict)
+						} else {
+							w.WriteHeader(http.StatusInternalServerError)
+						}
+						return
+					} else if !pfi.IsDir() {
+						w.WriteHeader(http.StatusConflict)
+						return
+					}
+				}
+			}
+		}
 	}
 	h.next.ServeHTTP(w, r)
 }
@@ -251,8 +276,12 @@ func (fs *WebDavFileSystem) Mkdir(ctx context.Context, fullDirPath string, perm
 		return err
 	}

+	dir, name := util.NormalizePath(fullDirPath).DirAndName()
+	if err := fs.checkParentDir(ctx, dir); err != nil {
+		return err
+	}
+
 	return fs.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
-		dir, name := util.NormalizePath(fullDirPath).DirAndName()
 		request := &filer_pb.CreateEntryRequest{
 			Directory: dir,
 			Entry: &filer_pb.Entry{
@@ -278,6 +307,23 @@ func (fs *WebDavFileSystem) Mkdir(ctx context.Context, fullDirPath string, perm
 	})
 }

+// checkParentDir returns os.ErrNotExist, which x/net/webdav answers with 409
+// Conflict, when dir is missing or is not a folder: RFC 4918 9.3 and 9.7.1
+// forbid creating the intermediate collections of a MKCOL or PUT.
+func (fs *WebDavFileSystem) checkParentDir(ctx context.Context, dir string) error {
+	if dir == "/" {
+		return nil
+	}
+	fi, err := fs.stat(ctx, dir)
+	if err != nil {
+		return err
+	}
+	if !fi.IsDir() {
+		return os.ErrNotExist
+	}
+	return nil
+}
+
 func (fs *WebDavFileSystem) OpenFile(ctx context.Context, fullFilePath string, flag int, perm os.FileMode) (webdav.File, error) {
 	glog.V(2).Infof("WebDavFileSystem.OpenFile %v %x", fullFilePath, flag)

@@ -291,6 +337,10 @@ func (fs *WebDavFileSystem) OpenFile(ctx context.Context, fullFilePath string, f
 		if strings.HasSuffix(fullFilePath, "/") {
 			return nil, os.ErrInvalid
 		}
+		dir, name := util.FullPath(fullFilePath).DirAndName()
+		if err := fs.checkParentDir(ctx, dir); err != nil {
+			return nil, err
+		}
 		fi, err := fs.stat(ctx, fullFilePath)
 		if err == nil {
 			if flag&os.O_EXCL != 0 || fi.IsDir() {
@@ -301,7 +351,6 @@ func (fs *WebDavFileSystem) OpenFile(ctx context.Context, fullFilePath string, f
 			return nil, err
 		}

-		dir, name := util.FullPath(fullFilePath).DirAndName()
 		err = fs.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
 			if err := filer_pb.CreateEntry(context.Background(), client, &filer_pb.CreateEntryRequest{
 				Directory: dir,
@@ -392,7 +441,7 @@ func (fs *WebDavFileSystem) Rename(ctx context.Context, oldName, newName string)
 	}

 	if _, err = fs.stat(ctx, oldName); err != nil {
-		return os.ErrExist
+		return err
 	}
 	oldName = string(util.NormalizePath(oldName))
 	newName = string(util.NormalizePath(newName))
@@ -411,6 +460,10 @@ func (fs *WebDavFileSystem) Rename(ctx context.Context, oldName, newName string)
 	oldDir, oldBaseName := util.FullPath(oldName).DirAndName()
 	newDir, newBaseName := util.FullPath(newName).DirAndName()

+	if err := fs.checkParentDir(ctx, newDir); err != nil {
+		return err
+	}
+
 	return fs.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {

 		request := &filer_pb.AtomicRenameEntryRequest{
@@ -72,10 +72,27 @@ func (f *noVolumeFiler) LookupVolume(_ context.Context, _ *filer_pb.LookupVolume

 type createRecordingFiler struct {
 	filer_pb.UnimplementedSeaweedFilerServer
-	req *filer_pb.CreateEntryRequest
+	req    *filer_pb.CreateEntryRequest
+	delReq *filer_pb.DeleteEntryRequest
+	dirs   map[string]bool // folders that exist, by full path
+	files  map[string]bool // files that exist, by full path
 }

-func (f *createRecordingFiler) LookupDirectoryEntry(_ context.Context, _ *filer_pb.LookupDirectoryEntryRequest) (*filer_pb.LookupDirectoryEntryResponse, error) {
+func (f *createRecordingFiler) LookupDirectoryEntry(_ context.Context, req *filer_pb.LookupDirectoryEntryRequest) (*filer_pb.LookupDirectoryEntryResponse, error) {
+	p := string(util.NewFullPath(req.Directory, req.Name))
+	if f.dirs[p] {
+		return &filer_pb.LookupDirectoryEntryResponse{Entry: &filer_pb.Entry{
+			Name:        req.Name,
+			IsDirectory: true,
+			Attributes:  &filer_pb.FuseAttributes{FileMode: uint32(os.ModeDir | 0755)},
+		}}, nil
+	}
+	if f.files[p] {
+		return &filer_pb.LookupDirectoryEntryResponse{Entry: &filer_pb.Entry{
+			Name:       req.Name,
+			Attributes: &filer_pb.FuseAttributes{FileMode: 0644},
+		}}, nil
+	}
 	return nil, filer_pb.ErrNotFound
 }

@@ -84,6 +101,11 @@ func (f *createRecordingFiler) CreateEntry(_ context.Context, req *filer_pb.Crea
 	return &filer_pb.CreateEntryResponse{}, nil
 }

+func (f *createRecordingFiler) DeleteEntry(_ context.Context, req *filer_pb.DeleteEntryRequest) (*filer_pb.DeleteEntryResponse, error) {
+	f.delReq = req
+	return &filer_pb.DeleteEntryResponse{}, nil
+}
+
 type deleteRecordingFiler struct {
 	filer_pb.UnimplementedSeaweedFilerServer
 	req      *filer_pb.DeleteEntryRequest
@@ -164,7 +186,7 @@ func TestWebDavFile_Read_InlineContent(t *testing.T) {

 // MKCOL paths end in "/"; the entry must be created under its own name, not as "" inside itself.
 func TestWebDavFileSystem_Mkdir_EntryName(t *testing.T) {
-	filer := &createRecordingFiler{}
+	filer := &createRecordingFiler{dirs: map[string]bool{"/dir": true}}
 	filerAddr := startFakeWebDavFiler(t, filer)
 	fs := &WebDavFileSystem{option: &WebDavOption{
 		Filer:          filerAddr,
@@ -179,6 +201,40 @@ func TestWebDavFileSystem_Mkdir_EntryName(t *testing.T) {
 	}
 }

+// MKCOL and PUT under a missing folder must fail with 409 (RFC 4918 9.3, 9.7.1),
+// which x/net/webdav answers for os.ErrNotExist, and must not create anything.
+func TestWebDavFileSystem_MissingParent(t *testing.T) {
+	filer := &createRecordingFiler{}
+	filerAddr := startFakeWebDavFiler(t, filer)
+	fs := &WebDavFileSystem{option: &WebDavOption{
+		Filer:          filerAddr,
+		GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+	}}
+
+	assert.ErrorIs(t, fs.Mkdir(context.Background(), "/missing/sub/", 0755), os.ErrNotExist)
+	_, err := fs.OpenFile(context.Background(), "/missing/f.txt", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
+	assert.ErrorIs(t, err, os.ErrNotExist)
+	assert.Nil(t, filer.req)
+
+	assert.NoError(t, fs.Mkdir(context.Background(), "/top/", 0755), "a folder at the root needs no parent")
+}
+
+// A PUT overwrite checks the parent before deleting the file it replaces:
+// a failed check must leave the old file in place.
+func TestWebDavFileSystem_OverwriteKeepsTargetOnParentFailure(t *testing.T) {
+	filer := &createRecordingFiler{files: map[string]bool{"/missing/f.txt": true}}
+	filerAddr := startFakeWebDavFiler(t, filer)
+	fs := &WebDavFileSystem{option: &WebDavOption{
+		Filer:          filerAddr,
+		GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+	}}
+
+	_, err := fs.OpenFile(context.Background(), "/missing/f.txt", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
+	assert.ErrorIs(t, err, os.ErrNotExist)
+	assert.Nil(t, filer.delReq)
+	assert.Nil(t, filer.req)
+}
+
 // DELETE, and MOVE or COPY with Overwrite: T, remove a collection through RemoveAll, which must take its contents too.
 func TestWebDavFileSystem_RemoveAll_Recursive(t *testing.T) {
 	filer := &deleteRecordingFiler{}
@@ -215,7 +271,10 @@ func TestWebDavFileSystem_RemoveAll_Recursive(t *testing.T) {
 func TestCopyMoveGuardHandler(t *testing.T) {
 	called := false
 	h := copyMoveGuardHandler{
-		fs: &stubDavFs{present: map[string]bool{"/dir": true, "/dir/sub": true, "/dirs": true, "/dir/other": true, "/anywhere": true}},
+		fs: &stubDavFs{
+			present: map[string]bool{"/dir": true, "/dir/sub": true, "/dirs": true, "/dir/other": true, "/anywhere": true},
+			dirs:    map[string]bool{"/dir": true, "/dirs": true},
+		},
 		next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			called = true
 		}),
@@ -280,6 +339,47 @@ func TestCopyMoveGuardHandler(t *testing.T) {
 	}
 }

+// MOVE or COPY under a missing collection is a 409: the webdav handler would
+// only report the failed Mkdir or Rename as 403.
+func TestCopyMoveGuardHandler_MissingDestParent(t *testing.T) {
+	called := false
+	h := copyMoveGuardHandler{
+		fs: &stubDavFs{
+			present:  map[string]bool{"/dir": true, "/dir/sub": true, "/file": true},
+			dirs:     map[string]bool{"/dir": true},
+			errPaths: map[string]bool{"/broken": true},
+		},
+		next: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true }),
+	}
+
+	for _, tc := range []struct {
+		method, src, dst string
+		want             int
+	}{
+		{"MOVE", "/dir/sub", "http://x/missing/sub", http.StatusConflict},
+		{"COPY", "/dir/sub", "http://x/missing/sub", http.StatusConflict},
+		{"COPY", "/dir", "http://x/missing/deep/dir", http.StatusConflict},
+		{"MOVE", "/dir/sub", "http://x/file/sub", http.StatusConflict}, // parent is a file
+		{"MOVE", "/dir/sub", "http://x/broken/sub", http.StatusInternalServerError},
+		{"COPY", "/dir/sub", "http://x/dir/other", 0},   // parent exists
+		{"MOVE", "/dir/sub", "http://x/moved", 0},       // destination at the root
+		{"MOVE", "/dir/sub", "http://y/missing/sub", 0}, // foreign host is a 502, not ours to judge
+		{"MOVE", "/dir/sub", "missing/sub", 0},          // a relative destination fails downstream
+	} {
+		called = false
+		req := httptest.NewRequest(tc.method, "http://x"+tc.src, nil)
+		req.Header.Set("Destination", tc.dst)
+		w := httptest.NewRecorder()
+		h.ServeHTTP(w, req)
+		if tc.want == 0 {
+			assert.True(t, called, "%s %s -> %s", tc.method, tc.src, tc.dst)
+		} else {
+			assert.Equal(t, tc.want, w.Code, "%s %s -> %s", tc.method, tc.src, tc.dst)
+			assert.False(t, called)
+		}
+	}
+}
+
 // A file copied or moved onto a collection URL is created at the name without
 // the slash: OpenFile refuses "name/" only after the collection is removed.
 func TestCopyMoveGuardHandler_FileOntoCollectionURL(t *testing.T) {
@@ -423,10 +523,18 @@ func TestWebDavFileSystem_OpenFile_DoesNotReplaceFolder(t *testing.T) {

 type renameRecordingFiler struct {
 	filer_pb.UnimplementedSeaweedFilerServer
-	req *filer_pb.AtomicRenameEntryRequest
+	req  *filer_pb.AtomicRenameEntryRequest
+	dirs map[string]bool // folders that exist, by full path
 }

 func (f *renameRecordingFiler) LookupDirectoryEntry(_ context.Context, req *filer_pb.LookupDirectoryEntryRequest) (*filer_pb.LookupDirectoryEntryResponse, error) {
+	if f.dirs[string(util.NewFullPath(req.Directory, req.Name))] {
+		return &filer_pb.LookupDirectoryEntryResponse{Entry: &filer_pb.Entry{
+			Name:        req.Name,
+			IsDirectory: true,
+			Attributes:  &filer_pb.FuseAttributes{FileMode: uint32(os.ModeDir | 0755)},
+		}}, nil
+	}
 	if req.Name != "file.txt" {
 		return nil, filer_pb.ErrNotFound
 	}
@@ -440,7 +548,7 @@ func (f *renameRecordingFiler) AtomicRenameEntry(_ context.Context, req *filer_p

 // MOVE of a file onto an existing folder removes the folder, then renames onto "/dir/coll/".
 func TestWebDavFileSystem_Rename_FileOntoSlashedPath(t *testing.T) {
-	filer := &renameRecordingFiler{}
+	filer := &renameRecordingFiler{dirs: map[string]bool{"/dir": true}}
 	filerAddr := startFakeWebDavFiler(t, filer)
 	fs := &WebDavFileSystem{option: &WebDavOption{
 		Filer:          filerAddr,
@@ -453,3 +561,21 @@ func TestWebDavFileSystem_Rename_FileOntoSlashedPath(t *testing.T) {
 		assert.Equal(t, "coll", filer.req.NewName)
 	}
 }
+
+// MOVE to a missing destination folder must fail before the rename: the filer
+// grows any missing parents itself, so an unchecked call would create them.
+func TestWebDavFileSystem_Rename_MissingDestParent(t *testing.T) {
+	filer := &renameRecordingFiler{dirs: map[string]bool{"/dir": true}}
+	filerAddr := startFakeWebDavFiler(t, filer)
+	fs := &WebDavFileSystem{option: &WebDavOption{
+		Filer:          filerAddr,
+		GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+	}}
+
+	assert.ErrorIs(t, fs.Rename(context.Background(), "/dir/file.txt", "/missing/moved.txt"), os.ErrNotExist)
+	assert.ErrorIs(t, fs.Rename(context.Background(), "/dir/nothere.txt", "/dir/file2.txt"), os.ErrNotExist)
+	assert.Nil(t, filer.req)
+
+	assert.NoError(t, fs.Rename(context.Background(), "/dir/file.txt", "/dir/moved.txt"))
+	assert.NotNil(t, filer.req)
+}