diff --git a/crates/engine/src/local_copy/context_impl/delta_transfer.rs b/crates/engine/src/local_copy/context_impl/delta_transfer.rs index f37ab4a91..b1024d561 100644 --- a/crates/engine/src/local_copy/context_impl/delta_transfer.rs +++ b/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 diff --git a/crates/engine/src/local_copy/tests/execute_delta.rs b/crates/engine/src/local_copy/tests/execute_delta.rs index 0e57b5940..e2ab5d0ab 100644 --- a/crates/engine/src/local_copy/tests/execute_delta.rs +++ b/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 { + 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"); +}