ssh commands: return better error messages

This improve the fix for #171 and return better error message for
SSH commands other than SCP too

common/connection.go

@@ -320,7 +320,7 @@ func (c *BaseConnection) RemoveDir(fsPath, virtualPath string) error {
 	}
 	if !fi.IsDir() || fi.Mode()&os.ModeSymlink == os.ModeSymlink {
 		c.Log(logger.LevelDebug, "cannot remove %#v is not a directory", fsPath)
-		return c.GetGenericError()
+		return c.GetGenericError(nil)
 	}
 
 	if err := c.Fs.Remove(fsPath, true); err != nil {
@@ -379,7 +379,7 @@ func (c *BaseConnection) Rename(fsSourcePath, fsTargetPath, virtualSourcePath, v
 	}
 	if !c.hasSpaceForRename(virtualSourcePath, virtualTargetPath, initialSize, fsSourcePath) {
 		c.Log(logger.LevelInfo, "denying cross rename due to space limit")
-		return c.GetGenericError()
+		return c.GetGenericError(ErrQuotaExceeded)
 	}
 	if err := c.Fs.Rename(fsSourcePath, fsTargetPath); err != nil {
 		c.Log(logger.LevelWarn, "failed to rename %#v -> %#v: %+v", fsSourcePath, fsTargetPath, err)
@@ -412,7 +412,7 @@ func (c *BaseConnection) CreateSymlink(fsSourcePath, fsTargetPath, virtualSource
 	}
 	if c.isCrossFoldersRequest(virtualSourcePath, virtualTargetPath) {
 		c.Log(logger.LevelWarn, "cross folder symlink is not supported, src: %v dst: %v", virtualSourcePath, virtualTargetPath)
-		return c.GetGenericError()
+		return c.GetOpUnsupportedError()
 	}
 	if c.User.IsMappedPath(fsSourcePath) {
 		c.Log(logger.LevelWarn, "symlinking a directory mapped as virtual folder is not allowed: %#v", fsSourcePath)
@@ -932,11 +932,14 @@ func (c *BaseConnection) GetOpUnsupportedError() error {
 }
 
 // GetGenericError returns an appropriate generic error for the connection protocol
-func (c *BaseConnection) GetGenericError() error {
+func (c *BaseConnection) GetGenericError(err error) error {
 	switch c.protocol {
 	case ProtocolSFTP:
 		return sftp.ErrSSHFxFailure
 	default:
+		if err == ErrPermissionDenied || err == ErrNotExist || err == ErrOpUnsupported || err == ErrQuotaExceeded {
+			return err
+		}
 		return ErrGenericFailure
 	}
 }
@@ -948,7 +951,7 @@ func (c *BaseConnection) GetFsError(err error) error {
 	} else if c.Fs.IsPermission(err) {
 		return c.GetPermissionDeniedError()
 	} else if err != nil {
-		return c.GetGenericError()
+		return c.GetGenericError(err)
 	}
 	return nil
 }

common/connection_test.go

@@ -227,7 +227,7 @@ func TestRemoveDir(t *testing.T) {
 	assert.NoError(t, err)
 	err = c.RemoveDir(testDir, "testDir")
 	if assert.Error(t, err) {
-		assert.EqualError(t, err, c.GetGenericError().Error())
+		assert.EqualError(t, err, c.GetGenericError(err).Error())
 	}
 	err = os.Remove(testDir)
 	assert.NoError(t, err)
@@ -349,7 +349,7 @@ func TestRename(t *testing.T) {
 	// and so the remaining space cannot be computed
 	err = c.Rename(filepath.Join(user.GetHomeDir(), "adir"), filepath.Join(user.GetHomeDir(), "another"), "/vdir1/sub/a", "/vdir2/b")
 	if assert.Error(t, err) {
-		assert.EqualError(t, err, c.GetGenericError().Error())
+		assert.EqualError(t, err, c.GetGenericError(err).Error())
 	}
 
 	err = os.RemoveAll(mappedPath1)
@@ -409,7 +409,7 @@ func TestCreateSymlink(t *testing.T) {
 	}
 	err = c.CreateSymlink(filepath.Join(user.GetHomeDir(), "b"), mappedPath1, "/vdir1/b", "/vdir2/b")
 	if assert.Error(t, err) {
-		assert.EqualError(t, err, c.GetGenericError().Error())
+		assert.EqualError(t, err, c.GetOpUnsupportedError().Error())
 	}
 	err = c.CreateSymlink(mappedPath1, filepath.Join(mappedPath1, "b"), "/vdir1/a", "/vdir1/b")
 	if assert.Error(t, err) {
@@ -1140,6 +1140,12 @@ func TestErrorsMapping(t *testing.T) {
 		} else {
 			assert.EqualError(t, err, ErrGenericFailure.Error())
 		}
+		err = conn.GetFsError(ErrPermissionDenied)
+		if protocol == ProtocolSFTP {
+			assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
+		} else {
+			assert.EqualError(t, err, ErrPermissionDenied.Error())
+		}
 		err = conn.GetFsError(nil)
 		assert.NoError(t, err)
 		err = conn.GetOpUnsupportedError()

ftpd/handler.go

@@ -101,7 +101,7 @@ func (c *Connection) Remove(name string) error {
 
 	if fi.IsDir() && fi.Mode()&os.ModeSymlink != os.ModeSymlink {
 		c.Log(logger.LevelDebug, "cannot remove %#v is not a file/symlink", p)
-		return c.GetGenericError()
+		return c.GetGenericError(nil)
 	}
 	return c.RemoveFile(p, name, fi)
 }

sftpd/scp.go

@@ -124,9 +124,8 @@ func (c *scpCommand) handleCreateDir(dirPath string) error {
 	}
 	if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(dirPath)) {
 		c.connection.Log(logger.LevelWarn, "error creating dir: %#v, permission denied", dirPath)
-		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(common.ErrPermissionDenied)
-		c.sendErrorMessage(err)
+		return common.ErrPermissionDenied
-		return err
 	}
 
 	err = c.createDir(p)
@@ -240,9 +239,8 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 
 	if !c.connection.User.IsFileAllowed(uploadFilePath) {
 		c.connection.Log(logger.LevelWarn, "writing file %#v is not allowed", uploadFilePath)
-		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(common.ErrPermissionDenied)
-		c.sendErrorMessage(err)
+		return common.ErrPermissionDenied
-		return err
 	}
 
 	p, err := c.connection.Fs.ResolvePath(uploadFilePath)
@@ -259,9 +257,8 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 	if (statErr == nil && stat.Mode()&os.ModeSymlink == os.ModeSymlink) || c.connection.Fs.IsNotExist(statErr) {
 		if !c.connection.User.HasPerm(dataprovider.PermUpload, path.Dir(uploadFilePath)) {
 			c.connection.Log(logger.LevelWarn, "cannot upload file: %#v, permission denied", uploadFilePath)
-			err := c.connection.Fs.GetPermissionError()
+			c.sendErrorMessage(common.ErrPermissionDenied)
-			c.sendErrorMessage(err)
+			return common.ErrPermissionDenied
-			return err
 		}
 		return c.handleUploadFile(p, filePath, sizeToRead, true, 0, uploadFilePath)
 	}
@@ -281,9 +278,8 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 
 	if !c.connection.User.HasPerm(dataprovider.PermOverwrite, uploadFilePath) {
 		c.connection.Log(logger.LevelWarn, "cannot overwrite file: %#v, permission denied", uploadFilePath)
-		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(common.ErrPermissionDenied)
-		c.sendErrorMessage(err)
+		return common.ErrPermissionDenied
-		return err
 	}
 
 	if common.Config.IsAtomicUploadEnabled() && c.connection.Fs.IsAtomicUploadSupported() {
@@ -463,9 +459,8 @@ func (c *scpCommand) handleDownload(filePath string) error {
 	if stat.IsDir() {
 		if !c.connection.User.HasPerm(dataprovider.PermDownload, filePath) {
 			c.connection.Log(logger.LevelWarn, "error downloading dir: %#v, permission denied", filePath)
-			err := c.connection.Fs.GetPermissionError()
+			c.sendErrorMessage(common.ErrPermissionDenied)
-			c.sendErrorMessage(err)
+			return common.ErrPermissionDenied
-			return err
 		}
 		err = c.handleRecursiveDownload(p, stat)
 		return err
@@ -473,16 +468,14 @@ func (c *scpCommand) handleDownload(filePath string) error {
 
 	if !c.connection.User.HasPerm(dataprovider.PermDownload, path.Dir(filePath)) {
 		c.connection.Log(logger.LevelWarn, "error downloading dir: %#v, permission denied", filePath)
-		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(common.ErrPermissionDenied)
-		c.sendErrorMessage(err)
+		return common.ErrPermissionDenied
-		return err
 	}
 
 	if !c.connection.User.IsFileAllowed(filePath) {
 		c.connection.Log(logger.LevelWarn, "reading file %#v is not allowed", filePath)
-		err := c.connection.Fs.GetPermissionError()
+		c.sendErrorMessage(common.ErrPermissionDenied)
-		c.sendErrorMessage(err)
+		return common.ErrPermissionDenied
-		return err
 	}
 
 	file, r, cancelFn, err := c.connection.Fs.Open(p, 0)

vfs/gcsfs.go

@@ -402,16 +402,6 @@ func (GCSFs) IsPermission(err error) bool {
 	return strings.Contains(err.Error(), "403")
 }
 
-// GetPermissionError returns a permission error for this FS
-func (GCSFs) GetPermissionError() error {
-	return errors.New("403 permission denied")
-}
-
-// GetNotExistError returns a not exist error for this FS
-func (GCSFs) GetNotExistError() error {
-	return errors.New("404 no such file or directory")
-}
-
 // CheckRootPath creates the specified local root directory if it does not exists
 func (fs GCSFs) CheckRootPath(username string, uid int, gid int) bool {
 	// we need a local directory for temporary files

vfs/osfs.go

@@ -183,16 +183,6 @@ func (OsFs) IsPermission(err error) bool {
 	return os.IsPermission(err)
 }
 
-// GetPermissionError returns a permission error for this FS
-func (OsFs) GetPermissionError() error {
-	return os.ErrPermission
-}
-
-// GetNotExistError returns a not exist error for this FS
-func (OsFs) GetNotExistError() error {
-	return os.ErrNotExist
-}
-
 // CheckRootPath creates the root directory if it does not exists
 func (fs OsFs) CheckRootPath(username string, uid int, gid int) bool {
 	var err error

vfs/s3fs.go

@@ -427,16 +427,6 @@ func (S3Fs) IsPermission(err error) bool {
 	return strings.Contains(err.Error(), "403")
 }
 
-// GetPermissionError returns a permission error for this FS
-func (S3Fs) GetPermissionError() error {
-	return errors.New("403 permission denied")
-}
-
-// GetNotExistError returns a not exist error for this FS
-func (S3Fs) GetNotExistError() error {
-	return errors.New("404 no such file or directory")
-}
-
 // CheckRootPath creates the specified local root directory if it does not exists
 func (fs S3Fs) CheckRootPath(username string, uid int, gid int) bool {
 	// we need a local directory for temporary files

vfs/vfs.go

@@ -40,8 +40,6 @@ type Fs interface {
 	ResolvePath(sftpPath string) (string, error)
 	IsNotExist(err error) bool
 	IsPermission(err error) bool
-	GetPermissionError() error
-	GetNotExistError() error
 	ScanRootDirContents() (int, int64, error)
 	GetDirSize(dirname string) (int, int64, error)
 	GetAtomicUploadPath(name string) string