fix getxattr and listxattr (#72)

previously, this will fail if /mnt/file doesn't have an xattr:

```
listxattr("/mnt/file", 0x7fe8b3686830, 256) = -1 EIO (Input/output error)
```

We should be returning the actual size only if the input size is
zero. Related issue is if the filesystem returns ERANGE, we should
propagate that error instead of returning the actual size.

Replaced go-xattr usage with x/sys/unix so we can test this.
geesefs-0-30-9
Ka-Hing Cheung 2019-12-11 00:41:37 -08:00 committed by Michael Stapelberg
parent 4ee1cf7f62
commit e7bcad2083
3 changed files with 51 additions and 43 deletions

View File

@ -603,17 +603,6 @@ func (c *Connection) kernelResponse(
if opErr != nil {
handled := false
if opErr == syscall.ERANGE {
switch o := op.(type) {
case *fuseops.GetXattrOp:
writeXattrSize(m, uint32(o.BytesRead))
handled = true
case *fuseops.ListXattrOp:
writeXattrSize(m, uint32(o.BytesRead))
handled = true
}
}
if !handled {
m.OutHeader().Error = -int32(syscall.EIO)
if errno, ok := opErr.(syscall.Errno); ok {
@ -792,14 +781,14 @@ func (c *Connection) kernelResponseForOp(
// convertInMessage already set up the destination buffer to be at the end
// of the out message. We need only shrink to the right size based on how
// much the user read.
if o.BytesRead == 0 {
if len(o.Dst) == 0 {
writeXattrSize(m, uint32(o.BytesRead))
} else {
m.ShrinkTo(buffer.OutMessageHeaderSize + o.BytesRead)
}
case *fuseops.ListXattrOp:
if o.BytesRead == 0 {
if len(o.Dst) == 0 {
writeXattrSize(m, uint32(o.BytesRead))
} else {
m.ShrinkTo(buffer.OutMessageHeaderSize + o.BytesRead)

View File

@ -26,6 +26,7 @@ import (
"github.com/jacobsa/fuse/fuseops"
"github.com/jacobsa/fuse/fuseutil"
"github.com/jacobsa/syncutil"
"golang.org/x/sys/unix"
)
type memFS struct {
@ -679,7 +680,7 @@ func (fs *memFS) GetXattr(ctx context.Context,
op.BytesRead = len(value)
if len(op.Dst) >= len(value) {
copy(op.Dst, value)
} else {
} else if len(op.Dst) != 0 {
err = syscall.ERANGE
}
} else {
@ -703,8 +704,8 @@ func (fs *memFS) ListXattr(ctx context.Context,
if err == nil && len(dst) >= keyLen {
copy(dst, key)
dst = dst[keyLen:]
} else {
err = syscall.ERANGE
} else if len(op.Dst) != 0 {
return syscall.ERANGE
}
op.BytesRead += keyLen
}
@ -735,11 +736,11 @@ func (fs *memFS) SetXattr(ctx context.Context,
_, ok := inode.xattrs[op.Name]
switch op.Flags {
case 0x1:
case unix.XATTR_CREATE:
if ok {
err = fuse.EEXIST
}
case 0x2:
case unix.XATTR_REPLACE:
if !ok {
err = fuse.ENOATTR
}

View File

@ -35,7 +35,7 @@ import (
"github.com/jacobsa/fuse/samples/memfs"
. "github.com/jacobsa/oglematchers"
. "github.com/jacobsa/ogletest"
"github.com/kahing/go-xattr"
"golang.org/x/sys/unix"
)
func TestMemFS(t *testing.T) { RunTests(t) }
@ -1792,6 +1792,8 @@ func (t *MemFSTest) RenameNonExistentFile() {
func (t *MemFSTest) NoXattrs() {
var err error
var sz int
var smallBuf [1]byte
// Create a file.
filePath := path.Join(t.Dir, "foo")
@ -1799,53 +1801,69 @@ func (t *MemFSTest) NoXattrs() {
AssertEq(nil, err)
// List xattr names.
names, err := xattr.List(filePath)
sz, err = unix.Listxattr(filePath, nil)
AssertEq(nil, err)
ExpectThat(names, ElementsAre())
AssertEq(0, sz)
// Attempt to read a non-existent xattr.
_, err = xattr.Getxattr(filePath, "foo", nil)
_, err = unix.Getxattr(filePath, "foo", nil)
ExpectEq(fuse.ENOATTR, err)
// Attempt to read a non-existent xattr with a buf.
_, err = unix.Getxattr(filePath, "foo", smallBuf[:])
ExpectEq(fuse.ENOATTR, err)
// List xattr names with a buf.
sz, err = unix.Listxattr(filePath, smallBuf[:])
AssertEq(nil, err)
ExpectEq(0, sz)
}
func (t *MemFSTest) SetXAttr() {
var err error
var sz int
var buf [1024]byte
// Create a file.
filePath := path.Join(t.Dir, "foo")
err = ioutil.WriteFile(filePath, []byte("taco"), 0600)
AssertEq(nil, err)
err = xattr.Setxattr(filePath, "foo", []byte("bar"), xattr.REPLACE)
err = unix.Setxattr(filePath, "foo", []byte("bar"), unix.XATTR_REPLACE)
AssertEq(fuse.ENOATTR, err)
err = xattr.Setxattr(filePath, "foo", []byte("bar"), xattr.CREATE)
err = unix.Setxattr(filePath, "foo", []byte("bar"), unix.XATTR_CREATE)
AssertEq(nil, err)
value, err := xattr.Get(filePath, "foo")
AssertEq(nil, err)
AssertEq("bar", string(value))
// List xattr with a buf that is too small.
_, err = unix.Listxattr(filePath, buf[:1])
ExpectEq(unix.ERANGE, err)
err = xattr.Setxattr(filePath, "foo", []byte("hello world"), xattr.REPLACE)
// List xattr to ask for name size.
sz, err = unix.Listxattr(filePath, nil)
AssertEq(nil, err)
AssertEq(4, sz)
value, err = xattr.Get(filePath, "foo")
// List xattr names.
sz, err = unix.Listxattr(filePath, buf[:sz])
AssertEq(nil, err)
AssertEq("hello world", string(value))
AssertEq(4, sz)
AssertEq("foo\000", string(buf[:sz]))
names, err := xattr.List(filePath)
AssertEq(nil, err)
AssertEq(1, len(names))
AssertEq("foo", names[0])
// Read xattr with a buf that is too small.
_, err = unix.Getxattr(filePath, "foo", buf[:1])
ExpectEq(unix.ERANGE, err)
err = xattr.Setxattr(filePath, "bar", []byte("hello world"), 0x0)
// Read xattr to ask for value size.
sz, err = unix.Getxattr(filePath, "foo", nil)
AssertEq(nil, err)
AssertEq(3, sz)
names, err = xattr.List(filePath)
// Read xattr value.
sz, err = unix.Getxattr(filePath, "foo", buf[:sz])
AssertEq(nil, err)
AssertEq(2, len(names))
ExpectThat(names, Contains("foo"))
ExpectThat(names, Contains("bar"))
AssertEq(3, sz)
AssertEq("bar", string(buf[:sz]))
}
func (t *MemFSTest) RemoveXAttr() {
@ -1856,16 +1874,16 @@ func (t *MemFSTest) RemoveXAttr() {
err = ioutil.WriteFile(filePath, []byte("taco"), 0600)
AssertEq(nil, err)
err = xattr.Removexattr(filePath, "foo")
err = unix.Removexattr(filePath, "foo")
AssertEq(fuse.ENOATTR, err)
err = xattr.Setxattr(filePath, "foo", []byte("bar"), xattr.CREATE)
err = unix.Setxattr(filePath, "foo", []byte("bar"), unix.XATTR_CREATE)
AssertEq(nil, err)
err = xattr.Removexattr(filePath, "foo")
err = unix.Removexattr(filePath, "foo")
AssertEq(nil, err)
_, err = xattr.Getxattr(filePath, "foo", nil)
_, err = unix.Getxattr(filePath, "foo", nil)
AssertEq(fuse.ENOATTR, err)
}