From 402cae80ad57b6148317824972e22a13b1c2f357 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 20 Sep 2026 13:39:04 +0200 Subject: [PATCH] parity: rsync-exact relative basis-dir resolution and fuzzy eligibility Resolve a relative --compare-dest/--copy-dest/--link-dest DIR against the destination directory and append the file's transfer-relative name, as rsync 3.4.1 does, instead of appending FastSync's source-mirrored wire path (the historical spelling stays as a fallback for existing layouts). Stop inheriting the ordinary delta engine's 16 KiB minimum and 10x size ratio in the -y/--fuzzy candidate search: rsync's find_fuzzy has no delta-size gate, so an oversized or sub-16-KiB sibling is now reused. The ordinary delta path's bounds are unchanged. --- src/shared/file_receive.c | 127 ++++++++---- tests/integration/test_parity_basis_fuzzy.py | 198 +++++++++++++++++++ tests/integration/test_parity_quickwins.py | 38 ++-- 3 files changed, 304 insertions(+), 59 deletions(-) create mode 100644 tests/integration/test_parity_basis_fuzzy.py diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index b724c4f..d41ca1d 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -1389,6 +1389,43 @@ bool file_basis_content_required(const Config* config) { return config != NULL && config->verify_basis; } +/* Probe one candidate basis file: open it (confined, O_NOFOLLOW) and apply + rsync's metadata quick-check; under --verify-basis also hash its bytes and + require the sender's digest. On a hit record `candidate` in `out` and return + true. The caller retains ownership of `candidate`. */ +static bool basis_match_probe(const Config* config, const char* candidate, + unsigned long long check_size, time_t check_mtime, + long check_mtime_nsec, const uint8_t* check_digest, + size_t check_digest_len, BasisDestType type, BasisMatch* out) { + int fd; + struct stat st; + if (!basis_open_regular(candidate, check_size, &fd, &st)) + return false; + bool hit = false; + if (file_basis_quick_match(config, &st, check_mtime, check_mtime_nsec)) { + hit = true; + if (file_basis_content_required(config)) { + uint8_t basis_digest[CHECKSUM_MAX_DIGEST_LEN]; + size_t basis_len = 0; + bool hashed = checksum_digest_fd((ChecksumAlgo)config->checksum_algo, config->checksum_seed, + fd, basis_digest, sizeof(basis_digest), &basis_len); + hit = hashed && basis_len == check_digest_len && check_digest_len > 0 && + memcmp(basis_digest, check_digest, check_digest_len) == 0; + } + } + close(fd); + if (!hit) + return false; + char* owned = str_dup(candidate); + if (!owned) + return false; + out->hit = true; + out->type = type; + out->basis_path = owned; + out->st = st; + return true; +} + /* Search the basis-dir list in command-line order and return the first match. By default (no --verify-basis) rsync's metadata quick-check is sufficient: basis_open_regular has already required an equal size, and @@ -1403,7 +1440,22 @@ bool file_basis_content_required(const Config* config) { --dry-run passes false because hashing a basis against a client-supplied digest would be a 1-bit content oracle. Without --verify-basis a dry-run can still confirm the metadata-only hit without reading any basis bytes, matching - rsync's read-only quick-check. */ + rsync's read-only quick-check. + + Path resolution (rsync 3.4.1 parity): rsync resolves a relative + --compare-dest/--copy-dest/--link-dest DIR against the destination directory + (the receiver's cwd) and appends the file's TRANSFER-RELATIVE name, e.g. + `--compare-dest=basis` with `rsync src/ dst/` probes `dst/basis/`. + FastSync's receive root IS the destination directory, but its default transfer + mirrors the absolute source path below that root, so check_path carries the + source-root scaffolding rsync would not append. Recover rsync's spelling with + utils_strip_transfer_root for a relative DIR; under -R/--files-from the wire + path is already transfer-relative, so it is used as-is. A relative DIR also + probes the historical mirror-appended spelling as a fallback, so existing + FastSync-laid-out snapshot trees keep resolving. An absolute DIR is used + verbatim and keeps appending the destination-relative check_path (FastSync's + mirrored layout). Every candidate stays confined to the authorized root by + file_open_secure_parent. */ static bool basis_match_find(const Config* config, const char* check_path, unsigned long long check_size, time_t check_mtime, long check_mtime_nsec, const uint8_t* check_digest, @@ -1416,47 +1468,39 @@ static bool basis_match_find(const Config* config, const char* check_path, the basis bytes. */ if (file_basis_content_required(config) && !hash_content) return false; + const char* transfer_rel = check_path; + if (!config->relative && config->files_from_set == NULL) + transfer_rel = utils_strip_transfer_root(check_path, config->send_directory); for (int i = 0; i < config->basis_count; i++) { const BasisDest* entry = &config->basis_dirs[i]; /* An absolute basis path is used verbatim (rsync semantics); a relative one is resolved below the receive root. Both remain subject to the receiver's authorized-root confinement inside file_open_secure_parent. */ - char* basis_dir = entry->path[0] == '/' ? str_dup(entry->path) - : path_cat(config->receive_root_directory, entry->path); + bool absolute = entry->path[0] == '/'; + char* basis_dir = + absolute ? str_dup(entry->path) : path_cat(config->receive_root_directory, entry->path); if (!basis_dir) continue; - char* candidate = path_cat(basis_dir, check_path); - free(basis_dir); - if (!candidate) - continue; - - int fd; - struct stat st; - if (basis_open_regular(candidate, check_size, &fd, &st)) { - if (file_basis_quick_match(config, &st, check_mtime, check_mtime_nsec)) { - bool hit = true; - if (file_basis_content_required(config)) { - uint8_t basis_digest[CHECKSUM_MAX_DIGEST_LEN]; - size_t basis_len = 0; - bool hashed = - checksum_digest_fd((ChecksumAlgo)config->checksum_algo, config->checksum_seed, fd, - basis_digest, sizeof(basis_digest), &basis_len); - hit = hashed && basis_len == check_digest_len && check_digest_len > 0 && - memcmp(basis_digest, check_digest, check_digest_len) == 0; - } - if (hit) { - out->hit = true; - out->type = entry->type; - out->basis_path = candidate; - candidate = NULL; /* ownership transferred to out */ - out->st = st; - close(fd); - return true; - } - } - close(fd); + const char* names[2]; + int name_count = 0; + if (absolute) + names[name_count++] = check_path; + else + names[name_count++] = transfer_rel; + if (!absolute && strcmp(transfer_rel, check_path) != 0) + names[name_count++] = check_path; /* historical mirror-appended spelling */ + bool found = false; + for (int n = 0; n < name_count && !found; n++) { + char* candidate = path_cat(basis_dir, names[n]); + if (!candidate) + continue; + found = basis_match_probe(config, candidate, check_size, check_mtime, check_mtime_nsec, + check_digest, check_digest_len, entry->type, out); + free(candidate); } - free(candidate); + free(basis_dir); + if (found) + return true; } return false; } @@ -1489,9 +1533,12 @@ static bool basis_match_find(const Config* config, const char* check_path, * followed and nothing outside the destination root is ever read; * * dotfiles, directories, the target's own name, and the .fastsync-stage / * temp scratch names are never candidates; - * * size gate = the delta engine's own bounds (delta_should_attempt: both - * files >= DELTA_MIN_FILE_SIZE, <= delta_max_file_size, ratio <= 10x), - * because FastSync's delta engine cannot use a basis outside them; + * * size gate = rsync's, NOT the ordinary delta engine's bounds: any + * non-empty regular sibling up to the receiver's whole-file buffer cap is + * eligible, regardless of the 16 KiB delta minimum or the 10x delta size + * ratio (rsync's find_fuzzy has no delta-size gate at all). The delta + * engine consumes the fuzzy basis through the same signature handshake + * whether or not it is inside delta_should_attempt's window; * * first pass = an exact size+mtime match wins regardless of name (rsync's * "fuzzy size/modtime match"); * * otherwise the winner minimizes rsync's weighted Levenshtein distance @@ -1639,8 +1686,7 @@ static void* fuzzy_basis_find_and_load(const Config* config, const char* check_p long check_mtime_nsec, unsigned long long* out_size) { *out_size = 0; if (!config || !config->receive_root_directory || !config->fuzzy || !config->use_delta || - !check_path || check_size < DELTA_MIN_FILE_SIZE || check_size > config->delta_max_file_size || - check_size > MAX_RECEIVE_WHOLE_FILE_SIZE) + !check_path || check_size > MAX_RECEIVE_WHOLE_FILE_SIZE) return NULL; char* full_path = path_cat(config->receive_root_directory, check_path); @@ -1717,8 +1763,7 @@ static void* fuzzy_basis_find_and_load(const Config* config, const char* check_p if (fstatat(dir_fd, name, &st, AT_SYMLINK_NOFOLLOW) != 0 || !S_ISREG(st.st_mode)) continue; unsigned long long cand_size = (unsigned long long)st.st_size; - if (cand_size == 0 || cand_size > MAX_RECEIVE_WHOLE_FILE_SIZE || - !delta_should_attempt(cand_size, check_size, config->delta_max_file_size)) + if (cand_size == 0 || cand_size > MAX_RECEIVE_WHOLE_FILE_SIZE) continue; long cand_nsec = 0; #ifdef __linux__ diff --git a/tests/integration/test_parity_basis_fuzzy.py b/tests/integration/test_parity_basis_fuzzy.py new file mode 100644 index 0000000..9f4a72f --- /dev/null +++ b/tests/integration/test_parity_basis_fuzzy.py @@ -0,0 +1,198 @@ +"""Differential rsync-parity coverage for two residuals closed on this branch. + +* A4 -- ``--compare-dest``/``--copy-dest``/``--link-dest`` relative-DIR + resolution: rsync resolves a relative DIR against the destination directory + and appends the file's TRANSFER-RELATIVE name. FastSync's default transfer + mirrors the absolute source path below its receive root, so a naive relative + DIR used to probe a different tree. These tests seed the basis at rsync's + spelling and assert FastSync finds it (byte-exact / hard-linked / sparse), + matching real rsync 3.4.1. + +* A5 -- ``-y``/``--fuzzy`` candidate eligibility: rsync's ``find_fuzzy`` has no + delta-size gate, so it reuses an oversized (>10x) or sub-16-KiB sibling; + FastSync used to decline both. These tests assert FastSync now uses the same + sibling as rsync (observable as ``Matched data``) with a byte-exact result. + +Every test skips cleanly when rsync is absent. +""" +import os +import shutil +import subprocess +import sys + +import pytest + +sys.path.insert(0, os.path.dirname(__file__)) +from common import ( # noqa: E402 + TEST_DATA_DIR, + clean_dir, + get_dest_received_dir, + run_client, +) + +RSYNC = shutil.which("rsync") +requires_rsync = pytest.mark.skipif(RSYNC is None, reason="rsync 3.4.1 not installed") + +OLD_MTIME = 1_500_000_000 + + +def _write(path, content, mtime=None): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "wb") as fh: + fh.write(content) + if mtime is not None: + os.utime(path, (mtime, mtime)) + + +def _read(path): + with open(path, "rb") as fh: + return fh.read() + + +def _rsync(args): + env = dict(os.environ, LC_ALL="C") + return subprocess.run([RSYNC] + args, capture_output=True, text=True, env=env, timeout=120) + + +def _stat_bytes(text, label): + for line in text.splitlines(): + if line.startswith(label + ":"): + return int(line.split(":", 1)[1].strip().split()[0].replace(",", "")) + return None + + +class TestRelativeBasisDirResolution: + """A4: a relative basis DIR must resolve to the same tree as rsync's.""" + + _FILES = { + "root.txt": b"root-basis-content\n", + "sub/nested.txt": b"nested-basis-content\n", + } + + def _seed_source(self, source): + clean_dir(source) + for rel, data in self._FILES.items(): + _write(os.path.join(source, rel), data, OLD_MTIME) + return self._FILES + + @requires_rsync + @pytest.mark.parametrize("flag", ["--compare-dest", "--link-dest"]) + def test_relative_dir_resolves_like_rsync(self, shared_server, flag): + tag = flag.lstrip("-") + source = os.path.join(TEST_DATA_DIR, f"relbasis_{tag}_src") + rdst = os.path.join(TEST_DATA_DIR, f"relbasis_{tag}_rdst") + fdst = os.path.join(TEST_DATA_DIR, f"relbasis_{tag}_fdst") + self._seed_source(source) + + # rsync: relative DIR -> dest/basis/. + clean_dir(rdst) + for rel, data in self._FILES.items(): + _write(os.path.join(rdst, "basis", rel), data, OLD_MTIME) + rs = _rsync(["-a", f"{flag}=basis", source + "/", rdst + "/"]) + assert rs.returncode == 0, rs.stderr + + # FastSync: the SAME relative spelling seeded at the SAME + # transfer-relative location under its destination root. + clean_dir(fdst) + for rel, data in self._FILES.items(): + _write(os.path.join(fdst, "basis", rel), data, OLD_MTIME) + result, _ = run_client(source, fdst, + flags=["-a", f"{flag}=basis", "--incremental"], + port=shared_server.port) + assert result.returncode == 0, (result.stderr or result.stdout)[:300] + received = get_dest_received_dir(fdst, source) + + for rel, data in self._FILES.items(): + rfile = os.path.join(rdst, rel) + ffile = os.path.join(received, rel) + basis = os.path.join(fdst, "basis", rel) + if flag == "--compare-dest": + # compare-dest never copies: both destinations stay sparse. + assert not os.path.exists(rfile), f"rsync copied {rel}" + assert not os.path.exists(ffile), ( + f"FastSync did not resolve the relative basis DIR at {basis!r} " + f"(expected {rel!r} to stay sparse like rsync)") + else: + # link-dest hard-links; a basis miss would transfer a new file. + assert os.path.exists(ffile), f"FastSync lost {rel}" + assert _read(ffile) == data + assert os.stat(ffile).st_ino == os.stat(basis).st_ino, ( + f"FastSync did not hard-link {rel!r} to the relative basis at " + f"{basis!r} (basis not resolved like rsync)") + + @requires_rsync + def test_relative_dir_copy_dest_content(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "relbasis_copy_src") + fdst = os.path.join(TEST_DATA_DIR, "relbasis_copy_fdst") + self._seed_source(source) + clean_dir(fdst) + for rel, data in self._FILES.items(): + _write(os.path.join(fdst, "basis", rel), data, OLD_MTIME) + result, _ = run_client(source, fdst, + flags=["-a", "--copy-dest=basis", "--incremental"], + port=shared_server.port) + assert result.returncode == 0, (result.stderr or result.stdout)[:300] + received = get_dest_received_dir(fdst, source) + for rel, data in self._FILES.items(): + ffile = os.path.join(received, rel) + assert os.path.exists(ffile), f"copy-dest did not materialize {rel}" + assert _read(ffile) == data + assert os.stat(ffile).st_ino != os.stat(os.path.join(fdst, "basis", rel)).st_ino + + +class TestFuzzyEligibilityWindow: + """A5: --fuzzy candidate eligibility must match rsync's uncapped window.""" + + BASE = b"the quick brown fox jumps over the lazy dog\n" * 4000 + + def _run_pair(self, shared_server, tag, payload, sibling): + source = os.path.join(TEST_DATA_DIR, f"fzw_{tag}_src") + dest = os.path.join(TEST_DATA_DIR, f"fzw_{tag}_dst") + rdst = os.path.join(TEST_DATA_DIR, f"fzw_{tag}_rdst") + clean_dir(source) + clean_dir(dest) + clean_dir(rdst) + _write(os.path.join(source, "report_v2.txt"), payload) + for root in (rdst, get_dest_received_dir(dest, source)): + _write(os.path.join(root, "report_v1.txt"), sibling) + + rs = _rsync(["-a", "--no-whole-file", "--fuzzy", "--stats", + source + "/", rdst + "/"]) + assert rs.returncode == 0, rs.stderr + result, _ = run_client( + source, dest, + flags=["-a", "--incremental", "--delta", "--fuzzy", "--stats"], + port=shared_server.port) + assert result.returncode == 0, (result.stderr or result.stdout)[:300] + + # The reconstructed file is byte-exact in every case. + assert _read(os.path.join(get_dest_received_dir(dest, source), + "report_v2.txt")) == payload + return rs, result + + @requires_rsync + def test_oversized_sibling_eligible_like_rsync(self, shared_server): + """A sibling 20x the source is used by rsync; FastSync must too (its old + 10x delta-size gate declined it).""" + n = 65536 + payload = (self.BASE * ((n // len(self.BASE)) + 1))[:n] + sibling = (self.BASE * 200)[: n * 20] + rs, result = self._run_pair(shared_server, "big", payload, sibling) + assert _stat_bytes(rs.stdout, "Matched data") > 0, \ + "rsync should use a >10x fuzzy basis" + assert _stat_bytes(result.stdout, "Matched data") > 0, ( + "FastSync's fuzzy eligibility must accept a >10x sibling like rsync " + f"(Matched data={_stat_bytes(result.stdout, 'Matched data')})") + + @requires_rsync + def test_small_source_sibling_eligible_like_rsync(self, shared_server): + """A sub-16-KiB source with an identical sibling is used by rsync; + FastSync's old 16 KiB delta minimum declined it.""" + n = 8192 + payload = (self.BASE * ((n // len(self.BASE)) + 1))[:n] + rs, result = self._run_pair(shared_server, "small", payload, payload) + assert _stat_bytes(rs.stdout, "Matched data") > 0, \ + "rsync applies --fuzzy below 16 KiB" + assert _stat_bytes(result.stdout, "Matched data") > 0, ( + "FastSync's fuzzy eligibility must accept a sub-16-KiB source like " + f"rsync (Matched data={_stat_bytes(result.stdout, 'Matched data')})") diff --git a/tests/integration/test_parity_quickwins.py b/tests/integration/test_parity_quickwins.py index 596b468..d2e13b5 100644 --- a/tests/integration/test_parity_quickwins.py +++ b/tests/integration/test_parity_quickwins.py @@ -861,11 +861,11 @@ class TestFuzzy: byte-exact result. FastSync ports rsync 3.4.1's weighted-Levenshtein name heuristic, so where both delta engines admit the candidate the tools pick the same basis (the ``fuzzy_basis`` differential asserts the tree and the - Matched/Literal counters match with the block size pinned). The residual is - candidate ELIGIBILITY: FastSync's delta size gate (both files >= 16 KiB and - a <= 10x size ratio) is narrower than rsync's, which empirically uses a - fuzzy basis well beyond 10x and below 16 KiB. These tests pin the window - boundary and prove the byte-exact fallback on both sides of it.""" + Matched/Literal counters match with the block size pinned). Candidate + ELIGIBILITY is now rsync's too: the fuzzy search no longer inherits the + ordinary delta engine's 16 KiB minimum or 10x size-ratio bound, so an + oversized or sub-16-KiB sibling is reused exactly as rsync reuses it. + These tests pin that window on both sides.""" _BASE = b"the quick brown fox jumps over the lazy dog\n" * 4000 @@ -900,9 +900,10 @@ class TestFuzzy: return rs, result @requires_rsync - def test_fuzzy_above_size_window_declines_but_tree_exact(self, shared_server): - """A sibling >10x the source is used by rsync but declined by FastSync's - delta size-ratio gate; both destinations stay byte-identical.""" + def test_fuzzy_above_size_window_matches_rsync(self, shared_server): + """A sibling >10x the source is used by rsync and by FastSync: fuzzy + eligibility is rsync's, not the ordinary delta size-ratio gate; both + destinations stay byte-identical and both reuse the basis.""" n = 65536 payload = (self._BASE * ((n // len(self._BASE)) + 1))[:n] sibling = (self._BASE * 200)[: n * 20] @@ -911,15 +912,16 @@ class TestFuzzy: rs, result = self._run_both(shared_server, source, dest, rdst, payload, sibling) assert _stat_bytes(rs.stdout, "Matched data") > 0, \ - "rsync should still use a >10x fuzzy basis" - assert _stat_bytes(result.stdout, "Matched data") == 0, \ - "FastSync's 10x delta size-ratio gate must decline the oversized basis" - assert _stat_bytes(result.stdout, "Literal data") == n + "rsync should use a >10x fuzzy basis" + assert _stat_bytes(result.stdout, "Matched data") > 0, \ + "FastSync must accept a >10x fuzzy basis like rsync" + assert _stat_bytes(result.stdout, "Literal data") < n @requires_rsync - def test_fuzzy_below_delta_minimum_declines_but_tree_exact(self, shared_server): - """A sibling below the 16 KiB delta minimum is used by rsync but never - enters FastSync's delta/fuzzy path; both trees stay byte-identical.""" + def test_fuzzy_below_delta_minimum_matches_rsync(self, shared_server): + """A sibling below the 16 KiB delta minimum is used by rsync and by + FastSync: fuzzy eligibility no longer inherits the delta engine's + minimum; both trees stay byte-identical and both reuse the basis.""" n = 8192 payload = (self._BASE * ((n // len(self._BASE)) + 1))[:n] source, dest, rdst = (self._src("small"), self._dst("small"), @@ -928,9 +930,9 @@ class TestFuzzy: payload, payload) assert _stat_bytes(rs.stdout, "Matched data") > 0, \ "rsync applies --fuzzy below 16 KiB" - assert _stat_bytes(result.stdout, "Matched data") == 0, \ - "FastSync's 16 KiB delta minimum must bypass the fuzzy basis" - assert _stat_bytes(result.stdout, "Literal data") == n + assert _stat_bytes(result.stdout, "Matched data") > 0, \ + "FastSync must apply --fuzzy below 16 KiB like rsync" + assert _stat_bytes(result.stdout, "Literal data") < n class TestIgnoreExistingShortCircuit: