From d762ea3b4e70a01e3516194d248d88c05594acee Mon Sep 17 00:00:00 2001 From: Torben Hansen <derhansen@gmail.com> Date: Thu, 20 Jul 2023 20:06:32 +0200 Subject: [PATCH] [BUGFIX] Harden Extbase LazyLoadingProxy When a domain model object has a lazy loading property (e.g. relation to another domain model object), the `AbstractGenericObjectValidator` may fail validation the domain model, if the database record for the related object has been deleted. This change fixes the problem by adding a null safe operator to the `$realInstance` variable in the magic `__get` call in LazyLoadingProxy. The return type for `_loadRealInstance` has been adapted, since the function actually can return `null`. This allows the removal of an entry in the phpstan baseline file. Additionally a todo in `AbstractGenericObjectValidator` has been removed, since the function actually supports lazy loading proxies as the initial error shows. Resolves: #101400 Releases: main, 12.4 Signed-off-by: Torben Hansen <derhansen@gmail.com> Change-Id: Ib39da09fdc94ef529f672af7f948d6a7fd82e531 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80104 Tested-by: Oliver Klee <typo3-coding@oliverklee.de> Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de> Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de> Reviewed-by: Nikita Hovratov <nikita.h@live.de> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de> Tested-by: Nikita Hovratov <nikita.h@live.de> --- Build/phpstan/phpstan-baseline.neon | 5 ----- .../Classes/Persistence/Generic/LazyLoadingProxy.php | 4 ++-- .../Validator/AbstractGenericObjectValidator.php | 1 - .../Functional/Persistence/LazyLoadingProxyTest.php | 11 +++++++++++ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Build/phpstan/phpstan-baseline.neon b/Build/phpstan/phpstan-baseline.neon index 3c23d4c80ddd..7590daf57293 100644 --- a/Build/phpstan/phpstan-baseline.neon +++ b/Build/phpstan/phpstan-baseline.neon @@ -1415,11 +1415,6 @@ parameters: count: 1 path: ../../typo3/sysext/extbase/Classes/Mvc/Request.php - - - message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#" - count: 1 - path: ../../typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php - - message: "#^Call to an undefined method TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\Generic\\\\Qom\\\\JoinConditionInterface\\:\\:getProperty1Name\\(\\)\\.$#" count: 1 diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php b/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php index 6573de906779..4bc55287a188 100644 --- a/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php +++ b/typo3/sysext/extbase/Classes/Persistence/Generic/LazyLoadingProxy.php @@ -72,7 +72,7 @@ class LazyLoadingProxy implements \Iterator, LoadingStrategyInterface /** * Populate this proxy by asking the $population closure. * - * @return object The instance (hopefully) returned + * @return object|null The instance (hopefully) returned */ public function _loadRealInstance() { @@ -137,7 +137,7 @@ class LazyLoadingProxy implements \Iterator, LoadingStrategyInterface if ($realInstance instanceof DomainObjectInterface) { return $realInstance->_getProperty($propertyName); } - return $realInstance->{$propertyName}; + return $realInstance?->{$propertyName}; } /** diff --git a/typo3/sysext/extbase/Classes/Validation/Validator/AbstractGenericObjectValidator.php b/typo3/sysext/extbase/Classes/Validation/Validator/AbstractGenericObjectValidator.php index df2740230993..7ffd3cf59a6b 100644 --- a/typo3/sysext/extbase/Classes/Validation/Validator/AbstractGenericObjectValidator.php +++ b/typo3/sysext/extbase/Classes/Validation/Validator/AbstractGenericObjectValidator.php @@ -63,7 +63,6 @@ abstract class AbstractGenericObjectValidator extends AbstractValidator implemen */ protected function getPropertyValue(object $object, string $propertyName): mixed { - // @todo add support for lazy loading proxies, if needed if (ObjectAccess::isPropertyGettable($object, $propertyName)) { return ObjectAccess::getProperty($object, $propertyName); } diff --git a/typo3/sysext/extbase/Tests/Functional/Persistence/LazyLoadingProxyTest.php b/typo3/sysext/extbase/Tests/Functional/Persistence/LazyLoadingProxyTest.php index 18834f1a10e6..d811ed561061 100644 --- a/typo3/sysext/extbase/Tests/Functional/Persistence/LazyLoadingProxyTest.php +++ b/typo3/sysext/extbase/Tests/Functional/Persistence/LazyLoadingProxyTest.php @@ -22,6 +22,7 @@ use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Http\ServerRequest; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Persistence\Generic\LazyLoadingProxy; +use TYPO3\CMS\Extbase\Reflection\ObjectAccess; use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase; use TYPO3Tests\BlogExample\Domain\Model\Administrator; use TYPO3Tests\BlogExample\Domain\Model\Blog; @@ -72,4 +73,14 @@ final class LazyLoadingProxyTest extends FunctionalTestCase self::assertSame('Blog Admin', $administrator->getUsername()); } + + /** + * @test + */ + public function nonExistingLazyLoadedPropertyReturnsNull(): void + { + $lazyLoadingProxy = new LazyLoadingProxy(new Blog(), 'administrator', 0); + + self::assertNull(ObjectAccess::getProperty($lazyLoadingProxy, 'name')); + } } -- GitLab