fix: filter precedence, files-from errors, NUL/CRLF and filter-rule rejections

Address an independent c-review of the files-from/filter feature: - .rsync-filter precedence now matches rsync: evaluate the innermost (current) directory's rules first, then ancestors, then the command-line base (--filter/-C), so a deeper file's '+' can re-include what a shallower '-' excluded (regression tests in both scan modes; single-thread and -m). - --files-from: a listed entry missing on disk and an empty list are now hard errors surfaced pre-transfer in send_files, send_files_multithreaded, dry-run and --list-only; '.' (whole tree) and empty listed dirs stay valid. - scanner_path_relative now handles a transfer root of / (previously the scanner aborted on children of /). - Reject unsupported rsync filter syntax explicitly (no silent no-ops): +/- modifiers other than '/' (! C s r p x) and rules beginning with ':'/'.' /'!' (merge/dir-merge/list-clear shorthands). Docs updated. - -0/--from0 NUL mode preserves entry bytes (no CR/LF trimming); only newline mode trims. Absolute-entry error message no longer includes the newline. - --no-from0/--no-cvs-exclude registered as negatable booleans. - RSYNC_COMPAT rows updated for the precedence, rejection list, NUL-mode detail and the documented O(entries x files) scalability bound of the allow-set (Summary unchanged: 62/3/5/1/76 = 147).
2026-09-06 14:16:12 +02:00
parent f4c15a7b78
commit 7f2180ef90
11 changed files with 339 additions and 29 deletions
@@ -1489,6 +1489,43 @@ class TestFilesFrom:
        assert result.returncode != 0, "missing --files-from file must be rejected"
        assert "--files-from" in result.stderr

+    def test_files_from_missing_entry_rejected(self):
+        source = self._make_source("ff_missing_src")
+        dest = os.path.join(TEST_DATA_DIR, "ff_missing_dst")
+        clean_dir(dest)
+        lst = self._write_list(b"top.txt\nno_such_file.txt\n")
+        result, _ = run_client(source, dest, flags=["--files-from", lst])
+        assert result.returncode != 0, "a listed-but-missing file must be a hard error"
+        assert "not found" in result.stderr
+
+    def test_files_from_missing_entry_rejected_dry_run(self):
+        source = self._make_source("ff_missing_dry_src")
+        dest = os.path.join(TEST_DATA_DIR, "ff_missing_dry_dst")
+        clean_dir(dest)
+        lst = self._write_list(b"gone.bin\n")
+        result, _ = run_client(source, dest, flags=["--files-from", lst, "--dry-run"])
+        assert result.returncode != 0, "dry-run must also reject a listed-but-missing file"
+        assert "not found" in result.stderr
+
+    def test_files_from_empty_list_rejected(self):
+        source = self._make_source("ff_empty_src")
+        dest = os.path.join(TEST_DATA_DIR, "ff_empty_dst")
+        clean_dir(dest)
+        lst = self._write_list(b"")
+        result, _ = run_client(source, dest, flags=["--files-from", lst])
+        assert result.returncode != 0, "an empty --files-from list must be rejected"
+        assert "no entries" in result.stderr
+
+    def test_files_from_empty_directory_listed_is_not_an_error(self, shared_server):
+        source = self._make_source("ff_emptydir_src")
+        os.makedirs(os.path.join(source, "emptydir"), exist_ok=True)
+        dest = os.path.join(TEST_DATA_DIR, "ff_emptydir_dst")
+        clean_dir(dest)
+        lst = self._write_list(b"emptydir\n")
+        result, _ = run_client(source, dest, flags=["--files-from", lst],
+                               port=shared_server.port)
+        assert result.returncode == 0, f"empty listed directory errored: {result.stderr[:200]}"
+
    def test_files_from_delete_deletes_unlisted(self):
        source = self._make_source("ff_delete_src")
        dest = os.path.join(TEST_DATA_DIR, "ff_delete_dst")
@@ -1620,6 +1657,36 @@ class TestFilters:
        assert not os.path.exists(os.path.join(received, ".rsync-filter")), \
            ".rsync-filter must not be transferred"

+    @pytest.mark.parametrize("mt", [False, True])
+    def test_per_dir_filter_deeper_file_overrides_outer(self, shared_server, mt):
+        source = os.path.join(TEST_DATA_DIR, "filter_ovr_src")
+        clean_dir(source)
+        entries = {
+            "top.tmp": b"outer excludes me\n",
+            "keep.txt": b"kept\n",
+            "sub/inside.tmp": b"inner re-includes me\n",
+            ".rsync-filter": b"- *.tmp\n",
+            "sub/.rsync-filter": b"+ *.tmp\n",
+        }
+        for rel, content in entries.items():
+            full = os.path.join(source, rel)
+            os.makedirs(os.path.dirname(full), exist_ok=True)
+            with open(full, "wb") as fh:
+                fh.write(content)
+        dest = os.path.join(TEST_DATA_DIR, "filter_ovr_dst")
+        clean_dir(dest)
+        flags = ["-F"] + (["-m"] if mt else [])
+        result, _ = run_client(source, dest, flags=flags, port=shared_server.port)
+        assert result.returncode == 0, f"-F override sync failed: {result.stderr[:200]}"
+        received = get_dest_received_dir(dest, source)
+        assert os.path.isfile(os.path.join(received, "keep.txt"))
+        assert os.path.isfile(os.path.join(received, "sub", "inside.tmp")), \
+            "inner + *.tmp must re-include what the root - *.tmp excluded"
+        assert not os.path.exists(os.path.join(received, "top.tmp")), \
+            "outer - *.tmp still excludes root-level tmp files"
+        assert not os.path.exists(os.path.join(received, ".rsync-filter"))
+        assert not os.path.exists(os.path.join(received, "sub", ".rsync-filter"))
+
    def test_filter_leaves_default_behavior_unchanged(self, shared_server):
        source = self._make_tree("filter_default_src")
        dest = os.path.join(TEST_DATA_DIR, "filter_default_dst")
@@ -1289,6 +1289,29 @@ static void test_parse_args_filter_rules() {
  char* missing_argv[] = {"fastsync", "/src", "/dst", "--filter"};
  EXPECT_EQ_INT(parse_args(cfg, 4, missing_argv, positional_args, &positional_count), -1);
  config_delete(cfg);
+
+  /* rsync shorthands/modifiers we do not support are rejected instead of being
+   * silently parsed as literal patterns. */
+  static const char* const unsupported[] = {
+      ": .rsync-filter", ". /tmp/rules", "-s foo", "-p bar", "-C", "-! *.o", "!",
+  };
+  for (size_t i = 0; i < sizeof(unsupported) / sizeof(unsupported[0]); i++) {
+    cfg = config_create();
+    positional_count = 0;
+    char* rule_argv[] = {"fastsync", "--filter", (char*)unsupported[i], "/src", "/dst"};
+    EXPECT_EQ_INT(parse_args(cfg, 5, rule_argv, positional_args, &positional_count), -1);
+    config_delete(cfg);
+  }
+
+  /* Supported spellings still parse: space- or slash-separated, attached
+   * wildcards, and anchored rules. */
+  cfg = config_create();
+  positional_count = 0;
+  char* ok_argv[] = {"fastsync",         "--filter=-*.o", "--filter=- /foo",
+                     "--filter=+ /bar/", "/src",          "/dst"};
+  EXPECT_EQ_INT(parse_args(cfg, 6, ok_argv, positional_args, &positional_count), 0);
+  EXPECT_EQ_INT(cfg->filters->size, 3);
+  config_delete(cfg);
 }

 /* -0/--from0, -C/--cvs-exclude and -F wire into their config flags. */
@@ -1316,6 +1339,19 @@ static void test_parse_args_from0_cvs_filter_file_flags() {
    EXPECT_EQ_INT(cfg->per_dir_filter, cases[i].per_dir);
    config_delete(cfg);
  }
+
+  /* The plain booleans are negatable (--no-* simply clears the flag). */
+  static const char* const on[][2] = {{"--from0", "--no-from0"}, {"-C", "--no-cvs-exclude"}};
+  for (size_t i = 0; i < sizeof(on) / sizeof(on[0]); i++) {
+    Config* cfg = config_create();
+    int positional_args[2];
+    int positional_count = 0;
+    char* argv[] = {"fastsync", (char*)on[i][0], (char*)on[i][1], "/src", "/dst"};
+    EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0);
+    EXPECT_FALSE(cfg->from0);
+    EXPECT_FALSE(cfg->cvs_exclude);
+    config_delete(cfg);
+  }
 }

 static void write_file_bytes(const char* path, const char* bytes, size_t len) {
@@ -1347,7 +1383,8 @@ static void test_parse_args_files_from() {
  config_delete(cfg);
  remove(list_path);

-  /* -0 switches the separator to NUL regardless of argument order. */
+  /* -0 switches the separator to NUL regardless of argument order, and NUL
+   * mode preserves entry bytes exactly (a trailing CR/LF is part of the name). */
  write_file_bytes(list_path, "x.txt\0y/z.bin\0", 14);
  cfg = config_create();
  positional_count = 0;
@@ -1365,6 +1402,21 @@ static void test_parse_args_files_from() {
  config_delete(cfg);
  remove(list_path);

+  write_file_bytes(list_path, "crlf\n\0tail\0", 11);
+  cfg = config_create();
+  positional_count = 0;
+  char* nul_nl_argv[] = {"fastsync",
+                         "--files-from="
+                         "cli_files_from_list.txt",
+                         "-0", "/src", "/dst"};
+  EXPECT_EQ_INT(parse_args(cfg, 5, nul_nl_argv, positional_args, &positional_count), 0);
+  set = (FileListSet*)cfg->files_from_set;
+  EXPECT_NOT_NULL(set);
+  EXPECT_TRUE(file_list_affects(set, "crlf\n"));
+  EXPECT_TRUE(file_list_affects(set, "tail"));
+  config_delete(cfg);
+  remove(list_path);
+
  /* A missing list file is a hard parse-time error. */
  cfg = config_create();
  positional_count = 0;
@@ -995,6 +995,89 @@ static void test_per_dir_filter(bool parallel) {
  rmdir(root);
 }

+/* scanner_path_relative maps an on-disk path to its transfer-relative path,
+ * including the "/" transfer-root edge case (regression: children of "/" used
+ * to abort the scan because the suffix was mis-read). */
+static void test_scanner_path_relative() {
+  char* rel = NULL;
+
+  rel = scanner_path_relative("/", "/");
+  EXPECT_NOT_NULL(rel);
+  EXPECT_EQ_STR(rel, "");
+  free(rel);
+
+  rel = scanner_path_relative("/", "/etc");
+  EXPECT_NOT_NULL(rel);
+  EXPECT_EQ_STR(rel, "etc");
+  free(rel);
+
+  rel = scanner_path_relative("/", "/etc/passwd");
+  EXPECT_NOT_NULL(rel);
+  EXPECT_EQ_STR(rel, "etc/passwd");
+  free(rel);
+
+  /* Normal roots: with and without a trailing slash on the root. */
+  rel = scanner_path_relative("/tmp/foo", "/tmp/foo");
+  EXPECT_NOT_NULL(rel);
+  EXPECT_EQ_STR(rel, "");
+  free(rel);
+
+  rel = scanner_path_relative("/tmp/foo", "/tmp/foo/bar");
+  EXPECT_NOT_NULL(rel);
+  EXPECT_EQ_STR(rel, "bar");
+  free(rel);
+
+  rel = scanner_path_relative("/tmp/foo/", "/tmp/foo/bar/baz.txt");
+  EXPECT_NOT_NULL(rel);
+  EXPECT_EQ_STR(rel, "bar/baz.txt");
+  free(rel);
+
+  /* A path outside the root maps to NULL. */
+  EXPECT_NULL(scanner_path_relative("/tmp/foo", "/tmp"));
+  EXPECT_NULL(scanner_path_relative("/tmp/foo", "/tmp/foobar"));
+}
+
+/* rsync precedence: a deeper .rsync-filter overrides a shallower one, so an
+ * inner "+ *.tmp" re-includes what the outer "- *.tmp" excluded. */
+static void test_per_dir_filter_override(bool parallel) {
+  const char* root = "test_scan_perdir_ovr";
+  const char* sub = "test_scan_perdir_ovr/sub";
+  EXPECT_EQ_INT(mkdir(root, 0755), 0);
+  EXPECT_EQ_INT(mkdir(sub, 0755), 0);
+  create_test_file("test_scan_perdir_ovr/.rsync-filter", "- *.tmp\n");
+  create_test_file("test_scan_perdir_ovr/sub/.rsync-filter", "+ *.tmp\n");
+  create_test_file("test_scan_perdir_ovr/top.tmp", "x");
+  create_test_file("test_scan_perdir_ovr/keep.txt", "keep");
+  create_test_file("test_scan_perdir_ovr/sub/inside.tmp", "x");
+
+  ScannerOptions options = {0};
+  options.per_dir_filters = true;
+  if (parallel)
+    options.num_threads = 2;
+  char** paths = NULL;
+  int count = 0;
+  int rc = parallel ? collect_files_parallel(root, &options, &paths, &count)
+                    : collect_files(root, &options, &paths, &count);
+  EXPECT_EQ_INT(rc, 0);
+  /* top.tmp is still excluded by the root file; inside.tmp is re-included by
+   * the subdir file; .rsync-filter files are never transferred. */
+  EXPECT_EQ_INT(count, 2);
+  EXPECT_TRUE(has_path(paths, count, "keep.txt"));
+  EXPECT_TRUE(has_path(paths, count, "sub/inside.tmp"));
+  EXPECT_FALSE(has_path(paths, count, "top.tmp"));
+  EXPECT_FALSE(has_path(paths, count, ".rsync-filter"));
+  EXPECT_FALSE(has_path(paths, count, "sub/.rsync-filter"));
+  free_paths(paths, count);
+
+  unlink("test_scan_perdir_ovr/top.tmp");
+  unlink("test_scan_perdir_ovr/keep.txt");
+  unlink("test_scan_perdir_ovr/sub/inside.tmp");
+  unlink("test_scan_perdir_ovr/.rsync-filter");
+  unlink("test_scan_perdir_ovr/sub/.rsync-filter");
+  rmdir(sub);
+  rmdir(root);
+}
+
 void test_scanner() {
  test_scanner_single_file();
  test_scanner_multiple_files();
@@ -1024,4 +1107,7 @@ void test_scanner() {
  test_cvs_defaults(true);
  test_per_dir_filter(false);
  test_per_dir_filter(true);
+  test_scanner_path_relative();
+  test_per_dir_filter_override(false);
+  test_per_dir_filter_override(true);
 }