Skip to content

Commit 3032cf4

Browse files
bahman026CarlSchwan
authored andcommitted
fix(files_external): handle empty FTP port parameter
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 on 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 of 21 unless the configured value is numeric, mirroring the guard added for SFTP in #58350. Casting without the check would not work, because (int)"" is 0 rather than 21. Co-authored-by: Carl Schwan <carl@carlschwan.eu> Signed-off-by: bahman026 <bahman026@gmail.com>
1 parent 12a45b3 commit 3032cf4

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

apps/files_external/lib/Lib/Storage/FTP.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ public function __construct(array $parameters) {
4848
$this->secure = false;
4949
}
5050
$this->root = isset($parameters['root']) ? '/' . ltrim($parameters['root']) : '/';
51-
$this->port = $parameters['port'] ?? 21;
51+
$parsedPort = $parameters['port'] ?? null;
52+
$this->port = is_numeric($parsedPort) ? (int)$parsedPort : 21;
5253
$this->utf8Mode = isset($parameters['utf8']) && $parameters['utf8'];
5354
} else {
5455
throw new \Exception('Creating ' . self::class . ' storage failed, required parameters not set');
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Files_External\Tests;
11+
12+
use OCA\Files_External\Lib\Storage\FTP;
13+
use PHPUnit\Framework\Attributes\DataProvider;
14+
use Test\TestCase;
15+
16+
class FtpTest extends TestCase {
17+
public static function portProvider(): array {
18+
$parameters = [
19+
'host' => 'somehost',
20+
'user' => 'someuser',
21+
'password' => 'somepassword',
22+
];
23+
24+
return [
25+
'no port given' => [$parameters, 21],
26+
'empty port' => [array_merge($parameters, ['port' => '']), 21],
27+
'null port' => [array_merge($parameters, ['port' => null]), 21],
28+
'non numeric port' => [array_merge($parameters, ['port' => 'ftp']), 21],
29+
'numeric string port' => [array_merge($parameters, ['port' => '2121']), 2121],
30+
'integer port' => [array_merge($parameters, ['port' => 2121]), 2121],
31+
];
32+
}
33+
34+
#[DataProvider('portProvider')]
35+
public function testPort(array $parameters, int $expectedPort): void {
36+
$instance = new FTP($parameters);
37+
38+
$this->assertSame($expectedPort, self::invokePrivate($instance, 'port'));
39+
}
40+
}

0 commit comments

Comments
 (0)