Fix SFTP storage id to be compatible with older ids

Remove port from SFTP storage id if it is 22.
This will prevent recreating a different storage entry due to id
mismatch after upgrade.

2 files changed, 66 insertions, 1 deletions

diff --git a/apps/files_external/lib/sftp.php b/apps/files_external/lib/sftp.php
index 392479c2f79..c457a87a6c7 100644
--- a/apps/files_external/lib/sftp.php
+++ b/apps/files_external/lib/sftp.php
@@ -133,7 +133,15 @@ class SFTP extends \OC\Files\Storage\Common {
 	 * {@inheritdoc}
 	 */
 	public function getId(){
-		return 'sftp::' . $this->user . '@' . $this->host . ':' . $this->port . '/' . $this->root;
+		$id = 'sftp::' . $this->user . '@' . $this->host;
+		if ($this->port !== 22) {
+			$id .= ':' . $this->port;
+		}
+		// note: this will double the root slash,
+		// we should not change it to keep compatible with
+		// old storage ids
+		$id .= '/' . $this->root;
+		return $id;
 	}
 
 	/**
diff --git a/apps/files_external/tests/backends/sftp.php b/apps/files_external/tests/backends/sftp.php
index e619fd7e13d..ae800326e58 100644
--- a/apps/files_external/tests/backends/sftp.php
+++ b/apps/files_external/tests/backends/sftp.php
@@ -47,4 +47,61 @@ class SFTP extends Storage {
 
 		parent::tearDown();
 	}
+
+	/**
+	 * @dataProvider configProvider
+	 */
+	public function testStorageId($config, $expectedStorageId) {
+		$instance = new \OC\Files\Storage\SFTP($config);
+		$this->assertEquals($expectedStorageId, $instance->getId());
+	}
+
+	function configProvider() {
+		return [
+			[
+				// no root path
+				[
+					'run' => true,
+					'host' => 'somehost',
+					'user' => 'someuser',
+					'password' => 'somepassword',
+					'root' => '',
+				],
+				'sftp::someuser@somehost//',
+			],
+			[
+				// without leading nor trailing slash
+				[
+					'run' => true,
+					'host' => 'somehost',
+					'user' => 'someuser',
+					'password' => 'somepassword',
+					'root' => 'remotedir/subdir',
+				],
+				'sftp::someuser@somehost//remotedir/subdir/',
+			],
+			[
+				// regular path
+				[
+					'run' => true,
+					'host' => 'somehost',
+					'user' => 'someuser',
+					'password' => 'somepassword',
+					'root' => '/remotedir/subdir/',
+				],
+				'sftp::someuser@somehost//remotedir/subdir/',
+			],
+			[
+				// different port
+				[
+					'run' => true,
+					'host' => 'somehost:8822',
+					'user' => 'someuser',
+					'password' => 'somepassword',
+					'root' => 'remotedir/subdir/',
+				],
+				'sftp::someuser@somehost:8822//remotedir/subdir/',
+			],
+		];
+	}
 }