Skip to content

Commit a933ba1

Browse files
authored
Merge pull request #47769 from nextcloud/artonge/fix/use_sha256_for_arguments_hash_forbg_jobs
fix: Use sha256 to hash arguments of background jobs
2 parents eac7c2e + dbf56b6 commit a933ba1

File tree

4 files changed

+89
-5
lines changed

4 files changed

+89
-5
lines changed
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
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+
10+
namespace OC\Core\Migrations;
11+
12+
use Closure;
13+
use OCP\DB\ISchemaWrapper;
14+
use OCP\DB\QueryBuilder\IQueryBuilder;
15+
use OCP\IDBConnection;
16+
use OCP\Migration\Attributes\ColumnType;
17+
use OCP\Migration\Attributes\ModifyColumn;
18+
use OCP\Migration\IOutput;
19+
use OCP\Migration\SimpleMigrationStep;
20+
21+
/**
22+
* Migrate the argument_hash column of oc_jobs to use sha256 instead of md5.
23+
*/
24+
#[ModifyColumn(table: 'jobs', name: 'argument_hash', type: ColumnType::STRING, description: 'Increase the column size from 32 to 64')]
25+
#[ModifyColumn(table: 'jobs', name: 'argument_hash', type: ColumnType::STRING, description: 'Rehash the argument_hash column using sha256')]
26+
class Version28000Date20240828142927 extends SimpleMigrationStep {
27+
public function __construct(
28+
protected IDBConnection $connection,
29+
) {
30+
}
31+
32+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
33+
/** @var ISchemaWrapper $schema */
34+
$schema = $schemaClosure();
35+
36+
// Increase the column size from 32 to 64
37+
$table = $schema->getTable('jobs');
38+
$table->modifyColumn('argument_hash', [
39+
'notnull' => false,
40+
'length' => 64,
41+
]);
42+
43+
return $schema;
44+
}
45+
46+
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
47+
$chunkSize = 1000;
48+
$offset = 0;
49+
$nullHash = hash('sha256', 'null');
50+
51+
$selectQuery = $this->connection->getQueryBuilder()
52+
->select('*')
53+
->from('jobs')
54+
->setMaxResults($chunkSize);
55+
56+
$insertQuery = $this->connection->getQueryBuilder();
57+
$insertQuery->update('jobs')
58+
->set('argument_hash', $insertQuery->createParameter('argument_hash'))
59+
->where($insertQuery->expr()->eq('id', $insertQuery->createParameter('id')));
60+
61+
do {
62+
$result = $selectQuery
63+
->setFirstResult($offset)
64+
->executeQuery();
65+
66+
$jobs = $result->fetchAll();
67+
$count = count($jobs);
68+
69+
foreach ($jobs as $jobRow) {
70+
if ($jobRow['argument'] === 'null') {
71+
$hash = $nullHash;
72+
} else {
73+
$hash = hash('sha256', $jobRow['argument']);
74+
}
75+
$insertQuery->setParameter('id', (string)$jobRow['id'], IQueryBuilder::PARAM_INT);
76+
$insertQuery->setParameter('argument_hash', $hash);
77+
$insertQuery->executeStatement();
78+
}
79+
80+
$offset += $chunkSize;
81+
} while ($count === $chunkSize);
82+
}
83+
}

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,7 @@
13641364
'OC\\Core\\Migrations\\Version28000Date20231004103301' => $baseDir . '/core/Migrations/Version28000Date20231004103301.php',
13651365
'OC\\Core\\Migrations\\Version28000Date20231103104802' => $baseDir . '/core/Migrations/Version28000Date20231103104802.php',
13661366
'OC\\Core\\Migrations\\Version28000Date20231126110901' => $baseDir . '/core/Migrations/Version28000Date20231126110901.php',
1367+
'OC\\Core\\Migrations\\Version28000Date20240828142927' => $baseDir . '/core/Migrations/Version28000Date20240828142927.php',
13671368
'OC\\Core\\Migrations\\Version29000Date20231126110901' => $baseDir . '/core/Migrations/Version29000Date20231126110901.php',
13681369
'OC\\Core\\Migrations\\Version29000Date20231213104850' => $baseDir . '/core/Migrations/Version29000Date20231213104850.php',
13691370
'OC\\Core\\Migrations\\Version29000Date20240124132201' => $baseDir . '/core/Migrations/Version29000Date20240124132201.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,6 +1397,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
13971397
'OC\\Core\\Migrations\\Version28000Date20231004103301' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231004103301.php',
13981398
'OC\\Core\\Migrations\\Version28000Date20231103104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231103104802.php',
13991399
'OC\\Core\\Migrations\\Version28000Date20231126110901' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231126110901.php',
1400+
'OC\\Core\\Migrations\\Version28000Date20240828142927' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20240828142927.php',
14001401
'OC\\Core\\Migrations\\Version29000Date20231126110901' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231126110901.php',
14011402
'OC\\Core\\Migrations\\Version29000Date20231213104850' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231213104850.php',
14021403
'OC\\Core\\Migrations\\Version29000Date20240124132201' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240124132201.php',

lib/private/BackgroundJob/JobList.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Psr\Log\LoggerInterface;
2121
use function get_class;
2222
use function json_encode;
23-
use function md5;
2423
use function strlen;
2524

2625
class JobList implements IJobList {
@@ -50,7 +49,7 @@ public function add($job, $argument = null, ?int $firstCheck = null): void {
5049
->values([
5150
'class' => $query->createNamedParameter($class),
5251
'argument' => $query->createNamedParameter($argumentJson),
53-
'argument_hash' => $query->createNamedParameter(md5($argumentJson)),
52+
'argument_hash' => $query->createNamedParameter(hash('sha256', $argumentJson)),
5453
'last_run' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
5554
'last_checked' => $query->createNamedParameter($firstCheck, IQueryBuilder::PARAM_INT),
5655
]);
@@ -60,7 +59,7 @@ public function add($job, $argument = null, ?int $firstCheck = null): void {
6059
->set('last_checked', $query->createNamedParameter($firstCheck, IQueryBuilder::PARAM_INT))
6160
->set('last_run', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))
6261
->where($query->expr()->eq('class', $query->createNamedParameter($class)))
63-
->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(md5($argumentJson))));
62+
->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(hash('sha256', $argumentJson))));
6463
}
6564
$query->executeStatement();
6665
}
@@ -81,7 +80,7 @@ public function remove($job, $argument = null): void {
8180
->where($query->expr()->eq('class', $query->createNamedParameter($class)));
8281
if (!is_null($argument)) {
8382
$argumentJson = json_encode($argument);
84-
$query->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(md5($argumentJson))));
83+
$query->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(hash('sha256', $argumentJson))));
8584
}
8685

8786
// Add galera safe delete chunking if using mysql
@@ -122,7 +121,7 @@ public function has($job, $argument): bool {
122121
$query->select('id')
123122
->from('jobs')
124123
->where($query->expr()->eq('class', $query->createNamedParameter($class)))
125-
->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(md5($argument))))
124+
->andWhere($query->expr()->eq('argument_hash', $query->createNamedParameter(hash('sha256', $argument))))
126125
->setMaxResults(1);
127126

128127
$result = $query->executeQuery();

0 commit comments

Comments
 (0)