Skip to content

Commit aeb67f2

Browse files
authored
Merge pull request #29605 from nextcloud/bugfix/29603/fix-scanner-nfd-existenz
Normalize file name before existence check in scanner
2 parents 11c29c1 + c92a0e4 commit aeb67f2

9 files changed

Lines changed: 129 additions & 35 deletions

File tree

apps/files/lib/Command/Scan.php

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,6 @@ protected function configure() {
105105
);
106106
}
107107

108-
public function checkScanWarning($fullPath, OutputInterface $output) {
109-
$normalizedPath = basename(\OC\Files\Filesystem::normalizePath($fullPath));
110-
$path = basename($fullPath);
111-
112-
if ($normalizedPath !== $path) {
113-
$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
114-
}
115-
}
116-
117108
protected function scanFiles($user, $path, OutputInterface $output, $backgroundScan = false, $recursive = true, $homeOnly = false) {
118109
$connection = $this->reconnectToDatabase($output);
119110
$scanner = new \OC\Files\Utils\Scanner(
@@ -141,12 +132,8 @@ protected function scanFiles($user, $path, OutputInterface $output, $backgroundS
141132
$output->writeln('Error while scanning, storage not available (' . $e->getMessage() . ')', OutputInterface::VERBOSITY_VERBOSE);
142133
});
143134

144-
$scanner->listen('\OC\Files\Utils\Scanner', 'scanFile', function ($path) use ($output) {
145-
$this->checkScanWarning($path, $output);
146-
});
147-
148-
$scanner->listen('\OC\Files\Utils\Scanner', 'scanFolder', function ($path) use ($output) {
149-
$this->checkScanWarning($path, $output);
135+
$scanner->listen('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', function ($fullPath) use ($output) {
136+
$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
150137
});
151138

152139
try {

apps/files/lib/Command/ScanAppData.php

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,6 @@ protected function configure() {
7373
$this->addArgument('folder', InputArgument::OPTIONAL, 'The appdata subfolder to scan', '');
7474
}
7575

76-
public function checkScanWarning($fullPath, OutputInterface $output) {
77-
$normalizedPath = basename(\OC\Files\Filesystem::normalizePath($fullPath));
78-
$path = basename($fullPath);
79-
80-
if ($normalizedPath !== $path) {
81-
$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
82-
}
83-
}
84-
8576
protected function scanFiles(OutputInterface $output, string $folder): int {
8677
try {
8778
$appData = $this->getAppDataFolder();
@@ -124,12 +115,8 @@ protected function scanFiles(OutputInterface $output, string $folder): int {
124115
$output->writeln('Error while scanning, storage not available (' . $e->getMessage() . ')', OutputInterface::VERBOSITY_VERBOSE);
125116
});
126117

127-
$scanner->listen('\OC\Files\Utils\Scanner', 'scanFile', function ($path) use ($output) {
128-
$this->checkScanWarning($path, $output);
129-
});
130-
131-
$scanner->listen('\OC\Files\Utils\Scanner', 'scanFolder', function ($path) use ($output) {
132-
$this->checkScanWarning($path, $output);
118+
$scanner->listen('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', function ($fullPath) use ($output) {
119+
$output->writeln("\t<error>Entry \"" . $fullPath . '" will not be accessible due to incompatible encoding</error>');
133120
});
134121

135122
try {

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,6 +1161,7 @@
11611161
'OC\\Files\\Storage\\Temporary' => $baseDir . '/lib/private/Files/Storage/Temporary.php',
11621162
'OC\\Files\\Storage\\Wrapper\\Availability' => $baseDir . '/lib/private/Files/Storage/Wrapper/Availability.php',
11631163
'OC\\Files\\Storage\\Wrapper\\Encoding' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encoding.php',
1164+
'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => $baseDir . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
11641165
'OC\\Files\\Storage\\Wrapper\\Encryption' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encryption.php',
11651166
'OC\\Files\\Storage\\Wrapper\\Jail' => $baseDir . '/lib/private/Files/Storage/Wrapper/Jail.php',
11661167
'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => $baseDir . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,6 +1190,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
11901190
'OC\\Files\\Storage\\Temporary' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Temporary.php',
11911191
'OC\\Files\\Storage\\Wrapper\\Availability' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Availability.php',
11921192
'OC\\Files\\Storage\\Wrapper\\Encoding' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encoding.php',
1193+
'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
11931194
'OC\\Files\\Storage\\Wrapper\\Encryption' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encryption.php',
11941195
'OC\\Files\\Storage\\Wrapper\\Jail' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Jail.php',
11951196
'OC\\Files\\Storage\\Wrapper\\PermissionsMask' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/PermissionsMask.php',

lib/private/Files/Cache/Scanner.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737

3838
use Doctrine\DBAL\Exception;
3939
use OC\Files\Filesystem;
40+
use OC\Files\Storage\Wrapper\Encoding;
4041
use OC\Hooks\BasicEmitter;
4142
use OCP\Files\Cache\IScanner;
4243
use OCP\Files\ForbiddenException;
@@ -419,7 +420,16 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
419420
if ($permissions === 0) {
420421
continue;
421422
}
422-
$file = $fileMeta['name'];
423+
$originalFile = $fileMeta['name'];
424+
$file = trim(\OC\Files\Filesystem::normalizePath($originalFile), '/');
425+
if (trim($originalFile, '/') !== $file) {
426+
// encoding mismatch, might require compatibility wrapper
427+
\OC::$server->getLogger()->debug('Scanner: Skipping non-normalized file name "'. $originalFile . '" in path "' . $path . '".', ['app' => 'core']);
428+
$this->emit('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', [$path ? $path . '/' . $originalFile : $originalFile]);
429+
// skip this entry
430+
continue;
431+
}
432+
423433
$newChildNames[] = $file;
424434
$child = $path ? $path . '/' . $file : $file;
425435
try {

lib/private/Files/Storage/Wrapper/Encoding.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
namespace OC\Files\Storage\Wrapper;
3030

3131
use OC\Cache\CappedMemoryCache;
32+
use OC\Files\Filesystem;
3233
use OCP\Files\Storage\IStorage;
3334
use OCP\ICache;
3435

@@ -162,7 +163,8 @@ public function rmdir($path) {
162163
* @return resource|bool
163164
*/
164165
public function opendir($path) {
165-
return $this->storage->opendir($this->findPathToUse($path));
166+
$handle = $this->storage->opendir($this->findPathToUse($path));
167+
return EncodingDirectoryWrapper::wrap($handle);
166168
}
167169

168170
/**
@@ -532,10 +534,16 @@ public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $t
532534
}
533535

534536
public function getMetaData($path) {
535-
return $this->storage->getMetaData($this->findPathToUse($path));
537+
$entry = $this->storage->getMetaData($this->findPathToUse($path));
538+
$entry['name'] = trim(Filesystem::normalizePath($entry['name']), '/');
539+
return $entry;
536540
}
537541

538542
public function getDirectoryContent($directory): \Traversable {
539-
return $this->storage->getDirectoryContent($this->findPathToUse($directory));
543+
$entries = $this->storage->getDirectoryContent($this->findPathToUse($directory));
544+
foreach ($entries as $entry) {
545+
$entry['name'] = trim(Filesystem::normalizePath($entry['name']), '/');
546+
yield $entry;
547+
}
540548
}
541549
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
/**
3+
* @copyright Copyright (c) 2021, Nextcloud GmbH.
4+
*
5+
* @author Robin Appelman <robin@icewind.nl>
6+
* @author Vincent Petry <vincent@nextcloud.com>
7+
*
8+
* @license AGPL-3.0
9+
*
10+
* This code is free software: you can redistribute it and/or modify
11+
* it under the terms of the GNU Affero General Public License, version 3,
12+
* as published by the Free Software Foundation.
13+
*
14+
* This program is distributed in the hope that it will be useful,
15+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
* GNU Affero General Public License for more details.
18+
*
19+
* You should have received a copy of the GNU Affero General Public License, version 3,
20+
* along with this program. If not, see <http://www.gnu.org/licenses/>
21+
*
22+
*/
23+
24+
namespace OC\Files\Storage\Wrapper;
25+
26+
use Icewind\Streams\DirectoryWrapper;
27+
use OC\Files\Filesystem;
28+
29+
/**
30+
* Normalize file names while reading directory entries
31+
*/
32+
class EncodingDirectoryWrapper extends DirectoryWrapper {
33+
/**
34+
* @return string
35+
*/
36+
public function dir_readdir() {
37+
$file = readdir($this->source);
38+
if ($file !== false && $file !== '.' && $file !== '..') {
39+
$file = trim(Filesystem::normalizePath($file), '/');
40+
}
41+
42+
return $file;
43+
}
44+
45+
/**
46+
* @param resource $source
47+
* @param callable $filter
48+
* @return resource|bool
49+
*/
50+
public static function wrap($source) {
51+
return self::wrapSource($source, [
52+
'source' => $source,
53+
]);
54+
}
55+
}

lib/private/Files/Utils/Scanner.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ protected function attachListener($mount) {
145145
$this->emit('\OC\Files\Utils\Scanner', 'postScanFolder', [$mount->getMountPoint() . $path]);
146146
$this->dispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
147147
});
148+
$scanner->listen('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', function ($path) use ($mount) {
149+
$this->emit('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', [$path]);
150+
});
148151
}
149152

150153
/**

tests/lib/Files/Storage/Wrapper/EncodingTest.php

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ protected function tearDown(): void {
3232

3333
public function directoryProvider() {
3434
$a = parent::directoryProvider();
35-
$a[] = [self::NFD_NAME];
35+
$a[] = [self::NFC_NAME];
3636
return $a;
3737
}
3838

@@ -199,4 +199,46 @@ public function testCopyAndMoveFromStorageEncodedFolder($sourceDir, $targetDir)
199199

200200
$this->assertEquals('bar', $this->instance->file_get_contents(self::NFC_NAME . '2/test2.txt'));
201201
}
202+
203+
public function testNormalizedDirectoryEntriesOpenDir() {
204+
$this->sourceStorage->mkdir('/test');
205+
$this->sourceStorage->mkdir('/test/' . self::NFD_NAME);
206+
207+
$this->assertTrue($this->instance->file_exists('/test/' . self::NFC_NAME));
208+
$this->assertTrue($this->instance->file_exists('/test/' . self::NFD_NAME));
209+
210+
$dh = $this->instance->opendir('/test');
211+
$content = [];
212+
while ($file = readdir($dh)) {
213+
if ($file != '.' and $file != '..') {
214+
$content[] = $file;
215+
}
216+
}
217+
218+
$this->assertCount(1, $content);
219+
$this->assertEquals(self::NFC_NAME, $content[0]);
220+
}
221+
222+
public function testNormalizedDirectoryEntriesGetDirectoryContent() {
223+
$this->sourceStorage->mkdir('/test');
224+
$this->sourceStorage->mkdir('/test/' . self::NFD_NAME);
225+
226+
$this->assertTrue($this->instance->file_exists('/test/' . self::NFC_NAME));
227+
$this->assertTrue($this->instance->file_exists('/test/' . self::NFD_NAME));
228+
229+
$content = iterator_to_array($this->instance->getDirectoryContent('/test'));
230+
$this->assertCount(1, $content);
231+
$this->assertEquals(self::NFC_NAME, $content[0]['name']);
232+
}
233+
234+
public function testNormalizedGetMetaData() {
235+
$this->sourceStorage->mkdir('/test');
236+
$this->sourceStorage->mkdir('/test/' . self::NFD_NAME);
237+
238+
$entry = $this->instance->getMetaData('/test/' . self::NFC_NAME);
239+
$this->assertEquals(self::NFC_NAME, $entry['name']);
240+
241+
$entry = $this->instance->getMetaData('/test/' . self::NFD_NAME);
242+
$this->assertEquals(self::NFC_NAME, $entry['name']);
243+
}
202244
}

0 commit comments

Comments
 (0)