diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index f8dd698..802ff5d 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -1971,3 +1971,129 @@ class TestFilters: """--filter/-C/-F rule layer: excludes prune, ordering is first-match-wins, the default with no matching rule is include, and legacy --exclude remains an independent layer.""" + + +class TestDeleteTiming: + """rsync deletion-timing family. --delete-before/--delete-during transmit + the keep-set manifest BEFORE any file data (the receiver deletes extras and + acks first); --delete/--delete-after/--delete-delay commit deletions only + after the whole transfer succeeded. Every timing flag implies --delete.""" + + def _seed(self, tag): + source = os.path.join(TEST_DATA_DIR, f"deltiming_{tag}_src") + clean_dir(source) + entries = { + "top.txt": b"top level\n", + "sub/deep.txt": b"deeply nested file\n", + } + for rel, content in entries.items(): + full = os.path.join(source, rel) + os.makedirs(os.path.dirname(full), exist_ok=True) + with open(full, "wb") as fh: + fh.write(content) + return source + + @pytest.mark.parametrize("flag", ["--delete-before", "--delete-during", "--del", + "--delete-after", "--delete-delay"]) + @pytest.mark.parametrize("mt", [False, True]) + def test_flag_removes_extras_on_success(self, flag, mt): + """Every timing flag is accepted, implies --delete, and on a successful + transfer removes the destination extras exactly like plain --delete.""" + source = self._seed("ok") + dest = os.path.join(TEST_DATA_DIR, "deltiming_ok_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) + extra = os.path.join(received, "extra.txt") + with open(extra, "wb") as fh: + fh.write(b"should be deleted") + + flags = [flag] + (["-m"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, \ + f"{flag} sync failed: {(result.stderr or result.stdout)[:300]}" + assert not os.path.exists(extra), f"{flag} did not remove the extra file" + mismatches, missing = verify_transfer(source, received) + assert not missing, f"{flag} missing files: {missing}" + assert not mismatches, f"{flag} mismatched files: {mismatches}" + + @pytest.mark.parametrize("flag", ["--delete-before", "--delete-during", "--del"]) + @pytest.mark.parametrize("mt", [False, True]) + def test_early_flags_delete_before_data(self, flag, mt): + """--delete-before/--delete-during remove extras (and a file blocking a + destination directory) BEFORE data is applied, so a nested write that + would fail while the blocker still exists succeeds.""" + source = self._seed("early") + dest = os.path.join(TEST_DATA_DIR, "deltiming_early_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) + extra = os.path.join(received, "extra.txt") + with open(extra, "wb") as fh: + fh.write(b"extra file") + blocker = os.path.join(received, "sub") + shutil.rmtree(blocker) + with open(blocker, "wb") as fh: + fh.write(b"blocks the nested destination directory") + + flags = [flag] + (["-m"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, \ + f"{flag} (early delete) did not remove the blocker in time: " \ + f"{(result.stderr or result.stdout)[:300]}" + assert not os.path.exists(extra), f"{flag} did not delete the extra before data" + assert _read_file(os.path.join(received, "sub", "deep.txt")) == b"deeply nested file\n", \ + f"{flag}: nested file was not written after the early deletion" + + @pytest.mark.parametrize("flag", ["--delete", "--delete-after", "--delete-delay"]) + def test_late_flags_commit_only_after_success(self, flag): + """Plain --delete/--delete-after/--delete-delay defer deletion until the + whole transfer succeeds: a mid-transfer write failure must leave every + extra in place (commit-style safety).""" + source = self._seed("late") + dest = os.path.join(TEST_DATA_DIR, "deltiming_late_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) + extra = os.path.join(received, "extra.txt") + with open(extra, "wb") as fh: + fh.write(b"extra file") + blocker = os.path.join(received, "sub") + shutil.rmtree(blocker) + with open(blocker, "wb") as fh: + fh.write(b"blocks the nested destination directory") + + result, _ = run_client(source, dest, flags=[flag], port=server.port) + assert result.returncode != 0, \ + f"{flag} unexpectedly succeeded (deletion must be deferred)" + assert os.path.exists(extra), \ + f"{flag} removed an extra although the transfer failed" + assert os.path.isfile(blocker), \ + f"{flag} deleted the blocker although the transfer failed" + + def test_early_flag_respected_when_server_refuses_delete(self, shared_server): + """With an --allow-delete-less server the client's early timing still + completes (no deadlock on the pre-delete ack) and simply never deletes, + exactly like the plain server policy.""" + source = self._seed("refused") + dest = os.path.join(TEST_DATA_DIR, "deltiming_refused_dst") + clean_dir(dest) + result, _ = run_client(source, dest, port=shared_server.port) + assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + extra = os.path.join(received, "extra.txt") + with open(extra, "wb") as fh: + fh.write(b"extra file") + result, _ = run_client(source, dest, flags=["--delete-before"], port=shared_server.port) + assert result.returncode == 0, \ + f"--delete-before against a refuse-delete server failed: {result.stderr[:300]}" + assert os.path.exists(extra), "unauthorized delete removed an extra file" diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 425d8aa..250e687 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -595,8 +595,9 @@ static void test_parse_args_relative_no_implied_mkpath() { config_delete(cfg); } -/* --del is recognized as the rsync alias, but its timing mode is not implemented. */ -static void test_parse_args_delete_during_alias_unimplemented() { +/* --del is accepted as the rsync alias for --delete-during: it enables + * deletion with the during (early) timing. */ +static void test_parse_args_delete_during_alias() { static const char* const options[] = {"--del", "--delete-during"}; for (size_t i = 0; i < sizeof(options) / sizeof(options[0]); i++) { @@ -605,12 +606,81 @@ static void test_parse_args_delete_during_alias_unimplemented() { int positional_args[2]; int positional_count = 0; - EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), -1); - EXPECT_FALSE(cfg->use_delete); + EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->use_delete); + EXPECT_TRUE(cfg->delete_during); + EXPECT_FALSE(cfg->delete_before); + EXPECT_FALSE(cfg->delete_delay); + EXPECT_FALSE(cfg->delete_after); config_delete(cfg); } } +/* Each rsync deletion-timing flag is accepted and implies --delete. */ +static void test_parse_args_delete_timing_flags() { + Config* cfg = config_create(); + char* argv[] = {"fastsync", "--delete-before", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->use_delete); + EXPECT_TRUE(cfg->delete_before); + config_delete(cfg); + + cfg = config_create(); + char* argv_after[] = {"fastsync", "--delete-after", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv_after, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->use_delete); + EXPECT_TRUE(cfg->delete_after); + EXPECT_FALSE(cfg->delete_before); + config_delete(cfg); + + cfg = config_create(); + char* argv_delay[] = {"fastsync", "--delete-delay", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv_delay, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->use_delete); + EXPECT_TRUE(cfg->delete_delay); + EXPECT_FALSE(cfg->delete_before); + EXPECT_FALSE(cfg->delete_after); + config_delete(cfg); +} + +/* Two different delete-timing flags on one command line are a conflict, not a + * silent last-one-wins choice. */ +static void test_parse_args_delete_timing_conflict_rejected() { + Config* cfg = config_create(); + char* argv[] = {"fastsync", "--delete-before", "--delete-after", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->use_delete); + EXPECT_FALSE(validate_config(cfg)); + config_delete(cfg); + + cfg = config_create(); + char* argv2[] = {"fastsync", "--delete-during", "--delete-delay", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, argv2, positional_args, &positional_count), 0); + EXPECT_FALSE(validate_config(cfg)); + config_delete(cfg); +} + +/* A timing flag whose --delete was then negated away must be rejected: timing + * without deletion is meaningless. */ +static void test_parse_args_delete_timing_without_delete_rejected() { + Config* cfg = config_create(); + char* argv[] = {"fastsync", "--delete-before", "--no-delete", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0); + EXPECT_FALSE(cfg->use_delete); + EXPECT_TRUE(cfg->delete_before); + EXPECT_FALSE(validate_config(cfg)); + config_delete(cfg); +} + /* Parsed-but-unimplemented options must fail instead of being silently accepted. */ static void test_parse_args_rejects_unimplemented_options() { static const char* const options[] = {"--silent", @@ -626,7 +696,6 @@ static void test_parse_args_rejects_unimplemented_options() { "--append", "--append-verify", "--delete-excluded", - "--delete-after", "--max-delete", "--prune-empty-dirs", "-e", @@ -635,7 +704,6 @@ static void test_parse_args_rejects_unimplemented_options() { "--compare-dest", "--copy-dest", "--link-dest", - "--delete-before", "--address", "--bind-address", "--ipv6", @@ -1542,7 +1610,10 @@ void test_client_cli() { test_parse_args_unknown_option(); test_parse_args_dirs_aliases(); test_parse_args_relative_no_implied_mkpath(); - test_parse_args_delete_during_alias_unimplemented(); + test_parse_args_delete_during_alias(); + test_parse_args_delete_timing_flags(); + test_parse_args_delete_timing_conflict_rejected(); + test_parse_args_delete_timing_without_delete_rejected(); test_parse_args_rejects_unimplemented_options(); test_parse_args_quiet(); test_parse_args_human_readable(); diff --git a/tests/test_config.c b/tests/test_config.c index a89b498..557879b 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -438,6 +438,129 @@ static void test_config_delay_updates_reserved_backup_rejected() { config_delete(c); } +static void test_config_delete_timing_early_helper() { + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + EXPECT_FALSE(config_delete_timing_early(cfg)); + EXPECT_TRUE(config_has_valid_delete_timing(cfg)); + cfg->use_delete = true; + EXPECT_TRUE(config_has_valid_delete_timing(cfg)); + EXPECT_FALSE(config_delete_timing_early(cfg)); + config_delete(cfg); + + cfg = config_create(); + cfg->use_delete = true; + cfg->delete_before = true; + EXPECT_TRUE(config_delete_timing_early(cfg)); + EXPECT_TRUE(config_has_valid_delete_timing(cfg)); + config_delete(cfg); + + cfg = config_create(); + cfg->use_delete = true; + cfg->delete_during = true; + EXPECT_TRUE(config_delete_timing_early(cfg)); + EXPECT_TRUE(config_has_valid_delete_timing(cfg)); + config_delete(cfg); + + cfg = config_create(); + cfg->use_delete = true; + cfg->delete_delay = true; + EXPECT_FALSE(config_delete_timing_early(cfg)); + EXPECT_TRUE(config_has_valid_delete_timing(cfg)); + config_delete(cfg); + + cfg = config_create(); + cfg->use_delete = true; + cfg->delete_after = true; + EXPECT_FALSE(config_delete_timing_early(cfg)); + EXPECT_TRUE(config_has_valid_delete_timing(cfg)); + config_delete(cfg); + + /* Two simultaneous timings are invalid. */ + cfg = config_create(); + cfg->use_delete = true; + cfg->delete_before = true; + cfg->delete_after = true; + EXPECT_TRUE(config_delete_timing_early(cfg)); + EXPECT_FALSE(config_has_valid_delete_timing(cfg)); + config_delete(cfg); + + /* A timing flag without deletion is invalid. */ + cfg = config_create(); + cfg->delete_delay = true; + EXPECT_FALSE(config_has_valid_delete_timing(cfg)); + EXPECT_FALSE(config_delete_timing_early(cfg)); + config_delete(cfg); +} + +/* New delete-timing fields must survive config_send/config_receive unchanged, + and a config carrying two conflicting timings must be rejected. */ +static void test_config_delete_timing_wire_roundtrip() { + if (is_running_under_valgrind()) + return; + + struct { + bool before, during, delay, after; + } cases[] = { + {false, false, false, false}, {true, false, false, false}, {false, true, false, false}, + {false, false, true, false}, {false, false, false, true}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + int p[2]; + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + pid_t pid = fork(); + if (pid == 0) { + close(p[1]); + io_set_fds(p[0], p[0]); + Config* recv = config_receive(p[0]); + bool ok = recv != NULL; + if (ok) { + ok = recv->use_delete && recv->delete_before == cases[i].before && + recv->delete_during == cases[i].during && recv->delete_delay == cases[i].delay && + recv->delete_after == cases[i].after; + } + config_delete(recv); + close(p[0]); + _exit(ok ? 0 : 1); + } else { + close(p[0]); + io_set_fds(p[1], p[1]); + Config* send_cfg = config_create(); + EXPECT_NOT_NULL(send_cfg); + send_cfg->send_directory = str_dup("/src"); + send_cfg->receive_root_directory = str_dup("/dst"); + send_cfg->use_delete = true; + send_cfg->delete_before = cases[i].before; + send_cfg->delete_during = cases[i].during; + send_cfg->delete_delay = cases[i].delay; + send_cfg->delete_after = cases[i].after; + bool sent = config_send(p[1], send_cfg); + int status; + waitpid(pid, &status, 0); + close(p[1]); + config_delete(send_cfg); + EXPECT_TRUE(sent); + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0); + } + } +} + +/* The receiver-side wire validation rejects a keep-set config with two + conflicting delete-timing flags. */ +static void test_config_delete_timing_conflict_rejected() { + if (is_running_under_valgrind()) + return; + Config* c = config_create(); + EXPECT_NOT_NULL(c); + c->send_directory = str_dup("/src"); + c->receive_root_directory = str_dup("/dst"); + c->use_delete = true; + c->delete_before = true; + c->delete_delay = true; + EXPECT_FALSE(roundtrip_config_ok(c)); + config_delete(c); +} + static void test_config_is_remote_dest() { /* Valid SSH-style destinations */ EXPECT_TRUE(config_is_remote_dest("user@host:/path")); @@ -474,6 +597,9 @@ void test_config() { test_config_string_null_vs_empty_roundtrip(); test_config_temp_dir_roundtrip(); test_config_delay_updates_reserved_backup_rejected(); + test_config_delete_timing_wire_roundtrip(); + test_config_delete_timing_conflict_rejected(); } + test_config_delete_timing_early_helper(); test_config_is_remote_dest(); }