util: restore ".." collapsing in clean_fname()

[dr-who]

Problem

Commit 9e08984 ("util: fixed issue in clean_fname()") fixed a genuine read-before-buffer in the ".." back-up loop, but the replacement loop stops in a different place than the one it replaced, and the surrounding test/assignment were carried over unchanged.

The old loop pre-decremented, so it stopped with s pointing at the preceding /:

while (s > limit && *--s != '/') {}

The new loop stops with s pointing at the start of the preceding component, one byte past that /:

while (s > limit && s[-1] != '/')
	s--;

But the follow-up is unchanged, so it is now off by one:

if (s != t - 1 && (s <= name || *s == '/')) {
	t = (s == name) ? name : s + 1;

*s is a component character and is essentially never /, so the collapse now only happens via the s == name case — a single leading component with no slash before it. Every other form silently stopped collapsing:

input	current	expected
a/../b	b	b (still worked)
/a/../b	/a/../b	/b
a/b/../c	a/b/../c	a/c
/usr/local/../bin	/usr/local/../bin	/usr/bin
x/y/../../z	x/y/../../z	z
one/two/three/../../four	one/two/three/../../four	one/four

Impact

clean_fname(..., CFN_COLLAPSE_DOT_DOT_DIRS) is rsync's canonical path normalizer — used for filter and merge-file names and for the --backup-dir, --partial-dir and --temp-dir options — so any such path containing an interior .. is left un-normalized.

User-visible example (a merge filter that can only be opened if .. is collapsed):

$ rsync -a --filter='merge a/none/../c' src/ dst/
rsync: [client] failed to open exclude file a/none/../c: No such file or directory (2)
rsync error: error in file IO (code 11) at exclude.c(1482)

Before 9e08984 this collapsed to a/c and worked.

Fix

Adjust the two spots for where the new loop actually stops: test the byte before s for the / (or that we reached the start of the name), and set t = s.

This keeps the underflow fix — s[-1] is only read when s is past name, and the s == name test short-circuits — while restoring the long-standing behaviour.

The CFN_REFUSE_DOT_DOT_DIRS path returns before this code and is unaffected, as is sanitize_path() (the separate daemon-side .. handler), so this is a path-normalization/file-selection fix, not a security change.

Test

testsuite/clean-fname-collapse_test.py drives the collapse through a merge-filter filename: the filter file lives at a/c but is named a/none/../c with no a/none directory, so it can only be opened if .. is collapsed. It also asserts the filter was actually applied. It fails before this change and passes after; full testsuite otherwise unchanged (106 passed, 0 failed).

[dr-who]

I ran mutation testing over this area to check the fix is actually pinned down, and it found a genuine weakness — in the test, not the code. Pushed e42f5917 to close it.

Method. Fifteen small edits to clean_fname()'s ".." logic (flip a comparison, shift a bound, move an assignment by one), each rebuilt and run against the suite. Ten survived. To tell a real gap from an equivalent mutant I classified each against a reference model of the intended semantics, so "survived" only counts when the output genuinely changes.

What survived that shouldn't have. Six of the ten changed behaviour and nothing caught them, including two that matter a lot here:

• restoring the pre-fix limit = name - 1 initialiser — i.e. a partial revert of this very fix — still passed;
• turning s == name into s != name, which both stops the collapse and reintroduces the out-of-bounds read that 9e08984 set out to remove.

Why. a/none/../c only exercises a ".." that eats a component with a / in front of it. It never reaches the s == name branch (collapsing the leading component), which is exactly where those mutants live.

Fix. Two more merge-filter names in the same test, covering the other two shapes:

• zzz/../f1 — eats the leading component, where the back-up reaches the start of the name with no earlier / to find;
• ../../updir (run from two directories down) — a leading ".." has nothing to collapse into and must be preserved.

Each name resolves only if that shape is handled exactly right; otherwise the merge file can't be opened, or a different file is opened and *.skip isn't filtered out.

Kill rate on behaviour-changing mutants: 5/11 → 10/11. Of the five still surviving, four are equivalent mutants (the model confirms they alter no output), so no test can kill them. The last one — advancing the scan by one byte instead of two — only shows on a name ending in .., which can't be a merge-filter file, so I left it uncovered rather than contorting the test to reach it.

Separately, on the OpenBSD red: that job timed out after 300s in daemon-munge, which I don't think this change can reach — daemon-munge_test.py contains no .., filter, merge or exclude, the daemon path uses sanitize_path() rather than the ..-collapsing clean_fname(), and the test passes 3/3 locally over the same TCP transport. OpenBSD is green on my other three PRs. This push re-triggers it, so we'll see whether it reproduces.