From 9b85221e50abc5f52c40f8d4aa4dbd2774d4bfc8 Mon Sep 17 00:00:00 2001
From: Dillard Blom <dillard.blom@ensembia.com>
Date: Mon, 19 Oct 2026 08:54:13 +0200
Subject: [PATCH] Read ahead: a plan that cannot be made leaves the restore to
 the usual reads; only a whole buffered SFTP packet counts as an answer
 waiting

---
 lib/Backend/SftpRangeFetcher.php            |  8 ++++++-
 lib/Repository/Repository.php               | 24 ++++++++++++---------
 tests/Unit/Backend/SftpRangeFetcherTest.php |  2 +-
 tests/Unit/Repository/ReadAheadTest.php     | 19 ++++++++++++++++
 4 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/lib/Backend/SftpRangeFetcher.php b/lib/Backend/SftpRangeFetcher.php
index 74651b5..def71af 100644
--- a/lib/Backend/SftpRangeFetcher.php
+++ b/lib/Backend/SftpRangeFetcher.php
@@ -150,7 +150,7 @@ final class SftpRangeFetcher {
 			$buffered = $vars['requestBuffer'] ?? null;
 			$channels = $vars['channel_buffers'] ?? null;
 			if ((is_array($buffered) && isset($job->queue[0]) && array_key_exists($job->queue[0][0], $buffered))
-				|| (is_string($vars['packet_buffer'] ?? null) && $vars['packet_buffer'] !== '')
+				|| (is_string($vars['packet_buffer'] ?? null) && self::wholePacket($vars['packet_buffer']))
 				|| (is_array($channels) && array_filter($channels) !== [])) {
 				$ready[] = $c;
 				continue;
@@ -173,6 +173,12 @@ final class SftpRangeFetcher {
 		return $n === false || $n === 0 ? array_keys($this->jobs) : array_keys($read);
 	}
 
+	/** Whether $buffer starts with a whole SFTP packet (length, then that many bytes). */
+	private static function wholePacket(string $buffer): bool {
+		$length = strlen($buffer) >= 4 ? unpack('N', $buffer) : false;
+		return is_array($length) && strlen($buffer) >= 4 + $length[1];
+	}
+
 	private function start(int $c, int $i): void {
 		[$path, $offset, $length] = $this->ranges[$i];
 		$fh = $this->spoolFile();
diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php
index 3c836b0..bc3b6f7 100644
--- a/lib/Repository/Repository.php
+++ b/lib/Repository/Repository.php
@@ -243,18 +243,22 @@ final class Repository {
 			@rmdir($dir);
 		}
 		$plan = new ReadAheadPlan(rtrim($target, '/') . '/.ngb-restore-spool-' . bin2hex(random_bytes(4)));
-		foreach ($this->streamEntries($snapshotId) as $entry) {
-			if ($prefix !== '' && !str_starts_with($entry['p'], $prefix)) {
-				continue;
-			}
-			foreach ($entry['b'] as $id) {
-				try {
-					[$pack, $offset, $length] = $this->index->get($id);
-				} catch (RepositoryException) {
-					continue; // the restore reports it when it gets there
+		try {
+			foreach ($this->streamEntries($snapshotId) as $entry) {
+				if ($prefix !== '' && !str_starts_with($entry['p'], $prefix)) {
+					continue;
+				}
+				foreach ($entry['b'] as $id) {
+					try {
+						[$pack, $offset, $length] = $this->index->get($id);
+					} catch (RepositoryException) {
+						continue; // the restore reports it when it gets there
+					}
+					$plan->add('packs/' . substr($pack, 0, 2) . '/' . $pack, $offset, $length);
 				}
-				$plan->add('packs/' . substr($pack, 0, 2) . '/' . $pack, $offset, $length);
 			}
+		} catch (\Throwable) {
+			return null; // reading ahead is only faster: the restore itself reads the snapshot again and reports what is wrong
 		}
 		if (count($plan->ranges()) < 2 || (!is_dir($plan->spool) && !@mkdir($plan->spool, 0700, true))) {
 			return null;
diff --git a/tests/Unit/Backend/SftpRangeFetcherTest.php b/tests/Unit/Backend/SftpRangeFetcherTest.php
index 51dedc1..5544d76 100644
--- a/tests/Unit/Backend/SftpRangeFetcherTest.php
+++ b/tests/Unit/Backend/SftpRangeFetcherTest.php
@@ -96,7 +96,7 @@ class SftpRangeFetcherTest extends TestCase {
 			$pair = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP);
 			$sockets[] = $pair;
 			$c->fsock = $pair[0];
-			$c->packet_buffer = 'more';
+			$c->packet_buffer = pack('N', 4) . 'more'; // a whole packet
 		}
 		$ranges = array_slice($this->ranges(), 0, 3);
 		$start = microtime(true);
diff --git a/tests/Unit/Repository/ReadAheadTest.php b/tests/Unit/Repository/ReadAheadTest.php
index 7d97709..2e9ef34 100644
--- a/tests/Unit/Repository/ReadAheadTest.php
+++ b/tests/Unit/Repository/ReadAheadTest.php
@@ -66,6 +66,15 @@ class ReadAheadTest extends TestCase {
 		$this->assertLessThanOrEqual(count($backend->inner->list('packs')) + 1, count($backend->asked));
 	}
 
+	public function testAPlanThatCannotBeMadeLeavesTheRestoreToTheUsualReads(): void {
+		$backend = new ReadAheadBackend(new LocalBackend($this->root), failSnapshotReads: 1);
+
+		$this->assertRestored($this->restore($backend));
+
+		$this->assertSame([], $backend->asked, 'no reading ahead without a plan');
+		$this->assertSame(2, $backend->snapshotReads, 'the restore itself read the snapshot again');
+	}
+
 	public function testTheEmptySpoolOfAStoppedRestoreIsRemoved(): void {
 		$target = $this->tempDir();
 		mkdir("$target/.ngb-restore-spool-0badc0de", 0700); // a restore that was killed
@@ -195,6 +204,8 @@ final class ReadAheadBackend implements IBackend, IReadsAhead {
 		private bool $unavailable = false,
 		/** reading a pack the usual way fails */
 		private bool $packsGone = false,
+		/** this many reads of a snapshot fail, then they work */
+		private int $failSnapshotReads = 0,
 	) {
 		$this->counter = new PackOpenCountingBackend($inner);
 	}
@@ -238,7 +249,15 @@ final class ReadAheadBackend implements IBackend, IReadsAhead {
 		$this->counter->put($path, $stream);
 	}
 
+	public int $snapshotReads = 0;
+
 	public function get(string $path) {
+		if (str_starts_with($path, 'snapshots/')) {
+			$this->snapshotReads++;
+			if ($this->snapshotReads <= $this->failSnapshotReads) {
+				throw new BackendException('Connection lost reading ' . $path);
+			}
+		}
 		if ($this->packsGone && str_starts_with($path, 'packs/')) {
 			throw new BackendException('Not found: ' . $path);
 		}