Skip to content

Commit f8eaffa

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

12 files changed

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

0 commit comments

Comments
 (0)