From a73c6569f931597ccf3fa513ced456a34ce7eaea Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 10 May 2024 16:04:44 +0200 Subject: [PATCH] fix the error message for errors that occur during file transfers we should special case path errors and replace the fs path with the virtual path. Thanks to @nezzzumi for reporting this issue Signed-off-by: Nicola Murino --- internal/common/connection.go | 8 +++++--- internal/common/transfer.go | 6 ++++++ internal/common/transfer_test.go | 8 ++++++++ internal/sftpd/internal_test.go | 2 +- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/common/connection.go b/internal/common/connection.go index 01800d2cd..1dad8c5d0 100644 --- a/internal/common/connection.go +++ b/internal/common/connection.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "path" "strings" @@ -1665,9 +1666,10 @@ func (c *BaseConnection) GetGenericError(err error) error { return fmt.Errorf("%w: %v", sftp.ErrSSHFxFailure, err.Error()) } if err != nil { - if e, ok := err.(*os.PathError); ok { - c.Log(logger.LevelError, "generic path error: %+v", e) - return fmt.Errorf("%w: %v %v", sftp.ErrSSHFxFailure, e.Op, e.Err.Error()) + var pathError *fs.PathError + if errors.As(err, &pathError) { + c.Log(logger.LevelError, "generic path error: %+v", pathError) + return fmt.Errorf("%w: %v %v", sftp.ErrSSHFxFailure, pathError.Op, pathError.Err.Error()) } c.Log(logger.LevelError, "generic error: %+v", err) return fmt.Errorf("%w: %v", sftp.ErrSSHFxFailure, ErrGenericFailure.Error()) diff --git a/internal/common/transfer.go b/internal/common/transfer.go index 5e02afbea..aaedbf621 100644 --- a/internal/common/transfer.go +++ b/internal/common/transfer.go @@ -16,6 +16,8 @@ package common import ( "errors" + "fmt" + "io/fs" "path" "sync" "sync/atomic" @@ -220,6 +222,10 @@ func (t *BaseTransfer) ConvertError(err error) error { } else if t.Fs.IsPermission(err) { return t.Connection.GetPermissionDeniedError() } + var pathError *fs.PathError + if errors.As(err, &pathError) { + return fmt.Errorf("%s %s: %s", pathError.Op, t.GetVirtualPath(), pathError.Err.Error()) + } return err } diff --git a/internal/common/transfer_test.go b/internal/common/transfer_test.go index 8f8203bdd..ff629d467 100644 --- a/internal/common/transfer_test.go +++ b/internal/common/transfer_test.go @@ -16,6 +16,7 @@ package common import ( "errors" + "fmt" "os" "path/filepath" "testing" @@ -226,6 +227,13 @@ func TestTransferErrors(t *testing.T) { conn := NewBaseConnection("id", ProtocolSFTP, "", "", u) transfer := NewBaseTransfer(file, conn, nil, testFile, testFile, "/transfer_test_file", TransferUpload, 0, 0, 0, 0, true, fs, dataprovider.TransferQuota{}) + pathError := &os.PathError{ + Op: "test", + Path: testFile, + Err: os.ErrInvalid, + } + err = transfer.ConvertError(pathError) + assert.EqualError(t, err, fmt.Sprintf("%s %s: %s", pathError.Op, "/transfer_test_file", pathError.Err.Error())) err = transfer.ConvertError(os.ErrNotExist) assert.ErrorIs(t, err, sftp.ErrSSHFxNoSuchFile) err = transfer.ConvertError(os.ErrPermission) diff --git a/internal/sftpd/internal_test.go b/internal/sftpd/internal_test.go index 9a426b301..b7b18e05c 100644 --- a/internal/sftpd/internal_test.go +++ b/internal/sftpd/internal_test.go @@ -1722,7 +1722,7 @@ func TestSCPUploadFiledata(t *testing.T) { transfer.Connection.AddTransfer(transfer) err = scpCommand.getUploadFileData(2, transfer) - assert.True(t, errors.Is(err, os.ErrClosed)) + assert.ErrorContains(t, err, os.ErrClosed.Error()) err = os.Remove(testfile) assert.NoError(t, err)