[TASK] Use ServerRequestInterface in SimpleDataHandlerController

* deprecate public properties
* deprecate public (non-routed) methods
* replace usages of _GP, getIndpEnv

Resolves: #84374
Releases: master
Change-Id: I00aa5a6db6a4fbe6373d6e30c6121dcc4cd67e48
Reviewed-on: https://review.typo3.org/56253
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>

typo3/sysext/backend/Classes/Controller/SimpleDataHandlerController.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Controller;
 
 /*
@@ -18,6 +19,8 @@
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Clipboard\Clipboard;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
+use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
+use TYPO3\CMS\Core\Compatibility\PublicPropertyDeprecationTrait;
 use TYPO3\CMS\Core\DataHandling\DataHandler;
 use TYPO3\CMS\Core\Http\HtmlResponse;
 use TYPO3\CMS\Core\Http\JsonResponse;
@@ -37,86 +40,180 @@
  */
 class SimpleDataHandlerController
 {
+    use PublicPropertyDeprecationTrait;
+
+    /**
+     * Properties which have been moved to protected status from public
+     *
+     * @var array
+     */
+    protected $deprecatedPublicProperties = [
+        'flags' => 'Using $flags of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'data' => 'Using $data of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'cmd' => 'Using $cmd of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'mirror' => 'Using $mirror of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'cacheCmd' => 'Using $cacheCmd of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'redirect' => 'Using $redirect of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'CB' => 'Using $CB of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+        'tce' => 'Using $tce of class SimpleDataHandlerController from the outside is discouraged, as this variable is only used for internal storage.',
+    ];
+
     /**
      * Array. Accepts options to be set in TCE object. Currently it supports "reverseOrder" (bool).
      *
      * @var array
      */
-    public $flags;
+    protected $flags;
 
     /**
      * Data array on the form [tablename][uid][fieldname] = value
      *
      * @var array
      */
-    public $data;
+    protected $data;
 
     /**
      * Command array on the form [tablename][uid][command] = value.
      * This array may get additional data set internally based on clipboard commands send in CB var!
      *
      * @var array
      */
-    public $cmd;
+    protected $cmd;
 
     /**
      * Array passed to ->setMirror.
      *
      * @var array
      */
-    public $mirror;
+    protected $mirror;
 
     /**
      * Cache command sent to ->clear_cacheCmd
      *
      * @var string
      */
-    public $cacheCmd;
+    protected $cacheCmd;
 
     /**
      * Redirect URL. Script will redirect to this location after performing operations (unless errors has occurred)
      *
      * @var string
      */
-    public $redirect;
+    protected $redirect;
 
     /**
      * Clipboard command array. May trigger changes in "cmd"
      *
      * @var array
      */
-    public $CB;
+    protected $CB;
 
     /**
      * TYPO3 Core Engine
      *
      * @var \TYPO3\CMS\Core\DataHandling\DataHandler
      */
-    public $tce;
+    protected $tce;
 
     /**
      * Constructor
      */
     public function __construct()
     {
+        // @deprecated since v9, will be obsolete in v10 with removal of init()
+        $request = $GLOBALS['TYPO3_REQUEST'];
         $GLOBALS['SOBE'] = $this;
-        $this->init();
+        // @deprecated since v9, will be moved out of __construct() in v10
+        $this->init($request);
+    }
+
+    /**
+     * Injects the request object for the current request or subrequest
+     * As this controller goes only through the processRequest() method, it just redirects to the given URL afterwards.
+     *
+     * @param ServerRequestInterface $request the current request
+     * @return ResponseInterface the response with the content
+     */
+    public function mainAction(ServerRequestInterface $request): ResponseInterface
+    {
+        $this->initializeClipboard();
+        $this->processRequest();
+
+        // Write errors to flash message queue
+        $this->tce->printLogErrorMessages();
+        if ($this->redirect) {
+            return new RedirectResponse(GeneralUtility::locationHeaderUrl($this->redirect), 303);
+        }
+        return new HtmlResponse('');
+    }
+
+    /**
+     * Processes all AJAX calls and returns a JSON formatted string
+     *
+     * @param ServerRequestInterface $request
+     * @return ResponseInterface
+     */
+    public function processAjaxRequest(ServerRequestInterface $request): ResponseInterface
+    {
+        // do the regular / main logic
+        $this->initializeClipboard();
+        $this->processRequest();
+
+        /** @var \TYPO3\CMS\Core\Messaging\FlashMessageService $flashMessageService */
+        $flashMessageService = GeneralUtility::makeInstance(FlashMessageService::class);
+
+        $content = [
+            'redirect' => $this->redirect,
+            'messages' => [],
+            'hasErrors' => false
+        ];
+
+        // Prints errors (= write them to the message queue)
+        $this->tce->printLogErrorMessages();
+
+        $messages = $flashMessageService->getMessageQueueByIdentifier()->getAllMessagesAndFlush();
+        if (!empty($messages)) {
+            foreach ($messages as $message) {
+                $content['messages'][] = [
+                    'title'    => $message->getTitle(),
+                    'message'  => $message->getMessage(),
+                    'severity' => $message->getSeverity()
+                ];
+                if ($message->getSeverity() === AbstractMessage::ERROR) {
+                    $content['hasErrors'] = true;
+                }
+            }
+        }
+        return new JsonResponse($content);
     }
 
     /**
      * Initialization of the class
+     *
+     * @param ServerRequestInterface $request
      */
-    public function init()
+    public function init(ServerRequestInterface $request = null): void
     {
+        if ($request === null) {
+            // Method signature in v10: protected function init(ServerRequestInterface $request)
+            trigger_error('Method init() will be set to protected in v10. Do not call from other extension', E_USER_DEPRECATED);
+            $request = $GLOBALS['TYPO3_REQUEST'];
+        }
+
         $beUser = $this->getBackendUser();
+
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
+
         // GPvars:
-        $this->flags = GeneralUtility::_GP('flags');
-        $this->data = GeneralUtility::_GP('data');
-        $this->cmd = GeneralUtility::_GP('cmd');
-        $this->mirror = GeneralUtility::_GP('mirror');
-        $this->cacheCmd = GeneralUtility::_GP('cacheCmd');
-        $this->redirect = GeneralUtility::sanitizeLocalUrl(GeneralUtility::_GP('redirect'));
-        $this->CB = GeneralUtility::_GP('CB');
+        $this->flags = $parsedBody['flags'] ?? $queryParams['flags'] ?? null;
+        $this->data = $parsedBody['data'] ?? $queryParams['data'] ?? null;
+        $this->cmd = $parsedBody['cmd'] ?? $queryParams['cmd'] ?? null;
+        $this->mirror = $parsedBody['mirror'] ?? $queryParams['mirror'] ?? null;
+        $this->cacheCmd = $parsedBody['cacheCmd'] ?? $queryParams['cacheCmd'] ?? null;
+        $redirect = $parsedBody['redirect'] ?? $queryParams['redirect'] ?? '';
+        $this->redirect = GeneralUtility::sanitizeLocalUrl($redirect);
+        $this->CB = $parsedBody['CB'] ?? $queryParams['CB'] ?? null;
         // Creating DataHandler object
         $this->tce = GeneralUtility::makeInstance(DataHandler::class);
         // Configuring based on user prefs.
@@ -143,8 +240,30 @@ public function init()
 
     /**
      * Clipboard pasting and deleting.
+     *
+     * @deprecated since v9, will be removed in v10
      */
     public function initClipboard()
+    {
+        trigger_error('Method initClipboard() will be replaced by protected method initializeClipboard() in v10. Do not call from other extension', E_USER_DEPRECATED);
+        $this->initializeClipboard();
+    }
+
+    /**
+     * Executing the posted actions ...
+     *
+     * @deprecated since v9, will be removed in v10
+     */
+    public function main()
+    {
+        trigger_error('Method main() will be replaced by protected method processRequest() in v10. Do not call from other extension', E_USER_DEPRECATED);
+        $this->processRequest();
+    }
+
+    /**
+     * Clipboard pasting and deleting.
+     */
+    protected function initializeClipboard(): void
     {
         if (is_array($this->CB)) {
             $clipObj = GeneralUtility::makeInstance(Clipboard::class);
@@ -167,7 +286,7 @@ public function initClipboard()
     /**
      * Executing the posted actions ...
      */
-    public function main()
+    protected function processRequest(): void
     {
         // LOAD DataHandler with data and cmd arrays:
         $this->tce->start($this->data, $this->cmd);
@@ -189,72 +308,12 @@ public function main()
         }
     }
 
-    /**
-     * Injects the request object for the current request or subrequest
-     * As this controller goes only through the main() method, it just redirects to the given URL afterwards.
-     *
-     * @param ServerRequestInterface $request the current request
-     * @return ResponseInterface the response with the content
-     */
-    public function mainAction(ServerRequestInterface $request): ResponseInterface
-    {
-        $this->initClipboard();
-        $this->main();
-
-        // Write errors to flash message queue
-        $this->tce->printLogErrorMessages();
-        if ($this->redirect) {
-            return new RedirectResponse(GeneralUtility::locationHeaderUrl($this->redirect), 303);
-        }
-        return new HtmlResponse('');
-    }
-
-    /**
-     * Processes all AJAX calls and returns a JSON formatted string
-     *
-     * @param ServerRequestInterface $request
-     * @return ResponseInterface
-     */
-    public function processAjaxRequest(ServerRequestInterface $request): ResponseInterface
-    {
-        // do the regular / main logic
-        $this->initClipboard();
-        $this->main();
-
-        /** @var \TYPO3\CMS\Core\Messaging\FlashMessageService $flashMessageService */
-        $flashMessageService = GeneralUtility::makeInstance(FlashMessageService::class);
-
-        $content = [
-            'redirect' => $this->redirect,
-            'messages' => [],
-            'hasErrors' => false
-        ];
-
-        // Prints errors (= write them to the message queue)
-        $this->tce->printLogErrorMessages();
-
-        $messages = $flashMessageService->getMessageQueueByIdentifier()->getAllMessagesAndFlush();
-        if (!empty($messages)) {
-            foreach ($messages as $message) {
-                $content['messages'][] = [
-                    'title'    => $message->getTitle(),
-                    'message'  => $message->getMessage(),
-                    'severity' => $message->getSeverity()
-                ];
-                if ($message->getSeverity() === AbstractMessage::ERROR) {
-                    $content['hasErrors'] = true;
-                }
-            }
-        }
-        return new JsonResponse($content);
-    }
-
     /**
      * Returns the current BE user.
      *
-     * @return \TYPO3\CMS\Core\Authentication\BackendUserAuthentication
+     * @return BackendUserAuthentication
      */
-    protected function getBackendUser()
+    protected function getBackendUser(): BackendUserAuthentication
     {
         return $GLOBALS['BE_USER'];
     }

typo3/sysext/core/Documentation/Changelog/master/Deprecation-84374-ProtectedMethodsAndPropertiesInSimpleDataHandlerController.rst

@@ -0,0 +1,55 @@
+.. include:: ../../Includes.txt
+
+======================================================================================
+Deprecation: #84374 - Protected methods and properties in SimpleDataHandlerController
+======================================================================================
+
+See :issue:`84374`
+
+Description
+===========
+
+This file is about third party usage (consumer that call the class as well as
+signals or hooks depending on it) of :php:`TYPO3\CMS\Backend\Controller\SimpleDataHandlerController`.
+
+A series of class properties has been set to protected.
+They will throw deprecation warnings if called public from outside:
+
+* :php:`flags`
+* [not scanned] :php:`data`
+* [not scanned] :php:`cmd`
+* :php:`mirror`
+* :php:`cacheCmd`
+* [not scanned] :php:`redirect`
+* :php:`CB`
+* [not scanned] :php:`tce`
+
+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()`
+
+
+Impact
+======
+
+Calling above method on an instance of
+:php:`SimpleDataHandlerController` will throw a deprecation warning in v9 and a PHP fatal in v10.
+
+
+Affected Installations
+======================
+
+The extension scanner will find all usages, but may also find some false positives.  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

typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php

@@ -1962,4 +1962,11 @@
             'Deprecation-84324-UseServerRequestInterfaceInFileFileController.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Controller\SimpleDataHandlerController->initClipboard' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-84374-ProtectedMethodsAndPropertiesInSimpleDataHandlerController.rst',
+        ],
+    ],
 ];