yves
/
rclone
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

vfs: Make OpenFile and friends return EINVAL if O_RDONLY and O_TRUNC 

Before this change Open("name", os.O_RDONLY|os.O_TRUNC) would have
truncated the file.  This is what Linux does, but is counterintuitive.
POSIX states this is undefined, so return an error in this case
instead.  This preserves the invariant O_RDONLY => file is not
changed.
s3-about
Nick Craig-Wood 2018-02-23 22:39:28 +00:00
parent 3282fd26af
commit 54deb01f00
7 changed files with 107 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/cmount/fs.go
View File

@ -540,6 +540,8 @@ func translateError(err error) (errc int) {
return -fuse.EROFS
case vfs.ENOSYS:
return -fuse.ENOSYS
case vfs.EINVAL:
return -fuse.EINVAL
}
fs.Errorf(nil, "IO error: %v", err)
return -fuse.EIO

2
cmd/mount/fs.go
View File

@ -91,6 +91,8 @@ func translateError(err error) error {
return fuse.Errno(syscall.EROFS)
case vfs.ENOSYS:
return fuse.ENOSYS
case vfs.EINVAL:
return fuse.Errno(syscall.EINVAL)
}
return err
}

1
vfs/errors.go
View File

@ -27,6 +27,7 @@ var (
ENOENT = os.ErrNotExist
EEXIST = os.ErrExist
EPERM = os.ErrPermission
EINVAL = os.ErrInvalid
// ECLOSED see errors_{old,new}.go
)

12
vfs/file.go
View File

@ -292,7 +292,6 @@ func (f *File) openRead() (fh *ReadFileHandle, err error) {
fh, err = newReadFileHandle(f, o)
if err != nil {
err = errors.Wrap(err, "open for read")
fs.Errorf(f, "File.openRead failed: %v", err)
return nil, err
}
@ -308,7 +307,6 @@ func (f *File) openWrite(flags int) (fh *WriteFileHandle, err error) {
fh, err = newWriteFileHandle(f.d, f, f.Path(), flags)
if err != nil {
err = errors.Wrap(err, "open for write")
fs.Errorf(f, "File.openWrite failed: %v", err)
return nil, err
}
@ -327,7 +325,6 @@ func (f *File) openRW(flags int) (fh *RWFileHandle, err error) {
fh, err = newRWFileHandle(f.d, f, f.Path(), flags)
if err != nil {
err = errors.Wrap(err, "open for read write")
fs.Errorf(f, "File.openRW failed: %v", err)
return nil, err
}
@ -400,9 +397,16 @@ func (f *File) Open(flags int) (fd Handle, err error) {
var (
write bool // if set need write support
read bool // if set need read support
rdwrMode = flags & (os.O_RDONLY | os.O_WRONLY | os.O_RDWR)
rdwrMode = flags & accessModeMask
)
// http://pubs.opengroup.org/onlinepubs/7908799/xsh/open.html
// The result of using O_TRUNC with O_RDONLY is undefined.
// Linux seems to truncate the file, but we prefer to return EINVAL
if rdwrMode == os.O_RDONLY && flags&os.O_TRUNC != 0 {
return nil, EINVAL
}
// Figure out the read/write intents
switch {
case rdwrMode == os.O_RDONLY:

168
vfs/open_test.go
View File

@ -36,13 +36,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_TRUNC,
what: "os.O_RDONLY|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_SYNC,
what: "os.O_RDONLY|os.O_SYNC",
@ -56,13 +56,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_EXCL,
what: "os.O_RDONLY|os.O_EXCL",
@ -76,13 +76,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_EXCL | os.O_TRUNC,
what: "os.O_RDONLY|os.O_EXCL|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_EXCL | os.O_SYNC,
what: "os.O_RDONLY|os.O_EXCL|os.O_SYNC",
@ -96,13 +96,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_EXCL | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_EXCL|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_CREATE,
what: "os.O_RDONLY|os.O_CREATE",
@ -116,13 +116,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_CREATE | os.O_TRUNC,
what: "os.O_RDONLY|os.O_CREATE|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_CREATE | os.O_SYNC,
what: "os.O_RDONLY|os.O_CREATE|os.O_SYNC",
@ -136,13 +136,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_CREATE | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_CREATE|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_CREATE | os.O_EXCL,
what: "os.O_RDONLY|os.O_CREATE|os.O_EXCL",
@ -156,10 +156,10 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_CREATE | os.O_EXCL | os.O_TRUNC,
what: "os.O_RDONLY|os.O_CREATE|os.O_EXCL|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: EEXIST,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
@ -176,10 +176,10 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_CREATE | os.O_EXCL | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_CREATE|os.O_EXCL|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: EEXIST,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
@ -196,13 +196,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_SYNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_SYNC",
@ -216,13 +216,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_EXCL,
what: "os.O_RDONLY|os.O_APPEND|os.O_EXCL",
@ -236,13 +236,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_EXCL | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_EXCL|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_EXCL | os.O_SYNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_EXCL|os.O_SYNC",
@ -256,13 +256,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_EXCL | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_EXCL|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: ENOENT,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE",
@ -276,13 +276,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE | os.O_SYNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE|os.O_SYNC",
@ -296,13 +296,13 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: nil,
readExistingErr: io.EOF,
writeExistingErr: EBADF,
contents: "",
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE | os.O_EXCL,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE|os.O_EXCL",
@ -316,10 +316,10 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE | os.O_EXCL | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE|os.O_EXCL|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: EEXIST,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",
@ -336,10 +336,10 @@ var openTests = []openTest{
}, {
flags: os.O_RDONLY | os.O_APPEND | os.O_CREATE | os.O_EXCL | os.O_SYNC | os.O_TRUNC,
what: "os.O_RDONLY|os.O_APPEND|os.O_CREATE|os.O_EXCL|os.O_SYNC|os.O_TRUNC",
openNonExistentErr: nil,
readNonExistentErr: io.EOF,
writeNonExistentErr: EBADF,
openExistingErr: EEXIST,
openNonExistentErr: EINVAL,
readNonExistentErr: nil,
writeNonExistentErr: nil,
openExistingErr: EINVAL,
readExistingErr: nil,
writeExistingErr: nil,
contents: "hello",

2
vfs/read_write_test.go
View File

@ -523,8 +523,10 @@ func testRWFileHandleOpenTest(t *testing.T, vfs *VFS, test *openTest) {
require.NoError(t, err, test.what)
// check
assert.Equal(t, test.openNonExistentErr, openNonExistentErr, "openNonExistentErr: %s: want=%v, got=%v", test.what, test.openNonExistentErr, openNonExistentErr)
assert.Equal(t, test.readNonExistentErr, readNonExistentErr, "readNonExistentErr: %s: want=%v, got=%v", test.what, test.readNonExistentErr, readNonExistentErr)
assert.Equal(t, test.writeNonExistentErr, writeNonExistentErr, "writeNonExistentErr: %s: want=%v, got=%v", test.what, test.writeNonExistentErr, writeNonExistentErr)
assert.Equal(t, test.openExistingErr, openExistingErr, "openExistingErr: %s: want=%v, got=%v", test.what, test.openExistingErr, openExistingErr)
assert.Equal(t, test.readExistingErr, readExistingErr, "readExistingErr: %s: want=%v, got=%v", test.what, test.readExistingErr, readExistingErr)
assert.Equal(t, test.writeExistingErr, writeExistingErr, "writeExistingErr: %s: want=%v, got=%v", test.what, test.writeExistingErr, writeExistingErr)
assert.Equal(t, test.contents, contents, test.what)

8
vfs/vfs.go
View File

@ -393,6 +393,14 @@ func decodeOpenFlags(flags int) string {
// OpenFile a file according to the flags and perm provided
func (vfs *VFS) OpenFile(name string, flags int, perm os.FileMode) (fd Handle, err error) {
defer log.Trace(name, "flags=%s, perm=%v", decodeOpenFlags(flags), perm)("fd=%v, err=%v", &fd, &err)
// http://pubs.opengroup.org/onlinepubs/7908799/xsh/open.html
// The result of using O_TRUNC with O_RDONLY is undefined.
// Linux seems to truncate the file, but we prefer to return EINVAL
if flags&accessModeMask == os.O_RDONLY && flags&os.O_TRUNC != 0 {
return nil, EINVAL
}
node, err := vfs.Stat(name)
if err != nil {
if err != ENOENT || flags&os.O_CREATE == 0 {