From 6c3c8a6a42699efae897000adfa9ca1273f72f15 Mon Sep 17 00:00:00 2001
From: Anja Leichsenring <aleichsenring@ab-softlab.de>
Date: Wed, 18 Jan 2023 16:39:06 +0100
Subject: [PATCH] [TASK] Avoid last usage of GU::_GPmerged() and deprecate
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After years of refactoring, TYPO3 core finally removes
a last usage of GeneralUtility::_GPmerged() which is
an anti-pattern method, since it accesses $_GET
and $_POST instead of retrieving things from
PSR-7 ServerRequestInterface $request.

Resolves: #99615
Releases: main
Change-Id: I3cb6f3a0cee0a7654dce331dc40ab51f2efe41f4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77477
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
---
 .../core/Classes/Utility/GeneralUtility.php   |  6 ++
 ...precation-99615-GeneralUtilityGPMerged.rst | 62 +++++++++++++++++++
 .../Tests/Unit/Utility/GeneralUtilityTest.php | 32 ----------
 .../Utility/GeneralUtilityTest.php            | 53 ++++++++++++++++
 .../Classes/Plugin/AbstractPlugin.php         | 29 +++++++--
 .../Php/MethodCallStaticMatcher.php           |  7 +++
 6 files changed, 152 insertions(+), 37 deletions(-)
 create mode 100644 typo3/sysext/core/Documentation/Changelog/12.2/Deprecation-99615-GeneralUtilityGPMerged.rst
 create mode 100644 typo3/sysext/core/Tests/UnitDeprecated/Utility/GeneralUtilityTest.php

diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
index 9b1c4aac131f..85a22309a583 100644
--- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php
+++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
@@ -123,9 +123,15 @@ class GeneralUtility
      *
      * @param string $parameter Key (variable name) from GET or POST vars
      * @return array Returns the GET vars merged recursively onto the POST vars.
+     * @deprecated since TYPO3 v12.2. will be removed in TYPO3 v13.0.
      */
     public static function _GPmerged($parameter)
     {
+        trigger_error(
+            'GeneralUtility::_GPmerged() will be removed in TYPO3 v13.0, retrieve request related' .
+            ' details from PSR-7 ServerRequestInterface instead.',
+            E_USER_DEPRECATED
+        );
         $postParameter = isset($_POST[$parameter]) && is_array($_POST[$parameter]) ? $_POST[$parameter] : [];
         $getParameter = isset($_GET[$parameter]) && is_array($_GET[$parameter]) ? $_GET[$parameter] : [];
         $mergedParameters = $getParameter;
diff --git a/typo3/sysext/core/Documentation/Changelog/12.2/Deprecation-99615-GeneralUtilityGPMerged.rst b/typo3/sysext/core/Documentation/Changelog/12.2/Deprecation-99615-GeneralUtilityGPMerged.rst
new file mode 100644
index 000000000000..89a374e98357
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/12.2/Deprecation-99615-GeneralUtilityGPMerged.rst
@@ -0,0 +1,62 @@
+.. include:: /Includes.rst.txt
+
+.. _deprecation-99615-1674056024:
+
+=================================================
+Deprecation: #99615 - GeneralUtility::_GPmerged()
+=================================================
+
+See :issue:`99615`
+
+Description
+===========
+
+Method :php:`\TYPO3\CMS\Backend\Utility\GeneralUtility::_GPmerged()` has
+been marked as deprecated and should not be used any longer.
+
+Modern code should access GET and POST data from the PSR-7 :php:`ServerRequestInterface`,
+and should avoid accessing superglobals :php:`$_GET` and :php:`$_POST`
+directly. This helps creating controller classes with a clean architecture. Some
+:php:`GeneralUtility` related helper methods like :php:`_GPmerged()` violate this,
+using them is considered a technical debt. They are being phased out.
+
+
+Impact
+======
+
+Calling the method will raise a deprecation level log error and will
+stop working with TYPO3 v13.
+
+
+Affected installations
+======================
+
+Instances with extensions using :php:`GeneralUtility::_GPmerged()` are affected.
+The extension scanner will find usages with a strong match.
+
+
+Migration
+=========
+
+:php:`GeneralUtility::_GPmerged()` is a helper method that retrieves
+request parameters and returns the value, while POST parameters take
+precedence over GET parameters, if both exist.
+
+The same result can be achieved by retrieving arguments from the request object.
+An instance of the PSR-7 :php:`ServerRequestInterface` is hand over to
+controllers by TYPO3 core PSR-15 :php:`RequestHandlerInterface` and Middleware
+implementations, and is available in various related scopes like the Frontend
+:php:`ContentObjectRenderer`.
+
+Typical code:
+
+..  code-block:: php
+
+    // Before
+    $getMergedWithPost = GeneralUtility::_GPmerged('tx_scheduler');
+
+    // After
+    $getMergedWithPost = $request->getQueryString()['tx_scheduler'];
+    ArrayUtility::mergeRecursiveWithOverrule($getMergedWithPost, $request->getParsedBody()['tx_scheduler']);
+
+.. index:: Backend, PHP-API, FullyScanned, ext:backend
diff --git a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php
index 4398d2551c53..370284c9c40b 100644
--- a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php
+++ b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php
@@ -132,38 +132,6 @@ class GeneralUtilityTest extends UnitTestCase
         ];
     }
 
-    ///////////////////////////
-    // Tests concerning _GPmerged
-    ///////////////////////////
-    /**
-     * @test
-     * @dataProvider gpMergedDataProvider
-     */
-    public function gpMergedWillMergeArraysFromGetAndPost($get, $post, $expected): void
-    {
-        $_POST = $post;
-        $_GET = $get;
-        self::assertEquals($expected, GeneralUtility::_GPmerged('cake'));
-    }
-
-    /**
-     * Data provider for gpMergedWillMergeArraysFromGetAndPost
-     */
-    public function gpMergedDataProvider(): array
-    {
-        $fullDataArray = ['cake' => ['a' => 'is a', 'b' => 'lie']];
-        $postPartData = ['cake' => ['b' => 'lie']];
-        $getPartData = ['cake' => ['a' => 'is a']];
-        $getPartDataModified = ['cake' => ['a' => 'is not a']];
-        return [
-            'Key doesn\' exist' => [['foo'], ['bar'], []],
-            'No POST data' => [$fullDataArray, [], $fullDataArray['cake']],
-            'No GET data' => [[], $fullDataArray, $fullDataArray['cake']],
-            'POST and GET are merged' => [$getPartData, $postPartData, $fullDataArray['cake']],
-            'POST is preferred over GET' => [$getPartDataModified, $fullDataArray, $fullDataArray['cake']],
-        ];
-    }
-
     ///////////////////////////////
     // Tests concerning _GET / _POST
     ///////////////////////////////
diff --git a/typo3/sysext/core/Tests/UnitDeprecated/Utility/GeneralUtilityTest.php b/typo3/sysext/core/Tests/UnitDeprecated/Utility/GeneralUtilityTest.php
new file mode 100644
index 000000000000..d3ed11f358b7
--- /dev/null
+++ b/typo3/sysext/core/Tests/UnitDeprecated/Utility/GeneralUtilityTest.php
@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Tests\UnitDeprecated\Utility;
+
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+class GeneralUtilityTest extends UnitTestCase
+{
+    /**
+     * @test
+     * @dataProvider gpMergedDataProvider
+     */
+    public function gpMergedWillMergeArraysFromGetAndPost($get, $post, $expected): void
+    {
+        $_POST = $post;
+        $_GET = $get;
+        self::assertEquals($expected, GeneralUtility::_GPmerged('cake'));
+    }
+
+    /**
+     * Data provider for gpMergedWillMergeArraysFromGetAndPost
+     */
+    public function gpMergedDataProvider(): array
+    {
+        $fullDataArray = ['cake' => ['a' => 'is a', 'b' => 'lie']];
+        $postPartData = ['cake' => ['b' => 'lie']];
+        $getPartData = ['cake' => ['a' => 'is a']];
+        $getPartDataModified = ['cake' => ['a' => 'is not a']];
+        return [
+            'Key doesn\' exist' => [['foo'], ['bar'], []],
+            'No POST data' => [$fullDataArray, [], $fullDataArray['cake']],
+            'No GET data' => [[], $fullDataArray, $fullDataArray['cake']],
+            'POST and GET are merged' => [$getPartData, $postPartData, $fullDataArray['cake']],
+            'POST is preferred over GET' => [$getPartDataModified, $fullDataArray, $fullDataArray['cake']],
+        ];
+    }
+}
diff --git a/typo3/sysext/frontend/Classes/Plugin/AbstractPlugin.php b/typo3/sysext/frontend/Classes/Plugin/AbstractPlugin.php
index 1ecd073e8ffd..291373534a03 100644
--- a/typo3/sysext/frontend/Classes/Plugin/AbstractPlugin.php
+++ b/typo3/sysext/frontend/Classes/Plugin/AbstractPlugin.php
@@ -35,10 +35,14 @@ use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 
 /**
- * Base class for frontend plugins
- * Most legacy frontend plugins are extension classes of this one.
- * This class contains functions which assists these plugins in creating lists, searching, displaying menus, page-browsing (next/previous/1/2/3) and handling links.
- * Functions are all prefixed "pi_" which is reserved for this class. Those functions can of course be overridden in the extension classes (that is the point...)
+ * Old school base class of frontend plugins.
+ *
+ * Various legacy frontend plugins extend this "abstract".
+ *
+ * The class comes with a series of helper methods which assist plugins in creating lists,
+ * searching, displaying menus, page-browsing (next/previous/1/2/3) and handling links.
+ * Functions are all prefixed "pi_" which is reserved for this class. Those functions
+ * can of course be overridden in the extension classes (that is the point...)
  *
  * @internal This class is not maintained anymore and may be removed in future versions.
  */
@@ -225,7 +229,7 @@ class AbstractPlugin
         $this->templateService = GeneralUtility::makeInstance(MarkerBasedTemplateService::class);
         // Setting piVars:
         if ($this->prefixId) {
-            $this->piVars = GeneralUtility::_GPmerged($this->prefixId);
+            $this->piVars = self::getRequestPostOverGetParameterWithPrefix($this->prefixId);
         }
         $this->LLkey = $this->frontendController->getLanguage()->getTypo3Language();
 
@@ -1354,4 +1358,19 @@ class AbstractPlugin
         }
         return $tempArr[$value] ?? '';
     }
+
+    /**
+     * Returns the global arrays $_GET and $_POST merged with $_POST taking precedence.
+     *
+     * @param string $parameter Key (variable name) from GET or POST vars
+     * @return array Returns the GET vars merged recursively onto the POST vars.
+     */
+    private static function getRequestPostOverGetParameterWithPrefix($parameter)
+    {
+        $postParameter = isset($_POST[$parameter]) && is_array($_POST[$parameter]) ? $_POST[$parameter] : [];
+        $getParameter = isset($_GET[$parameter]) && is_array($_GET[$parameter]) ? $_GET[$parameter] : [];
+        $mergedParameters = $getParameter;
+        ArrayUtility::mergeRecursiveWithOverrule($mergedParameters, $postParameter);
+        return $mergedParameters;
+    }
 }
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
index c61f16d47cb7..f501ea803231 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
@@ -1443,4 +1443,11 @@ return [
             'Deprecation-99579-BackendUtilityGetFuncCheck.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Utility\GeneralUtility::_GPmerged' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-99615-GeneralUtilityGPMerged.rst',
+        ],
+    ],
 ];
-- 
GitLab