From aa5d672a00f5bf64865d0e821623ed29bc416405 Mon Sep 17 00:00:00 2001
From: Andy Pan <i@andypan.me>
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 <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>
---
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 dab75b5d436..04392193aa6 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 9832e595ae7..538a75f912e 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 <file exists>", 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 <file exists>", err)
+ }
}
func TestCopyFSWithSymlinks(t *testing.T) {
--
GitLab