From c1dd8ddf591337c5fb5ca2da7d476a4f0fc254c5 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Wed, 27 Nov 2024 22:16:58 -0500 Subject: [PATCH] fix: replace null character when serializing Signed-off-by: SebastianKrupinski --- apps/dav/lib/DAV/CustomPropertiesBackend.php | 8 ++- .../unit/DAV/CustomPropertiesBackendTest.php | 18 +++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Repair.php | 2 + lib/private/Repair/RemoveBrokenProperties.php | 68 +++++++++++++++++++ 6 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 lib/private/Repair/RemoveBrokenProperties.php diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php index f73246161baa0..bc3c146875ba2 100644 --- a/apps/dav/lib/DAV/CustomPropertiesBackend.php +++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php @@ -519,7 +519,9 @@ private function encodeValueForDatabase(string $path, string $name, mixed $value $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]; } @@ -534,7 +536,9 @@ private function decodeValueFromDatabase(string $value, int $valueType) { 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 6fc87437fe058..fa71fd8016bf0 100644 --- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php +++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php @@ -1,4 +1,5 @@ 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 e515e3eff07e4..528bf3bb2dacb 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1876,6 +1876,7 @@ '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 cf2883c3070c5..43b5042c8b4eb 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1917,6 +1917,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 b1a824ba5e324..5b6daa36a0380 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -50,6 +50,7 @@ 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; @@ -206,6 +207,7 @@ public static function getRepairSteps(): array { public static function getExpensiveRepairSteps() { return [ new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()), + new RemoveBrokenProperties(\OCP\Server::get(IDBConnection::class)), new RepairMimeTypes( \OCP\Server::get(IConfig::class), \OCP\Server::get(IAppConfig::class), diff --git a/lib/private/Repair/RemoveBrokenProperties.php b/lib/private/Repair/RemoveBrokenProperties.php new file mode 100644 index 0000000000000..85939b39e5e65 --- /dev/null +++ b/lib/private/Repair/RemoveBrokenProperties.php @@ -0,0 +1,68 @@ +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"); + } +}