-
Notifications
You must be signed in to change notification settings - Fork 35
Description
Describe the bug
The SFTP backend's auto-disconnect timer has inverted logic in connTimerStart() that prematurely closes valid, working connections after idle periods. The condition (!IsValid() || !IsNil()) closes valid clients but ignores typed-nil clients, opposite of the intended behavior.
To Reproduce
Steps to reproduce the behavior:
- Configure SFTP backend with
AutoDisconnectenabled (default: 10 seconds) - Perform an SFTP file operation (read, write, etc.)
- Wait for idle timeout period (10+ seconds with no operations)
- Attempt another SFTP operation
- See connection errors, hung connections, or forced reconnection delays
Expected behavior
- Timer should close valid connections only when appropriate for auto-disconnect
- Timer should safely handle typed-nil clients without attempting to call Close() on them
- Subsequent operations after idle period should reconnect cleanly without errors
Actual behavior
- Valid connections are aggressively closed by timer during idle periods
- Users experience "connection reset", "broken pipe", or hung connection errors
- Performance degradation from constant reconnection overhead
- Typed-nil edge case is not handled (though this is masked by other issues)
Root Cause
Lines 143-144 and 148-149 in backend/sftp/fileSystem.go:
// BUGGY LOGIC - inverted conditions
if fs.sftpclient != nil && (!reflect.ValueOf(fs.sftpclient).IsValid() || !reflect.ValueOf(fs.sftpclient).IsNil()) {
_ = fs.sftpclient.Close() // Closes valid clients incorrectly
}Should be:
// CORRECT LOGIC
if fs.sftpclient != nil && reflect.ValueOf(fs.sftpclient).IsValid() && !reflect.ValueOf(fs.sftpclient).IsNil() {
_ = fs.sftpclient.Close() // Only close valid, non-nil clients
}Additional context
This bug was introduced in the recent typed-nil fix. The intention was to add typed-nil detection to prevent panics, but the boolean logic was inverted (using OR with negations instead of AND with correct conditions). The bug manifests primarily with infrequent SFTP operations where the auto-disconnect timer has time to fire between operations.