diff --git a/errors.go b/errors.go index af08be6..aa26325 100644 --- a/errors.go +++ b/errors.go @@ -5,6 +5,8 @@ import ( "encoding/binary" "errors" "fmt" + "io/fs" + "syscall" ) // RPCError provides the error interface for errors thrown by @@ -196,6 +198,57 @@ func (s *NFSStatusError) Unwrap() error { return s.WrappedErr } +// statusFromError translates err into the NFS status describing it, returning +// fallback for an error it does not recognize. +func statusFromError(err error, fallback NFSStatus) NFSStatus { + switch { + case err == nil: + return NFSStatusOk + case errors.Is(err, syscall.EROFS): + return NFSStatusROFS + case errors.Is(err, syscall.ENOSPC): + return NFSStatusNoSPC + case errors.Is(err, syscall.EDQUOT): + return NFSStatusDQuot + case errors.Is(err, syscall.ENOTEMPTY): + return NFSStatusNotEmpty + case errors.Is(err, syscall.EXDEV): + return NFSStatusXDev + case errors.Is(err, syscall.EISDIR): + return NFSStatusIsDir + case errors.Is(err, syscall.ENOTDIR): + return NFSStatusNotDir + case errors.Is(err, syscall.ENAMETOOLONG): + return NFSStatusNameTooLong + case errors.Is(err, syscall.EMLINK): + return NFSStatusMlink + case errors.Is(err, syscall.EFBIG): + return NFSStatusFBig + case errors.Is(err, syscall.ENXIO): + return NFSStatusNXIO + case errors.Is(err, syscall.EPERM): + return NFSStatusPerm + // ENOTEMPTY is also fs.ErrExist and EPERM is also fs.ErrPermission, so these + // sentinels come last. + case errors.Is(err, fs.ErrNotExist): + return NFSStatusNoEnt + case errors.Is(err, fs.ErrExist): + return NFSStatusExist + case errors.Is(err, fs.ErrPermission): + return NFSStatusAccess + case errors.Is(err, fs.ErrInvalid): + return NFSStatusInval + default: + return fallback + } +} + +// statusError wraps err in the NFSStatusError describing it, falling back to +// the given status when the error is unrecognized. +func statusError(err error, fallback NFSStatus) *NFSStatusError { + return &NFSStatusError{statusFromError(err, fallback), err} +} + // StatusErrorWithBody is an NFS error with a payload. type StatusErrorWithBody struct { NFSStatusError diff --git a/errors_test.go b/errors_test.go new file mode 100644 index 0000000..9e0016c --- /dev/null +++ b/errors_test.go @@ -0,0 +1,17 @@ +package nfs + +import ( + "errors" + "testing" +) + +func TestStatusErrorKeepsFallback(t *testing.T) { + err := errors.New("backend exploded") + statusErr := statusError(err, NFSStatusAccess) + if statusErr.NFSStatus != NFSStatusAccess { + t.Errorf("status = %v, want %v", statusErr.NFSStatus, NFSStatusAccess) + } + if !errors.Is(statusErr, err) { + t.Errorf("statusError dropped the wrapped error: %v", statusErr) + } +} diff --git a/nfs_oncreate.go b/nfs_oncreate.go index bce72b5..f18bbfc 100644 --- a/nfs_oncreate.go +++ b/nfs_oncreate.go @@ -70,7 +70,7 @@ func onCreate(ctx context.Context, w *response, userHandle Handler) error { } } else { if s, err := fs.Stat(fs.Join(path...)); err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } else if !s.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} } @@ -79,18 +79,18 @@ func onCreate(ctx context.Context, w *response, userHandle Handler) error { file, err := fs.Create(newFilePath) if err != nil { Log.Errorf("Error Creating: %v", err) - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } if err := file.Close(); err != nil { Log.Errorf("Error Creating: %v", err) - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } fp := userHandle.ToHandle(fs, newFile) changer := userHandle.Change(fs) if err := attrs.Apply(changer, fs, newFilePath); err != nil { Log.Errorf("Error applying attributes: %v\n", err) - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } writer := bytes.NewBuffer([]byte{}) diff --git a/nfs_ongetattr.go b/nfs_ongetattr.go index 5ecc240..6e64b92 100644 --- a/nfs_ongetattr.go +++ b/nfs_ongetattr.go @@ -3,7 +3,6 @@ package nfs import ( "bytes" "context" - "os" "github.com/willscott/go-nfs-client/nfs/xdr" ) @@ -22,10 +21,7 @@ func onGetAttr(ctx context.Context, w *response, userHandle Handler) error { fullPath := fs.Join(path...) info, err := fs.Lstat(fullPath) if err != nil { - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } attr := ToFileAttribute(info, fullPath) diff --git a/nfs_onlink.go b/nfs_onlink.go index 3af44c5..2a2a1c7 100644 --- a/nfs_onlink.go +++ b/nfs_onlink.go @@ -44,7 +44,7 @@ func onLink(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusExist, os.ErrExist} } if s, err := fs.Stat(fs.Join(path...)); err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } else if !s.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} } @@ -61,10 +61,10 @@ func onLink(ctx context.Context, w *response, userHandle Handler) error { err = cos.Link(string(target), newFilePath) if err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } if err := attrs.Apply(changer, fs, newFilePath); err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } writer := bytes.NewBuffer([]byte{}) diff --git a/nfs_onmkdir.go b/nfs_onmkdir.go index 2bb5598..e6787a2 100644 --- a/nfs_onmkdir.go +++ b/nfs_onmkdir.go @@ -49,21 +49,21 @@ func onMkdir(ctx context.Context, w *response, userHandle Handler) error { } } else { if s, err := fs.Stat(fs.Join(path...)); err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } else if !s.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} } } if err := fs.MkdirAll(newFolderPath, attrs.Mode(mkdirDefaultMode)); err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } fp := userHandle.ToHandle(fs, newFolder) changer := userHandle.Change(fs) if changer != nil { if err := attrs.Apply(changer, fs, newFolderPath); err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } } diff --git a/nfs_onread.go b/nfs_onread.go index b34949e..e63bd74 100644 --- a/nfs_onread.go +++ b/nfs_onread.go @@ -69,7 +69,7 @@ func onRead(ctx context.Context, w *response, userHandle Handler) error { // todo: multiple reads if size isn't full cnt, err := fh.ReadAt(resp.Data, int64(obj.Offset)) if err != nil && !errors.Is(err, io.EOF) { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } resp.Count = uint32(cnt) resp.Data = resp.Data[:resp.Count] diff --git a/nfs_onremove.go b/nfs_onremove.go index bf8df12..60caf8f 100644 --- a/nfs_onremove.go +++ b/nfs_onremove.go @@ -3,7 +3,6 @@ package nfs import ( "bytes" "context" - "errors" "os" "github.com/go-git/go-billy/v6" @@ -62,14 +61,8 @@ func onRemoveObj(ctx context.Context, w *response, userHandle Handler, directory fullPath := fs.Join(path...) dirInfo, err := fs.Stat(fullPath) - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - if os.IsPermission(err) { - return &NFSStatusError{NFSStatusAccess, err} - } if err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if !dirInfo.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} @@ -86,16 +79,10 @@ func onRemoveObj(ctx context.Context, w *response, userHandle Handler, directory // Lstat, not Stat: POSIX rmdir()/unlink() act on the entry itself, not what it points to : // rmdir() must reject a symlink to an empty directory, unlink() must remove the symlink itself. targetInfo, err := fs.Lstat(toDelete) - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - if os.IsPermission(err) { + if err != nil { // A permission error here means an ancestor directory in toDelete's path // lacks its executable permission, not that toDelete itself is inaccessible. - return &NFSStatusError{NFSStatusAccess, err} - } - if err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if directory && !targetInfo.IsDir() { @@ -107,17 +94,8 @@ func onRemoveObj(ctx context.Context, w *response, userHandle Handler, directory if directory { empty, err := isEmptyDir(ctx, userHandle, fs, toDelete) - // This error can come from a user-supplied OpenDir, which may return - // a wrapped os.ErrNotExist/os.ErrPermission. errors.Is sees through the - // wrapping; the os.IsNotExist/os.IsPermission checks elsewhere do not. - if errors.Is(err, os.ErrNotExist) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - if errors.Is(err, os.ErrPermission) { - return &NFSStatusError{NFSStatusAccess, err} - } if err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if !empty { return &NFSStatusError{NFSStatusNotEmpty, nil} @@ -125,20 +103,12 @@ func onRemoveObj(ctx context.Context, w *response, userHandle Handler, directory } err = fs.Remove(toDelete) - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - if os.IsPermission(err) { - return &NFSStatusError{NFSStatusAccess, err} - } if err != nil { - // We passed all directory/empty checks above, so an error here likely means - // the target changed underneath us (the TOCTOU window noted above) - e.g. a - // directory that just became non-empty. We report NFSStatusIO rather than - // trying to distinguish that case, since billy doesn't expose a portable way - // to recognize it. + // The checks above passed, so an error here means the target changed + // underneath us in the TOCTOU window - a directory that became non-empty, + // say. Log.Errorf("remove %q after passing directory checks: %v", toDelete, err) - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if err := userHandle.InvalidateHandle(fs, toDeleteHandle); err != nil { diff --git a/nfs_onrename.go b/nfs_onrename.go index 9be9c4f..a01951c 100644 --- a/nfs_onrename.go +++ b/nfs_onrename.go @@ -48,10 +48,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { fromDirPath := fs.Join(fromPath...) fromDirInfo, err := fs.Stat(fromDirPath) if err != nil { - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if !fromDirInfo.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} @@ -61,10 +58,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { toDirPath := fs.Join(toPath...) toDirInfo, err := fs.Stat(toDirPath) if err != nil { - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if !toDirInfo.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} @@ -78,13 +72,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { err = fs.Rename(fromLoc, toLoc) if err != nil { - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - if os.IsPermission(err) { - return &NFSStatusError{NFSStatusAccess, err} - } - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } if err := userHandle.InvalidateHandle(fs, oldHandle); err != nil { diff --git a/nfs_onsymlink.go b/nfs_onsymlink.go index 4495c11..730a79b 100644 --- a/nfs_onsymlink.go +++ b/nfs_onsymlink.go @@ -43,21 +43,21 @@ func onSymlink(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusExist, os.ErrExist} } if s, err := fs.Stat(fs.Join(path...)); err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } else if !s.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} } err = fs.Symlink(string(target), newFilePath) if err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } fp := userHandle.ToHandle(fs, append(path, string(obj.Filename))) changer := userHandle.Change(fs) if changer != nil { if err := attrs.Apply(changer, fs, newFilePath); err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } } diff --git a/nfs_onwrite.go b/nfs_onwrite.go index 31b227d..27d0bef 100644 --- a/nfs_onwrite.go +++ b/nfs_onwrite.go @@ -52,10 +52,7 @@ func onWrite(ctx context.Context, w *response, userHandle Handler) error { fullPath := fs.Join(path...) info, err := fs.Stat(fullPath) if err != nil { - if os.IsNotExist(err) { - return &NFSStatusError{NFSStatusNoEnt, err} - } - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } if !info.Mode().IsRegular() { return &NFSStatusError{NFSStatusInval, os.ErrInvalid} @@ -65,7 +62,7 @@ func onWrite(ctx context.Context, w *response, userHandle Handler) error { // now the actual op. file, err := fs.OpenFile(fs.Join(path...), os.O_RDWR, info.Mode().Perm()) if err != nil { - return &NFSStatusError{NFSStatusAccess, err} + return statusError(err, NFSStatusAccess) } defer func() { if file != nil { @@ -84,13 +81,13 @@ func onWrite(ctx context.Context, w *response, userHandle Handler) error { var writtenCount int if writtenCount, err = file.WriteAt(req.Data[:end], int64(req.Offset)); err != nil { - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } err = file.Close() file = nil // No more need to close on exit. if err != nil { Log.Errorf("error closing: %v", err) - return &NFSStatusError{NFSStatusIO, err} + return statusError(err, NFSStatusIO) } writer := bytes.NewBuffer([]byte{}) diff --git a/nfs_test.go b/nfs_test.go index 69d5c5a..8828be2 100644 --- a/nfs_test.go +++ b/nfs_test.go @@ -580,3 +580,39 @@ func readDir(target *nfsc.Target, dir string) ([]*readDirEntry, error) { return entries, nil } + +// erofsRenameFS is writable but fails every rename with EROFS. +type erofsRenameFS struct { + billy.Filesystem +} + +func (fs *erofsRenameFS) Rename(oldpath, newpath string) error { + return &os.PathError{Op: "rename", Path: oldpath, Err: syscall.EROFS} +} + +// TestOperationErrorKeepsItsStatus renames on a filesystem that reports itself +// writable but fails the rename with EROFS, and expects NFS3ERR_ROFS. +func TestOperationErrorKeepsItsStatus(t *testing.T) { + mem := memfs.New() + f, err := mem.Create("/from.txt") + if err != nil { + t.Fatal(err) + } + f.Close() + + handler := helpers.NewCachingHandler(helpers.NewNullAuthHandler(&erofsRenameFS{mem}), testCacheLimit) + target := serveAndMount(t, handler) + + err = target.Rename("/from.txt", "/to.txt") + if err == nil { + t.Fatal("expected rename to fail") + } + nfsErr, ok := err.(*nfsc.Error) + if !ok { + t.Fatalf("expected *nfsc.Error, got %T: %v", err, err) + } + if nfsErr.ErrorNum != uint32(nfs.NFSStatusROFS) { + t.Errorf("rename on read-only target reported %v (%d), want NFS3ERR_ROFS (%d)", + nfsErr.ErrorString, nfsErr.ErrorNum, nfs.NFSStatusROFS) + } +}