Skip to content

Commit

Permalink
refactor: Remove deprecated Util function for filename validation t…
Browse files Browse the repository at this point in the history
…o `FilenameValidator`

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux committed Jul 15, 2024
1 parent 4285f64 commit 09544aa
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
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;
Expand Down Expand Up @@ -59,6 +60,7 @@ public function __construct(
private IConfig $config,
private Manager $externalShareManager,
private LoggerInterface $logger,
private IFilenameValidator $filenameValidator,
) {
}

Expand Down Expand Up @@ -115,7 +117,7 @@ public function shareReceived(ICloudFederationShare $share) {
}

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

Expand Down
6 changes: 4 additions & 2 deletions apps/files/lib/Controller/ViewController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -220,8 +221,9 @@ public function index($dir = '', $view = '', $fileid = null, $fileNotFound = fal
$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 = \OCP\Server::get(FilenameValidator::class)->getForbiddenCharacters();
$this->initialState->provideInitialState('forbiddenCharacters', $forbiddenCharacters);

$event = new LoadAdditionalScriptsEvent();
Expand Down
29 changes: 0 additions & 29 deletions lib/private/legacy/OC_Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -1036,35 +1036,6 @@ public static function getHumanVersion() {
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
Expand Down
34 changes: 0 additions & 34 deletions lib/public/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
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
Expand Down Expand Up @@ -488,39 +487,6 @@ public static function uploadLimit(): int|float {
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
Expand Down
58 changes: 0 additions & 58 deletions tests/lib/UtilTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,64 +137,6 @@ public function testGetInstanceIdGeneratesValidId() {
$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
*/
Expand Down

0 comments on commit 09544aa

Please sign in to comment.