Merge pull request #46538 from nextcloud/fix/use-filename-validator

refactor: Migrate filename validation from `Storage` and `Util` to `FilenameValidator`
pull/46477/head^2
Ferdinand Thiessen 4 months ago committed by GitHub
commit 1b41e8f566
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 13
      apps/dav/lib/Connector/Sabre/Node.php
  2. 4
      apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
  3. 7
      apps/dav/tests/unit/Connector/Sabre/FileTest.php
  4. 4
      apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php
  5. 64
      apps/files/lib/Controller/ViewController.php
  6. 11
      apps/files/tests/Controller/ViewControllerTest.php
  7. 19
      lib/private/Files/FilenameValidator.php
  8. 76
      lib/private/Files/Storage/Common.php
  9. 8
      lib/private/Files/View.php
  10. 29
      lib/private/legacy/OC_Util.php
  11. 34
      lib/public/Util.php
  12. 88
      tests/lib/Files/FilenameValidatorTest.php
  13. 51
      tests/lib/Files/PathVerificationTest.php
  14. 67
      tests/lib/Files/Storage/CommonTest.php
  15. 58
      tests/lib/UtilTest.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());
}

@ -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')

@ -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");
}

@ -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);
}

@ -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();

@ -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();

@ -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;

@ -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;
}
/**

@ -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);

@ -1036,35 +1036,6 @@ class OC_Util {
return $version;
}
/**
* 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

@ -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
@ -488,39 +487,6 @@ class Util {
return \OC_Helper::uploadLimit();
}
/**
* 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

@ -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
@ -171,6 +182,73 @@ 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
*/
@ -178,7 +256,7 @@ class FilenameValidatorTest extends TestCase {
/** @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')

@ -106,57 +106,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
*/

@ -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() {

@ -137,64 +137,6 @@ class UtilTest extends \Test\TestCase {
$this->assertSame(1, $matchesRegex);
}
/**
* @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
*/

Loading…
Cancel
Save