Ensembia/nextcloud-ng_backup
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 17 Wiki Activity

Backup state stays valid JSON with a name that is not UTF-8; Bin reads on from an id, not a position

Browse Source
This commit is contained in:
Dillard Blom
2026-11-06 20:02:09 +01:00
parent 1736005648
commit 1b621b7149
8 changed files with 38 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/Controller/AdminApiController.php
+2 -2
View File
@@ -179,12 +179,12 @@ final class AdminApiController extends Controller {
}
}
public function trash(int $targetId, int $offset = 0): JSONResponse {
public function trash(int $targetId, string $after = ''): JSONResponse {
try {
$delay = $this->keys->deleteDelayDays() * 86400;
$repo = $this->targets->repository($this->targets->get((string)$targetId));
// In parts, so that the page can show how far it is (a read per removed restore point).
$page = $repo->trashPage($offset, microtime(true) + self::TRASH_PAGE_SECONDS);
$page = $repo->trashPage($after, microtime(true) + self::TRASH_PAGE_SECONDS);
$out = [];
foreach ($page['items'] as $id => $info) {
$out[] = ['id' => $id, 'label' => $info['label'], 'time' => $info['time'], 'removedAt' => $info['forgottenAt'],
lib/Job/BackupRun.php
+2 -2
View File
@@ -62,9 +62,9 @@ final class BackupRun {
if (!mb_check_encoding($logical, 'UTF-8') || !Repository::isSafePath($logical)) {
// A name a snapshot cannot hold (a backslash, a control character, not UTF-8). One
// such path must not make a backup fail, or the whole snapshot unreadable later.
// The cursor stays where it is: the state is JSON, which cannot hold such a name. A
// next step meets it again, and it is reported once.
self::recordUnreadable($state, $stats, mb_scrub($logical, 'UTF-8'));
$state['cur'] = null;
$state['after'] = $logical;
continue;
}
if (is_dir($path)) {
lib/Repository/Repository.php
+8 -3
View File
@@ -818,15 +818,20 @@ final class Repository {
/**
* Part of trash(), for a page that shows how far it is: each entry is a read of its own on the
* location. Reads on from $offset until $deadline (microtime(true)), at least one entry.
* location. Reads the entries with an id after $after, in order, until $deadline
* (microtime(true)), at least one. The id, not a position: a cleanup that changes the trash
* between two parts then makes no entry come twice or be missed.
*
* @return array{total: int, items: array<string, array<string, mixed>>}
*/
public function trashPage(int $offset, float $deadline): array {
public function trashPage(string $after, float $deadline): array {
$ids = array_map('strval', array_keys($this->catalog->trash));
sort($ids, SORT_STRING);
$items = [];
foreach (array_slice($ids, max(0, $offset)) as $id) {
foreach ($ids as $id) {
if (strcmp($id, $after) <= 0) {
continue;
}
try {
$items[$id] = $this->trashInfo($id);
} catch (\Throwable) {
src/api.js
+1 -1
View File
@@ -21,7 +21,7 @@ export default {
addSlot: (passphrase, label) => call('post', '/key/slots', { passphrase, label }),
replaceSlot: (slot, passphrase, label) => call('put', `/key/slots/${slot}`, { passphrase, label }),
removeSlot: (slot) => call('delete', `/key/slots/${slot}`),
trash: (targetId, offset = 0) => call('get', `/targets/${targetId}/trash`, { offset }),
trash: (targetId, after = '') => call('get', `/targets/${targetId}/trash`, { after }),
untrash: (targetId, snapshotId) => call('post', `/targets/${targetId}/trash/${snapshotId}`),
kitUrl: () => url('/key/kit'),
confirmKit: (accepted, phrase, code) => call('post', '/key/confirm', { accepted, phrase, code }),
src/components/Bin.vue
+2 -2
View File
@@ -78,13 +78,13 @@ async function loadTarget(target, load) {
reading.value = { name: target.name, total: null, done: 0 }
const items = []
for (;;) {
const page = await api.trash(target.id, items.length)
const page = await api.trash(target.id, items.length ? items[items.length - 1].id : '')
if (load !== current) {
return
}
items.push(...page.items)
reading.value = { name: target.name, total: page.total, done: Math.min(items.length, page.total) }
if (items.length >= page.total || !page.items.length) {
if (!page.items.length || items.length >= page.total) {
break
}
}
tests/Unit/Repository/CatalogTest.php
+19 -5
View File
@@ -58,16 +58,30 @@ class CatalogTest extends TestCase {
$again = $this->open();
$seen = [];
// A deadline already past: one entry per request, as on a slow location
$after = '';
for ($request = 0; count($seen) < 3 && $request < 5; $request++) {
$page = $again->trashPage(count($seen), 0.0);
$page = $again->trashPage($after, 0.0);
$this->assertSame(3, $page['total']);
$this->assertCount(1, $page['items']);
$seen += $page['items'];
$after = (string)array_key_last($page['items']);
}
$this->assertSame(array_keys($again->trash()), array_intersect(array_keys($again->trash()), array_keys($seen)));
$this->assertCount(3, $seen);
$this->assertSame(['total' => 3, 'items' => []], $again->trashPage(3, 0.0));
$this->assertCount(3, $again->trashPage(0, microtime(true) + 60)['items']);
$this->assertEqualsCanonicalizing(array_keys($again->trash()), array_keys($seen));
$this->assertSame(['total' => 3, 'items' => []], $again->trashPage($after, 0.0));
$this->assertCount(3, $again->trashPage('', microtime(true) + 60)['items']);
}
public function testAnEntryThatLeavesTheTrashBetweenPartsMakesNoneComeTwiceOrBeMissed(): void {
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor);
for ($i = 0; $i < 3; $i++) {
$repo->forget($repo->backupDirectory($this->src)['snapshot']);
}
$reader = $this->open();
$ids = array_keys($reader->trashPage('', microtime(true) + 60)['items']);
$first = $reader->trashPage('', 0.0)['items'];
$this->assertSame([$ids[0]], array_keys($first));
$this->open()->untrash($ids[0]); // another administrator, between two parts
$this->assertSame([$ids[1], $ids[2]], array_keys($this->open()->trashPage($ids[0], microtime(true) + 60)['items']));
}
public function testRollbackIsDetected(): void {
tests/Unit/Repository/EmptyDirsTest.php
+3 -2
View File
@@ -78,10 +78,11 @@ class EmptyDirsTest extends TestCase {
file_put_contents("$this->src/data/alice/files/\xff.txt", 'x');
$state = BackupRun::start(['data' => "$this->src/data"]);
for ($i = 0; $i < 50 && $state['phase'] !== 'commit'; $i++) {
$state = BackupRun::step($this->repo(), $state, microtime(true) + 30);
// A step per entry, state through JSON: as separate processes would
$state = json_decode(json_encode(BackupRun::step($this->repo(), $state, 0.0), JSON_THROW_ON_ERROR), true, 512, JSON_THROW_ON_ERROR);
}
$this->assertSame('commit', $state['phase']);
$this->assertSame(4, $state['stats']['unreadable']);
$this->assertSame(4, $state['stats']['unreadable'], 'each reported once, over several steps');
$this->repo()->recordSnapshot($state['snapshot']);
$this->assertSame(['data/alice/files/Hollow/Nested', 'data/skipped/inner'], array_column($this->repo()->emptyDirs($state['snapshot']), 0));
tests/integration/roundtrip.sh
+1 -1
View File
@@ -99,7 +99,7 @@ done
occ backup:list ci | grep " page " >/dev/null || fail "the snapshot of the ticked backup is not listed"
ok "without system cron: warning in backup:status and the API, backup finished by $i ticks of the open page"
# The Bin reads in parts, as the page does (what is in the parts: CatalogTest)
api "$API/targets/$TID/trash?offset=0" | php -r '$p = json_decode(stream_get_contents(STDIN), true); exit(is_int($p["total"] ?? null) && is_array($p["items"] ?? null) && count($p["items"]) <= $p["total"] ? 0 : 1);' || fail "the Bin API does not give a total and items"
api "$API/targets/$TID/trash" | php -r '$p = json_decode(stream_get_contents(STDIN), true); exit(is_int($p["total"] ?? null) && is_array($p["items"] ?? null) && count($p["items"]) <= $p["total"] ? 0 : 1);' || fail "the Bin API does not give a total and items"
ok "the Bin API reads in parts, with a total"
occ background:cron >/dev/null
$X php -f /var/www/html/cron.php >/dev/null 2>&1 || true