[TASK] Use ServerRequestInterface in File/FileUploadController

* deprecate public properties * deprecate public (non-routed) methods * fix rendering of "Overwrite" checkbox Change-Id: Ia5b15ad5fd32ab156cab5f3231bbf0c2b78a617c Resolves: #84326 Releases: master Reviewed-on: https://review.typo3.org/56206 Tested-by: TYPO3com <no-reply@typo3.com> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
TYPO3 · Mar 16, 2018 · a9d92a0 · a9d92a0
1 parent 16b7d84
commit a9d92a0
Show file tree

Hide file tree

Showing 4 changed files with 165 additions and 48 deletions.
diff --git a/typo3/sysext/backend/Classes/Controller/File/FileUploadController.php b/typo3/sysext/backend/Classes/Controller/File/FileUploadController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Controller\File;
 
 /*
@@ -17,8 +18,10 @@
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Template\ModuleTemplate;
+use TYPO3\CMS\Core\Compatibility\PublicPropertyDeprecationTrait;
 use TYPO3\CMS\Core\Http\HtmlResponse;
 use TYPO3\CMS\Core\Imaging\Icon;
+use TYPO3\CMS\Core\Localization\LanguageService;
 use TYPO3\CMS\Core\Resource\Exception\InsufficientFolderAccessPermissionsException;
 use TYPO3\CMS\Core\Resource\ResourceFactory;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -28,33 +31,45 @@
  */
 class FileUploadController
 {
+    use PublicPropertyDeprecationTrait;
+
+    /**
+     * @var array
+     */
+    protected $deprecatedPublicProperties = [
+        'title' => 'Using $title of class FileUploadController from outside is discouraged as this variable is only used for internal storage.',
+        'target' => 'Using $target of class FileUploadController from outside is discouraged as this variable is only used for internal storage.',
+        'returnUrl' => 'Using $returnUrl of class FileUploadController from outside is discouraged as this variable is only used for internal storage.',
+        'content' => 'Using $content of class FileUploadController from outside is discouraged as this variable is only used for internal storage.',
+    ];
+
     /**
      * Name of the filemount
      *
      * @var string
      */
-    public $title;
+    protected $title;
 
     /**
      * Set with the target path inputted in &target
      *
      * @var string
      */
-    public $target;
+    protected $target;
 
     /**
      * Return URL of list module.
      *
      * @var string
      */
-    public $returnUrl;
+    protected $returnUrl;
 
     /**
      * Accumulating content
      *
      * @var string
      */
-    public $content;
+    protected $content;
 
     /**
      * The folder object which is the target directory for the upload
@@ -76,23 +91,63 @@ class FileUploadController
     public function __construct()
     {
         $this->moduleTemplate = GeneralUtility::makeInstance(ModuleTemplate::class);
-        $GLOBALS['SOBE'] = $this;
         $this->getLanguageService()->includeLLFile('EXT:lang/Resources/Private/Language/locallang_misc.xlf');
-        $this->init();
+
+        // @deprecated since v9, will be moved out of __construct() in v10
+        $this->init($GLOBALS['TYPO3_REQUEST']);
+    }
+
+    /**
+     * Processes the request, currently everything is handled and put together via "renderContent()"
+     *
+     * @param ServerRequestInterface $request the current request
+     * @return ResponseInterface the response with the content
+     */
+    public function mainAction(ServerRequestInterface $request): ResponseInterface
+    {
+        $this->renderContent();
+        return new HtmlResponse($this->moduleTemplate->renderContent());
+    }
+
+    /**
+     * Main function, rendering the upload file form fields
+     *
+     * @deprecated since v9, will be removed in v10
+     */
+    public function main()
+    {
+        trigger_error('Method main() will be replaced by protected method renderContent() in v10. Do not call from other extension', E_USER_DEPRECATED);
+        $this->renderContent();
+    }
+
+    /**
+     * This function renders the upload form
+     *
+     * @return string The HTML form as a string, ready for outputting
+     * @deprecated since v9, will be removed in v10
+     */
+    public function renderUploadForm()
+    {
+        trigger_error('Method renderUploadForm() will be replaced by protected method renderUploadFormInternal() in v10. Do not call from other extension', E_USER_DEPRECATED);
+        return $this->renderUploadFormInternal();
     }
 
     /**
      * Initialize
      *
+     * @param ServerRequestInterface $request
      * @throws InsufficientFolderAccessPermissionsException
      */
-    protected function init()
+    protected function init(ServerRequestInterface $request): void
     {
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
+
         /** @var \TYPO3\CMS\Backend\Routing\UriBuilder $uriBuilder */
         $uriBuilder = GeneralUtility::makeInstance(\TYPO3\CMS\Backend\Routing\UriBuilder::class);
         // Initialize GPvars:
-        $this->target = GeneralUtility::_GP('target');
-        $this->returnUrl = GeneralUtility::sanitizeLocalUrl(GeneralUtility::_GP('returnUrl'));
+        $this->target = $parsedBody['target'] ?? $queryParams['target'] ?? null;
+        $this->returnUrl = GeneralUtility::sanitizeLocalUrl($parsedBody['returnUrl'] ?? $queryParams['returnUrl'] ?? '');
         if (!$this->returnUrl) {
             $this->returnUrl = (string)$uriBuilder->buildUriFromRoute('file_list', [
                 'id' => rawurlencode($this->target)
@@ -128,9 +183,9 @@ protected function init()
     }
 
     /**
-     * Main function, rendering the upload file form fields
+     * Render module content
      */
-    public function main()
+    protected function renderContent(): void
     {
         $lang = $this->getLanguageService();
         /** @var \TYPO3\CMS\Backend\Routing\UriBuilder $uriBuilder */
@@ -144,7 +199,7 @@ public function main()
             . '" method="post" id="FileUploadController" name="editform" enctype="multipart/form-data">';
         // Make page header:
         $pageContent .= '<h1>' . $lang->sL('LLL:EXT:lang/Resources/Private/Language/locallang_core.xlf:file_upload.php.pagetitle') . '</h1>';
-        $pageContent .= $this->renderUploadForm();
+        $pageContent .= $this->renderUploadFormInternal();
 
         // Header Buttons
         $buttonBar = $this->moduleTemplate->getDocHeaderComponent()->getButtonBar();
@@ -174,57 +229,43 @@ public function main()
      *
      * @return string The HTML form as a string, ready for outputting
      */
-    public function renderUploadForm()
+    protected function renderUploadFormInternal(): string
     {
         // Make checkbox for "overwrite"
         $content = '
-			<div id="c-override">
-				<p><label for="overwriteExistingFiles"><input type="checkbox" class="checkbox" name="overwriteExistingFiles" id="overwriteExistingFiles" value="replace" /> ' . htmlspecialchars($this->getLanguageService()->getLL('overwriteExistingFiles')) . '</label></p>
-				<p>&nbsp;</p>
-				<p>' . htmlspecialchars($this->getLanguageService()->getLL('uploadMultipleFilesInfo')) . '</p>
-			</div>
-			';
+            <div id="c-override">
+                <p class="checkbox"><label for="overwriteExistingFiles"><input type="checkbox" name="overwriteExistingFiles" id="overwriteExistingFiles" value="replace" /> ' . htmlspecialchars($this->getLanguageService()->getLL('overwriteExistingFiles')) . '</label></p>
+                <p>' . htmlspecialchars($this->getLanguageService()->getLL('uploadMultipleFilesInfo')) . '</p>
+            </div>
+            ';
         // Produce the number of upload-fields needed:
         $content .= '
-			<div id="c-upload">
-		';
+            <div id="c-upload">
+        ';
         // Adding 'size="50" ' for the sake of Mozilla!
         $content .= '
-				<input type="file" multiple="multiple" name="upload_1[]" />
-				<input type="hidden" name="data[upload][1][target]" value="' . htmlspecialchars($this->folderObject->getCombinedIdentifier()) . '" />
-				<input type="hidden" name="data[upload][1][data]" value="1" /><br />
-			';
+                <input type="file" multiple="multiple" name="upload_1[]" />
+                <input type="hidden" name="data[upload][1][target]" value="' . htmlspecialchars($this->folderObject->getCombinedIdentifier()) . '" />
+                <input type="hidden" name="data[upload][1][data]" value="1" /><br />
+            ';
         $content .= '
-			</div>
-		';
+            </div>
+        ';
         // Submit button:
         $content .= '
-			<div id="c-submit">
-				<input type="hidden" name="data[upload][1][redirect]" value="' . $this->returnUrl . '" /><br />
-				<input class="btn btn-default" type="submit" value="' . htmlspecialchars($this->getLanguageService()->sL('LLL:EXT:lang/Resources/Private/Language/locallang_core.xlf:file_upload.php.submit')) . '" />
-			</div>
-		';
-        return $content;
-    }
+            <div id="c-submit">
+                <input type="hidden" name="data[upload][1][redirect]" value="' . $this->returnUrl . '" /><br />
+                <input class="btn btn-default" type="submit" value="' . htmlspecialchars($this->getLanguageService()->sL('LLL:EXT:lang/Resources/Private/Language/locallang_core.xlf:file_upload.php.submit')) . '" />
+            </div>
+        ';
 
-    /**
-     * Processes the request, currently everything is handled and put together via "main()"
-     *
-     * @param ServerRequestInterface $request the current request
-     * @return ResponseInterface the response with the content
-     */
-    public function mainAction(ServerRequestInterface $request): ResponseInterface
-    {
-        $this->main();
-        return new HtmlResponse($this->moduleTemplate->renderContent());
+        return $content;
     }
 
     /**
-     * Returns LanguageService
-     *
-     * @return \TYPO3\CMS\Core\Localization\LanguageService
+     * @return LanguageService
      */
-    protected function getLanguageService()
+    protected function getLanguageService(): LanguageService
     {
         return $GLOBALS['LANG'];
     }

diff --git a/...aster/Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst b/...aster/Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst
@@ -0,0 +1,54 @@
+.. include:: ../../Includes.txt
+
+==============================================================================
+Deprecation: #84326 - Protected methods and properties in FileUploadController
+==============================================================================
+
+See :issue:`84326`
+
+Description
+===========
+
+This file is about third party usage of :php:`TYPO3\CMS\Backend\Controller\File\FileUploadController`.
+
+A series of class properties has been set to protected.
+They will throw deprecation warnings if called public from outside:
+
+* :php:`title`
+* :php:`target`
+* :php:`returnUrl`
+* [not scanned] :php:`content`
+
+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:`renderUploadForm()`
+
+Additionally :php:`$GLOBALS['SOBE']` is not set by the :php:`FileUploadController` constructor anymore.
+
+
+Impact
+======
+
+Calling one of the above methods or accessing one of the above properties on an instance of
+:php:`FileUploadController` 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. The most
+common property and method names like :php:`$content` are not registered and will not be found
+if an extension uses that on an instance of :php:`FileUploadController`.
+
+In general all extensions that set properties or 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
@@ -1941,4 +1941,11 @@
             'Deprecation-84274-ProtectedMethodsAndPropertiesInLoginController.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Controller\File\FileUploadController->renderUploadForm' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst',
+        ],
+    ],
 ];
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyProtectedMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyProtectedMatcher.php
@@ -347,4 +347,19 @@
             'Deprecation-84307-ProtectedMethodsAndPropertiesInNewContentElementController.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Controller\File\FileUploadController->title' => [
+        'restFiles' => [
+            'Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst',
+        ],
+    ],
+    'TYPO3\CMS\Backend\Controller\File\FileUploadController->target' => [
+        'restFiles' => [
+            'Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst',
+        ],
+    ],
+    'TYPO3\CMS\Backend\Controller\File\FileUploadController->returnUrl' => [
+        'restFiles' => [
+            'Deprecation-84326-ProtectedMethodsAndPropertiesInFileUploadController.rst',
+        ],
+    ],
 ];