From 1c9b660ac054b8a5078e02438d8edfcd7dd5e0e2 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 6 Sep 2026 21:49:57 +0200 Subject: [PATCH] feat: all-or-nothing bounded delete walker delete_extras_limited now rehearses a finite-capped deletion before unlinking anything (an identical fd-relative walk that counts files and directories) and returns DELETE_WALK_LIMIT_EXCEEDED with nothing removed when the run would exceed the cap, so --max-delete is enforced per run instead of truncating the deletion. A directory that still holds entries the walker leaves in place (protected excluded prefix, manifest-kept file, symlink) is left behind rather than failing the whole deletion, matching rsync's leave-non-empty-dirs behavior. Rehearsal/delete each open an independent file description so a prior pass cannot drain the directory stream. --- src/shared/utils.c | 150 ++++++++++++++++++++++++++++++++++++++++++--- src/shared/utils.h | 29 +++++++-- 2 files changed, 165 insertions(+), 14 deletions(-) diff --git a/src/shared/utils.c b/src/shared/utils.c index 616ea66..d2dbf89 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -221,10 +221,117 @@ static bool path_under_skip_prefix(const char* child_rel, bool at_root, 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, ArrayList* manifest, size_t cap, + size_t* count, bool* exceeds, const DeleteSkipEntry* skips, + int skip_count, bool* survives) { + /* 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. */ + 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; + 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, manifest, cap, count, exceeds, skips, + skip_count, &child_survives); + close(childfd); + } else if (errno != ENOENT) { + operation_ok = false; + } + if (!child_ok) + operation_ok = false; + if (is_dir_in_manifest(child_rel, manifest)) { + /* 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 = false; + for (int i = 0; i < manifest->size; i++) { + if (strcmp((char*)manifest->items[i], child_rel) == 0) { + found = true; + break; + } + } + 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, ArrayList* manifest, size_t max_delete, size_t* deleted_count, const DeleteSkipEntry* skips, int skip_count) { - int scanfd = dup(dirfd); + /* 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); @@ -282,7 +389,12 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes operation_ok = false; } else { if (unlinkat(dirfd, entry->d_name, AT_REMOVEDIR) != 0) { - if (errno != ENOENT) + /* 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)++; @@ -321,10 +433,13 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes return operation_ok; } -bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t max_delete, - const DeleteSkipEntry* skips, int skip_count) { +DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifest, + size_t max_delete, const DeleteSkipEntry* skips, + int skip_count, size_t* deleted_out) { + if (deleted_out) + *deleted_out = 0; if (!manifest) - return false; + return DELETE_WALK_ERROR; int rootfd; if (authorized_root_fd >= 0) { if (authorized_root_path) @@ -337,16 +452,35 @@ bool delete_extras_limited(const char* dest_root, ArrayList* manifest, size_t ma rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); } if (rootfd < 0) - return false; + 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, "", manifest, max_delete, &count, &exceeds, skips, + skip_count, &survives); + if (!counted_ok) { + close(rootfd); + return DELETE_WALK_ERROR; + } + if (exceeds) { + close(rootfd); + return DELETE_WALK_LIMIT_EXCEEDED; + } + } size_t deleted_count = 0; bool ok = delete_extras_fd(rootfd, "", manifest, max_delete, &deleted_count, skips, skip_count); if (close(rootfd) != 0) ok = false; - return ok; + if (deleted_out) + *deleted_out = deleted_count; + return ok ? DELETE_WALK_OK : DELETE_WALK_ERROR; } bool delete_extras(const char* dest_root, ArrayList* manifest) { - return delete_extras_limited(dest_root, manifest, SIZE_MAX, NULL, 0); + return delete_extras_limited(dest_root, manifest, SIZE_MAX, NULL, 0, NULL) == DELETE_WALK_OK; } bool has_path_traversal(const char* path) { diff --git a/src/shared/utils.h b/src/shared/utils.h index 4928e1e..cb2b3d3 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -9,22 +9,39 @@ char* str_dup(const char* string); 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); +/* Result of a bounded extra-file deletion run. */ +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, + /* A traversal or unlink failure aborted the deletion (partial removal is + possible, mirroring the delete pass). */ + DELETE_WALK_ERROR +} DeleteWalkResult; /* 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). */ + basis trees, and the sender-side protected filter-excluded prefixes, which + 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 DeleteSkipEntry* skips, int skip_count); + 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. */ +DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifest, + size_t max_delete, const DeleteSkipEntry* skips, + int skip_count, size_t* deleted_out); +bool delete_extras(const char* dest_root, 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; * callers should use utils_set_authorized_root with the canonical identity. */