Merge pull request #29605 from nextcloud/bugfix/29603/fix-scanner-nfd-existenz

Normalize file name before existence check in scanner

9 files changed, 129 insertions, 35 deletions

diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php
index 6a8697a5eaf..ff96fbf2dab 100644
--- a/apps/files/lib/Command/Scan.php
+++ b/apps/files/lib/Command/Scan.php
@@ -105,15 +105,6 @@ class Scan extends Base {
 			);
 	}
 
-	public function checkScanWarning($fullPath, OutputInterface $output) {
-		$normalizedPath = basename(\OC\Files\Filesystem::normalizePath($fullPath));
-		$path = basename($fullPath);
-
-		if ($normalizedPath !== $path) {
-			$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
-		}
-	}
-
 	protected function scanFiles($user, $path, OutputInterface $output, $backgroundScan = false, $recursive = true, $homeOnly = false) {
 		$connection = $this->reconnectToDatabase($output);
 		$scanner = new \OC\Files\Utils\Scanner(
@@ -141,12 +132,8 @@ class Scan extends Base {
 			$output->writeln('Error while scanning, storage not available (' . $e->getMessage() . ')', OutputInterface::VERBOSITY_VERBOSE);
 		});
 
-		$scanner->listen('\OC\Files\Utils\Scanner', 'scanFile', function ($path) use ($output) {
-			$this->checkScanWarning($path, $output);
-		});
-
-		$scanner->listen('\OC\Files\Utils\Scanner', 'scanFolder', function ($path) use ($output) {
-			$this->checkScanWarning($path, $output);
+		$scanner->listen('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', function ($fullPath) use ($output) {
+			$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
 		});
 
 		try {
diff --git a/apps/files/lib/Command/ScanAppData.php b/apps/files/lib/Command/ScanAppData.php
index 09153643727..59281b52bc4 100644
--- a/apps/files/lib/Command/ScanAppData.php
+++ b/apps/files/lib/Command/ScanAppData.php
@@ -73,15 +73,6 @@ class ScanAppData extends Base {
 		$this->addArgument('folder', InputArgument::OPTIONAL, 'The appdata subfolder to scan', '');
 	}
 
-	public function checkScanWarning($fullPath, OutputInterface $output) {
-		$normalizedPath = basename(\OC\Files\Filesystem::normalizePath($fullPath));
-		$path = basename($fullPath);
-
-		if ($normalizedPath !== $path) {
-			$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
-		}
-	}
-
 	protected function scanFiles(OutputInterface $output, string $folder): int {
 		try {
 			$appData = $this->getAppDataFolder();
@@ -124,12 +115,8 @@ class ScanAppData extends Base {
 			$output->writeln('Error while scanning, storage not available (' . $e->getMessage() . ')', OutputInterface::VERBOSITY_VERBOSE);
 		});
 
-		$scanner->listen('\OC\Files\Utils\Scanner', 'scanFile', function ($path) use ($output) {
-			$this->checkScanWarning($path, $output);
-		});
-
-		$scanner->listen('\OC\Files\Utils\Scanner', 'scanFolder', function ($path) use ($output) {
-			$this->checkScanWarning($path, $output);
+		$scanner->listen('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', function ($fullPath) use ($output) {
+			$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
 		});
 
 		try {
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index c85b22972dc..09d5159369e 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -1161,6 +1161,7 @@ return array(
     'OC\\Files\\Storage\\Temporary' => $baseDir . '/lib/private/Files/Storage/Temporary.php',
     'OC\\Files\\Storage\\Wrapper\\Availability' => $baseDir . '/lib/private/Files/Storage/Wrapper/Availability.php',
     'OC\\Files\\Storage\\Wrapper\\Encoding' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encoding.php',
+    'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => $baseDir . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
     'OC\\Files\\Storage\\Wrapper\\Encryption' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encryption.php',
     'OC\\Files\\Storage\\Wrapper\\Jail' => $baseDir . '/lib/private/Files/Storage/Wrapper/Jail.php',
     'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => $baseDir . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index c3e5e8d5dd1..932de0b7858 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -1190,6 +1190,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Files\\Storage\\Temporary' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Temporary.php',
         'OC\\Files\\Storage\\Wrapper\\Availability' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Availability.php',
         'OC\\Files\\Storage\\Wrapper\\Encoding' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encoding.php',
+        'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
         'OC\\Files\\Storage\\Wrapper\\Encryption' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encryption.php',
         'OC\\Files\\Storage\\Wrapper\\Jail' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Jail.php',
         'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php',
diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php
index 8baab8746fc..bdefca01f6f 100644
--- a/lib/private/Files/Cache/Scanner.php
+++ b/lib/private/Files/Cache/Scanner.php
@@ -37,6 +37,7 @@ namespace OC\Files\Cache;
 
 use Doctrine\DBAL\Exception;
 use OC\Files\Filesystem;
+use OC\Files\Storage\Wrapper\Encoding;
 use OC\Hooks\BasicEmitter;
 use OCP\Files\Cache\IScanner;
 use OCP\Files\ForbiddenException;
@@ -419,7 +420,16 @@ class Scanner extends BasicEmitter implements IScanner {
 			if ($permissions === 0) {
 				continue;
 			}
-			$file = $fileMeta['name'];
+			$originalFile = $fileMeta['name'];
+			$file = trim(\OC\Files\Filesystem::normalizePath($originalFile), '/');
+			if (trim($originalFile, '/') !== $file) {
+				// encoding mismatch, might require compatibility wrapper
+				\OC::$server->getLogger()->debug('Scanner: Skipping non-normalized file name "'. $originalFile . '" in path "' . $path . '".', ['app' => 'core']);
+				$this->emit('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', [$path ? $path . '/' . $originalFile : $originalFile]);
+				// skip this entry
+				continue;
+			}
+
 			$newChildNames[] = $file;
 			$child = $path ? $path . '/' . $file : $file;
 			try {
diff --git a/lib/private/Files/Storage/Wrapper/Encoding.php b/lib/private/Files/Storage/Wrapper/Encoding.php
index ac27697e68c..d6201dc8877 100644
--- a/lib/private/Files/Storage/Wrapper/Encoding.php
+++ b/lib/private/Files/Storage/Wrapper/Encoding.php
@@ -29,6 +29,7 @@
 namespace OC\Files\Storage\Wrapper;
 
 use OC\Cache\CappedMemoryCache;
+use OC\Files\Filesystem;
 use OCP\Files\Storage\IStorage;
 use OCP\ICache;
 
@@ -162,7 +163,8 @@ class Encoding extends Wrapper {
 	 * @return resource|bool
 	 */
 	public function opendir($path) {
-		return $this->storage->opendir($this->findPathToUse($path));
+		$handle = $this->storage->opendir($this->findPathToUse($path));
+		return EncodingDirectoryWrapper::wrap($handle);
 	}
 
 	/**
@@ -532,10 +534,16 @@ class Encoding extends Wrapper {
 	}
 
 	public function getMetaData($path) {
-		return $this->storage->getMetaData($this->findPathToUse($path));
+		$entry = $this->storage->getMetaData($this->findPathToUse($path));
+		$entry['name'] = trim(Filesystem::normalizePath($entry['name']), '/');
+		return $entry;
 	}
 
 	public function getDirectoryContent($directory): \Traversable {
-		return $this->storage->getDirectoryContent($this->findPathToUse($directory));
+		$entries = $this->storage->getDirectoryContent($this->findPathToUse($directory));
+		foreach ($entries as $entry) {
+			$entry['name'] = trim(Filesystem::normalizePath($entry['name']), '/');
+			yield $entry;
+		}
 	}
 }
diff --git a/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php b/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php
new file mode 100644
index 00000000000..935a15af4cf
--- /dev/null
+++ b/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php
@@ -0,0 +1,55 @@
+<?php
+/**
+ * @copyright Copyright (c) 2021, Nextcloud GmbH.
+ *
+ * @author Robin Appelman <robin@icewind.nl>
+ * @author Vincent Petry <vincent@nextcloud.com>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Files\Storage\Wrapper;
+
+use Icewind\Streams\DirectoryWrapper;
+use OC\Files\Filesystem;
+
+/**
+ * Normalize file names while reading directory entries
+ */
+class EncodingDirectoryWrapper extends DirectoryWrapper {
+	/**
+	 * @return string
+	 */
+	public function dir_readdir() {
+		$file = readdir($this->source);
+		if ($file !== false && $file !== '.' && $file !== '..') {
+			$file = trim(Filesystem::normalizePath($file), '/');
+		}
+
+		return $file;
+	}
+
+	/**
+	 * @param resource $source
+	 * @param callable $filter
+	 * @return resource|bool
+	 */
+	public static function wrap($source) {
+		return self::wrapSource($source, [
+			'source' => $source,
+		]);
+	}
+}
diff --git a/lib/private/Files/Utils/Scanner.php b/lib/private/Files/Utils/Scanner.php
index 72a7084f40d..faeb31db8cc 100644
--- a/lib/private/Files/Utils/Scanner.php
+++ b/lib/private/Files/Utils/Scanner.php
@@ -145,6 +145,9 @@ class Scanner extends PublicEmitter {
 			$this->emit('\OC\Files\Utils\Scanner', 'postScanFolder', [$mount->getMountPoint() . $path]);
 			$this->dispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
 		});
+		$scanner->listen('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', function ($path) use ($mount) {
+			$this->emit('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', [$path]);
+		});
 	}
 
 	/**
diff --git a/tests/lib/Files/Storage/Wrapper/EncodingTest.php b/tests/lib/Files/Storage/Wrapper/EncodingTest.php
index 498d9f78248..0901edf83fa 100644
--- a/tests/lib/Files/Storage/Wrapper/EncodingTest.php
+++ b/tests/lib/Files/Storage/Wrapper/EncodingTest.php
@@ -32,7 +32,7 @@ class EncodingTest extends \Test\Files\Storage\Storage {
 
 	public function directoryProvider() {
 		$a = parent::directoryProvider();
-		$a[] = [self::NFD_NAME];
+		$a[] = [self::NFC_NAME];
 		return $a;
 	}
 
@@ -199,4 +199,46 @@ class EncodingTest extends \Test\Files\Storage\Storage {
 
 		$this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME . '2/test2.txt'));
 	}
+
+	public function testNormalizedDirectoryEntriesOpenDir() {
+		$this->sourceStorage->mkdir('/test');
+		$this->sourceStorage->mkdir('/test/' . self::NFD_NAME);
+
+		$this->assertTrue($this->instance->file_exists('/test/' . self::NFC_NAME));
+		$this->assertTrue($this->instance->file_exists('/test/' . self::NFD_NAME));
+
+		$dh = $this->instance->opendir('/test');
+		$content = [];
+		while ($file = readdir($dh)) {
+			if ($file != '.' and $file != '..') {
+				$content[] = $file;
+			}
+		}
+
+		$this->assertCount(1, $content);
+		$this->assertEquals(self::NFC_NAME, $content[0]);
+	}
+
+	public function testNormalizedDirectoryEntriesGetDirectoryContent() {
+		$this->sourceStorage->mkdir('/test');
+		$this->sourceStorage->mkdir('/test/' . self::NFD_NAME);
+
+		$this->assertTrue($this->instance->file_exists('/test/' . self::NFC_NAME));
+		$this->assertTrue($this->instance->file_exists('/test/' . self::NFD_NAME));
+
+		$content = iterator_to_array($this->instance->getDirectoryContent('/test'));
+		$this->assertCount(1, $content);
+		$this->assertEquals(self::NFC_NAME, $content[0]['name']);
+	}
+
+	public function testNormalizedGetMetaData() {
+		$this->sourceStorage->mkdir('/test');
+		$this->sourceStorage->mkdir('/test/' . self::NFD_NAME);
+
+		$entry = $this->instance->getMetaData('/test/' . self::NFC_NAME);
+		$this->assertEquals(self::NFC_NAME, $entry['name']);
+
+		$entry = $this->instance->getMetaData('/test/' . self::NFD_NAME);
+		$this->assertEquals(self::NFC_NAME, $entry['name']);
+	}
 }