From 4bf4da37e5b359502f788934d765d022b225e7e9 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 6 Sep 2026 19:43:11 +0200 Subject: [PATCH] fix: free basis path on copy-dest hits; keep --delete staging skip top-level-only - copy-dest basis hits leaked the heap-allocated basis path: BASIS_DEST_COPY did not transfer it (only LINK does) and returned before the basis cleanup. basis_match_free is now called on every materialization return path (success and send-failure) after content/link ownership is transferred. - the --delete walker regression: the delay-updates staging name must be protected only as a DIRECT child of the receive root, while basis dirs may be skipped at any depth. delete_extras_limited now takes DeleteSkipEntry entries carrying a top_level_only flag instead of a flat prefix list, so a nested destination directory named .fastsync-stage is ordinary content again (its extras are deleted) and a basis tree is still never removed. --- src/shared/file_receive.c | 37 ++++++++++++++++++++------------ src/shared/utils.c | 44 ++++++++++++++++++++++----------------- src/shared/utils.h | 21 ++++++++++++------- 3 files changed, 63 insertions(+), 39 deletions(-) diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 15de700..b10989c 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -849,12 +849,14 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { } if (materialized) { if (!send_status(fd, STATUS_OK)) { + basis_match_free(&basis); file_destroy(materialized); close(old_fd); free(full_path); free(check_path); return NULL; } + basis_match_free(&basis); free(old_data); close(old_fd); free(full_path); @@ -1069,29 +1071,38 @@ int receive_manifest(int fd, const Config* config, int* next_status) { /* 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. Alternate basis directories (--compare-dest / - --copy-dest / --link-dest) are also excluded: 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. */ + 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; - const char** skip_prefixes = NULL; + DeleteSkipEntry* skips = NULL; bool deletion_ok = false; if (skip_count > 0) { - skip_prefixes = calloc((size_t)skip_count, sizeof(char*)); - if (!skip_prefixes) { + skips = calloc((size_t)skip_count, sizeof(DeleteSkipEntry)); + if (!skips) { array_list_delete(manifest); send_status(fd, STATUS_ERROR); return -1; } int idx = 0; - if (config->delay_updates) - skip_prefixes[idx++] = DELAY_UPDATES_STAGING_DIR; - for (int i = 0; i < config->basis_count; i++) - skip_prefixes[idx++] = config->basis_dirs[i].path; + if (config->delay_updates) { + skips[idx].prefix = DELAY_UPDATES_STAGING_DIR; + skips[idx].top_level_only = true; + idx++; + } + for (int i = 0; i < config->basis_count; i++) { + skips[idx].prefix = config->basis_dirs[i].path; + skips[idx].top_level_only = false; + idx++; + } } deletion_ok = delete_extras_limited(config->receive_root_directory, manifest, - MAX_SERVER_DELETE_COUNT, skip_prefixes, skip_count); - free(skip_prefixes); + MAX_SERVER_DELETE_COUNT, skips, skip_count); + free(skips); array_list_delete(manifest); if (!deletion_ok) send_status(fd, STATUS_ERROR); diff --git a/src/shared/utils.c b/src/shared/utils.c index ac9f934..616ea66 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -204,22 +204,26 @@ static bool is_dir_in_manifest(const char* rel_path, ArrayList* manifest) { return false; } -/* True when the relative path is, or lies below, one of the protected - prefixes. A prefix "a" therefore protects "a" and "a/b/c" but not "ab". */ -static bool path_under_skip_prefix(const char* rel_path, const char* const* prefixes, - int prefix_count) { - for (int i = 0; i < prefix_count; i++) { - size_t prefix_len = strlen(prefixes[i]); - if (strncmp(rel_path, prefixes[i], prefix_len) == 0 && - (rel_path[prefix_len] == '\0' || rel_path[prefix_len] == '/')) +/* True when child_rel is, or lies below, a protected entry. A prefix "a" + therefore protects "a" and "a/b/c" but not "ab". Entries with top_level_only + set only protect DIRECT children of the receive root (at_root); nested + directories that share such a name stay ordinary destination content. */ +static bool path_under_skip_prefix(const char* child_rel, bool at_root, + const DeleteSkipEntry* skips, int skip_count) { + for (int i = 0; i < skip_count; i++) { + if (skips[i].top_level_only && !at_root) + continue; + size_t prefix_len = strlen(skips[i].prefix); + if (strncmp(child_rel, skips[i].prefix, prefix_len) == 0 && + (child_rel[prefix_len] == '\0' || child_rel[prefix_len] == '/')) return true; } return false; } static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest, - size_t max_delete, size_t* deleted_count, - const char* const* skip_prefixes, int skip_prefix_count) { + size_t max_delete, size_t* deleted_count, const DeleteSkipEntry* skips, + int skip_count) { int scanfd = dup(dirfd); if (scanfd < 0) return false; @@ -238,11 +242,14 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes operation_ok = false; continue; } - /* A --delay-updates run keeps its staging directory below the receive - root, and basis-dir snapshots live there too. Their contents are not - manifest entries, so descending into them would delete every staged / - basis file as an "extra". */ - if (path_under_skip_prefix(child_rel, skip_prefixes, skip_prefix_count)) { + /* A --delay-updates run keeps its staging directory as a direct child of + the receive root, and basis-dir snapshots live below it too. Their + contents are not manifest entries, so descending into them would delete + every staged / basis file as an "extra". Only the staging name (a + top-level-only prefix) and the basis prefixes are protected: a nested + 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)) { free(child_rel); continue; } @@ -263,7 +270,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes bool child_removed = false; if (childfd >= 0) { child_removed = delete_extras_fd(childfd, child_rel, manifest, max_delete, deleted_count, - skip_prefixes, skip_prefix_count); + skips, skip_count); if (!child_removed) operation_ok = false; close(childfd); @@ -315,7 +322,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes } bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete, - const char* const* skip_prefixes, int skip_prefix_count) { + const DeleteSkipEntry* skips, int skip_count) { if (!manifest) return false; int rootfd; @@ -332,8 +339,7 @@ bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t ma if (rootfd < 0) return false; size_t deleted_count = 0; - bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count, skip_prefixes, - skip_prefix_count); + bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count, skips, skip_count); if (close(rootfd) != 0) ok = false; return ok; diff --git a/src/shared/utils.h b/src/shared/utils.h index a32ddc8..4928e1e 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -10,14 +10,21 @@ char* output_escape(const char* string, bool eight_bit_output); char* path_cat(const char* path1, const char* path2); bool glob_match(const char* pattern, const char* str); bool delete_extras(const char* dest_root, ArrayList* manifest); -/* Remove files/dirs under dest_root that are not listed in manifest. The - delete walker never descends into (and so never removes) an entry whose - relative path equals one of the skip_prefixes or lies below one: used to - protect the --delay-updates staging directory (files still to be published) - and the --compare-dest/--copy-dest/--link-dest basis trees (snapshots the - transfer links from, never destination content). */ +/* One protected entry for the delete walker. When top_level_only is true the + prefix is skipped only as a DIRECT child of dest_root (the --delay-updates + staging directory, which must not hide genuine extras inside a nested + destination directory that happens to share the staging name); otherwise the + prefix is skipped at any depth (the --compare-dest/--copy-dest/--link-dest + basis trees, which the transfer links from and are never destination + content). */ +typedef struct { + const char* prefix; + bool top_level_only; +} DeleteSkipEntry; +/* Remove files/dirs under dest_root that are not listed in manifest without + ever descending into a protected prefix (see DeleteSkipEntry). */ bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete, - const char* const* skip_prefixes, int skip_prefix_count); + const DeleteSkipEntry* skips, int skip_count); bool utils_set_authorized_root(int fd, const char* canonical_path); /* The fd-only compatibility form is fail-closed for path-based operations; * callers should use utils_set_authorized_root with the canonical identity. */