Merge pull request #27194 from nextcloud/new-ftp

5 files changed, 652 insertions, 119 deletions

diff --git a/.github/workflows/ftp.yml b/.github/workflows/ftp.yml
new file mode 100644
index 00000000000..87e66badd77
--- /dev/null
+++ b/.github/workflows/ftp.yml
@@ -0,0 +1,78 @@
+name: FTP
+on:
+  push:
+    branches:
+      - master
+      - stable*
+    paths:
+      - 'apps/files_external/**'
+  pull_request:
+    paths:
+      - 'apps/files_external/**'
+
+env:
+  APP_NAME: files_external
+
+jobs:
+  ftp-tests:
+    runs-on: ubuntu-latest
+
+    strategy:
+      # do not stop on another job's failure
+      fail-fast: false
+      matrix:
+        php-versions: ['7.4', '8.0']
+        ftpd: ['proftpd', 'vsftpd', 'pure-ftpd']
+
+    name: php${{ matrix.php-versions }}-${{ matrix.ftpd }}
+
+    steps:
+      - name: Checkout server
+        uses: actions/checkout@v2
+        with:
+          submodules: true
+
+      - name: Set up ftpd
+        run: |
+          sudo mkdir /tmp/ftp
+          sudo chown -R 0777 /tmp/ftp
+          if [[ "${{ matrix.ftpd }}" == 'proftpd' ]]; then docker run --name ftp -d --net host -e FTP_USERNAME=test -e FTP_PASSWORD=test -v /tmp/ftp:/home/test hauptmedia/proftpd; fi
+          if [[ "${{ matrix.ftpd }}" == 'vsftpd' ]]; then docker run --name ftp -d --net host -e FTP_USER=test -e FTP_PASS=test -e PASV_ADDRESS=127.0.0.1 -v /tmp/ftp:/home/vsftpd/test fauria/vsftpd; fi
+          if [[ "${{ matrix.ftpd }}" == 'pure-ftpd' ]]; then docker run --name ftp -d --net host -e "PUBLICHOST=localhost" -e FTP_USER_NAME=test -e FTP_USER_PASS=test -e FTP_USER_HOME=/home/test -v /tmp/ftp2:/home/test -v /tmp/ftp2:/etc/pure-ftpd/passwd stilliard/pure-ftpd; fi
+      - name: Set up php ${{ matrix.php-versions }}
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-versions }}
+          tools: phpunit
+          extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd
+
+      - name: Set up Nextcloud
+        run: |
+          mkdir data
+          ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password
+          ./occ app:enable --force ${{ env.APP_NAME }}
+          php -S localhost:8080 &
+      - name: smoketest ftp
+        run: |
+          php -r 'var_dump(file_put_contents("ftp://test:test@localhost/ftp.txt", "asd"));'
+          php -r 'var_dump(file_get_contents("ftp://test:test@localhost/ftp.txt"));'
+          php -r 'var_dump(mkdir("ftp://test:test@localhost/asdads"));'
+          ls -l /tmp/ftp
+      - name: PHPUnit
+        run: |
+          echo "<?php return ['run' => true,'host' => 'localhost','user' => 'test','password' => 'test', 'root' => ''];" > apps/${{ env.APP_NAME }}/tests/config.ftp.php
+          phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/FtpTest.php
+      - name: ftpd logs
+        if: always()
+        run: |
+          docker logs ftp
+
+  ftp-summary:
+    runs-on: ubuntu-latest
+    needs: ftp-tests
+
+    if: always()
+
+    steps:
+      - name: Summary status
+        run: if ${{ needs.ftp-tests.result != 'success' }}; then exit 1; fi
diff --git a/apps/files_external/lib/Lib/Storage/FTP.php b/apps/files_external/lib/Lib/Storage/FTP.php
index 5beed27ed76..d424ffe3cdd 100644
--- a/apps/files_external/lib/Lib/Storage/FTP.php
+++ b/apps/files_external/lib/Lib/Storage/FTP.php
@@ -1,19 +1,8 @@
 <?php
 /**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @copyright Copyright (c) 2021 Robin Appelman <robin@icewind.nl>
  *
- * @author Björn Schießle <bjoern@schiessle.org>
- * @author Christoph Wurst <christoph@winzerhof-wurst.at>
- * @author Felix Moeller <mail@felixmoeller.de>
- * @author Jörn Friedrich Dreyer <jfd@butonic.de>
- * @author Michael Gapczynski <GapczynskiM@gmail.com>
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Philipp Kapfer <philipp.kapfer@gmx.at>
  * @author Robin Appelman <robin@icewind.nl>
- * @author Robin McCorkell <robin@mccorkell.me.uk>
- * @author Roeland Jago Douma <roeland@famdouma.nl>
- * @author Thomas Müller <thomas.mueller@tmit.eu>
- * @author Vincent Petry <vincent@nextcloud.com>
  *
  * @license AGPL-3.0
  *
@@ -27,143 +16,362 @@
  * GNU Affero General Public License for more details.
  *
  * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program. If not, see <http://www.gnu.org/licenses/>
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
 namespace OCA\Files_External\Lib\Storage;
 
 use Icewind\Streams\CallbackWrapper;
+use Icewind\Streams\CountWrapper;
 use Icewind\Streams\IteratorDirectory;
-use Icewind\Streams\RetryWrapper;
+use OC\Files\Storage\Common;
+use OC\Files\Storage\PolyFill\CopyDirectory;
+use OCP\Constants;
+use OCP\Files\FileInfo;
+use OCP\Files\StorageNotAvailableException;
 
-class FTP extends StreamWrapper {
-	private $password;
-	private $user;
+class FTP extends Common {
+	use CopyDirectory;
+
+	private $root;
 	private $host;
+	private $password;
+	private $username;
 	private $secure;
-	private $root;
+	private $port;
+	private $utf8Mode;
+
+	/** @var FtpConnection|null */
+	private $connection;
 
 	public function __construct($params) {
 		if (isset($params['host']) && isset($params['user']) && isset($params['password'])) {
 			$this->host = $params['host'];
-			$this->user = $params['user'];
+			$this->username = $params['user'];
 			$this->password = $params['password'];
 			if (isset($params['secure'])) {
-				$this->secure = $params['secure'];
+				if (is_string($params['secure'])) {
+					$this->secure = ($params['secure'] === 'true');
+				} else {
+					$this->secure = (bool)$params['secure'];
+				}
 			} else {
 				$this->secure = false;
 			}
-			$this->root = isset($params['root'])?$params['root']:'/';
-			if (! $this->root || $this->root[0] !== '/') {
-				$this->root = '/'.$this->root;
+			$this->root = isset($params['root']) ? '/' . ltrim($params['root']) : '/';
+			$this->port = $params['port'] ?? 21;
+			$this->utf8Mode = isset($params['utf8']) && $params['utf8'];
+		} else {
+			throw new \Exception('Creating ' . self::class . ' storage failed, required parameters not set');
+		}
+	}
+
+	public function __destruct() {
+		$this->connection = null;
+	}
+
+	protected function getConnection(): FtpConnection {
+		if (!$this->connection) {
+			try {
+				$this->connection = new FtpConnection(
+					$this->secure,
+					$this->host,
+					$this->port,
+					$this->username,
+					$this->password
+				);
+			} catch (\Exception $e) {
+				throw new StorageNotAvailableException("Failed to create ftp connection", 0, $e);
 			}
-			if (substr($this->root, -1) !== '/') {
-				$this->root .= '/';
+			if ($this->utf8Mode) {
+				if (!$this->connection->setUtf8Mode()) {
+					throw new StorageNotAvailableException("Could not set UTF-8 mode");
+				}
 			}
-		} else {
-			throw new \Exception('Creating FTP storage failed');
 		}
+
+		return $this->connection;
 	}
 
 	public function getId() {
-		return 'ftp::' . $this->user . '@' . $this->host . '/' . $this->root;
+		return 'ftp::' . $this->username . '@' . $this->host . '/' . $this->root;
 	}
 
-	/**
-	 * construct the ftp url
-	 * @param string $path
-	 * @return string
-	 */
-	public function constructUrl($path) {
-		$url = 'ftp';
-		if ($this->secure) {
-			$url .= 's';
+	protected function buildPath($path) {
+		return rtrim($this->root . '/' . $path, '/');
+	}
+
+	public static function checkDependencies() {
+		if (function_exists('ftp_login')) {
+			return true;
+		} else {
+			return ['ftp'];
+		}
+	}
+
+	public function filemtime($path) {
+		$result = $this->getConnection()->mdtm($this->buildPath($path));
+
+		if ($result === -1) {
+			if ($this->is_dir($path)) {
+				$list = $this->getConnection()->mlsd($this->buildPath($path));
+				if (!$list) {
+					\OC::$server->getLogger()->warning("Unable to get last modified date for ftp folder ($path), failed to list folder contents");
+					return time();
+				}
+				$currentDir = current(array_filter($list, function ($item) {
+					return $item['type'] === 'cdir';
+				}));
+				if ($currentDir) {
+					$time = \DateTime::createFromFormat('YmdHis', $currentDir['modify']);
+					if ($time === false) {
+						throw new \Exception("Invalid date format for directory: $currentDir");
+					}
+					return $time->getTimestamp();
+				} else {
+					\OC::$server->getLogger()->warning("Unable to get last modified date for ftp folder ($path), folder contents doesn't include current folder");
+					return time();
+				}
+			} else {
+				return false;
+			}
+		} else {
+			return $result;
+		}
+	}
+
+	public function filesize($path) {
+		$result = $this->getConnection()->size($this->buildPath($path));
+		if ($result === -1) {
+			return false;
+		} else {
+			return $result;
+		}
+	}
+
+	public function rmdir($path) {
+		if ($this->is_dir($path)) {
+			$result = $this->getConnection()->rmdir($this->buildPath($path));
+			// recursive rmdir support depends on the ftp server
+			if ($result) {
+				return $result;
+			} else {
+				return $this->recursiveRmDir($path);
+			}
+		} elseif ($this->is_file($path)) {
+			return $this->unlink($path);
+		} else {
+			return false;
 		}
-		$url .= '://'.urlencode($this->user).':'.urlencode($this->password).'@'.$this->host.$this->root.$path;
-		return $url;
 	}
 
 	/**
-	 * Unlinks file or directory
 	 * @param string $path
+	 * @return bool
 	 */
+	private function recursiveRmDir($path): bool {
+		$contents = $this->getDirectoryContent($path);
+		$result = true;
+		foreach ($contents as $content) {
+			if ($content['mimetype'] === FileInfo::MIMETYPE_FOLDER) {
+				$result = $result && $this->recursiveRmDir($path . '/' . $content['name']);
+			} else {
+				$result = $result && $this->getConnection()->delete($this->buildPath($path . '/' . $content['name']));
+			}
+		}
+		$result = $result && $this->getConnection()->rmdir($this->buildPath($path));
+
+		return $result;
+	}
+
+	public function test() {
+		try {
+			return $this->getConnection()->systype() !== false;
+		} catch (\Exception $e) {
+			return false;
+		}
+	}
+
+	public function stat($path) {
+		if (!$this->file_exists($path)) {
+			return false;
+		}
+		return [
+			'mtime' => $this->filemtime($path),
+			'size' => $this->filesize($path),
+		];
+	}
+
+	public function file_exists($path) {
+		if ($path === '' || $path === '.' || $path === '/') {
+			return true;
+		}
+		return $this->filetype($path) !== false;
+	}
+
 	public function unlink($path) {
+		switch ($this->filetype($path)) {
+			case 'dir':
+				return $this->rmdir($path);
+			case 'file':
+				return $this->getConnection()->delete($this->buildPath($path));
+			default:
+				return false;
+		}
+	}
+
+	public function opendir($path) {
+		$files = $this->getConnection()->nlist($this->buildPath($path));
+		return IteratorDirectory::wrap($files);
+	}
+
+	public function mkdir($path) {
+		if ($this->is_dir($path)) {
+			return false;
+		}
+		return $this->getConnection()->mkdir($this->buildPath($path)) !== false;
+	}
+
+	public function is_dir($path) {
+		if ($path === "") {
+			return true;
+		}
+		if ($this->getConnection()->chdir($this->buildPath($path)) === true) {
+			$this->getConnection()->chdir('/');
+			return true;
+		} else {
+			return false;
+		}
+	}
+
+	public function is_file($path) {
+		return $this->filesize($path) !== false;
+	}
+
+	public function filetype($path) {
 		if ($this->is_dir($path)) {
-			return $this->rmdir($path);
+			return 'dir';
+		} elseif ($this->is_file($path)) {
+			return 'file';
 		} else {
-			$url = $this->constructUrl($path);
-			$result = unlink($url);
-			clearstatcache(true, $url);
-			return $result;
+			return false;
 		}
 	}
-	public function fopen($path,$mode) {
+
+	public function fopen($path, $mode) {
+		$useExisting = true;
 		switch ($mode) {
 			case 'r':
 			case 'rb':
+				return $this->readStream($path);
 			case 'w':
+			case 'w+':
 			case 'wb':
+			case 'wb+':
+				$useExisting = false;
+			// no break
 			case 'a':
 			case 'ab':
-				//these are supported by the wrapper
-				$context = stream_context_create(['ftp' => ['overwrite' => true]]);
-				$handle = fopen($this->constructUrl($path), $mode, false, $context);
-				return RetryWrapper::wrap($handle);
 			case 'r+':
-			case 'w+':
-			case 'wb+':
 			case 'a+':
 			case 'x':
 			case 'x+':
 			case 'c':
 			case 'c+':
 				//emulate these
-				if (strrpos($path, '.') !== false) {
-					$ext = substr($path, strrpos($path, '.'));
+				if ($useExisting and $this->file_exists($path)) {
+					if (!$this->isUpdatable($path)) {
+						return false;
+					}
+					$tmpFile = $this->getCachedFile($path);
 				} else {
-					$ext = '';
-				}
-				$tmpFile = \OC::$server->getTempManager()->getTemporaryFile();
-				if ($this->file_exists($path)) {
-					$this->getFile($path, $tmpFile);
+					if (!$this->isCreatable(dirname($path))) {
+						return false;
+					}
+					$tmpFile = \OC::$server->getTempManager()->getTemporaryFile();
 				}
-				$handle = fopen($tmpFile, $mode);
-				return CallbackWrapper::wrap($handle, null, null, function () use ($path, $tmpFile) {
-					$this->writeBack($tmpFile, $path);
+				$source = fopen($tmpFile, $mode);
+				return CallbackWrapper::wrap($source, null, null, function () use ($tmpFile, $path) {
+					$this->writeStream($path, fopen($tmpFile, 'r'));
+					unlink($tmpFile);
 				});
 		}
 		return false;
 	}
 
-	public function opendir($path) {
-		$dh = parent::opendir($path);
-		if (is_resource($dh)) {
-			$files = [];
-			while (($file = readdir($dh)) !== false) {
-				if ($file != '.' && $file != '..' && strpos($file, '#') === false) {
-					$files[] = $file;
-				}
-			}
-			return IteratorDirectory::wrap($files);
-		} else {
-			return false;
+	public function writeStream(string $path, $stream, int $size = null): int {
+		if ($size === null) {
+			$stream = CountWrapper::wrap($stream, function ($writtenSize) use (&$size) {
+				$size = $writtenSize;
+			});
 		}
+
+		$this->getConnection()->fput($this->buildPath($path), $stream);
+		fclose($stream);
+
+		return $size;
 	}
 
+	public function readStream(string $path) {
+		$stream = fopen('php://temp', 'w+');
+		$result = $this->getConnection()->fget($stream, $this->buildPath($path));
+		rewind($stream);
 
-	public function writeBack($tmpFile, $path) {
-		$this->uploadFile($tmpFile, $path);
-		unlink($tmpFile);
+		if (!$result) {
+			fclose($stream);
+			return false;
+		}
+		return $stream;
 	}
 
-	/**
-	 * check if php-ftp is installed
-	 */
-	public static function checkDependencies() {
-		if (function_exists('ftp_login')) {
-			return true;
+	public function touch($path, $mtime = null) {
+		if ($this->file_exists($path)) {
+			return false;
 		} else {
-			return ['ftp'];
+			$this->file_put_contents($path, '');
+			return true;
+		}
+	}
+
+	public function rename($path1, $path2) {
+		$this->unlink($path2);
+		return $this->getConnection()->rename($this->buildPath($path1), $this->buildPath($path2));
+	}
+
+	public function getDirectoryContent($directory): \Traversable {
+		$files = $this->getConnection()->mlsd($this->buildPath($directory));
+		$mimeTypeDetector = \OC::$server->getMimeTypeDetector();
+
+		foreach ($files as $file) {
+			$name = $file['name'];
+			if ($file['type'] === 'cdir' || $file['type'] === 'pdir') {
+				continue;
+			}
+			$permissions = Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE;
+			$isDir = $file['type'] === 'dir';
+			if ($isDir) {
+				$permissions += Constants::PERMISSION_CREATE;
+			}
+
+			$data = [];
+			$data['mimetype'] = $isDir ? FileInfo::MIMETYPE_FOLDER : $mimeTypeDetector->detectPath($name);
+			$data['mtime'] = \DateTime::createFromFormat('YmdGis', $file['modify'])->getTimestamp();
+			if ($data['mtime'] === false) {
+				$data['mtime'] = time();
+			}
+			if ($isDir) {
+				$data['size'] = -1; //unknown
+			} elseif (isset($file['size'])) {
+				$data['size'] = $file['size'];
+			} else {
+				$data['size'] = $this->filesize($directory . '/' . $name);
+			}
+			$data['etag'] = uniqid();
+			$data['storage_mtime'] = $data['mtime'];
+			$data['permissions'] = $permissions;
+			$data['name'] = $name;
+
+			yield $data;
 		}
 	}
 }
diff --git a/apps/files_external/lib/Lib/Storage/FtpConnection.php b/apps/files_external/lib/Lib/Storage/FtpConnection.php
new file mode 100644
index 00000000000..d87c44656f4
--- /dev/null
+++ b/apps/files_external/lib/Lib/Storage/FtpConnection.php
@@ -0,0 +1,234 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020 Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Files_External\Lib\Storage;
+
+/**
+ * Low level wrapper around the ftp functions that smooths over some difference between servers
+ */
+class FtpConnection {
+	/** @var resource */
+	private $connection;
+
+	public function __construct(bool $secure, string $hostname, int $port, string $username, string $password) {
+		if ($secure) {
+			$connection = ftp_ssl_connect($hostname, $port);
+		} else {
+			$connection = ftp_connect($hostname, $port);
+		}
+
+		if ($connection === false) {
+			throw new \Exception("Failed to connect to ftp");
+		}
+
+		if (ftp_login($connection, $username, $password) === false) {
+			throw new \Exception("Failed to connect to login to ftp");
+		}
+
+		ftp_pasv($connection, true);
+		$this->connection = $connection;
+	}
+
+	public function __destruct() {
+		if ($this->connection) {
+			ftp_close($this->connection);
+		}
+		$this->connection = null;
+	}
+
+	public function setUtf8Mode(): bool {
+		$response = ftp_raw($this->connection, "OPTS UTF8 ON");
+		return substr($response[0], 0, 3) === '200';
+	}
+
+	public function fput(string $path, $handle) {
+		return @ftp_fput($this->connection, $path, $handle, FTP_BINARY);
+	}
+
+	public function fget($handle, string $path) {
+		return @ftp_fget($this->connection, $handle, $path, FTP_BINARY);
+	}
+
+	public function mkdir(string $path) {
+		return @ftp_mkdir($this->connection, $path);
+	}
+
+	public function chdir(string $path) {
+		return @ftp_chdir($this->connection, $path);
+	}
+
+	public function delete(string $path) {
+		return @ftp_delete($this->connection, $path);
+	}
+
+	public function rmdir(string $path) {
+		return @ftp_rmdir($this->connection, $path);
+	}
+
+	public function rename(string $path1, string $path2) {
+		return @ftp_rename($this->connection, $path1, $path2);
+	}
+
+	public function mdtm(string $path) {
+		return @ftp_mdtm($this->connection, $path);
+	}
+
+	public function size(string $path) {
+		return @ftp_size($this->connection, $path);
+	}
+
+	public function systype() {
+		return @ftp_systype($this->connection);
+	}
+
+	public function nlist(string $path) {
+		$files = @ftp_nlist($this->connection, $path);
+		return array_map(function ($name) {
+			if (strpos($name, '/') !== false) {
+				$name = basename($name);
+			}
+			return $name;
+		}, $files);
+	}
+
+	public function mlsd(string $path) {
+		$files = @ftp_mlsd($this->connection, $path);
+
+		if ($files !== false) {
+			return array_map(function ($file) {
+				if (strpos($file['name'], '/') !== false) {
+					$file['name'] = basename($file['name']);
+				}
+				return $file;
+			}, $files);
+		} else {
+			// not all servers support mlsd, in those cases we parse the raw list ourselves
+			$rawList = @ftp_rawlist($this->connection, '-aln ' . $path);
+			if ($rawList === false) {
+				return false;
+			}
+			return $this->parseRawList($rawList, $path);
+		}
+	}
+
+	// rawlist parsing logic is based on the ftp implementation from https://github.com/thephpleague/flysystem
+	private function parseRawList(array $rawList, string $directory): array {
+		return array_map(function ($item) use ($directory) {
+			return $this->parseRawListItem($item, $directory);
+		}, $rawList);
+	}
+
+	private function parseRawListItem(string $item, string $directory): array {
+		$isWindows = preg_match('/^[0-9]{2,4}-[0-9]{2}-[0-9]{2}/', $item);
+
+		return $isWindows ? $this->parseWindowsItem($item, $directory) : $this->parseUnixItem($item, $directory);
+	}
+
+	private function parseUnixItem(string $item, string $directory): array {
+		$item = preg_replace('#\s+#', ' ', $item, 7);
+
+		if (count(explode(' ', $item, 9)) !== 9) {
+			throw new \RuntimeException("Metadata can't be parsed from item '$item' , not enough parts.");
+		}
+
+		[$permissions, /* $number */, /* $owner */, /* $group */, $size, $month, $day, $time, $name] = explode(' ', $item, 9);
+		if ($name === '.') {
+			$type = 'cdir';
+		} elseif ($name === '..') {
+			$type = 'pdir';
+		} else {
+			$type = substr($permissions, 0, 1) === 'd' ? 'dir' : 'file';
+		}
+
+		$parsedDate = (new \DateTime())
+			->setTimestamp(strtotime("$month $day $time"));
+		$tomorrow = (new \DateTime())->add(new \DateInterval("P1D"));
+
+		// since the provided date doesn't include the year, we either set it to the correct year
+		// or when the date would otherwise be in the future (by more then 1 day to account for timezone errors)
+		// we use last year
+		if ($parsedDate > $tomorrow) {
+			$parsedDate = $parsedDate->sub(new \DateInterval("P1Y"));
+		}
+
+		$formattedDate = $parsedDate
+			->format('YmdHis');
+
+		return [
+			'type' => $type,
+			'name' => $name,
+			'modify' => $formattedDate,
+			'perm' => $this->normalizePermissions($permissions),
+			'size' => (int)$size,
+		];
+	}
+
+	private function normalizePermissions(string $permissions) {
+		$isDir = substr($permissions, 0, 1) === 'd';
+		// remove the type identifier and only use owner permissions
+		$permissions = substr($permissions, 1, 4);
+
+		// map the string rights to the ftp counterparts
+		$filePermissionsMap = ['r' => 'r', 'w' => 'fadfw'];
+		$dirPermissionsMap = ['r' => 'e', 'w' => 'flcdmp'];
+
+		$map = $isDir ? $dirPermissionsMap : $filePermissionsMap;
+
+		return array_reduce(str_split($permissions), function ($ftpPermissions, $permission) use ($map) {
+			if (isset($map[$permission])) {
+				$ftpPermissions .= $map[$permission];
+			}
+			return $ftpPermissions;
+		}, '');
+	}
+
+	private function parseWindowsItem(string $item, string $directory): array {
+		$item = preg_replace('#\s+#', ' ', trim($item), 3);
+
+		if (count(explode(' ', $item, 4)) !== 4) {
+			throw new \RuntimeException("Metadata can't be parsed from item '$item' , not enough parts.");
+		}
+
+		[$date, $time, $size, $name] = explode(' ', $item, 4);
+
+		// Check for the correct date/time format
+		$format = strlen($date) === 8 ? 'm-d-yH:iA' : 'Y-m-dH:i';
+		$formattedDate = \DateTime::createFromFormat($format, $date . $time)->format('YmdGis');
+
+		if ($name === '.') {
+			$type = 'cdir';
+		} elseif ($name === '..') {
+			$type = 'pdir';
+		} else {
+			$type = ($size === '<DIR>') ? 'dir' : 'file';
+		}
+
+		return [
+			'type' => $type,
+			'name' => $name,
+			'modify' => $formattedDate,
+			'perm' => ($type === 'file') ? 'adfrw' : 'flcdmpe',
+			'size' => (int)$size,
+		];
+	}
+}
diff --git a/apps/files_external/tests/Storage/FtpTest.php b/apps/files_external/tests/Storage/FtpTest.php
index 507490204e3..9ca06f15112 100644
--- a/apps/files_external/tests/Storage/FtpTest.php
+++ b/apps/files_external/tests/Storage/FtpTest.php
@@ -48,50 +48,63 @@ class FtpTest extends \Test\Files\Storage\Storage {
 		if (! is_array($this->config) or ! $this->config['run']) {
 			$this->markTestSkipped('FTP backend not configured');
 		}
+		$rootInstance = new FTP($this->config);
+		$rootInstance->mkdir($id);
+
 		$this->config['root'] .= '/' . $id; //make sure we have an new empty folder to work in
 		$this->instance = new FTP($this->config);
-		$this->instance->mkdir('/');
 	}
 
 	protected function tearDown(): void {
 		if ($this->instance) {
-			\OCP\Files::rmdirr($this->instance->constructUrl(''));
+			$this->instance->rmdir('');
 		}
+		$this->instance = null;
 
 		parent::tearDown();
 	}
 
-	public function testConstructUrl() {
-		$config = [ 'host' => 'localhost',
-			'user' => 'ftp',
-			'password' => 'ftp',
-			'root' => '/',
-			'secure' => false ];
-		$instance = new FTP($config);
-		$this->assertEquals('ftp://ftp:ftp@localhost/', $instance->constructUrl(''));
+	/**
+	 * ftp has no proper way to handle spaces at the end of file names
+	 */
+	public function directoryProvider() {
+		return array_filter(parent::directoryProvider(), function ($item) {
+			return substr($item[0], -1) !== ' ';
+		});
+	}
+
 
-		$config['secure'] = true;
-		$instance = new FTP($config);
-		$this->assertEquals('ftps://ftp:ftp@localhost/', $instance->constructUrl(''));
+	/**
+	 * mtime for folders is only with a minute resolution
+	 */
+	public function testStat() {
+		$textFile = \OC::$SERVERROOT . '/tests/data/lorem.txt';
+		$ctimeStart = time();
+		$this->instance->file_put_contents('/lorem.txt', file_get_contents($textFile));
+		$this->assertTrue($this->instance->isReadable('/lorem.txt'));
+		$ctimeEnd = time();
+		$mTime = $this->instance->filemtime('/lorem.txt');
+		$this->assertTrue($this->instance->hasUpdated('/lorem.txt', $ctimeStart - 5));
+		$this->assertTrue($this->instance->hasUpdated('/', $ctimeStart - 61));
 
-		$config['secure'] = 'false';
-		$instance = new FTP($config);
-		$this->assertEquals('ftp://ftp:ftp@localhost/', $instance->constructUrl(''));
+		// check that ($ctimeStart - 5) <= $mTime <= ($ctimeEnd + 1)
+		$this->assertGreaterThanOrEqual(($ctimeStart - 5), $mTime);
+		$this->assertLessThanOrEqual(($ctimeEnd + 1), $mTime);
+		$this->assertEquals(filesize($textFile), $this->instance->filesize('/lorem.txt'));
 
-		$config['secure'] = 'true';
-		$instance = new FTP($config);
-		$this->assertEquals('ftps://ftp:ftp@localhost/', $instance->constructUrl(''));
+		$stat = $this->instance->stat('/lorem.txt');
+		//only size and mtime are required in the result
+		$this->assertEquals($stat['size'], $this->instance->filesize('/lorem.txt'));
+		$this->assertEquals($stat['mtime'], $mTime);
 
-		$config['root'] = '';
-		$instance = new FTP($config);
-		$this->assertEquals('ftps://ftp:ftp@localhost/somefile.txt', $instance->constructUrl('somefile.txt'));
+		if ($this->instance->touch('/lorem.txt', 100) !== false) {
+			$mTime = $this->instance->filemtime('/lorem.txt');
+			$this->assertEquals($mTime, 100);
+		}
 
-		$config['root'] = '/abc';
-		$instance = new FTP($config);
-		$this->assertEquals('ftps://ftp:ftp@localhost/abc/somefile.txt', $instance->constructUrl('somefile.txt'));
+		$mtimeStart = time();
 
-		$config['root'] = '/abc/';
-		$instance = new FTP($config);
-		$this->assertEquals('ftps://ftp:ftp@localhost/abc/somefile.txt', $instance->constructUrl('somefile.txt'));
+		$this->instance->unlink('/lorem.txt');
+		$this->assertTrue($this->instance->hasUpdated('/', $mtimeStart - 61));
 	}
 }
diff --git a/lib/private/Files/Storage/PolyFill/CopyDirectory.php b/lib/private/Files/Storage/PolyFill/CopyDirectory.php
index ff05eecb134..7fd418f6dca 100644
--- a/lib/private/Files/Storage/PolyFill/CopyDirectory.php
+++ b/lib/private/Files/Storage/PolyFill/CopyDirectory.php
@@ -64,15 +64,15 @@ trait CopyDirectory {
 	 */
 	abstract public function mkdir($path);
 
-	public function copy($source, $target) {
-		if ($this->is_dir($source)) {
-			if ($this->file_exists($target)) {
-				$this->unlink($target);
+	public function copy($path1, $path2) {
+		if ($this->is_dir($path1)) {
+			if ($this->file_exists($path2)) {
+				$this->unlink($path2);
 			}
-			$this->mkdir($target);
-			return $this->copyRecursive($source, $target);
+			$this->mkdir($path2);
+			return $this->copyRecursive($path1, $path2);
 		} else {
-			return parent::copy($source, $target);
+			return parent::copy($path1, $path2);
 		}
 	}