From 3e2ab899533e35a9350c637f7c8074021fa9c4ca Mon Sep 17 00:00:00 2001 From: Benni Mack <benni@typo3.org> Date: Fri, 25 May 2018 09:40:49 +0200 Subject: [PATCH] [BUGFIX] Allow writing temp files in composer mode The new Environment API allows to set the project path outside of the web root, also moving typo3temp/var/ to env:PROJECT_PATH + var/. However, the main method GeneralUtility::writeFileToTypo3tempDir() which is used for adding online media, charset conversion etc. is not adapted to allow files outside of typo3temp/ which needs adaptions wo also check for PROJECT_PATH + var/ in addition. Some generic tests were added to ensure the existing functionality still works. Resolves: #85077 Releases: master Change-Id: I664e152ecba39fbb86605af12e83f3ef10f878f9 Reviewed-on: https://review.typo3.org/57046 Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl> Tested-by: TYPO3com <no-reply@typo3.com> Reviewed-by: Susanne Moog <susanne.moog@typo3.org> Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de> Reviewed-by: Petra Arentzen <typo3@pegu.de> Reviewed-by: Jigal van Hemert <jigal.van.hemert@typo3.org> Tested-by: Jigal van Hemert <jigal.van.hemert@typo3.org> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> --- .../core/Classes/Utility/GeneralUtility.php | 80 ++++++++++------ .../Tests/Unit/Utility/GeneralUtilityTest.php | 92 +++++++++++++++++++ 2 files changed, 145 insertions(+), 27 deletions(-) diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php index c57aa9fd6d8b..fc184f73fe9a 100644 --- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php +++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php @@ -2017,40 +2017,66 @@ class GeneralUtility if (!static::validPathStr($filepath) || !$fI['basename'] || strlen($fI['basename']) >= 60) { return 'Input filepath "' . $filepath . '" was generally invalid!'; } + // Setting main temporary directory name (standard) - $dirName = PATH_site . 'typo3temp/'; - if (!@is_dir($dirName)) { - return 'PATH_site + "typo3temp/" was not a directory!'; - } - if (!static::isFirstPartOfStr($fI['dirname'], $dirName)) { - return '"' . $fI['dirname'] . '" was not within directory PATH_site + "typo3temp/"'; - } - // Checking if the "subdir" is found: - $subdir = substr($fI['dirname'], strlen($dirName)); - if ($subdir) { - if (preg_match('#^(?:[[:alnum:]_]+/)+$#', $subdir)) { - $dirName .= $subdir; - if (!@is_dir($dirName)) { - static::mkdir_deep(PATH_site . 'typo3temp/' . $subdir); + $allowedPathPrefixes = [ + PATH_site . 'typo3temp' => 'PATH_site + "typo3temp/"' + ]; + // Also allow project-path + /var/ + if (Environment::getVarPath() !== PATH_site . 'typo3temp/var') { + $relPath = substr(Environment::getVarPath(), strlen(Environment::getProjectPath()) + 1); + $allowedPathPrefixes[Environment::getVarPath()] = 'ProjectPath + ' . $relPath; + } + + $errorMessage = null; + foreach ($allowedPathPrefixes as $pathPrefix => $prefixLabel) { + $dirName = $pathPrefix . '/'; + // Invalid file path, let's check for the other path, if it exists + if (!static::isFirstPartOfStr($fI['dirname'], $dirName)) { + if ($errorMessage === null) { + $errorMessage = '"' . $fI['dirname'] . '" was not within directory ' . $prefixLabel; } - } else { - return 'Subdir, "' . $subdir . '", was NOT on the form "[[:alnum:]_]/+"'; + continue; } - } - // Checking dir-name again (sub-dir might have been created): - if (@is_dir($dirName)) { - if ($filepath === $dirName . $fI['basename']) { - static::writeFile($filepath, $content); - if (!@is_file($filepath)) { - return 'The file was not written to the disk. Please, check that you have write permissions to the typo3temp/ directory.'; + // This resets previous error messages from the first path + $errorMessage = null; + + if (!@is_dir($dirName)) { + $errorMessage = $prefixLabel . ' was not a directory!'; + // continue and see if the next iteration resets the errorMessage above + continue; + } + // Checking if the "subdir" is found + $subdir = substr($fI['dirname'], strlen($dirName)); + if ($subdir) { + if (preg_match('#^(?:[[:alnum:]_]+/)+$#', $subdir)) { + $dirName .= $subdir; + if (!@is_dir($dirName)) { + static::mkdir_deep($pathPrefix . '/' . $subdir); + } + } else { + $errorMessage = 'Subdir, "' . $subdir . '", was NOT on the form "[[:alnum:]_]/+"'; + break; + } + } + // Checking dir-name again (sub-dir might have been created) + if (@is_dir($dirName)) { + if ($filepath === $dirName . $fI['basename']) { + static::writeFile($filepath, $content); + if (!@is_file($filepath)) { + $errorMessage = 'The file was not written to the disk. Please, check that you have write permissions to the ' . $prefixLabel . ' directory.'; + break; + } + } else { + $errorMessage = 'Calculated file location didn\'t match input "' . $filepath . '".'; + break; } } else { - return 'Calculated file location didn\'t match input "' . $filepath . '".'; + $errorMessage = '"' . $dirName . '" is not a directory!'; + break; } - } else { - return '"' . $dirName . '" is not a directory!'; } - return null; + return $errorMessage; } /** diff --git a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php index b8754420c50f..cc127017020a 100644 --- a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php +++ b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php @@ -2872,6 +2872,98 @@ class GeneralUtilityTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase return array_shift($secondaryGroups); } + ///////////////////////////////////////////// + // Tests concerning writeFileToTypo3tempDir() + ///////////////////////////////////////////// + + /** + * @return array + */ + public function invalidFilePathForTypo3tempDirDataProvider() + { + return [ + [ + PATH_site . '../path/this-path-has-more-than-60-characters-in-one-base-path-you-can-even-count-more', + 'Input filepath "' . PATH_site . '../path/this-path-has-more-than-60-characters-in-one-base-path-you-can-even-count-more" was generally invalid!' + ], + [ + PATH_site . 'dummy/path/this-path-has-more-than-60-characters-in-one-base-path-you-can-even-count-more', + 'Input filepath "' . PATH_site . 'dummy/path/this-path-has-more-than-60-characters-in-one-base-path-you-can-even-count-more" was generally invalid!' + ], + [ + PATH_site . 'dummy/path/this-path-has-more-than-60-characters-in-one-base-path-you-can-even-count-more', + 'Input filepath "' . PATH_site . 'dummy/path/this-path-has-more-than-60-characters-in-one-base-path-you-can-even-count-more" was generally invalid!' + ], + [ + '/dummy/path/awesome', + '"/dummy/path/" was not within directory PATH_site + "typo3temp/"' + ], + [ + PATH_site . 'typo3conf/path', + '"' . PATH_site . 'typo3conf/" was not within directory PATH_site + "typo3temp/"', + ], + [ + PATH_site . 'typo3temp/táylor/swÃft', + 'Subdir, "táylor/", was NOT on the form "[[:alnum:]_]/+"', + ], + 'Path instead of file given' => [ + PATH_site . 'typo3temp/dummy/path/', + 'Calculated file location didn\'t match input "' . PATH_site . 'typo3temp/dummy/path/".' + ], + ]; + } + + /** + * @test + * @dataProvider invalidFilePathForTypo3tempDirDataProvider + * @param string $invalidFilePath + * @param string $expectedResult + */ + public function writeFileToTypo3tempDirFailsWithInvalidPath($invalidFilePath, string $expectedResult) + { + $result = GeneralUtility::writeFileToTypo3tempDir($invalidFilePath, 'dummy content to be written'); + $this->assertSame($result, $expectedResult); + } + + /** + * @return array + */ + public function validFilePathForTypo3tempDirDataProvider() + { + return [ + 'Default text file' => [ + PATH_site . 'typo3temp/var/paranoid/android.txt', + ], + 'Html file extension' => [ + PATH_site . 'typo3temp/var/karma.html', + ], + 'No file extension' => [ + PATH_site . 'typo3temp/var/no-surprises', + ], + 'Deep directory' => [ + PATH_site . 'typo3temp/var/climbing/up/the/walls', + ], + ]; + } + + /** + * @test + * @dataProvider validFilePathForTypo3tempDirDataProvider + * @param string $filePath + */ + public function writeFileToTypo3tempDirWorksWithValidPath($filePath) + { + $dummyContent = 'Please could you stop the noise, I\'m trying to get some rest from all the unborn chicken voices in my head.'; + + $this->testFilesToDelete[] = $filePath; + + $result = GeneralUtility::writeFileToTypo3tempDir($filePath, $dummyContent); + + $this->assertNull($result); + $this->assertFileExists($filePath); + $this->assertStringEqualsFile($filePath, $dummyContent); + } + /////////////////////////////// // Tests concerning mkdir_deep /////////////////////////////// -- GitLab