[BUGFIX] Move virtual storage 0 back to public path

It was a conceptual mistake to move storage 0 base path outside the document root (public path), because this complete compatibility layer relies on the fact that this storage is public. Moving it outside the public path, implicitly made this storage private, with all logical consequences like eID public URLs. This of course isn't desired and a breaking change of behaviour. With moving the base path back it means, that trying to access this BC layer with paths that are not within the public path, will not work any more. This is fine though, because this never worked before TYPO3 11.5 anyway. The reason why this was made working was EXT:form, which used the storage 0 compat layer for resolving form definitions from extensions, which did not work, when installing extensions into vendor folder (instead of the still current default, where extensions are installed in public folder). Accessing public resources from vendor installed extensions will still work with this change, because the public asset URL will automatically be calculated, but accessing private resources will thrown an exception to clarify that the storage 0 compat layer is not made to make this possible. Also fix form framework to not rely on FAL compatibility layer during form loading. Resolves: #95543 Relates: #95437 Releases: master Change-Id: Ie218dbb8ace9999f2bead77b671ae87f6ad46170 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/71512 Tested-by: core-ci <typo3@b13.com> Tested-by: Helmut Hummel <typo3@helhum.io> Tested-by: Jochen <rothjochen@gmail.com> Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de> Reviewed-by: Helmut Hummel <typo3@helhum.io> Reviewed-by: Jochen <rothjochen@gmail.com> Reviewed-by: Markus Klein <markus.klein@typo3.org> Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
TYPO3 · Oct 11, 2021 · b7d33ab · b7d33ab
1 parent f489cb5
commit b7d33ab
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 18 deletions.
diff --git a/typo3/sysext/core/Classes/Resource/ResourceFactory.php b/typo3/sysext/core/Classes/Resource/ResourceFactory.php
@@ -311,13 +311,21 @@ public function retrieveFileOrFolderObject($input)
                 return $this->getObjectFromCombinedIdentifier($input);
             }
             if ($prefix === 'EXT') {
-                $input = GeneralUtility::getFileAbsFileName($input);
-                if (empty($input)) {
+                $absoluteFilePath = GeneralUtility::getFileAbsFileName($input);
+                if (empty($absoluteFilePath)) {
                     return null;
                 }
+                if (str_starts_with($absoluteFilePath, Environment::getPublicPath())) {
+                    $relativePath = PathUtility::getRelativePath(Environment::getPublicPath() . '/', PathUtility::dirname($absoluteFilePath)) . PathUtility::basename($absoluteFilePath);
+                } else {
+                    try {
+                        $relativePath = PathUtility::getPublicResourceWebPath($input);
+                    } catch (\Throwable $e) {
+                        throw new ResourceDoesNotExistException(sprintf('Tried to access a private resource file "%s" from fallback compatibility storage. This storage only handles public files.', $input), 1633777536);
+                    }
+                }
 
-                $input = PathUtility::getRelativePath(Environment::getProjectPath() . '/', PathUtility::dirname($input)) . PathUtility::basename($input);
-                return $this->getFileObjectFromCombinedIdentifier($input);
+                return $this->getFileObjectFromCombinedIdentifier($relativePath);
             }
             return null;
         }

diff --git a/typo3/sysext/core/Classes/Resource/StorageRepository.php b/typo3/sysext/core/Classes/Resource/StorageRepository.php
@@ -366,7 +366,7 @@ public function getStorageObject($uid, array $recordData = [], &$fileIdentifier
                     'is_default' => false,
                 ];
                 $storageConfiguration = [
-                    'basePath' => Environment::getProjectPath(),
+                    'basePath' => Environment::getPublicPath(),
                     'pathType' => 'absolute',
                 ];
             } elseif ($recordData === [] || (int)$recordData['uid'] !== $uid) {
@@ -439,7 +439,7 @@ protected function initializeLocalStorageCache(): void
     {
         $this->localDriverStorageCache = [
             // implicit legacy storage in project's public path
-            0 => new LocalPath(Environment::getProjectPath(), LocalPath::TYPE_ABSOLUTE),
+            0 => new LocalPath(Environment::getPublicPath(), LocalPath::TYPE_ABSOLUTE),
         ];
         $storageObjects = $this->findByStorageType('Local');
         foreach ($storageObjects as $localStorage) {

diff --git a/typo3/sysext/core/Tests/Unit/Resource/ResourceFactoryTest.php b/typo3/sysext/core/Tests/Unit/Resource/ResourceFactoryTest.php
@@ -18,6 +18,7 @@
 namespace TYPO3\CMS\Core\Tests\Unit\Resource;
 
 use TYPO3\CMS\Core\Core\Environment;
+use TYPO3\CMS\Core\Resource\Exception\ResourceDoesNotExistException;
 use TYPO3\CMS\Core\Resource\ResourceFactory;
 use TYPO3\CMS\Core\Resource\ResourceStorage;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -34,6 +35,8 @@ class ResourceFactoryTest extends UnitTestCase
      */
     protected $resetSingletonInstances = true;
 
+    protected $backupEnvironment = true;
+
     /**
      * @var ResourceFactory
      */
@@ -134,4 +137,79 @@ public function retrieveFileOrFolderObjectReturnsFileIfPathIsGiven(): void
         $this->filesCreated[] = Environment::getPublicPath() . '/' . $filename;
         $this->subject->retrieveFileOrFolderObject($filename);
     }
+
+    /**
+     * @test
+     */
+    public function retrieveFileOrFolderObjectReturnsFileFromPublicFolderWhenProjectRootIsNotPublic(): void
+    {
+        Environment::initialize(
+            Environment::getContext(),
+            true,
+            true,
+            Environment::getProjectPath(),
+            Environment::getPublicPath() . '/typo3temp/public',
+            Environment::getVarPath(),
+            Environment::getConfigPath(),
+            Environment::getCurrentScript(),
+            Environment::isWindows() ? 'WINDOWS' : 'UNIX'
+        );
+
+        GeneralUtility::mkdir_deep(Environment::getPublicPath() . '/typo3temp');
+
+        $this->subject = $this->getAccessibleMock(ResourceFactory::class, ['getFileObjectFromCombinedIdentifier'], [], '', false);
+        $filename = 'typo3temp/var/tests/4711.txt';
+        $this->subject->expects(self::once())
+            ->method('getFileObjectFromCombinedIdentifier')
+            ->with($filename);
+        // Create and prepare test file
+        GeneralUtility::writeFileToTypo3tempDir(Environment::getPublicPath() . '/' . $filename, '42');
+        $this->filesCreated[] = Environment::getPublicPath() . '/' . $filename;
+        $this->subject->retrieveFileOrFolderObject($filename);
+    }
+
+    /**
+     * @test
+     */
+    public function retrieveFileOrFolderObjectReturnsFileFromPublicExtensionResourceWhenExtensionIsNotPublic(): void
+    {
+        Environment::initialize(
+            Environment::getContext(),
+            true,
+            true,
+            Environment::getProjectPath(),
+            Environment::getPublicPath() . '/typo3temp/public',
+            Environment::getVarPath(),
+            Environment::getConfigPath(),
+            Environment::getCurrentScript(),
+            Environment::isWindows() ? 'WINDOWS' : 'UNIX'
+        );
+        $this->subject = $this->getAccessibleMock(ResourceFactory::class, ['getFileObjectFromCombinedIdentifier'], [], '', false);
+        $this->subject->expects(self::once())
+            ->method('getFileObjectFromCombinedIdentifier')
+            ->with('_assets/d25de869aebcd01495d2fe67ad5b0e25/Icons/Extension.svg');
+        // Create and prepare test file
+        $this->subject->retrieveFileOrFolderObject('EXT:core/Resources/Public/Icons/Extension.svg');
+    }
+
+    /**
+     * @test
+     */
+    public function retrieveFileOrFolderObjectThrowsExceptionFromPrivateExtensionResourceWhenExtensionIsNotPublic(): void
+    {
+        Environment::initialize(
+            Environment::getContext(),
+            true,
+            true,
+            Environment::getProjectPath(),
+            Environment::getPublicPath() . '/typo3temp/public',
+            Environment::getVarPath(),
+            Environment::getConfigPath(),
+            Environment::getCurrentScript(),
+            Environment::isWindows() ? 'WINDOWS' : 'UNIX'
+        );
+        $this->subject = $this->getAccessibleMock(ResourceFactory::class, ['getFileObjectFromCombinedIdentifier'], [], '', false);
+        $this->expectException(ResourceDoesNotExistException::class);
+        $this->subject->retrieveFileOrFolderObject('EXT:core/Resources/Private/Templates/PageRenderer.html');
+    }
 }
diff --git a/typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php b/typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php
@@ -92,7 +92,12 @@ public function load(string $persistenceIdentifier): array
             return $yaml;
         }
 
-        $file = $this->retrieveFileByPersistenceIdentifier($persistenceIdentifier);
+        if (PathUtility::isExtensionPath($persistenceIdentifier)) {
+            $this->ensureValidPersistenceIdentifier($persistenceIdentifier);
+            $file = $persistenceIdentifier;
+        } else {
+            $file = $this->retrieveFileByPersistenceIdentifier($persistenceIdentifier);
+        }
 
         try {
             $yaml = $this->yamlSource->load([$file]);
@@ -702,6 +707,7 @@ protected function loadMetaData($persistenceIdentifier): array
             $persistenceIdentifier = $file->getCombinedIdentifier();
             $rawYamlContent = $file->getContents();
         } elseif (PathUtility::isExtensionPath($persistenceIdentifier)) {
+            $this->ensureValidPersistenceIdentifier($persistenceIdentifier);
             $rawYamlContent = false;
             $absoluteFilePath = GeneralUtility::getFileAbsFileName($persistenceIdentifier);
             if ($absoluteFilePath !== '' && file_exists($absoluteFilePath)) {
@@ -786,17 +792,7 @@ protected function generateErrorsIfFormDefinitionIsValidButHasInvalidFileExtensi
      */
     protected function retrieveFileByPersistenceIdentifier(string $persistenceIdentifier): File
     {
-        if (pathinfo($persistenceIdentifier, PATHINFO_EXTENSION) !== 'yaml') {
-            throw new PersistenceManagerException(sprintf('The file "%s" could not be loaded.', $persistenceIdentifier), 1477679819);
-        }
-
-        if (
-            $this->pathIsIntendedAsExtensionPath($persistenceIdentifier)
-            && !$this->isFileWithinAccessibleExtensionFolders($persistenceIdentifier)
-        ) {
-            $message = sprintf('The file "%s" could not be loaded. Please check your configuration option "persistenceManager.allowedExtensionPaths"', $persistenceIdentifier);
-            throw new PersistenceManagerException($message, 1484071985);
-        }
+        $this->ensureValidPersistenceIdentifier($persistenceIdentifier);
 
         try {
             $file = $this->resourceFactory->retrieveFileOrFolderObject($persistenceIdentifier);
@@ -816,6 +812,26 @@ protected function retrieveFileByPersistenceIdentifier(string $persistenceIdenti
         return $file;
     }
 
+    /**
+     * @param string $persistenceIdentifier
+     * @throws PersistenceManagerException
+     * @throws NoSuchFileException
+     */
+    protected function ensureValidPersistenceIdentifier(string $persistenceIdentifier): void
+    {
+        if (pathinfo($persistenceIdentifier, PATHINFO_EXTENSION) !== 'yaml') {
+            throw new PersistenceManagerException(sprintf('The file "%s" could not be loaded.', $persistenceIdentifier), 1477679819);
+        }
+
+        if (
+            $this->pathIsIntendedAsExtensionPath($persistenceIdentifier)
+            && !$this->isFileWithinAccessibleExtensionFolders($persistenceIdentifier)
+        ) {
+            $message = sprintf('The file "%s" could not be loaded. Please check your configuration option "persistenceManager.allowedExtensionPaths"', $persistenceIdentifier);
+            throw new PersistenceManagerException($message, 1484071985);
+        }
+    }
+
     /**
      * @param string $fileName
      * @return bool