Skip to content

Commit aa5d672

Browse files
panjf2000gopherbot
authored andcommitted
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>
1 parent 660e7d6 commit aa5d672

File tree

2 files changed

+20
-3
lines changed

2 files changed

+20
-3
lines changed

‎src/os/dir.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,9 @@ func ReadDir(name string) ([]DirEntry, error) {
136136
// from the source, and directories are created with mode 0o777
137137
// (before umask).
138138
//
139-
// CopyFS will not overwrite existing files, and returns an error
140-
// if a file name in fsys already exists in the destination.
139+
// CopyFS will not overwrite existing files. If a file name in fsys
140+
// already exists in the destination, CopyFS will return an error
141+
// such that errors.Is(err, fs.ErrExist) will be true.
141142
//
142143
// Symbolic links in fsys are not supported. A *PathError with Err set
143144
// to ErrInvalid is returned when copying from a symbolic link.
@@ -176,7 +177,7 @@ func CopyFS(dir string, fsys fs.FS) error {
176177
if err != nil {
177178
return err
178179
}
179-
w, err := OpenFile(newPath, O_CREATE|O_TRUNC|O_WRONLY, 0666|info.Mode()&0777)
180+
w, err := OpenFile(newPath, O_CREATE|O_EXCL|O_WRONLY, 0666|info.Mode()&0777)
180181
if err != nil {
181182
return err
182183
}

‎src/os/os_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3407,6 +3407,14 @@ func TestCopyFS(t *testing.T) {
34073407
t.Fatal("comparing two directories:", err)
34083408
}
34093409

3410+
// Test whether CopyFS disallows copying for disk filesystem when there is any
3411+
// existing file in the destination directory.
3412+
if err := CopyFS(tmpDir, fsys); !errors.Is(err, fs.ErrExist) {
3413+
t.Errorf("CopyFS should have failed and returned error when there is"+
3414+
"any existing file in the destination directory (in disk filesystem), "+
3415+
"got: %v, expected any error that indicates <file exists>", err)
3416+
}
3417+
34103418
// Test with memory filesystem.
34113419
fsys = fstest.MapFS{
34123420
"william": {Data: []byte("Shakespeare\n")},
@@ -3444,6 +3452,14 @@ func TestCopyFS(t *testing.T) {
34443452
}); err != nil {
34453453
t.Fatal("comparing two directories:", err)
34463454
}
3455+
3456+
// Test whether CopyFS disallows copying for memory filesystem when there is any
3457+
// existing file in the destination directory.
3458+
if err := CopyFS(tmpDir, fsys); !errors.Is(err, fs.ErrExist) {
3459+
t.Errorf("CopyFS should have failed and returned error when there is"+
3460+
"any existing file in the destination directory (in memory filesystem), "+
3461+
"got: %v, expected any error that indicates <file exists>", err)
3462+
}
34473463
}
34483464

34493465
func TestCopyFSWithSymlinks(t *testing.T) {

0 commit comments

Comments
 (0)