From 06c4026b74992e42c482460e66b22ca150171c00 Mon Sep 17 00:00:00 2001 From: TapTap Date: Mon, 21 Sep 2026 22:01:44 +0200 Subject: [PATCH] fix(filter): accept e/n/w/- merge modifiers on merge/dir-merge rules The earlier modifier-rejection change rejected e/n/w on all rules, but rsync 3.4.1 accepts them (plus the '-' merge-only modifier) on merge and dir-merge rules. Restrict the rejection to non-merge rules and consume the merge-file modifiers (e/n/w/-) so they no longer leak into the merge filename. - is_merge_rule()/is_merge_modifier_char() gate the merge-only modifiers. - scan vs consume sets: e/n/w still count as modifier-run chars on every rule (pure tokens like -new/-press stay rejected), but are only consumed on merge rules, preserving mixed-token parsing such as H,!secret -> ecret. - '-' is accepted/consumed only on merge/dir-merge (e.g. dir-merge,- .rules). - x remains rejected everywhere with its dedicated message. - e/n/w/- semantics remain unimplemented and are documented as accepted-but- ignored in filter.h. Tests: split the merge forms out of the rejection test into a new acceptance test asserting the merge file is read and dir_merge_names keeps the modifier- free basename; non-merge pure-modifier forms still rejected. --- src/shared/filter.c | 54 ++++++++++++++++++++++++++----------- src/shared/filter.h | 8 ++++-- tests/test_filter.c | 66 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 110 insertions(+), 18 deletions(-) diff --git a/src/shared/filter.c b/src/shared/filter.c index 4bfd462..56cc6ed 100644 --- a/src/shared/filter.c +++ b/src/shared/filter.c @@ -162,33 +162,57 @@ static bool is_modifier_char(char c) { return c == 's' || c == 'r' || c == 'p' || c == 'x' || c == '/' || c == '!' || c == 'C'; } -/* Modifiers rsync defines but FastSync does not implement. They must still be - * consumed as part of the modifier run so they are rejected explicitly instead - * of leaking into the pattern (which produced misleading failures such as - * "could not read merge file 'n file'"). */ +/* merge/dir-merge rules are the only rules rsync accepts the merge-file + * modifiers on. */ +static bool is_merge_rule(RuleKind kind) { + return kind == RULE_KIND_MERGE || kind == RULE_KIND_DIR_MERGE; +} + +/* Merge-file modifiers rsync defines but FastSync does not implement: + * 'e' exclude the merge file itself, 'n' do not inherit the merge file, 'w' + * word-split the merge file. They are recognized as part of a modifier run on + * every rule (so a pure e/n/w token is rejected rather than folded into the + * pattern), but are accepted (and ignored) only on merge/dir-merge rules. */ static bool is_unsupported_modifier_char(char c) { return c == 'e' || c == 'n' || c == 'w'; } -/* Characters that are part of a modifier run, whether supported or not. */ -static bool is_modifier_scan_char(char c) { - return is_modifier_char(c) || is_unsupported_modifier_char(c); +/* Merge-file modifiers rsync accepts on merge/dir-merge rules: 'e', 'n', 'w' + * and '-' (do not transfer the merge file). */ +static bool is_merge_modifier_char(char c) { + return c == 'e' || c == 'n' || c == 'w' || c == '-'; +} + +/* Characters that count as part of a modifier run for `kind` when deciding + * whether a token is a pure modifier run. e/n/w count on every rule so that a + * pure e/n/w token is rejected on non-merge rules; '-' only on merge rules. */ +static bool is_modifier_scan_char(char c, RuleKind kind) { + return is_modifier_char(c) || is_unsupported_modifier_char(c) || + (is_merge_rule(kind) && is_merge_modifier_char(c)); +} + +/* Characters actually consumed as modifiers for `kind`. The merge-file + * modifiers are consumed only on merge/dir-merge rules; elsewhere e/n/w fall + * through to the pattern (so mixed tokens such as "H,!secret" keep their + * historical "ecret" pattern). */ +static bool is_consumed_modifier_char(char c, RuleKind kind) { + return is_modifier_char(c) || (is_merge_rule(kind) && is_merge_modifier_char(c)); } /* Inspect the token that follows a rule name (up to the first space/underscore * or the end). If the token is composed *solely* of modifier characters and - * includes one FastSync does not implement, it is unambiguously a modifier run: + * includes one that is invalid for `kind`, it is unambiguously a modifier run: * return that character so the caller can reject it. A token that contains any * non-modifier character is a pattern (e.g. "-newfile") and returns '\0', which * keeps the historical parsing of mixed tokens such as "H,!secret" intact. */ -static char unsupported_modifier_in_token(const char* tok) { +static char unsupported_modifier_in_token(const char* tok, RuleKind kind) { if (*tok == '\0' || *tok == ' ' || *tok == '_') return '\0'; char bad = '\0'; for (const char* q = tok; *q != '\0' && *q != ' ' && *q != '_'; q++) { - if (!is_modifier_scan_char(*q)) + if (!is_modifier_scan_char(*q, kind)) return '\0'; - if (is_unsupported_modifier_char(*q)) + if (!is_merge_rule(kind) && is_unsupported_modifier_char(*q)) bad = *q; } return bad; @@ -238,9 +262,9 @@ static bool parse_rule_syntax(const char* text, RuleKind* kind, unsigned* sides, Only commit a modifier run that terminates at a separator or the end, so a pattern such as "*.tmp" written as "-*.tmp" is not mistaken for modifiers. */ if (*p == ',') { - *bad_mod = unsupported_modifier_in_token(p + 1); + *bad_mod = unsupported_modifier_in_token(p + 1, *kind); } else if (is_short) { - *bad_mod = unsupported_modifier_in_token(p); + *bad_mod = unsupported_modifier_in_token(p, *kind); } if (*bad_mod != '\0') return false; @@ -250,12 +274,12 @@ static bool parse_rule_syntax(const char* text, RuleKind* kind, unsigned* sides, if (*p == ',') { p++; mod_start = p; - while (is_modifier_char(*p)) + while (is_consumed_modifier_char(*p, *kind)) p++; mod_end = p; } else if (is_short) { const char* scan = p; - while (is_modifier_char(*scan)) + while (is_consumed_modifier_char(*scan, *kind)) scan++; if (*scan == '\0' || *scan == ' ' || *scan == '_') { mod_start = p; diff --git a/src/shared/filter.h b/src/shared/filter.h index bc9e9ca..c651afc 100644 --- a/src/shared/filter.h +++ b/src/shared/filter.h @@ -24,8 +24,12 @@ * clear/! clear the current rule list (takes no argument) * Modifiers: '/' absolute anchor, '!' negate match, 'C' inject CVS defaults, * 's' sender side, 'r' receiver side, 'p' perishable. The rsync 'x' - * (xattr-name) modifier and the merge-only 'e'/'n'/'w' modifiers are not - * implemented and are rejected explicitly. + * (xattr-name) modifier is not implemented and is rejected explicitly + * everywhere. The merge-file modifiers 'e' (exclude the merge file itself), + * 'n' (do not inherit the merge file), 'w' (word-split the merge file) and '-' + * (do not transfer the merge file) are accepted and consumed only on merge/ + * dir-merge rules (rejected on every other rule, matching rsync); their + * semantics are not implemented and they are otherwise ignored. * A trailing '/' makes a pattern match directories only. A leading '/' anchors * the pattern to its owner directory. */ diff --git a/tests/test_filter.c b/tests/test_filter.c index 4036cbd..ae426f9 100644 --- a/tests/test_filter.c +++ b/tests/test_filter.c @@ -40,11 +40,17 @@ static void test_filter_list_rejects_xattr_modifier() { } static void test_filter_list_rejects_unsupported_modifiers() { + /* The merge-file modifiers e/n/w/- are invalid on every non-merge rule; a + token made up solely of modifier characters is a modifier run, so it must + be rejected rather than folded into the pattern. */ static const char* const rules[] = { "-e foo", /* e: merge-only in rsync */ "-n foo", /* n: merge-only in rsync */ "-w foo", /* w: merge-only in rsync */ - "merge,n /tmp/x", ".e /tmp/x", "dir-merge,e .rules", "exclude,w foo", + "-new", /* pure modifier letters (n/e/w) */ + "-press", /* pure modifier letters (p/r/e/s) */ + "exclude,w foo", "exclude,e foo", "exclude,n foo", + "hide,w foo", "protect,n foo", "risk,e foo", }; for (size_t i = 0; i < sizeof(rules) / sizeof(rules[0]); i++) { FilterRuleList* list = filter_rule_list_create(); @@ -57,6 +63,63 @@ static void test_filter_list_rejects_unsupported_modifiers() { } } +/* rsync accepts the merge-file modifiers e/n/w/- on merge and dir-merge rules. + * They must be consumed so they never leak into the merge filename. */ +static void test_filter_list_accepts_merge_modifiers() { + char tmpl[] = "/tmp/fastsync_filter_mmod_XXXXXX"; + EXPECT_TRUE(mkdtemp(tmpl) != NULL); + char path[512]; + snprintf(path, sizeof(path), "%s/rules", tmpl); + FILE* fp = fopen(path, "w"); + EXPECT_NOT_NULL(fp); + fputs("- *.tmp\n", fp); + fclose(fp); + + /* merge with e/n/w/- consumes the modifiers and reads the right file. */ + static const char* const fmts[] = { + "merge,e %s", "merge,n %s", "merge,w %s", "merge,- %s", ".e %s", ".- %s", + }; + for (size_t i = 0; i < sizeof(fmts) / sizeof(fmts[0]); i++) { + FilterRuleList* list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + char rule[600]; + char err[256] = ""; + snprintf(rule, sizeof(rule), fmts[i], path); + bool ok = filter_rule_list_parse_append(list, rule, NULL, NULL, err, sizeof(err)); + if (!ok) + printf(" merge rule '%s' errored: %s\n", rule, err); + EXPECT_TRUE(ok); + EXPECT_EQ_INT(list->count, 1); + EXPECT_EQ_STR(list->items[0]->pattern, "*.tmp"); + filter_rule_list_free(list); + } + + /* dir-merge with e/n/w/- registers the basename without the modifiers. */ + static const struct { + const char* rule; + const char* want; + } drules[] = { + {"dir-merge,e .rules", ".rules"}, {"dir-merge,n .rules", ".rules"}, + {"dir-merge,w .rules", ".rules"}, {"dir-merge,- .rules", ".rules"}, + {":e .rules", ".rules"}, {":- .rules", ".rules"}, + }; + for (size_t i = 0; i < sizeof(drules) / sizeof(drules[0]); i++) { + FilterRuleList* list = filter_rule_list_create(); + EXPECT_NOT_NULL(list); + char err[256] = ""; + bool ok = filter_rule_list_parse_append(list, drules[i].rule, NULL, NULL, err, sizeof(err)); + if (!ok) + printf(" dir-merge rule '%s' errored: %s\n", drules[i].rule, err); + EXPECT_TRUE(ok); + EXPECT_EQ_INT(list->dir_merge_count, 1); + EXPECT_EQ_STR(list->dir_merge_names[0], drules[i].want); + filter_rule_list_free(list); + } + + unlink(path); + rmdir(tmpl); +} + static void test_filter_list_accepts_supported_rules_and_modifiers() { static const char* const rules[] = { "- *.tmp", "+ /a.txt", "-s foo", "-r foo", "-p foo", @@ -223,6 +286,7 @@ static void test_filter_rules_apply_supported_modifiers() { void test_filter() { test_filter_list_rejects_xattr_modifier(); test_filter_list_rejects_unsupported_modifiers(); + test_filter_list_accepts_merge_modifiers(); test_filter_list_accepts_supported_rules_and_modifiers(); test_filter_list_merge_file_still_supported(); test_filter_rule_parse_rejects_unsupported_and_keeps_supported();