flysystem-sftp
flysystem-sftp copied to clipboard
FIX: isConnected() does not detect that the server closed the connection
@frankdejonge this PR is ready for review :)
@frankdejonge Any chance this is going to get merged? This problem was described in https://github.com/thephpleague/flysystem-sftp/issues/66 but that issue was closed while the problem still exists.
In phpseclib the connection is only marked as closed after an operation has failed. So when iterating over a list of files and calling has
the result of the call will be false when the connection has gone away. The connection will be opened the next time has
is called. This problem leads to files incorrectly being marked as not existing.
The solution of calling ping to check if a connection is open is very similar to the FTP adapter which sends a NOOP package in the isConnected
method.
This PR is for v1 though the same problem exists in v2 when using the simple connectivity checker.
This PR is on my list for this weekend to review.
Hello guys, We are in the same situation as the one described here : https://github.com/thephpleague/flysystem-sftp/issues/66. Our worker(long running process) is handling messages that upload files on a sftp server, after some time it get disconnected generating an error then the message is queued again for processing, after 2-3 retries it finally get uploaded. Do you think that PR will fix the problem ? Thank you very much for your work and the amazing job you do on the different the php league libraries.
This PR is on my list for this weekend to review.
Hi, any news on this? I'm having the same problem
We actually had to implement our own retry layer due to this issue. @frankdejonge Any problems with this solution? I can rebase it on v2.
Hello it seems like @frankdejonge worked on it can't wait for that feature thanks man https://github.com/thephpleague/flysystem-sftp/commit/e1e8afa6aaad5faa582f6373ad71dcd8367a1893
Hi, I just had the issue, does the fix will be available soon ? @olsavmic Can you share the workaround you used pls ?
use League\Flysystem\PhpseclibV2\ConnectivityChecker;
use phpseclib\Net\SFTP;
class SftpPingConnectivityChecker implements ConnectivityChecker
{
public function isConnected(SFTP $connection): bool
{
return $connection->ping() && $connection->isConnected();
}
}
@kiropowered We just register our own connectivity checker that performs pinging. Depends on your project/configuration but if you're by chance using oneup/flysystem-bundle, the connectivity checker can be overridden directly in the configuration.
oneup_flysystem:
adapters:
sftp_adapter:
sftp:
options:
connectivityChecker: Fulfillment\CommonBundle\SFTP\SftpPingConnectivityChecker
Thank you for the answer. That's what I tried to do, unfortunatly I used the league/flysystem-bundle wich does not allow to override the connectivityChecker in the config(option currently not available). Sad the "official" bundle does not support that override, I will probably switch to this bundle thank you again
@kiropowered check this out https://stackoverflow.com/questions/70436411/how-to-override-a-file-in-vendor-directory we're using Laravel so we could not directly override the class, but using this method with Composer did.
This Pr could be closed since this fonctions has been implemented.
This is usable like this on Laravel 6+
'my_sftp_disk' => [
'driver' => 'sftp',
'host' => env('MY_SFTP_DISK_HOST'),
'username' => env('MY_SFTP_DISK_USERNAME'),
'password' => env('MY_SFTP_DISK_PASSWORD'),
'usePingForConnectivityCheck' => true,
],
Thanks @pvgnd for your contribution.