From 787d7956cb3e559856d64cf9c21e748130d58c02 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 9 Nov 2021 11:56:10 +0100 Subject: [PATCH 1/3] Normalize file name before existence check in scanner The scanner would not find a NFD-encoded file name in an existing file list that is normalized. This normalizes the file name before scanning. Fixes issues where scanning repeatedly would make NFD files flicker in and out of existence in the file cache. Signed-off-by: Vincent Petry --- lib/private/Files/Cache/Scanner.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 8baab8746fcf5..a88d49d32c233 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -420,6 +420,7 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s continue; } $file = $fileMeta['name']; + $file = trim(\OC\Files\Filesystem::normalizePath($file), '/'); $newChildNames[] = $file; $child = $path ? $path . '/' . $file : $file; try { From 2b4badf768ec1a9cc9f246f3598a7396d7a52a26 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 10 Nov 2021 15:09:25 +0100 Subject: [PATCH 2/3] Move storage encoding compatibility warning logic The encoding check for file names is now happening the Scanner, and an event will be emitted only if the storage doesn't contain the encoding compatibility wrapper. The event is listened to by the occ scan command to be able to display a warning in case of file name mismatches when they have NFD encoding. Signed-off-by: Vincent Petry --- apps/files/lib/Command/Scan.php | 17 ++--------------- apps/files/lib/Command/ScanAppData.php | 17 ++--------------- lib/private/Files/Cache/Scanner.php | 10 ++++++++-- lib/private/Files/Utils/Scanner.php | 3 +++ 4 files changed, 15 insertions(+), 32 deletions(-) diff --git a/apps/files/lib/Command/Scan.php b/apps/files/lib/Command/Scan.php index 6a8697a5eaf2e..ff96fbf2dabf4 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -105,15 +105,6 @@ protected function configure() { ); } - public function checkScanWarning($fullPath, OutputInterface $output) { - $normalizedPath = basename(\OC\Files\Filesystem::normalizePath($fullPath)); - $path = basename($fullPath); - - if ($normalizedPath !== $path) { - $output->writeln("\tEntry \"" . $fullPath . '" will not be accessible due to incompatible encoding'); - } - } - 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 @@ protected function scanFiles($user, $path, OutputInterface $output, $backgroundS $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("\tEntry \"" . $fullPath . '" will not be accessible due to incompatible encoding'); }); try { diff --git a/apps/files/lib/Command/ScanAppData.php b/apps/files/lib/Command/ScanAppData.php index 0915364372740..59281b52bc4cd 100644 --- a/apps/files/lib/Command/ScanAppData.php +++ b/apps/files/lib/Command/ScanAppData.php @@ -73,15 +73,6 @@ protected function configure() { $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("\tEntry \"" . $fullPath . '" will not be accessible due to incompatible encoding'); - } - } - protected function scanFiles(OutputInterface $output, string $folder): int { try { $appData = $this->getAppDataFolder(); @@ -124,12 +115,8 @@ protected function scanFiles(OutputInterface $output, string $folder): int { $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("\tEntry \"" . $fullPath . '" will not be accessible due to incompatible encoding'); }); try { diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index a88d49d32c233..f31885d54a608 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -37,6 +37,7 @@ 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,8 +420,13 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s if ($permissions === 0) { continue; } - $file = $fileMeta['name']; - $file = trim(\OC\Files\Filesystem::normalizePath($file), '/'); + $originalFile = $fileMeta['name']; + $file = trim(\OC\Files\Filesystem::normalizePath($originalFile), '/'); + if (trim($originalFile, '/') !== $file && !$this->storage->instanceOfStorage(Encoding::class)) { + // encoding mismatch, might require compatibility wrapper + $this->emit('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', [$path ? $path . '/' . $originalFile : $originalFile]); + } + $newChildNames[] = $file; $child = $path ? $path . '/' . $file : $file; try { diff --git a/lib/private/Files/Utils/Scanner.php b/lib/private/Files/Utils/Scanner.php index 72a7084f40d3f..faeb31db8cc0d 100644 --- a/lib/private/Files/Utils/Scanner.php +++ b/lib/private/Files/Utils/Scanner.php @@ -145,6 +145,9 @@ protected function attachListener($mount) { $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]); + }); } /** From a721d346b928bc2926b738670031c5f5e004647b Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 17 Nov 2021 09:19:10 +0100 Subject: [PATCH 3/3] Normalize directory entries in Encoding wrapper Directory entry file names are now normalized in getMetaData(), getDirectoryContents() and opendir(). This makes the scanner work properly as it assumes pre-normalized names. In case the names were not normalized, the scanner will now skip the entries and display a warning when applicable. Signed-off-by: Vincent Petry --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Files/Cache/Scanner.php | 5 +- .../Files/Storage/Wrapper/Encoding.php | 14 ++++- .../Wrapper/EncodingDirectoryWrapper.php | 55 +++++++++++++++++++ .../Files/Storage/Wrapper/EncodingTest.php | 44 ++++++++++++++- 6 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0218fd441025a..2f55631cb9aea 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1142,6 +1142,7 @@ '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 17eb949609e67..10b579c962443 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1171,6 +1171,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 f31885d54a608..bdefca01f6fed 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -422,9 +422,12 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s } $originalFile = $fileMeta['name']; $file = trim(\OC\Files\Filesystem::normalizePath($originalFile), '/'); - if (trim($originalFile, '/') !== $file && !$this->storage->instanceOfStorage(Encoding::class)) { + 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; diff --git a/lib/private/Files/Storage/Wrapper/Encoding.php b/lib/private/Files/Storage/Wrapper/Encoding.php index ac27697e68c97..d6201dc88776c 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 @@ public function rmdir($path) { * @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 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t } 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 0000000000000..935a15af4cfbc --- /dev/null +++ b/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php @@ -0,0 +1,55 @@ + + * @author Vincent Petry + * + * @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 + * + */ + +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/tests/lib/Files/Storage/Wrapper/EncodingTest.php b/tests/lib/Files/Storage/Wrapper/EncodingTest.php index 498d9f7824811..0901edf83fa06 100644 --- a/tests/lib/Files/Storage/Wrapper/EncodingTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncodingTest.php @@ -32,7 +32,7 @@ protected function tearDown(): void { public function directoryProvider() { $a = parent::directoryProvider(); - $a[] = [self::NFD_NAME]; + $a[] = [self::NFC_NAME]; return $a; } @@ -199,4 +199,46 @@ public function testCopyAndMoveFromStorageEncodedFolder($sourceDir, $targetDir) $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']); + } }