os: reject WriteAt if file opened in append mode

WriteAt use pwrite syscall on *nix or WriteFile on Windows.

On Linux/Windows, these system calls always write to end of file in
append mode, regardless of offset parameter.

It is hard (maybe impossible) to make WriteAt work portably.

Making WriteAt returns an error if file is opened in append mode, we
guarantee to get consistent behavior between platforms, also prevent
user from accidently corrupting their data.

Fixes #30716

Change-Id: If83d935a22a29eed2ff8fe53d13d0b4798aa2b81
Reviewed-on: https://go-review.googlesource.com/c/go/+/166578
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/os/export_test.go b/src/os/export_test.go
index d735aeea6117b75a1adbf0b0b28f44cbe9ce1ee7..812432cee4869c0de1424105afb3f840d0cc67dc 100644 (file)
--- a/src/os/export_test.go
+++ b/src/os/export_test.go
@@ -8,3 +8,4 @@ package os
 
 var Atime = atime
 var LstatP = &lstat
+var ErrWriteAtInAppendMode = errWriteAtInAppendMode

diff --git a/src/os/file.go b/src/os/file.go
index a44263ee8acbce29711fab5dc755158d79311c5a..258a3e6109840cf9ef27f2f5fe1bae8d15f6e1b9 100644 (file)
--- a/src/os/file.go
+++ b/src/os/file.go
@@ -163,13 +163,20 @@ func (f *File) Write(b []byte) (n int, err error) {
        return n, err
 }
 
+var errWriteAtInAppendMode = errors.New("os: invalid use of WriteAt on file opened with O_APPEND")
+
 // WriteAt writes len(b) bytes to the File starting at byte offset off.
 // It returns the number of bytes written and an error, if any.
 // WriteAt returns a non-nil error when n != len(b).
+//
+// If file was opened with the O_APPEND flag, WriteAt returns an error.
 func (f *File) WriteAt(b []byte, off int64) (n int, err error) {
        if err := f.checkValid("write"); err != nil {
                return 0, err
        }
+       if f.appendMode {
+               return 0, errWriteAtInAppendMode
+       }
 
        if off < 0 {
                return 0, &PathError{"writeat", f.name, errors.New("negative offset")}
@@ -286,7 +293,13 @@ func Create(name string) (*File, error) {
 // If there is an error, it will be of type *PathError.
 func OpenFile(name string, flag int, perm FileMode) (*File, error) {
        testlog.Open(name)
-       return openFileNolog(name, flag, perm)
+       f, err := openFileNolog(name, flag, perm)
+       if err != nil {
+               return nil, err
+       }
+       f.appendMode = flag&O_APPEND != 0
+
+       return f, nil
 }
 
 // lstat is overridden in tests.

diff --git a/src/os/file_plan9.go b/src/os/file_plan9.go
index 3fa12e681667c92d451cfde146987fd894f75871..14091873cf96c8faf80c617a0b9b8c1679634b94 100644 (file)
--- a/src/os/file_plan9.go
+++ b/src/os/file_plan9.go
@@ -22,9 +22,10 @@ func fixLongPath(path string) string {
 // can overwrite this data, which could cause the finalizer
 // to close the wrong file descriptor.
 type file struct {
-       fd      int
-       name    string
-       dirinfo *dirInfo // nil unless directory being read
+       fd         int
+       name       string
+       dirinfo    *dirInfo // nil unless directory being read
+       appendMode bool     // whether file is opened for appending
 }
 
 // Fd returns the integer Plan 9 file descriptor referencing the open file.

diff --git a/src/os/file_unix.go b/src/os/file_unix.go
index 4b62abfb5cd05256bd90ee6ab1f93740b41df00d..1cd8000dd4294acb58b7f3460faf72be37a879bf 100644 (file)
--- a/src/os/file_unix.go
+++ b/src/os/file_unix.go
@@ -52,6 +52,7 @@ type file struct {
        dirinfo     *dirInfo // nil unless directory being read
        nonblock    bool     // whether we set nonblocking mode
        stdoutOrErr bool     // whether this is stdout or stderr
+       appendMode  bool     // whether file is opened for appending
 }
 
 // Fd returns the integer Unix file descriptor referencing the open file.

diff --git a/src/os/file_windows.go b/src/os/file_windows.go
index f311ae11d9ac36179b5b250bd8fbd82b848b5f00..08444d728f6740f689d1013f9aee64137c5b3eae 100644 (file)
--- a/src/os/file_windows.go
+++ b/src/os/file_windows.go
@@ -19,9 +19,10 @@ import (
 // can overwrite this data, which could cause the finalizer
 // to close the wrong file descriptor.
 type file struct {
-       pfd     poll.FD
-       name    string
-       dirinfo *dirInfo // nil unless directory being read
+       pfd        poll.FD
+       name       string
+       dirinfo    *dirInfo // nil unless directory being read
+       appendMode bool     // whether file is opened for appending
 }
 
 // Fd returns the Windows handle referencing the open file.

diff --git a/src/os/os_test.go b/src/os/os_test.go
index c5c6b49e8f427e5a2b339f4b3fd0d553e548e3b1..1de46c29f50c30df734f321ffd6cd59fb663956c 100644 (file)
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -1646,6 +1646,21 @@ func TestWriteAtNegativeOffset(t *testing.T) {
        }
 }
 
+// Verify that WriteAt doesn't work in append mode.
+func TestWriteAtInAppendMode(t *testing.T) {
+       defer chtmpdir(t)()
+       f, err := OpenFile("write_at_in_append_mode.txt", O_APPEND|O_CREATE, 0666)
+       if err != nil {
+               t.Fatalf("OpenFile: %v", err)
+       }
+       defer f.Close()
+
+       _, err = f.WriteAt([]byte(""), 1)
+       if err != ErrWriteAtInAppendMode {
+               t.Fatalf("f.WriteAt returned %v, expected %v", err, ErrWriteAtInAppendMode)
+       }
+}
+
 func writeFile(t *testing.T, fname string, flag int, text string) string {
        f, err := OpenFile(fname, flag, 0666)
        if err != nil {