Skip to content

Commit 8b89666

Browse files
solracsfsolracsf
committed
fix(files/cache): uniqueness guarantees in the file cache layer
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent f8dbe23 commit 8b89666

3 files changed

Lines changed: 54 additions & 40 deletions

File tree

‎lib/private/Files/Cache/Cache.php‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,6 @@ public function calculateFolderSize($path, $entry = null) {
10151015
/**
10161016
* inner function because we can't add new params to the public function without breaking any child classes
10171017
*
1018-
* @param string $path
10191018
* @param array|null|ICacheEntry $entry (optional) meta data of the folder
10201019
* @param bool $ignoreUnknown don't mark the folder size as unknown if any of it's children are unknown
10211020
* @return int|float
@@ -1029,7 +1028,9 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10291028
$id = $entry['fileid'];
10301029

10311030
$query = $this->getQueryBuilder();
1032-
$query->select('size', 'unencrypted_size')
1031+
$query->selectAlias($query->func()->sum('size'), 'size_sum')
1032+
->selectAlias($query->func()->min('size'), 'size_min')
1033+
->selectAlias($query->func()->max('unencrypted_size'), 'unencrypted_max')
10331034
->from('filecache')
10341035
->whereStorageId($this->getNumericStorageId())
10351036
->whereParent($id);
@@ -1038,34 +1039,40 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10381039
}
10391040

10401041
$result = $query->executeQuery();
1041-
$rows = $result->fetchAll();
1042+
$agg = $result->fetch();
10421043
$result->closeCursor();
10431044

1044-
if ($rows) {
1045-
$sizes = array_map(function (array $row) {
1046-
return Util::numericToNumber($row['size']);
1047-
}, $rows);
1048-
$unencryptedOnlySizes = array_map(function (array $row) {
1049-
return Util::numericToNumber($row['unencrypted_size']);
1050-
}, $rows);
1045+
// SUM() returns NULL on empty set
1046+
if ($agg && $agg['size_sum'] !== null) {
1047+
$sum = Util::numericToNumber($agg['size_sum']);
1048+
$min = Util::numericToNumber($agg['size_min']);
1049+
$unencryptedMax = Util::numericToNumber($agg['unencrypted_max'] ?? 0);
1050+
1051+
$sum = 0 + $sum;
1052+
$min = 0 + $min;
1053+
$totalSize = ($min === -1) ? $min : $sum;
1054+
1055+
$query = $this->getQueryBuilder();
1056+
$query->select('size', 'unencrypted_size')
1057+
->from('filecache')
1058+
->whereStorageId($this->getNumericStorageId())
1059+
->whereParent($id);
1060+
if ($ignoreUnknown) {
1061+
$query->andWhere($query->expr()->gte('size', $query->createNamedParameter(0)));
1062+
}
1063+
$result = $query->executeQuery();
1064+
$rows = $result->fetchAll();
1065+
$result->closeCursor();
1066+
10511067
$unencryptedSizes = array_map(function (array $row) {
1052-
return Util::numericToNumber(($row['unencrypted_size'] > 0) ? $row['unencrypted_size'] : $row['size']);
1068+
$u = Util::numericToNumber($row['unencrypted_size']);
1069+
$s = Util::numericToNumber($row['size']);
1070+
return ($u > 0) ? $u : $s;
10531071
}, $rows);
10541072

1055-
$sum = array_sum($sizes);
1056-
$min = min($sizes);
1057-
10581073
$unencryptedSum = array_sum($unencryptedSizes);
1059-
$unencryptedMin = min($unencryptedSizes);
1060-
$unencryptedMax = max($unencryptedOnlySizes);
1074+
$unencryptedMin = $unencryptedSizes ? min($unencryptedSizes) : 0;
10611075

1062-
$sum = 0 + $sum;
1063-
$min = 0 + $min;
1064-
if ($min === -1) {
1065-
$totalSize = $min;
1066-
} else {
1067-
$totalSize = $sum;
1068-
}
10691076
if ($unencryptedMin === -1 || $min === -1) {
10701077
$unencryptedTotal = $unencryptedMin;
10711078
} else {
@@ -1077,15 +1084,16 @@ protected function calculateFolderSizeInner(string $path, $entry = null, bool $i
10771084
$unencryptedMax = 0;
10781085
}
10791086

1080-
// only set unencrypted size for a folder if any child entries have it set, or the folder is empty
1087+
// only set unencrypted size for a folder if any child entries have it set
1088+
// or if the folder is empty
10811089
$shouldWriteUnEncryptedSize = $unencryptedMax > 0 || $totalSize === 0 || ($entry['unencrypted_size'] ?? 0) > 0;
10821090
if ($entry['size'] !== $totalSize || (($entry['unencrypted_size'] ?? 0) !== $unencryptedTotal && $shouldWriteUnEncryptedSize)) {
10831091
if ($shouldWriteUnEncryptedSize) {
1084-
// if all children have an unencrypted size of 0, just set the folder unencrypted size to 0 instead of summing the sizes
1092+
// if all children have an unencrypted size of 0
1093+
// just set the folder unencrypted size to 0 instead of summing the sizes
10851094
if ($unencryptedMax === 0) {
10861095
$unencryptedTotal = 0;
10871096
}
1088-
10891097
$this->update($id, [
10901098
'size' => $totalSize,
10911099
'unencrypted_size' => $unencryptedTotal,

‎lib/private/Files/Cache/Propagator.php‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public function propagateChange(string $internalPath, int $time, int $sizeDiffer
7575

7676
$parentHashes = array_map('md5', $parents);
7777
sort($parentHashes); // Ensure rows are always locked in the same order
78-
$etag = uniqid(); // since we give all folders the same etag we don't ask the storage for the etag
78+
$etag = bin2hex(random_bytes(8)); // since we give all folders the same etag we don't ask the storage for the etag
7979

8080
$builder = $this->connection->getQueryBuilder();
8181
$hashParams = array_map(static fn (string $hash): ILiteral => $builder->expr()->literal($hash), $parentHashes);
@@ -218,29 +218,32 @@ public function commitBatch(): void {
218218
$query = $this->connection->getQueryBuilder();
219219
$query->update('filecache')
220220
->set('mtime', $query->func()->greatest('mtime', $query->createParameter('time')))
221-
->set('etag', $query->expr()->literal(uniqid()))
221+
->set('etag', $query->createNamedParameter('', IQueryBuilder::PARAM_STR))
222222
->where($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
223223
->andWhere($query->expr()->eq('fileid', $query->createParameter('fileid')));
224224

225225
$queryWithSize = $this->connection->getQueryBuilder();
226226
$queryWithSize->update('filecache')
227227
->set('mtime', $queryWithSize->func()->greatest('mtime', $queryWithSize->createParameter('time')))
228-
->set('etag', $queryWithSize->expr()->literal(uniqid()))
228+
->set('etag', $queryWithSize->createNamedParameter('', IQueryBuilder::PARAM_STR))
229229
->set('size', $queryWithSize->func()->add('size', $queryWithSize->createParameter('size')))
230230
->where($queryWithSize->expr()->eq('storage', $queryWithSize->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
231231
->andWhere($queryWithSize->expr()->eq('fileid', $queryWithSize->createParameter('fileid')));
232232

233233
while ($row = $result->fetchAssociative()) {
234234
$item = $this->batch[$row['path']];
235+
$newEtag = bin2hex(random_bytes(8));
235236
if ($item['size'] && $row['size'] > -1) {
236237
$queryWithSize->setParameter('fileid', $row['fileid'], IQueryBuilder::PARAM_INT)
237238
->setParameter('size', $item['size'], IQueryBuilder::PARAM_INT)
238-
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT)
239-
->executeStatement();
239+
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT);
240+
$queryWithSize->set('etag', $queryWithSize->createNamedParameter($newEtag, IQueryBuilder::PARAM_STR));
241+
$queryWithSize->executeStatement();
240242
} else {
241243
$query->setParameter('fileid', $row['fileid'], IQueryBuilder::PARAM_INT)
242-
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT)
243-
->executeStatement();
244+
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT);
245+
$query->set('etag', $query->createNamedParameter($newEtag, IQueryBuilder::PARAM_STR));
246+
$query->executeStatement();
244247
}
245248
}
246249
}
@@ -249,28 +252,31 @@ public function commitBatch(): void {
249252
$query = $this->connection->getQueryBuilder();
250253
$query->update('filecache')
251254
->set('mtime', $query->func()->greatest('mtime', $query->createParameter('time')))
252-
->set('etag', $query->expr()->literal(uniqid()))
255+
->set('etag', $query->createNamedParameter('', IQueryBuilder::PARAM_STR))
253256
->where($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
254257
->andWhere($query->expr()->eq('path_hash', $query->createParameter('hash')));
255258

256259
$queryWithSize = $this->connection->getQueryBuilder();
257260
$queryWithSize->update('filecache')
258261
->set('mtime', $queryWithSize->func()->greatest('mtime', $queryWithSize->createParameter('time')))
259-
->set('etag', $queryWithSize->expr()->literal(uniqid()))
262+
->set('etag', $queryWithSize->createNamedParameter('', IQueryBuilder::PARAM_STR))
260263
->set('size', $queryWithSize->func()->add('size', $queryWithSize->createParameter('size')))
261264
->where($queryWithSize->expr()->eq('storage', $queryWithSize->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
262265
->andWhere($queryWithSize->expr()->eq('path_hash', $queryWithSize->createParameter('hash')));
263266

264267
foreach ($this->batch as $item) {
268+
$newEtag = bin2hex(random_bytes(8));
265269
if ($item['size']) {
266270
$queryWithSize->setParameter('hash', $item['hash'], IQueryBuilder::PARAM_STR)
267271
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT)
268-
->setParameter('size', $item['size'], IQueryBuilder::PARAM_INT)
269-
->executeStatement();
272+
->setParameter('size', $item['size'], IQueryBuilder::PARAM_INT);
273+
$queryWithSize->set('etag', $queryWithSize->createNamedParameter($newEtag, IQueryBuilder::PARAM_STR));
274+
$queryWithSize->executeStatement();
270275
} else {
271276
$query->setParameter('hash', $item['hash'], IQueryBuilder::PARAM_STR)
272-
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT)
273-
->executeStatement();
277+
->setParameter('time', $item['time'], IQueryBuilder::PARAM_INT);
278+
$query->set('etag', $query->createNamedParameter($newEtag, IQueryBuilder::PARAM_STR));
279+
$query->executeStatement();
274280
}
275281
}
276282
}

‎lib/private/Files/Cache/Scanner.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ protected function scanChildren(string $path, $recursive, int $reuse, int $folde
423423
$updatedData['size'] = $size;
424424
}
425425
if ($etagChanged) {
426-
$updatedData['etag'] = uniqid();
426+
$updatedData['etag'] = bin2hex(random_bytes(8));
427427
}
428428
if ($updatedData) {
429429
$this->cache->update($folderId, $updatedData);

0 commit comments

Comments
 (0)