Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow to enforce windows compatibility #45220

9 changes: 9 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ protected function setUp(): void {
->willReturn(Constants::PERMISSION_READ);
}

protected function tearDown(): void {
// Reset invalid chars as we touched this during the tests
self::invokePrivate(\OCP\Util::class, 'invalidChars', [[]]);
parent::tearDown();
}

private function getDir($path = '/') {
$this->view->expects($this->once())
->method('getRelativePath')
Expand Down Expand Up @@ -411,6 +417,9 @@ public function testMoveSuccess($source, $destination, $updatables, $deletables)
* @dataProvider moveFailedInvalidCharsProvider
*/
public function testMoveFailedInvalidChars($source, $destination, $updatables, $deletables): void {
// Enforce * as an invalid character
self::invokePrivate(\OCP\Util::class, 'invalidChars', [['*', '/', '\\']]);

$this->expectException(\OCA\DAV\Connector\Sabre\Exception\InvalidPath::class);

$this->moveTest($source, $destination, $updatables, $deletables);
Expand Down
9 changes: 6 additions & 3 deletions apps/dav/tests/unit/Connector/Sabre/FileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ protected function tearDown(): void {
$userManager = \OCP\Server::get(IUserManager::class);
$userManager->get($this->user)->delete();

// Reset invalid chars as we touched this during the tests
self::invokePrivate(\OCP\Util::class, 'invalidChars', [[]]);
parent::tearDown();
}

Expand Down Expand Up @@ -823,7 +825,8 @@ public function testChunkedPutFailsFinalRename(): void {
* Test put file with invalid chars
*/
public function testSimplePutInvalidChars(): void {
// setup
// Enforce * as an invalid character
self::invokePrivate(\OCP\Util::class, 'invalidChars', [['*', '/', '\\']]);
/** @var View|MockObject */
$view = $this->getMockBuilder(View::class)
->onlyMethods(['getRelativePath'])
Expand Down Expand Up @@ -858,12 +861,12 @@ public function testSimplePutInvalidChars(): void {

/**
* Test setting name with setName() with invalid chars
*
*/
public function testSetNameInvalidChars(): void {
$this->expectException(\OCA\DAV\Connector\Sabre\Exception\InvalidPath::class);

// setup
// Enforce * as an invalid character
self::invokePrivate(\OCP\Util::class, 'invalidChars', [['*', '/', '\\']]);
/** @var View|MockObject */
$view = $this->getMockBuilder(View::class)
->onlyMethods(['getRelativePath'])
Expand Down
26 changes: 22 additions & 4 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -1961,21 +1961,39 @@
'updatedirectory' => '',

/**
* Blacklist a specific file or files and disallow the upload of files
* Allow to enforce Windows compatible file and folder names.
* Nextcloud by default supports all files valid on Linux,
* but as Windows has some stricter filename rules this can lead to sync errors when using Windows clients.
*
* To enforce only Windows compatible filenames, also on the webui, set this value to ``true``.
*
* This will deny filenames with characters not valid on Windows, as well as some reserved filenames and nameing rules (no trailing dot or space).
* Additionally this will enforce files to be case in-sensitivly unique in a folder.
*
* Defaults to ``false``
*
*/
'enforce_windows_compatibility' => false,

/**
* Deny a specific file or files and disallow the upload of files
* with this name. ``.htaccess`` is blocked by default.
*
* WARNING: USE THIS ONLY IF YOU KNOW WHAT YOU ARE DOING.
*
* Note that this list is case-insensitive.
*
* Defaults to ``array('.htaccess')``
*/
'blacklisted_files' => ['.htaccess'],

/**
* Blacklist characters from being used in filenames. This is useful if you
* Deny characters from being used in filenames. This is useful if you
* have a filesystem or OS which does not support certain characters like windows.
*
* The '/' and '\' characters are always forbidden.
* The '/' and '\' characters are always forbidden, as well as all characters in the ASCII range [0-31].
*
* Example for windows systems: ``array('?', '<', '>', ':', '*', '|', '"', chr(0), "\n", "\r")``
* Example for windows systems: ``array('?', '<', '>', ':', '*', '|', '"')``
* see https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
*
* Defaults to ``array()``
Expand Down
33 changes: 25 additions & 8 deletions lib/private/Files/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ class Filesystem {

private static ?CappedMemoryCache $normalizedPathCache = null;

/** @var string[]|null */
private static ?array $blacklist = null;

/**
* classname which used for hooks handling
* used as signalclass in OC_Hooks::emit()
Expand Down Expand Up @@ -462,15 +459,35 @@ public static function isValidPath($path) {
* @param string $filename
* @return bool
*/
public static function isFileBlacklisted($filename) {
public static function hasFilenameInvalidCharacters(string $filename): bool {
$invalidChars = \OCP\Util::getForbiddenFileNameChars();
foreach ($invalidChars as $char) {
if (str_contains($filename, $char)) {
return true;
}
}

$sanitizedFileName = filter_var($filename, FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW);
if ($sanitizedFileName !== $filename) {
return true;
}
return false;
}

/**
* @param string $filename
* @return bool
*/
public static function isFileBlacklisted(string $filename): bool {
$filename = self::normalizePath($filename);
$filename = basename($filename);

if (self::$blacklist === null) {
self::$blacklist = \OC::$server->getConfig()->getSystemValue('blacklisted_files', ['.htaccess']);
if ($filename === '') {
return false;
}

$filename = strtolower(basename($filename));
return in_array($filename, self::$blacklist);
$forbiddenNames = \OCP\Util::getForbiddenFilenames();
return in_array(mb_strtolower($filename), $forbiddenNames);
}

/**
Expand Down
60 changes: 30 additions & 30 deletions lib/private/Files/Storage/Common.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
use OCP\Files\Storage\IStorage;
use OCP\Files\Storage\IWriteStreamStorage;
use OCP\Files\StorageNotAvailableException;
use OCP\IConfig;
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
use OCP\Server;
Expand Down Expand Up @@ -375,7 +376,7 @@ public function getWatcher($path = '', $storage = null) {
}
if (!isset($this->watcher)) {
$this->watcher = new Watcher($storage);
$globalPolicy = \OC::$server->getConfig()->getSystemValue('filesystem_check_changes', Watcher::CHECK_NEVER);
$globalPolicy = \OCP\Server::get(IConfig::class)->getSystemValue('filesystem_check_changes', Watcher::CHECK_NEVER);
$this->watcher->setPolicy((int)$this->getMountOption('filesystem_check_changes', $globalPolicy));
}
return $this->watcher;
Expand Down Expand Up @@ -558,41 +559,40 @@ public function verifyPath($path, $fileName) {
throw new FileNameTooLongException();
}

// NOTE: $path will remain unverified for now
$this->verifyPosixPath($fileName);
}

/**
* @param string $fileName
* @throws InvalidPathException
*/
protected function verifyPosixPath($fileName) {
$invalidChars = \OCP\Util::getForbiddenFileNameChars();
$this->scanForInvalidCharacters($fileName, $invalidChars);

$fileName = trim($fileName);
$reservedNames = ['*'];
if (in_array($fileName, $reservedNames)) {
if (\OC\Files\Filesystem::isFileBlacklisted($fileName)) {
throw new ReservedWordException();
}
}

/**
* @param string $fileName
* @param string[] $invalidChars
* @throws InvalidPathException
*/
private function scanForInvalidCharacters(string $fileName, array $invalidChars) {
foreach ($invalidChars as $char) {
if (str_contains($fileName, $char)) {
throw new InvalidCharacterInPathException();
}
if (\OC\Files\Filesystem::hasFilenameInvalidCharacters($fileName)) {
throw new InvalidCharacterInPathException();
}

$sanitizedFileName = filter_var($fileName, FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW);
if ($sanitizedFileName !== $fileName) {
throw new InvalidCharacterInPathException();
$config = \OCP\Server::get(IConfig::class);
if ($config->getSystemValueBool('enforce_windows_compatibility', false)) {
// Windows does not allow filenames to end with a trailing dot or space
if (str_ends_with($fileName, '.') || str_ends_with($fileName, ' ')) {
throw new InvalidCharacterInPathException('Filenames must not end with a dot or space');
}

// Windows has path namespaces so e.g. `NUL` is a reserved word,
// but `NUL.txt` or `NUL.tar.gz` is considered the same and thus also reserved.
$basename = substr($fileName, 0, strpos($fileName, '.') ?: null);
if (\OC\Files\Filesystem::isFileBlacklisted($basename)) {
throw new ReservedWordException();
}

// Some windows systems are case insensitive,
// so to guarantee files can be synced we need to enfore case insensitivity
$content = $this->getDirectoryContent(dirname($path));
$fileName = strtolower($fileName);
foreach ($content as $subPath) {
if (strtolower($subPath['name']) === $fileName) {
throw new InvalidPathException('Filename is not case insensitivly unique');
}
}
}

// NOTE: $path will remain unverified for now
}

/**
Expand Down