[release-branch.go1.23] os: use O_EXCL instead of O_TRUNC in CopyFS to disallow rewri...

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 #68907

Change-Id: I3e2ab153609d3c8cf20ce5969d6f3ef593833cd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/606095
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit aa5d672a00f5bf64865d0e821623ed29bc416405)
Reviewed-on: https://go-review.googlesource.com/c/go/+/606415

diff --git a/src/os/dir.go b/src/os/dir.go
index dab75b5d436ce517a574cc7c3b0117a15dbcb37c..04392193aa6b03c8c07789560d8b02b655ffa547 100644 (file)
--- 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 94ac58dcb089ed01e2b80aa8da263b38ff137812..f1755dfa9139f840a31899a7c0a53e4d1374c934 100644 (file)
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -3354,6 +3354,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 <file exists>", err)
+       }
+
        // Test with memory filesystem.
        fsys = fstest.MapFS{
                "william":    {Data: []byte("Shakespeare\n")},
@@ -3391,6 +3399,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 <file exists>", err)
+       }
 }
 
 func TestCopyFSWithSymlinks(t *testing.T) {