Explorar o código

improve error messages for generic failures

Nicola Murino %!s(int64=4) %!d(string=hai) anos
pai
achega
0de0d3308c
Modificáronse 7 ficheiros con 48 adicións e 18 borrados
  1. 18 2
      common/connection.go
  2. 7 4
      common/connection_test.go
  3. 3 3
      ftpd/ftpd_test.go
  4. 2 2
      ftpd/handler.go
  5. 2 2
      sftpd/handler.go
  6. 4 2
      sftpd/internal_test.go
  7. 12 3
      sftpd/sftpd_test.go

+ 18 - 2
common/connection.go

@@ -1070,6 +1070,8 @@ func (c *BaseConnection) GetOpUnsupportedError() error {
 // GetQuotaExceededError returns an appropriate storage limit exceeded error for the connection protocol
 func (c *BaseConnection) GetQuotaExceededError() error {
 	switch c.protocol {
+	case ProtocolSFTP:
+		return fmt.Errorf("%w: %v", sftp.ErrSSHFxFailure, ErrQuotaExceeded.Error())
 	case ProtocolFTP:
 		return ftpserver.ErrStorageExceeded
 	default:
@@ -1080,8 +1082,16 @@ func (c *BaseConnection) GetQuotaExceededError() error {
 // IsQuotaExceededError returns true if the given error is a quota exceeded error
 func (c *BaseConnection) IsQuotaExceededError(err error) bool {
 	switch c.protocol {
+	case ProtocolSFTP:
+		if err == nil {
+			return false
+		}
+		if errors.Is(err, ErrQuotaExceeded) {
+			return true
+		}
+		return errors.Is(err, sftp.ErrSSHFxFailure) && strings.Contains(err.Error(), ErrQuotaExceeded.Error())
 	case ProtocolFTP:
-		return errors.Is(err, ftpserver.ErrStorageExceeded)
+		return errors.Is(err, ftpserver.ErrStorageExceeded) || errors.Is(err, ErrQuotaExceeded)
 	default:
 		return errors.Is(err, ErrQuotaExceeded)
 	}
@@ -1092,7 +1102,13 @@ func (c *BaseConnection) GetGenericError(err error) error {
 	switch c.protocol {
 	case ProtocolSFTP:
 		if err == vfs.ErrStorageSizeUnavailable {
-			return sftp.ErrSSHFxOpUnsupported
+			return fmt.Errorf("%w: %v", sftp.ErrSSHFxOpUnsupported, err.Error())
+		}
+		if err != nil {
+			if e, ok := err.(*os.PathError); ok {
+				return fmt.Errorf("%w: %v %v", sftp.ErrSSHFxFailure, e.Op, e.Err.Error())
+			}
+			return fmt.Errorf("%w: %v", sftp.ErrSSHFxFailure, err.Error())
 		}
 		return sftp.ErrSSHFxFailure
 	default:

+ 7 - 4
common/connection_test.go

@@ -230,7 +230,7 @@ func TestErrorsMapping(t *testing.T) {
 		conn.SetProtocol(protocol)
 		err := conn.GetFsError(fs, os.ErrNotExist)
 		if protocol == ProtocolSFTP {
-			assert.EqualError(t, err, sftp.ErrSSHFxNoSuchFile.Error())
+			assert.ErrorIs(t, err, sftp.ErrSSHFxNoSuchFile)
 		} else if protocol == ProtocolWebDAV || protocol == ProtocolFTP || protocol == ProtocolHTTP {
 			assert.EqualError(t, err, os.ErrNotExist.Error())
 		} else {
@@ -244,13 +244,15 @@ func TestErrorsMapping(t *testing.T) {
 		}
 		err = conn.GetFsError(fs, os.ErrClosed)
 		if protocol == ProtocolSFTP {
-			assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
+			assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
+			assert.Contains(t, err.Error(), os.ErrClosed.Error())
 		} else {
 			assert.EqualError(t, err, ErrGenericFailure.Error())
 		}
 		err = conn.GetFsError(fs, ErrPermissionDenied)
 		if protocol == ProtocolSFTP {
-			assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
+			assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
+			assert.Contains(t, err.Error(), ErrPermissionDenied.Error())
 		} else {
 			assert.EqualError(t, err, ErrPermissionDenied.Error())
 		}
@@ -262,7 +264,8 @@ func TestErrorsMapping(t *testing.T) {
 		}
 		err = conn.GetFsError(fs, vfs.ErrStorageSizeUnavailable)
 		if protocol == ProtocolSFTP {
-			assert.EqualError(t, err, sftp.ErrSSHFxOpUnsupported.Error())
+			assert.ErrorIs(t, err, sftp.ErrSSHFxOpUnsupported)
+			assert.Contains(t, err.Error(), vfs.ErrStorageSizeUnavailable.Error())
 		} else {
 			assert.EqualError(t, err, vfs.ErrStorageSizeUnavailable.Error())
 		}

+ 3 - 3
ftpd/ftpd_test.go

@@ -1997,7 +1997,7 @@ func TestAllocateAvailable(t *testing.T) {
 		code, response, err = client.SendCustomCommand("allo 150")
 		assert.NoError(t, err)
 		assert.Equal(t, ftp.StatusFileUnavailable, code)
-		assert.Contains(t, response, common.ErrQuotaExceeded.Error())
+		assert.Contains(t, response, ftpserver.ErrStorageExceeded.Error())
 
 		err = ftpUploadFile(testFilePath, testFileName, testFileSize, client, 0)
 		assert.NoError(t, err)
@@ -2017,7 +2017,7 @@ func TestAllocateAvailable(t *testing.T) {
 		code, response, err = client.SendCustomCommand("allo 50")
 		assert.NoError(t, err)
 		assert.Equal(t, ftp.StatusFileUnavailable, code)
-		assert.Contains(t, response, common.ErrQuotaExceeded.Error())
+		assert.Contains(t, response, ftpserver.ErrStorageExceeded.Error())
 
 		err = client.Quit()
 		assert.NoError(t, err)
@@ -2038,7 +2038,7 @@ func TestAllocateAvailable(t *testing.T) {
 		code, response, err = client.SendCustomCommand("allo 150")
 		assert.NoError(t, err)
 		assert.Equal(t, ftp.StatusFileUnavailable, code)
-		assert.Contains(t, response, common.ErrQuotaExceeded.Error())
+		assert.Contains(t, response, ftpserver.ErrStorageExceeded.Error())
 
 		code, response, err = client.SendCustomCommand("AVBL")
 		assert.NoError(t, err)

+ 2 - 2
ftpd/handler.go

@@ -229,7 +229,7 @@ func (c *Connection) AllocateSpace(size int) error {
 	c.UpdateLastActivity()
 	// check the max allowed file size first
 	if c.User.Filters.MaxUploadFileSize > 0 && int64(size) > c.User.Filters.MaxUploadFileSize {
-		return common.ErrQuotaExceeded
+		return c.GetQuotaExceededError()
 	}
 
 	// we don't have a path here so we check home dir and any virtual folders
@@ -251,7 +251,7 @@ func (c *Connection) AllocateSpace(size int) error {
 			}
 		}
 	}
-	return common.ErrQuotaExceeded
+	return c.GetQuotaExceededError()
 }
 
 // RemoveDir implements ClientDriverExtensionRemoveDir

+ 2 - 2
sftpd/handler.go

@@ -346,7 +346,7 @@ func (c *Connection) handleSFTPUploadToNewFile(fs vfs.Fs, resolvedPath, filePath
 	quotaResult := c.HasSpace(true, false, requestPath)
 	if !quotaResult.HasSpace {
 		c.Log(logger.LevelInfo, "denying file write due to quota limits")
-		return nil, sftp.ErrSSHFxFailure
+		return nil, c.GetQuotaExceededError()
 	}
 
 	if err := common.ExecutePreAction(&c.User, common.OperationPreUpload, resolvedPath, requestPath, c.GetProtocol(), 0, 0); err != nil {
@@ -378,7 +378,7 @@ func (c *Connection) handleSFTPUploadToExistingFile(fs vfs.Fs, pflags sftp.FileO
 	quotaResult := c.HasSpace(false, false, requestPath)
 	if !quotaResult.HasSpace {
 		c.Log(logger.LevelInfo, "denying file write due to quota limits")
-		return nil, sftp.ErrSSHFxFailure
+		return nil, c.GetQuotaExceededError()
 	}
 
 	osFlags := getOSOpenFlags(pflags)

+ 4 - 2
sftpd/internal_test.go

@@ -173,7 +173,8 @@ func TestUploadResumeInvalidOffset(t *testing.T) {
 
 	err = transfer.Close()
 	if assert.Error(t, err) {
-		assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
+		assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
+		assert.Contains(t, err.Error(), "invalid write offset")
 	}
 
 	err = os.Remove(testfile)
@@ -270,7 +271,8 @@ func TestTransferCancelFn(t *testing.T) {
 	transfer.TransferError(errFake)
 	err = transfer.Close()
 	if assert.Error(t, err) {
-		assert.EqualError(t, err, sftp.ErrSSHFxFailure.Error())
+		assert.ErrorIs(t, err, sftp.ErrSSHFxFailure)
+		assert.Contains(t, err.Error(), errFake.Error())
 	}
 	if assert.Error(t, transfer.ErrTransfer) {
 		assert.EqualError(t, transfer.ErrTransfer, errFake.Error())

+ 12 - 3
sftpd/sftpd_test.go

@@ -3564,7 +3564,10 @@ func TestQuotaLimits(t *testing.T) {
 			err = sftpUploadFile(testFilePath, testFileName+".quota", testFileSize, client)
 			assert.NoError(t, err)
 			err = sftpUploadFile(testFilePath, testFileName+".quota.1", testFileSize, client)
-			assert.Error(t, err, "user is over quota files, upload must fail")
+			if assert.Error(t, err, "user is over quota files, upload must fail") {
+				assert.Contains(t, err.Error(), "SSH_FX_FAILURE")
+				assert.Contains(t, err.Error(), common.ErrQuotaExceeded.Error())
+			}
 			// rename should work
 			err = client.Rename(testFileName+".quota", testFileName)
 			assert.NoError(t, err)
@@ -3579,7 +3582,10 @@ func TestQuotaLimits(t *testing.T) {
 			defer conn.Close()
 			defer client.Close()
 			err = sftpUploadFile(testFilePath, testFileName+".quota.1", testFileSize, client)
-			assert.Error(t, err, "user is over quota size, upload must fail")
+			if assert.Error(t, err, "user is over quota size, upload must fail") {
+				assert.Contains(t, err.Error(), "SSH_FX_FAILURE")
+				assert.Contains(t, err.Error(), common.ErrQuotaExceeded.Error())
+			}
 			err = client.Rename(testFileName, testFileName+".quota")
 			assert.NoError(t, err)
 			err = client.Rename(testFileName+".quota", testFileName)
@@ -3595,7 +3601,10 @@ func TestQuotaLimits(t *testing.T) {
 			defer conn.Close()
 			defer client.Close()
 			err = sftpUploadFile(testFilePath1, testFileName1, testFileSize1, client)
-			assert.Error(t, err)
+			if assert.Error(t, err) {
+				assert.Contains(t, err.Error(), "SSH_FX_FAILURE")
+				assert.Contains(t, err.Error(), common.ErrQuotaExceeded.Error())
+			}
 			_, err = client.Stat(testFileName1)
 			assert.Error(t, err)
 			_, err = client.Lstat(testFileName1)