From 2e7944482841355ebd3b600e4f160da2ce4e60cf 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 7bb0341e201..6dc22a79b14 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -421,6 +421,7 @@ class Scanner extends BasicEmitter implements IScanner { continue; } $file = $fileMeta['name']; + $file = trim(\OC\Files\Filesystem::normalizePath($file), '/'); $newChildNames[] = $file; $child = $path ? $path . '/' . $file : $file; try { From 88b5860e7021b8d41ade8d32f7990dc316d8b738 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 d8156da3669..a340433fcfb 100644 --- a/apps/files/lib/Command/Scan.php +++ b/apps/files/lib/Command/Scan.php @@ -107,15 +107,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("\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( @@ -143,12 +134,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("\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 854445c82dc..5517e6b93e3 100644 --- a/apps/files/lib/Command/ScanAppData.php +++ b/apps/files/lib/Command/ScanAppData.php @@ -74,15 +74,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("\tEntry \"" . $fullPath . '" will not be accessible due to incompatible encoding'); - } - } - protected function scanFiles(OutputInterface $output, string $folder): int { try { $appData = $this->getAppDataFolder(); @@ -125,12 +116,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("\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 6dc22a79b14..3674ebd2c01 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -38,6 +38,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; @@ -420,8 +421,13 @@ class Scanner extends BasicEmitter implements IScanner { 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 5509a49941b..b886eaca80d 100644 --- a/lib/private/Files/Utils/Scanner.php +++ b/lib/private/Files/Utils/Scanner.php @@ -146,6 +146,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]); + }); } /** From ca376648870be78ffbc19442e28cd38ebd265b50 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 cea7158b1b6..6edb74a7e26 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1131,6 +1131,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 1bfaea25c68..c898ec89276 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1160,6 +1160,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 3674ebd2c01..c0d5e5a89e6 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -423,9 +423,12 @@ class Scanner extends BasicEmitter implements IScanner { } $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 e2b486546af..252ef1b499d 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 @@ + + * @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 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']); + } }