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 <nicola.murino@gmail.com>
drakkan · May 10, 2024 · a73c656 · a73c656
1 parent a6a92f0
commit a73c656
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)