Merge pull request #49855 from nextcloud/backport/49528/stable30

[stable30] fix: replace null character when serializing

6 files changed, 96 insertions, 2 deletions

diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php
index dde97cabf37..9485e4fe7f4 100644
--- a/apps/dav/lib/DAV/CustomPropertiesBackend.php
+++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php
@@ -542,7 +542,9 @@ class CustomPropertiesBackend implements BackendInterface {
 			$value = $value->getHref();
 		} else {
 			$valueType = self::PROPERTY_TYPE_OBJECT;
-			$value = serialize($value);
+			// serialize produces null character
+			// these can not be properly stored in some databases and need to be replaced
+			$value = str_replace(chr(0), '\x00', serialize($value));
 		}
 		return [$value, $valueType];
 	}
@@ -557,7 +559,9 @@ class CustomPropertiesBackend implements BackendInterface {
 			case self::PROPERTY_TYPE_HREF:
 				return new Href($value);
 			case self::PROPERTY_TYPE_OBJECT:
-				return unserialize($value);
+				// some databases can not handel null characters, these are custom encoded during serialization
+				// this custom encoding needs to be first reversed before unserializing
+				return unserialize(str_replace('\x00', chr(0), $value));
 			case self::PROPERTY_TYPE_STRING:
 			default:
 				return $value;
diff --git a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
index 3c527e24363..606632fa45d 100644
--- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
+++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -458,4 +459,21 @@ class CustomPropertiesBackendTest extends TestCase {
 			[str_repeat('long_path1', 100), str_repeat('long_path2', 100)]
 		];
 	}
+
+	public function testDecodeValueFromDatabaseObjectCurrent(): void {
+		$propertyValue = 'O:48:"Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp":1:{s:8:"\x00*\x00value";s:6:"opaque";}';
+		$propertyType = 3;
+		$decodeValue = $this->invokePrivate($this->backend, 'decodeValueFromDatabase', [$propertyValue, $propertyType]);
+		$this->assertInstanceOf(\Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp::class, $decodeValue);
+		$this->assertEquals('opaque', $decodeValue->getValue());
+	}
+
+	public function testDecodeValueFromDatabaseObjectLegacy(): void {
+		$propertyValue = 'O:48:"Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp":1:{s:8:"' . chr(0) . '*' . chr(0) . 'value";s:6:"opaque";}';
+		$propertyType = 3;
+		$decodeValue = $this->invokePrivate($this->backend, 'decodeValueFromDatabase', [$propertyValue, $propertyType]);
+		$this->assertInstanceOf(\Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp::class, $decodeValue);
+		$this->assertEquals('opaque', $decodeValue->getValue());
+	}
+
 }
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index ef2b9ee2386..b6b479028d6 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -1829,6 +1829,7 @@ return array(
     'OC\\Repair\\Owncloud\\MoveAvatarsBackgroundJob' => $baseDir . '/lib/private/Repair/Owncloud/MoveAvatarsBackgroundJob.php',
     'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
     'OC\\Repair\\Owncloud\\UpdateLanguageCodes' => $baseDir . '/lib/private/Repair/Owncloud/UpdateLanguageCodes.php',
+    'OC\\Repair\\RemoveBrokenProperties' => $baseDir . '/lib/private/Repair/RemoveBrokenProperties.php',
     'OC\\Repair\\RemoveLinkShares' => $baseDir . '/lib/private/Repair/RemoveLinkShares.php',
     'OC\\Repair\\RepairDavShares' => $baseDir . '/lib/private/Repair/RepairDavShares.php',
     'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 41012d9373a..b8ca8eeaa15 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -1862,6 +1862,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Repair\\Owncloud\\MoveAvatarsBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/MoveAvatarsBackgroundJob.php',
         'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
         'OC\\Repair\\Owncloud\\UpdateLanguageCodes' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/UpdateLanguageCodes.php',
+        'OC\\Repair\\RemoveBrokenProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/RemoveBrokenProperties.php',
         'OC\\Repair\\RemoveLinkShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RemoveLinkShares.php',
         'OC\\Repair\\RepairDavShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairDavShares.php',
         'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',
diff --git a/lib/private/Repair.php b/lib/private/Repair.php
index 71e43541872..6c5ec65bf26 100644
--- a/lib/private/Repair.php
+++ b/lib/private/Repair.php
@@ -50,6 +50,7 @@ use OC\Repair\Owncloud\MigrateOauthTables;
 use OC\Repair\Owncloud\MoveAvatars;
 use OC\Repair\Owncloud\SaveAccountsTableData;
 use OC\Repair\Owncloud\UpdateLanguageCodes;
+use OC\Repair\RemoveBrokenProperties;
 use OC\Repair\RemoveLinkShares;
 use OC\Repair\RepairDavShares;
 use OC\Repair\RepairInvalidShares;
@@ -203,6 +204,7 @@ class Repair implements IOutput {
 	public static function getExpensiveRepairSteps() {
 		return [
 			new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()),
+			new RemoveBrokenProperties(\OC::$server->getDatabaseConnection()),
 			new RepairMimeTypes(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection()),
 			\OC::$server->get(ValidatePhoneNumber::class),
 			\OC::$server->get(DeleteSchedulingObjects::class),
diff --git a/lib/private/Repair/RemoveBrokenProperties.php b/lib/private/Repair/RemoveBrokenProperties.php
new file mode 100644
index 00000000000..85939b39e5e
--- /dev/null
+++ b/lib/private/Repair/RemoveBrokenProperties.php
@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Repair;
+
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class RemoveBrokenProperties implements IRepairStep {
+	/**
+	 * RemoveBrokenProperties constructor.
+	 *
+	 * @param IDBConnection $db
+	 */
+	public function __construct(
+		private IDBConnection $db,
+	) {
+	}
+
+	/**
+	 * @inheritdoc
+	 */
+	public function getName() {
+		return 'Remove broken DAV object properties';
+	}
+
+	/**
+	 * @inheritdoc
+	 */
+	public function run(IOutput $output) {
+		// retrieve all object properties
+		$qb = $this->db->getQueryBuilder();
+		$qb->select('id', 'propertyvalue')
+			->from('properties')
+			->where($qb->expr()->eq('valuetype', $qb->createNamedParameter('3', IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
+		$result = $qb->executeQuery();
+		// find broken object properties
+		$brokenIds = [];
+		while ($entry = $result->fetch()) {
+			if (!empty($entry['propertyvalue'])) {
+				$object = @unserialize(str_replace('\x00', chr(0), $entry['propertyvalue']));
+				if ($object === false) {
+					$brokenIds[] = $entry['id'];
+				}
+			} else {
+				$brokenIds[] = $entry['id'];
+			}
+		}
+		$result->closeCursor();
+		// delete broken object properties
+		$qb = $this->db->getQueryBuilder();
+		$qb->delete('properties')
+			->where($qb->expr()->in('id', $qb->createParameter('ids'), IQueryBuilder::PARAM_STR_ARRAY));
+		foreach (array_chunk($brokenIds, 1000) as $chunkIds) {
+			$qb->setParameter('ids', $chunkIds, IQueryBuilder::PARAM_STR_ARRAY);
+			$qb->executeStatement();
+		}
+		$total = count($brokenIds);
+		$output->info("$total broken object properties removed");
+	}
+}