refactor: Remove deprecated Util function for filename validation to FilenameValidator

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2024-07-15 16:29:44 +02:00
parent 69341e4306
commit f4ede27cdb
No known key found for this signature in database
GPG key ID: 45FAE7268762B400
6 changed files with 29 additions and 169 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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