Skip to content

Commit 5ceb404

Browse files
committed
fix(systemtags): remove duplicates, prevent and sanitize existing tags
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
1 parent 52a9940 commit 5ceb404

12 files changed

Lines changed: 410 additions & 3 deletions

File tree

apps/dav/lib/SystemTag/SystemTagPlugin.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ private function createTag($data, $contentType = 'application/json') {
162162
throw new BadRequest('Missing "name" attribute');
163163
}
164164

165-
$tagName = $data['name'];
165+
$tagName = Util::sanitizeWordsAndEmojis($data['name']);
166166
$userVisible = true;
167167
$userAssignable = true;
168168

apps/settings/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
'OCA\\Settings\\SetupChecks\\PushService' => $baseDir . '/../lib/SetupChecks/PushService.php',
131131
'OCA\\Settings\\SetupChecks\\RandomnessSecure' => $baseDir . '/../lib/SetupChecks/RandomnessSecure.php',
132132
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => $baseDir . '/../lib/SetupChecks/ReadOnlyConfig.php',
133+
'OCA\\Settings\\SetupChecks\\RepairSanitizeSystemTagsAvailable' => $baseDir . '/../lib/SetupChecks/RepairSanitizeSystemTagsAvailable.php',
133134
'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => $baseDir . '/../lib/SetupChecks/SchedulingTableSize.php',
134135
'OCA\\Settings\\SetupChecks\\SecurityHeaders' => $baseDir . '/../lib/SetupChecks/SecurityHeaders.php',
135136
'OCA\\Settings\\SetupChecks\\SupportedDatabase' => $baseDir . '/../lib/SetupChecks/SupportedDatabase.php',

apps/settings/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ class ComposerStaticInitSettings
145145
'OCA\\Settings\\SetupChecks\\PushService' => __DIR__ . '/..' . '/../lib/SetupChecks/PushService.php',
146146
'OCA\\Settings\\SetupChecks\\RandomnessSecure' => __DIR__ . '/..' . '/../lib/SetupChecks/RandomnessSecure.php',
147147
'OCA\\Settings\\SetupChecks\\ReadOnlyConfig' => __DIR__ . '/..' . '/../lib/SetupChecks/ReadOnlyConfig.php',
148+
'OCA\\Settings\\SetupChecks\\RepairSanitizeSystemTagsAvailable' => __DIR__ . '/..' . '/../lib/SetupChecks/RepairSanitizeSystemTagsAvailable.php',
148149
'OCA\\Settings\\SetupChecks\\SchedulingTableSize' => __DIR__ . '/..' . '/../lib/SetupChecks/SchedulingTableSize.php',
149150
'OCA\\Settings\\SetupChecks\\SecurityHeaders' => __DIR__ . '/..' . '/../lib/SetupChecks/SecurityHeaders.php',
150151
'OCA\\Settings\\SetupChecks\\SupportedDatabase' => __DIR__ . '/..' . '/../lib/SetupChecks/SupportedDatabase.php',

apps/settings/lib/AppInfo/Application.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
use OCA\Settings\SetupChecks\PushService;
6868
use OCA\Settings\SetupChecks\RandomnessSecure;
6969
use OCA\Settings\SetupChecks\ReadOnlyConfig;
70+
use OCA\Settings\SetupChecks\RepairSanitizeSystemTagsAvailable;
7071
use OCA\Settings\SetupChecks\SchedulingTableSize;
7172
use OCA\Settings\SetupChecks\SecurityHeaders;
7273
use OCA\Settings\SetupChecks\SupportedDatabase;
@@ -206,6 +207,7 @@ public function register(IRegistrationContext $context): void {
206207
$context->registerSetupCheck(PhpOutputBuffering::class);
207208
$context->registerSetupCheck(RandomnessSecure::class);
208209
$context->registerSetupCheck(ReadOnlyConfig::class);
210+
$context->registerSetupCheck(RepairSanitizeSystemTagsAvailable::class);
209211
$context->registerSetupCheck(SecurityHeaders::class);
210212
$context->registerSetupCheck(SchedulingTableSize::class);
211213
$context->registerSetupCheck(SupportedDatabase::class);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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 OCA\Settings\SetupChecks;
10+
11+
use OC\Repair\RepairSanitizeSystemTags;
12+
use OCP\IL10N;
13+
use OCP\SetupCheck\ISetupCheck;
14+
use OCP\SetupCheck\SetupResult;
15+
16+
class RepairSanitizeSystemTagsAvailable implements ISetupCheck {
17+
18+
public function __construct(
19+
private RepairSanitizeSystemTags $repairSanitizeSystemTags,
20+
private IL10N $l10n,
21+
) {
22+
}
23+
24+
public function getCategory(): string {
25+
return 'system';
26+
}
27+
28+
public function getName(): string {
29+
return $this->l10n->t('Sanitize and merge duplicate system tags available');
30+
}
31+
32+
public function run(): SetupResult {
33+
if ($this->repairSanitizeSystemTags->migrationsAvailable()) {
34+
return SetupResult::warning(
35+
$this->l10n->t('One or more system tags need to be sanitized or merged. This can take a long time on larger instances so this is not done automatically during upgrades. Use the command `occ maintenance:repair --include-expensive` to perform the migrations.'),
36+
);
37+
} else {
38+
return SetupResult::success('None');
39+
}
40+
}
41+
}

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,6 +2007,7 @@
20072007
'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',
20082008
'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php',
20092009
'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php',
2010+
'OC\\Repair\\RepairSanitizeSystemTags' => $baseDir . '/lib/private/Repair/RepairSanitizeSystemTags.php',
20102011
'OC\\RichObjectStrings\\RichTextFormatter' => $baseDir . '/lib/private/RichObjectStrings/RichTextFormatter.php',
20112012
'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php',
20122013
'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,6 +2048,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
20482048
'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',
20492049
'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php',
20502050
'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php',
2051+
'OC\\Repair\\RepairSanitizeSystemTags' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairSanitizeSystemTags.php',
20512052
'OC\\RichObjectStrings\\RichTextFormatter' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/RichTextFormatter.php',
20522053
'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php',
20532054
'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php',

lib/private/Repair.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
use OC\Repair\RepairInvalidShares;
5858
use OC\Repair\RepairLogoDimension;
5959
use OC\Repair\RepairMimeTypes;
60+
use OC\Repair\RepairSanitizeSystemTags;
6061
use OC\Template\JSCombiner;
6162
use OCA\DAV\Migration\DeleteSchedulingObjects;
6263
use OCA\DAV\Migration\RemoveObjectProperties;
@@ -221,6 +222,7 @@ public static function getExpensiveRepairSteps() {
221222
),
222223
\OCP\Server::get(DeleteSchedulingObjects::class),
223224
\OC::$server->get(RemoveObjectProperties::class),
225+
\OCP\Server::get(RepairSanitizeSystemTags::class),
224226
];
225227
}
226228

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-only
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace OC\Repair;
11+
12+
use OC\Migration\NullOutput;
13+
use OCP\DB\QueryBuilder\IQueryBuilder;
14+
use OCP\IDBConnection;
15+
use OCP\Migration\IOutput;
16+
use OCP\Migration\IRepairStep;
17+
use OCP\Util;
18+
19+
class RepairSanitizeSystemTags implements IRepairStep {
20+
private bool $dryRun = false;
21+
private int $changeCount = 0;
22+
23+
public function __construct(
24+
protected IDBConnection $connection,
25+
) {
26+
}
27+
28+
public function getName(): string {
29+
return 'Sanitize and merge duplicate system tags';
30+
}
31+
32+
public function migrationsAvailable(): bool {
33+
$this->dryRun = true;
34+
$this->run(new NullOutput());
35+
$this->dryRun = false;
36+
return $this->changeCount > 0;
37+
}
38+
39+
public function run(IOutput $output): void {
40+
$this->dryRun = false;
41+
$this->sanitizeAndMergeTags($output);
42+
}
43+
44+
private function sanitizeAndMergeTags(IOutput $output): void {
45+
$output->info('Starting sanitization of system tags...');
46+
47+
$tags = $this->getAllTags();
48+
49+
// Group tags by sanitized name
50+
$sanitizedMap = [];
51+
foreach ($tags as $tag) {
52+
$sanitizedMap[$tag['sanitizedName']][] = $tag;
53+
}
54+
55+
$initialCount = count($tags);
56+
$sanitizeCount = count($sanitizedMap);
57+
$output->info("Found $initialCount tags with $sanitizeCount unique sanitized names.");
58+
59+
// Get object counts for all tags in one query
60+
$objectCounts = $this->getAllTagObjectCounts();
61+
62+
// Process each sanitized name group
63+
foreach ($sanitizedMap as $sanitizedName => $group) {
64+
if (count($group) === 1) {
65+
// Single tag, check if another tag already has this sanitized name
66+
$tag = $group[0];
67+
if ($tag['originalName'] !== $sanitizedName) {
68+
// Check if the sanitized name would conflict with another existing tag
69+
if ($this->tagNameExists($sanitizedName, $tag['id'])) {
70+
$output->warning("Cannot sanitize tag ID {$tag['id']}: name '$sanitizedName' already exists");
71+
continue;
72+
}
73+
74+
if (!$this->dryRun) {
75+
$qb = $this->connection->getQueryBuilder();
76+
$qb->update('systemtag')
77+
->set('name', $qb->createNamedParameter($sanitizedName))
78+
->where($qb->expr()->eq('id', $qb->createNamedParameter($tag['id'])))
79+
->executeStatement();
80+
}
81+
$this->changeCount++;
82+
$output->info("Sanitized tag ID {$tag['id']}: '{$tag['originalName']}' → '$sanitizedName'");
83+
}
84+
continue;
85+
}
86+
87+
// Multiple tags with same sanitized name - merge them
88+
$this->mergeTagGroup($group, $sanitizedName, $objectCounts, $output);
89+
}
90+
91+
$output->info('System tag sanitization and merge completed.');
92+
}
93+
94+
private function mergeTagGroup(array $group, string $sanitizedName, array $objectCounts, IOutput $output): void {
95+
// Validate that all tags in the group have the same visibility and editable settings
96+
$firstTag = $group[0];
97+
$visibility = $firstTag['visibility'];
98+
$editable = $firstTag['editable'];
99+
100+
foreach ($group as $tag) {
101+
if ($tag['visibility'] !== $visibility || $tag['editable'] !== $editable) {
102+
$output->warning(
103+
"Cannot merge tag group '$sanitizedName': tags have different visibility or editable settings. "
104+
. 'Manual verification required. Tag IDs: ' . implode(', ', array_column($group, 'id'))
105+
);
106+
return;
107+
}
108+
}
109+
110+
// Determine which tag to keep (most object mappings, then lowest ID as tiebreaker)
111+
$keepTag = null;
112+
$maxCount = -1;
113+
114+
foreach ($group as $tag) {
115+
$count = $objectCounts[$tag['id']] ?? 0;
116+
if ($count > $maxCount || ($count === $maxCount && ($keepTag === null || $tag['id'] < $keepTag['id']))) {
117+
$maxCount = $count;
118+
$keepTag = $tag;
119+
}
120+
}
121+
122+
$keepId = $keepTag['id'];
123+
$duplicateIds = array_filter(array_column($group, 'id'), fn ($id) => $id !== $keepId);
124+
125+
if (empty($duplicateIds)) {
126+
return;
127+
}
128+
129+
if (!$this->dryRun) {
130+
$this->connection->beginTransaction();
131+
try {
132+
// Step 1: Delete ALL mappings from duplicate tags that conflict with keepId
133+
// This must happen FIRST before any updates to avoid unique constraint violations
134+
$this->deleteConflictingMappings($duplicateIds, $keepId);
135+
136+
// Step 2: Update all remaining mappings from duplicates to keepId
137+
// These won't conflict because we just deleted the conflicts
138+
$qb = $this->connection->getQueryBuilder();
139+
$qb->update('systemtag_object_mapping')
140+
->set('systemtagid', $qb->createNamedParameter($keepId))
141+
->where($qb->expr()->in('systemtagid', $qb->createNamedParameter($duplicateIds, IQueryBuilder::PARAM_INT_ARRAY)))
142+
->executeStatement();
143+
144+
// Step 3: Delete duplicate tags in bulk (safe now that mappings are gone)
145+
$qb = $this->connection->getQueryBuilder();
146+
$qb->delete('systemtag')
147+
->where($qb->expr()->in('id', $qb->createNamedParameter($duplicateIds, IQueryBuilder::PARAM_INT_ARRAY)))
148+
->executeStatement();
149+
150+
// Step 4: Sanitize the kept tag name if needed
151+
// This is safe because we've already deleted all duplicates with the same sanitized name
152+
if ($keepTag['originalName'] !== $sanitizedName) {
153+
$qb = $this->connection->getQueryBuilder();
154+
$qb->update('systemtag')
155+
->set('name', $qb->createNamedParameter($sanitizedName))
156+
->where($qb->expr()->eq('id', $qb->createNamedParameter($keepId)))
157+
->executeStatement();
158+
}
159+
160+
$this->connection->commit();
161+
} catch (\Exception $e) {
162+
$this->connection->rollBack();
163+
$output->warning("Failed to merge tag group '$sanitizedName': " . $e->getMessage());
164+
return;
165+
}
166+
}
167+
168+
$this->changeCount += count($duplicateIds);
169+
if ($keepTag['originalName'] !== $sanitizedName) {
170+
$this->changeCount++;
171+
}
172+
173+
$duplicateIdsList = implode(', ', $duplicateIds);
174+
$output->info("Merged tags [$duplicateIdsList] into ID $keepId (sanitized: '$sanitizedName')");
175+
}
176+
177+
/**
178+
* Delete mappings from duplicate tags where the same object is already mapped to keepId
179+
* This prevents unique constraint violations when updating systemtagid
180+
*/
181+
private function deleteConflictingMappings(array $duplicateIds, int $keepId): void {
182+
// For better performance with millions of rows, process in batches
183+
$batchSize = 1000;
184+
$offset = 0;
185+
186+
while (true) {
187+
// Get a batch of objects that are mapped to keepId
188+
$qb = $this->connection->getQueryBuilder();
189+
$qb->select('objectid', 'objecttype')
190+
->from('systemtag_object_mapping')
191+
->where($qb->expr()->eq('systemtagid', $qb->createNamedParameter($keepId)))
192+
->setMaxResults($batchSize)
193+
->setFirstResult($offset);
194+
195+
$result = $qb->executeQuery();
196+
$conflicts = $result->fetchAll();
197+
$result->closeCursor();
198+
199+
if (empty($conflicts)) {
200+
break;
201+
}
202+
203+
// Delete mappings from duplicate tags for these objects
204+
foreach ($conflicts as $conflict) {
205+
$qb = $this->connection->getQueryBuilder();
206+
$qb->delete('systemtag_object_mapping')
207+
->where($qb->expr()->in('systemtagid', $qb->createNamedParameter($duplicateIds, IQueryBuilder::PARAM_INT_ARRAY)))
208+
->andWhere($qb->expr()->eq('objectid', $qb->createNamedParameter($conflict['objectid'])))
209+
->andWhere($qb->expr()->eq('objecttype', $qb->createNamedParameter($conflict['objecttype'])))
210+
->executeStatement();
211+
}
212+
213+
if (count($conflicts) < $batchSize) {
214+
break;
215+
}
216+
217+
$offset += $batchSize;
218+
}
219+
}
220+
221+
/**
222+
* Check if a tag name already exists (excluding a specific tag ID)
223+
*/
224+
private function tagNameExists(string $name, int $excludeId): bool {
225+
$qb = $this->connection->getQueryBuilder();
226+
$qb->select('id')
227+
->from('systemtag')
228+
->where($qb->expr()->eq('name', $qb->createNamedParameter($name)))
229+
->andWhere($qb->expr()->neq('id', $qb->createNamedParameter($excludeId)))
230+
->setMaxResults(1);
231+
232+
$result = $qb->executeQuery();
233+
$exists = $result->fetch() !== false;
234+
$result->closeCursor();
235+
236+
return $exists;
237+
}
238+
239+
// Fetch all tags
240+
private function getAllTags(): array {
241+
$qb = $this->connection->getQueryBuilder();
242+
$qb->select('id', 'name', 'visibility', 'editable')
243+
->from('systemtag')
244+
->orderBy('name')
245+
->addOrderBy('id');
246+
247+
$result = $qb->executeQuery();
248+
$tags = [];
249+
250+
while ($row = $result->fetch()) {
251+
$tags[] = [
252+
'id' => (int)$row['id'],
253+
'originalName' => $row['name'],
254+
'sanitizedName' => Util::sanitizeWordsAndEmojis($row['name']),
255+
'visibility' => (int)$row['visibility'],
256+
'editable' => (int)$row['editable'],
257+
];
258+
}
259+
$result->closeCursor();
260+
return $tags;
261+
}
262+
263+
// Get object counts for all tags in one efficient query
264+
private function getAllTagObjectCounts(): array {
265+
$qb = $this->connection->getQueryBuilder();
266+
$qb->select('systemtagid')
267+
->selectAlias($qb->createFunction('COUNT(*)'), 'cnt')
268+
->from('systemtag_object_mapping')
269+
->groupBy('systemtagid');
270+
271+
$result = $qb->executeQuery();
272+
$counts = [];
273+
274+
while ($row = $result->fetch()) {
275+
$counts[(int)$row['systemtagid']] = (int)$row['cnt'];
276+
}
277+
$result->closeCursor();
278+
279+
return $counts;
280+
}
281+
}

0 commit comments

Comments
 (0)