[BUGFIX] Allow writing temp files in composer mode 46/57046/3
authorBenni Mack <benni@typo3.org>
Fri, 25 May 2018 07:40:49 +0000 (09:40 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Tue, 29 May 2018 23:19:24 +0000 (01:19 +0200)
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>
typo3/sysext/core/Classes/Utility/GeneralUtility.php
typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php

index c57aa9f..fc184f7 100644 (file)
@@ -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;
     }
 
     /**
index b875442..cc12701 100644 (file)
@@ -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
     ///////////////////////////////