test: pin the review findings
- ignore-errors scan-error test now runs single-threaded and under -m (the exact scan_directory_multithreaded path that had the use-after-free); - new integration test: --delete/--delete-before --ignore-errors with an unreadable SOURCE ROOT (sequential and -m) must fail and delete NOTHING; - new integration test: a destination-only file that merely matches an exclude rule is deleted under plain --delete (protection is sender-derived), while a source-excluded mirror is protected; - delete-protects/delete-excluded coverage extended to --delete-after and --delete-delay; - new unit test: the 100000-entry server hard bound is all-or-nothing (more extras than the bound -> nothing removed); - new integration test: --force is inert under --delay-updates (documented); - fixed the ignore-errors assertion message that stated the opposite of what it asserted.
This commit is contained in:
@@ -2136,12 +2136,13 @@ class TestDeletePolicy:
|
|||||||
fh.write(content)
|
fh.write(content)
|
||||||
|
|
||||||
@pytest.mark.parametrize("mt", [False, True])
|
@pytest.mark.parametrize("mt", [False, True])
|
||||||
@pytest.mark.parametrize("timing", ["--delete", "--delete-before"])
|
@pytest.mark.parametrize("timing",
|
||||||
|
["--delete", "--delete-before", "--delete-after", "--delete-delay"])
|
||||||
def test_delete_protects_excluded_by_default_and_delete_excluded_removes(self, mt, timing):
|
def test_delete_protects_excluded_by_default_and_delete_excluded_removes(self, mt, timing):
|
||||||
"""rsync parity: with --delete a destination mirror path whose source was
|
"""rsync parity: with a --delete timing the destination mirror path whose
|
||||||
excluded survives (protected by default); --delete-excluded opts back
|
source was excluded survives (protected by default); --delete-excluded
|
||||||
into deleting it. Verified single-threaded, -m, and an early timing
|
opts back into deleting it. Verified single-threaded and -m across every
|
||||||
run (--delete-before) where the manifest arrives before any data."""
|
timing (commit and early)."""
|
||||||
source = os.path.join(TEST_DATA_DIR, f"delexcl_{timing.strip('-')}_{mt}_src")
|
source = os.path.join(TEST_DATA_DIR, f"delexcl_{timing.strip('-')}_{mt}_src")
|
||||||
clean_dir(source)
|
clean_dir(source)
|
||||||
entries = {
|
entries = {
|
||||||
@@ -2307,8 +2308,34 @@ class TestDeletePolicy:
|
|||||||
assert os.path.isfile(os.path.join(received, "sub")), \
|
assert os.path.isfile(os.path.join(received, "sub")), \
|
||||||
"--force did not replace the directory with the file"
|
"--force did not replace the directory with the file"
|
||||||
assert _read_file(os.path.join(received, "sub")) == b"now a file\n"
|
assert _read_file(os.path.join(received, "sub")) == b"now a file\n"
|
||||||
assert not os.path.exists(os.path.join(received, "sub", "old.txt")), \
|
|
||||||
"--force left the old directory content behind"
|
def test_force_inert_under_delay_updates(self):
|
||||||
|
"""Documented divergence: --force acts on the immediate-install path; a
|
||||||
|
--delay-updates run stages into its own tree and its publication renames
|
||||||
|
over regular files only, so a blocking directory is not cleared and the
|
||||||
|
run fails."""
|
||||||
|
source = os.path.join(TEST_DATA_DIR, "force_delay_src")
|
||||||
|
clean_dir(source)
|
||||||
|
self._write(os.path.join(source, "sub", "old.txt"), b"old\n")
|
||||||
|
self._write(os.path.join(source, "keep.txt"), b"kept\n")
|
||||||
|
dest = os.path.join(TEST_DATA_DIR, "force_delay_dst")
|
||||||
|
clean_dir(dest)
|
||||||
|
with ServerManager() as server:
|
||||||
|
server.start(extra_args=["--allow-delete"])
|
||||||
|
result, _ = run_client(source, dest, port=server.port)
|
||||||
|
assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}"
|
||||||
|
received = get_dest_received_dir(dest, source)
|
||||||
|
os.unlink(os.path.join(source, "sub", "old.txt"))
|
||||||
|
os.rmdir(os.path.join(source, "sub"))
|
||||||
|
self._write(os.path.join(source, "sub"), b"now a file\n")
|
||||||
|
result, _ = run_client(source, dest, flags=["--force", "--delay-updates"],
|
||||||
|
port=server.port)
|
||||||
|
assert result.returncode != 0, \
|
||||||
|
"--force --delay-updates unexpectedly replaced the blocking directory"
|
||||||
|
assert os.path.isdir(os.path.join(received, "sub")), \
|
||||||
|
"blocking directory was cleared although --delay-updates should keep --force inert"
|
||||||
|
assert os.path.exists(os.path.join(received, "sub", "old.txt")), \
|
||||||
|
"blocking directory content was lost"
|
||||||
|
|
||||||
@pytest.mark.parametrize("mt", [False, True])
|
@pytest.mark.parametrize("mt", [False, True])
|
||||||
def test_prune_empty_dirs_dirs_mode(self, mt):
|
def test_prune_empty_dirs_dirs_mode(self, mt):
|
||||||
@@ -2396,14 +2423,22 @@ class TestDeletePolicy:
|
|||||||
assert os.path.exists(os.path.join(received, "a", "keep.log")), \
|
assert os.path.exists(os.path.join(received, "a", "keep.log")), \
|
||||||
"excluded file mirror was deleted under --delete (rsync protects it)"
|
"excluded file mirror was deleted under --delete (rsync protects it)"
|
||||||
|
|
||||||
def test_ignore_errors_keeps_deletion_active_on_scan_error(self):
|
def _run_client_as_nobody(self, source, dest, port, flags):
|
||||||
"""A source I/O error (unreadable directory) aborts the run so no
|
cmd = CLIENT_CMD + ["--source-dir", source, "--dest-dir", dest,
|
||||||
|
"--save-to-disk", "--server-port", str(port)] + flags
|
||||||
|
return subprocess.run(["setpriv", "--reuid=65534", "--regid=65534",
|
||||||
|
"--clear-groups"] + cmd, text=True, capture_output=True)
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("mt", [False, True])
|
||||||
|
def test_ignore_errors_keeps_deletion_active_on_scan_error(self, mt):
|
||||||
|
"""A source I/O error (unreadable subdirectory) aborts the run so no
|
||||||
deletion happens by default; --ignore-errors continues, still transfers
|
deletion happens by default; --ignore-errors continues, still transfers
|
||||||
the readable tree and still deletes. Run as an unprivileged user so the
|
the readable tree and still deletes, single-threaded and under -m. Run
|
||||||
mode-000 directory is genuinely unreadable."""
|
as an unprivileged user so the mode-000 directory is genuinely
|
||||||
|
unreadable."""
|
||||||
if os.geteuid() != 0 or shutil.which("setpriv") is None:
|
if os.geteuid() != 0 or shutil.which("setpriv") is None:
|
||||||
pytest.skip("requires root + setpriv to drop privileges for the client")
|
pytest.skip("requires root + setpriv to drop privileges for the client")
|
||||||
tag = f"ioerr_{os.getpid()}"
|
tag = f"ioerr_{os.getpid()}_{mt}"
|
||||||
source = os.path.join(TEST_DATA_DIR, f"{tag}_src")
|
source = os.path.join(TEST_DATA_DIR, f"{tag}_src")
|
||||||
clean_dir(source)
|
clean_dir(source)
|
||||||
self._write(os.path.join(source, "top.txt"), b"top\n")
|
self._write(os.path.join(source, "top.txt"), b"top\n")
|
||||||
@@ -2422,29 +2457,86 @@ class TestDeletePolicy:
|
|||||||
|
|
||||||
# Default: scan error aborts the run; nothing is deleted.
|
# Default: scan error aborts the run; nothing is deleted.
|
||||||
self._write(os.path.join(received, "extra.txt"), b"extra\n")
|
self._write(os.path.join(received, "extra.txt"), b"extra\n")
|
||||||
cmd = CLIENT_CMD + ["--source-dir", source, "--dest-dir", dest,
|
flags = ["--delete"] + (["-m"] if mt else [])
|
||||||
"--save-to-disk", "--server-port", str(server.port),
|
result = self._run_client_as_nobody(source, dest, server.port, flags)
|
||||||
"--delete"]
|
|
||||||
result = subprocess.run(["setpriv", "--reuid=65534", "--regid=65534",
|
|
||||||
"--clear-groups"] + cmd, text=True, capture_output=True)
|
|
||||||
assert result.returncode != 0, "unreadable source dir did not fail the run"
|
assert result.returncode != 0, "unreadable source dir did not fail the run"
|
||||||
assert os.path.exists(os.path.join(received, "extra.txt")), \
|
assert os.path.exists(os.path.join(received, "extra.txt")), \
|
||||||
"default run deleted although the scan hit an I/O error"
|
"default run deleted although the scan hit an I/O error"
|
||||||
|
|
||||||
# --ignore-errors: the readable tree transfers, deletion still runs.
|
# --ignore-errors: the readable tree transfers, deletion still runs.
|
||||||
self._write(os.path.join(received, "extra.txt"), b"extra\n")
|
self._write(os.path.join(received, "extra.txt"), b"extra\n")
|
||||||
cmd = CLIENT_CMD + ["--source-dir", source, "--dest-dir", dest,
|
flags = ["--delete", "--ignore-errors"] + (["-m"] if mt else [])
|
||||||
"--save-to-disk", "--server-port", str(server.port),
|
result = self._run_client_as_nobody(source, dest, server.port, flags)
|
||||||
"--delete", "--ignore-errors"]
|
|
||||||
result = subprocess.run(["setpriv", "--reuid=65534", "--regid=65534",
|
|
||||||
"--clear-groups"] + cmd, text=True, capture_output=True)
|
|
||||||
assert not os.path.exists(os.path.join(received, "extra.txt")), \
|
assert not os.path.exists(os.path.join(received, "extra.txt")), \
|
||||||
f"--ignore-errors did not keep deletion active: {result.stderr[:300]}"
|
f"--ignore-errors did not keep deletion active: {result.stderr[:300]}"
|
||||||
assert not os.path.exists(os.path.join(received, "locked")), \
|
assert not os.path.exists(os.path.join(received, "locked")), \
|
||||||
"mirror of the unreadable dir was not treated as an extra"
|
"mirror of the unreadable dir was left behind (should be an extra)"
|
||||||
finally:
|
finally:
|
||||||
os.chmod(os.path.join(source, "locked"), 0o755)
|
os.chmod(os.path.join(source, "locked"), 0o755)
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("mt", [False, True])
|
||||||
|
@pytest.mark.parametrize("timing", ["--delete", "--delete-before"])
|
||||||
|
def test_ignore_errors_unreadable_root_never_deletes(self, mt, timing):
|
||||||
|
"""An unreadable SOURCE ROOT must never be treated as a skippable scan
|
||||||
|
error: with --ignore-errors the sequential scanner treats the root as
|
||||||
|
fatal (matching the -m path, which cannot even create its scanner), so
|
||||||
|
no empty keep-set manifest is sent and the destination is never wiped.
|
||||||
|
Run as an unprivileged user so the mode-000 root is genuinely
|
||||||
|
unreadable."""
|
||||||
|
if os.geteuid() != 0 or shutil.which("setpriv") is None:
|
||||||
|
pytest.skip("requires root + setpriv to drop privileges for the client")
|
||||||
|
tag = f"rootio_{os.getpid()}_{mt}_{timing.strip('-')}"
|
||||||
|
source = os.path.join(TEST_DATA_DIR, f"{tag}_src")
|
||||||
|
clean_dir(source)
|
||||||
|
self._write(os.path.join(source, "file.txt"), b"content\n")
|
||||||
|
dest = os.path.join(TEST_DATA_DIR, f"{tag}_dst")
|
||||||
|
clean_dir(dest)
|
||||||
|
with ServerManager() as server:
|
||||||
|
server.start(extra_args=["--allow-delete"])
|
||||||
|
result, _ = run_client(source, dest, port=server.port)
|
||||||
|
assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}"
|
||||||
|
received = get_dest_received_dir(dest, source)
|
||||||
|
try:
|
||||||
|
os.chmod(source, 0)
|
||||||
|
self._write(os.path.join(received, "extra.txt"), b"extra\n")
|
||||||
|
flags = [timing, "--ignore-errors"] + (["-m"] if mt else [])
|
||||||
|
result = self._run_client_as_nobody(source, dest, server.port, flags)
|
||||||
|
assert result.returncode != 0, \
|
||||||
|
f"unreadable source root with {timing} (mt={mt}) unexpectedly succeeded"
|
||||||
|
assert os.path.exists(os.path.join(received, "file.txt")), \
|
||||||
|
f"{timing} (mt={mt}) wiped a kept destination file"
|
||||||
|
assert os.path.exists(os.path.join(received, "extra.txt")), \
|
||||||
|
f"{timing} (mt={mt}) deleted the extra although the scan could not read the root"
|
||||||
|
finally:
|
||||||
|
os.chmod(source, 0o755)
|
||||||
|
|
||||||
|
def test_delete_excluded_protection_is_sender_derived(self):
|
||||||
|
"""Plain --delete protects destination mirrors of files the SOURCE scan
|
||||||
|
excluded, but a destination-only file that merely matches an exclude
|
||||||
|
rule is still an extra and is removed (protection never re-applies rules
|
||||||
|
to the destination)."""
|
||||||
|
source = os.path.join(TEST_DATA_DIR, "senderderived_src")
|
||||||
|
clean_dir(source)
|
||||||
|
self._write(os.path.join(source, "keep.txt"), b"kept\n")
|
||||||
|
self._write(os.path.join(source, "secret.log"), b"secret\n")
|
||||||
|
dest = os.path.join(TEST_DATA_DIR, "senderderived_dst")
|
||||||
|
clean_dir(dest)
|
||||||
|
with ServerManager() as server:
|
||||||
|
server.start(extra_args=["--allow-delete"])
|
||||||
|
result, _ = run_client(source, dest, port=server.port)
|
||||||
|
assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}"
|
||||||
|
received = get_dest_received_dir(dest, source)
|
||||||
|
# A destination-only file that happens to match the exclude rule.
|
||||||
|
self._write(os.path.join(received, "stray.log"), b"never on the source\n")
|
||||||
|
result, _ = run_client(source, dest, flags=["--exclude", "*.log", "--delete"],
|
||||||
|
port=server.port)
|
||||||
|
assert result.returncode == 0, \
|
||||||
|
f"delete sync failed: {(result.stderr or result.stdout)[:300]}"
|
||||||
|
assert os.path.exists(os.path.join(received, "secret.log")), \
|
||||||
|
"source-excluded mirror was deleted under plain --delete"
|
||||||
|
assert not os.path.exists(os.path.join(received, "stray.log")), \
|
||||||
|
"destination-only file matching the exclude rule was left (should be deleted)"
|
||||||
|
|
||||||
|
|
||||||
def _pin_mtime(path, ts):
|
def _pin_mtime(path, ts):
|
||||||
os.utime(path, (ts, ts))
|
os.utime(path, (ts, ts))
|
||||||
|
|||||||
@@ -205,6 +205,53 @@ static void test_walker_unlimited_deletes_all() {
|
|||||||
free(root);
|
free(root);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* The 100000-entry server hard bound (MAX_SERVER_DELETE_COUNT, which this test
|
||||||
|
exercises through a literal to avoid reaching into file_receive.c) is also
|
||||||
|
all-or-nothing: a destination holding more extras than the bound must be left
|
||||||
|
completely untouched. Skipped under valgrind: 100k file creations would be
|
||||||
|
far too slow under instrumentation. */
|
||||||
|
static void test_walker_hard_bound_all_or_nothing() {
|
||||||
|
if (is_running_under_valgrind())
|
||||||
|
return;
|
||||||
|
enum { HARD_BOUND = 100000 };
|
||||||
|
char* root = make_walk_root("hardbound");
|
||||||
|
EXPECT_NOT_NULL(root);
|
||||||
|
int rootfd = open(root, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
|
||||||
|
EXPECT_TRUE(rootfd >= 0);
|
||||||
|
bool created = rootfd >= 0;
|
||||||
|
for (int i = 0; created && i < HARD_BOUND + 1; i++) {
|
||||||
|
char name[32];
|
||||||
|
snprintf(name, sizeof(name), "f%d", i);
|
||||||
|
int fd = openat(rootfd, name, O_WRONLY | O_CREAT | O_TRUNC, 0644);
|
||||||
|
if (fd < 0)
|
||||||
|
created = false;
|
||||||
|
else
|
||||||
|
close(fd);
|
||||||
|
}
|
||||||
|
EXPECT_TRUE(created);
|
||||||
|
const char* keeps[1] = {NULL};
|
||||||
|
ArrayList* manifest = make_manifest_strings(keeps, 0);
|
||||||
|
EXPECT_NOT_NULL(manifest);
|
||||||
|
size_t deleted = 999;
|
||||||
|
DeleteWalkResult result = delete_extras_limited(root, manifest, HARD_BOUND, NULL, 0, &deleted);
|
||||||
|
EXPECT_EQ_INT((int)result, (int)DELETE_WALK_LIMIT_EXCEEDED);
|
||||||
|
EXPECT_EQ_INT((int)deleted, 0);
|
||||||
|
EXPECT_TRUE(file_exists(root, "f0"));
|
||||||
|
EXPECT_TRUE(file_exists(root, "f100000"));
|
||||||
|
array_list_delete(manifest);
|
||||||
|
/* Fast cleanup: unlink every created name through the still-open root fd. */
|
||||||
|
if (rootfd >= 0) {
|
||||||
|
for (int i = 0; i < HARD_BOUND + 1; i++) {
|
||||||
|
char name[32];
|
||||||
|
snprintf(name, sizeof(name), "f%d", i);
|
||||||
|
(void)unlinkat(rootfd, name, 0);
|
||||||
|
}
|
||||||
|
close(rootfd);
|
||||||
|
}
|
||||||
|
rmdir(root);
|
||||||
|
free(root);
|
||||||
|
}
|
||||||
|
|
||||||
typedef struct {
|
typedef struct {
|
||||||
bool eight_bit_output;
|
bool eight_bit_output;
|
||||||
const char* expected;
|
const char* expected;
|
||||||
@@ -227,6 +274,7 @@ void test_shared_utils() {
|
|||||||
test_walker_max_delete_exceeded_deletes_nothing();
|
test_walker_max_delete_exceeded_deletes_nothing();
|
||||||
test_walker_max_delete_exact_bound_deletes();
|
test_walker_max_delete_exact_bound_deletes();
|
||||||
test_walker_unlimited_deletes_all();
|
test_walker_unlimited_deletes_all();
|
||||||
|
test_walker_hard_bound_all_or_nothing();
|
||||||
|
|
||||||
char formatted[32];
|
char formatted[32];
|
||||||
EXPECT_TRUE(format_human_bytes(0, formatted, sizeof(formatted)));
|
EXPECT_TRUE(format_human_bytes(0, formatted, sizeof(formatted)));
|
||||||
|
|||||||
Reference in New Issue
Block a user