aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apps/dav/lib/Connector/Sabre/Node.php13
-rw-r--r--apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php4
-rw-r--r--apps/dav/tests/unit/Connector/Sabre/FileTest.php7
-rw-r--r--apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php4
-rw-r--r--apps/files/lib/Controller/ViewController.php64
-rw-r--r--apps/files/tests/Controller/ViewControllerTest.php11
-rw-r--r--lib/private/Files/FilenameValidator.php19
-rw-r--r--lib/private/Files/Storage/Common.php76
-rw-r--r--lib/private/Files/View.php8
-rw-r--r--lib/private/legacy/OC_Util.php29
-rw-r--r--lib/public/Util.php34
-rw-r--r--tests/lib/Files/FilenameValidatorTest.php88
-rw-r--r--tests/lib/Files/PathVerificationTest.php51
-rw-r--r--tests/lib/Files/Storage/CommonTest.php67
-rw-r--r--tests/lib/UtilTest.php58
15 files changed, 186 insertions, 347 deletions
diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php
index 63e453c86af..379574b30d6 100644
--- a/apps/dav/lib/Connector/Sabre/Node.php
+++ b/apps/dav/lib/Connector/Sabre/Node.php
@@ -122,11 +122,11 @@ abstract class Node implements \Sabre\DAV\INode {
[$parentPath,] = \Sabre\Uri\split($this->path);
[, $newName] = \Sabre\Uri\split($name);
+ $newPath = $parentPath . '/' . $newName;
// verify path of the target
- $this->verifyPath();
+ $this->verifyPath($newPath);
- $newPath = $parentPath . '/' . $newName;
if (!$this->fileView->rename($this->path, $newPath)) {
throw new \Sabre\DAV\Exception('Failed to rename '. $this->path . ' to ' . $newPath);
@@ -355,10 +355,13 @@ abstract class Node implements \Sabre\DAV\INode {
return $this->info->getOwner();
}
- protected function verifyPath() {
+ protected function verifyPath(?string $path = null): void {
try {
- $fileName = basename($this->info->getPath());
- $this->fileView->verifyPath($this->path, $fileName);
+ $path = $path ?? $this->info->getPath();
+ $this->fileView->verifyPath(
+ dirname($path),
+ basename($path),
+ );
} catch (\OCP\Files\InvalidPathException $ex) {
throw new InvalidPath($ex->getMessage());
}
diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
index 3eca9b7ac1c..d2be66c13f5 100644
--- a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
@@ -397,7 +397,7 @@ class DirectoryTest extends \Test\TestCase {
public function moveFailedInvalidCharsProvider() {
return [
- ['a/b', 'a/*', ['a' => true, 'a/b' => true, 'a/c*' => false], []],
+ ['a/valid', "a/i\nvalid", ['a' => true, 'a/valid' => true, 'a/c*' => false], []],
];
}
@@ -463,7 +463,7 @@ class DirectoryTest extends \Test\TestCase {
$sourceNode = new Directory($view, $sourceInfo);
$targetNode = $this->getMockBuilder(Directory::class)
- ->setMethods(['childExists'])
+ ->onlyMethods(['childExists'])
->setConstructorArgs([$view, $targetInfo])
->getMock();
$targetNode->expects($this->once())->method('childExists')
diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
index 060f0294256..d099a431184 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
@@ -570,7 +570,7 @@ class FileTest extends TestCase {
->method('getRelativePath')
->willReturnArgument(0);
- $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, [
+ $info = new \OC\Files\FileInfo("/i\nvalid", $this->getMockStorage(), null, [
'permissions' => \OCP\Constants::PERMISSION_ALL,
'type' => FileInfo::TYPE_FOLDER,
], null);
@@ -611,12 +611,13 @@ class FileTest extends TestCase {
->method('getRelativePath')
->willReturnArgument(0);
- $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, [
+ $info = new \OC\Files\FileInfo('/valid', $this->getMockStorage(), null, [
'permissions' => \OCP\Constants::PERMISSION_ALL,
'type' => FileInfo::TYPE_FOLDER,
], null);
$file = new \OCA\DAV\Connector\Sabre\File($view, $info);
- $file->setName('/super*star.txt');
+
+ $file->setName("/i\nvalid");
}
diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php
index 2f41502d4f6..e1bbbbc310f 100644
--- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php
+++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php
@@ -23,6 +23,7 @@ use OCP\Federation\ICloudFederationProvider;
use OCP\Federation\ICloudFederationProviderManager;
use OCP\Federation\ICloudFederationShare;
use OCP\Federation\ICloudIdManager;
+use OCP\Files\IFilenameValidator;
use OCP\Files\NotFoundException;
use OCP\HintException;
use OCP\IConfig;
@@ -59,6 +60,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
private IConfig $config,
private Manager $externalShareManager,
private LoggerInterface $logger,
+ private IFilenameValidator $filenameValidator,
) {
}
@@ -115,7 +117,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
}
if ($remote && $token && $name && $owner && $remoteId && $shareWith) {
- if (!Util::isValidFileName($name)) {
+ if (!$this->filenameValidator->isFilenameValid($name)) {
throw new ProviderCouldNotAddShareException('The mountpoint name contains invalid characters.', '', Http::STATUS_BAD_REQUEST);
}
diff --git a/apps/files/lib/Controller/ViewController.php b/apps/files/lib/Controller/ViewController.php
index 0f24db6b077..3be7e61d010 100644
--- a/apps/files/lib/Controller/ViewController.php
+++ b/apps/files/lib/Controller/ViewController.php
@@ -7,6 +7,7 @@
*/
namespace OCA\Files\Controller;
+use OC\Files\FilenameValidator;
use OCA\Files\Activity\Helper;
use OCA\Files\AppInfo\Application;
use OCA\Files\Event\LoadAdditionalScriptsEvent;
@@ -34,57 +35,31 @@ use OCP\IL10N;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUserSession;
-use OCP\Share\IManager;
/**
* @package OCA\Files\Controller
*/
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
class ViewController extends Controller {
- private IURLGenerator $urlGenerator;
- private IL10N $l10n;
- private IConfig $config;
- private IEventDispatcher $eventDispatcher;
- private IUserSession $userSession;
- private IAppManager $appManager;
- private IRootFolder $rootFolder;
- private Helper $activityHelper;
- private IInitialState $initialState;
- private ITemplateManager $templateManager;
- private IManager $shareManager;
- private UserConfig $userConfig;
- private ViewConfig $viewConfig;
-
- public function __construct(string $appName,
+
+ public function __construct(
+ string $appName,
IRequest $request,
- IURLGenerator $urlGenerator,
- IL10N $l10n,
- IConfig $config,
- IEventDispatcher $eventDispatcher,
- IUserSession $userSession,
- IAppManager $appManager,
- IRootFolder $rootFolder,
- Helper $activityHelper,
- IInitialState $initialState,
- ITemplateManager $templateManager,
- IManager $shareManager,
- UserConfig $userConfig,
- ViewConfig $viewConfig
+ private IURLGenerator $urlGenerator,
+ private IL10N $l10n,
+ private IConfig $config,
+ private IEventDispatcher $eventDispatcher,
+ private IUserSession $userSession,
+ private IAppManager $appManager,
+ private IRootFolder $rootFolder,
+ private Helper $activityHelper,
+ private IInitialState $initialState,
+ private ITemplateManager $templateManager,
+ private UserConfig $userConfig,
+ private ViewConfig $viewConfig,
+ private FilenameValidator $filenameValidator,
) {
parent::__construct($appName, $request);
- $this->urlGenerator = $urlGenerator;
- $this->l10n = $l10n;
- $this->config = $config;
- $this->eventDispatcher = $eventDispatcher;
- $this->userSession = $userSession;
- $this->appManager = $appManager;
- $this->rootFolder = $rootFolder;
- $this->activityHelper = $activityHelper;
- $this->initialState = $initialState;
- $this->templateManager = $templateManager;
- $this->shareManager = $shareManager;
- $this->userConfig = $userConfig;
- $this->viewConfig = $viewConfig;
}
/**
@@ -220,8 +195,9 @@ class ViewController extends Controller {
$filesSortingConfig = json_decode($this->config->getUserValue($userId, 'files', 'files_sorting_configs', '{}'), true);
$this->initialState->provideInitialState('filesSortingConfig', $filesSortingConfig);
- // Forbidden file characters
- $forbiddenCharacters = \OCP\Util::getForbiddenFileNameChars();
+ // Forbidden file characters (deprecated use capabilities)
+ // TODO: Remove with next release of `@nextcloud/files`
+ $forbiddenCharacters = $this->filenameValidator->getForbiddenCharacters();
$this->initialState->provideInitialState('forbiddenCharacters', $forbiddenCharacters);
$event = new LoadAdditionalScriptsEvent();
diff --git a/apps/files/tests/Controller/ViewControllerTest.php b/apps/files/tests/Controller/ViewControllerTest.php
index 98147228cf1..aade58da5a3 100644
--- a/apps/files/tests/Controller/ViewControllerTest.php
+++ b/apps/files/tests/Controller/ViewControllerTest.php
@@ -7,6 +7,7 @@
*/
namespace OCA\Files\Tests\Controller;
+use OC\Files\FilenameValidator;
use OCA\Files\Activity\Helper;
use OCA\Files\Controller\ViewController;
use OCA\Files\Service\UserConfig;
@@ -87,10 +88,12 @@ class ViewControllerTest extends TestCase {
$this->activityHelper = $this->createMock(Helper::class);
$this->initialState = $this->createMock(IInitialState::class);
$this->templateManager = $this->createMock(ITemplateManager::class);
- $this->shareManager = $this->createMock(IManager::class);
$this->userConfig = $this->createMock(UserConfig::class);
$this->viewConfig = $this->createMock(ViewConfig::class);
- $this->viewController = $this->getMockBuilder('\OCA\Files\Controller\ViewController')
+
+ $filenameValidator = $this->createMock(FilenameValidator::class);
+
+ $this->viewController = $this->getMockBuilder(ViewController::class)
->setConstructorArgs([
'files',
$this->request,
@@ -104,11 +107,11 @@ class ViewControllerTest extends TestCase {
$this->activityHelper,
$this->initialState,
$this->templateManager,
- $this->shareManager,
$this->userConfig,
$this->viewConfig,
+ $filenameValidator,
])
- ->setMethods([
+ ->onlyMethods([
'getStorageInfo',
])
->getMock();
diff --git a/lib/private/Files/FilenameValidator.php b/lib/private/Files/FilenameValidator.php
index 0026dfdf451..b1ce8e02b13 100644
--- a/lib/private/Files/FilenameValidator.php
+++ b/lib/private/Files/FilenameValidator.php
@@ -11,9 +11,11 @@ use OCP\Files\EmptyFileNameException;
use OCP\Files\FileNameTooLongException;
use OCP\Files\IFilenameValidator;
use OCP\Files\InvalidCharacterInPathException;
+use OCP\Files\InvalidDirectoryException;
use OCP\Files\InvalidPathException;
use OCP\Files\ReservedWordException;
use OCP\IConfig;
+use OCP\IDBConnection;
use OCP\IL10N;
use OCP\L10N\IFactory;
use Psr\Log\LoggerInterface;
@@ -46,6 +48,7 @@ class FilenameValidator implements IFilenameValidator {
public function __construct(
IFactory $l10nFactory,
+ private IDBConnection $database,
private IConfig $config,
private LoggerInterface $logger,
) {
@@ -173,8 +176,9 @@ class FilenameValidator implements IFilenameValidator {
}
// the special directories . and .. would cause never ending recursion
+ // we check the trimmed name here to ensure unexpected trimming will not cause severe issues
if ($trimmed === '.' || $trimmed === '..') {
- throw new ReservedWordException();
+ throw new InvalidDirectoryException($this->l10n->t('Dot files are not allowed'));
}
// 255 characters is the limit on common file systems (ext/xfs)
@@ -183,6 +187,17 @@ class FilenameValidator implements IFilenameValidator {
throw new FileNameTooLongException();
}
+ if (!$this->database->supports4ByteText()) {
+ // verify database - e.g. mysql only 3-byte chars
+ if (preg_match('%(?:
+ \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3
+ | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15
+ | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16
+)%xs', $filename)) {
+ throw new InvalidCharacterInPathException();
+ }
+ }
+
if ($this->isForbidden($filename)) {
throw new ReservedWordException();
}
@@ -263,7 +278,7 @@ class FilenameValidator implements IFilenameValidator {
* @return string[]
*/
private function getConfigValue(string $key, array $fallback): array {
- $values = $this->config->getSystemValue($key, ['.filepart']);
+ $values = $this->config->getSystemValue($key, $fallback);
if (!is_array($values)) {
$this->logger->error('Invalid system config value for "' . $key . '" is ignored.');
$values = $fallback;
diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php
index e613e435f03..a8f8c05ab54 100644
--- a/lib/private/Files/Storage/Common.php
+++ b/lib/private/Files/Storage/Common.php
@@ -16,14 +16,10 @@ use OC\Files\Cache\Watcher;
use OC\Files\Filesystem;
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\Wrapper;
-use OCP\Files\EmptyFileNameException;
-use OCP\Files\FileNameTooLongException;
use OCP\Files\ForbiddenException;
use OCP\Files\GenericFileException;
-use OCP\Files\InvalidCharacterInPathException;
-use OCP\Files\InvalidDirectoryException;
+use OCP\Files\IFilenameValidator;
use OCP\Files\InvalidPathException;
-use OCP\Files\ReservedWordException;
use OCP\Files\Storage\ILockingStorage;
use OCP\Files\Storage\IStorage;
use OCP\Files\Storage\IWriteStreamStorage;
@@ -57,10 +53,9 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
protected $mountOptions = [];
protected $owner = null;
- /** @var ?bool */
- private $shouldLogLocks = null;
- /** @var ?LoggerInterface */
- private $logger;
+ private ?bool $shouldLogLocks = null;
+ private ?LoggerInterface $logger = null;
+ private ?IFilenameValidator $filenameValidator = null;
public function __construct($parameters) {
}
@@ -496,68 +491,21 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
* @throws InvalidPathException
*/
public function verifyPath($path, $fileName) {
- // verify empty and dot files
- $trimmed = trim($fileName);
- if ($trimmed === '') {
- throw new EmptyFileNameException();
- }
-
- if (\OC\Files\Filesystem::isIgnoredDir($trimmed)) {
- throw new InvalidDirectoryException();
- }
-
- if (!\OC::$server->getDatabaseConnection()->supports4ByteText()) {
- // verify database - e.g. mysql only 3-byte chars
- if (preg_match('%(?:
- \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3
- | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15
- | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16
-)%xs', $fileName)) {
- throw new InvalidCharacterInPathException();
- }
- }
-
- // 255 characters is the limit on common file systems (ext/xfs)
- // oc_filecache has a 250 char length limit for the filename
- if (isset($fileName[250])) {
- throw new FileNameTooLongException();
- }
+ $this->getFilenameValidator()
+ ->validateFilename($fileName);
// NOTE: $path will remain unverified for now
- $this->verifyPosixPath($fileName);
}
/**
- * @param string $fileName
- * @throws InvalidPathException
+ * Get the filename validator
+ * (cached for performance)
*/
- protected function verifyPosixPath($fileName) {
- $invalidChars = \OCP\Util::getForbiddenFileNameChars();
- $this->scanForInvalidCharacters($fileName, $invalidChars);
-
- $fileName = trim($fileName);
- $reservedNames = ['*'];
- if (in_array($fileName, $reservedNames)) {
- 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();
- }
- }
-
- $sanitizedFileName = filter_var($fileName, FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW);
- if ($sanitizedFileName !== $fileName) {
- throw new InvalidCharacterInPathException();
+ protected function getFilenameValidator(): IFilenameValidator {
+ if ($this->filenameValidator === null) {
+ $this->filenameValidator = \OCP\Server::get(IFilenameValidator::class);
}
+ return $this->filenameValidator;
}
/**
diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php
index 0150a3e117a..26d419842d6 100644
--- a/lib/private/Files/View.php
+++ b/lib/private/Files/View.php
@@ -716,6 +716,12 @@ class View {
return false;
}
+ try {
+ $this->verifyPath(dirname($target), basename($target));
+ } catch (InvalidPathException) {
+ return false;
+ }
+
$this->lockFile($source, ILockingProvider::LOCK_SHARED, true);
try {
$this->lockFile($target, ILockingProvider::LOCK_SHARED, true);
@@ -739,8 +745,6 @@ class View {
}
}
if ($run) {
- $this->verifyPath(dirname($target), basename($target));
-
$manager = Filesystem::getMountManager();
$mount1 = $this->getMount($source);
$mount2 = $this->getMount($target);
diff --git a/lib/private/legacy/OC_Util.php b/lib/private/legacy/OC_Util.php
index b7836e86d7b..d8045e8343d 100644
--- a/lib/private/legacy/OC_Util.php
+++ b/lib/private/legacy/OC_Util.php
@@ -1037,35 +1037,6 @@ class OC_Util {
}
/**
- * Returns whether the given file name is valid
- *
- * @param string $file file name to check
- * @return bool true if the file name is valid, false otherwise
- * @deprecated use \OC\Files\View::verifyPath()
- */
- public static function isValidFileName($file) {
- $trimmed = trim($file);
- if ($trimmed === '') {
- return false;
- }
- if (\OC\Files\Filesystem::isIgnoredDir($trimmed)) {
- return false;
- }
-
- // detect part files
- if (preg_match('/' . \OCP\Files\FileInfo::BLACKLIST_FILES_REGEX . '/', $trimmed) !== 0) {
- return false;
- }
-
- foreach (\OCP\Util::getForbiddenFileNameChars() as $char) {
- if (str_contains($trimmed, $char)) {
- return false;
- }
- }
- return true;
- }
-
- /**
* Check whether the instance needs to perform an upgrade,
* either when the core version is higher or any app requires
* an upgrade.
diff --git a/lib/public/Util.php b/lib/public/Util.php
index 4cee9addf10..17794810c1a 100644
--- a/lib/public/Util.php
+++ b/lib/public/Util.php
@@ -18,7 +18,6 @@ use OCP\L10N\IFactory;
use OCP\Mail\IMailer;
use OCP\Share\IManager;
use Psr\Container\ContainerExceptionInterface;
-use Psr\Log\LoggerInterface;
/**
* This class provides different helper functions to make the life of a developer easier
@@ -489,39 +488,6 @@ class Util {
}
/**
- * Get a list of characters forbidden in file names
- * @return string[]
- * @since 29.0.0
- */
- public static function getForbiddenFileNameChars(): array {
- // Get always forbidden characters
- $invalidChars = str_split(\OCP\Constants::FILENAME_INVALID_CHARS);
- if ($invalidChars === false) {
- $invalidChars = [];
- }
-
- // Get admin defined invalid characters
- $additionalChars = \OCP\Server::get(IConfig::class)->getSystemValue('forbidden_chars', []);
- if (!is_array($additionalChars)) {
- \OCP\Server::get(LoggerInterface::class)->error('Invalid system config value for "forbidden_chars" is ignored.');
- $additionalChars = [];
- }
- return array_merge($invalidChars, $additionalChars);
- }
-
- /**
- * Returns whether the given file name is valid
- * @param string $file file name to check
- * @return bool true if the file name is valid, false otherwise
- * @deprecated 8.1.0 use OCP\Files\Storage\IStorage::verifyPath()
- * @since 7.0.0
- * @suppress PhanDeprecatedFunction
- */
- public static function isValidFileName($file) {
- return \OC_Util::isValidFileName($file);
- }
-
- /**
* Compare two strings to provide a natural sort
* @param string $a first string to compare
* @param string $b second string to compare
diff --git a/tests/lib/Files/FilenameValidatorTest.php b/tests/lib/Files/FilenameValidatorTest.php
index 42705a23f02..1fcc03064a2 100644
--- a/tests/lib/Files/FilenameValidatorTest.php
+++ b/tests/lib/Files/FilenameValidatorTest.php
@@ -13,9 +13,11 @@ use OC\Files\FilenameValidator;
use OCP\Files\EmptyFileNameException;
use OCP\Files\FileNameTooLongException;
use OCP\Files\InvalidCharacterInPathException;
+use OCP\Files\InvalidDirectoryException;
use OCP\Files\InvalidPathException;
use OCP\Files\ReservedWordException;
use OCP\IConfig;
+use OCP\IDBConnection;
use OCP\IL10N;
use OCP\L10N\IFactory;
use PHPUnit\Framework\MockObject\MockObject;
@@ -26,6 +28,7 @@ class FilenameValidatorTest extends TestCase {
protected IFactory&MockObject $l10n;
protected IConfig&MockObject $config;
+ protected IDBConnection&MockObject $database;
protected LoggerInterface&MockObject $logger;
protected function setUp(): void {
@@ -41,6 +44,8 @@ class FilenameValidatorTest extends TestCase {
$this->config = $this->createMock(IConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);
+ $this->database = $this->createMock(IDBConnection::class);
+ $this->database->method('supports4ByteText')->willReturn(true);
}
/**
@@ -62,7 +67,7 @@ class FilenameValidatorTest extends TestCase {
'getForbiddenExtensions',
'getForbiddenFilenames',
])
- ->setConstructorArgs([$this->l10n, $this->config, $this->logger])
+ ->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger])
->getMock();
$validator->method('getForbiddenBasenames')
@@ -101,7 +106,7 @@ class FilenameValidatorTest extends TestCase {
'getForbiddenFilenames',
'getForbiddenCharacters',
])
- ->setConstructorArgs([$this->l10n, $this->config, $this->logger])
+ ->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger])
->getMock();
$validator->method('getForbiddenBasenames')
@@ -122,6 +127,9 @@ class FilenameValidatorTest extends TestCase {
'valid name' => [
'a: b.txt', ['.htaccess'], [], [], [], null
],
+ 'forbidden name in the middle is ok' => [
+ 'a.htaccess.txt', ['.htaccess'], [], [], [], null
+ ],
'valid name with some more parameters' => [
'a: b.txt', ['.htaccess'], [], ['exe'], ['~'], null
],
@@ -155,10 +163,13 @@ class FilenameValidatorTest extends TestCase {
'', [], [], [], [], EmptyFileNameException::class
],
'reserved unix name "."' => [
- '.', [], [], [], [], InvalidPathException::class
+ '.', [], [], [], [], InvalidDirectoryException::class
],
'reserved unix name ".."' => [
- '..', [], [], [], [], ReservedWordException::class
+ '..', [], [], [], [], InvalidDirectoryException::class
+ ],
+ 'weird but valid tripple dot name' => [
+ '...', [], [], [], [], null // is valid
],
'too long filename "."' => [
str_repeat('a', 251), [], [], [], [], FileNameTooLongException::class
@@ -172,13 +183,80 @@ class FilenameValidatorTest extends TestCase {
}
/**
+ * @dataProvider data4ByteUnicode
+ */
+ public function testDatabaseDoesNotSupport4ByteText($filename): void {
+ $database = $this->createMock(IDBConnection::class);
+ $database->expects($this->once())
+ ->method('supports4ByteText')
+ ->willReturn(false);
+ $this->expectException(InvalidCharacterInPathException::class);
+ $validator = new FilenameValidator($this->l10n, $database, $this->config, $this->logger);
+ $validator->validateFilename($filename);
+ }
+
+ public function data4ByteUnicode(): array {
+ return [
+ ['plane 1 𐪅'],
+ ['emoji 😶‍🌫️'],
+
+ ];
+ }
+
+ /**
+ * @dataProvider dataInvalidAsciiCharacters
+ */
+ public function testInvalidAsciiCharactersAreAlwaysForbidden(string $filename): void {
+ $this->expectException(InvalidPathException::class);
+ $validator = new FilenameValidator($this->l10n, $this->database, $this->config, $this->logger);
+ $validator->validateFilename($filename);
+ }
+
+ public function dataInvalidAsciiCharacters(): array {
+ return [
+ [\chr(0)],
+ [\chr(1)],
+ [\chr(2)],
+ [\chr(3)],
+ [\chr(4)],
+ [\chr(5)],
+ [\chr(6)],
+ [\chr(7)],
+ [\chr(8)],
+ [\chr(9)],
+ [\chr(10)],
+ [\chr(11)],
+ [\chr(12)],
+ [\chr(13)],
+ [\chr(14)],
+ [\chr(15)],
+ [\chr(16)],
+ [\chr(17)],
+ [\chr(18)],
+ [\chr(19)],
+ [\chr(20)],
+ [\chr(21)],
+ [\chr(22)],
+ [\chr(23)],
+ [\chr(24)],
+ [\chr(25)],
+ [\chr(26)],
+ [\chr(27)],
+ [\chr(28)],
+ [\chr(29)],
+ [\chr(30)],
+ [\chr(31)],
+ ];
+ }
+
+ /**
* @dataProvider dataIsForbidden
*/
public function testIsForbidden(string $filename, array $forbiddenNames, array $forbiddenBasenames, bool $expected): void {
/** @var FilenameValidator&MockObject */
$validator = $this->getMockBuilder(FilenameValidator::class)
->onlyMethods(['getForbiddenFilenames', 'getForbiddenBasenames'])
- ->setConstructorArgs([$this->l10n, $this->config, $this->logger])
+ ->setConstructorArgs([$this->l10n, $this->database, $this->config, $this->logger])
->getMock();
$validator->method('getForbiddenBasenames')
diff --git a/tests/lib/Files/PathVerificationTest.php b/tests/lib/Files/PathVerificationTest.php
index 4addd583354..0fd0c330ea5 100644
--- a/tests/lib/Files/PathVerificationTest.php
+++ b/tests/lib/Files/PathVerificationTest.php
@@ -107,57 +107,6 @@ class PathVerificationTest extends \Test\TestCase {
}
/**
- * @dataProvider providesInvalidCharsPosix
- */
- public function testPathVerificationInvalidCharsPosix($fileName) {
- $this->expectException(\OCP\Files\InvalidCharacterInPathException::class);
-
- $storage = new Local(['datadir' => '']);
-
- $fileName = " 123{$fileName}456 ";
- self::invokePrivate($storage, 'verifyPosixPath', [$fileName]);
- }
-
- public function providesInvalidCharsPosix() {
- return [
- [\chr(0)],
- [\chr(1)],
- [\chr(2)],
- [\chr(3)],
- [\chr(4)],
- [\chr(5)],
- [\chr(6)],
- [\chr(7)],
- [\chr(8)],
- [\chr(9)],
- [\chr(10)],
- [\chr(11)],
- [\chr(12)],
- [\chr(13)],
- [\chr(14)],
- [\chr(15)],
- [\chr(16)],
- [\chr(17)],
- [\chr(18)],
- [\chr(19)],
- [\chr(20)],
- [\chr(21)],
- [\chr(22)],
- [\chr(23)],
- [\chr(24)],
- [\chr(25)],
- [\chr(26)],
- [\chr(27)],
- [\chr(28)],
- [\chr(29)],
- [\chr(30)],
- [\chr(31)],
- ['/'],
- ['\\'],
- ];
- }
-
- /**
* @dataProvider providesValidPosixPaths
*/
public function testPathVerificationValidPaths($fileName) {
diff --git a/tests/lib/Files/Storage/CommonTest.php b/tests/lib/Files/Storage/CommonTest.php
index bd22d93fb69..321894cc23b 100644
--- a/tests/lib/Files/Storage/CommonTest.php
+++ b/tests/lib/Files/Storage/CommonTest.php
@@ -9,7 +9,10 @@ namespace Test\Files\Storage;
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\Wrapper;
+use OCP\Files\IFilenameValidator;
+use OCP\Files\InvalidCharacterInPathException;
use OCP\Files\InvalidPathException;
+use OCP\ITempManager;
use PHPUnit\Framework\MockObject\MockObject;
/**
@@ -20,65 +23,43 @@ use PHPUnit\Framework\MockObject\MockObject;
* @package Test\Files\Storage
*/
class CommonTest extends Storage {
- /**
- * @var string tmpDir
- */
- private $tmpDir;
- private array $invalidCharsBackup;
+ private string $tmpDir;
+ private IFilenameValidator&MockObject $filenameValidator;
protected function setUp(): void {
parent::setUp();
- $this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder();
+ $this->filenameValidator = $this->createMock(IFilenameValidator::class);
+ $this->overwriteService(IFilenameValidator::class, $this->filenameValidator);
+ $this->tmpDir = \OCP\Server::get(ITempManager::class)->getTemporaryFolder();
$this->instance = new \OC\Files\Storage\CommonTest(['datadir' => $this->tmpDir]);
- $this->invalidCharsBackup = \OC::$server->getConfig()->getSystemValue('forbidden_chars', []);
}
protected function tearDown(): void {
\OC_Helper::rmdirr($this->tmpDir);
- \OC::$server->getConfig()->setSystemValue('forbidden_chars', $this->invalidCharsBackup);
+ $this->restoreService(IFilenameValidator::class);
parent::tearDown();
}
- /**
- * @dataProvider dataVerifyPath
- */
- public function testVerifyPath(string $filename, array $additionalChars, bool $throws) {
- /** @var \OC\Files\Storage\CommonTest|MockObject $instance */
- $instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
- ->onlyMethods(['copyFromStorage', 'rmdir', 'unlink'])
- ->setConstructorArgs([['datadir' => $this->tmpDir]])
- ->getMock();
- $instance->method('copyFromStorage')
- ->willThrowException(new \Exception('copy'));
+ public function testVerifyPath() {
+ $this->filenameValidator
+ ->expects($this->once())
+ ->method('validateFilename')
+ ->with('invalid:char.txt')
+ ->willThrowException(new InvalidCharacterInPathException());
+ $this->expectException(InvalidPathException::class);
- \OC::$server->getConfig()->setSystemValue('forbidden_chars', $additionalChars);
-
- if ($throws) {
- $this->expectException(InvalidPathException::class);
- } else {
- $this->expectNotToPerformAssertions();
- }
- $instance->verifyPath('/', $filename);
+ $this->instance->verifyPath('/', 'invalid:char.txt');
}
- public function dataVerifyPath(): array {
- return [
- // slash is always forbidden
- 'invalid slash' => ['a/b.txt', [], true],
- // backslash is also forbidden
- 'invalid backslash' => ['a\\b.txt', [], true],
- // by default colon is not forbidden
- 'valid name' => ['a: b.txt', [], false],
- // colon can be added to the list of forbidden character
- 'invalid custom character' => ['a: b.txt', [':'], true],
- // make sure to not split the list entries as they migh contain Unicode sequences
- // in this example the "face in clouds" emoji contains the clouds emoji so only having clouds is ok
- 'valid unicode sequence' => ['🌫️.txt', ['😶‍🌫️'], false],
- // This is the reverse: clouds are forbidden -> so is also the face in the clouds emoji
- 'valid unicode sequence' => ['😶‍🌫️.txt', ['🌫️'], true],
- ];
+ public function testVerifyPathSucceed() {
+ $this->filenameValidator
+ ->expects($this->once())
+ ->method('validateFilename')
+ ->with('valid-char.txt');
+
+ $this->instance->verifyPath('/', 'valid-char.txt');
}
public function testMoveFromStorageWrapped() {
diff --git a/tests/lib/UtilTest.php b/tests/lib/UtilTest.php
index 82897cbca29..a83f27d5cf7 100644
--- a/tests/lib/UtilTest.php
+++ b/tests/lib/UtilTest.php
@@ -138,64 +138,6 @@ class UtilTest extends \Test\TestCase {
}
/**
- * @dataProvider filenameValidationProvider
- */
- public function testFilenameValidation($file, $valid) {
- // private API
- $this->assertEquals($valid, \OC_Util::isValidFileName($file));
- // public API
- $this->assertEquals($valid, \OCP\Util::isValidFileName($file));
- }
-
- public function filenameValidationProvider() {
- return [
- // valid names
- ['boringname', true],
- ['something.with.extension', true],
- ['now with spaces', true],
- ['.a', true],
- ['..a', true],
- ['.dotfile', true],
- ['single\'quote', true],
- [' spaces before', true],
- ['spaces after ', true],
- ['allowed chars including the crazy ones $%&_-^@!,()[]{}=;#', true],
- ['汉字也能用', true],
- ['und Ümläüte sind auch willkommen', true],
- // disallowed names
- ['', false],
- [' ', false],
- ['.', false],
- ['..', false],
- ['back\\slash', false],
- ['sl/ash', false],
- ['lt<lt', true],
- ['gt>gt', true],
- ['col:on', true],
- ['double"quote', true],
- ['pi|pe', true],
- ['dont?ask?questions?', true],
- ['super*star', true],
- ['new\nline', false],
-
- // better disallow these to avoid unexpected trimming to have side effects
- [' ..', false],
- ['.. ', false],
- ['. ', false],
- [' .', false],
-
- // part files not allowed
- ['.part', false],
- ['notallowed.part', false],
- ['neither.filepart', false],
-
- // part in the middle is ok
- ['super movie part one.mkv', true],
- ['super.movie.part.mkv', true],
- ];
- }
-
- /**
* Test needUpgrade() when the core version is increased
*/
public function testNeedUpgradeCore() {
generated by cgit v1.2.3 (git 2.39.1) at 2025-02-04 19:43:48 +0000