[TASK] CleanUp canonicalizing identifiers in FAL-drivers 82/25382/3
authorSteffen Ritter <info@rs-websystems.de>
Wed, 13 Nov 2013 14:45:29 +0000 (15:45 +0100)
committerSteffen Ritter <info@rs-websystems.de>
Thu, 21 Nov 2013 09:35:28 +0000 (10:35 +0100)
In https://review.typo3.org/23398 the possibility was
created, to use case-sensitive fileIdentifiers in
database collations which are not case sensitive. To
do so a new sanitiation of file-identifiers has been
introduced into the drivers.

During the patch-sets a new function has been created
while there would have been already some functions
perfectly fitting that task.

Introducing this new functionality in the old ones
showed, that these are used for non-identifier
path (absolute external), too which created problems.
Therefore this has been split out.

Finally the getAbsolutePath and variants manually
concatenating the basePath have been revised, and
duplicate calls to the canonicalize functions have
been removed.

Releases: 6.2
Resolves: #53604
Change-Id: I415cce898298e9cdaadf7ef4310b15a68c3bb5c6
Reviewed-on: https://review.typo3.org/25382
Reviewed-by: Frans Saris
Tested-by: Frans Saris
Reviewed-by: Philipp Gampe
Tested-by: Philipp Gampe
Reviewed-by: Steffen Ritter
Tested-by: Steffen Ritter
typo3/sysext/core/Classes/Resource/Driver/AbstractDriver.php
typo3/sysext/core/Classes/Resource/Driver/AbstractHierarchicalFilesystemDriver.php
typo3/sysext/core/Classes/Resource/Driver/LocalDriver.php

index 7d679f0..5e0af58 100644 (file)
@@ -262,21 +262,7 @@ abstract class AbstractDriver {
         */
        public function hashIdentifier($identifier) {
                $identifier = $this->canonicalizeAndCheckFileIdentifier($identifier);
-               return sha1($this->getUniqueIdentifier($identifier));
-       }
-
-       /**
-        * Calculate unique identifier by taking the case sensitivity of the file system
-        * into account
-        *
-        * @param string $identifier
-        * @return string
-        */
-       protected function getUniqueIdentifier($identifier) {
-               if (!$this->isCaseSensitiveFileSystem()) {
-                       $identifier = strtolower($identifier);
-               }
-               return $identifier;
+               return sha1($identifier);
        }
 
        /**
@@ -776,16 +762,24 @@ abstract class AbstractDriver {
        /**
         * Makes sure the path given as parameter is valid
         *
-        * @param string $fileIdentifier The file path (most times filePath)
+        * @param string $filePath The file path (most times filePath)
+        * @return string
+        */
+       abstract protected function canonicalizeAndCheckFilePath($filePath);
+
+       /**
+        * Makes sure the identifier given as parameter is valid
+        *
+        * @param string $fileIdentifier The file Identifier
         * @return string
         * @throws \TYPO3\CMS\Core\Resource\Exception\InvalidPathException
         */
        abstract protected function canonicalizeAndCheckFileIdentifier($fileIdentifier);
 
        /**
-        * Makes sure the path given as parameter is valid
+        * Makes sure the identifier given as parameter is valid
         *
-        * @param string $folderIdentifier The folder path (most times filePath)
+        * @param string $folderIdentifier The folder identifier
         * @return string
         */
        abstract protected function canonicalizeAndCheckFolderIdentifier($folderIdentifier);
index 197add8..b681541 100644 (file)
@@ -52,7 +52,7 @@ abstract class AbstractHierarchicalFilesystemDriver extends AbstractDriver {
         * @return string
         * @throws \TYPO3\CMS\Core\Resource\Exception\InvalidPathException
         */
-       protected function canonicalizeAndCheckFileIdentifier($filePath) {
+       protected function canonicalizeAndCheckFilePath($filePath) {
                $filePath = \TYPO3\CMS\Core\Utility\PathUtility::getCanonicalPath($filePath);
 
                // filePath must be valid
@@ -65,6 +65,21 @@ abstract class AbstractHierarchicalFilesystemDriver extends AbstractDriver {
        /**
         * Makes sure the Path given as parameter is valid
         *
+        * @param string $fileIdentifier The file path (including the file name!)
+        * @return string
+        */
+       protected function canonicalizeAndCheckFileIdentifier($fileIdentifier) {
+               $fileIdentifier = $this->canonicalizeAndCheckFilePath($fileIdentifier);
+               $fileIdentifier = '/' . ltrim($fileIdentifier, '/');
+               if (!$this->isCaseSensitiveFileSystem()) {
+                       $fileIdentifier = strtolower($fileIdentifier);
+               }
+               return $fileIdentifier;
+       }
+
+       /**
+        * Makes sure the Path given as parameter is valid
+        *
         * @param string $folderPath The file path (including the file name!)
         * @return string
         */
index a195e83..84b86c1 100644 (file)
@@ -227,15 +227,10 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @throws \InvalidArgumentException
         */
        public function getFileInfoByIdentifier($fileIdentifier, array $propertiesToExtract = array()) {
-               // Makes sure the Path given as parameter is valid
-               $fileIdentifier = $this->canonicalizeAndCheckFileIdentifier($fileIdentifier);
                $dirPath = PathUtility::dirname($fileIdentifier);
-               if ($dirPath === '' || $dirPath === '.') {
-                       $dirPath = '/';
-               } elseif ($dirPath !== '/') {
-                       $dirPath = '/' . trim($dirPath, '/') . '/';
-               }
-               $absoluteFilePath = $this->absoluteBasePath . ltrim($fileIdentifier, '/');
+               $dirPath = $this->canonicalizeAndCheckFolderIdentifier($dirPath);
+
+               $absoluteFilePath = $this->getAbsolutePath($fileIdentifier);
                // don't use $this->fileExists() because we need the absolute path to the file anyways, so we can directly
                // use PHP's filesystem method.
                if (!file_exists($absoluteFilePath)) {
@@ -420,7 +415,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @return array
         */
        protected function getFileList_itemCallback($fileName, $path, array $fileRow = array()) {
-               $filePath = $this->getAbsolutePath($this->canonicalizeAndCheckFileIdentifier($path . $fileName));
+               $filePath = $this->getAbsolutePath($path . $fileName);
                if (!is_file($filePath)) {
                        return array('', array());
                }
@@ -442,7 +437,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @return array
         */
        protected function getFolderList_itemCallback($folderName, $parentPath, array $folderRow = array()) {
-               $folderPath = $this->getAbsolutePath($this->canonicalizeAndCheckFileIdentifier($parentPath . $folderName));
+               $folderPath = $this->getAbsolutePath($parentPath . $folderName);
 
                if (!is_dir($folderPath)) {
                        return array('', array());
@@ -536,7 +531,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @throws \InvalidArgumentException
         */
        public function getSpecificFileInformation($filePath, $containerPath, $property) {
-               $identifier = $this->getUniqueIdentifier($containerPath . PathUtility::basename($filePath));
+               $identifier = $this->canonicalizeAndCheckFileIdentifier($containerPath . PathUtility::basename($filePath));
 
                switch ($property) {
                        case 'size':
@@ -573,12 +568,12 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         */
        protected function extractFolderInformation($folderPath, $containerPath) {
                $folderName = PathUtility::basename($folderPath);
-               $identifier = $this->getUniqueIdentifier($containerPath . PathUtility::basename($folderName));
+               $identifier = $this->canonicalizeAndCheckFolderIdentifier($containerPath . $folderName);
                $folderInformation = array(
                        'ctime' => filectime($folderPath),
                        'mtime' => filemtime($folderPath),
                        'name' => $folderName,
-                       'identifier' => $identifier . '/',
+                       'identifier' => $identifier,
                        'storage' => $this->storage->getUid()
                );
                return $folderInformation;
@@ -602,10 +597,10 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         */
        public function getAbsolutePath($file) {
                if ($file instanceof FileInterface) {
-                       $path = $this->absoluteBasePath . $this->canonicalizeAndCheckFileIdentifier(ltrim($file->getIdentifier(), '/'));
+                       $path = $this->absoluteBasePath . ltrim($this->canonicalizeAndCheckFileIdentifier($file->getIdentifier()), '/');
                } elseif ($file instanceof Folder) {
                        // We can assume a trailing slash here because it is added by the folder object on construction.
-                       $path = $this->absoluteBasePath . $this->canonicalizeAndCheckFolderIdentifier(ltrim($file->getIdentifier(), '/'));
+                       $path = $this->absoluteBasePath . ltrim($this->canonicalizeAndCheckFolderIdentifier($file->getIdentifier()), '/');
                } elseif (is_string($file)) {
                        $path = $this->absoluteBasePath . ltrim($file, '/');
                } else {
@@ -671,7 +666,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @throws \InvalidArgumentException
         */
        public function addFile($localFilePath, Folder $targetFolder, $fileName, \TYPO3\CMS\Core\Resource\AbstractFile $updateFileObject = NULL) {
-               $localFilePath = $this->canonicalizeAndCheckFileIdentifier($localFilePath);
+               $localFilePath = $this->canonicalizeAndCheckFilePath($localFilePath);
                // as for the "virtual storage" for backwards-compatibility, this check always fails, as the file probably lies under PATH_site
                // thus, it is not checked here
                if (GeneralUtility::isFirstPartOfStr($localFilePath, $this->absoluteBasePath) && $this->storage->getUid() > 0) {
@@ -708,8 +703,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @return boolean
         */
        public function resourceExists($identifier) {
-               $identifier = $this->canonicalizeAndCheckFileIdentifier(ltrim($identifier, '/'));
-               $absoluteResourcePath = $this->absoluteBasePath . $identifier;
+               $absoluteResourcePath = $this->getAbsolutePath($identifier);
                return file_exists($absoluteResourcePath);
        }
 
@@ -720,8 +714,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @return boolean
         */
        public function fileExists($identifier) {
-               $identifier = $this->canonicalizeAndCheckFileIdentifier(ltrim($identifier, '/'));
-               $absoluteFilePath = $this->absoluteBasePath . $identifier;
+               $absoluteFilePath = $this->getAbsolutePath($identifier);
                return is_file($absoluteFilePath);
        }
 
@@ -744,8 +737,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @return boolean
         */
        public function folderExists($identifier) {
-               $identifier = $this->canonicalizeAndCheckFileIdentifier(ltrim($identifier, '/'));
-               $absoluteFilePath = $this->absoluteBasePath . $identifier;
+               $absoluteFilePath = $this->getAbsolutePath($identifier);
                return is_dir($absoluteFilePath);
        }
 
@@ -758,7 +750,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         */
        public function folderExistsInFolder($folderName, Folder $folder) {
                $identifier = $folder->getIdentifier() . $folderName;
-               $identifier = $this->canonicalizeAndCheckFileIdentifier($identifier);
+               $identifier = $this->canonicalizeAndCheckFolderIdentifier($identifier);
                return $this->folderExists($identifier);
        }
 
@@ -805,7 +797,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         */
        public function addFileRaw($localFilePath, Folder $targetFolder, $targetFileName) {
                $fileIdentifier = $targetFolder->getIdentifier() . $targetFileName;
-               $absoluteFilePath = $this->absoluteBasePath . $fileIdentifier;
+               $absoluteFilePath = $this->getAbsolutePath($fileIdentifier);
                $result = copy($localFilePath, $absoluteFilePath);
                if ($result === FALSE || !file_exists($absoluteFilePath)) {
                        throw new \RuntimeException('Adding file ' . $localFilePath . ' at ' . $fileIdentifier . ' failed.');
@@ -824,9 +816,8 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         * @throws \RuntimeException
         */
        public function deleteFileRaw($identifier) {
-               $identifier = $this->canonicalizeAndCheckFileIdentifier(ltrim($identifier, '/'));
+               $targetPath = $this->getAbsolutePath($identifier);
 
-               $targetPath = $this->absoluteBasePath . $identifier;
                $result = unlink($targetPath);
                if ($result === FALSE || file_exists($targetPath)) {
                        throw new \RuntimeException('Deleting file ' . $identifier . ' failed.', 1320381534);
@@ -847,7 +838,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
        public function copyFileWithinStorage(FileInterface $file, Folder $targetFolder, $fileName) {
                // TODO add unit test
                $sourcePath = $this->getAbsolutePath($file);
-               $targetPath = ltrim($targetFolder->getIdentifier(), '/') . $fileName;
+               $targetPath = $targetFolder->getIdentifier() . $fileName;
                $targetPath = $this->canonicalizeAndCheckFileIdentifier($targetPath);
 
                copy($sourcePath, $this->absoluteBasePath . $targetPath);
@@ -869,7 +860,7 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
                $sourcePath = $this->getAbsolutePath($file);
                $targetIdentifier = $targetFolder->getIdentifier() . $fileName;
                $targetIdentifier = $this->canonicalizeAndCheckFileIdentifier($targetIdentifier);
-               $result = rename($sourcePath, $this->absoluteBasePath . $targetIdentifier);
+               $result = rename($sourcePath, $this->getAbsolutePath($targetIdentifier));
                if ($result === FALSE) {
                        throw new \RuntimeException('Moving file ' . $sourcePath . ' to ' . $targetIdentifier . ' failed.', 1315314712);
                }
@@ -1020,15 +1011,15 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
         */
        public function renameFile(FileInterface $file, $newName) {
                // Makes sure the Path given as parameter is valid
-               $newName = $this->canonicalizeAndCheckFileIdentifier($newName);
                $newName = $this->sanitizeFileName($newName);
                $newIdentifier = rtrim(GeneralUtility::fixWindowsFilePath(PathUtility::dirname($file->getIdentifier())), '/') . '/' . $newName;
+               $newIdentifier = $this->canonicalizeAndCheckFileIdentifier($newIdentifier);
                // The target should not exist already
                if ($this->fileExists($newIdentifier)) {
                        throw new \TYPO3\CMS\Core\Resource\Exception\ExistingTargetFileNameException('The target file already exists.', 1320291063);
                }
                $sourcePath = $this->getAbsolutePath($file);
-               $targetPath = $this->absoluteBasePath . '/' . ltrim($newIdentifier, '/');
+               $targetPath = $this->getAbsolutePath($newIdentifier);
                $result = rename($sourcePath, $targetPath);
                if ($result === FALSE) {
                        throw new \RuntimeException('Renaming file ' . $sourcePath . ' to ' . $targetPath . ' failed.', 1320375115);
@@ -1216,8 +1207,9 @@ class LocalDriver extends AbstractHierarchicalFilesystemDriver {
                        throw new \TYPO3\CMS\Core\Resource\Exception\InvalidFileNameException('Invalid characters in fileName "' . $fileName . '"', 1320572272);
                }
                $filePath = $parentFolder->getIdentifier() . $this->sanitizeFileName(ltrim($fileName, '/'));
-               $result = touch($this->absoluteBasePath . $filePath);
-               GeneralUtility::fixPermissions($this->absoluteBasePath . ltrim($filePath, '/'));
+               $absoluteFilePath = $this->getAbsolutePath($filePath);
+               $result = touch($absoluteFilePath);
+               GeneralUtility::fixPermissions($absoluteFilePath);
                clearstatcache();
                if ($result !== TRUE) {
                        throw new \RuntimeException('Creating file ' . $filePath . ' failed.', 1320569854);