Skip to content

Commit

Permalink
[TASK] Refactor GU::getFileAbsFileName
Browse files Browse the repository at this point in the history
Improve readability of the method by extracting distinct cases into own
blocks with early return statements.

Case 1: "EXT:" path
Case 2: (already) absolute path
Case 3: relative public path

Resolves: #100094
Releases: main
Change-Id: I664c3611ef86c779c0398c7a4f938297eb591a81
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/78034
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
  • Loading branch information
nhovratov authored and georgringer committed Mar 8, 2023
1 parent b64c638 commit b221f6c
Showing 1 changed file with 24 additions and 21 deletions.
45 changes: 24 additions & 21 deletions typo3/sysext/core/Classes/Utility/GeneralUtility.php
Original file line number Diff line number Diff line change
Expand Up @@ -2571,31 +2571,34 @@ protected static function encodeFileSystemPathComponentForUrlPath(string $path):
*/
public static function getFileAbsFileName($filename)
{
if ((string)$filename === '') {
$fileName = (string)$filename;
if ($fileName === '') {
return '';
}
// Extension
if (PathUtility::isExtensionPath($filename)) {
$checkForBackPath = fn (string $fileName): string => $fileName !== '' && static::validPathStr($fileName) ? $fileName : '';

// Extension "EXT:" path resolving.
if (PathUtility::isExtensionPath($fileName)) {
try {
$filename = ExtensionManagementUtility::resolvePackagePath($filename);
} catch (PackageException $e) {
$filename = '';
}
} elseif (!PathUtility::isAbsolutePath($filename)) {
// is relative. Prepended with the public web folder
$filename = Environment::getPublicPath() . '/' . $filename;
} elseif (!(
str_starts_with($filename, Environment::getProjectPath())
|| str_starts_with($filename, Environment::getPublicPath())
)) {
// absolute, but set to blank if not allowed
$filename = '';
}
if ((string)$filename !== '' && static::validPathStr($filename)) {
// checks backpath.
return $filename;
$fileName = ExtensionManagementUtility::resolvePackagePath($fileName);
} catch (PackageException) {
$fileName = '';
}
return $checkForBackPath($fileName);
}
return '';

// Absolute path, but set to blank if not inside allowed directories.
if (PathUtility::isAbsolutePath($fileName)) {
if (str_starts_with($fileName, Environment::getProjectPath()) ||
str_starts_with($fileName, Environment::getPublicPath())) {

This comment has been minimized.

Copy link
@MajPay

MajPay Jan 30, 2024

It might be possible there should be another assertion here:

// get additional setting
$lockRootPath = $GLOBALS['TYPO3_CONF_VARS']['BE']['lockRootPath'] ?? '';
// assert against setting
($lockRootPath && str_starts_with($path, $lockRootPath))

@see GeneralUtility::isAllowedAbsPath

I suspect this change to break my development setup
@see https://forge.typo3.org/issues/102993

It might be best to use isAllowedAbsPath() for this assertion

return $checkForBackPath($fileName);
}
return '';
}

// Relative path. Prepend with the public web folder.
$fileName = Environment::getPublicPath() . '/' . $fileName;
return $checkForBackPath($fileName);
}

/**
Expand Down

0 comments on commit b221f6c

Please sign in to comment.