Full restore: an empty listing never empties the index; read a failed pack once more; recognise and replace SFTP connections a Storage Box dropped quietly; version 0.9.11

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

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.]]></description>
-    <version>0.9.10</version>
+    <version>0.9.11</version>
     <licence>agpl</licence>
     <author mail="dillard.blom@ensembia.com">Dillard Blom</author>
     <namespace>NgBackup</namespace>
@@ -52,9 +52,9 @@ you need help with restoring a backup after a crash.]]></description>
     <website>https://github.com/dillardblom/ng_backup</website>
     <bugs>https://github.com/dillardblom/ng_backup/issues</bugs>
     <repository type="git">https://github.com/dillardblom/ng_backup</repository>
-    <screenshot>https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.4/screenshots/1-backups.png</screenshot>
+    <screenshot>https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.5/screenshots/1-backups.png</screenshot>
-    <screenshot>https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.4/screenshots/2-restore.png</screenshot>
+    <screenshot>https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.5/screenshots/2-restore.png</screenshot>
-    <screenshot>https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.4/screenshots/3-settings.png</screenshot>
+    <screenshot>https://raw.githubusercontent.com/dillardblom/ng_backup/v1.0.0-rc.5/screenshots/3-settings.png</screenshot>
     <dependencies>
         <php min-version="8.1"/>
         <lib>sodium</lib>

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<int, resource|null> 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<int, object> $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<object> $connections
 	 * @param list<array{0:string, 1:int, 2:int}> $ranges
+	 * @param (\Closure():?object)|null $connect a new connection in place of one that broke
+	 *                                         (null when it cannot be made)
 	 * @return \Generator<int, resource|null>
 	 */
-	public static function fetch(array $connections, string $root, array $ranges, string $spool): \Generator {
+	public static function fetch(array $connections, string $root, array $ranges, string $spool, ?\Closure $connect = null): \Generator {
-		$fetcher = new self($connections, rtrim($root, '/'), $ranges, $spool);
+		$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 */

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('.');
+			// A Storage Box drops a quiet connection without phpseclib noticing: it still says
-			return (bool)$sftp->isConnected();
+			// 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;
 		}

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<string, list<array{id:string, offset:int, length:int}>> 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<array{id:string, offset:int, length:int, raw:int, flags:int}> $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]) {

lib/Repository/Repository.php

@@ -841,9 +841,9 @@ final class Repository {
 					$done++;
 				}
 			}
-		} catch (BackendException $e) {
+		} catch (BackendException) {
-			// A pack moved (see loadBlob()): the rest one blob at a time, from the fresh index.
+			// A pack moved, or the read failed for a while (see loadBlob()): the rest one blob at a
-			$this->refreshIndex($e);
+			// 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
+			// was loaded (a disaster recovery test while the original server still runs): add the
-			// the index from the location; read again only if the blob has moved.
+			// new index files, and read the blob where it is now. Or the read failed for a while
-			$this->refreshIndex($e);
+			// (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);

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();

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;

tests/Unit/Repository/IndexListingTest.php

@@ -0,0 +1,148 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\NgBackup\Tests\Unit\Repository;
+
+use OCA\NgBackup\Backend\BackendException;
+use OCA\NgBackup\Backend\IBackend;
+use OCA\NgBackup\Backend\LocalBackend;
+use OCA\NgBackup\Crypto\KeyRing;
+use OCA\NgBackup\Repository\IIndexCache;
+use OCA\NgBackup\Repository\Repository;
+use OCA\NgBackup\Tests\Unit\TempDirTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * A listing of the location that comes back empty because a connection failed quietly (an SFTP
+ * connection a Storage Box dropped) must not empty the index. In a 1 TB disaster recovery test a
+ * pack read failed after 13 hours, the reload of the index got an empty listing, and the restore
+ * stopped with "Blob not in index".
+ */
+class IndexListingTest extends TestCase {
+	use TempDirTrait;
+
+	private string $root;
+	private KeyRing $keys;
+	private string $src;
+	private string $snapshot;
+
+	protected function setUp(): void {
+		$this->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);
+	}
+}