Skip to content

Commit de6bb54

Browse files
authored
Add support for phpseclib v3 and v4 (#12)
* Remove support for phpseclib v1 composer.json only allows phpseclib ^2.0, so the PEAR-style Net_SFTP, Crypt_RSA and NET_SFTP_LOCAL_FILE code paths are unreachable. * Use SFTP::filesize() instead of size() size() was removed in phpseclib v4, filesize() is available in v2, v3 and v4. * Resolve SOURCE_LOCAL_FILE from the SFTP connection class The constant is defined on phpseclib\Net\SFTP (v2), phpseclib3\Net\SFTP (v3) and phpseclib4\Net\SFTP (v4), so read it from the instantiated class. * Pass recursive flag to SFTP::mkdir() as third argument The signature is mkdir($dir, $mode = -1, $recursive = false) in phpseclib v2, v3 and v4, so `true` was passed as $mode and directories were never created recursively. With strict_types and phpseclib v4's typed $mode parameter this also causes a TypeError. * Link to phpseclib GitHub repository instead of SourceForge * Add support for phpseclib v3 and v4 * Delete directories recursively in deleteDir() and cleanDir() for SFTP Without the recursive flag phpseclib removes the path as a file, which fails for directories. The FTP implementation already deletes recursively. * Run SFTP tests against an SFTP server in CI The SFTP test was always skipped. Start an atmoz/sftp service container in CI and configure the test via SFTP_* environment variables. The test is still skipped if SFTP_HOST is not set. * Don't cancel remaining CI jobs when one job fails
1 parent 1fb6a80 commit de6bb54

4 files changed

Lines changed: 115 additions & 46 deletions

File tree

‎.github/workflows/main.yml‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,28 @@ jobs:
66
tests:
77
runs-on: ubuntu-latest
88

9+
services:
10+
sftp:
11+
image: atmoz/sftp
12+
ports:
13+
- 2222:22
14+
env:
15+
# user:password:uid:gid:writable directory inside the chroot
16+
SFTP_USERS: user:password:::upload
17+
918
strategy:
19+
# run all phpseclib/PHP combinations even if one of them fails
20+
fail-fast: false
1021
matrix:
1122
php: [8.0, 8.1]
23+
phpseclib: ['^2.0.35', '^3.0.57', '^4.0.2']
24+
exclude:
25+
- php: 8.0
26+
phpseclib: '^4.0.2'
27+
include:
28+
# phpseclib v2 has open security advisories, which Composer blocks by default
29+
- phpseclib: '^2.0.35'
30+
advisories-block: '0'
1231

1332
steps:
1433
- name: Checkout code
@@ -24,7 +43,26 @@ jobs:
2443
run: composer validate
2544

2645
- name: Install dependencies
27-
run: composer install --prefer-dist --no-progress --no-interaction --no-suggest
46+
env:
47+
COMPOSER_POLICY_ADVISORIES_BLOCK: ${{ matrix.advisories-block || '1' }}
48+
run: |
49+
composer require phpseclib/phpseclib:"${{ matrix.phpseclib }}" --no-update
50+
composer update --prefer-dist --no-progress --no-interaction
51+
52+
- name: Wait for SFTP server
53+
run: |
54+
for i in $(seq 30); do
55+
ssh-keyscan -p 2222 127.0.0.1 > /dev/null 2>&1 && exit 0
56+
sleep 1
57+
done
58+
echo "SFTP server did not start" >&2
59+
exit 1
2860
2961
- name: Run test suite
62+
env:
63+
SFTP_HOST: 127.0.0.1
64+
SFTP_PORT: 2222
65+
SFTP_USER: user
66+
SFTP_PASSWORD: password
67+
SFTP_ROOT: /upload
3068
run: php vendor/bin/codecept run

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"php": "^8.0",
1616
"codeception/codeception": "^5.0",
1717
"codeception/module-filesystem": "^3.0",
18-
"phpseclib/phpseclib": "^2.0.35"
18+
"phpseclib/phpseclib": "^2.0.35 || ^3.0.57 || ^4.0.2"
1919
},
2020
"require-dev": {
2121
"ext-ftp": "*"

‎src/Codeception/Module/FTP.php‎

Lines changed: 51 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
use Codeception\TestInterface;
88
use Codeception\Exception\ModuleException;
9+
use phpseclib\Net\SFTP AS SFTPv2;
10+
use phpseclib3\Net\SFTP AS SFTPv3;
11+
use phpseclib4\Net\SFTP AS SFTPv4;
912

1013
/**
1114
*
@@ -24,13 +27,11 @@
2427
* * SFTP
2528
*
2629
* Connection uses php build in FTP client for FTP,
27-
* connection to SFTP uses [phpseclib](https://phpseclib.sourceforge.net/) pulled in using composer.
30+
* connection to SFTP uses [phpseclib](https://github.com/phpseclib/phpseclib) pulled in using composer.
2831
*
29-
* For SFTP, add [phpseclib](https://phpseclib.sourceforge.net/) to require list.
32+
* For SFTP, add [phpseclib](https://github.com/phpseclib/phpseclib) to require list.
3033
* ```
31-
* "require": {
32-
* "phpseclib/phpseclib": "^2.0.14"
33-
* }
34+
* composer require phpseclib/phpseclib
3435
* ```
3536
*
3637
* ## Status
@@ -93,7 +94,7 @@ class FTP extends Filesystem
9394
{
9495
/**
9596
* FTP/SFTP connection handler
96-
* @var null|bool|\Net_SFTP|\phpseclib\Net\SFTP|resource
97+
* @var null|bool|SFTPv2|SFTPv3|SFTPv4|resource
9798
*/
9899
protected $ftp = null;
99100

@@ -352,7 +353,7 @@ public function deleteFile(string $filename): void
352353
*/
353354
public function deleteDir(string $dirname): void
354355
{
355-
$this->delete($this->absolutizePath($dirname));
356+
$this->delete($this->absolutizePath($dirname), true);
356357
}
357358

358359
/**
@@ -516,7 +517,7 @@ private function _closeConnection(): void
516517
*/
517518
private function _listFiles(string $path): array
518519
{
519-
$files = $this->isSFTP() ? @$this->ftp->nlist($path) : @ftp_nlist($this->ftp, $path);
520+
$files = $this->isSFTP() ? $this->sftp('nlist', $path) : @ftp_nlist($this->ftp, $path);
520521

521522
if ($files === false) {
522523
$this->fail("couldn't list files");
@@ -530,7 +531,7 @@ private function _listFiles(string $path): array
530531
*/
531532
private function _directory(): string
532533
{
533-
$pwd = $this->isSFTP() ? @$this->ftp->pwd() : @ftp_pwd($this->ftp);
534+
$pwd = $this->isSFTP() ? $this->sftp('pwd') : @ftp_pwd($this->ftp);
534535

535536
if (!$pwd) {
536537
$this->fail("couldn't get current directory");
@@ -544,7 +545,7 @@ private function _directory(): string
544545
*/
545546
private function _changeDirectory(string $path): void
546547
{
547-
$changed = $this->isSFTP() ? @$this->ftp->chdir($path) : @ftp_chdir($this->ftp, $path);
548+
$changed = $this->isSFTP() ? $this->sftp('chdir', $path) : @ftp_chdir($this->ftp, $path);
548549

549550
if (!$changed) {
550551
$this->fail("couldn't change directory {$path}");
@@ -565,7 +566,7 @@ private function _openFile(string $filename): void
565566
$tmp_file = $this->config['tmp'] . "/ftp_data_file.tmp";
566567

567568
if ($this->isSFTP()) {
568-
$downloaded = @$this->ftp->get($filename, $tmp_file);
569+
$downloaded = $this->sftp('get', $filename, $tmp_file);
569570
} else {
570571
$downloaded = @ftp_get($this->ftp, $tmp_file, $filename, FTP_BINARY);
571572
}
@@ -602,9 +603,9 @@ private function _writeToFile(string $filename, string $contents): void
602603

603604
// Upload the file to server
604605
if ($this->isSFTP()) {
605-
$flag = defined('NET_SFTP_LOCAL_FILE') ? NET_SFTP_LOCAL_FILE : \phpseclib\Net\SFTP::SOURCE_LOCAL_FILE;
606+
$flag = $this->ftp::SOURCE_LOCAL_FILE;
606607

607-
$uploaded = @$this->ftp->put($filename, $tmp_file, $flag);
608+
$uploaded = $this->sftp('put', $filename, $tmp_file, $flag);
608609
} else {
609610
$uploaded = ftp_put($this->ftp, $filename, $tmp_file, FTP_BINARY);
610611
}
@@ -619,7 +620,7 @@ private function _writeToFile(string $filename, string $contents): void
619620
*/
620621
private function makeDirectory(string $path): void
621622
{
622-
$created = $this->isSFTP() ? @$this->ftp->mkdir($path, true) : @ftp_mkdir($this->ftp, $path);
623+
$created = $this->isSFTP() ? $this->sftp('mkdir', $path, -1, true) : @ftp_mkdir($this->ftp, $path);
623624

624625
if (!$created) {
625626
$this->fail("couldn't make directory {$path}");
@@ -633,7 +634,7 @@ private function makeDirectory(string $path): void
633634
*/
634635
private function renameDirectory(string $path, string $rename): void
635636
{
636-
$renamed = $this->isSFTP() ? @$this->ftp->rename($path, $rename) : @ftp_rename($this->ftp, $path, $rename);
637+
$renamed = $this->isSFTP() ? $this->sftp('rename', $path, $rename) : @ftp_rename($this->ftp, $path, $rename);
637638

638639
if (!$renamed) {
639640
$this->fail("couldn't rename directory {$path} to {$rename}");
@@ -647,7 +648,7 @@ private function renameDirectory(string $path, string $rename): void
647648
*/
648649
private function delete(string $filename, bool $isDir = false): void
649650
{
650-
$deleted = $this->isSFTP() ? @$this->ftp->delete($filename, $isDir) : @$this->ftpDelete($filename);
651+
$deleted = $this->isSFTP() ? $this->sftp('delete', $filename, $isDir) : @$this->ftpDelete($filename);
651652

652653
if (!$deleted) {
653654
$this->fail("couldn't delete {$filename}");
@@ -685,7 +686,7 @@ private function ftpDelete(string $directory): bool
685686
private function clearDirectory(string $path): void
686687
{
687688
$this->debug("Clear directory: {$path}");
688-
$this->delete($path);
689+
$this->delete($path, true);
689690
$this->makeDirectory($path);
690691
}
691692

@@ -694,7 +695,7 @@ private function clearDirectory(string $path): void
694695
*/
695696
private function size(string $filename): int
696697
{
697-
$size = $this->isSFTP() ? (int)@$this->ftp->size($filename) : @ftp_size($this->ftp, $filename);
698+
$size = $this->isSFTP() ? (int)$this->sftp('filesize', $filename) : @ftp_size($this->ftp, $filename);
698699

699700
if ($size < 0) {
700701
$this->fail("couldn't get the file size for {$filename}");
@@ -709,7 +710,7 @@ private function size(string $filename): int
709710
private function modified(string $filename): int
710711
{
711712
if ($this->isSFTP()) {
712-
$info = @$this->ftp->lstat($filename);
713+
$info = $this->sftp('lstat', $filename);
713714
if ($info) {
714715
return $info['mtime'];
715716
}
@@ -722,29 +723,32 @@ private function modified(string $filename): int
722723

723724
protected function sftpConnect(string $user, string $password): void
724725
{
725-
if (class_exists('Net_SFTP')) {
726-
$this->ftp = new \Net_SFTP($this->config['host'], $this->config['port'], $this->config['timeout']);
727-
} elseif (class_exists(\phpseclib\Net\SFTP::class)) {
728-
$this->ftp = new \phpseclib\Net\SFTP($this->config['host'], $this->config['port'], $this->config['timeout']);
726+
if (class_exists(SFTPv4::class)) {
727+
$this->ftp = new SFTPv4($this->config['host'], $this->config['port'], $this->config['timeout']);
728+
} elseif (class_exists(SFTPv3::class)) {
729+
$this->ftp = new SFTPv3($this->config['host'], $this->config['port'], $this->config['timeout']);
730+
} elseif (class_exists(SFTPv2::class)) {
731+
$this->ftp = new SFTPv2($this->config['host'], $this->config['port'], $this->config['timeout']);
729732
} else {
730733
throw new ModuleException('FTP', 'phpseclib/phpseclib library is not installed');
731734
}
732735

733736
if (!empty($this->config['key'])) {
734737
$keyFile = file_get_contents($this->config['key']);
735738

736-
if (class_exists('Crypt_RSA')) {
737-
$password = new \Crypt_RSA();
739+
if (class_exists(\phpseclib4\Crypt\PublicKeyLoader::class)) {
740+
$password = \phpseclib4\Crypt\PublicKeyLoader::load($keyFile);
741+
} elseif (class_exists(\phpseclib3\Crypt\PublicKeyLoader::class)) {
742+
$password = \phpseclib3\Crypt\PublicKeyLoader::load($keyFile);
738743
} elseif (class_exists(\phpseclib\Crypt\RSA::class)) {
739744
$password = new \phpseclib\Crypt\RSA();
745+
$password->loadKey($keyFile);
740746
} else {
741747
throw new ModuleException('FTP', 'phpseclib/phpseclib library is not installed');
742748
}
743-
744-
$password->loadKey($keyFile);
745749
}
746750

747-
if (!$this->ftp->login($user, $password)) {
751+
if (!$this->sftp('login', $user, $password)) {
748752
$this->fail('failed to authenticate user');
749753
}
750754
}
@@ -768,6 +772,25 @@ protected function ftpConnect(string $user, string $password): void
768772
}
769773
}
770774

775+
/**
776+
* Calls a method on the phpseclib SFTP connection.
777+
*
778+
* phpseclib v2/v3 return false on failure, v4 throws exceptions and
779+
* returns void for many methods. Normalize to false on failure and
780+
* true for void results.
781+
*/
782+
private function sftp(string $method, mixed ...$args): mixed
783+
{
784+
try {
785+
$result = @$this->ftp->{$method}(...$args);
786+
} catch (\Exception $e) {
787+
$this->debug("SFTP {$method}() failed: {$e->getMessage()}");
788+
return false;
789+
}
790+
791+
return $result ?? true;
792+
}
793+
771794
protected function isSFTP(): bool
772795
{
773796
return strtolower($this->config['type']) === 'sftp';

‎tests/unit/Codeception/Module/SFTPTest.php‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,43 @@
88

99
final class SFTPTest extends TestCase
1010
{
11-
protected array $config = [
12-
'host' => '127.0.0.1',
13-
'port' => 22,
14-
'tmp' => 'temp',
15-
'user' => 'user',
16-
'password' => 'password',
17-
'type' => 'sftp'
18-
];
19-
2011
protected ?FTP $module = null;
2112

13+
/**
14+
* Writable directory on the SFTP server the test is run in
15+
*/
16+
protected string $root = '/';
17+
2218
public function _setUp()
2319
{
20+
$this->root = getenv('SFTP_ROOT') ?: '/';
21+
2422
$container = Stub::make('Codeception\Lib\ModuleContainer');
2523
$this->module = new FTP($container);
26-
$this->module->_setConfig($this->config);
24+
$this->module->_setConfig([
25+
'host' => getenv('SFTP_HOST') ?: '127.0.0.1',
26+
'port' => (int)(getenv('SFTP_PORT') ?: 22),
27+
'tmp' => sys_get_temp_dir(),
28+
'user' => getenv('SFTP_USER') ?: 'user',
29+
'password' => getenv('SFTP_PASSWORD') ?: 'password',
30+
'type' => 'sftp'
31+
]);
2732
}
2833

2934
public function testFlow()
3035
{
31-
$this->markTestSkipped('Requires SFTP server');
36+
if (!getenv('SFTP_HOST')) {
37+
$this->markTestSkipped('Requires SFTP server, set SFTP_HOST to run this test');
38+
}
3239

3340
$this->module->_before(Stub::makeEmpty('\Codeception\Test\Test'));
3441

35-
self::assertEquals('/', $this->module->grabDirectory());
42+
$this->module->amInPath($this->root);
43+
self::assertEquals($this->root, $this->module->grabDirectory());
3644

3745
$this->module->makeDir('TESTING');
3846
$this->module->amInPath('TESTING');
39-
self::assertEquals('/TESTING', $this->module->grabDirectory());
47+
self::assertEquals(rtrim($this->root, '/') . '/TESTING', $this->module->grabDirectory());
4048

4149
$files = $this->module->grabFileList();
4250
$this->module->writeToFile('test_ftp_123.txt', 'some data added here');
@@ -78,9 +86,9 @@ public function testFlow()
7886
$files = $this->module->grabFileList();
7987
self::assertNotContains('test_ftp_123.txt', $files);
8088

81-
$this->module->amInPath('/');
89+
$this->module->amInPath($this->root);
8290

83-
self::assertEquals('/', $this->module->grabDirectory());
91+
self::assertEquals($this->root, $this->module->grabDirectory());
8492

8593
$this->module->renameDir('TESTING', 'TESTING_NEW');
8694

@@ -90,7 +98,7 @@ public function testFlow()
9098
$this->module->makeDir('TESTING');
9199
$this->module->amInPath('TESTING');
92100
$this->module->writeToFile('test_ftp_123.txt', 'some data added here');
93-
$this->module->amInPath('/');
101+
$this->module->amInPath($this->root);
94102
self::assertGreaterThan(0, $this->module->grabFileCount('TESTING'));
95103
$this->module->cleanDir('TESTING');
96104
self::assertEquals(0, $this->module->grabFileCount('TESTING'));

0 commit comments

Comments
 (0)