From 1a6863f4b179c5c81f22cd4b4eccbe520975bfc5 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 13 Nov 2020 18:40:18 +0100 Subject: [PATCH] GCS uploads: check Close() error some code simplification too --- common/connection.go | 96 +++++++++++++++++++++++++------------------- vfs/gcsfs.go | 6 ++- vfs/vfs.go | 1 + 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/common/connection.go b/common/connection.go index f46f6caf..2ac4d77f 100644 --- a/common/connection.go +++ b/common/connection.go @@ -458,58 +458,72 @@ func (c *BaseConnection) ignoreSetStat() bool { return false } +func (c *BaseConnection) handleChmod(fsPath, pathForPerms string, attributes *StatAttributes) error { + if !c.User.HasPerm(dataprovider.PermChmod, pathForPerms) { + return c.GetPermissionDeniedError() + } + if c.ignoreSetStat() { + return nil + } + if err := c.Fs.Chmod(c.getRealFsPath(fsPath), attributes.Mode); err != nil { + c.Log(logger.LevelWarn, "failed to chmod path %#v, mode: %v, err: %+v", fsPath, attributes.Mode.String(), err) + return c.GetFsError(err) + } + logger.CommandLog(chmodLogSender, fsPath, "", c.User.Username, attributes.Mode.String(), c.ID, c.protocol, + -1, -1, "", "", "", -1) + return nil +} + +func (c *BaseConnection) handleChown(fsPath, pathForPerms string, attributes *StatAttributes) error { + if !c.User.HasPerm(dataprovider.PermChown, pathForPerms) { + return c.GetPermissionDeniedError() + } + if c.ignoreSetStat() { + return nil + } + if err := c.Fs.Chown(c.getRealFsPath(fsPath), attributes.UID, attributes.GID); err != nil { + c.Log(logger.LevelWarn, "failed to chown path %#v, uid: %v, gid: %v, err: %+v", fsPath, attributes.UID, + attributes.GID, err) + return c.GetFsError(err) + } + logger.CommandLog(chownLogSender, fsPath, "", c.User.Username, "", c.ID, c.protocol, attributes.UID, attributes.GID, + "", "", "", -1) + return nil +} + +func (c *BaseConnection) handleChtimes(fsPath, pathForPerms string, attributes *StatAttributes) error { + if !c.User.HasPerm(dataprovider.PermChtimes, pathForPerms) { + return c.GetPermissionDeniedError() + } + if c.ignoreSetStat() { + return nil + } + if err := c.Fs.Chtimes(c.getRealFsPath(fsPath), attributes.Atime, attributes.Mtime); err != nil { + c.Log(logger.LevelWarn, "failed to chtimes for path %#v, access time: %v, modification time: %v, err: %+v", + fsPath, attributes.Atime, attributes.Mtime, err) + return c.GetFsError(err) + } + accessTimeString := attributes.Atime.Format(chtimesFormat) + modificationTimeString := attributes.Mtime.Format(chtimesFormat) + logger.CommandLog(chtimesLogSender, fsPath, "", c.User.Username, "", c.ID, c.protocol, -1, -1, + accessTimeString, modificationTimeString, "", -1) + return nil +} + // SetStat set StatAttributes for the specified fsPath -// nolint:gocyclo func (c *BaseConnection) SetStat(fsPath, virtualPath string, attributes *StatAttributes) error { pathForPerms := c.getPathForSetStatPerms(fsPath, virtualPath) if attributes.Flags&StatAttrPerms != 0 { - if !c.User.HasPerm(dataprovider.PermChmod, pathForPerms) { - return c.GetPermissionDeniedError() - } - if c.ignoreSetStat() { - return nil - } - if err := c.Fs.Chmod(c.getRealFsPath(fsPath), attributes.Mode); err != nil { - c.Log(logger.LevelWarn, "failed to chmod path %#v, mode: %v, err: %+v", fsPath, attributes.Mode.String(), err) - return c.GetFsError(err) - } - logger.CommandLog(chmodLogSender, fsPath, "", c.User.Username, attributes.Mode.String(), c.ID, c.protocol, - -1, -1, "", "", "", -1) + return c.handleChmod(fsPath, pathForPerms, attributes) } if attributes.Flags&StatAttrUIDGID != 0 { - if !c.User.HasPerm(dataprovider.PermChown, pathForPerms) { - return c.GetPermissionDeniedError() - } - if c.ignoreSetStat() { - return nil - } - if err := c.Fs.Chown(c.getRealFsPath(fsPath), attributes.UID, attributes.GID); err != nil { - c.Log(logger.LevelWarn, "failed to chown path %#v, uid: %v, gid: %v, err: %+v", fsPath, attributes.UID, - attributes.GID, err) - return c.GetFsError(err) - } - logger.CommandLog(chownLogSender, fsPath, "", c.User.Username, "", c.ID, c.protocol, attributes.UID, attributes.GID, - "", "", "", -1) + return c.handleChown(fsPath, pathForPerms, attributes) } if attributes.Flags&StatAttrTimes != 0 { - if !c.User.HasPerm(dataprovider.PermChtimes, pathForPerms) { - return c.GetPermissionDeniedError() - } - if c.ignoreSetStat() { - return nil - } - if err := c.Fs.Chtimes(c.getRealFsPath(fsPath), attributes.Atime, attributes.Mtime); err != nil { - c.Log(logger.LevelWarn, "failed to chtimes for path %#v, access time: %v, modification time: %v, err: %+v", - fsPath, attributes.Atime, attributes.Mtime, err) - return c.GetFsError(err) - } - accessTimeString := attributes.Atime.Format(chtimesFormat) - modificationTimeString := attributes.Mtime.Format(chtimesFormat) - logger.CommandLog(chtimesLogSender, fsPath, "", c.User.Username, "", c.ID, c.protocol, -1, -1, - accessTimeString, modificationTimeString, "", -1) + return c.handleChtimes(fsPath, pathForPerms, attributes) } if attributes.Flags&StatAttrSize != 0 { diff --git a/vfs/gcsfs.go b/vfs/gcsfs.go index d5c6350c..e0e3aee9 100644 --- a/vfs/gcsfs.go +++ b/vfs/gcsfs.go @@ -218,8 +218,12 @@ func (fs *GCSFs) Create(name string, flag int) (*os.File, *PipeWriter, func(), e } go func() { defer cancelFn() - defer objectWriter.Close() + n, err := io.Copy(objectWriter, r) + closeErr := objectWriter.Close() + if err == nil { + err = closeErr + } r.CloseWithError(err) //nolint:errcheck p.Done(err) fsLog(fs, logger.LevelDebug, "upload completed, path: %#v, readed bytes: %v, err: %v", name, n, err) diff --git a/vfs/vfs.go b/vfs/vfs.go index c9086ff3..b1aa11d1 100644 --- a/vfs/vfs.go +++ b/vfs/vfs.go @@ -57,6 +57,7 @@ type Fs interface { GetMimeType(name string) (string, error) } +// ErrVfsUnsupported defines the error for an unsupported VFS operation var ErrVfsUnsupported = errors.New("Not supported") // QuotaCheckResult defines the result for a quota check