Skip to content

Commit e33bc51

Browse files
authored
Merge pull request #29794 from nextcloud/backport/29605/stable21
[stable21] Normalize file name before existence check in scanner
2 parents cda4df0 + ca37664 commit e33bc51

File tree

9 files changed

+129
-35
lines changed

9 files changed

+129
-35
lines changed

apps/files/lib/Command/Scan.php

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

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

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

154141
try {

apps/files/lib/Command/ScanAppData.php

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

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

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

136123
try {

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,6 +1131,7 @@
11311131
'OC\\Files\\Storage\\Temporary' => $baseDir . '/lib/private/Files/Storage/Temporary.php',
11321132
'OC\\Files\\Storage\\Wrapper\\Availability' => $baseDir . '/lib/private/Files/Storage/Wrapper/Availability.php',
11331133
'OC\\Files\\Storage\\Wrapper\\Encoding' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encoding.php',
1134+
'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => $baseDir . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
11341135
'OC\\Files\\Storage\\Wrapper\\Encryption' => $baseDir . '/lib/private/Files/Storage/Wrapper/Encryption.php',
11351136
'OC\\Files\\Storage\\Wrapper\\Jail' => $baseDir . '/lib/private/Files/Storage/Wrapper/Jail.php',
11361137
'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
@@ -1160,6 +1160,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
11601160
'OC\\Files\\Storage\\Temporary' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Temporary.php',
11611161
'OC\\Files\\Storage\\Wrapper\\Availability' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Availability.php',
11621162
'OC\\Files\\Storage\\Wrapper\\Encoding' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encoding.php',
1163+
'OC\\Files\\Storage\\Wrapper\\EncodingDirectoryWrapper' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/EncodingDirectoryWrapper.php',
11631164
'OC\\Files\\Storage\\Wrapper\\Encryption' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Encryption.php',
11641165
'OC\\Files\\Storage\\Wrapper\\Jail' => __DIR__ . '/../../..' . '/lib/private/Files/Storage/Wrapper/Jail.php',
11651166
'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
@@ -38,6 +38,7 @@
3838

3939
use Doctrine\DBAL\Exception;
4040
use OC\Files\Filesystem;
41+
use OC\Files\Storage\Wrapper\Encoding;
4142
use OC\Hooks\BasicEmitter;
4243
use OCP\Files\Cache\IScanner;
4344
use OCP\Files\ForbiddenException;
@@ -420,7 +421,16 @@ private function handleChildren($path, $recursive, $reuse, $folderId, $lock, &$s
420421
if ($permissions === 0) {
421422
continue;
422423
}
423-
$file = $fileMeta['name'];
424+
$originalFile = $fileMeta['name'];
425+
$file = trim(\OC\Files\Filesystem::normalizePath($originalFile), '/');
426+
if (trim($originalFile, '/') !== $file) {
427+
// encoding mismatch, might require compatibility wrapper
428+
\OC::$server->getLogger()->debug('Scanner: Skipping non-normalized file name "'. $originalFile . '" in path "' . $path . '".', ['app' => 'core']);
429+
$this->emit('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', [$path ? $path . '/' . $originalFile : $originalFile]);
430+
// skip this entry
431+
continue;
432+
}
433+
424434
$newChildNames[] = $file;
425435
$child = $path ? $path . '/' . $file : $file;
426436
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
@@ -146,6 +146,9 @@ protected function attachListener($mount) {
146146
$this->emit('\OC\Files\Utils\Scanner', 'postScanFolder', [$mount->getMountPoint() . $path]);
147147
$this->dispatcher->dispatchTyped(new FolderScannedEvent($mount->getMountPoint() . $path));
148148
});
149+
$scanner->listen('\OC\Files\Cache\Scanner', 'normalizedNameMismatch', function ($path) use ($mount) {
150+
$this->emit('\OC\Files\Utils\Scanner', 'normalizedNameMismatch', [$path]);
151+
});
149152
}
150153

151154
/**

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)