From fcd6c8e582820c34f27bef7437108fc90239265c Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Tue, 15 Dec 2020 13:19:33 +0100
Subject: [PATCH] [!!!][TASK] Protect RelationHandler internals

Class RelationHandler is a confusing mess with tons
of state spread over 28 class properties and 35
consuming methods. Nearly everything is public and
of course far from bug free.

The patch protects a series of properties and methods
that handle internal state and do not need to be
accessed from outside. For instance, the same state
created by the 'read' methods is done by start().

This is a starter patch to allow refactorings
towards a better manageable codebase.

Change-Id: I8aa45fcd0306209fcc298972ad850449d2c4f125
Resolves: #93080
Releases: master
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/67138
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
---
 .../core/Classes/Database/RelationHandler.php | 48 +++++++-------
 ...3080-RelationHandlerInternalsProtected.rst | 58 +++++++++++++++++
 .../Php/MethodCallMatcher.php                 | 49 +++++++++++++++
 .../Php/MethodCallStaticMatcher.php           |  7 +++
 .../Php/PropertyProtectedMatcher.php          | 62 ++++++++++++++++++-
 5 files changed, 201 insertions(+), 23 deletions(-)
 create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Breaking-93080-RelationHandlerInternalsProtected.rst

diff --git a/typo3/sysext/core/Classes/Database/RelationHandler.php b/typo3/sysext/core/Classes/Database/RelationHandler.php
index 165a80a5d1e5..baf305329f60 100644
--- a/typo3/sysext/core/Classes/Database/RelationHandler.php
+++ b/typo3/sysext/core/Classes/Database/RelationHandler.php
@@ -86,14 +86,14 @@ class RelationHandler
      *
      * @var string
      */
-    public $firstTable = '';
+    protected $firstTable = '';
 
     /**
      * Will contain the second table name in the $tablelist (for negative ids)
      *
      * @var string
      */
-    public $secondTable = '';
+    protected $secondTable = '';
 
     /**
      * If TRUE, uid_local and uid_foreign are switched, and the current table
@@ -101,42 +101,42 @@ class RelationHandler
      *
      * @var bool
      */
-    public $MM_is_foreign = false;
+    protected $MM_is_foreign = false;
 
     /**
      * Field name at the "local" side of the MM relation
      *
      * @var string
      */
-    public $MM_oppositeField = '';
+    protected $MM_oppositeField = '';
 
     /**
      * Only set if MM_is_foreign is set
      *
      * @var string
      */
-    public $MM_oppositeTable = '';
+    protected $MM_oppositeTable = '';
 
     /**
      * Only set if MM_is_foreign is set
      *
      * @var array
      */
-    public $MM_oppositeFieldConf = [];
+    protected $MM_oppositeFieldConf = [];
 
     /**
      * Is empty by default; if MM_is_foreign is set and there is more than one table
      * allowed (on the "local" side), then it contains the first table (as a fallback)
      * @var string
      */
-    public $MM_isMultiTableRelationship = '';
+    protected $MM_isMultiTableRelationship = '';
 
     /**
      * Current table => Only needed for reverse relations
      *
      * @var string
      */
-    public $currentTable;
+    protected $currentTable;
 
     /**
      * If a record should be undeleted
@@ -152,28 +152,28 @@ class RelationHandler
      *
      * @var array
      */
-    public $MM_match_fields = [];
+    protected $MM_match_fields = [];
 
     /**
      * This is set to TRUE if the MM table has a UID field.
      *
      * @var bool
      */
-    public $MM_hasUidField;
+    protected $MM_hasUidField;
 
     /**
      * Array of fields and value pairs used for insert in MM table
      *
      * @var array
      */
-    public $MM_insert_fields = [];
+    protected $MM_insert_fields = [];
 
     /**
      * Extra MM table where
      *
      * @var string
      */
-    public $MM_table_where = '';
+    protected $MM_table_where = '';
 
     /**
      * Usage of a MM field on the opposite relation.
@@ -186,6 +186,7 @@ class RelationHandler
      * If false, reference index is not updated.
      *
      * @var bool
+     * @deprecated since v11, will be removed in v12
      */
     protected $updateReferenceIndex = true;
 
@@ -226,7 +227,7 @@ class RelationHandler
      *
      * @return int
      */
-    public function getWorkspaceId(): int
+    protected function getWorkspaceId(): int
     {
         if (!isset($this->workspaceId)) {
             $this->workspaceId = (int)$GLOBALS['BE_USER']->workspace;
@@ -372,10 +373,14 @@ class RelationHandler
      * Sets whether the reference index shall be updated.
      *
      * @param bool $updateReferenceIndex Whether the reference index shall be updated
-     * @todo: Unused in core, should be removed, use ReferenceIndexUpdater instead
+     * @deprecated since v11, will be removed in v12
      */
     public function setUpdateReferenceIndex($updateReferenceIndex)
     {
+        trigger_error(
+            'Calling RelationHandler->setUpdateReferenceIndex() is deprecated. Use setReferenceIndexUpdater() instead.',
+            E_USER_DEPRECATED
+        );
         $this->updateReferenceIndex = (bool)$updateReferenceIndex;
     }
 
@@ -401,7 +406,7 @@ class RelationHandler
      * @param string $itemlist Item list
      * @param array $configuration Parent field configuration
      */
-    public function readList($itemlist, array $configuration)
+    protected function readList($itemlist, array $configuration)
     {
         if ((string)trim($itemlist) != '') {
             $tempItemArray = GeneralUtility::trimExplode(',', $itemlist);
@@ -482,7 +487,7 @@ class RelationHandler
      *
      * @param string $sortby The default_sortby field/command (e.g. 'price DESC')
      */
-    public function sortList($sortby)
+    protected function sortList($sortby)
     {
         // Sort directly without fetching additional data
         if ($sortby === 'uid') {
@@ -529,12 +534,11 @@ class RelationHandler
 
     /**
      * Reads the record tablename/id into the internal arrays itemArray and tableArray from MM records.
-     * You can call this function after start if you supply no list to start()
      *
      * @param string $tableName MM Tablename
      * @param int $uid Local UID
      */
-    public function readMM($tableName, $uid)
+    protected function readMM($tableName, $uid)
     {
         $key = 0;
         $theTable = null;
@@ -917,7 +921,7 @@ class RelationHandler
      * @param int $uid The uid of the parent record (this value is also on the foreign_table in the foreign_field)
      * @param array $conf TCA configuration for current field
      */
-    public function readForeignField($uid, $conf)
+    protected function readForeignField($uid, $conf)
     {
         if ($this->useLiveParentIds) {
             $uid = $this->getLiveDefaultId($this->currentTable, $uid);
@@ -1298,9 +1302,8 @@ class RelationHandler
      * @param string $table Table name
      * @param int $uid Record uid
      * @return array Result from ReferenceIndex->updateRefIndexTable() updated directly, else empty array
-     * @internal Should be protected
      */
-    public function updateRefIndex($table, $uid): array
+    protected function updateRefIndex($table, $uid): array
     {
         if (!$this->updateReferenceIndex) {
             return [];
@@ -1310,6 +1313,7 @@ class RelationHandler
             $this->referenceIndexUpdater->registerForUpdate((string)$table, (int)$uid, $this->getWorkspaceId());
             $statisticsArray = [];
         } else {
+            // @deprecated else branch can be dropped when setUpdateReferenceIndex() is dropped.
             // Update reference index directly if enabled
             $referenceIndex = GeneralUtility::makeInstance(ReferenceIndex::class);
             if (BackendUtility::isTableWorkspaceEnabled($table)) {
@@ -1603,7 +1607,7 @@ class RelationHandler
      * @param array $childRec The record row of the child record
      * @return bool Returns TRUE if looking from the symmetric ("other") side to the relation.
      */
-    public static function isOnSymmetricSide($parentUid, $parentConf, $childRec)
+    protected static function isOnSymmetricSide($parentUid, $parentConf, $childRec)
     {
         return MathUtility::canBeInterpretedAsInteger($childRec['uid'])
             && $parentConf['symmetric_field']
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-93080-RelationHandlerInternalsProtected.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-93080-RelationHandlerInternalsProtected.rst
new file mode 100644
index 000000000000..a598571d3b74
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Breaking-93080-RelationHandlerInternalsProtected.rst
@@ -0,0 +1,58 @@
+.. include:: ../../Includes.txt
+
+======================================================
+Breaking: #93080 - RelationHandler internals protected
+======================================================
+
+See :issue:`93080`
+
+Description
+===========
+
+Various properties and methods of class
+:php:`TYPO3\CMS\Core\Database\RelationHandler` have been set to protected:
+
+* :php:`$firstTable` - internal
+* :php:`$secondTable` - internal
+* :php:`$MM_is_foreign` - internal
+* :php:`$MM_oppositeField` - internal
+* :php:`$MM_oppositeTable` - internal
+* :php:`$MM_oppositeFieldConf` - internal
+* :php:`$MM_isMultiTableRelationship` - internal
+* :php:`$currentTable` - internal
+* :php:`$MM_match_fields` - internal
+* :php:`$MM_hasUidField` - internal
+* :php:`$MM_insert_fields` - internal
+* :php:`$MM_table_where` - internal
+* :php:`getWorkspaceId()` - internal
+* :php:`setUpdateReferenceIndex()` - still public but deprecated, logs deprecation on use.
+* :php:`readList()` - use class state after calling start()
+* :php:`sortList()` - use class state after calling start()
+* :php:`readMM()` - use class state after calling start()
+* :php:`readForeignField()` - use class state after calling start()
+* :php:`updateRefIndex()` - internal
+* :php:`isOnSymmetricSide()` - internal
+
+
+Impact
+======
+
+Calling above properties or methods will raise a fatal PHP error.
+
+
+Affected Installations
+======================
+
+It is quite unlikely many extensions are affected by this API change.
+The extension scanner finds affected extensions as weak matches.
+
+
+Migration
+=========
+
+Above properties and methods are considered internal, there shouldn't be any
+need to call them. Instances with extensions using those should be refactored
+to for instance call :php:`start()` instead of an additional call to :php:`readList()`.
+
+
+.. index:: PHP-API, FullyScanned, ext:core
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
index e4ef474e52b9..ddc6e54c2ab2 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
@@ -4641,4 +4641,53 @@ return [
             'Deprecation-93023-ReworkedSessionHandling.rst',
         ],
     ],
+    'TYPO3\CMS\Core\Database\RelationHandler->getWorkspaceId' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->setUpdateReferenceIndex' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->readList' => [
+        'numberOfMandatoryArguments' => 2,
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->sortList' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->readMM' => [
+        'numberOfMandatoryArguments' => 2,
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->readForeignField' => [
+        'numberOfMandatoryArguments' => 2,
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->updateRefIndex' => [
+        'numberOfMandatoryArguments' => 2,
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
 ];
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
index e04acc77fd21..676146cd856e 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
@@ -1057,4 +1057,11 @@ return [
             'Deprecation-92607-DeprecatedGeneralUtilityuniqueList.rst'
         ],
     ],
+    'TYPO3\CMS\Core\Database\RelationHandler::isOnSymmetricSide' => [
+        'numberOfMandatoryArguments' => 3,
+        'maximumNumberOfArguments' => 3,
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
 ];
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyProtectedMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyProtectedMatcher.php
index 1fa9cefcfe52..814edb43139b 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyProtectedMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/PropertyProtectedMatcher.php
@@ -1275,5 +1275,65 @@ return [
         'restFiles' => [
             'Breaking-93029-DroppedDeletedFieldFromSys_refindex.rst',
         ],
-    ]
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->firstTable' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->secondTable' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_is_foreign' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_oppositeField' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_oppositeTable' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_oppositeFieldConf' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_isMultiTableRelationship' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->currentTable' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_match_fields' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_hasUidField' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_insert_fields' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
+    'TYPO3\CMS\Core\Database\RelationHandler->MM_table_where' => [
+        'restFiles' => [
+            'Breaking-93080-RelationHandlerInternalsProtected.rst',
+        ],
+    ],
 ];
-- 
GitLab