From aa5d672a00f5bf64865d0e821623ed29bc416405 Mon Sep 17 00:00:00 2001 From: Andy Pan Date: Fri, 16 Aug 2024 08:04:57 +0800 Subject: [PATCH] os: use O_EXCL instead of O_TRUNC in CopyFS to disallow rewriting existing files On Linux, a call to creat() is equivalent to calling open() with flags equal to O_CREAT|O_WRONLY|O_TRUNC, which applies to other platforms as well in a similar manner. Thus, to force CopyFS's behavior to comply with the function comment, we need to replace O_TRUNC with O_EXCL. Fixes #68895 Change-Id: I3e2ab153609d3c8cf20ce5969d6f3ef593833cd1 Reviewed-on: https://go-review.googlesource.com/c/go/+/606095 Auto-Submit: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Reviewed-by: Ian Lance Taylor --- src/os/dir.go | 7 ++++--- src/os/os_test.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/os/dir.go b/src/os/dir.go index dab75b5d43..04392193aa 100644 --- a/src/os/dir.go +++ b/src/os/dir.go @@ -136,8 +136,9 @@ func ReadDir(name string) ([]DirEntry, error) { // from the source, and directories are created with mode 0o777 // (before umask). // -// CopyFS will not overwrite existing files, and returns an error -// if a file name in fsys already exists in the destination. +// CopyFS will not overwrite existing files. If a file name in fsys +// already exists in the destination, CopyFS will return an error +// such that errors.Is(err, fs.ErrExist) will be true. // // Symbolic links in fsys are not supported. A *PathError with Err set // to ErrInvalid is returned when copying from a symbolic link. @@ -176,7 +177,7 @@ func CopyFS(dir string, fsys fs.FS) error { if err != nil { return err } - w, err := OpenFile(newPath, O_CREATE|O_TRUNC|O_WRONLY, 0666|info.Mode()&0777) + w, err := OpenFile(newPath, O_CREATE|O_EXCL|O_WRONLY, 0666|info.Mode()&0777) if err != nil { return err } diff --git a/src/os/os_test.go b/src/os/os_test.go index 9832e595ae..538a75f912 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -3407,6 +3407,14 @@ func TestCopyFS(t *testing.T) { t.Fatal("comparing two directories:", err) } + // Test whether CopyFS disallows copying for disk filesystem when there is any + // existing file in the destination directory. + if err := CopyFS(tmpDir, fsys); !errors.Is(err, fs.ErrExist) { + t.Errorf("CopyFS should have failed and returned error when there is"+ + "any existing file in the destination directory (in disk filesystem), "+ + "got: %v, expected any error that indicates ", err) + } + // Test with memory filesystem. fsys = fstest.MapFS{ "william": {Data: []byte("Shakespeare\n")}, @@ -3444,6 +3452,14 @@ func TestCopyFS(t *testing.T) { }); err != nil { t.Fatal("comparing two directories:", err) } + + // Test whether CopyFS disallows copying for memory filesystem when there is any + // existing file in the destination directory. + if err := CopyFS(tmpDir, fsys); !errors.Is(err, fs.ErrExist) { + t.Errorf("CopyFS should have failed and returned error when there is"+ + "any existing file in the destination directory (in memory filesystem), "+ + "got: %v, expected any error that indicates ", err) + } } func TestCopyFSWithSymlinks(t *testing.T) { -- 2.48.1