diff --git a/chfs-ng/internal/api/server.go b/chfs-ng/internal/api/server.go index b7f2b4c..f7701d8 100644 --- a/chfs-ng/internal/api/server.go +++ b/chfs-ng/internal/api/server.go @@ -1110,32 +1110,34 @@ func (f *webdavStorageFS) Create(ctx context.Context, name string, body io.ReadC return nil, false, statErr } exclusive := opts != nil && opts.IfNoneMatch.IsWildcard() - var file *os.File if exclusive { - file, err = createExclusive(f.store, p) + file, createErr := createExclusive(f.store, p) + if createErr != nil { + if os.IsExist(createErr) { + return nil, false, webdav.NewHTTPError(http.StatusPreconditionFailed, fmt.Errorf("destination exists")) + } + return nil, false, createErr + } + _, copyErr := io.Copy(file, body) + closeErr := file.Close() + body.Close() + if copyErr != nil { + _ = f.store.Remove(p) + return nil, false, copyErr + } + if closeErr != nil { + _ = f.store.Remove(p) + return nil, false, closeErr + } } else { - file, err = f.store.Create(p) - } - if err != nil { - if exclusive && os.IsExist(err) { - return nil, false, webdav.NewHTTPError(http.StatusPreconditionFailed, fmt.Errorf("destination exists")) + // Write into a sibling temporary file and atomically replace the + // destination only after the complete request body has been received. + // A failed PUT therefore cannot truncate or corrupt the old file. + if err := copyFileToDestination(f.store, body, p); err != nil { + body.Close() + return nil, false, err } - return nil, false, err - } - _, copyErr := io.Copy(file, body) - closeErr := file.Close() - body.Close() - if copyErr != nil { - if exclusive { - _ = f.store.Remove(p) - } - return nil, false, copyErr - } - if closeErr != nil { - if exclusive { - _ = f.store.Remove(p) - } - return nil, false, closeErr + body.Close() } fi, err := f.info(p) return fi, true, err diff --git a/chfs-ng/internal/api/webdav_test.go b/chfs-ng/internal/api/webdav_test.go index c16c765..653e9dc 100644 --- a/chfs-ng/internal/api/webdav_test.go +++ b/chfs-ng/internal/api/webdav_test.go @@ -3,6 +3,7 @@ package api import ( "context" "errors" + "io" "net/http/httptest" "os" "path/filepath" @@ -147,6 +148,10 @@ type failingReader struct{} func (failingReader) Read([]byte) (int, error) { return 0, errors.New("source read failed") } +type readCloser struct{ io.Reader } + +func (r readCloser) Close() error { return nil } + func TestWebDAVStorageFSCopyFailureDoesNotCorruptDestination(t *testing.T) { root := t.TempDir() store := storage.Local{Root: root} @@ -172,6 +177,33 @@ func TestWebDAVStorageFSCopyFailureDoesNotCorruptDestination(t *testing.T) { } } +func TestWebDAVStorageFSCreateFailureDoesNotCorruptDestination(t *testing.T) { + root := t.TempDir() + store := storage.Local{Root: root} + if err := os.WriteFile(filepath.Join(root, "dst.txt"), []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + fs := &webdavStorageFS{store: store} + _, _, err := fs.Create(context.Background(), "/dst.txt", readCloser{Reader: failingReader{}}, nil) + if err == nil { + t.Fatal("expected PUT body failure") + } + got, err := os.ReadFile(filepath.Join(root, "dst.txt")) + if err != nil { + t.Fatal(err) + } + if string(got) != "original" { + t.Fatalf("destination was corrupted: %q", got) + } + matches, err := filepath.Glob(filepath.Join(root, "dst.txt.chfs-copy-*")) + if err != nil { + t.Fatal(err) + } + if len(matches) != 0 { + t.Fatalf("temporary PUT files leaked: %v", matches) + } +} + func TestWebDAVStorageFSCopyAndMove(t *testing.T) { root1 := t.TempDir() root2 := t.TempDir()