diff --git a/weed/server/webdav_server.go b/weed/server/webdav_server.go index 6abaa796da..2fc6fd321d 100644 --- a/weed/server/webdav_server.go +++ b/weed/server/webdav_server.go @@ -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{ diff --git a/weed/server/webdav_server_test.go b/weed/server/webdav_server_test.go index 61f418364e..b59bc18120 100644 --- a/weed/server/webdav_server_test.go +++ b/weed/server/webdav_server_test.go @@ -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) +}