[BUGFIX] Files with unclean path indexed multiple times 99/23199/2
authorStefan Neufeind <typo3.neufeind@speedpartner.de>
Sat, 6 Apr 2013 15:11:52 +0000 (17:11 +0200)
committerStefan Neufeind <typo3.neufeind@speedpartner.de>
Tue, 20 Aug 2013 20:37:42 +0000 (22:37 +0200)
When adding a file or requesting a file by an identifier
cleanup any . and .. in the path before handing off
to the driver so files are not indexed multiple times.

Fixes: #46989
Releases: 6.2, 6.1, 6.0
Change-Id: I4198a8885a6a148e68e1e0f717775f9af976a9ef
Reviewed-on: https://review.typo3.org/23199
Reviewed-by: Stefan Neufeind
Tested-by: Stefan Neufeind
typo3/sysext/core/Classes/Resource/Driver/AbstractHierarchicalFilesystemDriver.php
typo3/sysext/core/Classes/Resource/Driver/LocalDriver.php
typo3/sysext/core/Classes/Resource/ResourceStorage.php
typo3/sysext/core/Classes/Utility/File/BasicFileUtility.php
typo3/sysext/core/Classes/Utility/PathUtility.php
typo3/sysext/core/Tests/Unit/Utility/PathUtilityTest.php

index 41762c4..29b3609 100644 (file)
@@ -49,14 +49,16 @@ abstract class AbstractHierarchicalFilesystemDriver extends AbstractDriver {
         * Makes sure the Path given as parameter is valid
         *
         * @param string $filePath The file path (including the file name!)
-        * @return void
+        * @return string
         * @throws \TYPO3\CMS\Core\Resource\Exception\InvalidPathException
         */
-       protected function checkFilePath($filePath) {
+       protected function canonicalizeAndCheckFilePath($filePath) {
+               $filePath = \TYPO3\CMS\Core\Utility\PathUtility::getCanonicalPath($filePath);
                // filePath must be valid
                if (!$this->isPathValid($filePath)) {
                        throw new \TYPO3\CMS\Core\Resource\Exception\InvalidPathException('File ' . $filePath . ' is not valid (".." and "//" is not allowed in path).', 1320286857);
                }
+               return $filePath;
        }
 }
 
index f9d3b64..e6f25ed 100644 (file)
@@ -221,10 +221,8 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         */
        public function getFileInfoByIdentifier($fileIdentifier) {
                // Makes sure the Path given as parameter is valid
-               $this->checkFilePath($fileIdentifier);
-               $dirPath = \TYPO3\CMS\Core\Utility\GeneralUtility::fixWindowsFilePath(
-                       PathUtility::dirname($fileIdentifier)
-               );
+               $fileIdentifier = $this->canonicalizeAndCheckFilePath($fileIdentifier);
+               $dirPath = PathUtility::dirname($fileIdentifier);
                if ($dirPath === '' || $dirPath === '.') {
                        $dirPath = '/';
                } elseif ($dirPath !== '/') {
index 2490f0a..b9dee08 100644 (file)
@@ -705,6 +705,7 @@ class ResourceStorage {
         * @return FileInterface
         */
        public function addFile($localFilePath, Folder $targetFolder, $fileName = '', $conflictMode = 'changeName') {
+               $localFilePath = PathUtility::getCanonicalPath($localFilePath);
                // TODO check permissions (write on target, upload, ...)
                if (!file_exists($localFilePath)) {
                        throw new \InvalidArgumentException('File "' . $localFilePath . '" does not exist.', 1319552745);
index 18c1e84..b883dc9 100644 (file)
@@ -27,6 +27,9 @@ namespace TYPO3\CMS\Core\Utility\File;
  *  This copyright notice MUST APPEAR in all copies of the script!
  ***************************************************************/
 
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\PathUtility;
+
 /**
  * Contains class with basic file management functions
  *
@@ -132,11 +135,11 @@ class BasicFileUtility {
         * @deprecated All methods in this class should not be used anymore since TYPO3 6.0. Please use corresponding TYPO3\\CMS\\Core\\Resource\\ResourceStorage (fetched via BE_USERS->getFileStorages()), as all functions should be found there (in a cleaner manner).
         */
        public function init($mounts, $f_ext) {
-               \TYPO3\CMS\Core\Utility\GeneralUtility::logDeprecatedFunction();
-               $this->f_ext['webspace']['allow'] = \TYPO3\CMS\Core\Utility\GeneralUtility::uniqueList(strtolower($f_ext['webspace']['allow']));
-               $this->f_ext['webspace']['deny'] = \TYPO3\CMS\Core\Utility\GeneralUtility::uniqueList(strtolower($f_ext['webspace']['deny']));
-               $this->f_ext['ftpspace']['allow'] = \TYPO3\CMS\Core\Utility\GeneralUtility::uniqueList(strtolower($f_ext['ftpspace']['allow']));
-               $this->f_ext['ftpspace']['deny'] = \TYPO3\CMS\Core\Utility\GeneralUtility::uniqueList(strtolower($f_ext['ftpspace']['deny']));
+               GeneralUtility::logDeprecatedFunction('All methods in this class should not be used anymore since TYPO3 6.0. Please use corresponding TYPO3\\CMS\\Core\\Resource\\ResourceStorage (fetched via BE_USERS->getFileStorages()), as all functions should be found there (in a cleaner manner).');
+               $this->f_ext['webspace']['allow'] = GeneralUtility::uniqueList(strtolower($f_ext['webspace']['allow']));
+               $this->f_ext['webspace']['deny'] = GeneralUtility::uniqueList(strtolower($f_ext['webspace']['deny']));
+               $this->f_ext['ftpspace']['allow'] = GeneralUtility::uniqueList(strtolower($f_ext['ftpspace']['allow']));
+               $this->f_ext['ftpspace']['deny'] = GeneralUtility::uniqueList(strtolower($f_ext['ftpspace']['deny']));
 
                $this->mounts = $mounts;
                $this->webPath = \TYPO3\CMS\Core\Utility\GeneralUtility::getIndpEnv('TYPO3_DOCUMENT_ROOT');
@@ -287,7 +290,7 @@ class BasicFileUtility {
        public function is_directory($theDir) {
                // @todo: should go into the LocalDriver in a protected way (not important to the outside world)
                if ($this->isPathValid($theDir)) {
-                       $theDir = $this->cleanDirectoryName($theDir);
+                       $theDir = PathUtility::getCanonicalPath($theDir);
                        if (@is_dir($theDir)) {
                                return $theDir;
                        }
@@ -444,15 +447,15 @@ class BasicFileUtility {
         *
         *********************/
        /**
-        * Removes all dots, slashes and spaces after a path...
+        * Removes all dots, slashes and spaces after a path
         *
-        * @param       string          Input string
-        * @return      string          Output string
-        * @todo Define visibility
+        * @param string $theDir Input string
+        * @return string Output string
+        * @deprecated since 6.1, will be removed in two versions, use \TYPO3\CMS\Core\Utility\PathUtility::getCanonicalPath() instead
         */
        public function cleanDirectoryName($theDir) {
-               // @todo: should go into the LocalDriver in a protected way (not important to the outside world)
-               return preg_replace('/[\\/\\. ]*$/', '', $this->rmDoubleSlash($theDir));
+               GeneralUtility::logDeprecatedFunction();
+               return PathUtility::getCanonicalPath($theDir);
        }
 
        /**
index 3ec139c..82921d5 100644 (file)
@@ -129,7 +129,6 @@ class PathUtility {
                return rtrim($path, $separator) . $separator;
        }
 
-
        /**
         * Returns trailing name component of path
         * Since basename() is locale dependent we need to access
@@ -193,6 +192,106 @@ class PathUtility {
                setlocale(LC_CTYPE, $currentLocale);
                return $pathinfo;
        }
+
+       /**
+        * Checks if the $path is absolute or relative (detecting either '/' or 'x:/' as first part of string) and returns TRUE if so.
+        *
+        * @param string $path File path to evaluate
+        * @return boolean
+        */
+       static public function isAbsolutePath($path) {
+               // On Windows also a path starting with a drive letter is absolute: X:/
+               if (static::isWindows() && substr($path, 1, 2) === ':/') {
+                       return TRUE;
+               }
+               // Path starting with a / is always absolute, on every system
+               return substr($path, 0, 1) === '/';
+       }
+
+
+       /*********************
+        *
+        * Cleaning methods
+        *
+        *********************/
+       /**
+        * Resolves all dots, slashes and removes spaces after or before a path...
+        *
+        * @param string $path Input string
+        * @return string Canonical path, always without trailing slash
+        */
+       static public function getCanonicalPath($path) {
+               // Replace backslashes with slashes to work with Windows paths if given
+               $path = trim(str_replace('\\', '/', $path));
+
+               // TODO: do we really need this? Probably only in testing context for vfs?
+               $protocol = '';
+               if (strpos($path, '://') !== FALSE) {
+                       list($protocol, $path) = explode('://', $path);
+                       $protocol .= '://';
+               }
+
+               $absolutePathPrefix = '';
+               if (static::isAbsolutePath($path)) {
+                       if (static::isWindows() && substr($path, 1, 2) === ':/') {
+                               $absolutePathPrefix = substr($path, 0, 3);
+                               $path = substr($path, 3);
+                       } else {
+                               $path = ltrim($path, '/');
+                               $absolutePathPrefix = '/';
+                       }
+               }
+
+               $theDirParts = explode('/', $path);
+               $theDirPartsCount = count($theDirParts);
+               for ($partCount = 0; $partCount < $theDirPartsCount; $partCount++) {
+                       // double-slashes in path: remove element
+                       if ($theDirParts[$partCount] === '') {
+                               array_splice($theDirParts, $partCount, 1);
+                               $partCount--;
+                               $theDirPartsCount--;
+                       }
+                       // "." in path: remove element
+                       if ($theDirParts[$partCount] === '.') {
+                               array_splice($theDirParts, $partCount, 1);
+                               $partCount--;
+                               $theDirPartsCount--;
+                       }
+                       // ".." in path:
+                       if ($theDirParts[$partCount] === '..') {
+                               if ($partCount >= 1) {
+                                       // Rremove this and previous element
+                                       array_splice($theDirParts, $partCount - 1, 2);
+                                       $partCount -= 2;
+                                       $theDirPartsCount -= 2;
+                               } elseif ($absolutePathPrefix) {
+                                       // can't go higher than root dir
+                                       // simply remove this part and continue
+                                       array_splice($theDirParts, $partCount, 1);
+                                       $partCount--;
+                                       $theDirPartsCount--;
+                               }
+                       }
+               }
+
+               return $protocol . $absolutePathPrefix . implode('/', $theDirParts);
+       }
+
+       /*********************
+        *
+        * Helper methods
+        *
+        *********************/
+
+       /**
+        * Wrapper method to be able to test windows path transformation on other systems
+        *
+        * @return bool
+        */
+       static protected function isWindows() {
+               return TYPO3_OS === 'WIN';
+       }
+
 }
 
 
index cb9410b..5d416b8 100644 (file)
@@ -26,6 +26,7 @@ namespace TYPO3\CMS\Core\Tests\Unit\Utility;
  *
  * This copyright notice MUST APPEAR in all copies of the script!
  ***************************************************************/
+use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
  * Testcase for class \TYPO3\CMS\Core\Utility\PathUtility
@@ -200,6 +201,123 @@ class PathUtilityTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                );
        }
 
+       /**
+        * Data provider for getCanonicalPathCorrectlyCleansPath
+        *
+        * @return array
+        */
+       public function getCanonicalPathCorrectlyCleansPathDataProvider() {
+               return array(
+                       'removes single-dot-elements' => array(
+                               'abc/./def/././ghi',
+                               'abc/def/ghi'
+                       ),
+                       'removes ./ at beginning' => array(
+                               './abc/def/ghi',
+                               'abc/def/ghi'
+                       ),
+                       'removes double-slashes' => array(
+                               'abc//def/ghi',
+                               'abc/def/ghi'
+                       ),
+                       'removes double-slashes from front, but keeps absolute path' => array(
+                               '//abc/def/ghi',
+                               '/abc/def/ghi'
+                       ),
+                       'makes double-dot-elements go one level higher, test #1' => array(
+                               'abc/def/ghi/../..',
+                               'abc'
+                       ),
+                       'makes double-dot-elements go one level higher, test #2' => array(
+                               'abc/def/ghi/../123/456/..',
+                               'abc/def/123'
+                       ),
+                       'makes double-dot-elements go one level higher, test #3' => array(
+                               'abc/../../def/ghi',
+                               '../def/ghi'
+                       ),
+                       'makes double-dot-elements go one level higher, test #4' => array(
+                               'abc/def/ghi//../123/456/..',
+                               'abc/def/123'
+                       ),
+                       'truncates slash at the end' => array(
+                               'abc/def/ghi/',
+                               'abc/def/ghi'
+                       ),
+                       'keeps slash in front of absolute paths' => array(
+                               '/abc/def/ghi',
+                               '/abc/def/ghi'
+                       ),
+                       'keeps slash in front of absolute paths even if double-dot-elements want to go higher' => array(
+                               '/abc/../../def/ghi',
+                               '/def/ghi'
+                       ),
+                       'works with EXT-syntax-paths' => array(
+                               'EXT:abc/def/ghi/',
+                               'EXT:abc/def/ghi'
+                       ),
+                       'truncates ending slash with space' => array(
+                               'abc/def/ ',
+                               'abc/def'
+                       ),
+                       'truncates ending space' => array(
+                               'abc/def ',
+                               'abc/def'
+                       ),
+                       'truncates ending dot' => array(
+                               'abc/def/.',
+                               'abc/def'
+                       ),
+                       'does not truncates ending dot if part of name' => array(
+                               'abc/def.',
+                               'abc/def.'
+                       ),
+                       'protocol is not removed' => array(
+                               'vfs://def/../text.txt',
+                               'vfs://text.txt'
+                       ),
+                       'works with filenames' => array(
+                               '/def/../text.txt',
+                               '/text.txt'
+                       ),
+                       'absolute windwos path' => array(
+                               'C:\def\..\..\test.txt',
+                               'C:/test.txt'
+                       ),
+                       'double slashaes' => array(
+                               'abc//def',
+                               'abc/def'
+                       ),
+                       'multiple slashes' => array(
+                               'abc///////def',
+                               'abc/def'
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @dataProvider getCanonicalPathCorrectlyCleansPathDataProvider
+        */
+       public function getCanonicalPathCorrectlyCleansPath($inputName, $expectedResult) {
+               $className = uniqid('PathUtilityFixture');
+               $fixtureClassString = '
+                       namespace ' . ltrim(__NAMESPACE__, '\\') . ';
+                       class ' . $className . ' extends \\TYPO3\\CMS\\Core\\Utility\\PathUtility {
+                               static public function isWindows() {
+                                       return TRUE;
+                               }
+                       }
+               ';
+               eval($fixtureClassString);
+               $fullyQualifiedClassName = __NAMESPACE__ . '\\' . $className;
+
+               $this->assertEquals(
+                       $expectedResult,
+                       $fullyQualifiedClassName::getCanonicalPath($inputName)
+               );
+       }
+
 }
 
 ?>
\ No newline at end of file