diff --git a/weed/server/webdav_server.go b/weed/server/webdav_server.go index 5ca6707298..90576944e8 100644 --- a/weed/server/webdav_server.go +++ b/weed/server/webdav_server.go @@ -2,6 +2,7 @@ package weed_server import ( "context" + "errors" "fmt" "io" "net/http" @@ -88,6 +89,33 @@ func NewWebDavServer(option *WebDavOption) (ws *WebDavServer, err error) { return ws, nil } +// putOutcomeKey carries the flag WebDavFileSystem.OpenFile sets when a PUT +// replaced an existing entry, so the PUT can answer 204 instead of 201 +type putOutcomeKey struct{} + +// putOutcomeWriter turns the 201 of a PUT that replaced a file into 204 +// and swallows the status text webdav.Handler writes after it +type putOutcomeWriter struct { + http.ResponseWriter + replaced *bool + noContent bool +} + +func (w *putOutcomeWriter) WriteHeader(code int) { + if code == http.StatusCreated && *w.replaced { + code = http.StatusNoContent + w.noContent = true + } + w.ResponseWriter.WriteHeader(code) +} + +func (w *putOutcomeWriter) Write(b []byte) (int, error) { + if w.noContent { + return len(b), nil + } + return w.ResponseWriter.Write(b) +} + // an overwriting MOVE or COPY removes the destination before reading the // source, so a destination that contains the source removes it too; a // destination under the source copies itself unless the copy is shallow @@ -218,6 +246,14 @@ func (h copyMoveGuardHandler) moveHandler(r *http.Request) http.Handler { } func (h copyMoveGuardHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // a PUT that replaces a file answers 204 (RFC 9110 9.3.4); webdav.Handler + // always answers 201, so OpenFile reports a replace on the request + // context and the writer rewrites the status it sends + if r.Method == "PUT" { + replaced := new(bool) + w = &putOutcomeWriter{ResponseWriter: w, replaced: replaced} + r = r.WithContext(context.WithValue(r.Context(), putOutcomeKey{}, replaced)) + } // 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 { @@ -471,18 +507,11 @@ func (fs *WebDavFileSystem) OpenFile(ctx context.Context, fullFilePath string, f 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() { - return nil, os.ErrExist - } - fs.removeAll(ctx, fullFilePath, false) - } else if err != os.ErrNotExist { - return nil, err - } + var resp *filer_pb.CreateEntryResponse err = fs.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { - if err := filer_pb.CreateEntry(context.Background(), client, &filer_pb.CreateEntryRequest{ + var err error + resp, err = filer_pb.CreateEntryWithResponse(context.Background(), client, &filer_pb.CreateEntryRequest{ Directory: dir, Entry: &filer_pb.Entry{ Name: name, @@ -496,14 +525,26 @@ func (fs *WebDavFileSystem) OpenFile(ctx context.Context, fullFilePath string, f TtlSec: 0, }, }, + OExcl: flag&os.O_EXCL != 0, Signatures: []int32{fs.signature}, - }); err != nil { - return fmt.Errorf("create %s: %v", fullFilePath, err) - } - return nil + }) + return err }) if err != nil { - return nil, err + // a taken name — an exclusive create, or a file landing on a + // directory — is the os.ErrExist x/net/webdav answers 405 for + if errors.Is(err, filer_pb.ErrEntryAlreadyExists) || errors.Is(err, filer_pb.ErrExistingIsDirectory) { + return nil, os.ErrExist + } + return nil, fmt.Errorf("create %s: %w", fullFilePath, err) + } + // the filer fills OldEntry under its path lock while writing, so the + // outcome matches what the write did even against racing writers: a + // PUT that landed on an entry answers 204, a fresh create answers 201 + if resp.GetMetadataEvent().GetEventNotification().GetOldEntry() != nil { + if replaced, ok := ctx.Value(putOutcomeKey{}).(*bool); ok { + *replaced = true + } } return &WebDavFile{ fs: fs, diff --git a/weed/server/webdav_server_test.go b/weed/server/webdav_server_test.go index 697bc15f8c..be4f4dfbf7 100644 --- a/weed/server/webdav_server_test.go +++ b/weed/server/webdav_server_test.go @@ -98,7 +98,30 @@ func (f *createRecordingFiler) LookupDirectoryEntry(_ context.Context, req *file func (f *createRecordingFiler) CreateEntry(_ context.Context, req *filer_pb.CreateEntryRequest) (*filer_pb.CreateEntryResponse, error) { f.req = req - return &filer_pb.CreateEntryResponse{}, nil + p := string(util.NewFullPath(req.Directory, req.Entry.Name)) + resp := &filer_pb.CreateEntryResponse{} + if f.dirs[p] && !req.Entry.IsDirectory { + resp.Error = "existing entry is a directory" + resp.ErrorCode = filer_pb.FilerError_EXISTING_IS_DIRECTORY + return resp, nil + } + if req.OExcl && (f.files[p] || f.dirs[p]) { + resp.Error = "entry already exists" + resp.ErrorCode = filer_pb.FilerError_ENTRY_ALREADY_EXISTS + return resp, nil + } + if f.files[p] { + resp.MetadataEvent = &filer_pb.SubscribeMetadataResponse{ + EventNotification: &filer_pb.EventNotification{ + OldEntry: &filer_pb.Entry{Name: req.Entry.Name}, + }, + } + } + if f.files == nil { + f.files = map[string]bool{} + } + f.files[p] = true + return resp, nil } func (f *createRecordingFiler) DeleteEntry(_ context.Context, req *filer_pb.DeleteEntryRequest) (*filer_pb.DeleteEntryResponse, error) { @@ -124,6 +147,16 @@ func (f *deleteRecordingFiler) DeleteEntry(_ context.Context, req *filer_pb.Dele return &filer_pb.DeleteEntryResponse{}, nil } +func (f *deleteRecordingFiler) CreateEntry(_ context.Context, req *filer_pb.CreateEntryRequest) (*filer_pb.CreateEntryResponse, error) { + if f.existing != nil && f.existing.IsDirectory && !req.Entry.IsDirectory { + return &filer_pb.CreateEntryResponse{ + Error: "existing entry is a directory", + ErrorCode: filer_pb.FilerError_EXISTING_IS_DIRECTORY, + }, nil + } + return &filer_pb.CreateEntryResponse{}, nil +} + func startFakeWebDavFiler(t *testing.T, impl filer_pb.SeaweedFilerServer) pb.ServerAddress { t.Helper() lis, err := net.Listen("tcp", "127.0.0.1:0") @@ -842,6 +875,67 @@ func TestWebDavFileSystem_OpenFile_DoesNotReplaceFolder(t *testing.T) { assert.Nil(t, filer.req) } +// putOutcomeFS mimics the filer-backed fs: creating over an existing name +// marks the request's PUT outcome as a replace. +type putOutcomeFS struct { + webdav.FileSystem +} + +func (f putOutcomeFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (webdav.File, error) { + if flag&os.O_CREATE != 0 { + if _, err := f.FileSystem.Stat(ctx, name); err == nil { + if replaced, ok := ctx.Value(putOutcomeKey{}).(*bool); ok { + *replaced = true + } + } + } + return f.FileSystem.OpenFile(ctx, name, flag, perm) +} + +func TestCopyMoveGuardHandler_PutStatus(t *testing.T) { + fs := putOutcomeFS{FileSystem: webdav.NewMemFS()} + h := copyMoveGuardHandler{ + fs: fs, + next: &webdav.Handler{FileSystem: fs, LockSystem: webdav.NewMemLS()}, + } + put := func(body string) *httptest.ResponseRecorder { + req := httptest.NewRequest("PUT", "http://x/a.txt", strings.NewReader(body)) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + return w + } + assert.Equal(t, http.StatusCreated, put("one").Code) + w := put("two") + assert.Equal(t, http.StatusNoContent, w.Code) + assert.Empty(t, w.Body.String(), "a 204 carries no body") +} + +// The 204 comes from OpenFile's own bookkeeping: creating over an existing +// entry flags the request context, a fresh create does not. +func TestWebDavFileSystem_OpenFile_PutOutcomeFlag(t *testing.T) { + filer := &createRecordingFiler{files: map[string]bool{"/old.txt": true}} + filerAddr := startFakeWebDavFiler(t, filer) + fs := &WebDavFileSystem{option: &WebDavOption{ + Filer: filerAddr, + GrpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()), + }} + + open := func(name string) bool { + replaced := new(bool) + ctx := context.WithValue(context.Background(), putOutcomeKey{}, replaced) + f, err := fs.OpenFile(ctx, name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) + assert.NoError(t, err) + if f != nil { + assert.NoError(t, f.Close()) + } + return *replaced + } + + assert.True(t, open("/old.txt")) + assert.False(t, open("/new.txt")) + assert.True(t, open("/new.txt"), "a PUT over the entry the first create made is a replace") +} + type renameRecordingFiler struct { filer_pb.UnimplementedSeaweedFilerServer req *filer_pb.AtomicRenameEntryRequest