From 106950d4934059cfd8a602c4d5f5b801a493a73a 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 8baab8746fc..a88d49d32c2 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -420,6 +420,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 516b10de334295a5cf94238ac19c6a36dfbf9c77 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 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("\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 @@ 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 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("\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 @@ 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 a88d49d32c2..f31885d54a6 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,8 +420,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 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]); + }); } /** From f0661de4e554210b97d7a2d8f003df7dd7063465 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 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 f31885d54a6..bdefca01f6f 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -422,9 +422,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 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 @@ + + * @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']); + } }