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.
This commit is contained in:
2026-09-06 19:43:11 +02:00
parent 08a5815ca7
commit 4bf4da37e5
3 changed files with 63 additions and 39 deletions
+24 -13
View File
@@ -849,12 +849,14 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) {
} }
if (materialized) { if (materialized) {
if (!send_status(fd, STATUS_OK)) { if (!send_status(fd, STATUS_OK)) {
basis_match_free(&basis);
file_destroy(materialized); file_destroy(materialized);
close(old_fd); close(old_fd);
free(full_path); free(full_path);
free(check_path); free(check_path);
return NULL; return NULL;
} }
basis_match_free(&basis);
free(old_data); free(old_data);
close(old_fd); close(old_fd);
free(full_path); 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 /* With --delay-updates the staged (not yet published) files live directly
under the receive root in the staging directory; the delete walker must 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 not treat them as extras or it would remove every staged file before it
can be published. Alternate basis directories (--compare-dest / can be published. That staging name is protected only as a DIRECT child
--copy-dest / --link-dest) are also excluded: they are extra comparison of the receive root so a nested destination directory that happens to be
snapshots the user pointed at, not destination content, and deleting them named .fastsync-stage is still ordinary content. Alternate basis
would destroy the very files a --link-dest run just linked into place. */ 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; int skip_count = (config->delay_updates ? 1 : 0) + config->basis_count;
const char** skip_prefixes = NULL; DeleteSkipEntry* skips = NULL;
bool deletion_ok = false; bool deletion_ok = false;
if (skip_count > 0) { if (skip_count > 0) {
skip_prefixes = calloc((size_t)skip_count, sizeof(char*)); skips = calloc((size_t)skip_count, sizeof(DeleteSkipEntry));
if (!skip_prefixes) { if (!skips) {
array_list_delete(manifest); array_list_delete(manifest);
send_status(fd, STATUS_ERROR); send_status(fd, STATUS_ERROR);
return -1; return -1;
} }
int idx = 0; int idx = 0;
if (config->delay_updates) if (config->delay_updates) {
skip_prefixes[idx++] = DELAY_UPDATES_STAGING_DIR; skips[idx].prefix = DELAY_UPDATES_STAGING_DIR;
for (int i = 0; i < config->basis_count; i++) skips[idx].top_level_only = true;
skip_prefixes[idx++] = config->basis_dirs[i].path; 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, deletion_ok = delete_extras_limited(config->receive_root_directory, manifest,
MAX_SERVER_DELETE_COUNT, skip_prefixes, skip_count); MAX_SERVER_DELETE_COUNT, skips, skip_count);
free(skip_prefixes); free(skips);
array_list_delete(manifest); array_list_delete(manifest);
if (!deletion_ok) if (!deletion_ok)
send_status(fd, STATUS_ERROR); send_status(fd, STATUS_ERROR);
+25 -19
View File
@@ -204,22 +204,26 @@ static bool is_dir_in_manifest(const char* rel_path, ArrayList* manifest) {
return false; return false;
} }
/* True when the relative path is, or lies below, one of the protected /* True when child_rel is, or lies below, a protected entry. A prefix "a"
prefixes. A prefix "a" therefore protects "a" and "a/b/c" but not "ab". */ therefore protects "a" and "a/b/c" but not "ab". Entries with top_level_only
static bool path_under_skip_prefix(const char* rel_path, const char* const* prefixes, set only protect DIRECT children of the receive root (at_root); nested
int prefix_count) { directories that share such a name stay ordinary destination content. */
for (int i = 0; i < prefix_count; i++) { static bool path_under_skip_prefix(const char* child_rel, bool at_root,
size_t prefix_len = strlen(prefixes[i]); const DeleteSkipEntry* skips, int skip_count) {
if (strncmp(rel_path, prefixes[i], prefix_len) == 0 && for (int i = 0; i < skip_count; i++) {
(rel_path[prefix_len] == '\0' || rel_path[prefix_len] == '/')) 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 true;
} }
return false; return false;
} }
static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest, static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifest,
size_t max_delete, size_t* deleted_count, size_t max_delete, size_t* deleted_count, const DeleteSkipEntry* skips,
const char* const* skip_prefixes, int skip_prefix_count) { int skip_count) {
int scanfd = dup(dirfd); int scanfd = dup(dirfd);
if (scanfd < 0) if (scanfd < 0)
return false; return false;
@@ -238,11 +242,14 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
operation_ok = false; operation_ok = false;
continue; continue;
} }
/* A --delay-updates run keeps its staging directory below the receive /* A --delay-updates run keeps its staging directory as a direct child of
root, and basis-dir snapshots live there too. Their contents are not the receive root, and basis-dir snapshots live below it too. Their
manifest entries, so descending into them would delete every staged / contents are not manifest entries, so descending into them would delete
basis file as an "extra". */ every staged / basis file as an "extra". Only the staging name (a
if (path_under_skip_prefix(child_rel, skip_prefixes, skip_prefix_count)) { 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); free(child_rel);
continue; continue;
} }
@@ -263,7 +270,7 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes
bool child_removed = false; bool child_removed = false;
if (childfd >= 0) { if (childfd >= 0) {
child_removed = delete_extras_fd(childfd, child_rel, manifest, max_delete, deleted_count, child_removed = delete_extras_fd(childfd, child_rel, manifest, max_delete, deleted_count,
skip_prefixes, skip_prefix_count); skips, skip_count);
if (!child_removed) if (!child_removed)
operation_ok = false; operation_ok = false;
close(childfd); 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, 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) if (!manifest)
return false; return false;
int rootfd; int rootfd;
@@ -332,8 +339,7 @@ bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t ma
if (rootfd < 0) if (rootfd < 0)
return false; return false;
size_t deleted_count = 0; size_t deleted_count = 0;
bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count, skip_prefixes, bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count, skips, skip_count);
skip_prefix_count);
if (close(rootfd) != 0) if (close(rootfd) != 0)
ok = false; ok = false;
return ok; return ok;
+14 -7
View File
@@ -10,14 +10,21 @@ char* output_escape(const char* string, bool eight_bit_output);
char* path_cat(const char* path1, const char* path2); char* path_cat(const char* path1, const char* path2);
bool glob_match(const char* pattern, const char* str); bool glob_match(const char* pattern, const char* str);
bool delete_extras(const char* dest_root, ArrayList* manifest); bool delete_extras(const char* dest_root, ArrayList* manifest);
/* Remove files/dirs under dest_root that are not listed in manifest. The /* One protected entry for the delete walker. When top_level_only is true the
delete walker never descends into (and so never removes) an entry whose prefix is skipped only as a DIRECT child of dest_root (the --delay-updates
relative path equals one of the skip_prefixes or lies below one: used to staging directory, which must not hide genuine extras inside a nested
protect the --delay-updates staging directory (files still to be published) destination directory that happens to share the staging name); otherwise the
and the --compare-dest/--copy-dest/--link-dest basis trees (snapshots the prefix is skipped at any depth (the --compare-dest/--copy-dest/--link-dest
transfer links from, never destination content). */ 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, 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); bool utils_set_authorized_root(int fd, const char* canonical_path);
/* The fd-only compatibility form is fail-closed for path-based operations; /* The fd-only compatibility form is fail-closed for path-based operations;
* callers should use utils_set_authorized_root with the canonical identity. */ * callers should use utils_set_authorized_root with the canonical identity. */