From c4e0de8f08cace81599ae94bbb867d23c84593eb Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 6 Sep 2026 21:50:02 +0200 Subject: [PATCH] feat: split delete-manifest frame into keep-set + protected prefixes; enforce --max-delete and --force on the receiver The STATUS_MANIFEST frame now carries two count-delimited sections: the kept paths and a protected-prefix list (excluded-on-source paths the walker must not delete unless --delete-excluded opted out). The receiver's DeleteManifest is passed through the commit/early paths unchanged. manifest_delete_extras honors a client --max-delete (all-or-nothing) and produces a distinct error for it versus the 100000-entry server bound. --force clears a non-empty directory that blocks an incoming regular file (confined, symlink-safe) via a new file_remove_tree_secure helper. --- src/server/receiver.c | 20 ++--- src/server/receiver.h | 2 +- src/server/server.c | 2 +- src/shared/file_receive.c | 155 ++++++++++++++++++++++++++--------- src/shared/file_receive.h | 30 +++++-- src/shared/multiprocessing.c | 6 +- src/shared/multiprocessing.h | 18 +++- 7 files changed, 170 insertions(+), 63 deletions(-) diff --git a/src/server/receiver.c b/src/server/receiver.c index 384f186..0556292 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -143,7 +143,7 @@ int receiver_process(Config* config, int file_descriptor, const ReceiverSink* si the whole transfer succeeded. See receiver_process_pending() for how the -m receiver defers that commit until its disk writer has drained. */ int receiver_process_pending(Config* config, int file_descriptor, const ReceiverSink* sink, - ArrayList** pending_manifest) { + DeleteManifest** pending_manifest) { Status status; if (!receive_status(file_descriptor, &status)) return -1; @@ -151,7 +151,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver /* Parked keep-set for the late/commit timing. Every exit path below frees it exactly once; the only exception is the successful FINISHED handoff, which transfers ownership to *pending_manifest (used by the -m receiver). */ - ArrayList* deferred_manifest = NULL; + DeleteManifest* deferred_manifest = NULL; while (status == STATUS_NEXT || status == STATUS_CHUNK || status == STATUS_CHECK || status == STATUS_KEEPALIVE || status == STATUS_ABORT || status == STATUS_CHECK_BATCH || status == STATUS_MKDIR || status == STATUS_MANIFEST) { @@ -182,7 +182,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver if (!dir || !sink->store_file(dir, sink->context)) goto receive_error; } else if (status == STATUS_MANIFEST) { - ArrayList* manifest = receive_manifest_entries(file_descriptor); + DeleteManifest* manifest = receive_manifest_entries(file_descriptor); if (!manifest) goto fail; /* receive_manifest_entries already sent STATUS_ERROR */ if (early_delete) { @@ -192,7 +192,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver 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 ? manifest_delete_extras(config, manifest) : true; - array_list_delete(manifest); + delete_manifest_free(manifest); if (!deletion_ok) { send_status(file_descriptor, STATUS_ERROR); goto fail; @@ -204,15 +204,15 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver and commit the deletion only after STATUS_FINISHED. */ if (deferred_manifest) { log_message(LOG_LEVEL_ERROR, "Received a second delete manifest"); - array_list_delete(deferred_manifest); + delete_manifest_free(deferred_manifest); deferred_manifest = NULL; - array_list_delete(manifest); + delete_manifest_free(manifest); send_status(file_descriptor, STATUS_ERROR); goto fail; } deferred_manifest = manifest; } else { - array_list_delete(manifest); + delete_manifest_free(manifest); } goto next_status; } else { @@ -247,7 +247,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver deferred_manifest = NULL; } else { bool deletion_ok = manifest_delete_extras(config, deferred_manifest); - array_list_delete(deferred_manifest); + delete_manifest_free(deferred_manifest); deferred_manifest = NULL; if (!deletion_ok) { send_status(file_descriptor, STATUS_ERROR); @@ -269,14 +269,14 @@ fail: /* Failure exits that must not (or already did) report a STATUS_ERROR. The parked keep-set is dropped: never commit a deletion for a failed stream. */ if (deferred_manifest) { - array_list_delete(deferred_manifest); + delete_manifest_free(deferred_manifest); deferred_manifest = NULL; } return -1; receive_error: if (deferred_manifest) { - array_list_delete(deferred_manifest); + delete_manifest_free(deferred_manifest); deferred_manifest = NULL; } if (sink->send_error) diff --git a/src/server/receiver.h b/src/server/receiver.h index 7d41426..1b07e13 100644 --- a/src/server/receiver.h +++ b/src/server/receiver.h @@ -42,7 +42,7 @@ int receiver_process(Config* config, int file_descriptor, const ReceiverSink* si commit the deletion only after its disk writer has fully drained. Pass NULL to keep the default behaviour (delete before the success frame). */ int receiver_process_pending(Config* config, int file_descriptor, const ReceiverSink* sink, - ArrayList** pending_manifest); + DeleteManifest** pending_manifest); int receiver_receive_files(Config* config, int file_descriptor); #endif diff --git a/src/server/server.c b/src/server/server.c index 34eecd7..a8aa7e8 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -265,7 +265,7 @@ void handler(int file_descriptor) { if (!manifest_delete_extras(config, context->deferred_manifest)) { transfer_ok = false; } - array_list_delete(context->deferred_manifest); + delete_manifest_free(context->deferred_manifest); context->deferred_manifest = NULL; } } diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 0e63375..f28b3e1 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -218,6 +218,20 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi return FILE_SAVE_SKIPPED; } + /* --force (rsync semantics): an incoming regular file may replace a + destination DIRECTORY by removing that (possibly non-empty, symlink-safe) + tree first, so the atomic temp+rename below can install the file. Only the + immediate-install path does this: a --delay-updates run stages into its own + tree and is unaffected here (its publication renames over regular files + only). The blocking directory is removed only after the --update / + --existing / --ignore-existing decisions above, which see it as an existing + destination entry. */ + if (config && config->force_delete && !file->is_dir && + file_directory_exists_secure(destination_path)) { + if (!file_remove_tree_secure(destination_path)) + goto fail; + } + if (backup_enabled) { /* Back up the entry that the incoming write will replace. When writing through a partial dir the pre-existing destination file is the one to @@ -1021,63 +1035,93 @@ File* file_receive_directory(int file_descriptor) { } /* Read a delete-manifest frame (the STATUS_MANIFEST leading code has already - been consumed): an entry count followed by that many destination-relative - paths. The frame is self-delimiting (the count is authoritative), so the - caller decides what to do next and continues reading the following STATUS_* - frame. Returns an owned ArrayList of validated path strings, or NULL after - sending STATUS_ERROR when the frame is malformed (bad count, empty/absolute - path, path traversal, or an aggregate size beyond MAX_MANIFEST_BYTES). */ -ArrayList* receive_manifest_entries(int fd) { + 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. The frame is self-delimiting (the counts + are authoritative), so the caller decides what to do next and continues + reading the following STATUS_* frame. Returns an owned DeleteManifest, or + NULL after sending STATUS_ERROR when the frame is malformed (bad count, + empty/absolute path, path traversal, or an aggregate size beyond + MAX_MANIFEST_BYTES). */ +static bool receive_manifest_section(int fd, ArrayList* list, size_t* manifest_bytes) { int count; if (!receive_int(fd, &count)) { send_status(fd, STATUS_ERROR); - return NULL; + return false; } if (count < 0 || count > MAX_MANIFEST_ENTRIES) { send_status(fd, STATUS_ERROR); - return NULL; + return false; } - ArrayList* manifest = array_list_create(free); - if (!manifest) { - send_status(fd, STATUS_ERROR); - return NULL; - } - size_t manifest_bytes = 0; for (int i = 0; i < count; i++) { char* s = receive_str(fd); size_t entry_size = s ? strlen(s) : 0; if (!s || s[0] == '\0' || s[0] == '/' || has_path_traversal(s) || - entry_size > MAX_MANIFEST_BYTES - manifest_bytes || - (manifest_bytes += entry_size) > MAX_MANIFEST_BYTES || !array_list_add(manifest, s)) { + entry_size > MAX_MANIFEST_BYTES - *manifest_bytes || + (*manifest_bytes += entry_size) > MAX_MANIFEST_BYTES || !array_list_add(list, s)) { free(s); - array_list_delete(manifest); send_status(fd, STATUS_ERROR); - return NULL; + return false; } } + return true; +} + +DeleteManifest* receive_manifest_entries(int fd) { + DeleteManifest* manifest = calloc(1, sizeof(DeleteManifest)); + if (!manifest) { + send_status(fd, STATUS_ERROR); + return NULL; + } + manifest->keeps = array_list_create(free); + manifest->protected = array_list_create(free); + if (!manifest->keeps || !manifest->protected) { + delete_manifest_free(manifest); + send_status(fd, STATUS_ERROR); + return NULL; + } + size_t manifest_bytes = 0; + if (!receive_manifest_section(fd, manifest->keeps, &manifest_bytes) || + !receive_manifest_section(fd, manifest->protected, &manifest_bytes)) { + delete_manifest_free(manifest); + return NULL; + } return manifest; } -/* Remove every destination entry under the receive root that is not listed in - `manifest`, bounded by MAX_SERVER_DELETE_COUNT, using the symlink-safe - delete walker. 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. Prints a notice and returns true on success. */ -bool manifest_delete_extras(const Config* config, ArrayList* manifest) { - if (!config || !manifest) +void delete_manifest_free(DeleteManifest* manifest) { + if (!manifest) + return; + array_list_delete(manifest->keeps); + array_list_delete(manifest->protected); + free(manifest); +} + +/* 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 + 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 + 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) { + if (!config || !manifest || !manifest->keeps) return false; fprintf(stderr, "Deleting files not in manifest...\n"); - /* With --delay-updates the staged (not yet published) files live directly - under the receive root in the staging directory; the delete walker must - not treat them as extras or it would remove every staged file before it - can be published. That staging name is protected only as a DIRECT child - of the receive root so a nested destination directory that happens to be - named .fastsync-stage is still ordinary content. Alternate basis - directories (--compare-dest / --copy-dest / --link-dest) are excluded 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. */ - int skip_count = (config->delay_updates ? 1 : 0) + config->basis_count; + /* Protected entries: + - the --delay-updates staging name, protected only as a DIRECT child of the + receive root (a nested destination directory that happens to be named + .fastsync-stage is ordinary content); + - alternate basis directories (--compare-dest / --copy-dest / --link-dest) + 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. */ + int skip_count = (config->delay_updates ? 1 : 0) + config->basis_count + + (manifest->protected ? manifest->protected->size : 0); DeleteSkipEntry* skips = NULL; if (skip_count > 0) { skips = calloc((size_t)skip_count, sizeof(DeleteSkipEntry)); @@ -1094,9 +1138,40 @@ bool manifest_delete_extras(const Config* config, ArrayList* manifest) { skips[idx].top_level_only = false; idx++; } + for (int i = 0; i < manifest->protected->size; i++) { + skips[idx].prefix = (const char*)manifest->protected->items[i]; + skips[idx].top_level_only = false; + idx++; + } } - bool deletion_ok = delete_extras_limited(config->receive_root_directory, manifest, - MAX_SERVER_DELETE_COUNT, skips, skip_count); + /* 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); free(skips); - return deletion_ok; + 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; + } + if (result != DELETE_WALK_OK) { + log_message(LOG_LEVEL_ERROR, "deletion failed while removing extraneous files"); + return false; + } + return true; } diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h index f7e9f7f..3bc1cd0 100644 --- a/src/shared/file_receive.h +++ b/src/shared/file_receive.h @@ -10,14 +10,30 @@ File* file_receive(const Config* config, int file_descriptor); File* file_receive_directory(int file_descriptor); File* receive_incremental_check(int fd, const Config* config, bool* skipped); -/* Read a delete-manifest frame: entry count then paths (self-delimiting; the - leading STATUS_MANIFEST code has been consumed). Returns an owned path - ArrayList, or NULL after signalling STATUS_ERROR on a malformed frame. */ -ArrayList* receive_manifest_entries(int fd); + +/* A received delete-manifest frame: the keep-set (`keeps`, destination-relative + paths the sender transferred/keeps) plus `protected`, destination-relative + prefixes the sender asks the receiver never to delete (paths excluded on the + source, protected at any depth). When --delete-excluded is given the sender + transmits an empty protected list so excluded destination mirrors are treated + as ordinary extras. */ +typedef struct DeleteManifest { + ArrayList* keeps; + ArrayList* protected; +} DeleteManifest; + +void delete_manifest_free(DeleteManifest* manifest); +/* Read a delete-manifest frame: keep count + keeps, then protected count + + protected prefixes (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); /* Remove destination entries under config->receive_root_directory that are not - in `manifest` (bounded walk, staging-dir skip). The caller decides WHEN to - run it based on the negotiated delete timing. */ -bool manifest_delete_extras(const Config* config, ArrayList* manifest); + in `manifest` (bounded, all-or-nothing walk; staging-dir, basis-dir and + protected-prefix skips). `--max-delete` and `--force` are honored here. The + caller decides WHEN to run it based on the negotiated delete timing. Returns + false (and the transfer fails) when the deletion cannot be committed. */ +bool manifest_delete_extras(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 38636d4..32270d3 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -26,6 +26,8 @@ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* que context->scanner_done = false; context->loader_done = false; context->manifest = NULL; + context->excluded_paths = NULL; + context->scan_had_io_error = false; context->remove_source_files = NULL; context->early_delete = false; context->total_files = 0; @@ -82,6 +84,8 @@ void pipeline_context_sender_destroy(PipelineContextSender* context) { if (context->manifest) { array_list_delete(context->manifest); } + if (context->excluded_paths) + array_list_delete(context->excluded_paths); if (context->remove_source_files) array_list_delete(context->remove_source_files); config_delete(context->config); @@ -144,7 +148,7 @@ fail: void pipeline_context_receiver_destroy(PipelineContextReceiver* context) { config_delete(context->config); if (context->deferred_manifest) - array_list_delete(context->deferred_manifest); + delete_manifest_free(context->deferred_manifest); queue_destroy(context->queue); receiver_outcomes_destroy(&context->outcomes); mtx_destroy(&context->mutex); diff --git a/src/shared/multiprocessing.h b/src/shared/multiprocessing.h index d41cd5a..d515282 100644 --- a/src/shared/multiprocessing.h +++ b/src/shared/multiprocessing.h @@ -25,6 +25,18 @@ typedef struct { cnd_t condition_not_empty_loader; bool loader_done; ArrayList* manifest; + /* Protected prefixes (paths the source scan excluded by user rules) sent + with the keep-set manifest so --delete leaves them alone unless + --delete-excluded is set. NULL when not collecting. Populated by the + scanner thread (parallel workers append under mutex_scanner via the + 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; + /* A source I/O error (unreadable directory) was recorded during the scan. + Set by the pre-scan (before the threads start) or by the scanner thread + under mutex_scanner; the caller turns it into a non-zero exit when + --ignore-errors kept the run going. */ + bool scan_had_io_error; ArrayList* remove_source_files; /* True when --delete-before/--delete-during require the keep-set manifest to be transmitted before any file data: context->manifest is then prebuilt by @@ -65,9 +77,9 @@ typedef struct PipelineContextReceiver { protocol stream but hands the manifest here instead of deleting while the disk writer may still be draining; the caller (server.c) commits the deletion after both threads have joined, so no extra is removed unless the - transfer truly succeeded. NULL in the early delete modes (which delete at - the manifest). */ - ArrayList* deferred_manifest; + transfer truly succeeded. NULL in the early delete modes (which delete at + the manifest). */ + DeleteManifest* deferred_manifest; } PipelineContextReceiver; PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* queue_scanner,