Skip to content

Commit c1dd8dd

Browse files
fix: replace null character when serializing
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
1 parent 10852d3 commit c1dd8dd

File tree

6 files changed

+96
-2
lines changed

6 files changed

+96
-2
lines changed

apps/dav/lib/DAV/CustomPropertiesBackend.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,9 @@ private function encodeValueForDatabase(string $path, string $name, mixed $value
519519
$value = $value->getHref();
520520
} else {
521521
$valueType = self::PROPERTY_TYPE_OBJECT;
522-
$value = serialize($value);
522+
// serialize produces null character
523+
// these can not be properly stored in some databases and need to be replaced
524+
$value = str_replace(chr(0), '\x00', serialize($value));
523525
}
524526
return [$value, $valueType];
525527
}
@@ -534,7 +536,9 @@ private function decodeValueFromDatabase(string $value, int $valueType) {
534536
case self::PROPERTY_TYPE_HREF:
535537
return new Href($value);
536538
case self::PROPERTY_TYPE_OBJECT:
537-
return unserialize($value);
539+
// some databases can not handel null characters, these are custom encoded during serialization
540+
// this custom encoding needs to be first reversed before unserializing
541+
return unserialize(str_replace('\x00', chr(0), $value));
538542
case self::PROPERTY_TYPE_STRING:
539543
default:
540544
return $value;

apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/**
34
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
45
* SPDX-License-Identifier: AGPL-3.0-or-later
@@ -458,4 +459,21 @@ public function moveProvider() {
458459
[str_repeat('long_path1', 100), str_repeat('long_path2', 100)]
459460
];
460461
}
462+
463+
public function testDecodeValueFromDatabaseObjectCurrent(): void {
464+
$propertyValue = 'O:48:"Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp":1:{s:8:"\x00*\x00value";s:6:"opaque";}';
465+
$propertyType = 3;
466+
$decodeValue = $this->invokePrivate($this->backend, 'decodeValueFromDatabase', [$propertyValue, $propertyType]);
467+
$this->assertInstanceOf(\Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp::class, $decodeValue);
468+
$this->assertEquals('opaque', $decodeValue->getValue());
469+
}
470+
471+
public function testDecodeValueFromDatabaseObjectLegacy(): void {
472+
$propertyValue = 'O:48:"Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp":1:{s:8:"' . chr(0) . '*' . chr(0) . 'value";s:6:"opaque";}';
473+
$propertyType = 3;
474+
$decodeValue = $this->invokePrivate($this->backend, 'decodeValueFromDatabase', [$propertyValue, $propertyType]);
475+
$this->assertInstanceOf(\Sabre\CalDAV\Xml\Property\ScheduleCalendarTransp::class, $decodeValue);
476+
$this->assertEquals('opaque', $decodeValue->getValue());
477+
}
478+
461479
}

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1876,6 +1876,7 @@
18761876
'OC\\Repair\\Owncloud\\MoveAvatarsBackgroundJob' => $baseDir . '/lib/private/Repair/Owncloud/MoveAvatarsBackgroundJob.php',
18771877
'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
18781878
'OC\\Repair\\Owncloud\\UpdateLanguageCodes' => $baseDir . '/lib/private/Repair/Owncloud/UpdateLanguageCodes.php',
1879+
'OC\\Repair\\RemoveBrokenProperties' => $baseDir . '/lib/private/Repair/RemoveBrokenProperties.php',
18791880
'OC\\Repair\\RemoveLinkShares' => $baseDir . '/lib/private/Repair/RemoveLinkShares.php',
18801881
'OC\\Repair\\RepairDavShares' => $baseDir . '/lib/private/Repair/RepairDavShares.php',
18811882
'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1917,6 +1917,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19171917
'OC\\Repair\\Owncloud\\MoveAvatarsBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/MoveAvatarsBackgroundJob.php',
19181918
'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
19191919
'OC\\Repair\\Owncloud\\UpdateLanguageCodes' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/UpdateLanguageCodes.php',
1920+
'OC\\Repair\\RemoveBrokenProperties' => __DIR__ . '/../../..' . '/lib/private/Repair/RemoveBrokenProperties.php',
19201921
'OC\\Repair\\RemoveLinkShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RemoveLinkShares.php',
19211922
'OC\\Repair\\RepairDavShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairDavShares.php',
19221923
'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',

lib/private/Repair.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use OC\Repair\Owncloud\MoveAvatars;
5151
use OC\Repair\Owncloud\SaveAccountsTableData;
5252
use OC\Repair\Owncloud\UpdateLanguageCodes;
53+
use OC\Repair\RemoveBrokenProperties;
5354
use OC\Repair\RemoveLinkShares;
5455
use OC\Repair\RepairDavShares;
5556
use OC\Repair\RepairInvalidShares;
@@ -206,6 +207,7 @@ public static function getRepairSteps(): array {
206207
public static function getExpensiveRepairSteps() {
207208
return [
208209
new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()),
210+
new RemoveBrokenProperties(\OCP\Server::get(IDBConnection::class)),
209211
new RepairMimeTypes(
210212
\OCP\Server::get(IConfig::class),
211213
\OCP\Server::get(IAppConfig::class),
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Repair;
10+
11+
use OCP\DB\QueryBuilder\IQueryBuilder;
12+
use OCP\IDBConnection;
13+
use OCP\Migration\IOutput;
14+
use OCP\Migration\IRepairStep;
15+
16+
class RemoveBrokenProperties implements IRepairStep {
17+
/**
18+
* RemoveBrokenProperties constructor.
19+
*
20+
* @param IDBConnection $db
21+
*/
22+
public function __construct(
23+
private IDBConnection $db,
24+
) {
25+
}
26+
27+
/**
28+
* @inheritdoc
29+
*/
30+
public function getName() {
31+
return 'Remove broken DAV object properties';
32+
}
33+
34+
/**
35+
* @inheritdoc
36+
*/
37+
public function run(IOutput $output) {
38+
// retrieve all object properties
39+
$qb = $this->db->getQueryBuilder();
40+
$qb->select('id', 'propertyvalue')
41+
->from('properties')
42+
->where($qb->expr()->eq('valuetype', $qb->createNamedParameter('3', IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
43+
$result = $qb->executeQuery();
44+
// find broken object properties
45+
$brokenIds = [];
46+
while ($entry = $result->fetch()) {
47+
if (!empty($entry['propertyvalue'])) {
48+
$object = @unserialize(str_replace('\x00', chr(0), $entry['propertyvalue']));
49+
if ($object === false) {
50+
$brokenIds[] = $entry['id'];
51+
}
52+
} else {
53+
$brokenIds[] = $entry['id'];
54+
}
55+
}
56+
$result->closeCursor();
57+
// delete broken object properties
58+
$qb = $this->db->getQueryBuilder();
59+
$qb->delete('properties')
60+
->where($qb->expr()->in('id', $qb->createParameter('ids'), IQueryBuilder::PARAM_STR_ARRAY));
61+
foreach (array_chunk($brokenIds, 1000) as $chunkIds) {
62+
$qb->setParameter('ids', $chunkIds, IQueryBuilder::PARAM_STR_ARRAY);
63+
$qb->executeStatement();
64+
}
65+
$total = count($brokenIds);
66+
$output->info("$total broken object properties removed");
67+
}
68+
}

0 commit comments

Comments
 (0)