From 2289ae2f1f0327dd223013ddbfb800d044519e88 Mon Sep 17 00:00:00 2001
From: Alexander Schnitzler <git@alexanderschnitzler.de>
Date: Tue, 27 Jun 2023 09:13:24 +0200
Subject: [PATCH] [BUGFIX] Avoid symfony/property-access in
 getGettablePropertyNames()

ObjectAccess::getGettablePropertyNames() has quite the history by now.
It used to be quite simple, an is_callable() check for getters/hassers
and issers of objects. That didn't account for methods with mandatory
method arguments which was fixed by using reflection. Because runtime
reflection is slow, the usage of cached reflection (ClassSchema) had
been introduced. But, during that change, symfony/property-access had
also been introduces, which contradicts the idea of performance gain
because:

- symfony/property-access also uses uncached reflection
- symfony/property-access actually calls the accessors under test

As both is undesirable, the usage of symfony/property-access has been
removed again.

Releases: main, 12.4, 11.5
Resolves: #101176
Change-Id: I2bc796ebeaf2f1357fd3154b711910c6f553f4e4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79675
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
---
 .../Classes/Reflection/ObjectAccess.php       | 71 ++++++++++++++-----
 1 file changed, 53 insertions(+), 18 deletions(-)

diff --git a/typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php b/typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php
index 373b0c1f97d3..43b79da083ae 100644
--- a/typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php
+++ b/typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php
@@ -209,37 +209,72 @@ class ObjectAccess
         $classSchema = GeneralUtility::makeInstance(ReflectionService::class)
             ->getClassSchema($object);
 
-        $accessor = self::createAccessor();
-        $propertyNames = array_keys($classSchema->getProperties());
-        $accessiblePropertyNames = array_filter(
-            $propertyNames,
-            static fn (string $propertyName): bool => $accessor->isReadable($object, $propertyName)
-        );
-
-        foreach ($classSchema->getMethods() as $methodName => $methodDefinition) {
-            if (!$methodDefinition->isPublic()) {
+        $accessiblePropertyNames = [];
+        foreach ($classSchema->getProperties() as $propertyName => $propertyDefinition) {
+            if ($propertyDefinition->isPublic()) {
+                $accessiblePropertyNames[] = $propertyName;
                 continue;
             }
 
-            foreach ($methodDefinition->getParameters() as $methodParam) {
-                if (!$methodParam->isOptional()) {
-                    continue 2;
+            $accessors = [
+                'get' . ucfirst($propertyName),
+                'has' . ucfirst($propertyName),
+                'is' . ucfirst($propertyName),
+            ];
+
+            foreach ($accessors as $accessor) {
+                if (!$classSchema->hasMethod($accessor)) {
+                    continue;
                 }
+
+                if (!$classSchema->getMethod($accessor)->isPublic()) {
+                    continue;
+                }
+
+                foreach ($classSchema->getMethod($accessor)->getParameters() as $methodParam) {
+                    if (!$methodParam->isOptional()) {
+                        continue 2;
+                    }
+                }
+
+                if (!is_callable([$object, $accessor])) {
+                    continue;
+                }
+
+                $accessiblePropertyNames[] = $propertyName;
+            }
+        }
+
+        // Fallback mechanism to not break former behaviour
+        //
+        // todo: Checking accessor methods of virtual(non-existing) properties should be removed (breaking) in
+        //       upcoming versions. It was an unintentionally added "feature" in the past. It contradicts the method
+        //       name "getGettablePropertyNames".
+        foreach ($classSchema->getMethods() as $methodName => $methodDefinition) {
+            $propertyName = null;
+            if (str_starts_with($methodName, 'get') || str_starts_with($methodName, 'has')) {
+                $propertyName = lcfirst(substr($methodName, 3));
+            }
+
+            if (str_starts_with($methodName, 'is')) {
+                $propertyName = lcfirst(substr($methodName, 2));
             }
 
-            if (str_starts_with($methodName, 'get')) {
-                $accessiblePropertyNames[] = lcfirst(substr($methodName, 3));
+            if ($propertyName === null) {
                 continue;
             }
 
-            if (str_starts_with($methodName, 'has')) {
-                $accessiblePropertyNames[] = lcfirst(substr($methodName, 3));
+            if (!$methodDefinition->isPublic()) {
                 continue;
             }
 
-            if (str_starts_with($methodName, 'is')) {
-                $accessiblePropertyNames[] = lcfirst(substr($methodName, 2));
+            foreach ($methodDefinition->getParameters() as $methodParam) {
+                if (!$methodParam->isOptional()) {
+                    continue 2;
+                }
             }
+
+            $accessiblePropertyNames[] = $propertyName;
         }
 
         $accessiblePropertyNames = array_unique($accessiblePropertyNames);
-- 
GitLab