From 1b0329d78d34729224a011f971a8d0851fcc58f5 Mon Sep 17 00:00:00 2001 From: Bahman Jafarzadeh Date: Tue, 11 Aug 2026 14:38:10 +0330 Subject: [PATCH 1/3] fix(ftp): handle empty port parameter and ensure strictly numeric ports The external storage settings submit an empty string when the Port field is left blank. `??` only substitutes null or missing values, so `""` was assigned to $port and passed to FtpConnection::__construct(), whose $port parameter is typed int. PHP rejects a non-numeric string there, so adding an FTP storage without a port ended in a TypeError and an HTTP 500 - after the configuration had already been saved. Fall back to the default port unless the configured value is numeric, mirroring the guard added for SFTP in #58350. Signed-off-by: Bahman Jafarzadeh --- apps/files_external/lib/Lib/Storage/FTP.php | 3 +- .../tests/FtpConstructorTest.php | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 apps/files_external/tests/FtpConstructorTest.php diff --git a/apps/files_external/lib/Lib/Storage/FTP.php b/apps/files_external/lib/Lib/Storage/FTP.php index 2761f0123b97a..66e623e5a1b2c 100644 --- a/apps/files_external/lib/Lib/Storage/FTP.php +++ b/apps/files_external/lib/Lib/Storage/FTP.php @@ -49,7 +49,8 @@ public function __construct(array $parameters) { $this->secure = false; } $this->root = isset($parameters['root']) ? '/' . ltrim($parameters['root']) : '/'; - $this->port = $parameters['port'] ?? 21; + $parsedPort = $parameters['port'] ?? null; + $this->port = is_numeric($parsedPort) ? (int)$parsedPort : 21; $this->utf8Mode = isset($parameters['utf8']) && $parameters['utf8']; } else { throw new \Exception('Creating ' . self::class . ' storage failed, required parameters not set'); diff --git a/apps/files_external/tests/FtpConstructorTest.php b/apps/files_external/tests/FtpConstructorTest.php new file mode 100644 index 0000000000000..78c9d6b33e030 --- /dev/null +++ b/apps/files_external/tests/FtpConstructorTest.php @@ -0,0 +1,45 @@ + 'somehost', + 'user' => 'someuser', + 'password' => 'somepassword', + ]; + + return [ + 'no port given' => [$parameters, 21], + 'empty port' => [array_merge($parameters, ['port' => '']), 21], + 'null port' => [array_merge($parameters, ['port' => null]), 21], + 'non numeric port' => [array_merge($parameters, ['port' => 'ftp']), 21], + 'numeric string port' => [array_merge($parameters, ['port' => '2121']), 2121], + 'integer port' => [array_merge($parameters, ['port' => 2121]), 2121], + ]; + } + + #[DataProvider('portProvider')] + public function testPort(array $parameters, int $expectedPort): void { + $instance = new FTP($parameters); + + $this->assertSame($expectedPort, self::invokePrivate($instance, 'port')); + } +} From 046f875ef420c9eaccec4f2aee2bfad8c89887ef Mon Sep 17 00:00:00 2001 From: bahman <42313073+bahman026@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:00:05 +0330 Subject: [PATCH 2/3] Update apps/files_external/tests/FtpConstructorTest.php Co-authored-by: Carl Schwan Signed-off-by: bahman <42313073+bahman026@users.noreply.github.com> --- apps/files_external/tests/FtpConstructorTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/files_external/tests/FtpConstructorTest.php b/apps/files_external/tests/FtpConstructorTest.php index 78c9d6b33e030..f9b925af3bd1f 100644 --- a/apps/files_external/tests/FtpConstructorTest.php +++ b/apps/files_external/tests/FtpConstructorTest.php @@ -13,11 +13,6 @@ use PHPUnit\Framework\Attributes\DataProvider; use Test\TestCase; -/** - * The FTP connection is established lazily, so the constructor can be covered - * without a reachable FTP server. The backend tests in Storage/FtpTest.php do - * need one and are run separately. - */ class FtpConstructorTest extends TestCase { public static function portProvider(): array { $parameters = [ From f80378165fd3aad4bf977ec4d6578bc802e75f26 Mon Sep 17 00:00:00 2001 From: bahman026 Date: Wed, 12 Aug 2026 16:00:22 +0330 Subject: [PATCH 3/3] test(files_external): rename FTP test class to FtpTest Signed-off-by: bahman026 --- .../tests/{FtpConstructorTest.php => FtpTest.php} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename apps/files_external/tests/{FtpConstructorTest.php => FtpTest.php} (96%) diff --git a/apps/files_external/tests/FtpConstructorTest.php b/apps/files_external/tests/FtpTest.php similarity index 96% rename from apps/files_external/tests/FtpConstructorTest.php rename to apps/files_external/tests/FtpTest.php index f9b925af3bd1f..68e6bb723756f 100644 --- a/apps/files_external/tests/FtpConstructorTest.php +++ b/apps/files_external/tests/FtpTest.php @@ -13,7 +13,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use Test\TestCase; -class FtpConstructorTest extends TestCase { +class FtpTest extends TestCase { public static function portProvider(): array { $parameters = [ 'host' => 'somehost',