diff --git a/CHANGELOG.md b/CHANGELOG.md index 35ed537..03c1094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ the app store accepts a `-beta` suffix, but Nextcloud servers then only offer th beta/daily update channel, which would hide the app from most admins. The stability stage is instead carried by the app description, the git tag, the release, and this file. +## [1.0.0-rc.5] - 2026-10-20 + +Fifth release candidate for 1.0.0; the app version is 0.9.11. A full restore of 1 TB over SFTP +stopped after 13 hours with "Blob not in index"; the backup itself was fine. + +### Fixed +- A full restore no longer stops with "Blob not in index" after a pack could not be read for a + moment. Reading the index again then got an empty listing (from a connection the location had + dropped) and emptied the index. Reading the index again now only adds, an empty listing of a + location that has an index is an error instead of an empty index, and a pack that could not be + read is read once more before the restore gives up. +- An SFTP connection that a Hetzner Storage Box dropped without phpseclib noticing is now + recognised and replaced: the check asks the server and counts its answer. +- The extra connection a full restore reads ahead on is replaced when it breaks (up to 20 times), + instead of reading the rest of the restore the slow way. + ## [1.0.0-rc.4] - 2026-10-19 Fourth release candidate for 1.0.0; the app version is 0.9.10. 1.0.0-rc.3 (0.9.9) was not diff --git a/appinfo/info.xml b/appinfo/info.xml index 210a3ce..536aa46 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -41,7 +41,7 @@ welcome: use the "Share your experience" link on the admin page. Developed and sponsored by [Ensembia](https://www.ensembia.com). We can offer (paid) assistance if you need help with restoring a backup after a crash.]]> - 0.9.10 + 0.9.11 agpl Dillard Blom NgBackup @@ -52,9 +52,9 @@ you need help with restoring a backup after a crash.]]> https://github.com/dillardblom/ng_backup https://github.com/dillardblom/ng_backup/issues https://github.com/dillardblom/ng_backup - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.4/screenshots/1-backups.png - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.4/screenshots/2-restore.png - https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.4/screenshots/3-settings.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.5/screenshots/1-backups.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.5/screenshots/2-restore.png + https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.5/screenshots/3-settings.png sodium diff --git a/lib/Backend/SftpRangeFetcher.php b/lib/Backend/SftpRangeFetcher.php index def71af..3f45f99 100644 --- a/lib/Backend/SftpRangeFetcher.php +++ b/lib/Backend/SftpRangeFetcher.php @@ -54,6 +54,9 @@ final class SftpRangeFetcher { private const FXF_READ = 1; private const FX_OK = 0; + /** Connections that broke and are replaced, at most, in one restore. */ + public const MAX_RECONNECTS = 20; + private const OPEN = 0; private const READ = 1; private const CLOSE = 2; @@ -65,6 +68,8 @@ final class SftpRangeFetcher { /** @var array ranges read (or failed) and not yet given to the caller */ private array $done = []; private int $requestId = 0x4e520000; + /** Connections replaced so far (see MAX_RECONNECTS). */ + public int $reconnects = 0; /** * @param array $connections phpseclib 2 connections with their SFTP channel open @@ -75,6 +80,7 @@ final class SftpRangeFetcher { private string $root, private array $ranges, private string $spool, + private ?\Closure $connect = null, ) { foreach (array_keys($connections) as $c) { $this->chunks[$c] = self::CHUNK; @@ -86,10 +92,12 @@ final class SftpRangeFetcher { * * @param list $connections * @param list $ranges + * @param (\Closure():?object)|null $connect a new connection in place of one that broke + * (null when it cannot be made) * @return \Generator */ - public static function fetch(array $connections, string $root, array $ranges, string $spool): \Generator { - $fetcher = new self($connections, rtrim($root, '/'), $ranges, $spool); + public static function fetch(array $connections, string $root, array $ranges, string $spool, ?\Closure $connect = null): \Generator { + $fetcher = new self($connections, rtrim($root, '/'), $ranges, $spool, $connect); try { yield from $fetcher->run(); } finally { @@ -319,6 +327,27 @@ final class SftpRangeFetcher { } self::disconnect($this->connections[$c]); unset($this->connections[$c]); + // A long restore outlives connections (a Storage Box drops one now and then): without a + // new one, the rest would be read the slow way. + if ($this->connect === null || $this->reconnects >= self::MAX_RECONNECTS) { + return; + } + $this->reconnects++; + try { + $new = ($this->connect)(); + } catch (\Throwable) { + $new = null; + } + if (function_exists('OCP\Log\logger')) { + \OCP\Log\logger('ng_backup')->info($new === null + ? 'Reading ahead: an SFTP connection broke and no new one could be made' + : 'Reading ahead: an SFTP connection broke; reading on with a new one'); + } + if ($new !== null) { + $key = max(array_merge(array_keys($this->chunks), [-1])) + 1; + $this->connections[$key] = $new; + $this->chunks[$key] = self::CHUNK; + } } /** @return resource|null a new file in the spool, already unlinked: nothing is left behind if the process dies */ diff --git a/lib/Backend/StorageBackend.php b/lib/Backend/StorageBackend.php index ec5897d..984ecc6 100644 --- a/lib/Backend/StorageBackend.php +++ b/lib/Backend/StorageBackend.php @@ -282,10 +282,19 @@ final class StorageBackend implements IBackend, IReadsAhead { if ($connections === []) { return null; } - return (static function () use ($storages, $connections, $root, $abs, $spool): \Generator { + // in place of a connection that broke: a new storage, kept like the others + $connect = static function () use ($reconnect, &$storages): ?object { + $storage = $reconnect(); + $sftp = SftpReadStream::connection($storage); + if ($sftp !== null) { + $storages[] = $storage; + } + return $sftp; + }; + return (static function () use (&$storages, $connections, $root, $abs, $spool, $connect): \Generator { // the storages hold their connections: kept until the fetcher is done try { - yield from SftpRangeFetcher::fetch($connections, $root, $abs, $spool); + yield from SftpRangeFetcher::fetch($connections, $root, $abs, $spool, $connect); } finally { unset($storages); } @@ -478,8 +487,9 @@ final class StorageBackend implements IBackend, IReadsAhead { $this->lastUsed = microtime(true); } + /** After a failed request: ask the server (one round trip), as phpseclib may not know it is gone. */ private function reopenIfDead(): bool { - if ($this->reconnect === null || !$this->sftpReads || $this->alive(false) !== false) { + if ($this->reconnect === null || !$this->sftpReads || $this->alive(true) !== false) { return false; } $this->reopen(); @@ -508,8 +518,9 @@ final class StorageBackend implements IBackend, IReadsAhead { if (is_callable([$sftp, 'clearStatCache'])) { $sftp->clearStatCache(); // or stat() answers from memory } - $sftp->stat('.'); - return (bool)$sftp->isConnected(); + // A Storage Box drops a quiet connection without phpseclib noticing: it still says + // connected, and every request fails. So the answer counts, not what it says after. + return $sftp->stat('.') !== false && (bool)$sftp->isConnected(); } catch (\Throwable) { return false; } diff --git a/lib/Repository/BlobIndex.php b/lib/Repository/BlobIndex.php index 1fcfc3d..fac0fa2 100644 --- a/lib/Repository/BlobIndex.php +++ b/lib/Repository/BlobIndex.php @@ -7,6 +7,7 @@ declare(strict_types=1); namespace OCA\NgBackup\Repository; +use OCA\NgBackup\Backend\BackendException; use OCA\NgBackup\Backend\IBackend; use OCA\NgBackup\Crypto\StreamCipher; @@ -104,6 +105,11 @@ final class BlobIndex { $fetch[] = $path; } } + if ($present === [] && $cached !== []) { + // A repository never loses all its index files: an empty listing is a connection that + // failed quietly. Taken as is, it would empty the index and the cache. + throw new BackendException('The location listed no index files, while ' . count($cached) . ' are known: not trusting that listing'); + } // The rest is downloaded several at a time where the location allows it: on a large // repository these are tens of thousands of small files. getMany() keeps their order, so // the cached packs listed before each one are added first. @@ -192,6 +198,39 @@ final class BlobIndex { } /** @return array> packId => blobs currently located in that pack */ + /** + * Add the index files that appeared on the location since load(), such as those of packs + * another server's cleanup wrote while a restore runs. Only adds and updates: a blob is + * never dropped, as a listing that misses files (a connection that failed) must not make a + * restore lose blobs it knows. A blob in a new pack takes that pack's place. + */ + public function refresh(): void { + $known = array_flip(array_map(fn (array $b): string => $b[0], $this->blobs)); + $fetch = []; + $listed = 0; + foreach ($this->backend->listFiles('index') as $path) { + $listed++; + if (!isset($known[basename($path)])) { + $fetch[] = $path; + } + } + if ($listed === 0 && $this->blobs !== []) { + throw new BackendException('The location listed no index files, while ' . count($known) . ' are known: not trusting that listing'); + } + $toCache = []; + foreach ($this->backend->getMany($fetch) as $path => $data) { + $packId = basename($path); + /** @var list $entries authenticated: written by NG Backup */ + $entries = json_decode($this->cipher->decryptString($data, 'index:' . $packId), true, 512, JSON_THROW_ON_ERROR); + $this->downloaded++; + $this->add($packId, $entries); + $toCache[$packId] = $entries; + } + if ($toCache !== []) { + $this->cache?->putMany($this->repositoryId, $toCache); + } + } + public function packs(): array { $packs = []; foreach ($this->blobs as $id => [$pack, $offset, $length]) { diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index bc3b6f7..b761dcf 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -841,9 +841,9 @@ final class Repository { $done++; } } - } catch (BackendException $e) { - // A pack moved (see loadBlob()): the rest one blob at a time, from the fresh index. - $this->refreshIndex($e); + } catch (BackendException) { + // A pack moved, or the read failed for a while (see loadBlob()): the rest one blob at a + // time, each read once more after a failure. foreach (array_slice($ids, $done) as $id) { yield $this->loadBlob($id); } @@ -872,27 +872,25 @@ final class Repository { $encrypted = $this->packReader->read($pack, $offset, $length); } catch (BackendException $e) { // Another server's cleanup may have repacked this blob into a new pack since the index - // was loaded (a disaster recovery test while the original server still runs). Reload - // the index from the location; read again only if the blob has moved. - $this->refreshIndex($e); + // was loaded (a disaster recovery test while the original server still runs): add the + // new index files, and read the blob where it is now. Or the read failed for a while + // (a connection that died; the backend has replaced it by now): then read it once + // more where it was. A second failure is the real one. + try { + $this->index->refresh(); + } catch (\Throwable) { + // the location could not be listed either: read again where the blob was + } [$moved, $offset, $length] = $this->index->get($id); - if ($moved === $pack) { + try { + $encrypted = $this->packReader->read($moved, $offset, $length); + } catch (BackendException) { throw $e; } - $encrypted = $this->packReader->read($moved, $offset, $length); } return $this->decodeBlob($id, $encrypted); } - /** Reload the index from the location after a failed pack read; rethrows that failure if it cannot. */ - private function refreshIndex(BackendException $cause): void { - try { - $this->index->load(); - } catch (\Throwable) { - throw $cause; - } - } - /** Decrypts and checks a blob read from its pack; throws when it is corrupted or tampered with. */ private function decodeBlob(string $id, string $encrypted): string { [, , , $raw, $flags] = $this->index->get($id); diff --git a/tests/Unit/Backend/SftpRangeFetcherTest.php b/tests/Unit/Backend/SftpRangeFetcherTest.php index 5544d76..b969743 100644 --- a/tests/Unit/Backend/SftpRangeFetcherTest.php +++ b/tests/Unit/Backend/SftpRangeFetcherTest.php @@ -184,6 +184,38 @@ class SftpRangeFetcherTest extends TestCase { $this->assertSame(array_fill(0, 10, null), $result); } + public function testABrokenConnectionIsReplacedAndTheRestIsReadAhead(): void { + $connections = $this->connections(1, [['loseAt' => 1048576]]); + $made = []; + $connect = function () use (&$made): object { + $made[] = $c = new FakeSftpConnection($this->files); + return SftpReadStream::connection(new FakeSftpStorage($c)); + }; + $ranges = $this->ranges(); + + $result = self::collect(SftpRangeFetcher::fetch(self::opened($connections), '/home/', $ranges, $this->tempDir(), $connect)); + + $expected = $this->expected($ranges); + $expected[0] = null; // the range it was reading when it broke: read the usual way + $this->assertSame($expected, $result); + $this->assertCount(1, $made); + $this->assertTrue($made[0]->disconnected, 'the new one ends with the fetcher too'); + } + + public function testConnectionsThatKeepBreakingAreReplacedOnlySoOften(): void { + $made = 0; + $connect = function () use (&$made): object { + $made++; + return SftpReadStream::connection(new FakeSftpStorage(new FakeSftpConnection($this->files, loseAt: 0))); + }; + $ranges = array_merge($this->ranges(), $this->ranges(), $this->ranges()); + + $result = self::collect(SftpRangeFetcher::fetch(self::opened($this->connections(1, [['loseAt' => 0]])), '/home/', $ranges, $this->tempDir(), $connect)); + + $this->assertSame(array_fill(0, count($ranges), null), $result); + $this->assertSame(SftpRangeFetcher::MAX_RECONNECTS, $made); + } + public function testStoppingHalfwayEndsTheConnectionsAndLeavesNothingBehind(): void { $connections = $this->connections(2); $spool = $this->tempDir(); diff --git a/tests/Unit/Backend/StorageBackendReconnectTest.php b/tests/Unit/Backend/StorageBackendReconnectTest.php index 161fa0a..8c4aea0 100644 --- a/tests/Unit/Backend/StorageBackendReconnectTest.php +++ b/tests/Unit/Backend/StorageBackendReconnectTest.php @@ -77,6 +77,24 @@ class StorageBackendReconnectTest extends TestCase { $this->assertSame(1, $backend->reconnects); } + public function testASilentlyDroppedConnectionIsReplacedAfterAPause(): void { + $backend = $this->backend(); + $this->connections[0]->dropped = true; + $this->connections[0]->silent = true; // phpseclib still says connected + self::idle($backend); + $this->assertTrue($backend->exists('config')); + $this->assertSame(1, $backend->reconnects, 'the check failed, so the connection was replaced'); + } + + public function testAFailureOnASilentlyDroppedConnectionIsRepeated(): void { + $backend = $this->backend(); + $this->connections[0]->dropped = true; + $this->connections[0]->silent = true; + $fh = $backend->get('config'); + $this->assertSame('config', stream_get_contents($fh)); + $this->assertSame(1, $backend->reconnects); + } + public function testAMissingFileOnALiveConnectionIsJustMissing(): void { $backend = $this->backend(); $this->assertFalse($backend->exists('nothing')); @@ -121,6 +139,8 @@ interface SftpLikeStorage extends IStorage { /** The public phpseclib calls the backend uses to tell a dead connection. */ final class ReconnectFakeSftp { public bool $dropped = false; + /** Dropped without phpseclib noticing (as a Hetzner Storage Box does): it still says connected. */ + public bool $silent = false; private bool $connected = true; public int $requests = 0; @@ -139,6 +159,9 @@ final class ReconnectFakeSftp { public function request(): bool { $this->requests++; if ($this->dropped) { + if ($this->silent) { + return false; + } $this->connected = false; } return $this->connected; diff --git a/tests/Unit/Repository/IndexListingTest.php b/tests/Unit/Repository/IndexListingTest.php new file mode 100644 index 0000000..d1eab6b --- /dev/null +++ b/tests/Unit/Repository/IndexListingTest.php @@ -0,0 +1,148 @@ +root = $this->tempDir(); + $this->keys = KeyRing::generate(); + $this->src = $this->tempDir(); + for ($i = 0; $i < 12; $i++) { + file_put_contents("$this->src/f$i", random_bytes(3 * 1048576)); + } + $repo = Repository::initWithKey(new LocalBackend($this->root), $this->keys, $this->keys->wrap('pw')); + $this->snapshot = $repo->backupDirectory($this->src)['snapshot']; + } + + public function testARestoreGoesOnAfterAPackReadFailedAndTheListingCameBackEmpty(): void { + $backend = new FlakyBackend(new LocalBackend($this->root)); + $repo = Repository::openWithKey($backend, $this->keys); + $backend->failPackReads = 1; // the read of a pack fails once + $backend->emptyIndexListings = 1; // and listing the index right after comes back empty + $target = $this->tempDir(); + + $this->assertSame(12, $repo->restore($this->snapshot, $target)); + + $this->assertSame(self::hashTree($this->src), self::hashTree($target)); + $this->assertSame(0, $backend->failPackReads, 'the failure happened'); + $this->assertSame(0, $backend->emptyIndexListings, 'the empty listing was seen'); + } + + public function testAPackThatStaysUnreadableIsStillReportedAsSuch(): void { + $backend = new FlakyBackend(new LocalBackend($this->root)); + $repo = Repository::openWithKey($backend, $this->keys); + $backend->failPackReads = 1000; + + try { + $repo->restore($this->snapshot, $this->tempDir()); + $this->fail('the packs cannot be read'); + } catch (BackendException $e) { + $this->assertStringStartsWith('Not found: packs/', $e->getMessage()); + } + } + + public function testOpeningWithAnEmptyListingKeepsTheCachedIndex(): void { + $cache = new class implements IIndexCache { + public array $data = []; + public function all(string $r): array { return $this->data[$r] ?? []; } + public function put(string $r, string $p, array $e): void { $this->data[$r][$p] = $e; } + public function remove(string $r, array $ps): void { foreach ($ps as $p) { unset($this->data[$r][$p]); } } + public function putMany(string $r, array $packs): void { foreach ($packs as $p => $e) { $this->put($r, (string)$p, $e); } } + public function replaceAll(string $r, array $packs): void { $this->data[$r] = []; $this->putMany($r, $packs); } + }; + $backend = new FlakyBackend(new LocalBackend($this->root)); + Repository::openWithKey($backend, $this->keys, $cache); + $cached = $cache->data; + $this->assertNotSame([], $cached); + + $backend->emptyIndexListings = 1; + try { + Repository::openWithKey($backend, $this->keys, $cache); + $this->fail('an empty listing of a repository with an index must not be taken as is'); + } catch (BackendException $e) { + $this->assertStringContainsString('listed no index files', $e->getMessage()); + } + $this->assertSame($cached, $cache->data, 'the cache is untouched'); + } +} + +/** A location whose pack reads fail, and whose index listings come back empty, a set number of times. */ +final class FlakyBackend implements IBackend { + use \OCA\NgBackup\Backend\GetsOneByOne; + + public int $failPackReads = 0; + public int $emptyIndexListings = 0; + + public function __construct( + private IBackend $inner, + ) { + } + + public function put(string $path, $stream): void { + $this->inner->put($path, $stream); + } + + public function get(string $path) { + if (str_starts_with($path, 'packs/') && $this->failPackReads > 0) { + $this->failPackReads--; + throw new BackendException('Not found: ' . $path); + } + return $this->inner->get($path); + } + + public function getRange(string $path, int $offset, int $length): string { + if (str_starts_with($path, 'packs/') && $this->failPackReads > 0) { + $this->failPackReads--; + throw new BackendException('Not found: ' . $path); + } + return $this->inner->getRange($path, $offset, $length); + } + + public function exists(string $path): bool { + return $this->inner->exists($path); + } + + public function move(string $from, string $to): void { + $this->inner->move($from, $to); + } + + public function list(string $prefix): array { + return $this->inner->list($prefix); + } + + public function listFiles(string $dir): array { + if ($dir === 'index' && $this->emptyIndexListings > 0) { + $this->emptyIndexListings--; + return []; + } + return $this->inner->listFiles($dir); + } + + public function delete(string $path): void { + $this->inner->delete($path); + } +}