[TASK] Use ServerRequestInterface Object in File/FileController

Use ServerRequestInterface object introduced earlier throughout the controller instead accessing the global variables directly. Visibility of properties and methods became more restrictive and will report external usage by throwing deprecated errors. Change-Id: I68f220c4b7bb097006118d2b2065ab9f4e554b8f Resolves: #84324 Releases: master Reviewed-on: https://review.typo3.org/56208 Tested-by: TYPO3com <no-reply@typo3.com> Reviewed-by: Łukasz Uznański <l.uznanski@macopedia.pl> Tested-by: Łukasz Uznański <l.uznanski@macopedia.pl> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
TYPO3 · Mar 16, 2018 · ebdc14d · ebdc14d
1 parent 91a19b0
commit ebdc14d
Show file tree

Hide file tree

Showing 4 changed files with 170 additions and 85 deletions.
diff --git a/typo3/sysext/backend/Classes/Controller/File/FileController.php b/typo3/sysext/backend/Classes/Controller/File/FileController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Controller\File;
 
 /*
@@ -16,17 +17,23 @@
 
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use TYPO3\CMS\Backend\Clipboard\Clipboard;
+use TYPO3\CMS\Backend\Routing\UriBuilder;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
+use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Http\HtmlResponse;
 use TYPO3\CMS\Core\Http\JsonResponse;
 use TYPO3\CMS\Core\Http\RedirectResponse;
 use TYPO3\CMS\Core\Imaging\Icon;
 use TYPO3\CMS\Core\Imaging\IconFactory;
 use TYPO3\CMS\Core\Resource\DuplicationBehavior;
+use TYPO3\CMS\Core\Resource\File;
 use TYPO3\CMS\Core\Resource\Folder;
+use TYPO3\CMS\Core\Resource\ProcessedFile;
 use TYPO3\CMS\Core\Resource\ResourceFactory;
 use TYPO3\CMS\Core\Utility\File\ExtendedFileUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\HttpUtility;
 
 /**
  * Gateway for TCE (TYPO3 Core Engine) file-handling through POST forms.
@@ -85,79 +92,8 @@ class FileController
     public function __construct()
     {
         $GLOBALS['SOBE'] = $this;
-        $this->init();
-    }
-
-    /**
-     * Registering incoming data
-     */
-    protected function init()
-    {
-        // Set the GPvars from outside
-        $this->file = GeneralUtility::_GP('data');
-        if ($this->file === null) {
-            // This happens in clipboard mode only
-            $this->redirect = GeneralUtility::sanitizeLocalUrl(GeneralUtility::_GP('redirect'));
-        } else {
-            $mode = key($this->file);
-            $elementKey = key($this->file[$mode]);
-            $this->redirect = GeneralUtility::sanitizeLocalUrl($this->file[$mode][$elementKey]['redirect']);
-        }
-        $this->CB = GeneralUtility::_GP('CB');
-
-        if (isset($this->file['rename'][0]['conflictMode'])) {
-            $conflictMode = $this->file['rename'][0]['conflictMode'];
-            unset($this->file['rename'][0]['conflictMode']);
-            $this->overwriteExistingFiles = DuplicationBehavior::cast($conflictMode);
-        } else {
-            $this->overwriteExistingFiles = DuplicationBehavior::cast(GeneralUtility::_GP('overwriteExistingFiles'));
-        }
-        $this->initClipboard();
-        $this->fileProcessor = GeneralUtility::makeInstance(ExtendedFileUtility::class);
-    }
-
-    /**
-     * Initialize the Clipboard. This will fetch the data about files to paste/delete if such an action has been sent.
-     */
-    public function initClipboard()
-    {
-        if (is_array($this->CB)) {
-            $clipObj = GeneralUtility::makeInstance(\TYPO3\CMS\Backend\Clipboard\Clipboard::class);
-            $clipObj->initializeClipboard();
-            if ($this->CB['paste']) {
-                $clipObj->setCurrentPad($this->CB['pad']);
-                $this->file = $clipObj->makePasteCmdArray_file($this->CB['paste'], $this->file);
-            }
-            if ($this->CB['delete']) {
-                $clipObj->setCurrentPad($this->CB['pad']);
-                $this->file = $clipObj->makeDeleteCmdArray_file($this->file);
-            }
-        }
-    }
-
-    /**
-     * Performing the file admin action:
-     * Initializes the objects, setting permissions, sending data to object.
-     */
-    public function main()
-    {
-        // Initializing:
-        $this->fileProcessor->setActionPermissions();
-        $this->fileProcessor->setExistingFilesConflictMode($this->overwriteExistingFiles);
-        $this->fileProcessor->start($this->file);
-        $this->fileData = $this->fileProcessor->processData();
-    }
-
-    /**
-     * Redirecting the user after the processing has been done.
-     * Might also display error messages directly, if any.
-     */
-    public function finish()
-    {
-        BackendUtility::setUpdateSignal('updateFolderTree');
-        if ($this->redirect) {
-            \TYPO3\CMS\Core\Utility\HttpUtility::redirect($this->redirect);
-        }
+        // @deprecated since v9, will be moved out of __construct() in v10
+        $this->init($GLOBALS['TYPO3_REQUEST']);
     }
 
     /**
@@ -175,7 +111,7 @@ public function mainAction(ServerRequestInterface $request): ResponseInterface
 
         // go and edit the new created file
         if ($request->getParsedBody()['edit'] ?? '') {
-            /** @var \TYPO3\CMS\Core\Resource\File $file */
+            /** @var File $file */
             $file = $this->fileData['newfile'][0];
             $properties = $file->getProperties();
             $urlParameters = [
@@ -184,8 +120,7 @@ public function mainAction(ServerRequestInterface $request): ResponseInterface
             if ($this->redirect) {
                 $urlParameters['returnUrl'] = $this->redirect;
             }
-            /** @var \TYPO3\CMS\Backend\Routing\UriBuilder $uriBuilder */
-            $uriBuilder = GeneralUtility::makeInstance(\TYPO3\CMS\Backend\Routing\UriBuilder::class);
+            $uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
             $this->redirect = (string)$uriBuilder->buildUriFromRoute('file_edit', $urlParameters);
         }
         if ($this->redirect) {
@@ -237,11 +172,10 @@ public function processAjaxRequest(ServerRequestInterface $request): ResponseInt
      */
     public function fileExistsInFolderAction(ServerRequestInterface $request): ResponseInterface
     {
-        $fileName = $request->getParsedBody()['fileName'] ?? $request->getQueryParams()['fileName'];
-        $fileTarget = $request->getParsedBody()['fileTarget'] ?? $request->getQueryParams()['fileTarget'];
+        $fileName = $request->getParsedBody()['fileName'] ?? $request->getQueryParams()['fileName'] ?? null;
+        $fileTarget = $request->getParsedBody()['fileTarget'] ?? $request->getQueryParams()['fileTarget'] ?? null;
 
         $fileFactory = GeneralUtility::makeInstance(ResourceFactory::class);
-        /** @var Folder $fileTargetObject */
         $fileTargetObject = $fileFactory->retrieveFileOrFolderObject($fileTarget);
         $processedFileName = $fileTargetObject->getStorage()->sanitizeFileName($fileName, $fileTargetObject);
 
@@ -252,20 +186,112 @@ public function fileExistsInFolderAction(ServerRequestInterface $request): Respo
         return (new JsonResponse())->setPayload($result);
     }
 
+    /**
+     * Registering incoming data
+     *
+     * @param ServerRequestInterface $request
+     */
+    protected function init(ServerRequestInterface $request): void
+    {
+        // Set the GPvars from outside
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
+        $this->file = $parsedBody['data'] ?? $queryParams['data'] ?? null;
+        if ($this->file === null) {
+            // This happens in clipboard mode only
+            $this->redirect = GeneralUtility::sanitizeLocalUrl($parsedBody['redirect'] ?? $queryParams['redirect'] ?? '');
+        } else {
+            $mode = key($this->file);
+            $elementKey = key($this->file[$mode]);
+            $this->redirect = GeneralUtility::sanitizeLocalUrl($this->file[$mode][$elementKey]['redirect']);
+        }
+        $this->CB = $parsedBody['CB'] ?? $queryParams['CB'] ?? null;
+
+        if (isset($this->file['rename'][0]['conflictMode'])) {
+            $conflictMode = $this->file['rename'][0]['conflictMode'];
+            unset($this->file['rename'][0]['conflictMode']);
+            $this->overwriteExistingFiles = DuplicationBehavior::cast($conflictMode);
+        } else {
+            $this->overwriteExistingFiles = DuplicationBehavior::cast($parsedBody['overwriteExistingFiles'] ?? $queryParams['overwriteExistingFiles'] ?? null);
+        }
+        $this->initClipboard();
+        $this->fileProcessor = GeneralUtility::makeInstance(ExtendedFileUtility::class);
+    }
+
+    /**
+     * Initialize the Clipboard. This will fetch the data about files to paste/delete if such an action has been sent.
+     */
+    public function initClipboard(): void
+    {
+        // Foreign class call? Method will be protected in v10, giving core freedom to move stuff around
+        $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
+        if (end($backtrace)['class'] !== __CLASS__) {
+            // @deprecated since TYPO3 v9, this method will be set to protected in v10
+            trigger_error('Method initClipboard() will be set to protected in v10. Do not call from other extension', E_USER_DEPRECATED);
+        }
+        if (is_array($this->CB)) {
+            $clipObj = GeneralUtility::makeInstance(Clipboard::class);
+            $clipObj->initializeClipboard();
+            if ($this->CB['paste']) {
+                $clipObj->setCurrentPad($this->CB['pad']);
+                $this->file = $clipObj->makePasteCmdArray_file($this->CB['paste'], $this->file);
+            }
+            if ($this->CB['delete']) {
+                $clipObj->setCurrentPad($this->CB['pad']);
+                $this->file = $clipObj->makeDeleteCmdArray_file($this->file);
+            }
+        }
+    }
+
+    /**
+     * Performing the file admin action:
+     * Initializes the objects, setting permissions, sending data to object.
+     */
+    public function main(): void
+    {
+        // Foreign class call? Method will be protected in v10, giving core freedom to move stuff around
+        $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
+        if (end($backtrace)['class'] !== __CLASS__) {
+            // @deprecated since TYPO3 v9, this method will be set to protected in v10
+            trigger_error('Method main() will be set to protected in v10. Do not call from other extension', E_USER_DEPRECATED);
+        }
+        // Initializing:
+        $this->fileProcessor->setActionPermissions();
+        $this->fileProcessor->setExistingFilesConflictMode($this->overwriteExistingFiles);
+        $this->fileProcessor->start($this->file);
+        $this->fileData = $this->fileProcessor->processData();
+    }
+
+    /**
+     * Redirecting the user after the processing has been done.
+     * Might also display error messages directly, if any.
+     *
+     * @deprecated since TYPO3 v9. Will be removed in v10.
+     */
+    public function finish()
+    {
+        trigger_error('Method finish() will be removed in v10. Do not call from other extension', E_USER_DEPRECATED);
+        BackendUtility::setUpdateSignal('updateFolderTree');
+        if ($this->redirect) {
+            HttpUtility::redirect($this->redirect);
+        }
+    }
+
     /**
      * Flatten result value from FileProcessor
      *
      * The value can be a File, Folder or boolean
      *
-     * @param bool|\TYPO3\CMS\Core\Resource\File|\TYPO3\CMS\Core\Resource\Folder $result
+     * @param bool|File|Folder $result
+     *
      * @return bool|string|array
      */
     protected function flattenResultDataValue($result)
     {
-        if ($result instanceof \TYPO3\CMS\Core\Resource\File) {
+        if ($result instanceof File) {
             $thumbUrl = '';
             if (GeneralUtility::inList($GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext'], $result->getExtension())) {
-                $processedFile = $result->process(\TYPO3\CMS\Core\Resource\ProcessedFile::CONTEXT_IMAGEPREVIEW, []);
+                $processedFile = $result->process(ProcessedFile::CONTEXT_IMAGEPREVIEW, []);
                 if ($processedFile) {
                     $thumbUrl = $processedFile->getPublicUrl(true);
                 }
@@ -279,7 +305,7 @@ protected function flattenResultDataValue($result)
                     'thumbUrl' => $thumbUrl
                 ]
             );
-        } elseif ($result instanceof \TYPO3\CMS\Core\Resource\Folder) {
+        } elseif ($result instanceof Folder) {
             $result = $result->getIdentifier();
         }
 
@@ -289,9 +315,9 @@ protected function flattenResultDataValue($result)
     /**
      * Returns the current BE user.
      *
-     * @return \TYPO3\CMS\Core\Authentication\BackendUserAuthentication
+     * @return BackendUserAuthentication
      */
-    protected function getBackendUser()
+    protected function getBackendUser(): BackendUserAuthentication
     {
         return $GLOBALS['BE_USER'];
     }

diff --git a/typo3/sysext/backend/Tests/Unit/Controller/File/FileControllerTest.php b/typo3/sysext/backend/Tests/Unit/Controller/File/FileControllerTest.php
@@ -15,6 +15,7 @@
  */
 
 use Prophecy\Argument;
+use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Controller\File\FileController;
 use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Core\Http\ServerRequest;
@@ -75,6 +76,9 @@ protected function setUp()
         $this->fileResourceMock->expects($this->any())->method('getModificationTime')->will($this->returnValue(123456789));
         $this->fileResourceMock->expects($this->any())->method('getExtension')->will($this->returnValue('html'));
 
+        $serverRequest = $this->prophesize(ServerRequestInterface::class);
+        $GLOBALS['TYPO3_REQUEST'] = $serverRequest->reveal();
+
         $this->request = new ServerRequest();
         $this->response = new Response();
     }

diff --git a/...elog/master/Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst b/...elog/master/Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst
@@ -0,0 +1,41 @@
+.. include:: ../../Includes.txt
+
+=======================================================================
+Deprecation: #84324 - Use ServerRequestInterface in File/FileController
+=======================================================================
+
+See :issue:`84324`
+
+
+Description
+===========
+
+All methods not used as entry points by :php:`TYPO3\CMS\Backend\Http\RouteDispatcher` will be
+removed or set to protected in v10 and throw deprecation warnings if used from a third party:
+
+* [not scanned] :php:`main()`
+* :php: `initClipboard()`
+* :php: `finish()`
+
+Impact
+======
+
+Calling one of the above methods on an instance of
+:php:`FileController` will throw a deprecation warning in v9 and a PHP fatal in v10.
+
+
+Affected Installations
+======================
+
+The extension scanner will find most usages, but may also find some false positives. In general all extensions
+that call methods except :php:`mainAction()` are affected.
+
+
+Migration
+=========
+
+In general, extensions should not instantiate and re-use controllers of the core. Existing
+usages should be rewritten to be free of calls like these.
+
+
+.. index:: Backend, PHP-API, PartiallyScanned
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
@@ -1948,4 +1948,18 @@
             'Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Controller\File\FileController->initClipboard' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst',
+        ],
+    ],
+    'TYPO3\CMS\Backend\Controller\File\FileController->finish' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst',
+        ],
+    ],
 ];