fix(utils,file-list): bound keep/files-from indexes to O(M) memory

2026-09-13 04:52:10 +02:00
parent a4f4110397
commit 301cb0dbaf
6 changed files with 377 additions and 140 deletions
@@ -118,6 +118,79 @@ static void test_membership_matches_reference() {
  EXPECT_TRUE(file_list_affects(NULL, NULL));
 }

+/* Explicit ancestor/descendant coverage: a query that is a proper ancestor of
+   a listed entry is affected, and a query below a listed entry is affected,
+   while a component-boundary neighbor is not. */
+static void test_ancestor_and_descendant_queries() {
+  const char* path = "test_file_list_ancestor.txt";
+  char err[160];
+  write_list(path, "top/mid/leaf.txt\nsingle.txt\n");
+  FileListSet* set = file_list_load(path, false, err, sizeof(err));
+  EXPECT_NOT_NULL(set);
+
+  /* q is an ancestor of a listed entry. */
+  EXPECT_TRUE(file_list_affects(set, "top"));
+  EXPECT_TRUE(file_list_affects(set, "top/mid"));
+  EXPECT_FALSE(file_list_affects(set, "top/other")); /* neither direction */
+  EXPECT_FALSE(file_list_affects(set, "to"));        /* component boundary */
+
+  /* A listed entry is an ancestor of q. */
+  EXPECT_TRUE(file_list_affects(set, "single.txt"));
+  EXPECT_TRUE(file_list_affects(set, "single.txt/deeper"));
+  EXPECT_FALSE(file_list_affects(set, "single.txtx")); /* boundary */
+
+  check_queries(set,
+                (const char*[]){"top", "top/mid", "top/mid/leaf.txt", "top/other", "single.txt",
+                                "single.txt/deeper", "single.txtx", "to"},
+                8);
+  file_list_destroy(set);
+  remove(path);
+}
+
+/* Regression for the remote OOM: an adversarial --files-from entry made of a
+   very deep chain of repeated components must be indexed with memory
+   proportional to the entry count.  The old implementation stored one copied
+   ancestor prefix per component (O(L^2) bytes for a single entry); the sorted
+   index stores the exact entries only. */
+static void test_deep_paths_are_bounded() {
+  const char* path = "test_file_list_deep.txt";
+  enum { COMPONENTS = 20000 };
+  size_t entry_len = (size_t)COMPONENTS * 2; /* "a/" per component */
+  char* entry = malloc(entry_len + 1);
+  EXPECT_NOT_NULL(entry);
+  for (size_t i = 0; i < entry_len; i += 2) {
+    entry[i] = 'a';
+    entry[i + 1] = '/';
+  }
+  entry[entry_len - 1] = 'z'; /* .../a/z: a deep leaf name */
+  entry[entry_len] = '\0';
+
+  FILE* fp = fopen(path, "wb");
+  EXPECT_NOT_NULL(fp);
+  EXPECT_EQ_INT((int)fwrite(entry, 1, entry_len, fp), (int)entry_len);
+  EXPECT_EQ_INT(fputc('\n', fp), '\n');
+  fclose(fp);
+
+  char err[160];
+  FileListSet* set = file_list_load(path, false, err, sizeof(err));
+  EXPECT_NOT_NULL(set);
+  EXPECT_EQ_INT(set->count, 1);
+  /* One exact entry stored, not one node per path component. */
+  EXPECT_EQ_INT((int)set->index.sorted.count, 1);
+  EXPECT_EQ_INT((int)set->index.exact.size, 1);
+  EXPECT_TRUE(file_list_affects(set, entry)); /* exact */
+  EXPECT_TRUE(file_list_affects(set, "a"));   /* ancestor of the entry */
+  EXPECT_TRUE(file_list_affects(set, "a/a")); /* deeper ancestor */
+  EXPECT_FALSE(file_list_affects(set, "b"));  /* unrelated */
+  EXPECT_FALSE(file_list_affects(set, "aa")); /* component boundary */
+
+  file_list_destroy(set);
+  remove(path);
+  free(entry);
+}
+
 void test_file_list() {
  test_membership_matches_reference();
+  test_ancestor_and_descendant_queries();
+  test_deep_paths_are_bounded();
 }
@@ -456,7 +456,72 @@ static void test_fd_peer_ip() {
  EXPECT_EQ_STR(peer_string, "");
 }

+/* The keep/files-from indexes must store exactly the input entries (one node
+   each), never a copied ancestor prefix per component.  This builds a PathIndex
+   over paths thousands of components deep and checks the structural bound plus
+   the exact / descendant query semantics. */
+static void test_path_index_bounded() {
+  enum { COUNT = 8, COMPONENTS = 5000 };
+  size_t entry_len = (size_t)COMPONENTS * 2 + 2; /* trailing "xN" */
+  char* storage = malloc((size_t)COUNT * (entry_len + 1));
+  EXPECT_NOT_NULL(storage);
+  const char** entries = calloc(COUNT, sizeof(char*));
+  EXPECT_NOT_NULL(entries);
+  for (int i = 0; i < COUNT; i++) {
+    char* entry = storage + (size_t)i * (entry_len + 1);
+    size_t pos = 0;
+    for (int c = 0; c < COMPONENTS; c++) {
+      entry[pos++] = 'a';
+      entry[pos++] = '/';
+    }
+    entry[pos++] = 'x';
+    entry[pos++] = (char)('0' + i);
+    entry[pos] = '\0';
+    entries[i] = entry;
+  }
+
+  PathIndex index;
+  EXPECT_TRUE(path_index_build(&index, entries, COUNT));
+  EXPECT_EQ_INT((int)index.sorted.count, COUNT);
+  EXPECT_EQ_INT((int)index.exact.size, COUNT);
+  EXPECT_TRUE(path_index_contains(&index, entries[0]));
+  EXPECT_FALSE(path_index_contains(&index, "a"));
+  EXPECT_TRUE(path_index_has_descendant(&index, "a"));
+  EXPECT_TRUE(path_index_has_descendant(&index, "a/a"));
+  EXPECT_FALSE(path_index_has_descendant(&index, "aa"));
+  path_index_free(&index);
+
+  free((void*)entries);
+  free(storage);
+}
+
+static void test_path_index_semantics() {
+  const char* entries[] = {"a/b/c.txt", "a/b/d.txt", "x.txt", "deep/deeper/deepest"};
+  PathIndex index;
+  EXPECT_TRUE(path_index_build(&index, entries, 4));
+  EXPECT_TRUE(path_index_contains(&index, "a/b/c.txt"));
+  EXPECT_FALSE(path_index_contains(&index, "a/b"));
+  EXPECT_TRUE(path_index_contains_n(&index, "a/b/c.txt/ignored", 9));
+  EXPECT_FALSE(path_index_contains_n(&index, "a/b/c.txt/ignored", 10));
+  EXPECT_TRUE(path_index_has_descendant(&index, "a"));
+  EXPECT_TRUE(path_index_has_descendant(&index, "a/b"));
+  EXPECT_FALSE(path_index_has_descendant(&index, "a/b/c.txt"));
+  EXPECT_FALSE(path_index_has_descendant(&index, "ab"));
+  EXPECT_FALSE(path_index_has_descendant(&index, ""));
+  path_index_free(&index);
+
+  /* A zero-entry index answers no queries. */
+  PathIndex empty;
+  EXPECT_TRUE(path_index_build(&empty, NULL, 0));
+  EXPECT_EQ_INT((int)empty.sorted.count, 0);
+  EXPECT_FALSE(path_index_contains(&empty, "a"));
+  EXPECT_FALSE(path_index_has_descendant(&empty, "a"));
+  path_index_free(&empty);
+}
+
 void test_shared_utils() {
+  test_path_index_bounded();
+  test_path_index_semantics();
  test_walker_removes_extras_keeps_manifest_and_protected();
  test_walker_keeps_nested_manifest_dirs();
  test_walker_max_delete_exceeded_deletes_nothing();