From 4a18072ad0ef6d3cb25a6221a414067a16d40f90 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20B=C3=BCrk?= <stefan@buerk.tech>
Date: Mon, 22 Aug 2022 22:37:00 +0200
Subject: [PATCH] [BUGFIX] Avoid invalid property access when persisting
extbase objects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
With #95819 the detection of Domain Object properties was changed to use
ClassSchema in order to support uninitialized properties is models.
This was not possible with `DomainObjectInterface::_getProperties()`
which used `get_object_vars()` under the hood.
However, with mentioned change, other properties which had been
ignored by `get_object_vars()` were now propagated to persistence,
even if those properties were not configured in TCA. The change in
behavior is considered a regression and is now addressed:
The retrieval of the property value is delayed until after it is
ensured that the property is a persistable property (which asserts
that we are not reading from private properties).
Resolves: #98148
Related: #95819
Releases: main, 11.5
Change-Id: I94d41715a49953045d32361a00bc274c15112a1e
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75534
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: theline <typo3@root-access.de>
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: theline <typo3@root-access.de>
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Stefan Bürk <stefan@buerk.tech>
---
.../Classes/Persistence/Generic/Backend.php | 12 +++++++++---
.../Generic/Storage/Typo3DbBackend.php | 3 +--
.../Domain/Model/DateTimeImmutableExample.php | 18 ++++++++++++++++++
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
index 8c6348cc4eb5..1447dd9946be 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php
@@ -310,8 +310,11 @@ class Backend implements BackendInterface, SingletonInterface
$classSchema = $this->reflectionService->getClassSchema($className);
foreach ($classSchema->getDomainObjectProperties() as $property) {
$propertyName = $property->getName();
+ if (!$dataMap->isPersistableProperty($propertyName)) {
+ continue;
+ }
$propertyValue = $object->_getProperty($propertyName);
- if (!$dataMap->isPersistableProperty($propertyName) || $this->propertyValueIsLazyLoaded($propertyValue)) {
+ if ($this->propertyValueIsLazyLoaded($propertyValue)) {
continue;
}
$columnMap = $dataMap->getColumnMap($propertyName);
@@ -601,8 +604,11 @@ class Backend implements BackendInterface, SingletonInterface
$classSchema = $this->reflectionService->getClassSchema($className);
foreach ($classSchema->getDomainObjectProperties() as $property) {
$propertyName = $property->getName();
+ if (!$dataMap->isPersistableProperty($propertyName)) {
+ continue;
+ }
$propertyValue = $object->_getProperty($propertyName);
- if (!$dataMap->isPersistableProperty($propertyName) || $this->propertyValueIsLazyLoaded($propertyValue)) {
+ if ($this->propertyValueIsLazyLoaded($propertyValue)) {
continue;
}
$columnMap = $dataMap->getColumnMap($propertyName);
@@ -906,11 +912,11 @@ class Backend implements BackendInterface, SingletonInterface
$classSchema = $this->reflectionService->getClassSchema($className);
foreach ($classSchema->getDomainObjectProperties() as $property) {
$propertyName = $property->getName();
- $propertyValue = $object->_getProperty($propertyName);
$columnMap = $dataMap->getColumnMap($propertyName);
if ($columnMap === null) {
continue;
}
+ $propertyValue = $object->_getProperty($propertyName);
if ($property->getCascadeValue() === 'remove') {
if ($columnMap->getTypeOfRelation() === ColumnMap::RELATION_HAS_MANY) {
foreach ($propertyValue as $containedObject) {
diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
index 9ba598cd4fbd..2eed541d44e0 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
@@ -383,10 +383,9 @@ class Typo3DbBackend implements BackendInterface, SingletonInterface
$classSchema = $this->reflectionService->getClassSchema($className);
foreach ($classSchema->getDomainObjectProperties() as $property) {
$propertyName = $property->getName();
- $propertyValue = $object->_getProperty($propertyName);
-
// @todo We couple the Backend to the Entity implementation (uid, isClone); changes there breaks this method
if ($dataMap->isPersistableProperty($propertyName) && $propertyName !== AbstractDomainObject::PROPERTY_UID && $propertyName !== AbstractDomainObject::PROPERTY_PID && $propertyName !== 'isClone') {
+ $propertyValue = $object->_getProperty($propertyName);
$fieldName = $dataMap->getColumnMap($propertyName)->getColumnName();
if ($propertyValue === null) {
$whereClause[] = $queryBuilder->expr()->isNull($fieldName);
diff --git a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Classes/Domain/Model/DateTimeImmutableExample.php b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Classes/Domain/Model/DateTimeImmutableExample.php
index e1f8f7633e60..71405d4bc695 100644
--- a/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Classes/Domain/Model/DateTimeImmutableExample.php
+++ b/typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example/Classes/Domain/Model/DateTimeImmutableExample.php
@@ -21,6 +21,24 @@ use TYPO3\CMS\Extbase\DomainObject\AbstractEntity;
class DateTimeImmutableExample extends AbstractEntity
{
+ /**
+ * Static value which is not part of an "entity".
+ * (this property has to be ignored by Extbase when persisting this entity)
+ */
+ public static string $publicStaticValue;
+
+ /**
+ * Transient value, having a name starting with `_`.
+ * (this property has to be ignored by Extbase when persisting this entity)
+ */
+ public string $_publicTransientValue;
+
+ /**
+ * Transient value without any getter or setter.
+ * (this property has to be ignored by Extbase when persisting this entity)
+ */
+ private string $privateTransientValue; // @phpstan-ignore-line since it is unused on purpose
+
/**
* A datetimeImmutable stored in a text field
*/
--
GitLab