diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 81d58e4..78adf43 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -248,6 +248,25 @@ static int set_nonneg_int_option(int* dest, const char* value, const char* optio return 0; } +/* Parse a signed integer, clamping every negative value to -1. rsync's + --max-delete treats a negative argument (the deprecated -1 spelling) as "no + client limit", so -2/-5 must behave identically rather than being rejected. */ +static int set_signed_clamped_int_option(int* dest, const char* value, const char* option_name) { + if (!value || *value == '\0') { + log_message(LOG_LEVEL_ERROR, "%s must be an integer", option_name); + return -1; + } + char* endptr; + errno = 0; + long parsed = strtol(value, &endptr, 10); + if (errno != 0 || *endptr != '\0' || parsed < INT_MIN || parsed > INT_MAX) { + log_message(LOG_LEVEL_ERROR, "%s must be an integer", option_name); + return -1; + } + *dest = parsed < 0 ? -1 : (int)parsed; + return 0; +} + /* Forward decl: config_add_pattern is defined below, but the --remote-option * helper above needs it. */ static int config_add_pattern(char*** patterns, int* count, const char* value, const char* optname); @@ -616,6 +635,9 @@ typedef enum { OPT_POS_INT, OPT_NONNEG_INT, OPT_ULL, + /* A signed integer whose negative values are clamped to -1 (rsync's + "no limit" spelling for --max-delete). */ + OPT_SIGNED_INT, } OptKind; typedef struct { @@ -730,7 +752,7 @@ static const OptionEntry OPTION_TABLE[] = { {"--delete-delay", NULL, OPT_FLAG, offsetof(Config, delete_delay)}, {"--delete-after", NULL, OPT_FLAG, offsetof(Config, delete_after)}, {"--delete-excluded", NULL, OPT_FLAG, offsetof(Config, delete_excluded)}, - {"--max-delete", NULL, OPT_NONNEG_INT, offsetof(Config, max_delete)}, + {"--max-delete", NULL, OPT_SIGNED_INT, offsetof(Config, max_delete)}, {"--ignore-errors", NULL, OPT_FLAG, offsetof(Config, ignore_errors)}, {"--force", NULL, OPT_FLAG, offsetof(Config, force_delete)}, {"--prune-empty-dirs", "-m", OPT_FLAG, offsetof(Config, prune_empty_dirs)}, @@ -854,7 +876,8 @@ static const OptionEntry* find_table_option_with_equals(const char* arg, const c (entry->alias && strlen(entry->alias) == name_len && strncmp(arg, entry->alias, name_len) == 0)) { if (entry->kind == OPT_STRING || entry->kind == OPT_POS_INT || - entry->kind == OPT_NONNEG_INT || entry->kind == OPT_ULL) { + entry->kind == OPT_NONNEG_INT || entry->kind == OPT_ULL || + entry->kind == OPT_SIGNED_INT) { *value = equals + 1; return entry; } @@ -922,6 +945,8 @@ static int apply_table_option(Config* config, const OptionEntry* entry, const ch return set_positive_int_option((int*)field, value, entry->name); case OPT_NONNEG_INT: return set_nonneg_int_option((int*)field, value, entry->name); + case OPT_SIGNED_INT: + return set_signed_clamped_int_option((int*)field, value, entry->name); case OPT_ULL: { unsigned long long v; /* Size-limit options accept rsync-style suffixes (e.g. --max-size=2G); a @@ -2158,7 +2183,7 @@ static bool cli_long_takes_separate_value(const char* arg) { const OptionEntry* entry = find_table_option(arg); if (entry) return entry->kind == OPT_STRING || entry->kind == OPT_POS_INT || - entry->kind == OPT_NONNEG_INT || entry->kind == OPT_ULL; + entry->kind == OPT_NONNEG_INT || entry->kind == OPT_ULL || entry->kind == OPT_SIGNED_INT; static const char* const extra[] = { "--ssh-port", "--exclude", "--include", "--exclude-from", "--include-from", "--files-from", "--filter", "--delta-block", diff --git a/src/client/client_send.c b/src/client/client_send.c index 8b0cf1f..1c6702b 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -175,6 +175,8 @@ static bool prepare_scanner(const Config* config, int num_threads, PreparedScann options->ignore_missing_args = config->ignore_missing_args || config->delete_missing_args; options->excluded_paths = NULL; options->excluded_mutex = NULL; + options->size_skipped_paths = NULL; + options->synced_dirs = NULL; options->hardlinks = NULL; /* P7 Wave D: capture source directory metadata when a directory attribute is requested (-p for modes, -t for times unless -O omits them). Whether they @@ -654,7 +656,10 @@ static void mark_sender_done(PipelineContextSender* context) { file it processed, in send order: STATUS_NEXT means the file was written, STATUS_OK means the file was skipped/unchanged. Skipped sources are marked so the later removal pass keeps them. */ -static bool finalize_transfer(Client* client, const Config* config, ArrayList* remove_sources) { +static bool finalize_transfer(Client* client, const Config* config, ArrayList* remove_sources, + bool* delete_limit_out) { + if (delete_limit_out) + *delete_limit_out = false; if (!send_status(client->file_descriptor, STATUS_FINISHED)) return false; if (config->remove_source_files && remove_sources) { @@ -677,6 +682,15 @@ static bool finalize_transfer(Client* client, const Config* config, ArrayList* r Status status; if (!receive_status(client->file_descriptor, &status)) return false; + /* A capped --max-delete commit is a successful transfer that the client must + report with rsync's exit code 25 (not an error). */ + if (status == STATUS_DELETE_LIMIT) { + log_message(LOG_LEVEL_ERROR, + "Deletions stopped due to --max-delete limit; some deletions were skipped"); + if (delete_limit_out) + *delete_limit_out = true; + return true; + } if (status != STATUS_OK) { log_server_rejection("Receiver reported transfer failure"); return false; @@ -910,7 +924,8 @@ static int send_list_only(const Config* config) { frame. A heavily filtered source whose exclusion list is large therefore fails the run cleanly on the receiver rather than being truncated. */ static int send_delete_manifest(int fd, ArrayList* manifest, ArrayList* protected_prefixes, - ArrayList* missing_args) { + ArrayList* size_skipped, ArrayList* missing_args, + ArrayList* synced_dirs) { if (!send_status(fd, STATUS_MANIFEST)) return -1; int keep_count = manifest ? manifest->size : 0; @@ -920,12 +935,24 @@ static int send_delete_manifest(int fd, ArrayList* manifest, ArrayList* protecte if (!send_wire_str(fd, (char*)manifest->items[i])) return -1; } - int protected_count = protected_prefixes ? protected_prefixes->size : 0; + /* The receiver has ONE protected-prefix section; filter-excluded prefixes + (dropped under --delete-excluded) and size-pruned prefixes (always + protected) are concatenated into it. */ + int protected_count = + (protected_prefixes ? protected_prefixes->size : 0) + (size_skipped ? size_skipped->size : 0); if (!send_int(fd, protected_count)) return -1; - for (int i = 0; i < protected_count; i++) { - if (!send_wire_str(fd, (char*)protected_prefixes->items[i])) - return -1; + if (protected_prefixes) { + for (int i = 0; i < protected_prefixes->size; i++) { + if (!send_wire_str(fd, (char*)protected_prefixes->items[i])) + return -1; + } + } + if (size_skipped) { + for (int i = 0; i < size_skipped->size; i++) { + if (!send_wire_str(fd, (char*)size_skipped->items[i])) + return -1; + } } int missing_count = missing_args ? missing_args->size : 0; if (!send_int(fd, missing_count)) @@ -934,6 +961,13 @@ static int send_delete_manifest(int fd, ArrayList* manifest, ArrayList* protecte if (!send_wire_str(fd, (char*)missing_args->items[i])) return -1; } + int dirs_count = synced_dirs ? synced_dirs->size : 0; + if (!send_int(fd, dirs_count)) + return -1; + for (int i = 0; i < dirs_count; i++) { + if (!send_wire_str(fd, (char*)synced_dirs->items[i])) + return -1; + } return 0; } @@ -953,11 +987,12 @@ static int send_delete_manifest(int fd, ArrayList* manifest, ArrayList* protecte #define DELETE_ACK_KEEPALIVE_SEC 10 static bool send_delete_manifest_early(Client* client, ArrayList* manifest, - ArrayList* protected_prefixes, ArrayList* missing_args) { + ArrayList* protected_prefixes, ArrayList* size_skipped, + ArrayList* missing_args, ArrayList* synced_dirs) { if (!client || !manifest) return false; - if (send_delete_manifest(client->file_descriptor, manifest, protected_prefixes, missing_args) != - 0) + if (send_delete_manifest(client->file_descriptor, manifest, protected_prefixes, size_skipped, + missing_args, synced_dirs) != 0) return false; Status ack; /* The wait is long (up to an hour) and runs inline on this thread: a helper @@ -1761,7 +1796,8 @@ static int send_chunks_multithreaded(void* pipeline_context) { /* The keep-set manifest was prebuilt by a path-only pre-scan. Transmit it and wait for the receiver to delete extras before streaming any data. */ if (!send_delete_manifest_early(client, context->manifest, context->excluded_paths, - context->missing_args)) { + context->size_skipped_paths, context->missing_args, + context->synced_dirs)) { pipeline_cancel(context); disconnect_transfer_client(client); mark_sender_done(context); @@ -1868,13 +1904,15 @@ static int send_chunks_multithreaded(void* pipeline_context) { goto send_fail; } if (send_delete_manifest(client->file_descriptor, context->manifest, context->excluded_paths, - context->missing_args) != 0) + context->size_skipped_paths, context->missing_args, + context->synced_dirs) != 0) goto send_fail; } else if (context->config->delete_missing_args && !context->early_delete) { /* --delete-missing-args without --delete: no keep-set is built, but the exact-delete paths still ride the same manifest frame (commit once the transfer succeeded). */ - if (send_delete_manifest(client->file_descriptor, NULL, NULL, context->missing_args) != 0) + if (send_delete_manifest(client->file_descriptor, NULL, NULL, NULL, context->missing_args, + NULL) != 0) goto send_fail; } /* P7 Wave D: transmit the captured directory times last. The scanner thread @@ -1884,11 +1922,12 @@ static int send_chunks_multithreaded(void* pipeline_context) { if (!context->scan_stopped_early && !send_dir_times(client, context->config, context->dir_entries)) goto send_fail; - bool ok = finalize_transfer(client, context->config, context->remove_source_files); + bool delete_limit = false; + bool ok = finalize_transfer(client, context->config, context->remove_source_files, &delete_limit); + context->delete_limit = delete_limit; if (!ok && context->config->use_delete) log_message(LOG_LEVEL_ERROR, - "server reported a deletion failure (--delete); see the server log for the " - "reason (a --max-delete limit that the run would exceed deletes nothing)"); + "server reported a deletion failure (--delete); see the server log for the reason"); if (ok) remove_transferred_sources(context->config, context->remove_source_files); mtx_lock(&context->mutex_progress); @@ -1931,11 +1970,18 @@ static int scan_directory_multithreaded(void* pipeline_context) { prepared.options.dir_entries = context->dir_entries; prepared.options.dir_entries_mutex = &context->dir_entries_mutex; /* The keep-set manifest for the late modes is built from this data pass, so - the parallel scanner records the protected excluded prefixes here. The - early modes already transmitted the pre-scan keep-set and its protected - list, so the data pass must not append to it again. */ - if (!context->early_delete) + the parallel scanner records the protected excluded prefixes and the + synchronized directories here (the size-prune protection is collected in + every mode). The early modes already transmitted the pre-scan keep-set and + its protected lists, so the data pass must not append to them again. */ + if (!context->early_delete) { prepared.options.excluded_paths = context->excluded_paths; + /* The root marker for a full recursive transfer is already in the list; do + not let the scanner append every directory to it. */ + if (context->config->files_from_set != NULL) + prepared.options.synced_dirs = context->synced_dirs; + } + prepared.options.size_skipped_paths = context->size_skipped_paths; bool dirs_mode = prepared.options.dirs; /* -H also selects the sequential scanner (see the comment at the branch), * so the loop below must choose the scanner by which object exists, not by @@ -2242,6 +2288,9 @@ int send_files(Config* config) { ArrayList* dir_entries = NULL; /* Protected excluded prefixes (delete-excluded default protection). */ ArrayList* excluded = NULL; + /* Size-pruned prefixes (always protected) and synchronized directories. */ + ArrayList* size_skipped = NULL; + ArrayList* synced_dirs = NULL; bool delete_early = config->use_delete && config_delete_timing_early(config); bool send_failed = false; bool had_scan_io = false; @@ -2265,11 +2314,31 @@ int send_files(Config* config) { by user-selection rules so the receiver protects their destination mirrors from --delete (rsync's default). Only scans that build the keep-set get the sink attached (prescan for early timing, the streaming data pass otherwise). */ - if (config->use_delete && !config->delete_excluded) { - excluded = array_list_create(free); - if (!excluded) + if (config->use_delete) { + if (!config->delete_excluded) { + excluded = array_list_create(free); + if (!excluded) + goto send_fail; + prepared.options.excluded_paths = excluded; + } + size_skipped = array_list_create(free); + synced_dirs = array_list_create(free); + if (!size_skipped || !synced_dirs) goto send_fail; - prepared.options.excluded_paths = excluded; + prepared.options.size_skipped_paths = size_skipped; + /* Only a --files-from subset confines the extras walk to the directories + the scan synchronized; a full recursive transfer deletes throughout the + receive root, so mark the root itself (the "." sentinel) and let the + scanner record nothing extra. */ + if (config->files_from_set == NULL) { + char* root_marker = str_dup("."); + if (!root_marker || !array_list_add(synced_dirs, root_marker)) { + free(root_marker); + goto send_fail; + } + } else { + prepared.options.synced_dirs = synced_dirs; + } } /* The late-timing modes (plain --delete / --delete-after / --delete-delay) build the manifest while streaming and send it after the last data frame. @@ -2296,13 +2365,16 @@ int send_files(Config* config) { "with an empty keep-set (--delete)"); prescan_ok = false; } else { - early_ok = send_delete_manifest_early(client, early_manifest, excluded, missing_args); + early_ok = send_delete_manifest_early(client, early_manifest, excluded, size_skipped, + missing_args, synced_dirs); } } array_list_delete(early_manifest); - /* The keep-set (and its protected prefixes) are already on the wire; the - data pass must not append to the exclusion list again. */ + /* The keep-set (and its protected prefixes and synchronized directories) are + already on the wire; the data pass must not append to those lists again. */ prepared.options.excluded_paths = NULL; + prepared.options.size_skipped_paths = NULL; + prepared.options.synced_dirs = NULL; if (!prescan_ok || !early_ok) goto send_fail; } else if (config->use_delete) { @@ -2446,7 +2518,8 @@ int send_files(Config* config) { --delete-missing-args exact-path deletions only after the transfer succeeds. In the early modes (--delete-before/--delete-during) the manifest already went out up front, so nothing is re-sent here. */ - if (send_delete_manifest(client->file_descriptor, manifest, excluded, missing_args) != 0) { + if (send_delete_manifest(client->file_descriptor, manifest, excluded, size_skipped, + missing_args, synced_dirs) != 0) { if (manifest) { array_list_delete(manifest); manifest = NULL; @@ -2464,11 +2537,11 @@ int send_files(Config* config) { applying them until after its own deletion/publication phase. */ if (!send_dir_times(client, config, dir_entries)) goto send_fail; - bool ok = finalize_transfer(client, config, remove_sources); + bool delete_limit = false; + bool ok = finalize_transfer(client, config, remove_sources, &delete_limit); if (!ok && config->use_delete) log_message(LOG_LEVEL_ERROR, - "server reported a deletion failure (--delete); see the server log for the " - "reason (a --max-delete limit that the run would exceed deletes nothing)"); + "server reported a deletion failure (--delete); see the server log for the reason"); if (ok) remove_transferred_sources(config, remove_sources); if (config->show_progress && !config->quiet) @@ -2477,8 +2550,13 @@ int send_files(Config* config) { log_info_message(LOG_INFO_STATS, "Transfer summary: %d files, %.1f MB", total_files, (double)total_bytes / (double)BYTES_PER_MIB); /* --ignore-errors: an unreadable source directory was skipped but the run - still completed (and deleted); report the run as errored like rsync does. */ - ret = (ok && !had_scan_io) ? 0 : 1; + still completed (and deleted); report the run as errored like rsync does. + A --max-delete-capped commit is a successful transfer that rsync reports + with exit code 25. */ + if (!ok || had_scan_io) + ret = 1; + else + ret = delete_limit ? 25 : 0; send_fail: /* Single cleanup path for all exits. The manifest is intentionally deleted @@ -2487,6 +2565,10 @@ send_fail: array_list_delete(manifest); if (excluded) array_list_delete(excluded); + if (size_skipped) + array_list_delete(size_skipped); + if (synced_dirs) + array_list_delete(synced_dirs); if (missing_args) array_list_delete(missing_args); if (remove_sources) @@ -2592,16 +2674,41 @@ int send_files_multithreaded(Config** config_ptr) { return 1; } } + /* Size-pruned mirrors stay protected under every mode (even + --delete-excluded); synchronized directories confine the walk. A full + recursive transfer marks the receive root itself (".") so the walk is not + confined; only a --files-from subset records concrete directories. */ + context->size_skipped_paths = array_list_create(free); + context->synced_dirs = array_list_create(free); + if (!context->size_skipped_paths || !context->synced_dirs) { + pipeline_context_sender_destroy(context); + return 1; + } + if (config->files_from_set == NULL) { + char* root_marker = str_dup("."); + if (!root_marker || !array_list_add(context->synced_dirs, root_marker)) { + free(root_marker); + pipeline_context_sender_destroy(context); + return 1; + } + } if (config_delete_timing_early(config)) { /* --delete-before/--delete-during: build the complete keep-set manifest (paths only, nothing loaded or sent) up front so the sender thread can transmit it before the first data byte. The path-only pre-scan also - fills the protected excluded prefixes. */ + fills the protected excluded prefixes and synchronized directories. */ PreparedScanner prepared; memset(&prepared, 0, sizeof(prepared)); bool prepared_ok = prepare_scanner(config, config->scanner_threads, &prepared); - if (prepared_ok && context->excluded_paths) - prepared.options.excluded_paths = context->excluded_paths; + if (prepared_ok) { + if (context->excluded_paths) + prepared.options.excluded_paths = context->excluded_paths; + prepared.options.size_skipped_paths = context->size_skipped_paths; + /* The root marker for a full recursive transfer is already in the list; + only a --files-from subset needs the scanner to record directories. */ + if (config->files_from_set != NULL) + prepared.options.synced_dirs = context->synced_dirs; + } bool prebuilt = prepared_ok && scan_paths_only(config, &prepared.options, context->manifest, &context->scan_had_io_error); prepared_scanner_destroy(&prepared); @@ -2683,9 +2790,13 @@ int send_files_multithreaded(Config** config_ptr) { scan_io = context->scan_had_io_error; mtx_unlock(&context->mutex_scanner); bool sender_ok = sender_result == thrd_success; + bool delete_limit = context->delete_limit; /* --ignore-errors: the run completed (and deleted) past an unreadable source - directory; report it as errored like rsync does. */ + directory; report it as errored like rsync does. A --max-delete-capped + commit is a successful transfer that rsync reports with exit code 25. */ pipeline_context_sender_destroy(context); client_set_abort_armed(false); - return sender_ok && !scan_io ? 0 : 1; + if (!sender_ok || scan_io) + return 1; + return delete_limit ? 25 : 0; } diff --git a/src/client/scanner.c b/src/client/scanner.c index 11074b6..caef32e 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -156,9 +156,13 @@ typedef struct { bool is_symlink; char* link_target; /* True when the entry was pruned by a user selection rule (--filter/-C/per-dir - rules, the --exclude/--include layer, or --max-size/--min-size) rather than - skipped for another reason (unreadable, symlink policy, not applicable). */ + rules or the --exclude/--include layer) rather than skipped for another + reason (unreadable, symlink policy, not applicable). */ bool excluded; + /* True when the entry was skipped specifically by --max-size/--min-size. + Size pruning protects the destination mirror even under --delete-excluded, + so it is recorded into a separate sink from `excluded`. */ + bool size_excluded; } ScannerEntry; /* --one-file-system (-x) decision. Only directories can carry a different @@ -302,19 +306,49 @@ static bool excluded_sink_append(ArrayList* list, mtx_t* mtx, const char* rel) { return ok; } -/* Record one pruned-by-user-selection filesystem path in the scanner's - exclusion sink (see ScannerOptions.excluded_paths). The stored form is the - entry's wire/destination-relative path (a single leading '/' removed, exactly - how manifest keep entries are stored), so the receiver's walker prefixes - match the destination layout. An allocation failure is a fatal scan error. */ -static void scanner_record_excluded(DirectoryScanner* scanner, const char* fs_path) { - if (!scanner->options.excluded_paths || !fs_path) +/* Record one pruned filesystem path in a delete-protection sink. The stored + form is the entry's wire/destination-relative path (a single leading '/' + removed, exactly how manifest keep entries are stored), so the receiver's + walker prefixes match the destination layout. An allocation failure is a + fatal scan error. */ +static void scanner_record_protected(DirectoryScanner* scanner, const char* fs_path, + ArrayList* sink) { + if (!sink || !fs_path) return; const char* rel = *fs_path == '/' ? fs_path + 1 : fs_path; - if (!excluded_sink_append(scanner->options.excluded_paths, scanner->options.excluded_mutex, rel)) + if (!excluded_sink_append(sink, scanner->options.excluded_mutex, rel)) scanner->failed = true; } +/* A user-selection exclusion (--filter/-C/per-dir or --exclude/--include). */ +static void scanner_record_excluded(DirectoryScanner* scanner, const char* fs_path) { + scanner_record_protected(scanner, fs_path, scanner->options.excluded_paths); +} + +/* A --max-size/--min-size prune (always protected, even under --delete-excluded). */ +static void scanner_record_size_skipped(DirectoryScanner* scanner, const char* fs_path) { + scanner_record_protected(scanner, fs_path, scanner->options.size_skipped_paths); +} + +/* Record a directory the scan synchronized. `fs_path` is its absolute path and + `rel` its path relative to the transfer root ("" for the root); the stored + form matches the wire layout (the bare relative path in -R+--files-from, else + the source path with a leading '/' removed, with "." for the receive root). + Returns false on allocation failure. */ +static bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_path, + const char* rel, bool relative_mode) { + if (!options->synced_dirs) + return true; + if (!file_list_dir_in_scope(options->file_list, rel)) + return true; + const char* dest = relative_mode ? rel : fs_path; + if (dest[0] == '/') + dest++; + if (dest[0] == '\0') + dest = "."; + return excluded_sink_append(options->synced_dirs, options->excluded_mutex, dest); +} + /* Merge the open directory's own .rsync-filter rules into the inherited * context, returning the context used for this directory's entries. On a parse * error the scanner is marked failed. Returns 0 on success, -1 on failure. */ @@ -357,6 +391,7 @@ static int open_directory_filter_context(DirectoryScanner* scanner, const Filter static int scanner_inspect_entry(const ScannerOptions* options, const char* containing_dir, const char* link_rel, const char* name, ScannerEntry* entry) { entry->excluded = false; + entry->size_excluded = false; entry->is_symlink = false; entry->link_target = NULL; entry->path = path_cat(containing_dir, name); @@ -443,6 +478,7 @@ apply_filters: if ((options->max_size > 0 && (unsigned long long)entry->stats.st_size > options->max_size) || (options->min_size > 0 && (unsigned long long)entry->stats.st_size < options->min_size)) { entry->excluded = true; + entry->size_excluded = true; goto skip; } return 1; @@ -722,6 +758,18 @@ static int open_next_directory(DirectoryScanner* scanner) { scanner->current_path = NULL; return -1; } + /* A successfully opened directory is synchronized for --delete: record it + so the receiver confines its extras walk to these (and the root sentinel + ".") instead of the whole receive root. */ + if (!scanner_record_synced_dir(&scanner->options, scanner->current_path, scanner->current_rel, + scanner->relative_mode)) { + closedir(scanner->current_dir); + scanner->current_dir = NULL; + free(scanner->current_path); + scanner->current_path = NULL; + scanner->failed = true; + return -1; + } if (scanner->options.capture_dir_times && !scanner_capture_dir_time(scanner->options.dir_entries, scanner->options.dir_entries_mutex, scanner->root_path, scanner->current_path, scanner->relative_mode, @@ -1041,16 +1089,19 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { break; } if (inspection == 0) { - /* The entry was pruned by a user selection rule (exclude/include/size) or - skipped for another reason; only the user-selection prunes protect the - corresponding destination mirror from --delete. */ + /* A user-selection exclude protects its destination mirror from --delete + unless --delete-excluded; a size prune is always protected. Other + skips (unreadable, symlink policy) protect nothing. */ if (inspected.excluded) { char* abs_path = path_cat(scanner->current_path, entry->d_name); if (!abs_path) { scanner->failed = true; break; } - scanner_record_excluded(scanner, abs_path); + if (inspected.size_excluded) + scanner_record_size_skipped(scanner, abs_path); + else + scanner_record_excluded(scanner, abs_path); free(abs_path); } continue; @@ -1404,16 +1455,19 @@ static void scan_root_entry(const ScannerOptions* options, const FilterNode* roo return; } if (inspection == 0) { - if (inspected.excluded && options->excluded_paths) { - /* A root-level user-selection prune protects the destination mirror of - the same-named wire path (at the root the bare name is the wire path in - every layout). */ + ArrayList* sink = NULL; + if (inspected.excluded) + sink = inspected.size_excluded ? options->size_skipped_paths : options->excluded_paths; + if (sink) { + /* A root-level prune protects the destination mirror of the same-named + wire path (at the root the bare name is the wire path in every + layout). */ char* abs_path = path_cat(root_directory, entry->d_name); if (!abs_path) { ps->failed = true; } else { const char* rel = *abs_path == '/' ? abs_path + 1 : abs_path; - if (!excluded_sink_append(options->excluded_paths, options->excluded_mutex, rel)) + if (!excluded_sink_append(sink, options->excluded_mutex, rel)) ps->failed = true; free(abs_path); } @@ -1534,6 +1588,14 @@ static bool scan_root_directory(ParallelScanner* ps, const char* root_directory, log_perror("Could not open root directory for parallel scan"); return false; } + /* The parallel scanner opens the transfer root directly (not through + open_next_directory), so record it as synchronized here. */ + if (!scanner_record_synced_dir(options, root_directory, "", + options->relative && options->file_list != NULL)) { + closedir(dir); + ps->failed = true; + return false; + } const struct dirent* entry; while ((entry = readdir(dir)) != NULL) { if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) diff --git a/src/client/scanner.h b/src/client/scanner.h index 51200e2..774b08d 100644 --- a/src/client/scanner.h +++ b/src/client/scanner.h @@ -73,17 +73,32 @@ typedef struct { bool prune_empty_dirs; /* Delete-excluded protection sink (optional): when non-NULL the scanner * appends the destination-relative path of every entry it prunes because a - * USER SELECTION rule excluded it (--filter/-C/per-dir rules, the legacy - * --exclude/--include layer, and --max-size/--min-size). The sender turns - * this list into the manifest's protected prefixes so `--delete` leaves the - * destination mirror of excluded source paths alone (rsync's default), and - * empties it when --delete-excluded opts back into deleting them. NOT - * recorded for --files-from subset pruning (whose delete semantics stay - * keep-set-only) or for -R/--files-from relative wire paths. When - * `excluded_mutex` is non-NULL it is taken around every append (the parallel - * scanner shares one list across its worker threads). */ + * USER SELECTION rule excluded it (--filter/-C/per-dir rules and the legacy + * --exclude/--include layer). The sender turns this list into the manifest's + * protected prefixes so `--delete` leaves the destination mirror of excluded + * source paths alone (rsync's default), and drops it when --delete-excluded + * opts back into deleting them. NOT recorded for --files-from subset pruning + * (whose delete semantics derive from the synchronized-directory set) or for + * -R/--files-from relative wire paths. When `excluded_mutex` is non-NULL it + * is taken around every append (the parallel scanner shares one list across + * its worker threads). */ ArrayList* excluded_paths; mtx_t* excluded_mutex; + /* Size-prune protection sink (optional): when non-NULL the scanner appends + * the destination-relative path of every entry it skipped because of + * --max-size/--min-size. rsync never deletes a size-skipped source mirror, + * even under --delete-excluded, so the sender always transmits this list as + * protected prefixes (unlike excluded_paths, which --delete-excluded drops). + * Guarded by `excluded_mutex` like excluded_paths. */ + ArrayList* size_skipped_paths; + /* Synchronized-directory sink (optional): when non-NULL the scanner appends + * the destination-relative path of every directory it is about to traverse + * that lies inside a --files-from listed directory (or of every traversed + * directory when there is no list). The sender sends this set with the delete + * manifest so the receiver confines its extras walk to synchronized + * directories, exactly like rsync; the receive root is the "." sentinel. + * Guarded by `excluded_mutex`. */ + ArrayList* synced_dirs; /* --ignore-errors: an unreadable directory during the scan is recorded as an * I/O error and skipped instead of aborting the scan. Client-only. */ bool ignore_io_errors; diff --git a/src/server/receiver.c b/src/server/receiver.c index c2e2e8a..51b935c 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -43,17 +43,19 @@ void receiver_outcomes_destroy(ReceiverOutcomes* outcomes) { /* End-of-transfer success frame. When --remove-source-files was negotiated each processed data file is acknowledged first (STATUS_NEXT = written, STATUS_OK = skipped) so the sender never removes a source the receiver did - not actually store. The frame always ends with a plain STATUS_OK. */ -bool receiver_send_final_success(int fd, const Config* config, const ReceiverOutcomes* outcomes) { + not actually store. The frame ends with `final_status` (STATUS_OK, or + STATUS_DELETE_LIMIT when a --max-delete commit was capped). */ +bool receiver_send_final_success(int fd, const Config* config, const ReceiverOutcomes* outcomes, + Status final_status) { if (!config->remove_source_files) - return send_status(fd, STATUS_OK); + return send_status(fd, final_status); size_t count = outcomes ? outcomes->count : 0; for (size_t i = 0; i < count; i++) { Status per_file = outcomes->entries[i] == FILE_SAVE_WRITTEN ? STATUS_NEXT : STATUS_OK; if (!send_status(fd, per_file)) return false; } - return send_status(fd, STATUS_OK); + return send_status(fd, final_status); } static bool receiver_process_chunk(Chunk* chunk, const ReceiverSink* sink) { @@ -346,15 +348,19 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver moment it arrives, before any file data. Delete now and acknowledge so the sender only starts streaming once the deletion committed (or failed). This is the rsync delete-before/delete-during window: a - later transfer failure does not restore these deletions. */ - bool deletion_ok = (config->use_delete || config->delete_missing_args) - ? manifest_delete_all(config, manifest) - : true; + later transfer failure does not restore these deletions. A + --max-delete-capped commit still succeeds and the transfer proceeds; + the terminal success frame reports the cap. */ + DeleteCommitResult deletion = (config->use_delete || config->delete_missing_args) + ? manifest_delete_all(config, manifest) + : DELETE_COMMIT_OK; delete_manifest_free(manifest); - if (!deletion_ok) { + if (deletion == DELETE_COMMIT_ERROR) { send_status(file_descriptor, STATUS_ERROR); goto fail; } + if (deletion == DELETE_COMMIT_LIMIT_REACHED && sink->note_delete_limit) + sink->note_delete_limit(sink->context); if (!send_status(file_descriptor, STATUS_OK)) goto fail; } else if (config->use_delete || config->delete_missing_args) { @@ -407,13 +413,15 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver *pending_manifest = deferred_manifest; deferred_manifest = NULL; } else { - bool deletion_ok = manifest_delete_all(config, deferred_manifest); + DeleteCommitResult deletion = manifest_delete_all(config, deferred_manifest); delete_manifest_free(deferred_manifest); deferred_manifest = NULL; - if (!deletion_ok) { + if (deletion == DELETE_COMMIT_ERROR) { send_status(file_descriptor, STATUS_ERROR); goto fail; } + if (deletion == DELETE_COMMIT_LIMIT_REACHED && sink->note_delete_limit) + sink->note_delete_limit(sink->context); } } if (sink->send_success) { @@ -454,6 +462,9 @@ typedef struct { after the whole transfer (and its delete/publication phases) has run so a child write never clobbers a directory mtime. */ DirTimeList dir_times; + /* Set when a --max-delete commit was capped; the terminal frame then carries + STATUS_DELETE_LIMIT so the sender exits 25 like rsync. */ + bool delete_limit_reached; } ReceiverSaveContext; static bool receiver_save_file(File* file, void* context_pointer) { @@ -494,12 +505,18 @@ static bool receiver_save_file(File* file, void* context_pointer) { return result != FILE_SAVE_ERROR; } +static void receiver_note_delete_limit(void* context_pointer) { + ReceiverSaveContext* context = context_pointer; + context->delete_limit_reached = true; +} + static bool receiver_send_success_frame(int fd, void* context_pointer) { ReceiverSaveContext* context = context_pointer; + Status final_status = context->delete_limit_reached ? STATUS_DELETE_LIMIT : STATUS_OK; /* Server-contacting --dry-run: nothing was staged or written, so there is nothing to publish and no directory times to stamp. */ if (context->config->dry_run) - return receiver_send_final_success(fd, context->config, &context->outcomes); + return receiver_send_final_success(fd, context->config, &context->outcomes, final_status); /* --delay-updates: the whole protocol stream (including manifest/delete handling, which ran inside receiver_process) has succeeded and every staged file was fully written. Publish them atomically now, before the @@ -517,13 +534,14 @@ static bool receiver_send_success_frame(int fd, void* context_pointer) { before calling this success frame. */ dir_metadata_list_apply(&context->dir_times, context->config->receive_root_directory, context->config); - return receiver_send_final_success(fd, context->config, &context->outcomes); + return receiver_send_final_success(fd, context->config, &context->outcomes, final_status); } int receiver_receive_files(Config* config, int file_descriptor) { ReceiverSaveContext context = {.config = config, .outcomes = {0}}; dir_time_list_init(&context.dir_times); - ReceiverSink sink = {receiver_save_file, &context, true, true, receiver_send_success_frame}; + ReceiverSink sink = {receiver_save_file, &context, true, true, receiver_send_success_frame, + receiver_note_delete_limit}; int ret = receiver_process(config, file_descriptor, &sink); if (ret != 0 && config->delay_updates && config->delay_context) delay_updates_cleanup(config->delay_context); diff --git a/src/server/receiver.h b/src/server/receiver.h index 9f3efa3..e169c41 100644 --- a/src/server/receiver.h +++ b/src/server/receiver.h @@ -4,6 +4,7 @@ #include "config.h" #include "file.h" #include "file_receive.h" +#include "protocol.h" #include #include @@ -21,6 +22,12 @@ typedef struct { typedef bool (*ReceiverSuccessFrame)(int fd, void* context); +/* Records that a --max-delete commit stopped with extras left over, so the + caller's terminal success frame can carry STATUS_DELETE_LIMIT instead of + STATUS_OK. The commit runs on the receiver thread, so the flag is stored in + the sink's own context rather than in a shared global. */ +typedef void (*ReceiverNoteDeleteLimit)(void* context); + typedef struct { ReceiverFileSink store_file; void* context; @@ -28,13 +35,18 @@ typedef struct { bool send_success; /* Emits the end-of-transfer success frame. When the sender requested --remove-source-files this includes one per-file status per processed - data file followed by the final STATUS_OK; otherwise just STATUS_OK. */ + data file followed by the final status; otherwise just the final status. */ ReceiverSuccessFrame send_success_frame; + /* Optional; may be NULL when the sink has no --max-delete handling. */ + ReceiverNoteDeleteLimit note_delete_limit; } ReceiverSink; bool receiver_outcomes_append(ReceiverOutcomes* outcomes, unsigned char code); void receiver_outcomes_destroy(ReceiverOutcomes* outcomes); -bool receiver_send_final_success(int fd, const Config* config, const ReceiverOutcomes* outcomes); +/* Send the terminal success frame. `final_status` is usually STATUS_OK, or + STATUS_DELETE_LIMIT when a --max-delete commit was capped. */ +bool receiver_send_final_success(int fd, const Config* config, const ReceiverOutcomes* outcomes, + Status final_status); int receiver_process(Config* config, int file_descriptor, const ReceiverSink* sink); /* receiver_process with an escape hatch for the commit-style (late) deletion: diff --git a/src/server/receiver_pipeline.c b/src/server/receiver_pipeline.c index 8e9d662..c983adf 100644 --- a/src/server/receiver_pipeline.c +++ b/src/server/receiver_pipeline.c @@ -27,6 +27,7 @@ PipelineContextReceiver* pipeline_context_receiver_create(Config* config, Queue* context->queued_bytes = 0; context->max_queue_bytes = 0; context->deferred_manifest = NULL; + context->delete_limit_reached = false; atomic_init(&context->cancelled, false); int init = 0; if (mtx_init(&context->mutex, mtx_plain) != thrd_success) @@ -135,6 +136,15 @@ static bool receiver_enqueue_file(File* file, void* context_pointer) { return pipeline_context_receiver_enqueue_file(context, file); } +/* Early delete modes (--delete-before/--delete-during) commit the manifest + inside receiver_process_pending on this thread; record a capped commit so + server.c's terminal frame can report STATUS_DELETE_LIMIT. The plain bool is + safe: receive_thread writes it before the main thread joins the thread. */ +static void receiver_pipeline_note_delete_limit(void* context_pointer) { + PipelineContextReceiver* context = (PipelineContextReceiver*)context_pointer; + context->delete_limit_reached = true; +} + static void receiver_thread_fail(PipelineContextReceiver* context) { mtx_lock(&context->mutex); atomic_store(&context->cancelled, true); @@ -152,7 +162,8 @@ int receive_thread(void* pipeline_context) { const Config* config = context->config; mtx_unlock(&context->mutex); - ReceiverSink sink = {receiver_enqueue_file, context, false, false, NULL}; + ReceiverSink sink = { + receiver_enqueue_file, context, false, false, NULL, receiver_pipeline_note_delete_limit}; if (receiver_process_pending((Config*)config, file_descriptor, &sink, &context->deferred_manifest) != 0) { receiver_thread_fail(context); diff --git a/src/server/receiver_pipeline.h b/src/server/receiver_pipeline.h index 7aef579..2f9d604 100644 --- a/src/server/receiver_pipeline.h +++ b/src/server/receiver_pipeline.h @@ -41,6 +41,10 @@ typedef struct PipelineContextReceiver { transfer truly succeeded. NULL in the early delete modes (which delete at the manifest). */ DeleteManifest* deferred_manifest; + /* Set by server.c when the deferred delete commit hit the --max-delete + budget; the terminal success frame then carries STATUS_DELETE_LIMIT + (rsync exit 25) while the transfer itself still succeeds. */ + bool delete_limit_reached; /* P7 Wave D: directory metadata collected by write_thread from received directory entries. Only write_thread mutates it (before it joins); the caller (server.c) applies it after the delete/delay-updates phase. */ diff --git a/src/server/server.c b/src/server/server.c index f0ffad2..ea975bb 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -949,8 +949,13 @@ void handler(int file_descriptor) { --delay-updates run; the walker skips the staging directory. A server-contacting --dry-run deletes nothing (no manifest is sent). */ if (context->deferred_manifest) { - if (!manifest_delete_all(config, context->deferred_manifest)) { + DeleteCommitResult deletion = manifest_delete_all(config, context->deferred_manifest); + if (deletion == DELETE_COMMIT_ERROR) { transfer_ok = false; + } else if (deletion == DELETE_COMMIT_LIMIT_REACHED) { + /* The transfer still succeeds; the terminal frame reports the capped + deletion so the sender exits 25 like rsync. */ + context->delete_limit_reached = true; } delete_manifest_free(context->deferred_manifest); context->deferred_manifest = NULL; @@ -974,7 +979,8 @@ void handler(int file_descriptor) { dir_metadata_list_apply(&context->dir_times, config->receive_root_directory, config); } if (transfer_ok) { - if (!receiver_send_final_success(file_descriptor, config, &context->outcomes)) + Status final_status = context->delete_limit_reached ? STATUS_DELETE_LIMIT : STATUS_OK; + if (!receiver_send_final_success(file_descriptor, config, &context->outcomes, final_status)) transfer_ok = false; } else { send_error_detail(file_descriptor, "transfer failed on receiver"); diff --git a/src/shared/config.h b/src/shared/config.h index 8a5317c..575b0b8 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -850,8 +850,25 @@ typedef struct Config { * version before parsing anything else) is what keeps a 2.22 client and a 2.21 * server from ever reaching that state. The fixed-width FileMetadata layout is * UNCHANGED: the receiver still gates attribute application on use_metadata, - * which is now DERIVED from these attributes by config_derived_use_metadata(). */ -#define PROTOCOL_VERSION "2.22.0" + * which is now DERIVED from these attributes by config_derived_use_metadata(). + * + * Delete-Semantics Wave (#290): 2.22.0 -> 2.23.0. + * + * WHY the bump, grounded in the wire: the delete-manifest frame gains a fourth + * trailing section (protocol 2.23.0): a synchronized-directory count followed by + * that many destination-relative directory paths (the receive root is "."). + * The receiver confines its extras walk to these directories, so `--files-from` + * with `--delete` only removes inside listed directory subtrees (rsync parity) + * instead of deleting every untransmitted path under the receive root. The + * frame stream also gains STATUS_DELETE_LIMIT, the terminal success status sent + * instead of STATUS_OK when a --max-delete commit removes up to the bound and + * skips the rest (the sender then exits 25 like rsync). The config-frame LAYOUT + * is unchanged. Any manifest/frame-sequence change must bump the protocol + * version: a 2.22 peer would desynchronize on the extra trailing section or the + * unknown status, and the strict same-version handshake (config_receive rejects + * a mismatched version before parsing anything else) is what keeps a 2.23 client + * and a 2.22 server from ever reaching that state. */ +#define PROTOCOL_VERSION "2.23.0" #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024) /* Upper bound on total basis-dir entries (rsync caps --link-dest at 20). */ #define MAX_BASIS_DIRS 64 diff --git a/src/shared/delay_updates.c b/src/shared/delay_updates.c index c867023..bfaca1b 100644 --- a/src/shared/delay_updates.c +++ b/src/shared/delay_updates.c @@ -264,6 +264,20 @@ static bool delay_publish_entry(DelayUpdatesContext* context, const Config* conf const StagedFileEntry* entry) { if (!delay_publish_backup(context, config, entry)) return false; + /* --force: an incoming regular file/symlink may replace a destination + DIRECTORY (possibly non-empty). The immediate-install path handles this in + file_receive; a --delay-updates run stages elsewhere and only discovers the + blocking directory here, so clear it before the rename (rsync's + "could not make way for new regular file" without --force). */ + if (config && config->force_delete && file_directory_exists_secure(entry->final_path)) { + if (!file_remove_tree_secure(entry->final_path)) { + char* escaped = output_escape(entry->final_path, false); + log_message(LOG_LEVEL_ERROR, "could not remove destination directory blocking '%s': %s", + escaped ? escaped : "", strerror(errno)); + free(escaped); + return false; + } + } if (!file_rename_secure(entry->staged_path, entry->final_path)) { if (errno == EXDEV) { char* escaped = output_escape(entry->final_path, false); diff --git a/src/shared/file_list.c b/src/shared/file_list.c index 5b2a138..e861221 100644 --- a/src/shared/file_list.c +++ b/src/shared/file_list.c @@ -240,3 +240,29 @@ bool file_list_affects(const FileListSet* set, const char* rel) { entry (binary search for the first entry at or after `rel` + '/'). */ return path_index_has_descendant(&set->index, rel); } + +bool file_list_dir_in_scope(const FileListSet* set, const char* rel) { + if (!set || set->whole_tree) + return true; + if (!rel || rel[0] == '\0') + return false; + /* `rel` itself is listed, or one of its ancestor prefixes is an exact listed + directory (a listed prefix of a directory path is necessarily a + directory). */ + size_t len = strlen(rel); + while (len > 0) { + const char* slash = NULL; + for (size_t i = len; i-- > 0;) { + if (rel[i] == '/') { + slash = rel + i; + break; + } + } + if (!slash) + break; + len = (size_t)(slash - rel); + if (path_index_contains_n(&set->index, rel, len)) + return true; + } + return path_index_contains(&set->index, rel); +} diff --git a/src/shared/file_list.h b/src/shared/file_list.h index b18a8ee..d9e86f3 100644 --- a/src/shared/file_list.h +++ b/src/shared/file_list.h @@ -40,4 +40,14 @@ void file_list_destroy(FileListSet* set); * this returns true, files are transferred only when it returns true. */ bool file_list_affects(const FileListSet* set, const char* rel); +/* True when the DIRECTORY `rel` (path relative to the source root) is inside a + * listed directory subtree: `rel` itself is a listed entry, or one of `rel`'s + * ancestor directory prefixes is an exact listed entry. Unlike + * file_list_affects this does NOT treat an ancestor of a listed entry as + * affected, so an implied parent directory of a listed file is not synchronized + * (rsync deletes nothing in it). With no set or a whole-tree set every + * directory is in scope. This is the delete-walker's "synchronized directory" + * predicate. */ +bool file_list_dir_in_scope(const FileListSet* set, const char* rel); + #endif diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 7c09860..94e7d2c 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -2845,8 +2845,10 @@ File* file_receive_special(int file_descriptor) { /* Read a delete-manifest frame (the STATUS_MANIFEST leading code has already been consumed): a keep-set entry count followed by that many destination-relative paths, then a protected-prefix count followed by that - many destination-relative prefixes, then (protocol 2.10.0+) a missing-args - count followed by that many destination-relative delete paths. The frame is + many destination-relative prefixes, then a missing-args count followed by that + many destination-relative delete paths, then (protocol 2.23.0) a + synchronized-directory count followed by that many destination-relative + directory paths (the receive root is the "." sentinel). The frame is self-delimiting (the counts are authoritative), so the caller decides what to do next and continues reading the following STATUS_* frame. Every section is validated identically: an entry must be non-empty, relative and traversal-free @@ -2891,7 +2893,8 @@ DeleteManifest* receive_manifest_entries(int fd) { manifest->keeps = array_list_create(free); manifest->protected = array_list_create(free); manifest->missing = array_list_create(free); - if (!manifest->keeps || !manifest->protected || !manifest->missing) { + manifest->dirs = array_list_create(free); + if (!manifest->keeps || !manifest->protected || !manifest->missing || !manifest->dirs) { delete_manifest_free(manifest); send_status(fd, STATUS_ERROR); return NULL; @@ -2900,7 +2903,8 @@ DeleteManifest* receive_manifest_entries(int fd) { size_t manifest_entries = 0; if (!receive_manifest_section(fd, manifest->keeps, &manifest_bytes, &manifest_entries) || !receive_manifest_section(fd, manifest->protected, &manifest_bytes, &manifest_entries) || - !receive_manifest_section(fd, manifest->missing, &manifest_bytes, &manifest_entries)) { + !receive_manifest_section(fd, manifest->missing, &manifest_bytes, &manifest_entries) || + !receive_manifest_section(fd, manifest->dirs, &manifest_bytes, &manifest_entries)) { delete_manifest_free(manifest); return NULL; } @@ -2913,18 +2917,33 @@ void delete_manifest_free(DeleteManifest* manifest) { array_list_delete(manifest->keeps); array_list_delete(manifest->protected); array_list_delete(manifest->missing); + array_list_delete(manifest->dirs); free(manifest); } +/* Shared --max-delete budget for one receiver-side deletion commit. Both the + --delete-missing-args exact-path removals and the ordinary extras walk draw + from the same tally, matching rsync (whose --max-delete counts every deleted + file or directory). `max_delete` is SIZE_MAX for an unlimited budget. */ +typedef struct { + size_t max_delete; + size_t deleted; + size_t skipped; + bool limit_hit; +} DeleteBudgetState; + /* Remove every destination entry under the receive root that is not in the - keep-set, bounded by MAX_SERVER_DELETE_COUNT (or a smaller client - --max-delete=NUM, which is all-or-nothing), using the symlink-safe delete - walker. With --delay-updates the not-yet-published staging directory is a + keep-set, bounded by the shared budget (a smaller client --max-delete=NUM + replaces the server hard bound; rsync deletes up to the bound and skips the + rest). With --delay-updates the not-yet-published staging directory is a direct child of the receive root and must not be treated as a set of extras; - the manifest's protected prefixes (paths excluded on the source) and the + the manifest's protected prefixes (paths excluded on the source), the + size-pruned prefixes (--max-size/--min-size, always protected) and the alternate basis directories are never destination content and are skipped at - any depth. Prints a notice and returns true on success. */ -bool manifest_delete_extras(const Config* config, DeleteManifest* manifest) { + any depth. Returns true unless a traversal/unlink error aborted the walk; + the budget's limit_hit/skipped fields report a cap-stopped run. */ +static bool delete_extras_budgeted(const Config* config, DeleteManifest* manifest, + DeleteBudgetState* budget) { if (!config || !manifest || !manifest->keeps) return false; fprintf(stderr, "Deleting files not in manifest...\n"); @@ -2936,9 +2955,10 @@ bool manifest_delete_extras(const Config* config, DeleteManifest* manifest) { at any depth: they are extra comparison snapshots the user pointed at, not destination content, and deleting them would destroy the very files a --link-dest run just linked into place; - - the sender-side protected prefixes (source paths excluded by filters), at - any depth, so an excluded destination mirror survives --delete unless - --delete-excluded opts back into removing it. */ + - the sender-side protected prefixes (source paths excluded by filters and + paths pruned by --max-size/--min-size), at any depth, so their destination + mirror survives --delete unless --delete-excluded opts back into removing + the filter-excluded ones (size-pruned entries are always protected). */ int skip_count = (config->delay_updates ? 1 : 0) + config->basis_count + (manifest->protected ? manifest->protected->size : 0); DeleteSkipEntry* skips = NULL; @@ -2963,30 +2983,19 @@ bool manifest_delete_extras(const Config* config, DeleteManifest* manifest) { idx++; } } - /* A client --max-delete=NUM smaller than the server's hard bound replaces it - for this run; both still bound the walk. The walker is all-or-nothing, so - a run that would delete more than the bound removes nothing and fails with - an error that names the bound that was hit. */ - bool user_limited = - config->max_delete >= 0 && (size_t)config->max_delete < MAX_SERVER_DELETE_COUNT; - size_t cap = user_limited ? (size_t)config->max_delete : MAX_SERVER_DELETE_COUNT; - size_t deleted_count = 0; - DeleteWalkResult result = delete_extras_limited(config->receive_root_directory, manifest->keeps, - cap, skips, skip_count, &deleted_count); + size_t remaining = + budget->max_delete == SIZE_MAX ? SIZE_MAX : budget->max_delete - budget->deleted; + size_t deleted = 0; + size_t skipped = 0; + DeleteWalkResult result = + delete_extras_limited(config->receive_root_directory, manifest->keeps, manifest->dirs, + remaining, skips, skip_count, &deleted, &skipped); free(skips); - if (result == DELETE_WALK_LIMIT_EXCEEDED) { - if (user_limited) { - log_message(LOG_LEVEL_ERROR, - "deletion stopped: the destination holds more than --max-delete=%d extraneous " - "entries; no files were deleted", - config->max_delete); - } else { - log_message(LOG_LEVEL_ERROR, - "deletion stopped: the destination holds more than %u extraneous entries " - "(server deletion limit); no files were deleted", - (unsigned)MAX_SERVER_DELETE_COUNT); - } - return false; + budget->deleted += deleted; + budget->skipped += skipped; + if (result == DELETE_WALK_LIMIT_REACHED) { + budget->limit_hit = true; + return true; } if (result != DELETE_WALK_OK) { log_message(LOG_LEVEL_ERROR, "deletion failed while removing extraneous files"); @@ -3004,10 +3013,12 @@ bool manifest_delete_extras(const Config* config, DeleteManifest* manifest) { removed recursively only when --delete or --force is in effect (rsync parity: the man page says a non-empty directory mirror is only deleted with --force or --delete); otherwise it is left with a warning and the run continues. A - mirror that does not exist is a no-op. Returns false only on a genuine error - (a confinement failure on a validated path or an I/O error), which fails the - run. */ -bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest) { + mirror that does not exist is a no-op. Each removal draws from the shared + --max-delete budget: once it is exhausted the remaining requests are skipped + and counted. Returns false only on a genuine error (a confinement failure on + a validated path or an I/O error), which fails the run. */ +static bool delete_missing_args_budgeted(const Config* config, DeleteManifest* manifest, + DeleteBudgetState* budget) { if (!config || !manifest) return false; if (!manifest->missing || manifest->missing->size == 0) @@ -3081,6 +3092,16 @@ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest free(full); continue; } + /* An entry that exists is one deletion: skip it (and count it) when the + shared --max-delete budget is already exhausted. */ + if (budget->deleted >= budget->max_delete) { + budget->limit_hit = true; + budget->skipped++; + close(parent_fd); + free(leaf); + free(full); + continue; + } bool removed = false; if (S_ISDIR(st.st_mode)) { if (unlinkat(parent_fd, leaf, AT_REMOVEDIR) == 0) { @@ -3091,10 +3112,35 @@ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest free(leaf); leaf = NULL; if (config->use_delete || config->force_delete) { - if (!file_remove_tree_secure(full)) + /* Remove the contents entry-by-entry through the budgeted extras + walker so every deleted file/dir counts toward --max-delete (rsync + parity); the now-empty directory itself costs one more. A run that + hits the cap leaves the remaining entries in place. */ + ArrayList* no_keeps = array_list_create(free); + size_t remaining = budget->max_delete - budget->deleted; + size_t contents_deleted = 0; + size_t contents_skipped = 0; + DeleteWalkResult walk = + no_keeps ? delete_extras_limited(full, no_keeps, NULL, remaining, NULL, 0, + &contents_deleted, &contents_skipped) + : DELETE_WALK_ERROR; + if (no_keeps) + array_list_delete(no_keeps); + budget->deleted += contents_deleted; + budget->skipped += contents_skipped; + if (walk == DELETE_WALK_LIMIT_REACHED) { + budget->limit_hit = true; + } else if (walk != DELETE_WALK_OK) { ok = false; - else + } else if (budget->deleted >= budget->max_delete) { + budget->limit_hit = true; + budget->skipped++; + } else if (file_remove_tree_secure(full)) { + budget->deleted++; removed = true; + } else { + ok = false; + } } else { char* escaped = output_escape(rel, log_get_8_bit_output()); log_message(LOG_LEVEL_WARNING, @@ -3114,6 +3160,7 @@ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest } } if (removed) { + budget->deleted++; char* escaped = output_escape(rel, log_get_8_bit_output()); fprintf(stderr, " Deleted: %s\n", escaped ? escaped : ""); free(escaped); @@ -3129,24 +3176,58 @@ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest return ok; } +/* Public wrappers used outside the commit path (and by unit tests): no + --max-delete budget. */ +bool manifest_delete_extras(const Config* config, DeleteManifest* manifest) { + DeleteBudgetState budget = { + .max_delete = SIZE_MAX, .deleted = 0, .skipped = 0, .limit_hit = false}; + return delete_extras_budgeted(config, manifest, &budget); +} + +bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest) { + DeleteBudgetState budget = { + .max_delete = SIZE_MAX, .deleted = 0, .skipped = 0, .limit_hit = false}; + return delete_missing_args_budgeted(config, manifest, &budget); +} + /* Commit every deletion family the manifest carries. The --delete-missing-args exact-path deletions run FIRST: they are explicit user requests and must not be blocked by the extras walker's filter-exclusion protection (a protected leftover inside a missing-argument directory must not make that user-requested removal fail). The ordinary extras walk then runs when --delete is active. - Returns true when there was nothing to do or every requested deletion - committed. */ -bool manifest_delete_all(const Config* config, DeleteManifest* manifest) { + Both draw from one --max-delete budget; the result reports a cap-stopped + (partial) commit distinctly so the client can exit 25 like rsync. */ +DeleteCommitResult manifest_delete_all(const Config* config, DeleteManifest* manifest) { if (!config || !manifest) - return false; + return DELETE_COMMIT_ERROR; /* Central no-mutation guard: a dry-run never deletes. No manifest is sent on the dry-run path, but a hostile/buggy peer could; treat it as a no-op so the receiver can never remove anything. */ if (config->dry_run) - return true; - if (config->delete_missing_args && !manifest_delete_missing_args(config, manifest)) - return false; - if (config->use_delete && !manifest_delete_extras(config, manifest)) - return false; - return true; + return DELETE_COMMIT_OK; + /* A client --max-delete=NUM smaller than the server's hard bound replaces it + for this run; both still bound the commit. */ + bool user_limited = + config->max_delete >= 0 && (size_t)config->max_delete < MAX_SERVER_DELETE_COUNT; + DeleteBudgetState budget = {.max_delete = user_limited ? (size_t)config->max_delete + : MAX_SERVER_DELETE_COUNT, + .deleted = 0, + .skipped = 0, + .limit_hit = false}; + if (config->delete_missing_args && !delete_missing_args_budgeted(config, manifest, &budget)) + return DELETE_COMMIT_ERROR; + if (config->use_delete && !delete_extras_budgeted(config, manifest, &budget)) + return DELETE_COMMIT_ERROR; + if (budget.limit_hit) { + if (user_limited) { + log_message(LOG_LEVEL_ERROR, "Deletions stopped due to --max-delete limit (%zu skipped)", + budget.skipped); + } else { + log_message(LOG_LEVEL_ERROR, + "Deletions stopped due to the server deletion limit of %u (%zu skipped)", + (unsigned)MAX_SERVER_DELETE_COUNT, budget.skipped); + } + return DELETE_COMMIT_LIMIT_REACHED; + } + return DELETE_COMMIT_OK; } diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h index 49ec861..c88cdee 100644 --- a/src/shared/file_receive.h +++ b/src/shared/file_receive.h @@ -85,11 +85,18 @@ typedef struct DeleteManifest { ArrayList* keeps; ArrayList* protected; ArrayList* missing; + /* Destination-relative paths of the directories the sender synchronized for + this run. The extras walker only removes entries directly inside one of + these (the receive root is the "." sentinel); `--files-from` runs therefore + leave untransmitted directories and the unlisted parts of listed ones + alone, matching rsync's "delete only in synchronized directories". */ + ArrayList* dirs; } DeleteManifest; void delete_manifest_free(DeleteManifest* manifest); -/* Read a delete-manifest frame: keep count + keeps, then protected count + - protected prefixes, then missing count + missing paths (self-delimiting; the +/* Read a delete-manifest frame (protocol 2.23.0): keep count + keeps, then + protected count + protected prefixes, then missing count + missing paths, + then synchronized-directory count + directory paths (self-delimiting; the leading STATUS_MANIFEST code has been consumed). Returns an owned DeleteManifest, or NULL after signalling STATUS_ERROR on a malformed frame. */ DeleteManifest* receive_manifest_entries(int fd); @@ -108,11 +115,22 @@ bool manifest_delete_extras(const Config* config, DeleteManifest* manifest); confinement or I/O error (the run then fails); tolerated per-path cases are reported and skipped. */ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest); +/* Outcome of committing a delete manifest. LIMIT_REACHED reports rsync's + partial --max-delete result: the budget allowed some deletions and the rest + were skipped (the run still stores all file data but the client exits 25). */ +typedef enum { + DELETE_COMMIT_OK = 0, + DELETE_COMMIT_LIMIT_REACHED, + DELETE_COMMIT_ERROR +} DeleteCommitResult; + /* Run every deletion family the manifest carries: the --delete-missing-args exact-path deletions first (user requests are not blocked by exclusion - protection), then the ordinary extras walk when --delete is active. Returns - true when nothing to do or everything committed. */ -bool manifest_delete_all(const Config* config, DeleteManifest* manifest); + protection), then the ordinary extras walk when --delete is active. Both + share one --max-delete budget. Returns DELETE_COMMIT_OK when nothing was to + do or everything committed, DELETE_COMMIT_LIMIT_REACHED when the budget + stopped part of the work, or DELETE_COMMIT_ERROR on a genuine failure. */ +DeleteCommitResult manifest_delete_all(const Config* config, DeleteManifest* manifest); /* Outcome of a single file_save_to_disk operation. The receiver needs to distinguish "written" from "skipped" so --remove-source-files can be told diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 2155837..8c752aa 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -30,6 +30,8 @@ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* que context->max_queue_bytes = 0; context->manifest = NULL; context->excluded_paths = NULL; + context->size_skipped_paths = NULL; + context->synced_dirs = NULL; context->missing_args = NULL; context->scan_had_io_error = false; context->remove_source_files = NULL; @@ -44,6 +46,7 @@ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* que protocol_session_set_max_alloc(&context->allocation_session, config->max_alloc); context->dir_entries = NULL; context->dir_entries_mutex_init = false; + context->delete_limit = false; int init = 0; if (config->use_metadata) { context->dir_entries = array_list_create(file_destroy); @@ -186,6 +189,10 @@ void pipeline_context_sender_destroy(PipelineContextSender* context) { } if (context->excluded_paths) array_list_delete(context->excluded_paths); + if (context->size_skipped_paths) + array_list_delete(context->size_skipped_paths); + if (context->synced_dirs) + array_list_delete(context->synced_dirs); if (context->missing_args) array_list_delete(context->missing_args); if (context->remove_source_files) diff --git a/src/shared/multiprocessing.h b/src/shared/multiprocessing.h index 4bf1fac..f8b475f 100644 --- a/src/shared/multiprocessing.h +++ b/src/shared/multiprocessing.h @@ -42,6 +42,18 @@ typedef struct { scanner's exclusion sink) or, in the early modes, by the path-only pre-scan on the calling thread before the pipeline starts. */ ArrayList* excluded_paths; + /* --max-size/--min-size pruned source paths. These are ALWAYS sent as + protected prefixes (even with --delete-excluded), so the destination + mirrors of size-skipped files survive --delete like rsync. Populated by + the scanner thread (workers append under mutex_scanner) or, in the early + modes, by the path-only pre-scan on the calling thread. */ + ArrayList* size_skipped_paths; + /* Destination-relative paths of the directories the source scan synchronized + for this run (the receive root is the "." sentinel). Sent with the + manifest so the receiver confines its extras walk to them, matching rsync's + "delete only in synchronized directories" (notably for --files-from). + Populated by the scanner thread or the early pre-scan. */ + ArrayList* synced_dirs; /* --delete-missing-args: the destination-relative mirrors of the --files-from entries that are missing under the source. Computed by the preflight on the calling thread before the pipeline starts; the sender thread transmits @@ -83,6 +95,10 @@ typedef struct { ArrayList* dir_entries; mtx_t dir_entries_mutex; bool dir_entries_mutex_init; + /* Set by the sender thread when the receiver reported a --max-delete-capped + deletion (STATUS_DELETE_LIMIT): the transfer succeeded and the process must + exit 25 like rsync. Read by the caller after the sender thread is joined. */ + bool delete_limit; } PipelineContextSender; /* `config` is borrowed and must outlive the context: destroy does NOT free it, diff --git a/src/shared/protocol.h b/src/shared/protocol.h index 4c2491d..81aa58a 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -155,7 +155,15 @@ enum NET_STATUS { * (the receiver reads none in dry-run). STATUS_OK keeps its meaning in this * path ("already up to date / nothing to do"). Appended after * STATUS_ERROR_DETAIL so no existing status is renumbered. */ - STATUS_DRY_RUN_TRANSFER + STATUS_DRY_RUN_TRANSFER, + /* --max-delete budget exhausted (protocol 2.23.0). Sent by the receiver as + * the terminal success status INSTEAD of STATUS_OK when a --delete/ + * --delete-missing-args commit removed up to the --max-delete bound but had + * to skip further extras. The transfer itself succeeded and all file data is + * stored; the sender maps this to rsync's exit code 25 ("the --max-delete + * limit stopped deletions"). Appended after STATUS_DRY_RUN_TRANSFER so no + * existing status is renumbered. */ + STATUS_DELETE_LIMIT }; void io_set_fds(int read_fd, int write_fd); diff --git a/src/shared/utils.c b/src/shared/utils.c index 07b3d45..00704d0 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -584,22 +584,41 @@ bool path_under_skip_prefix(const char* child_rel, bool at_root, const DeleteSki return false; } -/* All-or-nothing max-delete needs to know BEFORE any unlink whether the run - would delete more than max_delete entries. This rehearsal pass walks the - destination with the same decisions as the delete pass but never touches the - filesystem: it counts every regular file the delete pass would unlink and - every directory it would rmdir (a directory is removed only once every entry - below it has been removed and nothing the walker leaves in place survives). - Entries the walker never removes (symlinks, manifest-listed files, protected - prefixes) mark the enclosing directory as surviving, exactly as they would - make a real rmdir fail with ENOTEMPTY. Stops early once *count reaches the - cap (sets *exceeds). Returns false on a traversal error. */ -static bool count_extras_fd(int dirfd, const char* rel_path, const PathIndex* keep, size_t cap, - size_t* count, bool* exceeds, const DeleteSkipEntry* skips, - int skip_count, bool* survives) { +/* Per-run deletion budget and tallies. `max_delete` is the cap on the number + of entries the walker may remove (SIZE_MAX = unlimited); once it is reached + the remaining extras are counted in `skipped` and left in place, matching + rsync's partial --max-delete behavior. */ +typedef struct { + size_t max_delete; + size_t deleted; + size_t skipped; + bool limit_hit; +} DeleteBudget; + +/* True when direct children of the directory named by `rel` may be removed. + With no synchronization info (dirs == NULL) the whole tree is deletable; when + a dirs index is supplied only its exact entries are (the receive root is the + "." sentinel). */ +static bool is_synced_dir(const PathIndex* dirs, const char* rel) { + if (!dirs) + return true; + return path_index_contains(dirs, rel[0] == '\0' ? "." : rel); +} + +/* Remove the extras directly inside the directory open on `dirfd`, recursing + into every child directory so kept content below a synchronized prefix is + reached. `all_removed` reports whether every child entry was removed (so the + caller may rmdir this directory). A child directory is never removed when it + is itself a synchronized directory or holds kept content; with a dirs index + supplied, direct children of a non-synchronized directory are never extras at + all (they are left in place but still descended into). Symlinks are unlinked + like any other non-directory extra (never followed). */ +static bool delete_extras_fd(int dirfd, const char* rel_path, const PathIndex* keep, + const PathIndex* dirs, DeleteBudget* budget, + const DeleteSkipEntry* skips, int skip_count, bool parent_deletable, + bool* all_removed) { /* openat(dirfd, ".") opens an independent file description: a dup() would - share dirfd's file offset, and a prior rehearsal pass must not have drained - this directory's stream before the delete pass reads it again. */ + share dirfd's file offset and a prior pass could leave the stream drained. */ int scanfd = openat(dirfd, ".", O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); if (scanfd < 0) return false; @@ -610,93 +629,10 @@ static bool count_extras_fd(int dirfd, const char* rel_path, const PathIndex* ke } bool operation_ok = true; bool local_survives = false; - bool at_root = rel_path[0] == '\0'; - const struct dirent* entry; - while ((entry = readdir(dir)) != NULL) { - if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) - continue; - if (*exceeds) - break; - char* child_rel = path_cat((char*)rel_path, entry->d_name); - if (!child_rel) { - operation_ok = false; - continue; - } - if (path_under_skip_prefix(child_rel, at_root, skips, skip_count)) { - local_survives = true; - free(child_rel); - continue; - } - struct stat st; - if (fstatat(dirfd, entry->d_name, &st, AT_SYMLINK_NOFOLLOW) != 0) { - if (errno != ENOENT) - operation_ok = false; - free(child_rel); - continue; - } - if (S_ISLNK(st.st_mode)) { - local_survives = true; - free(child_rel); - continue; - } - if (S_ISDIR(st.st_mode)) { - int childfd = openat(dirfd, entry->d_name, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - bool child_ok = true; - bool child_survives = true; - if (childfd >= 0) { - child_ok = count_extras_fd(childfd, child_rel, keep, cap, count, exceeds, skips, skip_count, - &child_survives); - close(childfd); - } else if (errno != ENOENT) { - operation_ok = false; - } - if (!child_ok) - operation_ok = false; - if (keep_is_dir(keep, child_rel)) { - /* A directory with kept content below it is never removed. */ - local_survives = true; - } else if (child_survives) { - /* The directory still holds entries the walker leaves in place, so an - rmdir would fail with ENOTEMPTY; the delete pass leaves it behind - rather than reporting an error (matching rsync). */ - local_survives = true; - } else { - if (*count >= cap) { - *exceeds = true; - } else { - (*count)++; - } - } - } else { - bool found = keep_is_file(keep, child_rel); - if (!found) { - if (*count >= cap) { - *exceeds = true; - } else { - (*count)++; - } - } - } - free(child_rel); - } - closedir(dir); - *survives = local_survives; - return operation_ok; -} - -static bool delete_extras_fd(int dirfd, const char* rel_path, const PathIndex* keep, - size_t max_delete, size_t* deleted_count, const DeleteSkipEntry* skips, - int skip_count) { - /* Independent file description (see count_extras_fd). */ - int scanfd = openat(dirfd, ".", O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - if (scanfd < 0) - return false; - DIR* dir = fdopendir(scanfd); - if (!dir) { - close(scanfd); - return false; - } - bool operation_ok = true; + /* A directory is deletable when it or ANY ancestor is synchronized; the + `parent_deletable` flag carries that down the recursion so dest-only + directories below a synchronized root are removed wholesale. */ + bool deletable = parent_deletable || is_synced_dir(dirs, rel_path); const struct dirent* entry; while ((entry = readdir(dir)) != NULL) { if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) @@ -714,6 +650,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, const PathIndex* k destination directory that happens to be called .fastsync-stage is ordinary content. */ if (path_under_skip_prefix(child_rel, rel_path[0] == '\0', skips, skip_count)) { + local_survives = true; free(child_rel); continue; } @@ -724,55 +661,58 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, const PathIndex* k free(child_rel); continue; } - // Skip symlinks to prevent following them outside the destination tree - if (S_ISLNK(st.st_mode)) { - free(child_rel); - continue; - } if (S_ISDIR(st.st_mode)) { int childfd = openat(dirfd, entry->d_name, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - bool child_removed = false; + bool child_all_removed = false; if (childfd >= 0) { - child_removed = delete_extras_fd(childfd, child_rel, keep, max_delete, deleted_count, skips, - skip_count); - if (!child_removed) + if (!delete_extras_fd(childfd, child_rel, keep, dirs, budget, skips, skip_count, deletable, + &child_all_removed)) operation_ok = false; close(childfd); } else if (errno != ENOENT) { operation_ok = false; } - if (child_removed && !keep_is_dir(keep, child_rel)) { - if (*deleted_count >= max_delete) { - operation_ok = false; + bool child_synced = dirs && path_index_contains(dirs, child_rel); + if (child_synced || keep_is_dir(keep, child_rel)) { + /* A synchronized directory and a directory holding kept content are + never removed. */ + local_survives = true; + } else if (child_all_removed && deletable) { + if (budget->deleted >= budget->max_delete) { + budget->limit_hit = true; + budget->skipped++; + local_survives = true; + } else if (unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0) { + /* ENOENT: already gone (fine). ENOTEMPTY/EEXIST: the directory + still holds entries the walker leaves in place (a protected + excluded prefix, a kept file the manifest protects, a symlink); + rsync leaves such a directory behind, so this is not an error. + Only genuine I/O failures abort the deletion. */ + if (errno != ENOENT && errno != ENOTEMPTY && errno != EEXIST) + operation_ok = false; + local_survives = true; } else { - if (unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0) { - /* ENOENT: already gone (fine). ENOTEMPTY/EEXIST: the directory - still holds entries the walker leaves in place (a protected - excluded prefix, a kept file the manifest protects, a symlink); - rsync leaves such a directory behind, so this is not an error. - Only genuine I/O failures abort the deletion. */ - if (errno != ENOENT && errno != ENOTEMPTY && errno != EEXIST) - operation_ok = false; - } else { - (*deleted_count)++; - } + budget->deleted++; } + } else { + local_survives = true; } } else { - // Check if relative path is in manifest bool found = keep_is_file(keep, child_rel); - if (!found) { - if (*deleted_count >= max_delete) { + if (found || !deletable) { + /* Kept file, or a child of a directory that is not synchronized: never + an extra for this run. */ + local_survives = true; + } else if (budget->deleted >= budget->max_delete) { + budget->limit_hit = true; + budget->skipped++; + local_survives = true; + } else if (unlinkat(dirfd, entry->d_name, 0) != 0) { + if (errno != ENOENT) operation_ok = false; - free(child_rel); - continue; - } - if (unlinkat(dirfd, entry->d_name, 0) != 0) { - if (errno != ENOENT) - operation_ok = false; - } else { - (*deleted_count)++; - } + local_survives = true; + } else { + budget->deleted++; char* escaped_path = output_escape(child_rel, log_get_8_bit_output()); fprintf(stderr, " Deleted: %s\n", escaped_path ? escaped_path : ""); free(escaped_path); @@ -781,21 +721,33 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, const PathIndex* k free(child_rel); } closedir(dir); + *all_removed = !local_survives; return operation_ok; } DeleteWalkResult delete_extras_limited(const char* dest_root, const ArrayList* manifest, - size_t max_delete, const DeleteSkipEntry* skips, - int skip_count, size_t* deleted_out) { + const ArrayList* synced_dirs, size_t max_delete, + const DeleteSkipEntry* skips, int skip_count, + size_t* deleted_out, size_t* skipped_out) { if (deleted_out) *deleted_out = 0; + if (skipped_out) + *skipped_out = 0; if (!manifest) return DELETE_WALK_ERROR; - /* Index the keep-set once so both passes answer membership in O(path length) - instead of scanning every manifest entry for every destination entry. */ + /* Index the keep-set (and the synchronized-dir set, when supplied) once so + membership is answered in O(path length) instead of scanning every entry + for every destination entry. */ PathIndex keep; if (!build_keep_index(manifest, &keep)) return DELETE_WALK_ERROR; + PathIndex dirs; + bool have_dirs = synced_dirs != NULL; + if (have_dirs && + !path_index_build(&dirs, (const char* const*)synced_dirs->items, (size_t)synced_dirs->size)) { + path_index_free(&keep); + return DELETE_WALK_ERROR; + } int rootfd; int root_fd = utils_get_authorized_root_fd(); if (root_fd >= 0) { @@ -810,39 +762,31 @@ DeleteWalkResult delete_extras_limited(const char* dest_root, const ArrayList* m } if (rootfd < 0) { path_index_free(&keep); + if (have_dirs) + path_index_free(&dirs); return DELETE_WALK_ERROR; } - if (max_delete != SIZE_MAX) { - /* Rehearse the deletion first so a run that would exceed the cap removes - nothing (rsync's all-or-nothing --max-delete contract). */ - size_t count = 0; - bool exceeds = false; - bool survives = false; - bool counted_ok = count_extras_fd(rootfd, "", &keep, max_delete, &count, &exceeds, skips, - skip_count, &survives); - if (!counted_ok) { - close(rootfd); - path_index_free(&keep); - return DELETE_WALK_ERROR; - } - if (exceeds) { - close(rootfd); - path_index_free(&keep); - return DELETE_WALK_LIMIT_EXCEEDED; - } - } - size_t deleted_count = 0; - bool ok = delete_extras_fd(rootfd, "", &keep, max_delete, &deleted_count, skips, skip_count); + DeleteBudget budget = {.max_delete = max_delete, .deleted = 0, .skipped = 0, .limit_hit = false}; + bool all_removed = false; + bool ok = delete_extras_fd(rootfd, "", &keep, have_dirs ? &dirs : NULL, &budget, skips, + skip_count, false, &all_removed); if (close(rootfd) != 0) ok = false; path_index_free(&keep); + if (have_dirs) + path_index_free(&dirs); if (deleted_out) - *deleted_out = deleted_count; - return ok ? DELETE_WALK_OK : DELETE_WALK_ERROR; + *deleted_out = budget.deleted; + if (skipped_out) + *skipped_out = budget.skipped; + if (!ok) + return DELETE_WALK_ERROR; + return budget.limit_hit ? DELETE_WALK_LIMIT_REACHED : DELETE_WALK_OK; } bool delete_extras(const char* dest_root, const ArrayList* manifest) { - return delete_extras_limited(dest_root, manifest, SIZE_MAX, NULL, 0, NULL) == DELETE_WALK_OK; + return delete_extras_limited(dest_root, manifest, NULL, SIZE_MAX, NULL, 0, NULL, NULL) == + DELETE_WALK_OK; } bool has_path_traversal(const char* path) { diff --git a/src/shared/utils.h b/src/shared/utils.h index cda0cd8..0cca144 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -98,10 +98,10 @@ bool glob_match(const char* pattern, const char* str); typedef enum { /* Every extra entry was removed (or there were none). */ DELETE_WALK_OK = 0, - /* The destination holds more extras than the numeric cap for this run. With - the all-or-nothing max-delete semantics NOTHING was removed (the walker - counts first and refuses to start when the run would exceed the limit). */ - DELETE_WALK_LIMIT_EXCEEDED, + /* The numeric cap for this run was reached before every extra was removed. + The walker removed exactly the entries the cap allowed and skipped (without + removing) the rest, matching rsync's partial --max-delete behavior. */ + DELETE_WALK_LIMIT_REACHED, /* A traversal or unlink failure aborted the deletion (partial removal is possible, mirroring the delete pass). */ DELETE_WALK_ERROR @@ -122,19 +122,22 @@ typedef struct { only DIRECT children of the destination root, i.e. child_rel has no '/'). */ bool path_under_skip_prefix(const char* child_rel, bool at_root, const DeleteSkipEntry* skips, int skip_count); -/* Remove files/dirs under dest_root that are not listed in manifest without - ever descending into a protected prefix (see DeleteSkipEntry). When - max_delete is not SIZE_MAX the run is all-or-nothing: extras are counted - first and DELETE_WALK_LIMIT_EXCEEDED is returned (with nothing removed) when - the count would exceed the cap. `deleted_out` optionally receives the number - of entries actually removed. The all-or-nothing guarantee holds only while - the destination tree is not being concurrently modified: the rehearsal pass - and the delete pass are two separate walks, so a concurrent change between - them (another process adding/removing entries) can make the second pass - delete a different set than the first one counted. */ +/* Remove files/dirs/symlinks under dest_root that are not listed in manifest + without ever descending into a protected prefix (see DeleteSkipEntry). When + `synced_dirs` is non-NULL, extras are only removed directly inside a directory + whose destination-relative path is an exact entry in that list (the receive + root is the "." sentinel); directories outside the synchronized set are still + descended into so kept content below a listed directory is preserved, but + nothing in them is removed. A NULL `synced_dirs` keeps the legacy behavior of + treating the whole destination tree as deletable. `max_delete` caps the + number of removed entries (SIZE_MAX = unlimited): the walker removes up to the + cap and returns DELETE_WALK_LIMIT_REACHED when more extras remained. + `deleted_out`/`skipped_out` optionally receive the number of entries removed + and the number skipped because of the cap. */ DeleteWalkResult delete_extras_limited(const char* dest_root, const ArrayList* manifest, - size_t max_delete, const DeleteSkipEntry* skips, - int skip_count, size_t* deleted_out); + const ArrayList* synced_dirs, size_t max_delete, + const DeleteSkipEntry* skips, int skip_count, + size_t* deleted_out, size_t* skipped_out); bool delete_extras(const char* dest_root, const ArrayList* manifest); bool utils_set_authorized_root(int fd, const char* canonical_path); /* The fd-only compatibility form is fail-closed for path-based operations; diff --git a/tests/integration/test_fault_injection.py b/tests/integration/test_fault_injection.py index df2f607..7c7127f 100644 --- a/tests/integration/test_fault_injection.py +++ b/tests/integration/test_fault_injection.py @@ -36,7 +36,7 @@ from common import ( # noqa: E402 verify_transfer, ) -PROTOCOL_VERSION = b"2.22.0" +PROTOCOL_VERSION = b"2.23.0" STATUS_MANIFEST = 5 STATUS_OK = 0 diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index 9ca3681..a975fce 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -2854,8 +2854,9 @@ class TestRelativeFilesFrom: "bare relative layout must not appear without -R" def test_relative_delete_manifest_stays_consistent(self): - """--delete derives from the sent (-R) relative paths, so a later - subset run removes unlisted relative entries but keeps listed ones.""" + """--delete with --files-from is confined to the synchronized directories + (rsync parity): listing a FILE does not make its parent a delete scope, + but listing the DIRECTORY does.""" source = _make_relative_source("rel_del_src") dest = os.path.join(TEST_DATA_DIR, "rel_del_dst") clean_dir(dest) @@ -2867,14 +2868,27 @@ class TestRelativeFilesFrom: assert result.returncode == 0, f"seed -R sync failed: {result.stderr[:200]}" assert os.path.isfile(os.path.join(dest, "sub", "y.txt")) + # A file-only listing leaves sub/ unsynchronized: y.txt survives. subset = _write_rel_list(b"sub/x.txt\n") result, _ = run_client(source, dest, flags=["--files-from", subset, "-R", "--delete"], port=server.port) assert result.returncode == 0, f"-R delete sync failed: {result.stderr[:200]}" assert os.path.isfile(os.path.join(dest, "sub", "x.txt")), "listed file was deleted" + assert os.path.exists(os.path.join(dest, "sub", "y.txt")), \ + "file-only --files-from made the parent a delete scope (rsync keeps it)" + + # Listing the directory synchronizes it: a source-removed y.txt is now + # an in-scope extra and is deleted. + os.unlink(os.path.join(source, "sub", "y.txt")) + listed_dir = _write_rel_list(b"sub/\n") + result, _ = run_client(source, dest, + flags=["--files-from", listed_dir, "-R", "--delete"], + port=server.port) + assert result.returncode == 0, f"-R dir delete sync failed: {result.stderr[:200]}" + assert os.path.isfile(os.path.join(dest, "sub", "x.txt")) assert not os.path.exists(os.path.join(dest, "sub", "y.txt")), \ - "unlisted relative file was not deleted" + "directory-listed --delete did not remove the in-scope extra" class TestMissingArgs: @@ -2991,14 +3005,15 @@ class TestMissingArgs: assert os.path.isfile(os.path.join(dest, "a.txt")) assert os.path.isfile(os.path.join(dest, "sub", "b.txt")) - # Now with --delete the unrelated extra is an ordinary extra and must go. + # --delete is confined to synchronized directories: no listed + # directory, so the root-level unrelated extra survives (rsync parity). lst2 = _write_rel_list(b"a.txt\ngone.txt\nsub/b.txt\n") flags2 = ["--files-from", lst2, "-R", "--delete-missing-args", "--delete"] + \ (["--threads"] if mt else []) result, _ = run_client(source, dest, flags=flags2, port=server.port) assert result.returncode == 0, f"delete-missing + delete sync failed: {result.stderr[:300]}" - assert not os.path.exists(os.path.join(dest, "unrelated.txt")), \ - "--delete did not remove the unrelated extra" + assert os.path.isfile(os.path.join(dest, "unrelated.txt")), \ + "--delete under --files-from removed an extra outside a listed directory" assert not os.path.exists(os.path.join(dest, "gone.txt")) assert os.path.isfile(os.path.join(dest, "a.txt")) @@ -3029,6 +3044,64 @@ class TestMissingArgs: "the full-source-mirror path of the missing entry was not deleted" assert os.path.isfile(os.path.join(received, "a.txt")) + @pytest.mark.parametrize("mt", [False, True]) + def test_delete_missing_args_respects_max_delete_budget(self, mt): + """#290 (5): --delete-missing-args deletions draw from the same + --max-delete budget as the ordinary extras walk: only the first N happen + and the run exits 25 like rsync.""" + source = self._make_source("mg_budget_src") + dest = os.path.join(TEST_DATA_DIR, "mg_budget_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 failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + for name in ("gone1.txt", "gone2.txt", "gone3.txt"): + with open(os.path.join(received, name), "w") as fh: + fh.write("stale") + lst = _write_rel_list(b"a.txt\ngone1.txt\ngone2.txt\ngone3.txt\n") + flags = ["--files-from", lst, "--delete-missing-args", "--max-delete=2"] + \ + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 25, \ + f"--delete-missing-args --max-delete=2 should exit 25: {result.stderr[:300]}" + remaining = [n for n in ("gone1.txt", "gone2.txt", "gone3.txt") + if os.path.exists(os.path.join(received, n))] + assert len(remaining) == 1, \ + f"missing-args deletions ignored the --max-delete budget: {remaining}" + assert os.path.isfile(os.path.join(received, "a.txt")) + + @pytest.mark.parametrize("mt", [False, True]) + def test_delete_missing_nonempty_dir_counts_each_entry_against_budget(self, mt): + """A non-empty missing-arg directory with --force/--delete is removed + entry-by-entry, each counting toward --max-delete (rsync parity): with a + small cap the run stops after N files and leaves the rest in place.""" + source = self._make_source("mg_dirbudget_src") + dest = os.path.join(TEST_DATA_DIR, "mg_dirbudget_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 failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + gone = os.path.join(received, "gone") + os.makedirs(gone) + for i in range(4): + with open(os.path.join(gone, f"f{i}"), "w") as fh: + fh.write("stale") + lst = _write_rel_list(b"a.txt\ngone\n") + flags = ["--files-from", lst, "--delete-missing-args", "--force", + "--max-delete=2"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 25, \ + f"non-empty missing-arg dir should cap at 2 and exit 25: {result.stderr[:300]}" + assert os.path.isdir(gone), \ + "the non-empty missing-arg directory should survive a capped run" + remaining = len(os.listdir(gone)) + assert remaining == 2, f"expected 2 entries left, found {remaining}" + assert os.path.isfile(os.path.join(received, "a.txt")) + @pytest.mark.parametrize("mt", [False, True]) def test_delete_missing_args_not_blocked_by_exclude_protection(self, mt): """A missing-arg mirror that sits under a filter-excluded directory is an @@ -3061,8 +3134,10 @@ class TestMissingArgs: "the explicit missing-arg deletion was blocked by exclusion protection" assert os.path.isfile(os.path.join(received, "prot", "kept.txt")), \ "the excluded-but-present destination file must stay (default protection)" - assert not os.path.exists(os.path.join(received, "extra.txt")), \ - "--delete did not remove the unrelated extra" + # A file-only --files-from listing synchronizes no directory, so the + # root-level extra is outside the delete scope (rsync parity). + assert os.path.isfile(os.path.join(received, "extra.txt")), \ + "--delete under --files-from removed an extra outside a listed directory" assert os.path.isfile(os.path.join(received, "a.txt")) @pytest.mark.parametrize("mt", [False, True]) @@ -3087,8 +3162,10 @@ class TestMissingArgs: assert not os.path.exists(os.path.join(dest, "gone.txt")), \ "early timing did not remove the missing-arg mirror" assert os.path.isfile(os.path.join(dest, "a.txt")), "a.txt was not transferred" - assert not os.path.exists(os.path.join(dest, "extra.txt")), \ - "--delete-before implies --delete: unrelated extras must go" + # --delete-before implies --delete, but the extras walk is still + # confined to synchronized directories: no listed directory here. + assert os.path.isfile(os.path.join(dest, "extra.txt")), \ + "--delete-before under --files-from removed an extra outside a listed directory" @pytest.mark.parametrize("mt", [False, True]) @pytest.mark.parametrize("relative", [False, True]) @@ -3098,6 +3175,12 @@ class TestMissingArgs: exact-path deletions must not abort the --delete extras walk. Covers the -R bare-relative layout and the full source-mirror layout.""" source = self._make_source("mg_deep_src") + # A listed directory gives the extras walk a synchronized scope to work + # in, so the test can prove the absent-parent missing entry did not abort + # it. + os.makedirs(os.path.join(source, "scope")) + with open(os.path.join(source, "scope", "keep.txt"), "w") as fh: + fh.write("kept\n") dest = os.path.join(TEST_DATA_DIR, "mg_deep_dst") clean_dir(dest) rel_flags = ["-R"] if relative else [] @@ -3117,16 +3200,22 @@ class TestMissingArgs: assert os.path.isfile(os.path.join(target_root, "a.txt")) with open(os.path.join(target_root, "extra.txt"), "w") as fh: fh.write("extra") + os.makedirs(os.path.join(target_root, "scope"), exist_ok=True) + with open(os.path.join(target_root, "scope", "extra.txt"), "w") as fh: + fh.write("extra") - lst = _write_rel_list(b"a.txt\nsub/gone.txt\n") + lst = _write_rel_list(b"a.txt\nscope/\nsub/gone.txt\n") flags = ["--files-from", lst, "--delete-missing-args", "--delete"] + rel_flags + \ (["--threads"] if mt else []) result, _ = run_client(source, dest, flags=flags, port=server.port) assert result.returncode == 0, \ f"deep missing-entry sync failed: {result.stderr[:300]}" assert _read_file(os.path.join(target_root, "a.txt")) == b"a\n" - assert not os.path.exists(os.path.join(target_root, "extra.txt")), \ + assert _read_file(os.path.join(target_root, "scope", "keep.txt")) == b"kept\n" + assert not os.path.exists(os.path.join(target_root, "scope", "extra.txt")), \ "--delete extras walk was aborted by the absent-parent missing entry" + # The root-level extra is outside every listed directory: it survives. + assert os.path.exists(os.path.join(target_root, "extra.txt")) assert not os.path.exists(os.path.join(target_root, "sub")), \ "the absent parent directory of the missing entry was created" @@ -3535,6 +3624,154 @@ def _seed_delete_tree(tag, entries, dest): return source, received +class TestDeleteScope: + """#290 (1): --delete with --files-from is confined to the directories the + transfer synchronized (rsync parity), so untransmitted paths outside a + listed directory subtree are never deleted. Data-loss capable.""" + + def _write(self, path, content): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "wb") as fh: + fh.write(content) + + def _seed(self, tag): + source = os.path.join(TEST_DATA_DIR, f"dscope_{tag}_src") + clean_dir(source) + for rel, content in { + "listed.txt": b"listed\n", + "unlisted.txt": b"unlisted\n", + "other/c.txt": b"c\n", + "sub/x.txt": b"x\n", + "sub/y.txt": b"y\n", + }.items(): + self._write(os.path.join(source, rel), content) + dest = os.path.join(TEST_DATA_DIR, f"dscope_{tag}_dst") + clean_dir(dest) + server = ServerManager() + server.start(extra_args=["--allow-delete"]) + result, _ = run_client(source, dest, port=server.port) + assert result.returncode == 0, f"seed failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + self._write(os.path.join(received, "sub", "extra.txt"), b"in-scope extra\n") + self._write(os.path.join(received, "rootextra.txt"), b"root extra\n") + self._write(os.path.join(received, "other", "extra.txt"), b"other extra\n") + return source, dest, received, server + + @pytest.mark.parametrize("mt", [False, True]) + @pytest.mark.ci + def test_files_from_delete_confined_to_listed_dirs(self, mt): + source, dest, received, server = self._seed(f"dir_{mt}") + try: + listed = _write_rel_list(b"listed.txt\nsub/\n") + flags = ["--files-from", listed, "--delete"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, f"delete failed: {result.stderr[:300]}" + assert not os.path.exists(os.path.join(received, "sub", "extra.txt")), \ + "in-scope extra under a listed directory was not deleted" + assert os.path.isfile(os.path.join(received, "sub", "x.txt")) + assert os.path.exists(os.path.join(received, "unlisted.txt")), \ + "unlisted path outside a listed directory was deleted (data loss)" + assert os.path.exists(os.path.join(received, "other", "c.txt")), \ + "unlisted sibling directory was deleted (data loss)" + assert os.path.exists(os.path.join(received, "rootextra.txt")), \ + "receive-root extra outside a listed directory was deleted (data loss)" + finally: + server.stop() + + @pytest.mark.parametrize("mt", [False, True]) + @pytest.mark.ci + def test_files_from_delete_file_listing_keeps_parent_extras(self, mt): + source, dest, received, server = self._seed(f"file_{mt}") + try: + # Listing a FILE does not synchronize its parent directory, so the + # parent's extras survive exactly like rsync. + listed = _write_rel_list(b"listed.txt\nsub/x.txt\n") + flags = ["--files-from", listed, "--delete"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, f"delete failed: {result.stderr[:300]}" + assert os.path.exists(os.path.join(received, "sub", "extra.txt")), \ + "a file-only --files-from made its parent a delete scope" + assert os.path.exists(os.path.join(received, "rootextra.txt")) + assert os.path.exists(os.path.join(received, "unlisted.txt")) + finally: + server.stop() + + +class TestDeleteExtraneousSymlinks: + """#290 (3): --delete unlinks extraneous destination symlinks (never follows + them), matching rsync, and leaves their targets intact.""" + + @pytest.mark.parametrize("mt", [False, True]) + @pytest.mark.ci + def test_delete_unlinks_extraneous_symlinks(self, mt): + source = os.path.join(TEST_DATA_DIR, f"dsym_{mt}_src") + clean_dir(source) + with open(os.path.join(source, "keep.txt"), "wb") as fh: + fh.write(b"kept\n") + dest = os.path.join(TEST_DATA_DIR, f"dsym_{mt}_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 failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + outside = os.path.join(TEST_DATA_DIR, f"dsym_{mt}_outside") + clean_dir(outside) + with open(os.path.join(outside, "secret.txt"), "wb") as fh: + fh.write(b"secret\n") + os.symlink("keep.txt", os.path.join(received, "link_file")) + os.symlink(outside, os.path.join(received, "link_dir")) + os.symlink("/nonexistent-target", os.path.join(received, "link_broken")) + os.makedirs(os.path.join(received, "realdir"), exist_ok=True) + os.symlink("../realdir", os.path.join(received, "realdir", "self")) + + flags = ["--delete"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, f"delete failed: {result.stderr[:300]}" + assert not os.path.lexists(os.path.join(received, "link_file")), \ + "extraneous symlink to a file was not unlinked" + assert not os.path.lexists(os.path.join(received, "link_dir")), \ + "extraneous symlink to a directory was not unlinked" + assert not os.path.lexists(os.path.join(received, "link_broken")), \ + "extraneous dangling symlink was not unlinked" + assert not os.path.lexists(os.path.join(received, "realdir", "self")), \ + "extraneous self-referential symlink was not unlinked" + assert os.path.isfile(os.path.join(received, "keep.txt")) + assert os.path.isfile(os.path.join(outside, "secret.txt")), \ + "an extraneous symlink was followed and its target deleted" + + +class TestSizePruneProtection: + """#290 (2): --max-size/--min-size pruned source mirrors survive --delete + even with --delete-excluded (rsync keeps them).""" + + @pytest.mark.parametrize("mt", [False, True]) + @pytest.mark.parametrize("flag", ["--max-size=1000", "--min-size=1000"]) + @pytest.mark.ci + def test_size_pruned_mirror_survives_delete_excluded(self, mt, flag): + source = os.path.join(TEST_DATA_DIR, f"dsize_{mt}_{flag.strip('-=')}_src") + clean_dir(source) + with open(os.path.join(source, "small.txt"), "wb") as fh: + fh.write(b"small\n") + with open(os.path.join(source, "big.bin"), "wb") as fh: + fh.write(b"0" * 5000) + dest = os.path.join(TEST_DATA_DIR, f"dsize_{mt}_{flag.strip('-=')}_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 failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + + flags = [flag, "--delete", "--delete-excluded"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, f"size delete failed: {result.stderr[:300]}" + assert os.path.isfile(os.path.join(received, "small.txt")), \ + "size-pruned small mirror was deleted under --delete-excluded" + assert os.path.isfile(os.path.join(received, "big.bin")), \ + "size-pruned big mirror was deleted under --delete-excluded" + + class TestDeletePolicy: """Deletion-policy family: --delete-excluded, --max-delete, --force, --ignore-errors and --prune-empty-dirs.""" @@ -3631,8 +3868,9 @@ class TestDeletePolicy: @pytest.mark.parametrize("mt", [False, True]) @pytest.mark.parametrize("timing", ["--delete", "--delete-before"]) - def test_max_delete_exceeded_fails_without_deleting(self, mt, timing): - """A run that would exceed --max-delete deletes nothing and fails.""" + def test_max_delete_exceeded_deletes_up_to_cap_and_exits_25(self, mt, timing): + """rsync parity: --max-delete=N deletes up to N extras, skips the rest and + still succeeds as a transfer, exiting 25 with a diagnostic.""" source = os.path.join(TEST_DATA_DIR, f"maxdel_{timing.strip('-')}_{mt}_src") clean_dir(source) self._write(os.path.join(source, "keep.txt"), b"kept\n") @@ -3643,19 +3881,51 @@ class TestDeletePolicy: 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) - extras = [] for i in range(4): - name = f"e{i}.txt" - self._write(os.path.join(received, name), b"extra\n") - extras.append(os.path.join(received, name)) + self._write(os.path.join(received, f"e{i}.txt"), b"extra\n") flags = ["--max-delete=2", timing] + (["--threads"] if mt else []) result, _ = run_client(source, dest, flags=flags, port=server.port) - assert result.returncode != 0, \ - f"--max-delete=2 with 4 extras unexpectedly succeeded: {result.stderr[:300]}" - for path in extras: - assert os.path.exists(path), \ - "--max-delete overrun deleted files (must be all-or-nothing)" + assert result.returncode == 25, \ + f"--max-delete=2 with 4 extras should exit 25: {result.stderr[:300]}" + remaining = [i for i in range(4) + if os.path.exists(os.path.join(received, f"e{i}.txt"))] + assert len(remaining) == 2, \ + f"--max-delete=2 deleted {4 - len(remaining)} extras, expected 2" + assert os.path.isfile(os.path.join(received, "keep.txt")) + assert "--max-delete" in (result.stderr or result.stdout) + + @pytest.mark.parametrize("mt", [False, True]) + def test_max_delete_zero_and_negative(self, mt): + """--max-delete=0 warns about every extra without deleting (exit 25); + a negative value is rsync's deprecated unlimited spelling (exit 0).""" + source = os.path.join(TEST_DATA_DIR, f"maxdelzn_{mt}_src") + clean_dir(source) + self._write(os.path.join(source, "keep.txt"), b"kept\n") + dest = os.path.join(TEST_DATA_DIR, f"maxdelzn_{mt}_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) + for i in range(3): + self._write(os.path.join(received, f"e{i}.txt"), b"extra\n") + flags = ["--max-delete=0", "--delete"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 25, f"--max-delete=0 should exit 25: {result.stderr[:300]}" + for i in range(3): + assert os.path.exists(os.path.join(received, f"e{i}.txt")), \ + "--max-delete=0 deleted an extra" + + # -1 (and any negative) means no client limit: every extra goes. + flags = ["--max-delete=-1", "--delete"] + (["--threads"] if mt else []) + result, _ = run_client(source, dest, flags=flags, port=server.port) + assert result.returncode == 0, \ + f"--max-delete=-1 should be unlimited: {result.stderr[:300]}" + for i in range(3): + assert not os.path.exists(os.path.join(received, f"e{i}.txt")), \ + "--max-delete=-1 did not remove every extra" @pytest.mark.parametrize("mt", [False, True]) def test_max_delete_not_exceeded_deletes_exactly(self, mt): @@ -3718,16 +3988,16 @@ class TestDeletePolicy: "--force did not replace the directory with the file" assert _read_file(os.path.join(received, "sub")) == b"now a file\n" - 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") + @pytest.mark.parametrize("mt", [False, True]) + def test_force_replaces_dir_under_delay_updates(self, mt): + """rsync parity: --force also acts during a --delay-updates publication, + clearing a non-empty destination directory that blocks an incoming file + (without --force the run fails and the directory survives).""" + source = os.path.join(TEST_DATA_DIR, f"force_delay_{mt}_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") + dest = os.path.join(TEST_DATA_DIR, f"force_delay_{mt}_dst") clean_dir(dest) with ServerManager() as server: server.start(extra_args=["--allow-delete"]) @@ -3737,14 +4007,24 @@ class TestDeletePolicy: 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"], + + # Without --force the blocking directory is untouched and the run fails. + result, _ = run_client(source, dest, flags=["--delay-updates"] + (["--threads"] if mt else []), 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" + "--delay-updates replaced a non-empty directory without --force" + assert os.path.isdir(os.path.join(received, "sub")) + assert os.path.exists(os.path.join(received, "sub", "old.txt")) + + # With --force the publication clears it and installs the file. + result, _ = run_client(source, dest, + flags=["--force", "--delay-updates"] + (["--threads"] if mt else []), + port=server.port) + assert result.returncode == 0, \ + f"--force --delay-updates failed: {result.stderr[:300]}" + assert os.path.isfile(os.path.join(received, "sub")), \ + "--force under --delay-updates did not replace the blocking directory" + assert _read_file(os.path.join(received, "sub")) == b"now a file\n" @pytest.mark.parametrize("mt", [False, True]) def test_prune_empty_dirs_dirs_mode(self, mt): diff --git a/tests/integration/test_preflight.py b/tests/integration/test_preflight.py index f72af11..745e796 100644 --- a/tests/integration/test_preflight.py +++ b/tests/integration/test_preflight.py @@ -94,14 +94,14 @@ def _seed_protocol_source(source): class TestProtocol: @pytest.mark.ci def test_protocol_current_version_accepted(self, shared_server): - """--protocol=2.22.0 (the current PROTOCOL_VERSION) is accepted and the + """--protocol=2.23.0 (the current PROTOCOL_VERSION) is accepted and the transfer completes normally.""" source = os.path.join(TEST_DATA_DIR, "proto_ok_src") dest = os.path.join(TEST_DATA_DIR, "proto_ok_dst") shutil.rmtree(dest, ignore_errors=True) os.makedirs(dest) _seed_protocol_source(source) - result, _ = run_client(source, dest, flags=["--protocol=2.22.0"], + result, _ = run_client(source, dest, flags=["--protocol=2.23.0"], port=shared_server.port) assert result.returncode == 0, \ f"--protocol current run failed: {(result.stderr or result.stdout)[:400]}" diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 506068a..824a43d 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -317,7 +317,7 @@ static void test_parse_args_protocol_accept_current() { Config* cfg = valid_client_config(); EXPECT_NOT_NULL(cfg); char* argv_equals[] = {"fastsync", "--source-dir", "/src", - "--dest-dir", "/dst", "--protocol=2.22.0"}; + "--dest-dir", "/dst", "--protocol=2.23.0"}; int positional_args[2]; int positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 6, argv_equals, positional_args, &positional_count), 0); @@ -327,7 +327,7 @@ static void test_parse_args_protocol_accept_current() { cfg = valid_client_config(); EXPECT_NOT_NULL(cfg); char* argv_space[] = {"fastsync", "--source-dir", "/src", "--dest-dir", - "/dst", "--protocol", "2.22.0"}; + "/dst", "--protocol", "2.23.0"}; positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 7, argv_space, positional_args, &positional_count), 0); EXPECT_EQ_STR(cfg->version, PROTOCOL_VERSION); @@ -2602,10 +2602,13 @@ static void test_parse_args_delete_policy_invalid_values() { EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), -1); config_delete(cfg); + /* A negative --max-delete is rsync's deprecated "no client limit" spelling: + parse succeeds and every negative value clamps to -1. */ cfg = config_create(); char* argv2[] = {"fastsync", "--max-delete=-3", "/src", "/dst"}; positional_count = 0; - EXPECT_EQ_INT(parse_args(cfg, 4, argv2, positional_args, &positional_count), -1); + EXPECT_EQ_INT(parse_args(cfg, 4, argv2, positional_args, &positional_count), 0); + EXPECT_EQ_INT(cfg->max_delete, -1); config_delete(cfg); } diff --git a/tests/test_config.c b/tests/test_config.c index 99c8404..68c35d4 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -2665,14 +2665,14 @@ static void golden_config_populate(Config* c) { c->copy_as_gid = 222; } -/* The pinned golden frame (protocol 2.22.0). The values below are the only +/* The pinned golden frame (protocol 2.23.0). The values below are the only * thing that ties the generated table to the historical wire format; update - * them ONLY with a PROTOCOL_VERSION bump and a documented reason. The 2.22.0 - * preserve-attribute split appends four serialized bools - * (preserve_perms/times/owner/group) to CONFIG_WIRE_METADATA_TIMES_FIELDS after - * omit_link_times. */ + * them ONLY with a PROTOCOL_VERSION bump and a documented reason. The 2.23.0 + * delete-semantics wave keeps the config-frame LAYOUT unchanged, but the + * embedded version string moves to "2.23.0", so the byte-exact hash changes + * while the length stays 653. */ #define GOLDEN_WIRE_LEN 653 -#define GOLDEN_WIRE_HASH 95530566005420798ULL +#define GOLDEN_WIRE_HASH 3267254725292157519ULL static unsigned long long fnv1a_64(const unsigned char* buf, size_t len) { unsigned long long h = 1469598103934665603ULL; @@ -2754,7 +2754,7 @@ static unsigned long long capture_wire_hash(const Config* cfg, size_t* out_len) return h; } -/* Byte-for-byte wire compatibility guard (protocol 2.22.0). The expected hash +/* Byte-for-byte wire compatibility guard (protocol 2.23.0). The expected hash * pins the pre-X-macro byte stream; the refactor MUST NOT change it. */ static void test_config_wire_golden() { if (is_running_under_valgrind()) diff --git a/tests/test_file_list.c b/tests/test_file_list.c index 777b43a..f8f0972 100644 --- a/tests/test_file_list.c +++ b/tests/test_file_list.c @@ -148,6 +148,50 @@ static void test_ancestor_and_descendant_queries() { remove(path); } +/* The delete-walker's synchronized-directory predicate: a directory is in scope + only when it is a listed directory or lies below one, NOT when it is merely an + implied parent of a listed file. */ +static void test_dir_in_scope() { + char err[160]; + + /* NULL set / empty list semantics. */ + EXPECT_TRUE(file_list_dir_in_scope(NULL, "anything")); + + const char* path = "test_file_list_dirscope.txt"; + write_list(path, "d1/leaf.txt\n"); + FileListSet* set = file_list_load(path, false, err, sizeof(err)); + EXPECT_NOT_NULL(set); + /* d1 is only an implied parent of a listed FILE: not synchronized. */ + EXPECT_FALSE(file_list_dir_in_scope(set, "d1")); + EXPECT_FALSE(file_list_dir_in_scope(set, "d1/sub")); + EXPECT_FALSE(file_list_dir_in_scope(set, "other")); + file_list_destroy(set); + remove(path); + + /* A listed DIRECTORY synchronizes itself and its whole subtree. */ + write_list(path, "d1/\nother\n"); + set = file_list_load(path, false, err, sizeof(err)); + EXPECT_NOT_NULL(set); + EXPECT_TRUE(file_list_dir_in_scope(set, "d1")); + EXPECT_TRUE(file_list_dir_in_scope(set, "d1/sub/deep")); + EXPECT_TRUE(file_list_dir_in_scope(set, "other")); + EXPECT_TRUE(file_list_dir_in_scope(set, "other/x")); + EXPECT_FALSE(file_list_dir_in_scope(set, "d2")); + EXPECT_FALSE(file_list_dir_in_scope(set, "d1x")); /* component boundary */ + EXPECT_FALSE(file_list_dir_in_scope(set, "")); + file_list_destroy(set); + remove(path); + + /* "." lists the whole tree. */ + write_list(path, ".\n"); + set = file_list_load(path, false, err, sizeof(err)); + EXPECT_NOT_NULL(set); + EXPECT_TRUE(file_list_dir_in_scope(set, "")); + EXPECT_TRUE(file_list_dir_in_scope(set, "anything/at/all")); + file_list_destroy(set); + remove(path); +} + /* Regression for the remote OOM: an adversarial --files-from entry made of a very deep chain of repeated components must be indexed with memory proportional to the entry count. The old implementation stored one copied @@ -219,6 +263,7 @@ static void test_oversized_entry_rejected() { void test_file_list() { test_membership_matches_reference(); test_ancestor_and_descendant_queries(); + test_dir_in_scope(); test_deep_paths_are_bounded(); test_oversized_entry_rejected(); } \ No newline at end of file diff --git a/tests/test_server.c b/tests/test_server.c index 26a2237..706b88c 100644 --- a/tests/test_server.c +++ b/tests/test_server.c @@ -645,9 +645,10 @@ static void test_late_second_manifest_frees_both() { config_delete(cfg); } -/* A delete-manifest frame with a third (missing-args) section round-trips: the - receiver keeps all three sections and the missing paths are confined exactly - like the keep-set (a traversal entry in the missing section is rejected). +/* A delete-manifest frame with all four sections round-trips: the receiver + keeps the keep-set, protected prefixes, missing-args paths and synchronized + directories, and every section is confined exactly like the keep-set (a + traversal entry in the missing section is rejected). receive_manifest_entries() reads the counts directly (the leading STATUS_MANIFEST code is consumed by the caller, so these frames do not send it). */ @@ -666,6 +667,9 @@ static void test_receive_manifest_three_sections() { EXPECT_TRUE(send_int(p[1], 2)); EXPECT_TRUE(send_str(p[1], "gone.txt")); EXPECT_TRUE(send_str(p[1], "dir/gone.bin")); + EXPECT_TRUE(send_int(p[1], 2)); + EXPECT_TRUE(send_str(p[1], ".")); + EXPECT_TRUE(send_str(p[1], "dir")); DeleteManifest* manifest = receive_manifest_entries(p[0]); EXPECT_NOT_NULL(manifest); @@ -676,9 +680,12 @@ static void test_receive_manifest_three_sections() { EXPECT_EQ_INT(manifest->missing->size, 2); EXPECT_EQ_STR((char*)manifest->missing->items[0], "gone.txt"); EXPECT_EQ_STR((char*)manifest->missing->items[1], "dir/gone.bin"); + EXPECT_EQ_INT(manifest->dirs->size, 2); + EXPECT_EQ_STR((char*)manifest->dirs->items[0], "."); + EXPECT_EQ_STR((char*)manifest->dirs->items[1], "dir"); delete_manifest_free(manifest); - /* A traversal entry in the third section is rejected like every other. */ + /* A traversal entry in the missing section is rejected like every other. */ EXPECT_TRUE(send_int(p[1], 0)); EXPECT_TRUE(send_int(p[1], 0)); EXPECT_TRUE(send_int(p[1], 1)); @@ -780,6 +787,7 @@ static void test_receiver_pending_commits_missing_args() { EXPECT_TRUE(send_int(p[1], 2)); EXPECT_TRUE(send_str(p[1], "gone.txt")); EXPECT_TRUE(send_str(p[1], "never_here.txt")); + EXPECT_TRUE(send_int(p[1], 0)); /* no synchronized directories */ EXPECT_TRUE(send_status(p[1], STATUS_FINISHED)); /* NULL pending: the single-threaded commit path deletes at FINISHED. The diff --git a/tests/test_shared_utils.c b/tests/test_shared_utils.c index 448dd86..af4ab15 100644 --- a/tests/test_shared_utils.c +++ b/tests/test_shared_utils.c @@ -136,7 +136,8 @@ static void test_walker_removes_extras_keeps_manifest_and_protected() { EXPECT_NOT_NULL(manifest); DeleteSkipEntry skip = {"prot", false}; size_t deleted = 0; - DeleteWalkResult result = delete_extras_limited(root, manifest, 100000, &skip, 1, &deleted); + DeleteWalkResult result = + delete_extras_limited(root, manifest, NULL, 100000, &skip, 1, &deleted, NULL); EXPECT_EQ_INT((int)result, (int)DELETE_WALK_OK); EXPECT_FALSE(file_exists(root, "a.txt")); EXPECT_TRUE(file_exists(root, "keep.txt")); @@ -170,7 +171,8 @@ static void test_walker_keeps_nested_manifest_dirs() { ArrayList* manifest = make_manifest_strings(keeps, 3); EXPECT_NOT_NULL(manifest); size_t deleted = 0; - DeleteWalkResult result = delete_extras_limited(root, manifest, 100000, NULL, 0, &deleted); + DeleteWalkResult result = + delete_extras_limited(root, manifest, NULL, 100000, NULL, 0, &deleted, NULL); EXPECT_EQ_INT((int)result, (int)DELETE_WALK_OK); EXPECT_FALSE(file_exists(root, "extra.txt")); EXPECT_TRUE(file_exists(root, "keepdir/deep/keep.txt")); @@ -187,7 +189,9 @@ static void test_walker_keeps_nested_manifest_dirs() { free(root); } -static void test_walker_max_delete_exceeded_deletes_nothing() { +/* --max-delete is a partial cap (rsync parity): delete up to the limit, skip + the rest, and report DELETE_WALK_LIMIT_REACHED. */ +static void test_walker_max_delete_partial_deletes_up_to_cap() { char* root = make_walk_root("maxdel"); EXPECT_NOT_NULL(root); EXPECT_TRUE(write_file_at(root, "a.txt", "extra")); @@ -197,12 +201,15 @@ static void test_walker_max_delete_exceeded_deletes_nothing() { ArrayList* manifest = make_manifest_strings(keeps, 0); EXPECT_NOT_NULL(manifest); size_t deleted = 999; - DeleteWalkResult result = delete_extras_limited(root, manifest, 2, NULL, 0, &deleted); - EXPECT_EQ_INT((int)result, (int)DELETE_WALK_LIMIT_EXCEEDED); - EXPECT_EQ_INT((int)deleted, 0); - EXPECT_TRUE(file_exists(root, "a.txt")); - EXPECT_TRUE(file_exists(root, "b.txt")); - EXPECT_TRUE(file_exists(root, "c.txt")); + size_t skipped = 0; + DeleteWalkResult result = + delete_extras_limited(root, manifest, NULL, 2, NULL, 0, &deleted, &skipped); + EXPECT_EQ_INT((int)result, (int)DELETE_WALK_LIMIT_REACHED); + EXPECT_EQ_INT((int)deleted, 2); + EXPECT_EQ_INT((int)skipped, 1); + int remaining = (file_exists(root, "a.txt") ? 1 : 0) + (file_exists(root, "b.txt") ? 1 : 0) + + (file_exists(root, "c.txt") ? 1 : 0); + EXPECT_EQ_INT(remaining, 1); array_list_delete(manifest); remove_walk_tree(root); free(root); @@ -217,7 +224,7 @@ static void test_walker_max_delete_exact_bound_deletes() { ArrayList* manifest = make_manifest_strings(keeps, 0); EXPECT_NOT_NULL(manifest); size_t deleted = 0; - DeleteWalkResult result = delete_extras_limited(root, manifest, 2, NULL, 0, &deleted); + DeleteWalkResult result = delete_extras_limited(root, manifest, NULL, 2, NULL, 0, &deleted, NULL); EXPECT_EQ_INT((int)result, (int)DELETE_WALK_OK); EXPECT_EQ_INT((int)deleted, 2); EXPECT_FALSE(file_exists(root, "a.txt")); @@ -227,6 +234,77 @@ static void test_walker_max_delete_exact_bound_deletes() { free(root); } +/* Extraneous destination symlinks (including one pointing at a directory) must + be unlinked, never followed, so their targets survive. */ +static void test_walker_removes_extraneous_symlinks() { + char* root = make_walk_root("symlink"); + char* outside = make_walk_root("symlink_out"); + EXPECT_NOT_NULL(root); + EXPECT_NOT_NULL(outside); + EXPECT_TRUE(write_file_at(outside, "secret.txt", "keep")); + EXPECT_TRUE(write_file_at(root, "keep.txt", "kept")); + char* link_file = path_cat(root, "link_file"); + char* link_dir = path_cat(root, "link_dir"); + char* link_broken = path_cat(root, "link_broken"); + EXPECT_NOT_NULL(link_file); + EXPECT_NOT_NULL(link_dir); + EXPECT_NOT_NULL(link_broken); + EXPECT_EQ_INT(symlink("keep.txt", link_file), 0); + EXPECT_EQ_INT(symlink(outside, link_dir), 0); + EXPECT_EQ_INT(symlink("/nonexistent-target", link_broken), 0); + const char* keeps[] = {"keep.txt"}; + ArrayList* manifest = make_manifest_strings(keeps, 1); + EXPECT_NOT_NULL(manifest); + size_t deleted = 0; + DeleteWalkResult result = + delete_extras_limited(root, manifest, NULL, 100000, NULL, 0, &deleted, NULL); + EXPECT_EQ_INT((int)result, (int)DELETE_WALK_OK); + EXPECT_FALSE(file_exists(root, "link_file")); + EXPECT_FALSE(file_exists(root, "link_dir")); + EXPECT_FALSE(file_exists(root, "link_broken")); + EXPECT_TRUE(file_exists(root, "keep.txt")); + EXPECT_TRUE(file_exists(outside, "secret.txt")); + free(link_file); + free(link_dir); + free(link_broken); + array_list_delete(manifest); + remove_walk_tree(root); + remove_walk_tree(outside); + free(root); + free(outside); +} + +/* With a synchronized-dir set, extras outside it survive while extras directly + inside a listed directory are removed; the receive root is the "." sentinel. */ +static void test_walker_confines_deletion_to_synced_dirs() { + char* root = make_walk_root("synced"); + EXPECT_NOT_NULL(root); + EXPECT_TRUE(write_file_at(root, "rootextra.txt", "keep")); + EXPECT_EQ_INT(make_subdir(root, "inscope"), 0); + EXPECT_TRUE(write_file_at(root, "inscope/extra.txt", "delete")); + EXPECT_TRUE(write_file_at(root, "inscope/keep.txt", "kept")); + EXPECT_EQ_INT(make_subdir(root, "outscope"), 0); + EXPECT_TRUE(write_file_at(root, "outscope/extra.txt", "keep")); + const char* keeps[] = {"inscope/keep.txt"}; + ArrayList* manifest = make_manifest_strings(keeps, 1); + ArrayList* dirs = array_list_create(free); + EXPECT_NOT_NULL(manifest); + EXPECT_NOT_NULL(dirs); + EXPECT_TRUE(array_list_add(dirs, str_dup("inscope"))); + size_t deleted = 0; + DeleteWalkResult result = + delete_extras_limited(root, manifest, dirs, 100000, NULL, 0, &deleted, NULL); + EXPECT_EQ_INT((int)result, (int)DELETE_WALK_OK); + EXPECT_TRUE(file_exists(root, "rootextra.txt")); + EXPECT_FALSE(file_exists(root, "inscope/extra.txt")); + EXPECT_TRUE(file_exists(root, "inscope/keep.txt")); + EXPECT_TRUE(file_exists(root, "outscope/extra.txt")); + array_list_delete(manifest); + array_list_delete(dirs); + remove_walk_tree(root); + free(root); +} + static void test_walker_unlimited_deletes_all() { char* root = make_walk_root("unlim"); EXPECT_NOT_NULL(root); @@ -245,53 +323,6 @@ static void test_walker_unlimited_deletes_all() { 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 = true; - 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 { bool eight_bit_output; const char* expected; @@ -553,10 +584,11 @@ void test_shared_utils() { test_getdelim_bounded(); test_walker_removes_extras_keeps_manifest_and_protected(); test_walker_keeps_nested_manifest_dirs(); - test_walker_max_delete_exceeded_deletes_nothing(); + test_walker_max_delete_partial_deletes_up_to_cap(); test_walker_max_delete_exact_bound_deletes(); + test_walker_removes_extraneous_symlinks(); + test_walker_confines_deletion_to_synced_dirs(); test_walker_unlimited_deletes_all(); - test_walker_hard_bound_all_or_nothing(); test_loopback_helpers(); test_fd_peer_ip();