fix(engine): probe the basis short final block at its own length

crates/engine/src/local_copy/context_impl/delta_transfer.rs

@@ -304,16 +304,79 @@ impl<'a> CopyContext<'a> {
             }
         }
 
-        // EOF tail match: the window now holds the file's short trailing bytes
-        // (fewer than block_length). Upstream rsync matches this short tail
-        // against the basis's final partial block via `l = MIN(blength,
-        // len-offset)` (`match.c:222-224`). Mirror that: probe the same-length
-        // basis block and, on a hit, flush preceding literals then emit the
-        // short matched block. Without this the trailing partial block is
-        // always sent as literal data, diverging from upstream deltas.
-        let tail_matched_block = {
-            let digest = rolling.digest();
-            index.find_tail_match_window(digest, &window, &mut scratch)
+        // EOF tail match against the basis's short final block.
+        //
+        // Upstream reaches this block by shrinking the rolling window: once
+        // `offset + k >= len` there is no next byte to add, so `more` is false
+        // and `k` decrements on every step (`match.c:321`, `match.c:331`). Its
+        // scan runs to `end = len + 1 - s->sums[s->count-1].len`
+        // (`match.c:174`) - bounded by the LAST block's length, not by
+        // `blength` - so the window keeps narrowing until it is exactly as long
+        // as that final short block. Only then does the candidate pass
+        // `l = MIN(blength, len-offset); if (l != s->sums[i].len) continue;`
+        // (`match.c:222-224`), because a short block is the only one whose
+        // recorded length is below `blength`.
+        //
+        // Probing whatever the loop happened to leave behind fails twice over.
+        //
+        // First, the length. Each failed full-window probe pops one byte, so at
+        // EOF the window normally holds `block_length - 1` bytes, and
+        // `find_tail_match` rejects every candidate on `block.len() != tail_len`
+        // - the same length-equality rule as upstream. Only a basis whose final
+        // block happens to be exactly `block_length - 1` could even reach the
+        // strong-sum comparison.
+        //
+        // Second, the digest. `outgoing` is recorded when a byte is popped but
+        // the rolling sum is not corrected until the *next* push rolls it out,
+        // so after the final pop the digest still covers the popped byte. At EOF
+        // there is no next push, leaving the digest describing `block_length`
+        // bytes while the window holds one fewer. Even the coincidental
+        // length match above would fail on sum1/sum2.
+        //
+        // So drain to the basis's final block length EXPLICITLY and recompute
+        // the sum over exactly those bytes. That is upstream's shrinking window
+        // expressed as a single probe at the one width that can match, instead
+        // of re-probing at every intermediate width.
+        //
+        // What hid all of this: the window lands on the final block's length,
+        // with a digest that happens to agree, only when a match `clear()`ed it
+        // with exactly that many bytes left - an identical source, where every
+        // full block matches. A fixture built that way passes either way.
+        let short_tail_len = index
+            .block_count()
+            .checked_sub(1)
+            .map(|last| index.block(last).len())
+            .filter(|&len| len > 0 && len < index.block_length());
+        let tail_matched_block = match short_tail_len {
+            // A basis whose length is an exact multiple of the block length has
+            // no short final block, so upstream has nothing extra to offer and
+            // this is a clean no-op.
+            None => None,
+            Some(tail_len) => {
+                while window.len() > tail_len {
+                    if let Some(front) = window.pop_front() {
+                        pending_literals.push(front);
+                    }
+                }
+                // A source shorter than the final basis block never reaches the
+                // probe: upstream's `end` would be non-positive and its scan
+                // would not run either.
+                if window.len() == tail_len {
+                    // Recompute rather than adjust: the digest carried out of
+                    // the loop covers neither the pre-drain window nor the
+                    // post-drain one (see the stale-`outgoing` note above), so
+                    // there is no increment that would repair it.
+                    rolling.reset();
+                    let (first, second) = window.as_slices();
+                    rolling.update(first);
+                    if !second.is_empty() {
+                        rolling.update(second);
+                    }
+                    index.find_tail_match_window(rolling.digest(), &window, &mut scratch)
+                } else {
+                    None
+                }
+            }
         };
         if let Some(block_index) = tail_matched_block {
             // A matched trailing partial block is one confirmed match, and its

crates/engine/src/local_copy/tests/execute_delta.rs

@@ -135,3 +135,137 @@ fn execute_with_report_records_min_size_skip_notice_without_copying() {
     assert_eq!(report.summary().regular_files_total(), 1);
     assert_eq!(report.summary().bytes_copied(), 0);
 }
+
+/// Deterministic pseudo-random bytes from a 64-bit LCG (Knuth's MMIX
+/// constants), taking the high bits so consecutive outputs do not share low-bit
+/// structure.
+///
+/// The generator matters as much as the sizes here. A short-period or repeating
+/// fixture lets an unaligned window match some earlier full block purely by
+/// content, which swallows the tail and makes the test assert about the data
+/// rather than about the matcher. This period is far longer than any fixture
+/// below.
+fn lcg_bytes(len: usize, seed: u64) -> Vec<u8> {
+    let mut state = seed;
+    (0..len)
+        .map(|_| {
+            state = state
+                .wrapping_mul(6_364_136_223_846_793_005)
+                .wrapping_add(1_442_695_040_888_963_407);
+            (state >> 33) as u8
+        })
+        .collect()
+}
+
+/// Runs one local delta copy whose basis is `full_blocks` whole blocks plus
+/// `tail` trailing bytes, with the source's LAST FULL block replaced.
+///
+/// Returns `(matched_bytes, literal_bytes)`.
+///
+/// Modifying the last full block is what makes the fixture load-bearing. It
+/// stops the scan matching block-aligned right up to the end, so the loop
+/// slides byte-by-byte and reaches EOF holding `block - 1` bytes - a full
+/// window. An identical source instead has a match clear the window exactly
+/// `tail` bytes early, which is the reachability accident that let the tail
+/// probe look correct: such a fixture passes with or without the fix.
+fn delta_tail_fixture(block: usize, full_blocks: usize, tail: usize) -> (u64, u64, bool) {
+    let temp = tempdir().expect("tempdir");
+    let source_path = temp.path().join("source.bin");
+    let dest_path = temp.path().join("dest.bin");
+
+    let len = full_blocks * block + tail;
+    let basis = lcg_bytes(len, 0x5DEE_CE66_D1CE_4B9D);
+    let mut source = basis.clone();
+    let modified_start = (full_blocks - 1) * block;
+    source[modified_start..modified_start + block]
+        .copy_from_slice(&lcg_bytes(block, 0x0BAD_C0DE_0BAD_C0DE));
+
+    fs::write(&dest_path, &basis).expect("write basis");
+    set_file_mtime(&dest_path, FileTime::from_unix_time(1, 0)).expect("dest mtime");
+    fs::write(&source_path, &source).expect("write source");
+    set_file_mtime(&source_path, FileTime::from_unix_time(2, 0)).expect("source mtime");
+
+    let operands = vec![
+        source_path.into_os_string(),
+        dest_path.clone().into_os_string(),
+    ];
+    let plan = LocalCopyPlan::from_operands(&operands).expect("plan");
+    let block_size = NonZeroU32::new(block as u32).expect("block size is non-zero");
+    let summary = plan
+        .execute_with_options(
+            LocalCopyExecution::Apply,
+            LocalCopyOptions::default()
+                .whole_file(false)
+                .with_block_size_override(Some(block_size)),
+        )
+        .expect("delta copy succeeds");
+
+    let exact = fs::read(&dest_path).expect("read dest") == source;
+    (summary.matched_bytes(), summary.bytes_copied(), exact)
+}
+
+#[test]
+fn delta_matches_the_basis_short_final_block_at_a_full_eof_window() {
+    // 292 x 700 + 400, the shape upstream was measured on. The 400-byte
+    // remainder is the only basis block whose recorded length is below the
+    // block length, so it is the only one that can satisfy upstream's
+    // `l = MIN(blength, len-offset); if (l != s->sums[i].len) continue;`
+    // (match.c:222-224) - and only at the file's own end, because upstream's
+    // scan is bounded by that block's length
+    // (`end = len + 1 - s->sums[s->count-1].len`, match.c:174) and its window
+    // shrinks to reach it (match.c:321,331).
+    //
+    // WHY THIS ASSERTION AND NOT BYTE-EQUALITY: reconstruction is byte-exact
+    // either way. The defect is that the trailing block is re-sent as literal
+    // data that upstream matches, so only the literal/matched split can see it.
+    let (matched, literal, exact) = delta_tail_fixture(700, 292, 400);
+
+    assert!(exact, "reconstruction must be byte-exact");
+    // 291 untouched full blocks (203,700) plus the 400-byte tail.
+    assert_eq!(
+        matched, 204_100,
+        "the basis's short final block must be matched, not re-sent"
+    );
+    // Only the one modified full block is literal. Before the fix this was
+    // 1,100: the 700-byte modified block plus the 400-byte tail.
+    assert_eq!(literal, 700, "only the modified block may be literal");
+}
+
+#[test]
+fn delta_tail_match_handles_a_single_byte_final_block() {
+    // tail_len == 1 is the narrowest short block that exists. zsync 0.6 fixed
+    // an out-of-bounds access "when processing the last block of a
+    // non-compressed download", so the degenerate widths are exactly where this
+    // family of implementations has gone wrong before.
+    let (matched, literal, exact) = delta_tail_fixture(700, 4, 1);
+
+    assert!(exact, "reconstruction must be byte-exact");
+    assert_eq!(matched, 3 * 700 + 1, "a one-byte final block still matches");
+    assert_eq!(literal, 700);
+}
+
+#[test]
+fn delta_tail_match_handles_a_final_block_one_byte_short() {
+    // tail_len == block_length - 1 is the widest short block. It is the width
+    // the EOF window itself happens to hold, so a probe that used "whatever is
+    // left in the window" would pass here while failing every other width -
+    // this pins that the probe length comes from the basis, not from the
+    // window.
+    let (matched, literal, exact) = delta_tail_fixture(700, 4, 699);
+
+    assert!(exact, "reconstruction must be byte-exact");
+    assert_eq!(matched, 3 * 700 + 699);
+    assert_eq!(literal, 700);
+}
+
+#[test]
+fn delta_tail_probe_is_a_no_op_when_the_basis_has_no_short_block() {
+    // An exact multiple has no final short block, so upstream has nothing extra
+    // to offer and the probe must not fire at all. If it degraded to matching
+    // "whatever is left in the window" it could emit a spurious short Copy here.
+    let (matched, literal, exact) = delta_tail_fixture(700, 4, 0);
+
+    assert!(exact, "reconstruction must be byte-exact");
+    assert_eq!(matched, 3 * 700, "no tail block exists to match");
+    assert_eq!(literal, 700, "only the modified block is literal");
+}