From 91197fd7cf28749d4d9cf6122c0c1ee597e8920a Mon Sep 17 00:00:00 2001 From: TapTap Date: Fri, 18 Sep 2026 20:39:20 +0200 Subject: [PATCH] fix(fs): rsync push iconv direction; gate empty-dir emission; reclassify --temp-dir - --iconv now matches rsync's push direction: the destination charset is the client spec's REMOTE half, so a default receiver writes wire names verbatim; a server's own --iconv LOCAL overrides it (daemon charset analog). Updated unit + integration tests and added a default-server differential gate case. - Empty-directory emission is gated behind a new ScannerOptions.emit_empty_dirs set only by the real sender, so low-level scanner helpers keep the historical file-only list. - --temp-dir reclassified to Divergent: relative dirs match rsync exactly (resolved under the destination), but an absolute path is deliberately rejected by the confined receiver; differential test added. - Docs/tally: 109 Parity / 22 Caveat / 25 Divergent. --- RSYNC_COMPAT.md | 21 +++---- src/client/client_send.c | 3 + src/client/scanner.c | 5 +- src/client/scanner.h | 6 ++ src/client/usage.c | 4 +- src/shared/charset.c | 25 ++++---- src/shared/charset.h | 11 ++-- tests/integration/test_differential_parity.py | 5 ++ tests/integration/test_features.py | 55 ++++++++++++++++++ tests/integration/test_iconv.py | 57 +++++++++++-------- tests/test_iconv.c | 20 ++++++- 11 files changed, 157 insertions(+), 55 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 588445a..c232079 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -6,9 +6,9 @@ This document maps rsync's full feature set to FastSync's current implementation | Status | Count | Description | |--------|-------|-------------| -| ✅ Parity | 108 | Reproduces rsync's semantics for this option's scope | -| ⚠️ Caveat | 24 | Wired and tested, but carries a documented behavioral difference from rsync (named in the row and/or the wave notes) | -| ❌ Divergent | 24 | Rejected, an accepted no-op, deliberately non-rsync (native config/auth/batch, privileged namespaces, safe-subset privilege), or impossible on any portable filesystem call | +| ✅ Parity | 109 | Reproduces rsync's semantics for this option's scope | +| ⚠️ Caveat | 22 | Wired and tested, but carries a documented behavioral difference from rsync (named in the row and/or the wave notes) | +| ❌ Divergent | 25 | Rejected, an accepted no-op, deliberately non-rsync (native config/auth/batch, privileged namespaces, safe-subset privilege), or impossible on any portable filesystem call | | **Total** | **156** | One row per rsync option/feature group; a row may name several spellings | This matrix reports honest rsync parity, not "implemented" as a synonym for @@ -127,7 +127,7 @@ Every one of those has an entry below with its remaining caveats. | `--backup-dir=DIR` | Backup directory hierarchy | ✅ Parity | `backup_dir` config field | | `--suffix=SUFFIX` | Backup suffix (default ~) | ✅ Parity | `suffix` config field | | `--delay-updates` | Put updated files in place at end | ⚠️ Caveat | Successfully received files are staged under a private 0700 `.fastsync-stage` dir inside the receive root and atomically renamed into their final destinations only after the whole transfer (manifest/delete handling included) succeeds, just before the success/outcome frame is sent. The delete walker deliberately skips the staging dir at the receive root, so `--delete` removes genuine extras but never the staged files (deletion runs before publication; rsync's delete-after ordering is not implemented). `--existing`/`--ignore-existing`/`--update` decide against the final destination path at stage time; `--backup` moves the old file aside at publication, and **`--force` is honored at publication** (protocol 2.23.0): a staged regular file or symlink may replace a destination directory that blocks it. Incompatible with `--inplace` and with `--backup-dir=.fastsync-stage` (the internal staging name is reserved; both are rejected). The staging dir name is fixed, so two simultaneous delayed transfers to the same destination root are serialized with an exclusive advisory lock held for the whole transfer: the second session fails cleanly instead of corrupting the first. Aborting or failing before publication installs nothing and removes the staging tree; a crash between stage and publish leaves staged leftovers that the next delayed run wipes at start (process death releases the lock). A stage→publish failure aborts the transfer (best-effort cleanup of the not-yet-published staged files; already-published files are not rolled back). Works in single-threaded and `-j`/`--threads` modes | -| `-T`, `--temp-dir=DIR` | Create temporary files in DIR | ⚠️ Caveat | `--temp-dir` with the rsync short `-T` (the timeout alias moved to long-only `--timeout`). **Protocol 2.23.0 receiver policy: the scratch dir is confined to the receive root — a relative dir is resolved below it, and an absolute path or one containing `..` is rejected by the receiver** (an absolute/foreign-filesystem scratch dir was the divergence; rsync's standalone mode would follow an absolute `--temp-dir`, while its daemon also confines). Temp copies use a unique name there and are atomically renamed into place. **On `EXDEV` (scratch dir and destination on different filesystems) the receiver falls back to a non-atomic copy instead of aborting the transfer**, matching rsync. `--inplace` and `--partial-dir` writes bypass the scratch dir | +| `-T`, `--temp-dir=DIR` | Create temporary files in DIR | ❌ Divergent | `--temp-dir` with the rsync short `-T` (the timeout alias moved to long-only `--timeout`). A **relative** dir matches rsync exactly: it is resolved below the receive/destination root and must already exist (differentially verified: `rsync -a --temp-dir=scratch src/ dst/` and FastSync produce identical trees and an empty scratch dir). **Reclassified as a deliberate divergence because an absolute `--temp-dir` is rejected by the receiver** — it is resolved verbatim by rsync standalone (which will use `/tmp` or any other absolute directory, including one outside the destination), but FastSync's security-reviewed receiver confines the scratch dir to the authorized receive root and rejects any absolute path or one containing `..`. A differential test confirms rsync exits 0 using an absolute scratch dir while FastSync refuses before writing anything into it (the scratch dir stays empty). Its daemon mode also confines relative to the module, but standalone rsync's absolute-temp-dir behavior is not reproduced because it would let a client place receiver scratch files outside the sandbox. Temp copies use a unique name in the scratch dir and are atomically renamed into place; **on `EXDEV` (scratch dir and destination on different filesystems, reachable via a confined relative symlink) the receiver falls back to a non-atomic copy instead of aborting**, matching rsync. `--inplace` and `--partial-dir` writes bypass the scratch dir | | `--partial` | Keep partially transferred files | ✅ Parity | On a failed/interrupted write the already-written temp file is retained at the destination path (best-effort rename instead of unlink) so a later `--append`/`--append-verify` run can resume it. Retention never runs when no data was actually written or under `--ignore-existing`/`--existing` (the destination is not ours to overwrite), and it only ever renames the already-written temp. A failed rename falls back to the normal unlink | | `--partial-dir=DIR` | Keep partial files in DIR | ✅ Parity | With `--partial`, the working file is written under the confined partial directory (a relative dir below the receive root) and atomically renamed into place once complete, so an interrupted transfer leaves a resumable copy there and completed transfers do not linger under it. `--inplace` bypasses the partial dir (rsync parity). Requires `--partial` | @@ -734,7 +734,7 @@ modes or links. | `--stop-at=TIME` | Stop at specified time | ✅ Parity | Deadline transfer stop (client-only, never serialized). Protocol 2.26.0 accepts rsync's full date/time grammar (`2030-12-31T23:59`, `2030/12/31T23:59`, `2030-12-31`, `12-31`, `14:00`, `:59`, `1`) in addition to FastSync's `HH:MM[:SS]` and `now+N[smhd]`; a past time stops immediately. Everything already transferred is kept and an early stop suppresses the late `--delete` keep-set so unscanned source mirrors survive. Works single-threaded and under `-j`/`--threads` | | `--fsync` | Fsync every written file before publication | ✅ Parity | | | `--protocol=NUM` | Force older protocol version | ❌ Divergent | Forces the wire protocol version for this transfer. FastSync has exactly ONE wire format (`PROTOCOL_VERSION`, currently 2.26.0) with no downgrade/backward-compat code paths, so `--protocol=2.26.0` is accepted (it sets the version claim the client sends, which the server already requires to match exactly) and **every other value is rejected up front** with a clear error before any connection — it does not and cannot speak an older or virtual wire format. Divergence from rsync (which negotiates a range and downgrades to an integer 0..31): FastSync's honest contract is force-to-the-one-supported-value; a genuine downgrade would require a per-version compatibility layer that does not exist. Client-only; the server-side exact-match check is unchanged. `--protocol=2.25.0`/`2.24.0`/`2.23.0`/`2.22.0`/`2.21.0`/`2.20.0`/`2.19.0`/`2.18.0`/`2.17.0`/`2.16.0`/`2.15.0`/`216`/`31`/garbage are all rejected. See the Phase-6 protocol note below | -| `--iconv=CONVERT_SPEC` | Charset conversion | ⚠️ Caveat | Charset conversion of FILE NAMES (not content) at the protocol boundary via iconv(3): `--iconv=LOCAL[,REMOTE]` — the sender converts each local filename LOCAL→REMOTE before transmitting, and the receiver converts each wire filename REMOTE→LOCAL before creating/writing. The full CONVERT_SPEC is serialized into the config frame as a new trailing string field so the peer knows the wire charset; **PROTOCOL_VERSION bumped 2.15.0 → 2.16.0**. `LOCAL[,REMOTE]` parse: single charset ⇒ LOCAL==REMOTE (identity both ways); garbage rejected up front; protocol 2.26.0 additionally accepts `--iconv=.` (the locale's default charset for both directions), `--iconv=-` and `--no-iconv` (disable conversion). Validation probes BOTH directions (a spec that only opens one way is refused, as is a NUL-emitting target charset like utf-16/utf-32/ucs-2, since filenames cannot contain NUL). An unrepresentable name (EILSEQ/EINVAL) fails that path cleanly with a logged `--iconv: cannot convert file name ...` and is never written mangled/truncated. Conversion is applied at EVERY wire-path site (regular/MKDIR/hardlink path+target/symlink path+target/SPECIAL, the delete manifest, the incremental-check path, and the `-s`/`chunk_serialize` embedded blob path), on both client and server (`--iconv` is also a server/daemon option). Zero overhead when unset. See the Phase-6 iconv notes below | +| `--iconv=CONVERT_SPEC` | Charset conversion | ✅ Parity | Charset conversion of FILE NAMES (not content) at the protocol boundary via iconv(3): `--iconv=LOCAL[,REMOTE]` — the sender converts each local filename LOCAL→REMOTE before transmitting, matching rsync's rule that the spec "stays the same whether you're pushing or pulling": on a PUSH the destination end's charset is the spec's REMOTE half, so the default receiver writes the wire bytes verbatim, and only a server started with its own `--iconv` (the daemon `charset` analog) declares a different destination charset and converts REMOTE→that LOCAL (rsync push parity, differential-tested with and without a server `--iconv`). The full CONVERT_SPEC is serialized into the config frame as a new trailing string field so the peer knows the wire charset; **PROTOCOL_VERSION bumped 2.15.0 → 2.16.0**. `LOCAL[,REMOTE]` parse: single charset ⇒ LOCAL==REMOTE (identity both ways); garbage rejected up front; protocol 2.26.0 additionally accepts `--iconv=.` (the locale's default charset for both directions), `--iconv=-` and `--no-iconv` (disable conversion). Validation probes BOTH directions (a spec that only opens one way is refused, as is a NUL-emitting target charset like utf-16/utf-32/ucs-2, since filenames cannot contain NUL). An unrepresentable name (EILSEQ/EINVAL) fails that path cleanly with a logged `--iconv: cannot convert file name ...` and is never written mangled/truncated. Conversion is applied at EVERY wire-path site (regular/MKDIR/hardlink path+target/symlink path+target/SPECIAL, the delete manifest, the incremental-check path, and the `-s`/`chunk_serialize` embedded blob path), on both client and server (`--iconv` is also a server/daemon option). Zero overhead when unset. See the Phase-6 iconv notes below | | `--checksum-seed=NUM` | Set checksum seed | ✅ Parity | Sets the seed for FastSync's whole-file xxHash digest (full 64-bit seed) and for the delta path's per-block xxHash32 strong checksum (low 32 bits of the seed). **As of protocol 2.23.0 a seed of `0` — the default when the flag is unset — is randomized per transfer and the chosen seed is sent to the receiver**, exactly like rsync, so two runs against different content do not share a predictable seed; an explicit non-zero seed is used verbatim, so an explicit seed deterministically reproduces every computed digest on BOTH endpoints (the seed crosses in the config frame). `--checksum-choice=md5` has no seed and ignores it (documented). The value is a strict decimal 0..2⁶⁴-1 (blank, signed, or non-numeric values are rejected). Like rsync, a seed only matters where a digest is actually computed (`--checksum` or a basis-dir run, or a delta transfer); it does not by itself enable `--checksum`/`--delta` | | `--secluded-args`, `-s` | Use protocol to send args | ❌ Divergent | Accepted for CLI compatibility (including the rsync short `-s`, Phase 7 Wave A) but a documented **no-op / divergence**. rsync's `-s` protects arguments from shell expansion by shipping them over the protocol; FastSync never passes remote arguments through a shell expansion boundary in the first place — its SSH transport builds the remote argv as **single-quote-escaped shell words** (`ssh_build_remote_command`), so the injection/leak that `-s` guards against does not exist and there is nothing to "seclude". Implementing a true arg-send protocol would mean replacing the argv-based SSH launch with an in-band argument channel, a large redesign of the transport that buys no security here. Chunk serialization remains the long-only `--chunk-serialization`. | | `--protect-args` | Old name of --secluded-args | ❌ Divergent | Accepted for CLI compatibility as a documented no-op; the same rationale as `--secluded-args`/`-s` (FastSync's remote SSH argv is already built injection-safe, so there is no argument-leak to close) | @@ -848,7 +848,7 @@ These are the hardest compatibility items because they require durable formats o **Phase 6, Wave A (stop deadline) shipping note:** `--stop-after=MINS` and `--stop-at=TIME` are client-only sender stop deadlines. `--stop-after` takes a positive minute count (0/negative/garbage rejected); `--stop-at` takes `HH:MM`, `HH:MM:SS`, or `now+N[smhd]` (a past time stops immediately, a garbage spec is rejected at parse time). The deadline is computed once at the start of the transfer (CLOCK_MONOTONIC for `--stop-after`, wall clock via `time()` for `--stop-at`) and checked at every chunk boundary in both the single-threaded `send_files` loop and the multithreaded `send_chunks_multithreaded` path, and inside the scanner loops so a busy scan itself stops. When it fires, the transfer stops ELEGANTLY: the in-flight chunk completes, the existing completion tail runs (summary, `disconnect`), and the run returns 0 — exactly like rsync's clean early stop. Because the deadline is client-only and never crosses the wire config frame, no PROTOCOL_VERSION bump is required. The safety-critical interaction is with `--delete`: FastSync streams while scanning, so a deadline can cut the source scan short and yield a PARTIAL keep-set manifest; committing that would make the receiver delete destination mirrors of source files not yet scanned. So the sender tracks `scan_stopped_early` and, when it is true on the late/delete-after (`--delete`/`--delete-after`/`--delete-delay`) path, SUPPRESSES the keep-set manifest (logs a warning) so no deletion happens from an incomplete set — this is the safe direction (preserves data; the delete simply does not run). `--delete-before`/`--delete-during` are unaffected: their complete pre-scan runs before any data and ignores the deadline (a stop can be exceeded by that pre-scan). Under `-j`/`--threads` the stop is symmetric and the scanner thread's still-in-progress manifest appends can never race the tail because the tail does not read the manifest on the early-stop path. -**Phase 6, Wave B (iconv) shipping note (PROTOCOL 2.15.0 → 2.16.0):** `--iconv=LOCAL[,REMOTE]` converts file NAMES at the wire boundary (never content). The full CONVERT_SPEC is serialized into the config frame as a new trailing string field (empty→NULL canonicalized), so both ends share the same wire charset interpretation; this required the PROTOCOL bump because the frame is a strict ordered sequence and a peer that does not parse the new trailing field would desynchronize. Each end derives LOCAL (its own charset) and REMOTE (the wire charset): the sender opens LOCAL→REMOTE and converts every transmitted filename; the receiver opens REMOTE→LOCAL and converts every received filename before creating/writing. Conversion is applied at every wire-path site (regular/MKDIR/hardlink path+target/symlink path+target/SPECIAL, the delete manifest keep/protected/missing entries, the incremental-check path, and the embedded `-s`/chunk-blob path). A name it cannot convert (EILSEQ/EINVAL) is failed cleanly with a logged `--iconv: cannot convert file name ...` and is never written truncated/mangled. Validation probes both directions up front (both the sender local→remote and the receiver remote→local, and, for a server/daemon with its own `--iconv`, the client-REMOTE→server-LOCAL pair) so an unusable spec is rejected before the connection rather than mid-transfer, and NUL-emitting target charsets (utf-16/utf-32/ucs-2) are refused because filenames cannot contain NUL. Divergence documented upstream: the receiver does NOT half-swap; the wire charset always comes from the sender's REMOTE half, so a server whose local charset differs from the client's LOCAL must declare it with its own `--iconv`. Conversion is process-global and runs on a single thread per process (sender thread / receiver-loop thread), initialized before worker threads start and freed after they join. +**Phase 6, Wave B (iconv) shipping note (PROTOCOL 2.15.0 → 2.16.0):** `--iconv=LOCAL[,REMOTE]` converts file NAMES at the wire boundary (never content). The full CONVERT_SPEC is serialized into the config frame as a new trailing string field (empty→NULL canonicalized), so both ends share the same wire charset interpretation; this required the PROTOCOL bump because the frame is a strict ordered sequence and a peer that does not parse the new trailing field would desynchronize. Each end derives its charset and the wire charset: the sender opens LOCAL→REMOTE and converts every transmitted filename; on a push the receiver's destination charset is the spec's REMOTE half, so it writes the wire bytes verbatim, unless the server was started with its own `--iconv` naming a different LOCAL charset (then it opens REMOTE→that LOCAL). Conversion is applied at every wire-path site (regular/MKDIR/hardlink path+target/symlink path+target/SPECIAL, the delete manifest keep/protected/missing entries, the incremental-check path, and the embedded `-s`/chunk-blob path). A name it cannot convert (EILSEQ/EINVAL) is failed cleanly with a logged `--iconv: cannot convert file name ...` and is never written truncated/mangled. Validation probes both directions up front (both the sender local→remote and the receiver remote→destination, and, for a server/daemon with its own `--iconv`, the client-REMOTE→server-LOCAL pair) so an unusable spec is rejected before the connection rather than mid-transfer, and NUL-emitting target charsets (utf-16/utf-32/ucs-2) are refused because filenames cannot contain NUL. The wire charset always comes from the sender's REMOTE half; a server whose local charset differs from the client's REMOTE must declare it with its own `--iconv` (the daemon `charset` analog). Conversion is process-global and runs on a single thread per process (sender thread / receiver-loop thread), initialized before worker threads start and freed after they join. **Phase 6, Wave C (protocol-version) shipping note (no PROTOCOL_VERSION change):** `--protocol=NUM` lets the client force the wire protocol version for a transfer. FastSync's protocol is a single lockstep format: the config frame is a strict ordered sequence and the server requires the client's version string to equal `PROTOCOL_VERSION` exactly (`config_receive_with_validate`, src/shared/config.c) — there are no older-format code paths and no downgrade/negotiation machinery, so a lower/higher/virtual version can never be spoken. The honest contract is therefore: the current `PROTOCOL_VERSION` (2.26.0 as of the parity-completion wave) is accepted and stored into the client's `version` claim (which `config_send` already transmits), and every other value — `2.25.0`, `2.24.0`, `2.23.0`, `2.22.0`, `2.21.0`, `2.20.0`, `2.19.0`, `2.18.0`, `2.18`, `2.17.0`, `2.16.0`, `2.15.0`, `3.0.0`, rsync-integer spellings like `216`/`31`, garbage, empty — is rejected up front in `validate_config()` before any connection, with a clear error that FastSync supports only its current wire protocol and cannot speak an older or virtual one. Implementation is client-only: a server-side `--protocol` is intentionally not added because the server has no negotiation (it only enforces exact match), and it could only ever be the current version. This preserves (and slightly tightens) existing validation: the client now also refuses to launch with a version it cannot actually speak, rather than only the server rejecting it later. A genuine downgrade would require a per-version compatibility layer for every frame/feature added since (append 2.10, preallocate 2.11, hardlinks 2.12, devices/specials/symlink-trust/xattr 2.13, remote-option 2.14, daemon module/auth 2.15, iconv 2.16, dir/symlink times 2.17, privilege flags --super/--copy-as 2.18, SCRAM daemon auth 2.19, packed metadata 2.20, error-detail/dry-run 2.21, preserve-attribute split 2.22, rsync-parity wave 2.23) and is intentionally out of scope — documented divergences from rsync's integer-negotiated downgrade remain. @@ -893,7 +893,7 @@ These are the last compatibility items and the closing phase toward rsync flag p **Wire:** two trailing config-frame blocks after the `--iconv` spec, in fixed order — `send_privilege_options`/`receive_privilege_options` (one `super_mode` int, validated `0..2`), then `send_copy_as_options`/`receive_copy_as_options` (presence int + two int32 ids, validated `>= 0`, with `copy_as_set ⇒ use_metadata`). `PROTOCOL_VERSION` bumped **2.17.0 → 2.18.0**. **Divergences from rsync:** rsync's `--super` elevates the receiver and `--copy-as` actually switches its credentials; FastSync never elevates and only permits/forwards confined attempts, and `--copy-as` forces ownership rather than switching identity. -**Honest status after the parity-completion wave (protocol 2.26.0), updated by the rsync-parity-stats and rsync-parity-fs passes.** ✅ Parity 108 / ⚠️ Caveat 24 / ❌ Divergent 24 = 156 rows. Earlier revisions of this document reported "143 ✅ / 0 divergence / 0 partial"; that conflated "parsed and tested" with "rsync parity", because many rows carried documented behavioral differences and some short options were not parsed at all. This reclassification makes every difference explicit. The completion wave closed 23 previously-caveated rows (9 that triage showed were already parity, plus 14 genuine fixes) and turned the 17 inherently non-rsync rows — native daemon config/auth, the FastSync batch container, the safe-subset device/privilege flags, `-X`'s privileged namespaces, `--fake-super`'s native xattr format, and the `--old-args` no-op — into explicit ❌ divergences. The stats pass flipped `--delete-delay` to ✅ (actual-removal accounting) and reclassified `--out-format` to ❌ (protocol-specific `%b`/delta-`%c`), and sharpened the `--stats`/`--progress`/`--checksum-choice` residuals. The fs pass flipped `-d/--dirs` to ✅: recursive transfers now recreate empty source directories (and replace a blocking destination non-directory with an incoming directory), and `-R --no-implied-dirs --files-from` places a listed file under a missing implied parent with default attributes instead of refusing. The remaining ⚠️ rows are the ones with a documented residual (see the row notes and the **Parity Completion Wave (protocol 2.26.0)** section below). +**Honest status after the parity-completion wave (protocol 2.26.0), updated by the rsync-parity-stats and rsync-parity-fs passes.** ✅ Parity 109 / ⚠️ Caveat 22 / ❌ Divergent 25 = 156 rows. Earlier revisions of this document reported "143 ✅ / 0 divergence / 0 partial"; that conflated "parsed and tested" with "rsync parity", because many rows carried documented behavioral differences and some short options were not parsed at all. This reclassification makes every difference explicit. The completion wave closed 23 previously-caveated rows (9 that triage showed were already parity, plus 14 genuine fixes) and turned the 17 inherently non-rsync rows — native daemon config/auth, the FastSync batch container, the safe-subset device/privilege flags, `-X`'s privileged namespaces, `--fake-super`'s native xattr format, and the `--old-args` no-op — into explicit ❌ divergences. The stats pass flipped `--delete-delay` to ✅ (actual-removal accounting) and reclassified `--out-format` to ❌ (protocol-specific `%b`/delta-`%c`), and sharpened the `--stats`/`--progress`/`--checksum-choice` residuals. The fs pass flipped `-d/--dirs` and `--iconv` to ✅ — recursive transfers now recreate empty source directories (and replace a blocking destination non-directory with an incoming directory); `-R --no-implied-dirs --files-from` places a listed file under a missing implied parent with default attributes instead of refusing; and `--iconv` now reproduces rsync's push direction (destination charset = the spec's REMOTE half) — and reclassified `--temp-dir` to ❌ (the receiver confines the scratch dir to the receive root, so an absolute temp dir is deliberately rejected although standalone rsync follows it). The remaining ⚠️ rows are the ones with a documented residual (see the row notes and the **Parity Completion Wave (protocol 2.26.0)** section below). **Preserve-attribute split (protocol 2.21.0 → 2.22.0) — ✅ implemented.** FastSync splits the former single metadata bundle into four independent, rsync-compatible per-attribute flags — `-p/--perms`, `-t/--times`, `-o/--owner`, `-g/--group` — each with a negation (`--no-perms`/`--no-times`/`--no-owner`/`--no-group`, short `--no-p`/`--no-t`/`--no-o`/`--no-g`), plus `--no-preserve` clearing all four. `-a/--archive` is now full rsync `-rlptgoD` (owner and group included, though their application stays privilege-gated), `-A/--acls` implies `-p`, `-X/--xattrs` does not, `-E/--executability` sets only executability, and `-U`/`-N` do not imply `-t`. `--incremental`/`--delta` still auto-preserve perms+times unless the user explicitly negated them. Wire: the binary config frame gains four appended booleans (`preserve_perms`/`preserve_times`/`preserve_owner`/`preserve_group`) after `omit_link_times`, so `PROTOCOL_VERSION` is bumped **2.21.0 → 2.22.0**; the fixed-width `FileMetadata` layout is unchanged and the receiver gates the metadata frame on a derived `use_metadata`. Receiver behavior: each attribute is applied independently, directory modes are applied under `-p` (at the end of the transfer, alongside dir times), symlink mode under `-p`, and `-O/--omit-dir-times` suppresses directory times only. Documented divergences as of 2.22.0, **all but (d)/(e) removed by the rsync-parity wave (protocol 2.23.0)**: (a) the mode-masking divergence is **gone** — under `-p` the source mode is now copied exactly, including `S_IWGRP`/`S_IWOTH` and setuid/setgid/sticky; (b) a brand-new file without `-p` still gets `source_mode & ~umask` when metadata is present (else the historical fixed `0644`), and a new *directory* without `-p` still uses FastSync's `0755` default; (c) the `--chmod`-implies-`-p` divergence is **gone** — `--chmod` no longer implies `-p` (rsync parity); (d) `-o`/`-g` map by name on the receiver with a raw-numeric fallback (only numeric ids cross the wire); (e) a daemon module without `client owner = yes` does not refuse a plain `-a`/`-o`/`-g` — it forces super off, applies no ownership, and logs a warning, while explicit `--chown`/`--usermap`/`--groupmap`/`--numeric-ids`/`--copy-as`/`--super` are still refused. @@ -1140,9 +1140,10 @@ These remain after the wave; the individual rows carry the precise wording. sender-derived). `--ignore-errors` exits 23 but its EACCES differential is not exercised in CI. - **`--delay-updates`** uses a fixed staging name with an advisory lock and - deletes before publication; **`--temp-dir`** rejects absolute/foreign paths; - **`--remote-option`** is SSH-only; **`--iconv`** keeps the receiver - half-swap/charset-declaration caveat. + deletes before publication; **`--temp-dir`** rejects absolute/foreign paths + (deliberately confined, see the row); **`--remote-option`** is SSH-only. + **`--iconv`** now matches rsync's push direction (destination charset = the + spec's REMOTE half; a server `--iconv` overrides it). - **Basis dirs** do not re-apply attributes on a match, keep the `--size-only` mtime caveat, and share the 256 MiB whole-file cap; **`--fuzzy`** has a different tie-break order; and **`--bwlimit`** rejects rsync's diff --git a/src/client/client_send.c b/src/client/client_send.c index d87580a..eeaf537 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -412,6 +412,9 @@ static bool prepare_scanner(const Config* config, int num_threads, PreparedScann options->exclude_per_dir_filter_files = config->per_dir_filter_count >= 2; options->dirs = config->dirs; options->relative = config->relative; + /* A real recursive transfer recreates empty source directories (rsync + parity); low-level scanner users leave this off. */ + options->emit_empty_dirs = true; /* --no-implied-dirs only has meaning with -R (rsync): without it the option is a documented no-op, so the scanner must not suppress directory metadata. */ diff --git a/src/client/scanner.c b/src/client/scanner.c index c3c53eb..1ea5887 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -1399,8 +1399,9 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { if (entry == NULL) { /* The directory is exhausted: if nothing was transferred or descended from it, recreate it at the destination as an explicit entry. */ - if (!scanner->current_dir_produced && !scanner->options.prune_empty_dirs && - !scanner->options.list_dirs && scanner->options.file_list == NULL) { + if (scanner->options.emit_empty_dirs && !scanner->current_dir_produced && + !scanner->options.prune_empty_dirs && !scanner->options.list_dirs && + scanner->options.file_list == NULL) { if (!scanner_emit_empty_dir(scanner, chunk_data)) scanner->failed = true; } diff --git a/src/client/scanner.h b/src/client/scanner.h index 88ee9f5..7f6b119 100644 --- a/src/client/scanner.h +++ b/src/client/scanner.h @@ -151,6 +151,12 @@ typedef struct { bool capture_dir_times; ArrayList* dir_entries; mtx_t* dir_entries_mutex; + /* Recreate empty source directories on a recursive transfer: emit a + * payload-less directory entry for every traversed directory that produced + * no transferred/descended child. Off by default so low-level scanner users + * (unit helpers, --list-only) see only the historical file list; the real + * sender sets it in prepare_scanner. */ + bool emit_empty_dirs; /* --no-implied-dirs with -R + --files-from: a directory that is only an * implied parent of a listed entry (not itself listed, nor below a listed * directory) must not carry source metadata; it is created with default diff --git a/src/client/usage.c b/src/client/usage.c index 8823909..f51add9 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -90,8 +90,8 @@ void print_usage(void) { printf(" entry's destination mirror receiver-side. Independent of\n"); printf(" --delete (it does not imply --delete; a non-empty directory\n"); printf(" mirror is removed only with --force or --delete)\n"); - printf(" -m, --prune-empty-dirs Do not transfer empty directory entries (--dirs mode);\n"); - printf(" recursive transfers never send empty dirs\n"); + printf(" -m, --prune-empty-dirs Do not create empty directories (a recursive transfer\n"); + printf(" otherwise recreates them, like rsync)\n"); printf(" Note: each timing flag implies --delete. Combining a timing flag with\n"); printf(" --no-delete (in either order) is rejected as a config error.\n"); printf(" --ignore-existing Skip files that already exist on receiver\n"); diff --git a/src/shared/charset.c b/src/shared/charset.c index 75a2ac5..547ef4c 100644 --- a/src/shared/charset.c +++ b/src/shared/charset.c @@ -168,9 +168,14 @@ bool charset_spec_valid_direction(const char* from_charset, const char* to_chars return direction_probe_valid(from_charset, to_charset); } -/* The receiver's real conversion is wire(client REMOTE) -> server-local (the - * server's own --iconv LOCAL half, or the client's LOCAL half when the server - * has no --iconv). A dedicated pre-ack check so an impossible direction is +/* The receiver's conversion is wire charset -> destination charset. rsync's + * CONVERT_SPEC is LOCAL,REMOTE and "stays the same whether you're pushing or + * pulling", so for a PUSH (FastSync's only direction) the destination end's + * charset is the spec's REMOTE half: the client converts LOCAL -> REMOTE on the + * sender and the receiver writes the wire bytes verbatim. Only a server that + * declares its OWN --iconv (the daemon "charset" analog) has a different local + * charset, and then it is that spec's LOCAL half and the receiver converts + * wire -> server-local. A dedicated pre-ack check so an impossible direction is * rejected before the connection instead of refusing mid-transfer. */ bool charset_wire_receiver_spec_valid(const char* spec, const char* server_spec) { if (!spec) @@ -180,7 +185,7 @@ bool charset_wire_receiver_spec_valid(const char* spec, const char* server_spec) if (charset_spec_parse(spec, &local, &remote) != 0) return false; const char* wire = remote; - const char* target_local = local; + const char* target_local = remote; char* server_local = NULL; char* server_remote = NULL; if (server_spec) { @@ -302,13 +307,13 @@ bool charset_wire_init_receiver(const char* spec, const char* server_spec) { char* remote; if (charset_spec_parse(spec, &local, &remote) != 0) return false; - /* The wire charset is the client spec's REMOTE half; the local charset is - * the client spec's LOCAL half unless the server was itself started with - * --iconv naming a different local charset (the server halves above never - * travel, so the server's own flag is the only way its local charset can - * differ from what the client assumed). */ + /* The wire charset is the client spec's REMOTE half (rsync's LOCAL,REMOTE + * spec stays the same push or pull, so on a push the destination end's + * charset is REMOTE and the receiver writes the wire bytes verbatim). Only a + * server started with its own --iconv declares a different local charset (the + * server halves above never travel), and then it is that spec's LOCAL half. */ const char* wire = remote; - const char* target_local = local; + const char* target_local = remote; char* server_local = NULL; char* server_remote = NULL; if (server_spec) { diff --git a/src/shared/charset.h b/src/shared/charset.h index 49cd0f3..874c901 100644 --- a/src/shared/charset.h +++ b/src/shared/charset.h @@ -57,8 +57,9 @@ void charset_conversion_close(void* conversion); /* Process-wide wire conversion. charset_wire_init_sender (client side) opens * LOCAL->REMOTE; charset_wire_init_receiver (server side) opens * wire(REMOTE)->server-local. server_spec is the server's own --iconv, whose - * LOCAL half may override the local charset the client assumed; NULL reuses - * the client spec's LOCAL half. Both return false on an unsupported spec. + * LOCAL half overrides the destination charset; NULL means the destination + * charset is the client spec's REMOTE half (rsync's push semantics: the wire + * bytes are written verbatim). Both return false on an unsupported spec. * The state is freed with charset_wire_free. */ bool charset_wire_init_sender(const char* spec); bool charset_wire_init_receiver(const char* spec, const char* server_spec); @@ -66,9 +67,9 @@ void charset_wire_free(void); bool charset_wire_active(void); /* Pre-ack receiver-direction sanity (see charset_wire_init_receiver): true - * when the exact wire->server-local conversion the receiver will use (client - * spec's REMOTE half into the server's own LOCAL half, or the client's LOCAL - * half when the server has no --iconv) opens and produces NUL-free output. */ + * when the exact wire->destination conversion the receiver will use (client + * spec's REMOTE half into the server's own LOCAL half, or REMOTE->REMOTE when + * the server has no --iconv) opens and produces NUL-free output. */ bool charset_wire_receiver_spec_valid(const char* spec, const char* server_spec); /* Convert a path across the wire in the process direction. Returns a malloc'd diff --git a/tests/integration/test_differential_parity.py b/tests/integration/test_differential_parity.py index 37239a1..a929824 100644 --- a/tests/integration/test_differential_parity.py +++ b/tests/integration/test_differential_parity.py @@ -233,6 +233,11 @@ _CASES = [ ["-a", "--iconv=ISO-8859-1,UTF-8"], server_args=("--allow-super", "--iconv=UTF-8"), ref="--iconv conversion (receiver declares its own charset)"), + # rsync's spec is LOCAL,REMOTE and the destination end's charset is REMOTE + # on a push, so a default server writes the wire (UTF-8) names verbatim. + H.Case("iconv_default_server", "iconv", + ["-a", "--iconv=ISO-8859-1,UTF-8"], + ref="--iconv push direction (default receiver charset = REMOTE)"), # --- partial ---------------------------------------------------------- H.Case("partial_complete", "basic", ["-a", "--partial"], ref="--partial"), diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index 1c03485..8fe1b1c 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -2376,6 +2376,61 @@ class TestTempDir: assert not mismatches, f"Mismatch: {mismatches}" self._assert_clean_scratch(os.path.join(dest, "scratch")) + @pytest.mark.skipif(shutil.which("rsync") is None, reason="rsync not installed") + def test_relative_temp_dir_matches_rsync_absolute_rejected(self): + """Differential: a relative --temp-dir is resolved under the destination + by both (rsync 3.4.1 and FastSync), producing identical trees. An + absolute --temp-dir is used verbatim by rsync standalone, but the + receiver deliberately confines it to the receive root (security + invariant), so FastSync rejects it without writing outside the root. + """ + source = self._make_source("tempdir_diff_src") + rdst = os.path.join(TEST_DATA_DIR, "tempdir_diff_rdst") + fdst = os.path.join(TEST_DATA_DIR, "tempdir_diff_fdst") + clean_dir(rdst) + clean_dir(fdst) + os.makedirs(os.path.join(rdst, "scratch"), exist_ok=True) + os.makedirs(os.path.join(fdst, "scratch"), exist_ok=True) + r = subprocess.run(["rsync", "-a", "--temp-dir=scratch", source + "/", rdst + "/"], + capture_output=True, text=True, + env=dict(os.environ, LC_ALL="C")) + assert r.returncode == 0, r.stderr + with ServerManager() as server: + server.start() + result, _ = run_client(source, fdst, flags=["--temp-dir=scratch"], + port=server.port) + assert result.returncode == 0, (result.stderr or result.stdout)[:300] + # rsync lays the source contents directly in rdst; FastSync mirrors the + # absolute source path below fdst. Compare the mirrored content trees + # (the scratch dir lives at each destination root). + rtree = sorted(os.path.relpath(os.path.join(dp, n), rdst) + for dp, dn, fn in os.walk(rdst) + for n in dn + fn if os.path.join(dp, n) != os.path.join(rdst, "scratch")) + mirror = get_dest_received_dir(fdst, source) + ftree = sorted(os.path.relpath(os.path.join(dp, n), mirror) + for dp, dn, fn in os.walk(mirror) for n in dn + fn) + assert rtree == ftree, f"relative temp-dir tree mismatch: {rtree} != {ftree}" + assert _walk_tmp_files(os.path.join(rdst, "scratch")) == [] + assert _walk_tmp_files(os.path.join(fdst, "scratch")) == [] + + # Absolute temp dir: rsync accepts it; FastSync rejects it safely. + abs_scratch = os.path.join(TEST_DATA_DIR, "tempdir_diff_abs") + clean_dir(abs_scratch) + rdst2 = os.path.join(TEST_DATA_DIR, "tempdir_diff_rdst2") + clean_dir(rdst2) + r2 = subprocess.run(["rsync", "-a", "--temp-dir=" + abs_scratch, source + "/", rdst2 + "/"], + capture_output=True, text=True, + env=dict(os.environ, LC_ALL="C")) + assert r2.returncode == 0, r2.stderr + fdst2 = os.path.join(TEST_DATA_DIR, "tempdir_diff_fdst2") + clean_dir(fdst2) + with ServerManager() as server: + server.start() + result2, _ = run_client(source, fdst2, flags=["--temp-dir", abs_scratch], + port=server.port) + assert result2.returncode != 0, "an absolute --temp-dir must be rejected (confined)" + assert os.listdir(abs_scratch) == [], "receiver wrote into an unconfined temp dir" + def test_default_behavior_has_no_scratch_dir(self, shared_server): source = self._make_source("tempdir_default_src") dest = os.path.join(TEST_DATA_DIR, "tempdir_default_dst") diff --git a/tests/integration/test_iconv.py b/tests/integration/test_iconv.py index 4a69aa1..fe7522f 100644 --- a/tests/integration/test_iconv.py +++ b/tests/integration/test_iconv.py @@ -1,10 +1,12 @@ """--iconv=CONVERT_SPEC file-NAME charset conversion integration tests. -The client converts every source file name from LOCAL to REMOTE before it goes -on the wire, and the receiver converts it back from REMOTE to LOCAL, so a -source tree using one charset can be written into a destination tree using -another (rsync compatibility; content bytes are never touched). +rsync's spec is ``--iconv=LOCAL,REMOTE`` (the order is the same push or pull). +The sender converts each source name from LOCAL to REMOTE for the wire, and on +a PUSH the receiver's charset is the spec's REMOTE half, so it writes the wire +bytes verbatim (only a server with its own ``--iconv`` declares a different +destination charset and re-converts). Content bytes are never touched. """ +import codecs import os import shutil @@ -16,6 +18,11 @@ LATIN1_NAME = b"caf\xe9.txt" UTF8_NAME = "caf\u00e9.txt".encode("utf-8") +def _to_utf8(name_bytes): + """The UTF-8 encoding of a name that is stored as ISO-8859-1 bytes.""" + return codecs.encode(codecs.decode(name_bytes, "iso-8859-1"), "utf-8") + + def _make(tag): source = os.path.join(TEST_DATA_DIR, f"iconv_{tag}_src") dest = os.path.join(TEST_DATA_DIR, f"iconv_{tag}_dst") @@ -41,10 +48,10 @@ def _dest_file(source, dest, name): @pytest.mark.ci -def test_iconv_latin1_roundtrip(shared_server): - """A source file whose name is ISO-8859-1 bytes is transferred with - --iconv=iso-8859-1,utf-8 and lands on the destination with the ORIGINAL - latin1 name (the wire carried it as UTF-8).""" +def test_iconv_latin1_to_utf8_dest(shared_server): + """rsync push parity: --iconv=iso-8859-1,utf-8 converts a latin1 source name + to the spec's REMOTE (UTF-8) on the wire and the default receiver writes it + verbatim, so the destination name is UTF-8 (not the source's latin1).""" source, dest = _make("latin1") _place_bytes(source, LATIN1_NAME) @@ -53,8 +60,10 @@ def test_iconv_latin1_roundtrip(shared_server): ) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - dst = _dest_file(source, dest, LATIN1_NAME) - assert os.path.exists(dst), f"dest latin1-named file not found under {dest}" + dst = _dest_file(source, dest, UTF8_NAME) + assert os.path.exists(dst), f"dest UTF-8-named file not found under {dest}" + assert not os.path.exists(_dest_file(source, dest, LATIN1_NAME)), \ + "destination kept the latin1 name instead of the wire (UTF-8) charset" @pytest.mark.ci @@ -153,7 +162,7 @@ def test_iconv_expanding_name_growth(shared_server): ) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - assert os.path.exists(_dest_file(source, dest, name_bytes)) + assert os.path.exists(_dest_file(source, dest, _to_utf8(name_bytes))) def test_iconv_symlink_path_and_target(shared_server): @@ -170,11 +179,13 @@ def test_iconv_symlink_path_and_target(shared_server): ) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - dst_target = _dest_file(source, dest, target) - dst_link = _dest_file(source, dest, b"link\xe9") - assert os.path.exists(dst_target), "dest latin1 target file missing" - assert os.path.islink(dst_link), "dest latin1 symlink missing" - assert os.readlink(dst_link) == target, "symlink target not preserved/decoded" + utf8_target = _to_utf8(target) + utf8_link = _to_utf8(b"link\xe9") + dst_target = _dest_file(source, dest, utf8_target) + dst_link = _dest_file(source, dest, utf8_link) + assert os.path.exists(dst_target), "dest UTF-8 target file missing" + assert os.path.islink(dst_link), "dest UTF-8 symlink missing" + assert os.readlink(dst_link) == utf8_target, "symlink target not wire-converted" with open(dst_link, "rb") as fh: assert fh.read() == b"t\n" @@ -198,8 +209,8 @@ def test_iconv_hardlink_path_and_target(shared_server): ) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - dst_a = _dest_file(source, dest, a) - dst_b = _dest_file(source, dest, b) + dst_a = _dest_file(source, dest, _to_utf8(a)) + dst_b = _dest_file(source, dest, _to_utf8(b)) assert os.path.exists(dst_a) and os.path.exists(dst_b) assert os.stat(dst_a).st_ino == os.stat(dst_b).st_ino, \ "hard-link relationship not preserved across the transfer" @@ -221,16 +232,16 @@ def test_iconv_delete_manifest_consistent(shared_server): flags = ["--iconv=iso-8859-1,utf-8"] result, _ = run_client(source, dest, flags=flags, port=server.port) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - assert os.path.exists(_dest_file(source, dest, keep)) - assert os.path.exists(_dest_file(source, dest, gone)) + assert os.path.exists(_dest_file(source, dest, _to_utf8(keep))) + assert os.path.exists(_dest_file(source, dest, _to_utf8(gone))) os.remove(os.path.join(os.fsencode(source), gone)) result, _ = run_client( source, dest, flags=flags + ["--delete"], port=server.port ) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - assert os.path.exists(_dest_file(source, dest, keep)), "kept file deleted" - assert not os.path.exists(_dest_file(source, dest, gone)), \ + assert os.path.exists(_dest_file(source, dest, _to_utf8(keep))), "kept file deleted" + assert not os.path.exists(_dest_file(source, dest, _to_utf8(gone))), \ "missing file was not deleted" @@ -247,4 +258,4 @@ def test_iconv_chunk_serialization_blob(shared_server): ) assert result.returncode == 0, (result.stderr or result.stdout)[:400] - assert os.path.exists(_dest_file(source, dest, name)) \ No newline at end of file + assert os.path.exists(_dest_file(source, dest, _to_utf8(name))) \ No newline at end of file diff --git a/tests/test_iconv.c b/tests/test_iconv.c index da31243..3e60457 100644 --- a/tests/test_iconv.c +++ b/tests/test_iconv.c @@ -148,10 +148,23 @@ static void test_iconv_wire_sender_converts_local_to_remote() { charset_wire_free(); } -static void test_iconv_wire_receiver_converts_remote_to_local() { +/* rsync push parity: with no server --iconv the destination charset is the + * client spec's REMOTE half, so the receiver writes the wire bytes verbatim. */ +static void test_iconv_wire_receiver_default_writes_remote() { EXPECT_TRUE(charset_wire_init_receiver("utf-8,iso-8859-1", NULL)); char* local = charset_wire_apply("caf\xe9"); EXPECT_NOT_NULL(local); + EXPECT_EQ_INT(strcmp(local, "caf\xe9"), 0); + free(local); + charset_wire_free(); +} + +/* A server that declares its own --iconv LOCAL converts wire(REMOTE) into that + * declared charset (the daemon "charset" analog). */ +static void test_iconv_wire_receiver_server_local_override() { + EXPECT_TRUE(charset_wire_init_receiver("utf-8,iso-8859-1", "utf-8")); + char* local = charset_wire_apply("caf\xe9"); + EXPECT_NOT_NULL(local); EXPECT_EQ_INT(strcmp(local, "caf\xc3\xa9"), 0); free(local); charset_wire_free(); @@ -179,7 +192,7 @@ static void test_iconv_wire_str_roundtrip() { close(p[1]); io_set_fds(p[0], p[0]); charset_wire_free(); - charset_wire_init_receiver("utf-8,iso-8859-1", NULL); + charset_wire_init_receiver("utf-8,iso-8859-1", "utf-8"); char* got = receive_wire_str(p[0]); bool ok = got != NULL && strcmp(got, "caf\xc3\xa9") == 0; free(got); @@ -211,7 +224,8 @@ void test_iconv() { test_iconv_exact_fill_no_overflow(); test_iconv_growth_expanding_name(); test_iconv_wire_sender_converts_local_to_remote(); - test_iconv_wire_receiver_converts_remote_to_local(); + test_iconv_wire_receiver_default_writes_remote(); + test_iconv_wire_receiver_server_local_override(); test_iconv_wire_disabled_passthrough(); // This subtest forks to exercise the wire string handshake; the instrumented // parent is too slow under valgrind for the child's blocking reads.