From 884530c9a2316178a23615ee48e43fca4a243b1a Mon Sep 17 00:00:00 2001 From: TapTap Date: Thu, 24 Sep 2026 00:24:52 +0200 Subject: [PATCH] fix(filter): correct per-directory rule owner coordinate; post-clear ownership and bounds --- src/client/scanner_filter.c | 61 +++++++--- src/client/scanner_internal.h | 5 +- src/client/scanner_parallel.c | 5 +- src/shared/delete_plan.c | 25 ++++- src/shared/filter.c | 82 +++++++++----- src/shared/filter.h | 5 + tests/integration/test_differential_parity.py | 58 ++++++++++ tests/test_delete_plan.c | 105 ++++++++++++++++++ tests/test_filter.c | 54 +++++++++ 9 files changed, 351 insertions(+), 49 deletions(-) diff --git a/src/client/scanner_filter.c b/src/client/scanner_filter.c index f352ae1..d4b0007 100644 --- a/src/client/scanner_filter.c +++ b/src/client/scanner_filter.c @@ -443,17 +443,16 @@ 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. */ -bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_path, const char* rel, - bool relative_mode) { - if (!options->synced_dirs && !options->plan_dirs) - return true; - if (!file_list_dir_in_scope(options->file_list, rel)) - return true; +/* The destination-relative coordinate the receiver's delete walkers match + against for an entry at `fs_path` (with `rel` its path relative to the + transfer root, "" for the root): `relative_prefix + rel` under -R+--relative, + the bare relative path under -R+--files-from, else the source path with a + leading '/' removed, with "." for the receive root. Shared by the + synchronized-directory sink and the mirrored per-directory rule owners so + both live in the same coordinate system. Returns an owned string, or NULL on + allocation failure. */ +char* scanner_dest_rel_path(const ScannerOptions* options, const char* fs_path, const char* rel, + bool relative_mode) { char* prefixed = NULL; const char* dest; if (relative_mode) { @@ -461,7 +460,7 @@ bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_pat } else if (options->relative_prefix) { prefixed = scanner_prefix_send_path(options->relative_prefix, rel); if (!prefixed) - return false; + return NULL; dest = prefixed; } else { dest = fs_path; @@ -470,6 +469,24 @@ bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_pat dest++; if (dest[0] == '\0') dest = "."; + char* out = str_dup(dest); + free(prefixed); + return out; +} + +/* 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 (see scanner_dest_rel_path). Returns false on + allocation failure. */ +bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_path, const char* rel, + bool relative_mode) { + if (!options->synced_dirs && !options->plan_dirs) + return true; + if (!file_list_dir_in_scope(options->file_list, rel)) + return true; + char* dest = scanner_dest_rel_path(options, fs_path, rel, relative_mode); + if (!dest) + return false; bool ok = true; if (options->synced_dirs) ok = excluded_sink_append(options->synced_dirs, options->excluded_mutex, dest); @@ -478,7 +495,7 @@ bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_pat than deleted as an extra; the receive root (".") is implicit. */ if (ok && options->plan_dirs && strcmp(dest, ".") != 0) ok = excluded_sink_append(options->plan_dirs, options->excluded_mutex, dest); - free(prefixed); + free(dest); return ok; } @@ -487,7 +504,8 @@ bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_pat * fresh list. Returns NULL on allocation/parse failure (message in `err`); * returns an empty list (and *any_exists=false) when no file exists. */ FilterRuleList* read_dir_filters(const ScannerOptions* options, const char* dir_path, - const char* rel, bool* any_exists, char* err, size_t err_size) { + const char* rel, bool relative_mode, bool* any_exists, char* err, + size_t err_size) { if (err && err_size > 0) err[0] = '\0'; const FilterRuleList* base = options->base_filters; @@ -519,22 +537,31 @@ FilterRuleList* read_dir_filters(const ScannerOptions* options, const char* dir_ } } /* Mirror the directory's rules into the delete-carrier sink so the receiver - * can reconstruct its per-directory protect/risk set. */ + * can reconstruct its per-directory protect/risk set. The mirrored rules + * carry the destination-relative owner coordinate (not the transfer-root- + * relative one the sender's own evaluation uses) so the receiver's delete + * walkers, which match against receive-root-relative paths, find them. */ if (options->per_dir_rules && own->count > 0) { + char* owner = scanner_dest_rel_path(options, dir_path, rel, relative_mode); + if (!owner) + goto fail; mtx_t* mtx = options->excluded_mutex; if (mtx) mtx_lock(mtx); for (int i = 0; i < own->count; i++) { FilterRule* copy = filter_rule_clone(own->items[i]); - if (!copy || !filter_rule_list_add(options->per_dir_rules, copy)) { + if (!copy || !filter_rule_set_owner(copy, owner) || + !filter_rule_list_add(options->per_dir_rules, copy)) { filter_rule_free(copy); if (mtx) mtx_unlock(mtx); + free(owner); goto fail; } } if (mtx) mtx_unlock(mtx); + free(owner); } return own; fail: @@ -552,7 +579,7 @@ int open_directory_filter_context(DirectoryScanner* scanner, const FilterNode* i bool any_exists = false; FilterRuleList* own = read_dir_filters(&scanner->options, scanner->current_path, scanner->current_rel ? scanner->current_rel : "", - &any_exists, err, sizeof(err)); + scanner->relative_mode, &any_exists, err, sizeof(err)); if (!own) { /* read_dir_filters() leaves `err` set on a parse/allocation failure even when an earlier merge file in the same directory existed (any_exists true); diff --git a/src/client/scanner_internal.h b/src/client/scanner_internal.h index c772d9d..d32bd40 100644 --- a/src/client/scanner_internal.h +++ b/src/client/scanner_internal.h @@ -73,6 +73,8 @@ File* scanner_build_dir_file(const char* path, const struct stat* stats, const ScannerOptions* options); char* child_rel_path(const char* parent_rel, const char* name); char* scanner_prefix_send_path(const char* prefix, const char* rel); +char* scanner_dest_rel_path(const ScannerOptions* options, const char* fs_path, const char* rel, + bool relative_mode); bool entry_passes_selection(const FileListSet* file_list, const FilterRuleList* base, const FilterNode* node, const char* rel, const char* leaf, bool is_dir, bool per_dir_filters, bool exclude_filter_files, bool* protect_out); @@ -92,7 +94,8 @@ void scanner_record_size_skipped(DirectoryScanner* scanner, const char* fs_path) bool scanner_record_synced_dir(const ScannerOptions* options, const char* fs_path, const char* rel, bool relative_mode); FilterRuleList* read_dir_filters(const ScannerOptions* options, const char* dir_path, - const char* rel, bool* any_exists, char* err, size_t err_size); + const char* rel, bool relative_mode, bool* any_exists, char* err, + size_t err_size); int open_directory_filter_context(DirectoryScanner* scanner, const FilterNode* inherited); int scanner_inspect_entry(const ScannerOptions* options, const char* containing_dir, const char* link_rel, const char* name, ScannerEntry* entry); diff --git a/src/client/scanner_parallel.c b/src/client/scanner_parallel.c index f82eccb..40e4330 100644 --- a/src/client/scanner_parallel.c +++ b/src/client/scanner_parallel.c @@ -589,8 +589,9 @@ ParallelScanner* parallel_scanner_create_with_options(const char* root_directory { char err[256]; bool any_exists = false; - FilterRuleList* own = - read_dir_filters(options, root_directory, "", &any_exists, err, sizeof(err)); + FilterRuleList* own = read_dir_filters(options, root_directory, "", + options->relative && options->file_list != NULL, + &any_exists, err, sizeof(err)); if (!own) { /* A parse/allocation failure must fail the scan even when an earlier merge file in the same directory existed (see the sequential scanner). */ diff --git a/src/shared/delete_plan.c b/src/shared/delete_plan.c index 1939960..91e3bb4 100644 --- a/src/shared/delete_plan.c +++ b/src/shared/delete_plan.c @@ -334,7 +334,10 @@ static const char* filter_dir_rule_owner(const FilterRule* rule) { * order = the sender's traversal/rule order). Rules of one directory are * appended to the sink contiguously, so runs reproduce the compilation order. * Bounded by MAX_FILTER_RULES / MAX_FILTER_BYTES and MAX_PROTECT_PATTERN_LEN so - * the peer never sees a frame it would reject. */ + * the peer never sees a frame it would reject. The sender enforces exactly the + * receiver's limits (including the cumulative owner+pattern byte budget) and + * fails with a clear local error instead of emitting a frame that would abort + * the transfer with STATUS_ERROR. */ bool delete_filter_dir_rules_send(int fd, const FilterRuleList* rules) { int count = rules ? rules->count : 0; if (count < 0 || count > MAX_FILTER_RULES) { @@ -342,13 +345,31 @@ bool delete_filter_dir_rules_send(int fd, const FilterRuleList* rules) { MAX_FILTER_RULES); return false; } + size_t bytes = 0; for (int i = 0; i < count; i++) { const FilterRule* rule = rules->items[i]; + const char* owner = filter_dir_rule_owner(rule); + size_t owner_len = strlen(owner); size_t pattern_len = rule && rule->pattern ? strlen(rule->pattern) : 0; - if (!rule || !rule->pattern || pattern_len == 0 || pattern_len > MAX_PROTECT_PATTERN_LEN) { + if (!rule || !rule->pattern || pattern_len == 0) { log_message(LOG_LEVEL_ERROR, "invalid per-directory filter pattern"); return false; } + if (pattern_len > MAX_PROTECT_PATTERN_LEN) { + log_message(LOG_LEVEL_ERROR, + "per-directory filter pattern exceeds %d bytes (use a shorter pattern)", + MAX_PROTECT_PATTERN_LEN); + return false; + } + if (!(owner_len == 0 || (owner[0] != '/' && !has_path_traversal(owner)))) { + log_message(LOG_LEVEL_ERROR, "invalid per-directory filter owner directory"); + return false; + } + if (owner_len + pattern_len > MAX_FILTER_BYTES - bytes) { + log_message(LOG_LEVEL_ERROR, "per-directory filter rules exceed %d bytes", MAX_FILTER_BYTES); + return false; + } + bytes += owner_len + pattern_len; } int groups = 0; for (int i = 0; i < count;) { diff --git a/src/shared/filter.c b/src/shared/filter.c index 8c4087d..f8365bd 100644 --- a/src/shared/filter.c +++ b/src/shared/filter.c @@ -11,8 +11,6 @@ /* Write a diagnostic message into the caller's optional buffer. */ #define filter_set_error utils_set_error -static bool set_rule_owner(FilterRule* rule, const char* owner); - /* ---- Ordered rule lists ---- */ void filter_rule_free(FilterRule* rule) { @@ -93,7 +91,7 @@ static bool filter_list_add_exclude_self(FilterRuleList* list, const char* name) rule->action = FILTER_ACTION_EXCLUDE; rule->sides = FILTER_SIDE_SENDER | FILTER_SIDE_RECEIVER; rule->pattern = str_dup(base); - if (!rule->pattern || !set_rule_owner(rule, "")) { + if (!rule->pattern || !filter_rule_set_owner(rule, "")) { filter_rule_free(rule); return false; } @@ -116,10 +114,22 @@ bool filter_rule_list_add_dir_merge_ex(FilterRuleList* list, const char* name, b if (!list || !name || name[0] == '\0') return false; for (int i = 0; i < list->dir_merge_count; i++) { - if (strcmp(list->dir_merges[i].name, name) == 0) + if (strcmp(list->dir_merges[i].name, name) == 0) { + /* rsync keeps the first registration (first-wins), but the 'e' modifier + is a list side effect, not a registration field: honor it on the + duplicate path too, adding the implicit exclude-self rule at most + once. */ + if (exclude_self && !list->dir_merges[i].exclude_self) { + if (!filter_list_add_exclude_self(list, name)) + return false; + list->dir_merges[i].exclude_self = true; + } return true; + } } if (list->dir_merge_count == list->dir_merge_capacity) { + if (list->dir_merge_capacity > INT_MAX / 2) + return false; int new_cap = list->dir_merge_capacity > 0 ? list->dir_merge_capacity * 2 : 4; FilterDirMerge* grown = realloc(list->dir_merges, (size_t)new_cap * sizeof(*grown)); if (!grown) @@ -145,7 +155,9 @@ bool filter_rule_list_add_dir_merge_ex(FilterRuleList* list, const char* name, b return true; } -static bool set_rule_owner(FilterRule* rule, const char* owner) { +bool filter_rule_set_owner(FilterRule* rule, const char* owner) { + if (!rule) + return false; char* dup = str_dup(owner ? owner : ""); if (!dup) return false; @@ -596,7 +608,7 @@ static bool filter_list_append_cvs(FilterRuleList* list, unsigned sides) { } memcpy(rule->pattern, CVS_DEFAULTS[i].pattern, plen); rule->pattern[plen] = '\0'; - if (!set_rule_owner(rule, "")) { + if (!filter_rule_set_owner(rule, "")) { filter_rule_free(rule); return false; } @@ -647,7 +659,12 @@ static bool filter_merge_read(FilterRuleList* list, FILE* fp, const char* displa const FilterDirMerge* spec, const FilterParseOptions* opts, const char* base_dir, const char* owner_rel, int depth, char* err, size_t err_size) { - int rules_before = list->count; + /* Lowest list index this read is responsible for. A "clear"/"!" inside the + * file resets list->count to 0 (freeing the caller's earlier rules too), so + * the base must follow it down: otherwise post-clear rules sit below the + * original count and never receive an owner (nor no-inherit) and are missed + * by the rollback. */ + int floor = list->count; char* line = NULL; size_t cap = 0; bool ok = true; @@ -687,6 +704,8 @@ static bool filter_merge_read(FilterRuleList* list, FILE* fp, const char* displa token[tlen] = '\0'; if (!filter_merge_append_token(list, token, spec, opts, base_dir, depth, err, err_size)) ok = false; + if (list->count < floor) + floor = list->count; /* a "clear" reset the list below this read's base */ free(token); } } else { @@ -697,27 +716,28 @@ static bool filter_merge_read(FilterRuleList* list, FILE* fp, const char* displa continue; if (!filter_merge_append_token(list, lp, spec, opts, base_dir, depth, err, err_size)) ok = false; + if (list->count < floor) + floor = list->count; /* a "clear" reset the list below this read's base */ } } free(line); if (!ok) { - /* Drop the rules this read appended (a "clear" inside the file may have - * freed earlier rules too; clamp like filter_file_rollback). */ - int first = rules_before < list->count ? rules_before : list->count; - for (int i = first; i < list->count; i++) + /* Drop every live rule this read is responsible for. After a "clear" that + * base is 0, so the post-clear rules are freed too instead of leaking. */ + for (int i = floor; i < list->count; i++) filter_rule_free(list->items[i]); - list->count = first; + list->count = floor; return false; } - for (int i = rules_before; i < list->count; i++) { + for (int i = floor; i < list->count; i++) { FilterRule* rule = list->items[i]; if (spec->no_inherit) rule->no_inherit = true; - if (owner_rel && !set_rule_owner(rule, owner_rel)) { + if (owner_rel && !filter_rule_set_owner(rule, owner_rel)) { filter_set_error(err, err_size, "memory allocation failed"); - for (int j = rules_before; j < list->count; j++) + for (int j = floor; j < list->count; j++) filter_rule_free(list->items[j]); - list->count = rules_before; + list->count = floor; return false; } } @@ -912,11 +932,10 @@ FilterRuleList* filter_base_build(const char* const* rule_texts, int rule_count, /* Undo the rules and dir-merge registrations that one merge file appended, * leaving the caller's earlier content intact. A "clear" rule inside the file * frees every rule, including the caller's; clamp to the surviving count so - * those already-freed rules are never resurrected and freed a second time. */ -/* Undo the rules and dir-merge registrations that one merge file appended, - * leaving the caller's earlier content intact. A "clear" rule inside the file - * frees every rule, including the caller's; clamp to the surviving count so - * those already-freed rules are never resurrected and freed a second time. */ + * those already-freed rules are never resurrected and freed a second time. + * (filter_merge_read() has already rolled its own range back by the time this + * runs, so on a post-clear failure `list->count` is below `rules_before` and + * this is a no-op for the rules.) */ static void filter_file_rollback(FilterRuleList* list, int rules_before, int dir_merges_before) { int first = rules_before < list->count ? rules_before : list->count; for (int i = first; i < list->count; i++) @@ -1016,11 +1035,16 @@ static FilterAction rule_matches(const FilterRule* rule, const char* rel_path, c return FILTER_ACTION_NONE; if (!(rule->sides & side)) return FILTER_ACTION_NONE; - /* A rule applies only to entries below its owner directory. */ + /* A rule applies only to entries below its owner directory. The receive + * root's destination-relative coordinate may be written as "." (the + * synced-directory sentinel), which is the same scope as the empty owner. */ + const char* owner = rule->owner; + if (owner && strcmp(owner, ".") == 0) + owner = ""; const char* rel2 = rel_path; - if (rule->owner && rule->owner[0] != '\0') { - size_t owner_len = strlen(rule->owner); - if (strncmp(rule->owner, rel_path, owner_len) != 0) + if (owner && owner[0] != '\0') { + size_t owner_len = strlen(owner); + if (strncmp(owner, rel_path, owner_len) != 0) return FILTER_ACTION_NONE; if (rel_path[owner_len] != '/') return FILTER_ACTION_NONE; @@ -1078,10 +1102,14 @@ FilterAction filter_dir_rules_apply_side(const FilterRuleList* dir_rules, const for (;;) { for (int i = 0; i < dir_rules->count; i++) { const FilterRule* rule = dir_rules->items[i]; - size_t rule_owner_len = rule && rule->owner ? strlen(rule->owner) : 0; + const char* rule_owner = rule && rule->owner ? rule->owner : ""; + /* "." is the receive root's coordinate (see rule_matches). */ + if (strcmp(rule_owner, ".") == 0) + rule_owner = ""; + size_t rule_owner_len = strlen(rule_owner); if (rule_owner_len != owner_len) continue; - if (owner_len != 0 && memcmp(rule->owner, rel_path, owner_len) != 0) + if (owner_len != 0 && memcmp(rule_owner, rel_path, owner_len) != 0) continue; FilterAction action = rule_matches(rule, rel_path, leaf, is_dir, FILTER_SIDE_RECEIVER); if (action != FILTER_ACTION_NONE) diff --git a/src/shared/filter.h b/src/shared/filter.h index d5f5ea7..9c4ae2f 100644 --- a/src/shared/filter.h +++ b/src/shared/filter.h @@ -100,6 +100,11 @@ FilterRule* filter_rule_parse(const char* line, const FilterParseOptions* opts, void filter_rule_free(FilterRule* rule); /* Deep-copy a rule (owned pattern/owner). Returns NULL on allocation failure. */ FilterRule* filter_rule_clone(const FilterRule* rule); +/* Replace a rule's owner directory (owned copy of `owner`, "" for the transfer + * root). Returns false on allocation failure, leaving the rule unchanged. + * Used to re-express a mirrored per-directory rule in the receiver's + * destination-relative coordinate system. */ +bool filter_rule_set_owner(FilterRule* rule, const char* owner); FilterRuleList* filter_rule_list_create(void); /* Append a fully-parsed rule (takes ownership). Returns false on OOM. */ diff --git a/tests/integration/test_differential_parity.py b/tests/integration/test_differential_parity.py index d95e95c..b2c15af 100644 --- a/tests/integration/test_differential_parity.py +++ b/tests/integration/test_differential_parity.py @@ -154,6 +154,33 @@ def seed_perdir_exclude(_src, rroot, froot): _mk(os.path.join(root, "other.txt"), b"dest-only deleted\n", _OLD_MTIME) +def seed_perdir_subdir_protect(_src, rroot, froot): + """A SUBDIRECTORY-owned `.rsync-filter` (its owner is not the transfer root): + the receiver must re-derive the `P` rules in the destination-relative + coordinate system, otherwise the destination-only nested extras are wrongly + deleted (silent data loss). A root-level extra is included so a too-broad + rule would over-protect. The file is seeded on both destinations because + rsync's receiver reads the per-directory file locally for delete-during.""" + for root in (_src, rroot, froot): + _mk(os.path.join(root, "sub", ".rsync-filter"), b"P nested.log\nP extra.log\n") + for root in (rroot, froot): + _mk(os.path.join(root, "sub", "nested.log"), b"dest-only protected\n", _OLD_MTIME) + _mk(os.path.join(root, "sub", "extra.log"), b"dest-only protected 2\n", _OLD_MTIME) + _mk(os.path.join(root, "sub", "other.txt"), b"dest-only deleted\n", _OLD_MTIME) + _mk(os.path.join(root, "root_extra.txt"), b"root dest-only deleted\n", _OLD_MTIME) + + +def seed_perdir_subdir_exclude(_src, rroot, froot): + """A SUBDIRECTORY-owned unqualified exclude (`-`): dual-sided, so it protects + the matching destination-only nested extra under plain --delete and is opted + back in by --delete-excluded.""" + for root in (_src, rroot, froot): + _mk(os.path.join(root, "sub", ".rsync-filter"), b"- nested.log\n") + for root in (rroot, froot): + _mk(os.path.join(root, "sub", "nested.log"), b"dest-only excluded\n", _OLD_MTIME) + _mk(os.path.join(root, "sub", "other.txt"), b"dest-only deleted\n", _OLD_MTIME) + + def _seed_rules(content): def seed(_src, _rroot, _froot): _mk(os.path.join(_src, ".rules"), content) @@ -336,6 +363,37 @@ _CASES = [ ["-a", "-F", "--delete", "--delete-excluded"], seed=seed_perdir_exclude, server_args=DELETE, ci=True, ref="-F per-directory exclude under --delete-excluded is at risk"), + # #316: a rule owned by a SUBDIRECTORY (not the transfer root) must be + # re-expressed in the receiver's destination-relative coordinate system, or + # the dest-only extras it protects are silently deleted. + H.Case("filter_perdir_subdir_protect", "filters", + ["-a", "-F", "--delete"], + seed=seed_perdir_subdir_protect, server_args=DELETE, ci=True, + ref="-F subdirectory-owned P rule under the default --delete timing"), + H.Case("filter_perdir_subdir_protect_during", "filters", + ["-a", "-F", "--delete-during"], + seed=seed_perdir_subdir_protect, server_args=DELETE, ci=True, + ref="-F subdirectory-owned P rule under --delete-during"), + H.Case("filter_perdir_subdir_protect_delay", "filters", + ["-a", "-F", "--delete-delay"], + seed=seed_perdir_subdir_protect, server_args=DELETE, ci=True, + ref="-F subdirectory-owned P rule under --delete-delay"), + H.Case("filter_perdir_subdir_protect_before", "filters", + ["-a", "-F", "--delete-before"], + seed=seed_perdir_subdir_protect, server_args=DELETE, ci=True, + ref="-F subdirectory-owned P rule under the whole-tree --delete-before commit"), + H.Case("filter_perdir_subdir_protect_after", "filters", + ["-a", "-F", "--delete-after"], + seed=seed_perdir_subdir_protect, server_args=DELETE, ci=True, + ref="-F subdirectory-owned P rule under the whole-tree --delete-after commit"), + H.Case("filter_perdir_subdir_exclude_protect", "filters", + ["-a", "-F", "--delete"], + seed=seed_perdir_subdir_exclude, server_args=DELETE, ci=True, + ref="-F subdirectory-owned exclude protects its destination mirror"), + H.Case("filter_perdir_subdir_exclude_deleted", "filters", + ["-a", "-F", "--delete", "--delete-excluded"], + seed=seed_perdir_subdir_exclude, server_args=DELETE, ci=True, + ref="-F subdirectory-owned exclude under --delete-excluded is at risk"), # Merge-file modifiers (#315): e/n/w/- semantics match rsync 3.4.1. H.Case("dir_merge_e", "filters", ["-a", "--filter=:e .rules"], seed=_seed_rules(b"- *.log\n"), ci=True, diff --git a/tests/test_delete_plan.c b/tests/test_delete_plan.c index f026eec..5a82858 100644 --- a/tests/test_delete_plan.c +++ b/tests/test_delete_plan.c @@ -11,8 +11,23 @@ #include #include #include +#include #include +/* Discards everything written to `fd` until EOF, so a sender that regresses to + * emitting an over-budget frame does not block forever on a full socket. */ +typedef struct { + int fd; +} DrainArg; + +static int drain_fd_thread(void* arg) { + DrainArg* drain = arg; + char buffer[8192]; + while (read(drain->fd, buffer, sizeof(buffer)) > 0) + ; + return 0; +} + /* Send one STATUS_DELETE_PLAN body (the leading status is consumed by the * caller/receiver entry point) describing `dir` with no kept children. */ static void send_plan_frame(int fd, const char* dir) { @@ -366,6 +381,95 @@ static void test_filter_dir_rules_receive_bounds(void) { close(p[1]); } +/* The per-directory rule sender enforces exactly the receiver's limits: an + * over-long pattern and an over-budget owner+pattern total are rejected locally + * with a clear error instead of emitting a frame the peer would abort the + * transfer on. A valid block still round-trips. */ +static void test_filter_dir_rules_send_bounds(void) { + int p[2]; + + /* An over-long pattern is rejected before anything is written. */ + { + FilterRuleList* list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + FilterRule* rule = calloc(1, sizeof(FilterRule)); + EXPECT_NOT_NULL(rule); + rule->action = FILTER_ACTION_EXCLUDE; + rule->sides = FILTER_SIDE_SENDER | FILTER_SIDE_RECEIVER; + rule->owner = str_dup("sub"); + rule->pattern = malloc(MAX_PROTECT_PATTERN_LEN + 2); + EXPECT_NOT_NULL(rule->owner); + EXPECT_NOT_NULL(rule->pattern); + memset(rule->pattern, 'a', MAX_PROTECT_PATTERN_LEN + 1); + rule->pattern[MAX_PROTECT_PATTERN_LEN + 1] = '\0'; + EXPECT_TRUE(filter_rule_list_add(list, rule)); + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + EXPECT_FALSE(delete_filter_dir_rules_send(p[1], list)); + close(p[0]); + close(p[1]); + filter_rule_list_free(list); + } + + /* A cumulative owner+pattern total over MAX_FILTER_BYTES is rejected. */ + { + FilterRuleList* list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + int per = MAX_PROTECT_PATTERN_LEN; + int need = MAX_FILTER_BYTES / per + 1; + EXPECT_TRUE(need < MAX_FILTER_RULES); + for (int i = 0; i < need; i++) { + FilterRule* rule = calloc(1, sizeof(FilterRule)); + EXPECT_NOT_NULL(rule); + rule->action = FILTER_ACTION_EXCLUDE; + rule->sides = FILTER_SIDE_SENDER | FILTER_SIDE_RECEIVER; + rule->owner = str_dup(""); + rule->pattern = malloc((size_t)per + 1); + EXPECT_NOT_NULL(rule->owner); + EXPECT_NOT_NULL(rule->pattern); + memset(rule->pattern, 'b', (size_t)per); + rule->pattern[per] = '\0'; + EXPECT_TRUE(filter_rule_list_add(list, rule)); + } + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + DrainArg drain = {p[0]}; + thrd_t drainer; + EXPECT_EQ_INT(thrd_create(&drainer, drain_fd_thread, &drain), thrd_success); + bool sent = delete_filter_dir_rules_send(p[1], list); + close(p[1]); + thrd_join(drainer, NULL); + EXPECT_FALSE(sent); + close(p[0]); + filter_rule_list_free(list); + } + + /* A valid block still round-trips through send -> receive. */ + { + FilterRuleList* list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + FilterRule* rule = calloc(1, sizeof(FilterRule)); + EXPECT_NOT_NULL(rule); + rule->action = FILTER_ACTION_EXCLUDE; + rule->sides = FILTER_SIDE_SENDER | FILTER_SIDE_RECEIVER; + rule->owner = str_dup("sub"); + rule->pattern = str_dup("*.log"); + EXPECT_NOT_NULL(rule->owner); + EXPECT_NOT_NULL(rule->pattern); + EXPECT_TRUE(filter_rule_list_add(list, rule)); + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + EXPECT_TRUE(delete_filter_dir_rules_send(p[1], list)); + FilterRuleList* out = NULL; + EXPECT_TRUE(delete_filter_dir_rules_receive(p[0], &out)); + EXPECT_NOT_NULL(out); + EXPECT_EQ_INT(out->count, 1); + EXPECT_EQ_STR(out->items[0]->owner, "sub"); + EXPECT_EQ_STR(out->items[0]->pattern, "*.log"); + filter_rule_list_free(out); + close(p[0]); + close(p[1]); + filter_rule_list_free(list); + } +} + void test_delete_plan(void) { test_delete_delay_refilled_dir_removed_recursively(); test_delete_delay_removed_file_counted(); @@ -373,4 +477,5 @@ void test_delete_plan(void) { test_delete_delay_actual_removal_charges_budget(); test_config_only_frame_applies_missing_args(); test_filter_dir_rules_receive_bounds(); + test_filter_dir_rules_send_bounds(); } diff --git a/tests/test_filter.c b/tests/test_filter.c index 03a38cd..f4a190b 100644 --- a/tests/test_filter.c +++ b/tests/test_filter.c @@ -258,6 +258,59 @@ static void test_filter_list_accepts_supported_rules_and_modifiers() { } } +/* A "clear"/"!" inside a merge file resets the list to empty. Rules read after + * it must still be owned by the merge file's directory (and marked no-inherit + * when the dir-merge says so). The base index must follow the clear down: when + * it was captured before the clear, post-clear rules sat below it and were left + * globally owned by "" (and unmarked). */ +static void test_filter_merge_clear_then_owner() { + char tmpl[] = "/tmp/fastsync_filter_clear_XXXXXX"; + EXPECT_TRUE(mkdtemp(tmpl) != NULL); + char path[512]; + snprintf(path, sizeof(path), "%s/.rsync-filter", tmpl); + FILE* fp = fopen(path, "w"); + EXPECT_NOT_NULL(fp); + fputs("- *.tmp\n!\nP *.log\n", fp); + fclose(fp); + + FilterRuleList* list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + char err[256] = ""; + /* A pre-existing rule that the in-file clear must discard. */ + EXPECT_TRUE(filter_rule_list_parse_append(list, "- keep.txt", NULL, NULL, err, sizeof(err))); + EXPECT_EQ_INT(list->count, 1); + + FilterDirMerge spec = {.name = ".rsync-filter", .no_inherit = true}; + bool exists = false; + EXPECT_TRUE(filter_dir_merge_append(list, tmpl, &spec, "sub", NULL, &exists, err, sizeof(err))); + EXPECT_TRUE(exists); + /* Only the post-clear rule survives, owned by "sub" and no-inherit. */ + EXPECT_EQ_INT(list->count, 1); + EXPECT_EQ_STR(list->items[0]->pattern, "*.log"); + EXPECT_EQ_STR(list->items[0]->owner, "sub"); + EXPECT_TRUE(list->items[0]->no_inherit); + filter_rule_list_free(list); + + /* A parse failure after the clear must roll the list back to the post-clear + * base (empty here), freeing the post-clear rule rather than retaining it. */ + fp = fopen(path, "w"); + EXPECT_NOT_NULL(fp); + fputs("- *.tmp\n!\nP *.log\n-e bogus\n", fp); + fclose(fp); + list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + EXPECT_TRUE(filter_rule_list_parse_append(list, "- keep.txt", NULL, NULL, err, sizeof(err))); + EXPECT_EQ_INT(list->count, 1); + exists = false; + EXPECT_FALSE(filter_dir_merge_append(list, tmpl, &spec, "sub", NULL, &exists, err, sizeof(err))); + EXPECT_TRUE(exists); + EXPECT_EQ_INT(list->count, 0); + filter_rule_list_free(list); + + unlink(path); + rmdir(tmpl); +} + static void test_filter_list_merge_file_still_supported() { char tmpl[] = "/tmp/fastsync_filter_XXXXXX"; EXPECT_TRUE(mkdtemp(tmpl) != NULL); @@ -430,6 +483,7 @@ void test_filter() { test_filter_list_accepts_merge_modifiers(); test_filter_list_accepts_supported_rules_and_modifiers(); test_filter_list_merge_file_still_supported(); + test_filter_merge_clear_then_owner(); test_filter_rule_parse_rejects_unsupported_and_keeps_supported(); test_filter_rules_apply_supported_modifiers(); test_filter_dir_rules_chain();