fix(filter): correct per-directory rule owner coordinate; post-clear ownership and bounds

This commit is contained in:
2026-09-24 00:24:52 +02:00
parent b414f197af
commit 884530c9a2
9 changed files with 351 additions and 49 deletions
@@ -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,
+105
View File
@@ -11,8 +11,23 @@
#include <string.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <threads.h>
#include <unistd.h>
/* 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();
}
+54
View File
@@ -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();