diff options
| -rw-r--r-- | apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php | 4 | ||||
| -rw-r--r-- | apps/files/lib/Controller/ViewController.php | 64 | ||||
| -rw-r--r-- | apps/files/tests/Controller/ViewControllerTest.php | 11 | ||||
| -rw-r--r-- | lib/private/legacy/OC_Util.php | 29 | ||||
| -rw-r--r-- | lib/public/Util.php | 34 | ||||
| -rw-r--r-- | tests/lib/UtilTest.php | 58 |
6 files changed, 30 insertions, 170 deletions
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/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/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() { |