[release-branch.go1.23] os: use O_EXCL instead of O_TRUNC in CopyFS t…

…o disallow rewriting existing files does not exist 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 aa5d672) Reviewed-on: https://go-review.googlesource.com/c/go/+/606415
golang · Aug 21, 2024 · 3c93405 · 3c93405
1 parent dbecb41
commit 3c93405
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 3 deletions.
diff --git 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
@@ -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) {