[TASK] Use ServerRequestInterface in Wizard/EditController

This patch changes the EditController to consistently use
ServerRequestInterface instead of GeneralUtility and
HttpUtility.

The method main() is kept to keep this change non-breaking,
it translates the given response objects back to strings/redirects
which match existing behaviour.

Also, to prepare for refactoring in v10,
main() and the new protected properties will throw a
deprecation notice if called.

Resolves: #84327
Releases: master
Change-Id: I9bd055f4a81e5f9ca324954bc03206c9f463cb11
Reviewed-on: https://review.typo3.org/56232
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/Wizard/EditController.php

@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Controller\Wizard;
 
 /*
@@ -16,8 +17,10 @@
 
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use TYPO3\CMS\Core\Compatibility\PublicPropertyDeprecationTrait;
 use TYPO3\CMS\Core\Database\RelationHandler;
 use TYPO3\CMS\Core\Http\HtmlResponse;
+use TYPO3\CMS\Core\Http\RedirectResponse;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\HttpUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
@@ -27,19 +30,41 @@
  */
 class EditController extends AbstractWizardController
 {
+    use PublicPropertyDeprecationTrait;
+
+    /**
+     * Properties which have been moved to protected status from public
+     *
+     * @var array
+     */
+    protected $deprecatedPublicProperties = [
+        'P' => 'Using $P of class EditController from the outside is discouraged, as this variable is only used for internal storage.',
+        'doClose' => 'Using $doClose of class EditController from the outside is discouraged, as this variable is only used for internal storage.',
+    ];
+
     /**
      * Wizard parameters, coming from FormEngine linking to the wizard.
      *
+     * Contains the following parts:
+     * - table
+     * - field
+     * - formName
+     * - hmac
+     * - fieldChangeFunc
+     * - fieldChangeFuncHash
+     * - currentValue
+     * - currentSelectedValues
+     *
      * @var array
      */
-    public $P;
+    protected $P;
 
     /**
      * Boolean; if set, the window will be closed by JavaScript
      *
      * @var int
      */
-    public $doClose;
+    protected $doClose;
 
     /**
      * A little JavaScript to close the open window.
@@ -56,17 +81,24 @@ public function __construct()
         $this->getLanguageService()->includeLLFile('EXT:lang/Resources/Private/Language/locallang_wizards.xlf');
         $GLOBALS['SOBE'] = $this;
 
-        $this->init();
+        // @deprecated since v9, will be moved out of __construct() in v10
+        $this->init($GLOBALS['TYPO3_REQUEST']);
     }
 
     /**
      * Initialization of the script
+     *
+     * @param  ServerRequestInterface
      */
-    protected function init()
+    protected function init(ServerRequestInterface $request)
     {
-        $this->P = GeneralUtility::_GP('P');
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
+
+        $this->P = $parsedBody['P'] ?? $queryParams['P'] ?? [];
+
         // Used for the return URL to FormEngine so that we can close the window.
-        $this->doClose = GeneralUtility::_GP('doClose');
+        $this->doClose = $parsedBody['doClose'] ?? $queryParams['doClose'] ?? 0;
     }
 
     /**
@@ -78,29 +110,51 @@ protected function init()
      */
     public function mainAction(ServerRequestInterface $request): ResponseInterface
     {
-        $content = $this->main();
-        return new HtmlResponse($content);
+        $content = $this->processRequest($request);
+        return $content;
     }
 
     /**
      * Main function
      * Makes a header-location redirect to an edit form IF POSSIBLE from the passed data - otherwise the window will
      * just close.
      *
+     * @deprecated since v9, will be removed in v10
      * @return string
      */
     public function main()
+    {
+        trigger_error('Method main() will be set to protected in v10. Do not call from other extension', E_USER_DEPRECATED);
+        $request = $GLOBALS['TYPO3_REQUEST'];
+
+        $response = $this->processRequest($request);
+
+        if ($response instanceof RedirectResponse) {
+            HttpUtility::redirect($response->getHeaders()['location'][0]);
+        } else {
+            return $response->getBody()->getContents();
+        }
+    }
+
+    /**
+     * Process request function
+     * Makes a header-location redirect to an edit form IF POSSIBLE from the passed data - otherwise the window will
+     * just close.
+     *
+     * @param  ServerRequestInterface $request
+     * @return ResponseInterface
+     */
+    protected function processRequest(ServerRequestInterface $request): ResponseInterface
     {
         if ($this->doClose) {
-            return $this->closeWindow;
+            return new HtmlResponse($this->closeWindow);
         }
         // Initialize:
         $table = $this->P['table'];
         $field = $this->P['field'];
         $config = $GLOBALS['TCA'][$table]['columns'][$field]['config'];
         $fTable = $config['foreign_table'];
 
-        /** @var \TYPO3\CMS\Backend\Routing\UriBuilder $uriBuilder */
         $uriBuilder = GeneralUtility::makeInstance(\TYPO3\CMS\Backend\Routing\UriBuilder::class);
 
         $urlParameters = [
@@ -118,8 +172,10 @@ public function main()
             $urlParameters['edit[' . $fTable . '][' . $this->P['currentValue'] . ']'] = 'edit';
             // Redirect to FormEngine
             $url = (string)$uriBuilder->buildUriFromRoute('record_edit', $urlParameters);
-            HttpUtility::redirect($url);
-        } elseif (is_array($config)
+            return new RedirectResponse($url);
+        }
+
+        if (is_array($config)
             && $this->P['currentSelectedValues']
             && (
                 $config['type'] === 'select'
@@ -133,7 +189,6 @@ public function main()
             $allowedTables = $config['type'] === 'group' ? $config['allowed'] : $config['foreign_table'];
             $prependName = 1;
             // Selecting selected values into an array:
-            /** @var RelationHandler $relationHandler */
             $relationHandler = GeneralUtility::makeInstance(RelationHandler::class);
             $relationHandler->start($this->P['currentSelectedValues'], $allowedTables);
             $value = $relationHandler->getValueArray($prependName);
@@ -144,9 +199,9 @@ public function main()
             }
             // Redirect to FormEngine
             $url = (string)$uriBuilder->buildUriFromRoute('record_edit', $urlParameters);
-            HttpUtility::redirect($url);
-        } else {
-            return $this->closeWindow;
+
+            return new RedirectResponse($url);
         }
+        return new HtmlResponse($this->closeWindow);
     }
 }

typo3/sysext/core/Documentation/Changelog/master/Deprecation-84327-DeprecatedPublicMethodsAndPropertiesInWizardEditController.rst

@@ -0,0 +1,45 @@
+.. include:: ../../Includes.txt
+
+=======================================================================================
+Deprecation: #84327 - Deprecated public methods and properties in Wizard/EditController
+=======================================================================================
+
+See :issue:`84327`
+
+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\Wizard\EditController`.
+
+A series of class properties has been set to protected.
+They will throw deprecation warnings if called public from outside:
+
+* [not scanned] :php:`$P`
+* :php:`$doClose`
+
+The following method will be refactored/set to protected in v10 and should no longer be used:
+
+* [not scanned] :php:`main()`
+
+
+Impact
+======
+
+Calling one of the above methods or accessing one of the above properties on an instance of
+:php:`Wizard/EditController` will throw a deprecation warning in v9 and a PHP fatal in v10.
+
+
+Affected Installations
+======================
+
+The extension scanner will detect only detect usage of :php:`$doClose`, other calls are not scanned to prevent false positives.
+
+
+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, ext:backend

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

@@ -487,4 +487,9 @@
             'Deprecation-84341-ProtectedMethodsAndPropertiesInNewRecordController.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Controller\Wizard\EditController->doClose' => [
+        'restFiles' => [
+            'Deprecation-84327-DeprecatedPublicMethodsAndPropertiesInWizardEditController.rst',
+        ],
+    ],
 ];