From ebdc14d28ee925e465d220b3ad46477b49772252 Mon Sep 17 00:00:00 2001 From: Anja Date: Fri, 16 Mar 2018 10:44:42 +0100 Subject: [PATCH] [TASK] Use ServerRequestInterface Object in File/FileController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Łukasz Uznański Tested-by: Łukasz Uznański Reviewed-by: Christian Kuhn Tested-by: Christian Kuhn --- .../Controller/File/FileController.php | 196 ++++++++++-------- .../Controller/File/FileControllerTest.php | 4 + ...erRequestInterfaceInFileFileController.rst | 41 ++++ .../Php/MethodCallMatcher.php | 14 ++ 4 files changed, 170 insertions(+), 85 deletions(-) create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst diff --git a/typo3/sysext/backend/Classes/Controller/File/FileController.php b/typo3/sysext/backend/Classes/Controller/File/FileController.php index 8d3f3deeffab..9111b8c690f9 100644 --- a/typo3/sysext/backend/Classes/Controller/File/FileController.php +++ b/typo3/sysext/backend/Classes/Controller/File/FileController.php @@ -1,4 +1,5 @@ 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 index df6f65b46467..76d7e006753c 100644 --- 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/typo3/sysext/core/Documentation/Changelog/master/Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst new file mode 100644 index 000000000000..19e90d87b5ea --- /dev/null +++ b/typo3/sysext/core/Documentation/Changelog/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 index 951775096119..df5394602055 100644 --- 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', + ], + ], ];