fix(filter): reject the x modifier in the --filter list parser

The standalone filter_rule_parse() already rejected the rsync xattr-name
'x' modifier, but the list parser used by --filter/-f silently dropped the
flag for merge/dir-merge rules (and relied on a second parse for plain
rules).  Reject it explicitly in filter_list_parse_append_depth() with the
same diagnostic, so '-x', 'merge,x' and 'dir-merge,x' all fail cleanly.

Also reject the unimplemented rsync merge modifiers 'e', 'n' and 'w'
instead of folding them into the pattern, which previously produced
misleading errors such as "could not read merge file 'n file'".  Only a
token made up solely of modifier characters is treated as a modifier run,
so glued patterns ('-newfile', '-e2e') and mixed tokens ("H,!secret")
keep their historical parsing.

Adds tests/test_filter.c with focused rejection and supported-syntax
cases.

CMakeLists.txt

@@ -226,6 +226,7 @@ set(TEST_SRCS
 	tests/test_file.c
 	tests/test_file_list.c
 	tests/test_file_sendfile.c
+	tests/test_filter.c
 	tests/test_format.c
 	tests/test_fuzz_smoke.c
 	tests/test_glob.c

src/shared/filter.c

@@ -172,14 +172,50 @@ 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'"). */
+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);
+}
+
+/* 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:
+ * 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) {
+  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))
+      return '\0';
+    if (is_unsupported_modifier_char(*q))
+      bad = *q;
+  }
+  return bad;
+}
+
 /* Parse "RULE[,MODIFIERS] [PATTERN]".  On success `kind`, `sides`,
  * `sides_explicit`, `negate`, `anchored_mod`, `perishable`, `xattr`,
  * `cvs_inject` and the pattern span (`pat_start`/`pat_len`, possibly 0 for
- * merge/clear) are filled.  Returns true on success. */
+ * merge/clear) are filled.  Returns true on success.
+ *
+ * On failure `*bad_mod` is set to the offending modifier character when the
+ * rule carried a modifier FastSync does not implement, and left '\0' for a
+ * generic syntax error so callers can emit a precise diagnostic. */
 static bool parse_rule_syntax(const char* text, RuleKind* kind, unsigned* sides,
                               bool* sides_explicit, bool* negate, bool* anchored_mod,
                               bool* perishable, bool* xattr, bool* cvs_inject,
-                              const char** pat_start, size_t* pat_len) {
+                              const char** pat_start, size_t* pat_len, char* bad_mod) {
   const char* p = text;
   *sides = FILTER_SIDE_SENDER | FILTER_SIDE_RECEIVER;
   *sides_explicit = false;
@@ -190,6 +226,7 @@ static bool parse_rule_syntax(const char* text, RuleKind* kind, unsigned* sides,
   *cvs_inject = false;
   *pat_start = NULL;
   *pat_len = 0;
+  *bad_mod = '\0';
 
   bool is_short = false;
   if (short_rule_char(*p, kind)) {
@@ -210,6 +247,14 @@ static bool parse_rule_syntax(const char* text, RuleKind* kind, unsigned* sides,
   /* Modifiers: long names require a comma; short names may attach directly.
      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);
+  } else if (is_short) {
+    *bad_mod = unsupported_modifier_in_token(p);
+  }
+  if (*bad_mod != '\0')
+    return false;
+
   const char* mod_start = p;
   const char* mod_end = p;
   if (*p == ',') {
@@ -290,9 +335,13 @@ FilterRule* filter_rule_parse(const char* line, const FilterParseOptions* opts,
   bool sides_explicit, negate, anchored_mod, perishable, xattr, cvs_inject;
   const char* pat;
   size_t pat_len;
+  char bad_mod;
   if (!parse_rule_syntax(p, &kind, &sides, &sides_explicit, &negate, &anchored_mod, &perishable,
-                         &xattr, &cvs_inject, &pat, &pat_len)) {
-    filter_set_error(err, err_size, "unrecognized filter rule syntax");
+                         &xattr, &cvs_inject, &pat, &pat_len, &bad_mod)) {
+    if (bad_mod != '\0')
+      filter_set_error(err, err_size, "unsupported filter modifier '%c'", bad_mod);
+    else
+      filter_set_error(err, err_size, "unrecognized filter rule syntax");
     return NULL;
   }
   if (cvs_inject) {
@@ -401,7 +450,6 @@ FilterRule* filter_rule_parse(const char* line, const FilterParseOptions* opts,
   rule->dir_only = dir_only;
   rule->negate = negate;
   rule->perishable = perishable;
-  (void)xattr; /* xattr-name rules never match file/dir names; accepted/ignored */
   return rule;
 }
 
@@ -530,16 +578,27 @@ static bool filter_list_parse_append_depth(FilterRuleList* list, const char* lin
   bool sides_explicit, negate, anchored_mod, perishable, xattr, cvs_inject;
   const char* pat;
   size_t pat_len;
+  char bad_mod;
   if (!parse_rule_syntax(p, &kind, &sides, &sides_explicit, &negate, &anchored_mod, &perishable,
-                         &xattr, &cvs_inject, &pat, &pat_len)) {
-    filter_set_error(err, err_size, "unrecognized filter rule syntax: %s", p);
+                         &xattr, &cvs_inject, &pat, &pat_len, &bad_mod)) {
+    if (bad_mod != '\0')
+      filter_set_error(err, err_size, "unsupported filter modifier '%c': %s", bad_mod, p);
+    else
+      filter_set_error(err, err_size, "unrecognized filter rule syntax: %s", p);
     return false;
   }
   (void)sides_explicit;
   (void)negate;
   (void)anchored_mod;
   (void)perishable;
-  (void)xattr;
+
+  /* xattr-name rules are not implemented; reject them everywhere (including on
+   * merge/dir-merge, where the flag would otherwise be silently dropped) with
+   * the same diagnostic the standalone parser gives. */
+  if (xattr) {
+    filter_set_error(err, err_size, "xattr-name filter rules (the x modifier) are not supported");
+    return false;
+  }
 
   if (cvs_inject) {
     /* "C" injects the CVS defaults in place; no pattern is expected. */

src/shared/filter.h

@@ -23,7 +23,9 @@
  *   dir-merge/: per-directory merge file (registered for the scanner)
  *   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, 'x' xattr name rule.
+ * '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.
  * A trailing '/' makes a pattern match directories only.  A leading '/' anchors
  * the pattern to its owner directory.
  */

tests/runner.c

@@ -16,6 +16,7 @@
 #include "test_file.h"
 #include "test_file_list.h"
 #include "test_file_sendfile.h"
+#include "test_filter.h"
 #include "test_format.h"
 #include "test_fuzz_smoke.h"
 #include "test_glob.h"
@@ -80,6 +81,7 @@ int main() {
   RUN_TEST(test_receiver_timeout);
   RUN_TEST(test_metadata);
   RUN_TEST(test_glob);
+  RUN_TEST(test_filter);
   RUN_TEST(test_iconv);
   RUN_TEST(test_file);
   RUN_TEST(test_file_list);

tests/test_filter.c

@@ -0,0 +1,222 @@
+#include "test_filter.h"
+#include "filter.h"
+#include "test_utils.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+/* Every `-f`/`--filter` rule string is validated through the list parser, so
+ * the list parser must reject the modifiers the standalone parser rejects
+ * rather than silently folding them into a pattern. */
+
+static void test_filter_list_rejects_xattr_modifier() {
+  static const char* const rules[] = {
+      "-x user.foo",             /* short exclude + x */
+      "exclude,x user.foo",      /* long exclude + x */
+      "+x user.foo",             /* include + x */
+      "hide,x *.tmp",            /* hide + x */
+      "dir-merge,x .rules",      /* x must not be dropped on dir-merge */
+      "merge,x /tmp/nonexistent" /* x must not be dropped on merge */
+  };
+  for (size_t i = 0; i < sizeof(rules) / sizeof(rules[0]); i++) {
+    FilterRuleList* list = filter_rule_list_create();
+    EXPECT_NOT_NULL(list);
+    char err[256] = "";
+    bool ok = filter_rule_list_parse_append(list, rules[i], NULL, NULL, err, sizeof(err));
+    EXPECT_FALSE(ok);
+    EXPECT_TRUE(strstr(err, "xattr") != NULL);
+    filter_rule_list_free(list);
+  }
+
+  /* A bare "x" is not a rule at all: rejected as generic bad syntax. */
+  FilterRuleList* list = filter_rule_list_create();
+  EXPECT_NOT_NULL(list);
+  char err[256] = "";
+  EXPECT_FALSE(filter_rule_list_parse_append(list, "x user.foo", NULL, NULL, err, sizeof(err)));
+  EXPECT_TRUE(err[0] != '\0');
+  filter_rule_list_free(list);
+}
+
+static void test_filter_list_rejects_unsupported_modifiers() {
+  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",
+  };
+  for (size_t i = 0; i < sizeof(rules) / sizeof(rules[0]); i++) {
+    FilterRuleList* list = filter_rule_list_create();
+    EXPECT_NOT_NULL(list);
+    char err[256] = "";
+    bool ok = filter_rule_list_parse_append(list, rules[i], NULL, NULL, err, sizeof(err));
+    EXPECT_FALSE(ok);
+    EXPECT_TRUE(strstr(err, "unsupported filter modifier") != NULL);
+    filter_rule_list_free(list);
+  }
+}
+
+static void test_filter_list_accepts_supported_rules_and_modifiers() {
+  static const char* const rules[] = {
+      "- *.tmp",  "+ /a.txt",         "-s foo",     "-r foo",     "-p foo",
+      "-! *.o",   "-/ foo",           "hide *.tmp", "show *.txt", "protect *.bak",
+      "risk *.o", "dir-merge .rules", "-C",
+  };
+  for (size_t i = 0; i < sizeof(rules) / sizeof(rules[0]); i++) {
+    FilterRuleList* list = filter_rule_list_create();
+    EXPECT_NOT_NULL(list);
+    char err[256] = "";
+    bool ok = filter_rule_list_parse_append(list, rules[i], NULL, NULL, err, sizeof(err));
+    if (!ok)
+      printf("    rule '%s' errored: %s\n", rules[i], err);
+    EXPECT_TRUE(ok);
+    filter_rule_list_free(list);
+  }
+
+  /* A glued word is a pattern, not a modifier run (no separator). */
+  {
+    FilterRuleList* list = filter_rule_list_create();
+    char err[128] = "";
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "-newfile", NULL, NULL, err, sizeof(err)));
+    EXPECT_EQ_INT(list->count, 1);
+    EXPECT_EQ_STR(list->items[0]->pattern, "newfile");
+    filter_rule_list_free(list);
+
+    list = filter_rule_list_create();
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "-e2e", NULL, NULL, err, sizeof(err)));
+    EXPECT_EQ_INT(list->count, 1);
+    EXPECT_EQ_STR(list->items[0]->pattern, "e2e");
+    filter_rule_list_free(list);
+
+    list = filter_rule_list_create();
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "-*.o", NULL, NULL, err, sizeof(err)));
+    EXPECT_EQ_INT(list->count, 1);
+    EXPECT_EQ_STR(list->items[0]->pattern, "*.o");
+    filter_rule_list_free(list);
+
+    /* A comma with no modifier still treats the rest as the pattern. */
+    list = filter_rule_list_create();
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "exclude,foo", NULL, NULL, err, sizeof(err)));
+    EXPECT_EQ_INT(list->count, 1);
+    EXPECT_EQ_STR(list->items[0]->pattern, "foo");
+    filter_rule_list_free(list);
+  }
+
+  /* `!` clears the list. */
+  {
+    FilterRuleList* list = filter_rule_list_create();
+    char err[128] = "";
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "- *.tmp", NULL, NULL, err, sizeof(err)));
+    EXPECT_EQ_INT(list->count, 1);
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "!", NULL, NULL, err, sizeof(err)));
+    EXPECT_EQ_INT(list->count, 0);
+    filter_rule_list_free(list);
+  }
+
+  /* -C injects the CVS defaults. */
+  {
+    FilterRuleList* list = filter_rule_list_create();
+    char err[128] = "";
+    EXPECT_TRUE(filter_rule_list_parse_append(list, "-C", NULL, NULL, err, sizeof(err)));
+    EXPECT_TRUE(list->count > 0);
+    filter_rule_list_free(list);
+  }
+}
+
+static void test_filter_list_merge_file_still_supported() {
+  char tmpl[] = "/tmp/fastsync_filter_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);
+
+  FilterRuleList* list = filter_rule_list_create();
+  char err[256] = "";
+  char rule[600];
+  snprintf(rule, sizeof(rule), "merge %s", path);
+  EXPECT_TRUE(filter_rule_list_parse_append(list, rule, NULL, NULL, err, sizeof(err)));
+  EXPECT_EQ_INT(list->count, 1);
+  filter_rule_list_free(list);
+
+  /* The same merge with the x modifier is rejected, not silently read. */
+  list = filter_rule_list_create();
+  snprintf(rule, sizeof(rule), "merge,x %s", path);
+  EXPECT_FALSE(filter_rule_list_parse_append(list, rule, NULL, NULL, err, sizeof(err)));
+  EXPECT_TRUE(strstr(err, "xattr") != NULL);
+  filter_rule_list_free(list);
+
+  unlink(path);
+  rmdir(tmpl);
+}
+
+static void test_filter_rule_parse_rejects_unsupported_and_keeps_supported() {
+  char err[256] = "";
+
+  EXPECT_NULL(filter_rule_parse("-x user.foo", NULL, err, sizeof(err)));
+  EXPECT_TRUE(strstr(err, "xattr") != NULL);
+
+  EXPECT_NULL(filter_rule_parse("-e foo", NULL, err, sizeof(err)));
+  EXPECT_TRUE(strstr(err, "unsupported filter modifier") != NULL);
+
+  FilterRule* rule = filter_rule_parse("- *.tmp", NULL, err, sizeof(err));
+  EXPECT_NOT_NULL(rule);
+  EXPECT_EQ_STR(rule->pattern, "*.tmp");
+  filter_rule_free(rule);
+
+  rule = filter_rule_parse("-newfile", NULL, err, sizeof(err));
+  EXPECT_NOT_NULL(rule);
+  EXPECT_EQ_STR(rule->pattern, "newfile");
+  filter_rule_free(rule);
+}
+
+static void test_filter_rules_apply_supported_modifiers() {
+  /* exclude */
+  {
+    const char* texts[] = {"- *.tmp"};
+    FilterRuleList* list = filter_base_build(texts, 1, false, false, NULL, 0);
+    EXPECT_NOT_NULL(list);
+    EXPECT_EQ_INT(filter_rules_apply(list, "b.tmp", "b.tmp", false), FILTER_ACTION_EXCLUDE);
+    EXPECT_EQ_INT(filter_rules_apply(list, "a.txt", "a.txt", false), FILTER_ACTION_NONE);
+    filter_rule_list_free(list);
+  }
+  /* anchored include then exclude-all */
+  {
+    const char* texts[] = {"+ /a.txt", "- *"};
+    FilterRuleList* list = filter_base_build(texts, 2, false, false, NULL, 0);
+    EXPECT_NOT_NULL(list);
+    EXPECT_EQ_INT(filter_rules_apply(list, "a.txt", "a.txt", false), FILTER_ACTION_INCLUDE);
+    EXPECT_EQ_INT(filter_rules_apply(list, "b.txt", "b.txt", false), FILTER_ACTION_EXCLUDE);
+    filter_rule_list_free(list);
+  }
+  /* negate */
+  {
+    const char* texts[] = {"-! *.o"};
+    FilterRuleList* list = filter_base_build(texts, 1, false, false, NULL, 0);
+    EXPECT_NOT_NULL(list);
+    EXPECT_EQ_INT(filter_rules_apply(list, "foo.c", "foo.c", false), FILTER_ACTION_EXCLUDE);
+    EXPECT_EQ_INT(filter_rules_apply(list, "foo.o", "foo.o", false), FILTER_ACTION_NONE);
+    filter_rule_list_free(list);
+  }
+  /* dir-only trailing slash */
+  {
+    const char* texts[] = {"+ dir/", "- *"};
+    FilterRuleList* list = filter_base_build(texts, 2, false, false, NULL, 0);
+    EXPECT_NOT_NULL(list);
+    EXPECT_EQ_INT(filter_rules_apply(list, "dir", "dir", true), FILTER_ACTION_INCLUDE);
+    EXPECT_EQ_INT(filter_rules_apply(list, "dir", "dir", false), FILTER_ACTION_EXCLUDE);
+    filter_rule_list_free(list);
+  }
+}
+
+void test_filter() {
+  test_filter_list_rejects_xattr_modifier();
+  test_filter_list_rejects_unsupported_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();
+  test_filter_rules_apply_supported_modifiers();
+}

tests/test_filter.h

@@ -0,0 +1,6 @@
+#ifndef TEST_FILTER_H
+#define TEST_FILTER_H
+
+void test_filter(void);
+
+#endif