Improve legacy daemon greeting validation#4
Merged
Conversation
3 tasks
oferchen
added a commit
that referenced
this pull request
Apr 30, 2026
Adds tools/ci/run_upstream_testsuite.sh, which runs upstream rsync's testsuite/*.test scripts against oc-rsync. Mirrors upstream's runtests.sh contract: exports $RSYNC, $TOOLDIR, $srcdir, $suitedir, $scratchdir, $TLS_ARGS, $POSIXLY_CORRECT, and $ECHO_* per test. Difference vs upstream runtests.sh: $RSYNC points at oc-rsync, not upstream. Upstream sources are still needed for helper tools (tls, getgroups, lsh.sh) and config artefacts (config.h, shconfig); the harness fetches and configures them on first run. Known failures live in tools/ci/upstream_testsuite_known_failures.conf and are grouped by category (lsh.sh-routed remote-shell tests, daemon-mode tests, ACL/xattr, devices, chown, hardlinks INC_RECURSE, atimes/crtimes, etc). Tests in this list are reported XFAIL on failure and UPASS on unexpected success so the list stays self-curating. Wires up risk area #4 from the audit: protocol edge-case compatibility against upstream's own conformance suite. Usage: tools/ci/run_upstream_testsuite.sh # all tests WHICHTESTS=00-hello.test tools/ci/... # one test PRESERVE_SCRATCH=yes tools/ci/... # keep per-test dirs
oferchen
added a commit
that referenced
this pull request
May 1, 2026
…3488) * docs(fast_io): design session-level io_uring ring pool (#1409) (#3444) * docs(fast_io): audit io_uring SQPOLL and DEFER_TASKRUN for socket I/O (#1267) (#3445) * docs(ci): audit Windows ACL/xattr CI matrix gaps (#1869) (#3446) * docs(fast_io): audit MADV_WILLNEED prefault for mmap'd basis files (#1662) (#3447) * docs(batch): investigate zstd as batch-compatible compression alternative (#1685) (#3448) * docs(fast_io): document mmap page-fault impact on io_uring SQPOLL (#1661) (#3449) * docs: audit --delete-during ordering vs upstream rsync 3.4.1 (#3453) Captures the audit findings from #1893 in docs/architecture/delete-during.md and lays the groundwork for the documentation work tracked in #1894. Contrasts upstream's per-directory interleaved deletion (generator.c::recv_generator + delete_in_dir) against oc-rsync's batched pre-transfer sweep (crates/transfer/src/receiver/transfer.rs:532), enumerates the resulting differences in phase ordering, determinism, filter evaluation, and error semantics, and lists the follow-up actions: CHANGELOG / man-page note, interop test for concurrent new+deleted entries, .rsync-filter investigation, and a possible --delete-strict-order opt-in. Refs #1893, #1894. * feat: add --jump-host proxy-jump CLI flag (#3454) Expose OpenSSH ProxyJump (-J) as --jump-host. Comma-separated [user@]HOST[:PORT] hops are forwarded to the remote shell as `ssh -J <value>` before the destination operand when the configured remote shell is OpenSSH. Empty values are rejected at every layer. Note: only the long form `--jump-host` is provided. Upstream rsync 3.4.1 binds `-J` to `--omit-link-times` (options.c:647), so reusing the short flag would break wire compatibility. Refs #1881 * docs: audit --iconv inert critical gap (#3455) Document the parse-but-never-applied architectural dead-end where the CLI accepts --iconv, validates and stores IconvSetting on ClientConfig, and forwards the option to the remote peer over SSH, but the local process never receives a FilenameConverter. ServerConfigBuilder::iconv() exists with zero call sites; receiver and generator already pull a converter from connection.iconv but always observe None. Severity: critical. Filenames with non-ASCII bytes silently bypass transcoding on every local-side path (file-list ingest, file-list emit, filter matching, daemon module serving) when --iconv is supplied. Companion to docs/audits/iconv-pipeline.md, narrowing on the single missing IconvSetting -> FilenameConverter bridge that currently makes every gap in that broader pipeline simultaneously unobservable. Tracks tasks #1909, #1910, #1918. Remediation path enumerated against existing #1911-#1919 task chain. * fix: normalise Windows backslash to forward slash in wire-encoded paths (#3456) A Windows oc-rsync sender that builds a `FileEntry` whose path was constructed via `Path::join` / `PathBuf::push` (the normal case for recursive transfers) emitted the native string with `\` separators verbatim on the wire. A POSIX rsync receiver decoding those bytes treated every `\` as part of a single filename, producing one literal filename per source file instead of the expected directory hierarchy. Upstream rsync 3.4.1 writes filename bytes verbatim in `flist.c:send_file_entry()` (lines 534-570). The wire format remains `/`-separated because every upstream build either runs on a POSIX kernel or under Cygwin's POSIX layer, which presents `/`-separated paths to the application before this code is reached. oc-rsync targets native Win32 directly, so the sender must perform the separator normalisation explicitly. Before this fix on Windows: let mut path = PathBuf::from(\"subdir\"); path.push(\"file.txt\"); let entry = FileEntry::new_file(path, 1024, 0o644); entry.name_bytes() == b\"subdir\\\\file.txt\" // wrong After: entry.name_bytes() == b\"subdir/file.txt\" // matches upstream The fix introduces a single-purpose helper `crates/protocol/src/flist/wire_path.rs::path_bytes_to_wire` that mirrors the existing identity-on-Unix / convert-on-Windows pattern of `wire_mode.rs`. The helper is applied at every wire-encode call site: - `FileEntry::name_bytes()` (filename emission via `write_entry`) - `write_symlink_target()` (symlink target emission) - `strip_leading_slashes()` Windows branch trims both `/` and `\` The `name_bytes()` accessor signature changes from `&[u8]` to `Cow<'_, [u8]>` so the helper can borrow on Unix (zero allocation) and own only when conversion is required on Windows. Sort comparators borrow the inner slice via `Deref` and continue to work unchanged. Tests added: - `wire_path` module: 8 unit tests covering forward-slash identity, empty path, dot path, Unix borrow, Unix backslash preservation, Windows translation, mixed separators. - `flist::write` regression tests: assert the wire-encoded filename contains no `\` byte and that a writer-then-reader roundtrip yields `subdir/file.txt` regardless of host platform. - Symlink-target regression test asserting no `\` byte appears in the encoded target bytes. Companion audit: docs/audits/windows-path.md cites upstream flist.c:534-570 and util1.c:955-961. Closes #1905 Refs #1939 * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids (#3450) * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids Closes #1879 and #1880. The previous `host_contains_colon` heuristic in `SshCommand::target_argument` treated any string containing `:` as IPv6 and wrapped it in brackets. That silently accepted malformed input such as `2001:db8:::1` or `garbage:input` and forwarded the unparseable literal to ssh. Replace it with strict `Ipv6Addr::from_str` validation via a new `parse_host_for_ssh` returning a `HostKind` enum (`Hostname` / `Ipv4` / `Ipv6 { addr, zone }`). The parser also recognises RFC 4007 scoped zone identifiers (e.g. `fe80::1%eth0`) and re-emits them inside the bracket form (`[addr%zone]`) per upstream rsync convention. Zone ids are rejected when empty or when they contain whitespace or `]`. Before: `host_contains_colon("2001:db8:::1") == true` -> emits `[2001:db8:::1]`. After: `parse_host_for_ssh("2001:db8:::1")` returns `Err(InvalidIpv6)`, so the input is passed through unchanged and ssh surfaces the resolution failure instead of receiving a malformed bracketed literal. Includes unit tests for: bare IPv4, bare hostname, bracketed IPv6, IPv6 with zone (`fe80::1%eth0` and `[fe80::1%en0]`), malformed multi-`::` IPv6, zone with whitespace (rejected), and direct `parse_host_for_ssh` classification across all `HostKind` and `BuildError` variants. * style: apply cargo fmt to ssh ipv6 validation * feat: add AppleDouble filter and optional xattr merge for macOS (#3457) * feat: add AppleDouble filter for macOS sidecar exclusion (#1907) Add the `--apple-double-skip` option which appends a perishable `._*` exclusion to the filter chain. macOS writes AppleDouble sidecar files on filesystems that cannot represent extended attributes natively (FAT, exFAT, NFS, SMB) to carry FinderInfo, resource forks, and xattrs. Replicating them across machines clutters destinations with stale metadata. The filter half of #1907 ships now. Upstream rsync 3.4.1 has no equivalent flag, so the option uses the descriptive `--apple-double-skip` name. The optional xattr merge half is deferred: the existing `apple-fs` crate does not expose AppleDouble parsing or xattr merging primitives, so a follow-up task will land that piece alongside the required parser. - New `filters::apple_double` module with the canonical `._*` pattern. - `FilterSet::from_rules_with_apple_double` mirrors the `from_rules_with_cvs` convenience constructor. - CLI plumbing through `ParsedArgs`, the command builder, the filter-rules collector, and the workflow runner. - Integration tests under `crates/filters/tests/apple_double.rs` covering top-level/nested matching, perishable semantics, override precedence, and clear-rule interaction. - Frontend tests cover argument parsing and an end-to-end transfer scenario that confirms `._foo` files are dropped while their parents copy through. - Man page and `--help` text updated. Refs #1907 * style: apply cargo fmt to apple-double filter * refactor: use shell-words crate for ssh remote-shell parsing (#3451) * refactor: use shell-words crate for ssh remote-shell parsing Replace the bespoke byte-level state machine in `parse_remote_shell` with `shell_words::split`, the well-supported POSIX shell tokenizer. - Tokenization is delegated to the `shell-words` crate (1.1). - The wrapper retains the three behaviours callers rely on: empty/whitespace input -> `Empty`, interior NUL -> `InteriorNull`, non-UTF-8 input -> `InvalidEncoding`. - The historical `UnterminatedEscape`/`UnterminatedSingleQuote`/ `UnterminatedDoubleQuote` variants collapse into a single `Parse(String)` variant carrying the `shell_words::ParseError` description. No external code matched on the removed variants. - Adds a property test corpus (`crates/rsync_io/tests/parse_remote_shell.rs`) asserting parity with `shell_words::split` over arbitrary RSYNC_RSH-style inputs, plus explicit unit tests for the historical behaviours (quoting, escapes, whitespace, NUL byte, invalid Unicode). Tasks: #1877, #1878. * style: apply cargo fmt to ssh shell-words tokenizer tests * chore: update Cargo.lock for shell-words dependency Cargo.lock was not updated when shell-words = "1.1" was added to crates/rsync_io/Cargo.toml in cc01bbc, causing the MSRV 1.88 workflow to fail with --locked. * fix(ssh): reject trailing backslash escape in remote-shell tokenizer shell_words::split silently accepts a dangling trailing backslash, but upstream rsync (and the previous bespoke tokenizer) reject it. Add a small validator that walks the string tracking single- and double-quote regions so that backslashes inside single quotes are not flagged, and returns true only when the input ends with an unescaped backslash that has no character to escape. This restores the test parser_rejects_trailing_escape that fails on the shell-words crate alone. * test: allow stricter Parse rejection vs shell_words in parity property The bespoke remote-shell tokenizer rejects dangling trailing backslashes, matching upstream rsync's `tokens = parse_arguments(...)`. The new shell-words-based wrapper preserves that strictness via has_trailing_escape, but `shell_words::split` silently absorbs the trailing backslash and returns Ok. The parity property test was therefore failing on inputs ending in a bare `\\` because parse_remote_shell returned Err(Parse) while shell_words returned Ok. The wrapper is intentionally a tightening, not a parity match in both directions. Add an explicit (Err(Parse), Ok(_)) arm so the property test encodes the contract: we may reject inputs shell_words accepts, but never the other way around. * chore: disable coverage workflow on push/pull_request (#3460) Nightly llvm-cov 22.1 SIGSEGVs deterministically inside llvm::coverage::CoverageMapping::getInstantiationGroups, breaking both master and PR runs. Restrict the workflow to manual workflow_dispatch until a known-good nightly is pinned via RUSTUP_TOOLCHAIN. * fix: avoid mmap basis when paired with io_uring writer (#3459) * fix: avoid mmap basis when paired with io_uring writer DeltaApplicator opened its basis file via MapFile::open_adaptive, which selects mmap for files >= 1 MiB on Unix. The applicator has no production caller today, but is the obvious next target for an io_uring-output rewrite. Wiring an IoUringWriter to the applicator without changing the basis path would submit mmap-backed pointers to io_uring SQEs on block-ref copies >= the writer batch threshold, exposing two failure modes: 1. Cold-page faults on the basis file are serviced under the SQE submission thread (worse, the SQPOLL kernel thread when SQPOLL is enabled), turning a "free" zero-copy write into a synchronous fault and stalling other in-flight SQEs on the same poller. 2. Concurrent truncation of the basis file raises SIGBUS while the kernel is dereferencing the page on our behalf - recovery from in-kernel SIGBUS is not signal-safe. Upstream rsync 3.4.1 deliberately avoids mmap(2) for basis files for the same truncation reason (fileio.c:214-217). Our BufferedMap matches that decision. Closes the F1 hazard from docs/audits/mmap-iouring-co-usage.md (#1660) by introducing BasisWriterKind in DeltaApplyConfig: when the writer is io_uring-backed, DeltaApplicator::new opens the basis via the new MapFile::open_adaptive_buffered (forces AdaptiveMapStrategy::Buffered regardless of size). Standard / std-write writers retain the existing adaptive selection. Non-Unix is unaffected since BufferedMap is the only basis strategy there. Tests cover both directions: io_uring writer + 2 MiB basis stays on BufferedMap, standard writer + 2 MiB basis still picks mmap on Unix. docs/design/basis-file-io-policy.md updated with implementation status noting that the io_uring-pairing axis is now wired; remaining hazard columns (--inplace, --append, --copy-devices, sparse, network FS) are future work tracked in the same doc. Refs: #1906 (this task), #1660 (audit). * fix: drop redundant Write import in applicator tests The test module brings in `Write` via `use super::*;` which re-exports the file-level `use std::io::{self, Read, Write};`. The explicit `use std::io::Write as _;` is therefore unused and trips clippy under `-D warnings`. * feat: bridge IconvSetting to FilenameConverter (#3458) Adds IconvSetting::resolve_converter() to map the CLI-side iconv setting onto a transfer-side FilenameConverter, and wires the converter through apply_common_server_flags so every SSH and daemon ServerConfig builder populates ConnectionConfig.iconv when --iconv is in effect. Without this bridge, the CLI parsed --iconv, validated it, forwarded it to the remote peer over SSH, but the local file-list reader and writer hooks always observed None and silently passed raw bytes through. Also rejects --iconv=LOCAL,REMOTE with a hard error when the iconv cargo feature is compiled out, rather than silently no-opping. Closes #1911 Closes #1915 * perf: wire IoUringDiskBatch into disk_commit hot path (#3452) * perf: wire IoUringDiskBatch into disk_commit hot path Task #1901. The disk-commit thread previously created an IoUringDiskBatch via try_create_disk_batch() but bound it to `_disk_batch`, leaving every write to fall through to the buffered path (thread.rs:127). All disk writes therefore bypassed io_uring batching even when the policy was Auto or Enabled on Linux 5.6+. Introduce a Writer enum in disk_commit/writer.rs that dispatches between ReusableBufWriter (existing 256 KB buffered path) and a new IoUring variant that borrows the disk thread's persistent IoUringDiskBatch. The IoUring variant is cfg-gated to all(target_os = "linux", feature = "io_uring") so non-Linux builds never reference it. In disk_commit/thread.rs, drop the underscore from the batch binding and thread `disk_batch.as_mut()` into both process_file and process_whole_file on every iteration of the message loop. The batch is created once per thread and reused across all files via begin_file/commit_file. In disk_commit/process.rs, replace the unconditional ReusableBufWriter construction with a make_writer() helper that registers the file with the batch when (a) the batch is present and (b) sparse mode is disabled. Sparse writes require Seek, which IoUringDiskBatch does not implement, so sparse mode keeps using the buffered writer via buffered_for_sparse(). Semantics preserved: - Temp-file commit ordering: Writer::finish() calls commit_file(do_fsync) which flushes + fsyncs + detaches the file before commit_file() runs the rename. Buffered path drops the writer (closing the file) before rename, identical to prior behaviour. - fsync: do_fsync flag flows through Writer::flush_and_sync (buffered) and Writer::finish (io_uring). The io_uring path makes flush_and_sync a no-op so commit_file performs both atomically in a single SQE. - Error propagation: io::Result is preserved end-to-end with file_path context attached on fsync/commit errors. * fix: gate unused Writer::finish parameters on non-Linux builds The do_fsync and file_path parameters on Writer::finish are only consumed by the io_uring arm. On non-Linux (or with the io_uring feature disabled) the Buffered arm ignores them, which under -D warnings escalates to a build failure. * fix(disk_commit): force buffered writer for append mode The IoUringDiskBatch writes via SQEs with absolute offsets starting at 0, ignoring the file position set by `file.seek(append_offset)` in open_output_file. This caused --append-mode transfers under io_uring to overwrite the existing file prefix with zeros, surfacing as the standalone:append interop test failing with NUL-byte corruption. Gate Writer::IoUring off whenever begin.append_offset > 0 and route those transfers through ReusableBufWriter, which honors the seek via standard Write::write_all. upstream: receiver.c:307-308 - file position is the source of truth in append mode; any writer that ignores it cannot be used. * docs(compress): convert restating comments to rustdoc in lz4 (#2026) (#3462) Remove inline comments inside lz4 codec function bodies and tests that simply echo the next statement. All public-item rustdoc and upstream rsync references (token.c) remain intact. * fix(filters): propagate P (protect) filter modifier in sender-side filter exchange (#3463) oc-rsync was emitting `P pattern` directly on the wire while also attaching the `r` (receiver-side) modifier derived from the rule's `applies_to_receiver` flag. Upstream rsync rejects `Pr` because the `P` prefix already specifies the side and `r` after a side-specifying prefix triggers the "invalid modifier" error in exclude.c:1270-1271. Upstream's `get_rule_prefix()` (exclude.c:1536-1572) only ever emits `+`, `-`, or `:` as the leading character; `P` and `R` exist only as parser sugar that lower to plain include/exclude rules carrying the `FILTRULE_RECEIVER_SIDE` flag. The wire encoding for a P rule is `-r pattern`; the wire encoding for an R rule is `+r pattern`. Normalize Protect/Risk in `build_modern_prefix` so the wire stream matches upstream byte-for-byte. The receiver-side flag is forced when serializing Protect/Risk rule types so that hand-built `FilterRuleWireFormat` values without `receiver_side=true` still serialize correctly. Reproduced and verified the fix against upstream rsync 3.4.1 daemon in the rsync-profile podman container with `--filter='P *.log'` push: protected files now survive `--delete`. Removes `standalone:delete-filter-protect` from KNOWN_FAILURES and DASHBOARD_ENTRIES. upstream: exclude.c:1536-1542 upstream: exclude.c:1569-1572 upstream: exclude.c:1201-1206 * docs(checksums): convert restating comments to rustdoc in rolling (#2018) (#3461) Cleans up the `crates/checksums/src/rolling/` test modules: - Drop decorative banner divider in front of the golden-tests block; replace with a short paragraph that describes intent and references upstream checksum.c. - Remove restating-the-code comments (e.g. "Verify it matches manual computation", "Verify rolling produces correct checksum after shift") whose information is already conveyed by the surrounding code or test name. - Sharpen wire-format comments to call out their connection to upstream rsync's packed (s2 << 16) | s1 layout. Production source files in `rolling/` already use proper rustdoc on every public item, so no `///` conversions are required there. SAFETY comments and SIMD weight-table explanations are preserved verbatim. No runtime behaviour changes. * fix(checksums): apply checksum_seed to MD4 strong checksum (upstream parity) (#3465) Upstream rsync's `get_checksum2()` (checksum.c:358-396) appends the negotiated `checksum_seed` after the data when computing MD4 block strong sums: memcpy(buf1, buf, len); if (checksum_seed) { SIVAL(buf1, len, checksum_seed); len += 4; } Our MD4 path was unconditionally calling `Md4::digest(data)` and ignoring the seed, so block-level strong checksums diverged from upstream whenever the session negotiated a non-zero seed. This silently hurt delta-transfer match quality across rsync<->oc-rsync interop and inflated `hash_hits` / `false_alarms` counters in delta-stats output. Changes: * Add `Md4::digest_with_seed(seed, data)` mirroring upstream's append-seed- after-data semantics. A zero seed is a no-op (matches upstream's `if (checksum_seed)` guard). * Add `SignatureAlgorithm::Md4Seeded { seed }` variant. Introducing a new variant rather than mutating the unit `Md4` keeps the 30+ existing call-sites that match on `Md4` working. * `ChecksumFactory::signature_algorithm()` now selects `Md4Seeded` when MD4 (or the legacy `None`/protocol<27 default) runs with a non-zero seed, and keeps unit `Md4` for the seed=0 case so existing wire goldens stay byte- identical. * Extend exhaustive matches in `signature::algorithm`, the parallel checksum executor, and the delta-transfer bench to cover `Md4Seeded`. Tests: * `md4_seeded_appends_seed_after_data` - verifies the byte layout matches building `(data || seed_le_bytes)` and hashing with plain MD4. * `md4_seeded_zero_seed_matches_unseeded` - guards the upstream zero-seed short-circuit. * `md4_seeded_negative_seed_is_le_two_complement` - confirms i32 wire encoding for negative seeds. * `md4_seeded_signature_matches_upstream_format` - parity at the `SignatureAlgorithm` layer. * Three `ChecksumFactory` tests covering MD4/None x seed=0/non-zero. upstream: checksum.c:358-396 `get_checksum2()` MD4 branch * fix(filters): correct R-before-P ordering in delete-filter-risk interop test (#3464) * fix(filters): correct R-before-P ordering in delete-filter-risk interop test The standalone:delete-filter-risk interop test had the filter rules in the wrong order: --filter='P *.log' --filter='P *.sh' --filter='R *.log'. Upstream rsync evaluates filter rules first-match-wins (exclude.c:1038-1065 check_filter()). For R (risk) to override P (protect) on *.log, it must be listed BEFORE the P rule. With R last, the P rule matched first and the *.log files were never deleted. Verified that upstream rsync 3.4.1 itself produces the same "wrong" outcome with the original P-first ordering, confirming the test expectation - not the filter engine - was incorrect. oc-rsync's filter engine already mirrors upstream semantics; the unit test risk_rule_allows_deletion_before_protection in crates/filters/src/tests.rs explicitly documents the required R-before-P order. Reordered both directions (up->oc and oc->up) to use --filter='R *.log' --filter='P *.log' --filter='P *.sh' and updated the function-level comment to cite exclude.c:1201-1207 (R rflags) and exclude.c:1038-1065 (first-match-wins). Verified end-to-end in the rsync-profile container that both upstream->oc and oc->upstream daemon pushes now produce the expected outcome: - source files transferred - risky.log + subdir/nested.log deleted (R overrides P) - keeper.sh preserved (P with no preceding R) - destonly.txt deleted (no protection) Removed the entry from KNOWN_FAILURES and DASHBOARD_ENTRIES in tools/ci/known_failures.conf. upstream: exclude.c:1201-1207 'R' = FILTRULE_INCLUDE|FILTRULE_RECEIVER_SIDE upstream: exclude.c:1038-1065 check_filter() first-match-wins * fix(filters): suppress redundant r/s modifier on Risk/Protect wire prefix The wire encoder unconditionally emitted `s`/`r` modifiers when the sender_side/receiver_side flags were set on a FilterRuleWireFormat. This produced output like `Rr *.log` for a Risk rule (which oc-rsync internally tags as receiver-side). Upstream rsync's parser rejects that with `invalid modifier 'r' at position 1 in filter rule`, breaking the delete-filter-risk interop scenario in oc->upstream direction with exit 12. Upstream's filter parser (exclude.c:1180-1207) sets prefix_specifies_side when the rule type char is R/P (and S/H), and exclude.c:1269-1278 rejects an explicit `r`/`s` modifier in that case. Match upstream's get_rule_prefix (exclude.c:1525-1587), which never emits the redundant flag for those prefixes. Suppress `s`/`r` emission when the rule type is Risk or Protect. Add unit tests at the prefix builder level and wire-level tests including a Risk roundtrip to lock in the format. * docs(protocol): convert restating comments to rustdoc in varint (#1999) (#3466) Remove decorative banner comments from the varint test module and tighten internal comments in `read_varlong` to delete pure restatement while preserving upstream rsync C source references (`io.c:read_varlong`, `memcpy(u.b, b2+1, min_bytes-1)`). No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in zlib (#2029) (#3467) Remove decorative and restating-the-code comments from the zlib module's tests, while preserving every upstream rsync C source reference and comments that explain non-obvious test design rationale. Non-test files (decoder.rs, encoder.rs, helpers.rs, level.rs, mod.rs) already use proper rustdoc and required no changes. * docs: clean up comments in core::timeout (#2014) (#3471) * docs(signature): convert restating comments to rustdoc (#2034) (#3474) Drops inline comments that merely echo the immediately following code in block_size.rs, pipelined_gen.rs, and async_gen.rs. Reworks the few that remain to focus on the WHY (upstream-derived phase 2 short-circuit, sqrt bound derivation, BATCH_SIZE cross-file consistency, disk I/O thread cap). Comment-only change with no functional impact. * docs(metadata): convert restating comments to rustdoc in chmod (#2022) (#3473) Remove section-banner comments and pure-restatement comments from chmod test modules. Convert a stranded `///` doc-comment in `upstream_compatibility` to a proper `//!` module doc. Preserve all comments that reference upstream rsync semantics or that explain the non-obvious conditional-X / assign-empty-perms / octal-validation rules. No functional code changes. * feat(ci): add upstream rsync testsuite harness (#3470) Adds tools/ci/run_upstream_testsuite.sh, which runs upstream rsync's testsuite/*.test scripts against oc-rsync. Mirrors upstream's runtests.sh contract: exports $RSYNC, $TOOLDIR, $srcdir, $suitedir, $scratchdir, $TLS_ARGS, $POSIXLY_CORRECT, and $ECHO_* per test. Difference vs upstream runtests.sh: $RSYNC points at oc-rsync, not upstream. Upstream sources are still needed for helper tools (tls, getgroups, lsh.sh) and config artefacts (config.h, shconfig); the harness fetches and configures them on first run. Known failures live in tools/ci/upstream_testsuite_known_failures.conf and are grouped by category (lsh.sh-routed remote-shell tests, daemon-mode tests, ACL/xattr, devices, chown, hardlinks INC_RECURSE, atimes/crtimes, etc). Tests in this list are reported XFAIL on failure and UPASS on unexpected success so the list stays self-curating. Wires up risk area #4 from the audit: protocol edge-case compatibility against upstream's own conformance suite. Usage: tools/ci/run_upstream_testsuite.sh # all tests WHICHTESTS=00-hello.test tools/ci/... # one test PRESERVE_SCRATCH=yes tools/ci/... # keep per-test dirs * docs: clean up comments in protocol::error_recovery (#1985) (#3472) Remove restatement and decorative banner comments that echo the code without adding value. Replace two banner-style remarks with focused notes that explain non-obvious behavior (over-receive handling, why HashMap re-insert is the desired semantic, the conservative default classification, and the resume-vs-retry policy on transient errors). Scope is limited to crates/protocol/src/error_recovery/. No functional changes; tests and public APIs are unchanged. Refs #1985 * docs(checksums): convert restating comments to rustdoc in strong (#2020) (#3469) Audit of `crates/checksums/src/strong/` (excluding `strategy/`, `md4_tests.rs`, `xxh64_tests.rs` per task scope). Production source files are already well-documented from prior cleanup passes: every public type, trait, function, and method carries rustdoc; upstream rsync references (`upstream: checksum.c:get_checksum2()` and similar) are preserved on the seed-handling paths in `md4.rs` and `md5.rs`; no decorative banners, debug checkpoints, or commented-out code remain in scope. The single substantive change in this pass adds a quirk comment to the private `detect()` helper in `openssl_support.rs` that captures non-obvious upstream behaviour: MD4 may be absent on OpenSSL builds without the legacy provider, so the MD4 probe is best-effort and overall detection succeeds as long as MD5 works. This explains the otherwise-mysterious `if let Some(md4) = ...` whose result is deliberately discarded. No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in strategy (#2028) (#3468) Remove decorative banner/divider comments from the compress::strategy module. The module's rustdoc was already comprehensive; this cleanup removes the remaining `// ---- Section ----` test-grouping banners that violate the no-decorative-divider rule from CLAUDE.md. All upstream rsync C source references and load-bearing internal comments are preserved verbatim. No production code, function bodies, signatures, types, or imports were modified. * docs: add v0.6 known limitations and security hardening notes Adds a "Known Limitations / Architectural Trade-offs" section to README.md covering PBUF_RING 5.19+ requirement, io_uring buffer pool ceiling, SSH double-compression interaction, single-thread delta computation, daemon TLS gap, and Windows IOCP wiring status (#1868). Updates SECURITY.md supported-versions table to 0.6.x and adds a "Hardening Notes" section covering recycle_buffer release-mode bounds-check gap, bgid u16 namespace exhaustion, SSH double-compression amplification surface, daemon TLS-in-front guidance (stunnel / SSH tunnel / reverse proxy), and daemon module hardening defaults. Cross-links the two documents so operators planning a deployment have a single source of truth for the documented trade-offs. * docs: comment cleanup in protocol::flist::hardlink Remove restating-the-code comments from the hardlink test suite and fix two malformed upstream references in types.rs that used `// upstream:` inside rustdoc blocks (rendered literally in generated docs). Preserved: all upstream rsync source references, non-obvious WHY explanations (FxHash collision rationale, swapped-value asymmetry, weak-mixing input categorisation), and module-level documentation. * docs(filters): audit .rsync-filter per-directory inheritance (#2050) Compare oc-rsync's per-directory merge file inheritance against upstream rsync 3.4.1. Documents the push/pop lifecycle, modifier semantics (n, e, p, s, r, !), and parent-dirscan behaviour. Identifies three behavioural bugs in FilterChain (n modifier ignored, !/clear not propagated to scopes, include-only scopes cannot override outer excludes), one intentional divergence (parent_dirscan), and one untested area (nested dir-merge declarations).
oferchen
added a commit
that referenced
this pull request
May 1, 2026
* docs(fast_io): design session-level io_uring ring pool (#1409) (#3444) * docs(fast_io): audit io_uring SQPOLL and DEFER_TASKRUN for socket I/O (#1267) (#3445) * docs(ci): audit Windows ACL/xattr CI matrix gaps (#1869) (#3446) * docs(fast_io): audit MADV_WILLNEED prefault for mmap'd basis files (#1662) (#3447) * docs(batch): investigate zstd as batch-compatible compression alternative (#1685) (#3448) * docs(fast_io): document mmap page-fault impact on io_uring SQPOLL (#1661) (#3449) * docs: audit --delete-during ordering vs upstream rsync 3.4.1 (#3453) Captures the audit findings from #1893 in docs/architecture/delete-during.md and lays the groundwork for the documentation work tracked in #1894. Contrasts upstream's per-directory interleaved deletion (generator.c::recv_generator + delete_in_dir) against oc-rsync's batched pre-transfer sweep (crates/transfer/src/receiver/transfer.rs:532), enumerates the resulting differences in phase ordering, determinism, filter evaluation, and error semantics, and lists the follow-up actions: CHANGELOG / man-page note, interop test for concurrent new+deleted entries, .rsync-filter investigation, and a possible --delete-strict-order opt-in. Refs #1893, #1894. * feat: add --jump-host proxy-jump CLI flag (#3454) Expose OpenSSH ProxyJump (-J) as --jump-host. Comma-separated [user@]HOST[:PORT] hops are forwarded to the remote shell as `ssh -J <value>` before the destination operand when the configured remote shell is OpenSSH. Empty values are rejected at every layer. Note: only the long form `--jump-host` is provided. Upstream rsync 3.4.1 binds `-J` to `--omit-link-times` (options.c:647), so reusing the short flag would break wire compatibility. Refs #1881 * docs: audit --iconv inert critical gap (#3455) Document the parse-but-never-applied architectural dead-end where the CLI accepts --iconv, validates and stores IconvSetting on ClientConfig, and forwards the option to the remote peer over SSH, but the local process never receives a FilenameConverter. ServerConfigBuilder::iconv() exists with zero call sites; receiver and generator already pull a converter from connection.iconv but always observe None. Severity: critical. Filenames with non-ASCII bytes silently bypass transcoding on every local-side path (file-list ingest, file-list emit, filter matching, daemon module serving) when --iconv is supplied. Companion to docs/audits/iconv-pipeline.md, narrowing on the single missing IconvSetting -> FilenameConverter bridge that currently makes every gap in that broader pipeline simultaneously unobservable. Tracks tasks #1909, #1910, #1918. Remediation path enumerated against existing #1911-#1919 task chain. * fix: normalise Windows backslash to forward slash in wire-encoded paths (#3456) A Windows oc-rsync sender that builds a `FileEntry` whose path was constructed via `Path::join` / `PathBuf::push` (the normal case for recursive transfers) emitted the native string with `\` separators verbatim on the wire. A POSIX rsync receiver decoding those bytes treated every `\` as part of a single filename, producing one literal filename per source file instead of the expected directory hierarchy. Upstream rsync 3.4.1 writes filename bytes verbatim in `flist.c:send_file_entry()` (lines 534-570). The wire format remains `/`-separated because every upstream build either runs on a POSIX kernel or under Cygwin's POSIX layer, which presents `/`-separated paths to the application before this code is reached. oc-rsync targets native Win32 directly, so the sender must perform the separator normalisation explicitly. Before this fix on Windows: let mut path = PathBuf::from(\"subdir\"); path.push(\"file.txt\"); let entry = FileEntry::new_file(path, 1024, 0o644); entry.name_bytes() == b\"subdir\\\\file.txt\" // wrong After: entry.name_bytes() == b\"subdir/file.txt\" // matches upstream The fix introduces a single-purpose helper `crates/protocol/src/flist/wire_path.rs::path_bytes_to_wire` that mirrors the existing identity-on-Unix / convert-on-Windows pattern of `wire_mode.rs`. The helper is applied at every wire-encode call site: - `FileEntry::name_bytes()` (filename emission via `write_entry`) - `write_symlink_target()` (symlink target emission) - `strip_leading_slashes()` Windows branch trims both `/` and `\` The `name_bytes()` accessor signature changes from `&[u8]` to `Cow<'_, [u8]>` so the helper can borrow on Unix (zero allocation) and own only when conversion is required on Windows. Sort comparators borrow the inner slice via `Deref` and continue to work unchanged. Tests added: - `wire_path` module: 8 unit tests covering forward-slash identity, empty path, dot path, Unix borrow, Unix backslash preservation, Windows translation, mixed separators. - `flist::write` regression tests: assert the wire-encoded filename contains no `\` byte and that a writer-then-reader roundtrip yields `subdir/file.txt` regardless of host platform. - Symlink-target regression test asserting no `\` byte appears in the encoded target bytes. Companion audit: docs/audits/windows-path.md cites upstream flist.c:534-570 and util1.c:955-961. Closes #1905 Refs #1939 * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids (#3450) * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids Closes #1879 and #1880. The previous `host_contains_colon` heuristic in `SshCommand::target_argument` treated any string containing `:` as IPv6 and wrapped it in brackets. That silently accepted malformed input such as `2001:db8:::1` or `garbage:input` and forwarded the unparseable literal to ssh. Replace it with strict `Ipv6Addr::from_str` validation via a new `parse_host_for_ssh` returning a `HostKind` enum (`Hostname` / `Ipv4` / `Ipv6 { addr, zone }`). The parser also recognises RFC 4007 scoped zone identifiers (e.g. `fe80::1%eth0`) and re-emits them inside the bracket form (`[addr%zone]`) per upstream rsync convention. Zone ids are rejected when empty or when they contain whitespace or `]`. Before: `host_contains_colon("2001:db8:::1") == true` -> emits `[2001:db8:::1]`. After: `parse_host_for_ssh("2001:db8:::1")` returns `Err(InvalidIpv6)`, so the input is passed through unchanged and ssh surfaces the resolution failure instead of receiving a malformed bracketed literal. Includes unit tests for: bare IPv4, bare hostname, bracketed IPv6, IPv6 with zone (`fe80::1%eth0` and `[fe80::1%en0]`), malformed multi-`::` IPv6, zone with whitespace (rejected), and direct `parse_host_for_ssh` classification across all `HostKind` and `BuildError` variants. * style: apply cargo fmt to ssh ipv6 validation * feat: add AppleDouble filter and optional xattr merge for macOS (#3457) * feat: add AppleDouble filter for macOS sidecar exclusion (#1907) Add the `--apple-double-skip` option which appends a perishable `._*` exclusion to the filter chain. macOS writes AppleDouble sidecar files on filesystems that cannot represent extended attributes natively (FAT, exFAT, NFS, SMB) to carry FinderInfo, resource forks, and xattrs. Replicating them across machines clutters destinations with stale metadata. The filter half of #1907 ships now. Upstream rsync 3.4.1 has no equivalent flag, so the option uses the descriptive `--apple-double-skip` name. The optional xattr merge half is deferred: the existing `apple-fs` crate does not expose AppleDouble parsing or xattr merging primitives, so a follow-up task will land that piece alongside the required parser. - New `filters::apple_double` module with the canonical `._*` pattern. - `FilterSet::from_rules_with_apple_double` mirrors the `from_rules_with_cvs` convenience constructor. - CLI plumbing through `ParsedArgs`, the command builder, the filter-rules collector, and the workflow runner. - Integration tests under `crates/filters/tests/apple_double.rs` covering top-level/nested matching, perishable semantics, override precedence, and clear-rule interaction. - Frontend tests cover argument parsing and an end-to-end transfer scenario that confirms `._foo` files are dropped while their parents copy through. - Man page and `--help` text updated. Refs #1907 * style: apply cargo fmt to apple-double filter * refactor: use shell-words crate for ssh remote-shell parsing (#3451) * refactor: use shell-words crate for ssh remote-shell parsing Replace the bespoke byte-level state machine in `parse_remote_shell` with `shell_words::split`, the well-supported POSIX shell tokenizer. - Tokenization is delegated to the `shell-words` crate (1.1). - The wrapper retains the three behaviours callers rely on: empty/whitespace input -> `Empty`, interior NUL -> `InteriorNull`, non-UTF-8 input -> `InvalidEncoding`. - The historical `UnterminatedEscape`/`UnterminatedSingleQuote`/ `UnterminatedDoubleQuote` variants collapse into a single `Parse(String)` variant carrying the `shell_words::ParseError` description. No external code matched on the removed variants. - Adds a property test corpus (`crates/rsync_io/tests/parse_remote_shell.rs`) asserting parity with `shell_words::split` over arbitrary RSYNC_RSH-style inputs, plus explicit unit tests for the historical behaviours (quoting, escapes, whitespace, NUL byte, invalid Unicode). Tasks: #1877, #1878. * style: apply cargo fmt to ssh shell-words tokenizer tests * chore: update Cargo.lock for shell-words dependency Cargo.lock was not updated when shell-words = "1.1" was added to crates/rsync_io/Cargo.toml in cc01bbc, causing the MSRV 1.88 workflow to fail with --locked. * fix(ssh): reject trailing backslash escape in remote-shell tokenizer shell_words::split silently accepts a dangling trailing backslash, but upstream rsync (and the previous bespoke tokenizer) reject it. Add a small validator that walks the string tracking single- and double-quote regions so that backslashes inside single quotes are not flagged, and returns true only when the input ends with an unescaped backslash that has no character to escape. This restores the test parser_rejects_trailing_escape that fails on the shell-words crate alone. * test: allow stricter Parse rejection vs shell_words in parity property The bespoke remote-shell tokenizer rejects dangling trailing backslashes, matching upstream rsync's `tokens = parse_arguments(...)`. The new shell-words-based wrapper preserves that strictness via has_trailing_escape, but `shell_words::split` silently absorbs the trailing backslash and returns Ok. The parity property test was therefore failing on inputs ending in a bare `\\` because parse_remote_shell returned Err(Parse) while shell_words returned Ok. The wrapper is intentionally a tightening, not a parity match in both directions. Add an explicit (Err(Parse), Ok(_)) arm so the property test encodes the contract: we may reject inputs shell_words accepts, but never the other way around. * chore: disable coverage workflow on push/pull_request (#3460) Nightly llvm-cov 22.1 SIGSEGVs deterministically inside llvm::coverage::CoverageMapping::getInstantiationGroups, breaking both master and PR runs. Restrict the workflow to manual workflow_dispatch until a known-good nightly is pinned via RUSTUP_TOOLCHAIN. * fix: avoid mmap basis when paired with io_uring writer (#3459) * fix: avoid mmap basis when paired with io_uring writer DeltaApplicator opened its basis file via MapFile::open_adaptive, which selects mmap for files >= 1 MiB on Unix. The applicator has no production caller today, but is the obvious next target for an io_uring-output rewrite. Wiring an IoUringWriter to the applicator without changing the basis path would submit mmap-backed pointers to io_uring SQEs on block-ref copies >= the writer batch threshold, exposing two failure modes: 1. Cold-page faults on the basis file are serviced under the SQE submission thread (worse, the SQPOLL kernel thread when SQPOLL is enabled), turning a "free" zero-copy write into a synchronous fault and stalling other in-flight SQEs on the same poller. 2. Concurrent truncation of the basis file raises SIGBUS while the kernel is dereferencing the page on our behalf - recovery from in-kernel SIGBUS is not signal-safe. Upstream rsync 3.4.1 deliberately avoids mmap(2) for basis files for the same truncation reason (fileio.c:214-217). Our BufferedMap matches that decision. Closes the F1 hazard from docs/audits/mmap-iouring-co-usage.md (#1660) by introducing BasisWriterKind in DeltaApplyConfig: when the writer is io_uring-backed, DeltaApplicator::new opens the basis via the new MapFile::open_adaptive_buffered (forces AdaptiveMapStrategy::Buffered regardless of size). Standard / std-write writers retain the existing adaptive selection. Non-Unix is unaffected since BufferedMap is the only basis strategy there. Tests cover both directions: io_uring writer + 2 MiB basis stays on BufferedMap, standard writer + 2 MiB basis still picks mmap on Unix. docs/design/basis-file-io-policy.md updated with implementation status noting that the io_uring-pairing axis is now wired; remaining hazard columns (--inplace, --append, --copy-devices, sparse, network FS) are future work tracked in the same doc. Refs: #1906 (this task), #1660 (audit). * fix: drop redundant Write import in applicator tests The test module brings in `Write` via `use super::*;` which re-exports the file-level `use std::io::{self, Read, Write};`. The explicit `use std::io::Write as _;` is therefore unused and trips clippy under `-D warnings`. * feat: bridge IconvSetting to FilenameConverter (#3458) Adds IconvSetting::resolve_converter() to map the CLI-side iconv setting onto a transfer-side FilenameConverter, and wires the converter through apply_common_server_flags so every SSH and daemon ServerConfig builder populates ConnectionConfig.iconv when --iconv is in effect. Without this bridge, the CLI parsed --iconv, validated it, forwarded it to the remote peer over SSH, but the local file-list reader and writer hooks always observed None and silently passed raw bytes through. Also rejects --iconv=LOCAL,REMOTE with a hard error when the iconv cargo feature is compiled out, rather than silently no-opping. Closes #1911 Closes #1915 * perf: wire IoUringDiskBatch into disk_commit hot path (#3452) * perf: wire IoUringDiskBatch into disk_commit hot path Task #1901. The disk-commit thread previously created an IoUringDiskBatch via try_create_disk_batch() but bound it to `_disk_batch`, leaving every write to fall through to the buffered path (thread.rs:127). All disk writes therefore bypassed io_uring batching even when the policy was Auto or Enabled on Linux 5.6+. Introduce a Writer enum in disk_commit/writer.rs that dispatches between ReusableBufWriter (existing 256 KB buffered path) and a new IoUring variant that borrows the disk thread's persistent IoUringDiskBatch. The IoUring variant is cfg-gated to all(target_os = "linux", feature = "io_uring") so non-Linux builds never reference it. In disk_commit/thread.rs, drop the underscore from the batch binding and thread `disk_batch.as_mut()` into both process_file and process_whole_file on every iteration of the message loop. The batch is created once per thread and reused across all files via begin_file/commit_file. In disk_commit/process.rs, replace the unconditional ReusableBufWriter construction with a make_writer() helper that registers the file with the batch when (a) the batch is present and (b) sparse mode is disabled. Sparse writes require Seek, which IoUringDiskBatch does not implement, so sparse mode keeps using the buffered writer via buffered_for_sparse(). Semantics preserved: - Temp-file commit ordering: Writer::finish() calls commit_file(do_fsync) which flushes + fsyncs + detaches the file before commit_file() runs the rename. Buffered path drops the writer (closing the file) before rename, identical to prior behaviour. - fsync: do_fsync flag flows through Writer::flush_and_sync (buffered) and Writer::finish (io_uring). The io_uring path makes flush_and_sync a no-op so commit_file performs both atomically in a single SQE. - Error propagation: io::Result is preserved end-to-end with file_path context attached on fsync/commit errors. * fix: gate unused Writer::finish parameters on non-Linux builds The do_fsync and file_path parameters on Writer::finish are only consumed by the io_uring arm. On non-Linux (or with the io_uring feature disabled) the Buffered arm ignores them, which under -D warnings escalates to a build failure. * fix(disk_commit): force buffered writer for append mode The IoUringDiskBatch writes via SQEs with absolute offsets starting at 0, ignoring the file position set by `file.seek(append_offset)` in open_output_file. This caused --append-mode transfers under io_uring to overwrite the existing file prefix with zeros, surfacing as the standalone:append interop test failing with NUL-byte corruption. Gate Writer::IoUring off whenever begin.append_offset > 0 and route those transfers through ReusableBufWriter, which honors the seek via standard Write::write_all. upstream: receiver.c:307-308 - file position is the source of truth in append mode; any writer that ignores it cannot be used. * docs(compress): convert restating comments to rustdoc in lz4 (#2026) (#3462) Remove inline comments inside lz4 codec function bodies and tests that simply echo the next statement. All public-item rustdoc and upstream rsync references (token.c) remain intact. * fix(filters): propagate P (protect) filter modifier in sender-side filter exchange (#3463) oc-rsync was emitting `P pattern` directly on the wire while also attaching the `r` (receiver-side) modifier derived from the rule's `applies_to_receiver` flag. Upstream rsync rejects `Pr` because the `P` prefix already specifies the side and `r` after a side-specifying prefix triggers the "invalid modifier" error in exclude.c:1270-1271. Upstream's `get_rule_prefix()` (exclude.c:1536-1572) only ever emits `+`, `-`, or `:` as the leading character; `P` and `R` exist only as parser sugar that lower to plain include/exclude rules carrying the `FILTRULE_RECEIVER_SIDE` flag. The wire encoding for a P rule is `-r pattern`; the wire encoding for an R rule is `+r pattern`. Normalize Protect/Risk in `build_modern_prefix` so the wire stream matches upstream byte-for-byte. The receiver-side flag is forced when serializing Protect/Risk rule types so that hand-built `FilterRuleWireFormat` values without `receiver_side=true` still serialize correctly. Reproduced and verified the fix against upstream rsync 3.4.1 daemon in the rsync-profile podman container with `--filter='P *.log'` push: protected files now survive `--delete`. Removes `standalone:delete-filter-protect` from KNOWN_FAILURES and DASHBOARD_ENTRIES. upstream: exclude.c:1536-1542 upstream: exclude.c:1569-1572 upstream: exclude.c:1201-1206 * docs(checksums): convert restating comments to rustdoc in rolling (#2018) (#3461) Cleans up the `crates/checksums/src/rolling/` test modules: - Drop decorative banner divider in front of the golden-tests block; replace with a short paragraph that describes intent and references upstream checksum.c. - Remove restating-the-code comments (e.g. "Verify it matches manual computation", "Verify rolling produces correct checksum after shift") whose information is already conveyed by the surrounding code or test name. - Sharpen wire-format comments to call out their connection to upstream rsync's packed (s2 << 16) | s1 layout. Production source files in `rolling/` already use proper rustdoc on every public item, so no `///` conversions are required there. SAFETY comments and SIMD weight-table explanations are preserved verbatim. No runtime behaviour changes. * fix(checksums): apply checksum_seed to MD4 strong checksum (upstream parity) (#3465) Upstream rsync's `get_checksum2()` (checksum.c:358-396) appends the negotiated `checksum_seed` after the data when computing MD4 block strong sums: memcpy(buf1, buf, len); if (checksum_seed) { SIVAL(buf1, len, checksum_seed); len += 4; } Our MD4 path was unconditionally calling `Md4::digest(data)` and ignoring the seed, so block-level strong checksums diverged from upstream whenever the session negotiated a non-zero seed. This silently hurt delta-transfer match quality across rsync<->oc-rsync interop and inflated `hash_hits` / `false_alarms` counters in delta-stats output. Changes: * Add `Md4::digest_with_seed(seed, data)` mirroring upstream's append-seed- after-data semantics. A zero seed is a no-op (matches upstream's `if (checksum_seed)` guard). * Add `SignatureAlgorithm::Md4Seeded { seed }` variant. Introducing a new variant rather than mutating the unit `Md4` keeps the 30+ existing call-sites that match on `Md4` working. * `ChecksumFactory::signature_algorithm()` now selects `Md4Seeded` when MD4 (or the legacy `None`/protocol<27 default) runs with a non-zero seed, and keeps unit `Md4` for the seed=0 case so existing wire goldens stay byte- identical. * Extend exhaustive matches in `signature::algorithm`, the parallel checksum executor, and the delta-transfer bench to cover `Md4Seeded`. Tests: * `md4_seeded_appends_seed_after_data` - verifies the byte layout matches building `(data || seed_le_bytes)` and hashing with plain MD4. * `md4_seeded_zero_seed_matches_unseeded` - guards the upstream zero-seed short-circuit. * `md4_seeded_negative_seed_is_le_two_complement` - confirms i32 wire encoding for negative seeds. * `md4_seeded_signature_matches_upstream_format` - parity at the `SignatureAlgorithm` layer. * Three `ChecksumFactory` tests covering MD4/None x seed=0/non-zero. upstream: checksum.c:358-396 `get_checksum2()` MD4 branch * fix(filters): correct R-before-P ordering in delete-filter-risk interop test (#3464) * fix(filters): correct R-before-P ordering in delete-filter-risk interop test The standalone:delete-filter-risk interop test had the filter rules in the wrong order: --filter='P *.log' --filter='P *.sh' --filter='R *.log'. Upstream rsync evaluates filter rules first-match-wins (exclude.c:1038-1065 check_filter()). For R (risk) to override P (protect) on *.log, it must be listed BEFORE the P rule. With R last, the P rule matched first and the *.log files were never deleted. Verified that upstream rsync 3.4.1 itself produces the same "wrong" outcome with the original P-first ordering, confirming the test expectation - not the filter engine - was incorrect. oc-rsync's filter engine already mirrors upstream semantics; the unit test risk_rule_allows_deletion_before_protection in crates/filters/src/tests.rs explicitly documents the required R-before-P order. Reordered both directions (up->oc and oc->up) to use --filter='R *.log' --filter='P *.log' --filter='P *.sh' and updated the function-level comment to cite exclude.c:1201-1207 (R rflags) and exclude.c:1038-1065 (first-match-wins). Verified end-to-end in the rsync-profile container that both upstream->oc and oc->upstream daemon pushes now produce the expected outcome: - source files transferred - risky.log + subdir/nested.log deleted (R overrides P) - keeper.sh preserved (P with no preceding R) - destonly.txt deleted (no protection) Removed the entry from KNOWN_FAILURES and DASHBOARD_ENTRIES in tools/ci/known_failures.conf. upstream: exclude.c:1201-1207 'R' = FILTRULE_INCLUDE|FILTRULE_RECEIVER_SIDE upstream: exclude.c:1038-1065 check_filter() first-match-wins * fix(filters): suppress redundant r/s modifier on Risk/Protect wire prefix The wire encoder unconditionally emitted `s`/`r` modifiers when the sender_side/receiver_side flags were set on a FilterRuleWireFormat. This produced output like `Rr *.log` for a Risk rule (which oc-rsync internally tags as receiver-side). Upstream rsync's parser rejects that with `invalid modifier 'r' at position 1 in filter rule`, breaking the delete-filter-risk interop scenario in oc->upstream direction with exit 12. Upstream's filter parser (exclude.c:1180-1207) sets prefix_specifies_side when the rule type char is R/P (and S/H), and exclude.c:1269-1278 rejects an explicit `r`/`s` modifier in that case. Match upstream's get_rule_prefix (exclude.c:1525-1587), which never emits the redundant flag for those prefixes. Suppress `s`/`r` emission when the rule type is Risk or Protect. Add unit tests at the prefix builder level and wire-level tests including a Risk roundtrip to lock in the format. * docs(protocol): convert restating comments to rustdoc in varint (#1999) (#3466) Remove decorative banner comments from the varint test module and tighten internal comments in `read_varlong` to delete pure restatement while preserving upstream rsync C source references (`io.c:read_varlong`, `memcpy(u.b, b2+1, min_bytes-1)`). No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in zlib (#2029) (#3467) Remove decorative and restating-the-code comments from the zlib module's tests, while preserving every upstream rsync C source reference and comments that explain non-obvious test design rationale. Non-test files (decoder.rs, encoder.rs, helpers.rs, level.rs, mod.rs) already use proper rustdoc and required no changes. * docs: clean up comments in core::timeout (#2014) (#3471) * docs(signature): convert restating comments to rustdoc (#2034) (#3474) Drops inline comments that merely echo the immediately following code in block_size.rs, pipelined_gen.rs, and async_gen.rs. Reworks the few that remain to focus on the WHY (upstream-derived phase 2 short-circuit, sqrt bound derivation, BATCH_SIZE cross-file consistency, disk I/O thread cap). Comment-only change with no functional impact. * docs(metadata): convert restating comments to rustdoc in chmod (#2022) (#3473) Remove section-banner comments and pure-restatement comments from chmod test modules. Convert a stranded `///` doc-comment in `upstream_compatibility` to a proper `//!` module doc. Preserve all comments that reference upstream rsync semantics or that explain the non-obvious conditional-X / assign-empty-perms / octal-validation rules. No functional code changes. * feat(ci): add upstream rsync testsuite harness (#3470) Adds tools/ci/run_upstream_testsuite.sh, which runs upstream rsync's testsuite/*.test scripts against oc-rsync. Mirrors upstream's runtests.sh contract: exports $RSYNC, $TOOLDIR, $srcdir, $suitedir, $scratchdir, $TLS_ARGS, $POSIXLY_CORRECT, and $ECHO_* per test. Difference vs upstream runtests.sh: $RSYNC points at oc-rsync, not upstream. Upstream sources are still needed for helper tools (tls, getgroups, lsh.sh) and config artefacts (config.h, shconfig); the harness fetches and configures them on first run. Known failures live in tools/ci/upstream_testsuite_known_failures.conf and are grouped by category (lsh.sh-routed remote-shell tests, daemon-mode tests, ACL/xattr, devices, chown, hardlinks INC_RECURSE, atimes/crtimes, etc). Tests in this list are reported XFAIL on failure and UPASS on unexpected success so the list stays self-curating. Wires up risk area #4 from the audit: protocol edge-case compatibility against upstream's own conformance suite. Usage: tools/ci/run_upstream_testsuite.sh # all tests WHICHTESTS=00-hello.test tools/ci/... # one test PRESERVE_SCRATCH=yes tools/ci/... # keep per-test dirs * docs: clean up comments in protocol::error_recovery (#1985) (#3472) Remove restatement and decorative banner comments that echo the code without adding value. Replace two banner-style remarks with focused notes that explain non-obvious behavior (over-receive handling, why HashMap re-insert is the desired semantic, the conservative default classification, and the resume-vs-retry policy on transient errors). Scope is limited to crates/protocol/src/error_recovery/. No functional changes; tests and public APIs are unchanged. Refs #1985 * docs(checksums): convert restating comments to rustdoc in strong (#2020) (#3469) Audit of `crates/checksums/src/strong/` (excluding `strategy/`, `md4_tests.rs`, `xxh64_tests.rs` per task scope). Production source files are already well-documented from prior cleanup passes: every public type, trait, function, and method carries rustdoc; upstream rsync references (`upstream: checksum.c:get_checksum2()` and similar) are preserved on the seed-handling paths in `md4.rs` and `md5.rs`; no decorative banners, debug checkpoints, or commented-out code remain in scope. The single substantive change in this pass adds a quirk comment to the private `detect()` helper in `openssl_support.rs` that captures non-obvious upstream behaviour: MD4 may be absent on OpenSSL builds without the legacy provider, so the MD4 probe is best-effort and overall detection succeeds as long as MD5 works. This explains the otherwise-mysterious `if let Some(md4) = ...` whose result is deliberately discarded. No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in strategy (#2028) (#3468) Remove decorative banner/divider comments from the compress::strategy module. The module's rustdoc was already comprehensive; this cleanup removes the remaining `// ---- Section ----` test-grouping banners that violate the no-decorative-divider rule from CLAUDE.md. All upstream rsync C source references and load-bearing internal comments are preserved verbatim. No production code, function bodies, signatures, types, or imports were modified. * docs: add v0.6 known limitations and security hardening notes Adds a "Known Limitations / Architectural Trade-offs" section to README.md covering PBUF_RING 5.19+ requirement, io_uring buffer pool ceiling, SSH double-compression interaction, single-thread delta computation, daemon TLS gap, and Windows IOCP wiring status (#1868). Updates SECURITY.md supported-versions table to 0.6.x and adds a "Hardening Notes" section covering recycle_buffer release-mode bounds-check gap, bgid u16 namespace exhaustion, SSH double-compression amplification surface, daemon TLS-in-front guidance (stunnel / SSH tunnel / reverse proxy), and daemon module hardening defaults. Cross-links the two documents so operators planning a deployment have a single source of truth for the documented trade-offs. * docs: comment cleanup in protocol::flist::hardlink Remove restating-the-code comments from the hardlink test suite and fix two malformed upstream references in types.rs that used `// upstream:` inside rustdoc blocks (rendered literally in generated docs). Preserved: all upstream rsync source references, non-obvious WHY explanations (FxHash collision rationale, swapped-value asymmetry, weak-mixing input categorisation), and module-level documentation. * docs(engine): audit --fuzzy basis-file search algorithm (#2051) Document oc-rsync's --fuzzy / -y basis-file search and scoring against upstream rsync 3.4.1 (generator.c::find_fuzzy() and util1.c::fuzzy_distance()). Identifies six divergences (scoring metric, phase A size+mtime short-circuit, FNAMECMP_FUZZY wire emission, suffix heuristics, already-sent filtering, tie-breaker direction) and three matches (-yy plumbing, --whole-file/--inplace /--append interactions). Recommends Levenshtein-based scoring, phase A short-circuit, and ITEM_BASIS_TYPE_FOLLOWS emission.
oferchen
added a commit
that referenced
this pull request
May 5, 2026
…3488) * docs(fast_io): design session-level io_uring ring pool (#1409) (#3444) * docs(fast_io): audit io_uring SQPOLL and DEFER_TASKRUN for socket I/O (#1267) (#3445) * docs(ci): audit Windows ACL/xattr CI matrix gaps (#1869) (#3446) * docs(fast_io): audit MADV_WILLNEED prefault for mmap'd basis files (#1662) (#3447) * docs(batch): investigate zstd as batch-compatible compression alternative (#1685) (#3448) * docs(fast_io): document mmap page-fault impact on io_uring SQPOLL (#1661) (#3449) * docs: audit --delete-during ordering vs upstream rsync 3.4.1 (#3453) Captures the audit findings from #1893 in docs/architecture/delete-during.md and lays the groundwork for the documentation work tracked in #1894. Contrasts upstream's per-directory interleaved deletion (generator.c::recv_generator + delete_in_dir) against oc-rsync's batched pre-transfer sweep (crates/transfer/src/receiver/transfer.rs:532), enumerates the resulting differences in phase ordering, determinism, filter evaluation, and error semantics, and lists the follow-up actions: CHANGELOG / man-page note, interop test for concurrent new+deleted entries, .rsync-filter investigation, and a possible --delete-strict-order opt-in. Refs #1893, #1894. * feat: add --jump-host proxy-jump CLI flag (#3454) Expose OpenSSH ProxyJump (-J) as --jump-host. Comma-separated [user@]HOST[:PORT] hops are forwarded to the remote shell as `ssh -J <value>` before the destination operand when the configured remote shell is OpenSSH. Empty values are rejected at every layer. Note: only the long form `--jump-host` is provided. Upstream rsync 3.4.1 binds `-J` to `--omit-link-times` (options.c:647), so reusing the short flag would break wire compatibility. Refs #1881 * docs: audit --iconv inert critical gap (#3455) Document the parse-but-never-applied architectural dead-end where the CLI accepts --iconv, validates and stores IconvSetting on ClientConfig, and forwards the option to the remote peer over SSH, but the local process never receives a FilenameConverter. ServerConfigBuilder::iconv() exists with zero call sites; receiver and generator already pull a converter from connection.iconv but always observe None. Severity: critical. Filenames with non-ASCII bytes silently bypass transcoding on every local-side path (file-list ingest, file-list emit, filter matching, daemon module serving) when --iconv is supplied. Companion to docs/audits/iconv-pipeline.md, narrowing on the single missing IconvSetting -> FilenameConverter bridge that currently makes every gap in that broader pipeline simultaneously unobservable. Tracks tasks #1909, #1910, #1918. Remediation path enumerated against existing #1911-#1919 task chain. * fix: normalise Windows backslash to forward slash in wire-encoded paths (#3456) A Windows oc-rsync sender that builds a `FileEntry` whose path was constructed via `Path::join` / `PathBuf::push` (the normal case for recursive transfers) emitted the native string with `\` separators verbatim on the wire. A POSIX rsync receiver decoding those bytes treated every `\` as part of a single filename, producing one literal filename per source file instead of the expected directory hierarchy. Upstream rsync 3.4.1 writes filename bytes verbatim in `flist.c:send_file_entry()` (lines 534-570). The wire format remains `/`-separated because every upstream build either runs on a POSIX kernel or under Cygwin's POSIX layer, which presents `/`-separated paths to the application before this code is reached. oc-rsync targets native Win32 directly, so the sender must perform the separator normalisation explicitly. Before this fix on Windows: let mut path = PathBuf::from(\"subdir\"); path.push(\"file.txt\"); let entry = FileEntry::new_file(path, 1024, 0o644); entry.name_bytes() == b\"subdir\\\\file.txt\" // wrong After: entry.name_bytes() == b\"subdir/file.txt\" // matches upstream The fix introduces a single-purpose helper `crates/protocol/src/flist/wire_path.rs::path_bytes_to_wire` that mirrors the existing identity-on-Unix / convert-on-Windows pattern of `wire_mode.rs`. The helper is applied at every wire-encode call site: - `FileEntry::name_bytes()` (filename emission via `write_entry`) - `write_symlink_target()` (symlink target emission) - `strip_leading_slashes()` Windows branch trims both `/` and `\` The `name_bytes()` accessor signature changes from `&[u8]` to `Cow<'_, [u8]>` so the helper can borrow on Unix (zero allocation) and own only when conversion is required on Windows. Sort comparators borrow the inner slice via `Deref` and continue to work unchanged. Tests added: - `wire_path` module: 8 unit tests covering forward-slash identity, empty path, dot path, Unix borrow, Unix backslash preservation, Windows translation, mixed separators. - `flist::write` regression tests: assert the wire-encoded filename contains no `\` byte and that a writer-then-reader roundtrip yields `subdir/file.txt` regardless of host platform. - Symlink-target regression test asserting no `\` byte appears in the encoded target bytes. Companion audit: docs/audits/windows-path.md cites upstream flist.c:534-570 and util1.c:955-961. Closes #1905 Refs #1939 * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids (#3450) * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids Closes #1879 and #1880. The previous `host_contains_colon` heuristic in `SshCommand::target_argument` treated any string containing `:` as IPv6 and wrapped it in brackets. That silently accepted malformed input such as `2001:db8:::1` or `garbage:input` and forwarded the unparseable literal to ssh. Replace it with strict `Ipv6Addr::from_str` validation via a new `parse_host_for_ssh` returning a `HostKind` enum (`Hostname` / `Ipv4` / `Ipv6 { addr, zone }`). The parser also recognises RFC 4007 scoped zone identifiers (e.g. `fe80::1%eth0`) and re-emits them inside the bracket form (`[addr%zone]`) per upstream rsync convention. Zone ids are rejected when empty or when they contain whitespace or `]`. Before: `host_contains_colon("2001:db8:::1") == true` -> emits `[2001:db8:::1]`. After: `parse_host_for_ssh("2001:db8:::1")` returns `Err(InvalidIpv6)`, so the input is passed through unchanged and ssh surfaces the resolution failure instead of receiving a malformed bracketed literal. Includes unit tests for: bare IPv4, bare hostname, bracketed IPv6, IPv6 with zone (`fe80::1%eth0` and `[fe80::1%en0]`), malformed multi-`::` IPv6, zone with whitespace (rejected), and direct `parse_host_for_ssh` classification across all `HostKind` and `BuildError` variants. * style: apply cargo fmt to ssh ipv6 validation * feat: add AppleDouble filter and optional xattr merge for macOS (#3457) * feat: add AppleDouble filter for macOS sidecar exclusion (#1907) Add the `--apple-double-skip` option which appends a perishable `._*` exclusion to the filter chain. macOS writes AppleDouble sidecar files on filesystems that cannot represent extended attributes natively (FAT, exFAT, NFS, SMB) to carry FinderInfo, resource forks, and xattrs. Replicating them across machines clutters destinations with stale metadata. The filter half of #1907 ships now. Upstream rsync 3.4.1 has no equivalent flag, so the option uses the descriptive `--apple-double-skip` name. The optional xattr merge half is deferred: the existing `apple-fs` crate does not expose AppleDouble parsing or xattr merging primitives, so a follow-up task will land that piece alongside the required parser. - New `filters::apple_double` module with the canonical `._*` pattern. - `FilterSet::from_rules_with_apple_double` mirrors the `from_rules_with_cvs` convenience constructor. - CLI plumbing through `ParsedArgs`, the command builder, the filter-rules collector, and the workflow runner. - Integration tests under `crates/filters/tests/apple_double.rs` covering top-level/nested matching, perishable semantics, override precedence, and clear-rule interaction. - Frontend tests cover argument parsing and an end-to-end transfer scenario that confirms `._foo` files are dropped while their parents copy through. - Man page and `--help` text updated. Refs #1907 * style: apply cargo fmt to apple-double filter * refactor: use shell-words crate for ssh remote-shell parsing (#3451) * refactor: use shell-words crate for ssh remote-shell parsing Replace the bespoke byte-level state machine in `parse_remote_shell` with `shell_words::split`, the well-supported POSIX shell tokenizer. - Tokenization is delegated to the `shell-words` crate (1.1). - The wrapper retains the three behaviours callers rely on: empty/whitespace input -> `Empty`, interior NUL -> `InteriorNull`, non-UTF-8 input -> `InvalidEncoding`. - The historical `UnterminatedEscape`/`UnterminatedSingleQuote`/ `UnterminatedDoubleQuote` variants collapse into a single `Parse(String)` variant carrying the `shell_words::ParseError` description. No external code matched on the removed variants. - Adds a property test corpus (`crates/rsync_io/tests/parse_remote_shell.rs`) asserting parity with `shell_words::split` over arbitrary RSYNC_RSH-style inputs, plus explicit unit tests for the historical behaviours (quoting, escapes, whitespace, NUL byte, invalid Unicode). Tasks: #1877, #1878. * style: apply cargo fmt to ssh shell-words tokenizer tests * chore: update Cargo.lock for shell-words dependency Cargo.lock was not updated when shell-words = "1.1" was added to crates/rsync_io/Cargo.toml in cc01bbc, causing the MSRV 1.88 workflow to fail with --locked. * fix(ssh): reject trailing backslash escape in remote-shell tokenizer shell_words::split silently accepts a dangling trailing backslash, but upstream rsync (and the previous bespoke tokenizer) reject it. Add a small validator that walks the string tracking single- and double-quote regions so that backslashes inside single quotes are not flagged, and returns true only when the input ends with an unescaped backslash that has no character to escape. This restores the test parser_rejects_trailing_escape that fails on the shell-words crate alone. * test: allow stricter Parse rejection vs shell_words in parity property The bespoke remote-shell tokenizer rejects dangling trailing backslashes, matching upstream rsync's `tokens = parse_arguments(...)`. The new shell-words-based wrapper preserves that strictness via has_trailing_escape, but `shell_words::split` silently absorbs the trailing backslash and returns Ok. The parity property test was therefore failing on inputs ending in a bare `\\` because parse_remote_shell returned Err(Parse) while shell_words returned Ok. The wrapper is intentionally a tightening, not a parity match in both directions. Add an explicit (Err(Parse), Ok(_)) arm so the property test encodes the contract: we may reject inputs shell_words accepts, but never the other way around. * chore: disable coverage workflow on push/pull_request (#3460) Nightly llvm-cov 22.1 SIGSEGVs deterministically inside llvm::coverage::CoverageMapping::getInstantiationGroups, breaking both master and PR runs. Restrict the workflow to manual workflow_dispatch until a known-good nightly is pinned via RUSTUP_TOOLCHAIN. * fix: avoid mmap basis when paired with io_uring writer (#3459) * fix: avoid mmap basis when paired with io_uring writer DeltaApplicator opened its basis file via MapFile::open_adaptive, which selects mmap for files >= 1 MiB on Unix. The applicator has no production caller today, but is the obvious next target for an io_uring-output rewrite. Wiring an IoUringWriter to the applicator without changing the basis path would submit mmap-backed pointers to io_uring SQEs on block-ref copies >= the writer batch threshold, exposing two failure modes: 1. Cold-page faults on the basis file are serviced under the SQE submission thread (worse, the SQPOLL kernel thread when SQPOLL is enabled), turning a "free" zero-copy write into a synchronous fault and stalling other in-flight SQEs on the same poller. 2. Concurrent truncation of the basis file raises SIGBUS while the kernel is dereferencing the page on our behalf - recovery from in-kernel SIGBUS is not signal-safe. Upstream rsync 3.4.1 deliberately avoids mmap(2) for basis files for the same truncation reason (fileio.c:214-217). Our BufferedMap matches that decision. Closes the F1 hazard from docs/audits/mmap-iouring-co-usage.md (#1660) by introducing BasisWriterKind in DeltaApplyConfig: when the writer is io_uring-backed, DeltaApplicator::new opens the basis via the new MapFile::open_adaptive_buffered (forces AdaptiveMapStrategy::Buffered regardless of size). Standard / std-write writers retain the existing adaptive selection. Non-Unix is unaffected since BufferedMap is the only basis strategy there. Tests cover both directions: io_uring writer + 2 MiB basis stays on BufferedMap, standard writer + 2 MiB basis still picks mmap on Unix. docs/design/basis-file-io-policy.md updated with implementation status noting that the io_uring-pairing axis is now wired; remaining hazard columns (--inplace, --append, --copy-devices, sparse, network FS) are future work tracked in the same doc. Refs: #1906 (this task), #1660 (audit). * fix: drop redundant Write import in applicator tests The test module brings in `Write` via `use super::*;` which re-exports the file-level `use std::io::{self, Read, Write};`. The explicit `use std::io::Write as _;` is therefore unused and trips clippy under `-D warnings`. * feat: bridge IconvSetting to FilenameConverter (#3458) Adds IconvSetting::resolve_converter() to map the CLI-side iconv setting onto a transfer-side FilenameConverter, and wires the converter through apply_common_server_flags so every SSH and daemon ServerConfig builder populates ConnectionConfig.iconv when --iconv is in effect. Without this bridge, the CLI parsed --iconv, validated it, forwarded it to the remote peer over SSH, but the local file-list reader and writer hooks always observed None and silently passed raw bytes through. Also rejects --iconv=LOCAL,REMOTE with a hard error when the iconv cargo feature is compiled out, rather than silently no-opping. Closes #1911 Closes #1915 * perf: wire IoUringDiskBatch into disk_commit hot path (#3452) * perf: wire IoUringDiskBatch into disk_commit hot path Task #1901. The disk-commit thread previously created an IoUringDiskBatch via try_create_disk_batch() but bound it to `_disk_batch`, leaving every write to fall through to the buffered path (thread.rs:127). All disk writes therefore bypassed io_uring batching even when the policy was Auto or Enabled on Linux 5.6+. Introduce a Writer enum in disk_commit/writer.rs that dispatches between ReusableBufWriter (existing 256 KB buffered path) and a new IoUring variant that borrows the disk thread's persistent IoUringDiskBatch. The IoUring variant is cfg-gated to all(target_os = "linux", feature = "io_uring") so non-Linux builds never reference it. In disk_commit/thread.rs, drop the underscore from the batch binding and thread `disk_batch.as_mut()` into both process_file and process_whole_file on every iteration of the message loop. The batch is created once per thread and reused across all files via begin_file/commit_file. In disk_commit/process.rs, replace the unconditional ReusableBufWriter construction with a make_writer() helper that registers the file with the batch when (a) the batch is present and (b) sparse mode is disabled. Sparse writes require Seek, which IoUringDiskBatch does not implement, so sparse mode keeps using the buffered writer via buffered_for_sparse(). Semantics preserved: - Temp-file commit ordering: Writer::finish() calls commit_file(do_fsync) which flushes + fsyncs + detaches the file before commit_file() runs the rename. Buffered path drops the writer (closing the file) before rename, identical to prior behaviour. - fsync: do_fsync flag flows through Writer::flush_and_sync (buffered) and Writer::finish (io_uring). The io_uring path makes flush_and_sync a no-op so commit_file performs both atomically in a single SQE. - Error propagation: io::Result is preserved end-to-end with file_path context attached on fsync/commit errors. * fix: gate unused Writer::finish parameters on non-Linux builds The do_fsync and file_path parameters on Writer::finish are only consumed by the io_uring arm. On non-Linux (or with the io_uring feature disabled) the Buffered arm ignores them, which under -D warnings escalates to a build failure. * fix(disk_commit): force buffered writer for append mode The IoUringDiskBatch writes via SQEs with absolute offsets starting at 0, ignoring the file position set by `file.seek(append_offset)` in open_output_file. This caused --append-mode transfers under io_uring to overwrite the existing file prefix with zeros, surfacing as the standalone:append interop test failing with NUL-byte corruption. Gate Writer::IoUring off whenever begin.append_offset > 0 and route those transfers through ReusableBufWriter, which honors the seek via standard Write::write_all. upstream: receiver.c:307-308 - file position is the source of truth in append mode; any writer that ignores it cannot be used. * docs(compress): convert restating comments to rustdoc in lz4 (#2026) (#3462) Remove inline comments inside lz4 codec function bodies and tests that simply echo the next statement. All public-item rustdoc and upstream rsync references (token.c) remain intact. * fix(filters): propagate P (protect) filter modifier in sender-side filter exchange (#3463) oc-rsync was emitting `P pattern` directly on the wire while also attaching the `r` (receiver-side) modifier derived from the rule's `applies_to_receiver` flag. Upstream rsync rejects `Pr` because the `P` prefix already specifies the side and `r` after a side-specifying prefix triggers the "invalid modifier" error in exclude.c:1270-1271. Upstream's `get_rule_prefix()` (exclude.c:1536-1572) only ever emits `+`, `-`, or `:` as the leading character; `P` and `R` exist only as parser sugar that lower to plain include/exclude rules carrying the `FILTRULE_RECEIVER_SIDE` flag. The wire encoding for a P rule is `-r pattern`; the wire encoding for an R rule is `+r pattern`. Normalize Protect/Risk in `build_modern_prefix` so the wire stream matches upstream byte-for-byte. The receiver-side flag is forced when serializing Protect/Risk rule types so that hand-built `FilterRuleWireFormat` values without `receiver_side=true` still serialize correctly. Reproduced and verified the fix against upstream rsync 3.4.1 daemon in the rsync-profile podman container with `--filter='P *.log'` push: protected files now survive `--delete`. Removes `standalone:delete-filter-protect` from KNOWN_FAILURES and DASHBOARD_ENTRIES. upstream: exclude.c:1536-1542 upstream: exclude.c:1569-1572 upstream: exclude.c:1201-1206 * docs(checksums): convert restating comments to rustdoc in rolling (#2018) (#3461) Cleans up the `crates/checksums/src/rolling/` test modules: - Drop decorative banner divider in front of the golden-tests block; replace with a short paragraph that describes intent and references upstream checksum.c. - Remove restating-the-code comments (e.g. "Verify it matches manual computation", "Verify rolling produces correct checksum after shift") whose information is already conveyed by the surrounding code or test name. - Sharpen wire-format comments to call out their connection to upstream rsync's packed (s2 << 16) | s1 layout. Production source files in `rolling/` already use proper rustdoc on every public item, so no `///` conversions are required there. SAFETY comments and SIMD weight-table explanations are preserved verbatim. No runtime behaviour changes. * fix(checksums): apply checksum_seed to MD4 strong checksum (upstream parity) (#3465) Upstream rsync's `get_checksum2()` (checksum.c:358-396) appends the negotiated `checksum_seed` after the data when computing MD4 block strong sums: memcpy(buf1, buf, len); if (checksum_seed) { SIVAL(buf1, len, checksum_seed); len += 4; } Our MD4 path was unconditionally calling `Md4::digest(data)` and ignoring the seed, so block-level strong checksums diverged from upstream whenever the session negotiated a non-zero seed. This silently hurt delta-transfer match quality across rsync<->oc-rsync interop and inflated `hash_hits` / `false_alarms` counters in delta-stats output. Changes: * Add `Md4::digest_with_seed(seed, data)` mirroring upstream's append-seed- after-data semantics. A zero seed is a no-op (matches upstream's `if (checksum_seed)` guard). * Add `SignatureAlgorithm::Md4Seeded { seed }` variant. Introducing a new variant rather than mutating the unit `Md4` keeps the 30+ existing call-sites that match on `Md4` working. * `ChecksumFactory::signature_algorithm()` now selects `Md4Seeded` when MD4 (or the legacy `None`/protocol<27 default) runs with a non-zero seed, and keeps unit `Md4` for the seed=0 case so existing wire goldens stay byte- identical. * Extend exhaustive matches in `signature::algorithm`, the parallel checksum executor, and the delta-transfer bench to cover `Md4Seeded`. Tests: * `md4_seeded_appends_seed_after_data` - verifies the byte layout matches building `(data || seed_le_bytes)` and hashing with plain MD4. * `md4_seeded_zero_seed_matches_unseeded` - guards the upstream zero-seed short-circuit. * `md4_seeded_negative_seed_is_le_two_complement` - confirms i32 wire encoding for negative seeds. * `md4_seeded_signature_matches_upstream_format` - parity at the `SignatureAlgorithm` layer. * Three `ChecksumFactory` tests covering MD4/None x seed=0/non-zero. upstream: checksum.c:358-396 `get_checksum2()` MD4 branch * fix(filters): correct R-before-P ordering in delete-filter-risk interop test (#3464) * fix(filters): correct R-before-P ordering in delete-filter-risk interop test The standalone:delete-filter-risk interop test had the filter rules in the wrong order: --filter='P *.log' --filter='P *.sh' --filter='R *.log'. Upstream rsync evaluates filter rules first-match-wins (exclude.c:1038-1065 check_filter()). For R (risk) to override P (protect) on *.log, it must be listed BEFORE the P rule. With R last, the P rule matched first and the *.log files were never deleted. Verified that upstream rsync 3.4.1 itself produces the same "wrong" outcome with the original P-first ordering, confirming the test expectation - not the filter engine - was incorrect. oc-rsync's filter engine already mirrors upstream semantics; the unit test risk_rule_allows_deletion_before_protection in crates/filters/src/tests.rs explicitly documents the required R-before-P order. Reordered both directions (up->oc and oc->up) to use --filter='R *.log' --filter='P *.log' --filter='P *.sh' and updated the function-level comment to cite exclude.c:1201-1207 (R rflags) and exclude.c:1038-1065 (first-match-wins). Verified end-to-end in the rsync-profile container that both upstream->oc and oc->upstream daemon pushes now produce the expected outcome: - source files transferred - risky.log + subdir/nested.log deleted (R overrides P) - keeper.sh preserved (P with no preceding R) - destonly.txt deleted (no protection) Removed the entry from KNOWN_FAILURES and DASHBOARD_ENTRIES in tools/ci/known_failures.conf. upstream: exclude.c:1201-1207 'R' = FILTRULE_INCLUDE|FILTRULE_RECEIVER_SIDE upstream: exclude.c:1038-1065 check_filter() first-match-wins * fix(filters): suppress redundant r/s modifier on Risk/Protect wire prefix The wire encoder unconditionally emitted `s`/`r` modifiers when the sender_side/receiver_side flags were set on a FilterRuleWireFormat. This produced output like `Rr *.log` for a Risk rule (which oc-rsync internally tags as receiver-side). Upstream rsync's parser rejects that with `invalid modifier 'r' at position 1 in filter rule`, breaking the delete-filter-risk interop scenario in oc->upstream direction with exit 12. Upstream's filter parser (exclude.c:1180-1207) sets prefix_specifies_side when the rule type char is R/P (and S/H), and exclude.c:1269-1278 rejects an explicit `r`/`s` modifier in that case. Match upstream's get_rule_prefix (exclude.c:1525-1587), which never emits the redundant flag for those prefixes. Suppress `s`/`r` emission when the rule type is Risk or Protect. Add unit tests at the prefix builder level and wire-level tests including a Risk roundtrip to lock in the format. * docs(protocol): convert restating comments to rustdoc in varint (#1999) (#3466) Remove decorative banner comments from the varint test module and tighten internal comments in `read_varlong` to delete pure restatement while preserving upstream rsync C source references (`io.c:read_varlong`, `memcpy(u.b, b2+1, min_bytes-1)`). No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in zlib (#2029) (#3467) Remove decorative and restating-the-code comments from the zlib module's tests, while preserving every upstream rsync C source reference and comments that explain non-obvious test design rationale. Non-test files (decoder.rs, encoder.rs, helpers.rs, level.rs, mod.rs) already use proper rustdoc and required no changes. * docs: clean up comments in core::timeout (#2014) (#3471) * docs(signature): convert restating comments to rustdoc (#2034) (#3474) Drops inline comments that merely echo the immediately following code in block_size.rs, pipelined_gen.rs, and async_gen.rs. Reworks the few that remain to focus on the WHY (upstream-derived phase 2 short-circuit, sqrt bound derivation, BATCH_SIZE cross-file consistency, disk I/O thread cap). Comment-only change with no functional impact. * docs(metadata): convert restating comments to rustdoc in chmod (#2022) (#3473) Remove section-banner comments and pure-restatement comments from chmod test modules. Convert a stranded `///` doc-comment in `upstream_compatibility` to a proper `//!` module doc. Preserve all comments that reference upstream rsync semantics or that explain the non-obvious conditional-X / assign-empty-perms / octal-validation rules. No functional code changes. * feat(ci): add upstream rsync testsuite harness (#3470) Adds tools/ci/run_upstream_testsuite.sh, which runs upstream rsync's testsuite/*.test scripts against oc-rsync. Mirrors upstream's runtests.sh contract: exports $RSYNC, $TOOLDIR, $srcdir, $suitedir, $scratchdir, $TLS_ARGS, $POSIXLY_CORRECT, and $ECHO_* per test. Difference vs upstream runtests.sh: $RSYNC points at oc-rsync, not upstream. Upstream sources are still needed for helper tools (tls, getgroups, lsh.sh) and config artefacts (config.h, shconfig); the harness fetches and configures them on first run. Known failures live in tools/ci/upstream_testsuite_known_failures.conf and are grouped by category (lsh.sh-routed remote-shell tests, daemon-mode tests, ACL/xattr, devices, chown, hardlinks INC_RECURSE, atimes/crtimes, etc). Tests in this list are reported XFAIL on failure and UPASS on unexpected success so the list stays self-curating. Wires up risk area #4 from the audit: protocol edge-case compatibility against upstream's own conformance suite. Usage: tools/ci/run_upstream_testsuite.sh # all tests WHICHTESTS=00-hello.test tools/ci/... # one test PRESERVE_SCRATCH=yes tools/ci/... # keep per-test dirs * docs: clean up comments in protocol::error_recovery (#1985) (#3472) Remove restatement and decorative banner comments that echo the code without adding value. Replace two banner-style remarks with focused notes that explain non-obvious behavior (over-receive handling, why HashMap re-insert is the desired semantic, the conservative default classification, and the resume-vs-retry policy on transient errors). Scope is limited to crates/protocol/src/error_recovery/. No functional changes; tests and public APIs are unchanged. Refs #1985 * docs(checksums): convert restating comments to rustdoc in strong (#2020) (#3469) Audit of `crates/checksums/src/strong/` (excluding `strategy/`, `md4_tests.rs`, `xxh64_tests.rs` per task scope). Production source files are already well-documented from prior cleanup passes: every public type, trait, function, and method carries rustdoc; upstream rsync references (`upstream: checksum.c:get_checksum2()` and similar) are preserved on the seed-handling paths in `md4.rs` and `md5.rs`; no decorative banners, debug checkpoints, or commented-out code remain in scope. The single substantive change in this pass adds a quirk comment to the private `detect()` helper in `openssl_support.rs` that captures non-obvious upstream behaviour: MD4 may be absent on OpenSSL builds without the legacy provider, so the MD4 probe is best-effort and overall detection succeeds as long as MD5 works. This explains the otherwise-mysterious `if let Some(md4) = ...` whose result is deliberately discarded. No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in strategy (#2028) (#3468) Remove decorative banner/divider comments from the compress::strategy module. The module's rustdoc was already comprehensive; this cleanup removes the remaining `// ---- Section ----` test-grouping banners that violate the no-decorative-divider rule from <internal-tool-doc>. All upstream rsync C source references and load-bearing internal comments are preserved verbatim. No production code, function bodies, signatures, types, or imports were modified. * docs: add v0.6 known limitations and security hardening notes Adds a "Known Limitations / Architectural Trade-offs" section to README.md covering PBUF_RING 5.19+ requirement, io_uring buffer pool ceiling, SSH double-compression interaction, single-thread delta computation, daemon TLS gap, and Windows IOCP wiring status (#1868). Updates SECURITY.md supported-versions table to 0.6.x and adds a "Hardening Notes" section covering recycle_buffer release-mode bounds-check gap, bgid u16 namespace exhaustion, SSH double-compression amplification surface, daemon TLS-in-front guidance (stunnel / SSH tunnel / reverse proxy), and daemon module hardening defaults. Cross-links the two documents so operators planning a deployment have a single source of truth for the documented trade-offs. * docs: comment cleanup in protocol::flist::hardlink Remove restating-the-code comments from the hardlink test suite and fix two malformed upstream references in types.rs that used `// upstream:` inside rustdoc blocks (rendered literally in generated docs). Preserved: all upstream rsync source references, non-obvious WHY explanations (FxHash collision rationale, swapped-value asymmetry, weak-mixing input categorisation), and module-level documentation. * docs(filters): audit .rsync-filter per-directory inheritance (#2050) Compare oc-rsync's per-directory merge file inheritance against upstream rsync 3.4.1. Documents the push/pop lifecycle, modifier semantics (n, e, p, s, r, !), and parent-dirscan behaviour. Identifies three behavioural bugs in FilterChain (n modifier ignored, !/clear not propagated to scopes, include-only scopes cannot override outer excludes), one intentional divergence (parent_dirscan), and one untested area (nested dir-merge declarations).
oferchen
added a commit
that referenced
this pull request
May 5, 2026
* docs(fast_io): design session-level io_uring ring pool (#1409) (#3444) * docs(fast_io): audit io_uring SQPOLL and DEFER_TASKRUN for socket I/O (#1267) (#3445) * docs(ci): audit Windows ACL/xattr CI matrix gaps (#1869) (#3446) * docs(fast_io): audit MADV_WILLNEED prefault for mmap'd basis files (#1662) (#3447) * docs(batch): investigate zstd as batch-compatible compression alternative (#1685) (#3448) * docs(fast_io): document mmap page-fault impact on io_uring SQPOLL (#1661) (#3449) * docs: audit --delete-during ordering vs upstream rsync 3.4.1 (#3453) Captures the audit findings from #1893 in docs/architecture/delete-during.md and lays the groundwork for the documentation work tracked in #1894. Contrasts upstream's per-directory interleaved deletion (generator.c::recv_generator + delete_in_dir) against oc-rsync's batched pre-transfer sweep (crates/transfer/src/receiver/transfer.rs:532), enumerates the resulting differences in phase ordering, determinism, filter evaluation, and error semantics, and lists the follow-up actions: CHANGELOG / man-page note, interop test for concurrent new+deleted entries, .rsync-filter investigation, and a possible --delete-strict-order opt-in. Refs #1893, #1894. * feat: add --jump-host proxy-jump CLI flag (#3454) Expose OpenSSH ProxyJump (-J) as --jump-host. Comma-separated [user@]HOST[:PORT] hops are forwarded to the remote shell as `ssh -J <value>` before the destination operand when the configured remote shell is OpenSSH. Empty values are rejected at every layer. Note: only the long form `--jump-host` is provided. Upstream rsync 3.4.1 binds `-J` to `--omit-link-times` (options.c:647), so reusing the short flag would break wire compatibility. Refs #1881 * docs: audit --iconv inert critical gap (#3455) Document the parse-but-never-applied architectural dead-end where the CLI accepts --iconv, validates and stores IconvSetting on ClientConfig, and forwards the option to the remote peer over SSH, but the local process never receives a FilenameConverter. ServerConfigBuilder::iconv() exists with zero call sites; receiver and generator already pull a converter from connection.iconv but always observe None. Severity: critical. Filenames with non-ASCII bytes silently bypass transcoding on every local-side path (file-list ingest, file-list emit, filter matching, daemon module serving) when --iconv is supplied. Companion to docs/audits/iconv-pipeline.md, narrowing on the single missing IconvSetting -> FilenameConverter bridge that currently makes every gap in that broader pipeline simultaneously unobservable. Tracks tasks #1909, #1910, #1918. Remediation path enumerated against existing #1911-#1919 task chain. * fix: normalise Windows backslash to forward slash in wire-encoded paths (#3456) A Windows oc-rsync sender that builds a `FileEntry` whose path was constructed via `Path::join` / `PathBuf::push` (the normal case for recursive transfers) emitted the native string with `\` separators verbatim on the wire. A POSIX rsync receiver decoding those bytes treated every `\` as part of a single filename, producing one literal filename per source file instead of the expected directory hierarchy. Upstream rsync 3.4.1 writes filename bytes verbatim in `flist.c:send_file_entry()` (lines 534-570). The wire format remains `/`-separated because every upstream build either runs on a POSIX kernel or under Cygwin's POSIX layer, which presents `/`-separated paths to the application before this code is reached. oc-rsync targets native Win32 directly, so the sender must perform the separator normalisation explicitly. Before this fix on Windows: let mut path = PathBuf::from(\"subdir\"); path.push(\"file.txt\"); let entry = FileEntry::new_file(path, 1024, 0o644); entry.name_bytes() == b\"subdir\\\\file.txt\" // wrong After: entry.name_bytes() == b\"subdir/file.txt\" // matches upstream The fix introduces a single-purpose helper `crates/protocol/src/flist/wire_path.rs::path_bytes_to_wire` that mirrors the existing identity-on-Unix / convert-on-Windows pattern of `wire_mode.rs`. The helper is applied at every wire-encode call site: - `FileEntry::name_bytes()` (filename emission via `write_entry`) - `write_symlink_target()` (symlink target emission) - `strip_leading_slashes()` Windows branch trims both `/` and `\` The `name_bytes()` accessor signature changes from `&[u8]` to `Cow<'_, [u8]>` so the helper can borrow on Unix (zero allocation) and own only when conversion is required on Windows. Sort comparators borrow the inner slice via `Deref` and continue to work unchanged. Tests added: - `wire_path` module: 8 unit tests covering forward-slash identity, empty path, dot path, Unix borrow, Unix backslash preservation, Windows translation, mixed separators. - `flist::write` regression tests: assert the wire-encoded filename contains no `\` byte and that a writer-then-reader roundtrip yields `subdir/file.txt` regardless of host platform. - Symlink-target regression test asserting no `\` byte appears in the encoded target bytes. Companion audit: docs/audits/windows-path.md cites upstream flist.c:534-570 and util1.c:955-961. Closes #1905 Refs #1939 * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids (#3450) * fix: validate IPv6 host with Ipv6Addr and support scoped zone ids Closes #1879 and #1880. The previous `host_contains_colon` heuristic in `SshCommand::target_argument` treated any string containing `:` as IPv6 and wrapped it in brackets. That silently accepted malformed input such as `2001:db8:::1` or `garbage:input` and forwarded the unparseable literal to ssh. Replace it with strict `Ipv6Addr::from_str` validation via a new `parse_host_for_ssh` returning a `HostKind` enum (`Hostname` / `Ipv4` / `Ipv6 { addr, zone }`). The parser also recognises RFC 4007 scoped zone identifiers (e.g. `fe80::1%eth0`) and re-emits them inside the bracket form (`[addr%zone]`) per upstream rsync convention. Zone ids are rejected when empty or when they contain whitespace or `]`. Before: `host_contains_colon("2001:db8:::1") == true` -> emits `[2001:db8:::1]`. After: `parse_host_for_ssh("2001:db8:::1")` returns `Err(InvalidIpv6)`, so the input is passed through unchanged and ssh surfaces the resolution failure instead of receiving a malformed bracketed literal. Includes unit tests for: bare IPv4, bare hostname, bracketed IPv6, IPv6 with zone (`fe80::1%eth0` and `[fe80::1%en0]`), malformed multi-`::` IPv6, zone with whitespace (rejected), and direct `parse_host_for_ssh` classification across all `HostKind` and `BuildError` variants. * style: apply cargo fmt to ssh ipv6 validation * feat: add AppleDouble filter and optional xattr merge for macOS (#3457) * feat: add AppleDouble filter for macOS sidecar exclusion (#1907) Add the `--apple-double-skip` option which appends a perishable `._*` exclusion to the filter chain. macOS writes AppleDouble sidecar files on filesystems that cannot represent extended attributes natively (FAT, exFAT, NFS, SMB) to carry FinderInfo, resource forks, and xattrs. Replicating them across machines clutters destinations with stale metadata. The filter half of #1907 ships now. Upstream rsync 3.4.1 has no equivalent flag, so the option uses the descriptive `--apple-double-skip` name. The optional xattr merge half is deferred: the existing `apple-fs` crate does not expose AppleDouble parsing or xattr merging primitives, so a follow-up task will land that piece alongside the required parser. - New `filters::apple_double` module with the canonical `._*` pattern. - `FilterSet::from_rules_with_apple_double` mirrors the `from_rules_with_cvs` convenience constructor. - CLI plumbing through `ParsedArgs`, the command builder, the filter-rules collector, and the workflow runner. - Integration tests under `crates/filters/tests/apple_double.rs` covering top-level/nested matching, perishable semantics, override precedence, and clear-rule interaction. - Frontend tests cover argument parsing and an end-to-end transfer scenario that confirms `._foo` files are dropped while their parents copy through. - Man page and `--help` text updated. Refs #1907 * style: apply cargo fmt to apple-double filter * refactor: use shell-words crate for ssh remote-shell parsing (#3451) * refactor: use shell-words crate for ssh remote-shell parsing Replace the bespoke byte-level state machine in `parse_remote_shell` with `shell_words::split`, the well-supported POSIX shell tokenizer. - Tokenization is delegated to the `shell-words` crate (1.1). - The wrapper retains the three behaviours callers rely on: empty/whitespace input -> `Empty`, interior NUL -> `InteriorNull`, non-UTF-8 input -> `InvalidEncoding`. - The historical `UnterminatedEscape`/`UnterminatedSingleQuote`/ `UnterminatedDoubleQuote` variants collapse into a single `Parse(String)` variant carrying the `shell_words::ParseError` description. No external code matched on the removed variants. - Adds a property test corpus (`crates/rsync_io/tests/parse_remote_shell.rs`) asserting parity with `shell_words::split` over arbitrary RSYNC_RSH-style inputs, plus explicit unit tests for the historical behaviours (quoting, escapes, whitespace, NUL byte, invalid Unicode). Tasks: #1877, #1878. * style: apply cargo fmt to ssh shell-words tokenizer tests * chore: update Cargo.lock for shell-words dependency Cargo.lock was not updated when shell-words = "1.1" was added to crates/rsync_io/Cargo.toml in cc01bbc, causing the MSRV 1.88 workflow to fail with --locked. * fix(ssh): reject trailing backslash escape in remote-shell tokenizer shell_words::split silently accepts a dangling trailing backslash, but upstream rsync (and the previous bespoke tokenizer) reject it. Add a small validator that walks the string tracking single- and double-quote regions so that backslashes inside single quotes are not flagged, and returns true only when the input ends with an unescaped backslash that has no character to escape. This restores the test parser_rejects_trailing_escape that fails on the shell-words crate alone. * test: allow stricter Parse rejection vs shell_words in parity property The bespoke remote-shell tokenizer rejects dangling trailing backslashes, matching upstream rsync's `tokens = parse_arguments(...)`. The new shell-words-based wrapper preserves that strictness via has_trailing_escape, but `shell_words::split` silently absorbs the trailing backslash and returns Ok. The parity property test was therefore failing on inputs ending in a bare `\\` because parse_remote_shell returned Err(Parse) while shell_words returned Ok. The wrapper is intentionally a tightening, not a parity match in both directions. Add an explicit (Err(Parse), Ok(_)) arm so the property test encodes the contract: we may reject inputs shell_words accepts, but never the other way around. * chore: disable coverage workflow on push/pull_request (#3460) Nightly llvm-cov 22.1 SIGSEGVs deterministically inside llvm::coverage::CoverageMapping::getInstantiationGroups, breaking both master and PR runs. Restrict the workflow to manual workflow_dispatch until a known-good nightly is pinned via RUSTUP_TOOLCHAIN. * fix: avoid mmap basis when paired with io_uring writer (#3459) * fix: avoid mmap basis when paired with io_uring writer DeltaApplicator opened its basis file via MapFile::open_adaptive, which selects mmap for files >= 1 MiB on Unix. The applicator has no production caller today, but is the obvious next target for an io_uring-output rewrite. Wiring an IoUringWriter to the applicator without changing the basis path would submit mmap-backed pointers to io_uring SQEs on block-ref copies >= the writer batch threshold, exposing two failure modes: 1. Cold-page faults on the basis file are serviced under the SQE submission thread (worse, the SQPOLL kernel thread when SQPOLL is enabled), turning a "free" zero-copy write into a synchronous fault and stalling other in-flight SQEs on the same poller. 2. Concurrent truncation of the basis file raises SIGBUS while the kernel is dereferencing the page on our behalf - recovery from in-kernel SIGBUS is not signal-safe. Upstream rsync 3.4.1 deliberately avoids mmap(2) for basis files for the same truncation reason (fileio.c:214-217). Our BufferedMap matches that decision. Closes the F1 hazard from docs/audits/mmap-iouring-co-usage.md (#1660) by introducing BasisWriterKind in DeltaApplyConfig: when the writer is io_uring-backed, DeltaApplicator::new opens the basis via the new MapFile::open_adaptive_buffered (forces AdaptiveMapStrategy::Buffered regardless of size). Standard / std-write writers retain the existing adaptive selection. Non-Unix is unaffected since BufferedMap is the only basis strategy there. Tests cover both directions: io_uring writer + 2 MiB basis stays on BufferedMap, standard writer + 2 MiB basis still picks mmap on Unix. docs/design/basis-file-io-policy.md updated with implementation status noting that the io_uring-pairing axis is now wired; remaining hazard columns (--inplace, --append, --copy-devices, sparse, network FS) are future work tracked in the same doc. Refs: #1906 (this task), #1660 (audit). * fix: drop redundant Write import in applicator tests The test module brings in `Write` via `use super::*;` which re-exports the file-level `use std::io::{self, Read, Write};`. The explicit `use std::io::Write as _;` is therefore unused and trips clippy under `-D warnings`. * feat: bridge IconvSetting to FilenameConverter (#3458) Adds IconvSetting::resolve_converter() to map the CLI-side iconv setting onto a transfer-side FilenameConverter, and wires the converter through apply_common_server_flags so every SSH and daemon ServerConfig builder populates ConnectionConfig.iconv when --iconv is in effect. Without this bridge, the CLI parsed --iconv, validated it, forwarded it to the remote peer over SSH, but the local file-list reader and writer hooks always observed None and silently passed raw bytes through. Also rejects --iconv=LOCAL,REMOTE with a hard error when the iconv cargo feature is compiled out, rather than silently no-opping. Closes #1911 Closes #1915 * perf: wire IoUringDiskBatch into disk_commit hot path (#3452) * perf: wire IoUringDiskBatch into disk_commit hot path Task #1901. The disk-commit thread previously created an IoUringDiskBatch via try_create_disk_batch() but bound it to `_disk_batch`, leaving every write to fall through to the buffered path (thread.rs:127). All disk writes therefore bypassed io_uring batching even when the policy was Auto or Enabled on Linux 5.6+. Introduce a Writer enum in disk_commit/writer.rs that dispatches between ReusableBufWriter (existing 256 KB buffered path) and a new IoUring variant that borrows the disk thread's persistent IoUringDiskBatch. The IoUring variant is cfg-gated to all(target_os = "linux", feature = "io_uring") so non-Linux builds never reference it. In disk_commit/thread.rs, drop the underscore from the batch binding and thread `disk_batch.as_mut()` into both process_file and process_whole_file on every iteration of the message loop. The batch is created once per thread and reused across all files via begin_file/commit_file. In disk_commit/process.rs, replace the unconditional ReusableBufWriter construction with a make_writer() helper that registers the file with the batch when (a) the batch is present and (b) sparse mode is disabled. Sparse writes require Seek, which IoUringDiskBatch does not implement, so sparse mode keeps using the buffered writer via buffered_for_sparse(). Semantics preserved: - Temp-file commit ordering: Writer::finish() calls commit_file(do_fsync) which flushes + fsyncs + detaches the file before commit_file() runs the rename. Buffered path drops the writer (closing the file) before rename, identical to prior behaviour. - fsync: do_fsync flag flows through Writer::flush_and_sync (buffered) and Writer::finish (io_uring). The io_uring path makes flush_and_sync a no-op so commit_file performs both atomically in a single SQE. - Error propagation: io::Result is preserved end-to-end with file_path context attached on fsync/commit errors. * fix: gate unused Writer::finish parameters on non-Linux builds The do_fsync and file_path parameters on Writer::finish are only consumed by the io_uring arm. On non-Linux (or with the io_uring feature disabled) the Buffered arm ignores them, which under -D warnings escalates to a build failure. * fix(disk_commit): force buffered writer for append mode The IoUringDiskBatch writes via SQEs with absolute offsets starting at 0, ignoring the file position set by `file.seek(append_offset)` in open_output_file. This caused --append-mode transfers under io_uring to overwrite the existing file prefix with zeros, surfacing as the standalone:append interop test failing with NUL-byte corruption. Gate Writer::IoUring off whenever begin.append_offset > 0 and route those transfers through ReusableBufWriter, which honors the seek via standard Write::write_all. upstream: receiver.c:307-308 - file position is the source of truth in append mode; any writer that ignores it cannot be used. * docs(compress): convert restating comments to rustdoc in lz4 (#2026) (#3462) Remove inline comments inside lz4 codec function bodies and tests that simply echo the next statement. All public-item rustdoc and upstream rsync references (token.c) remain intact. * fix(filters): propagate P (protect) filter modifier in sender-side filter exchange (#3463) oc-rsync was emitting `P pattern` directly on the wire while also attaching the `r` (receiver-side) modifier derived from the rule's `applies_to_receiver` flag. Upstream rsync rejects `Pr` because the `P` prefix already specifies the side and `r` after a side-specifying prefix triggers the "invalid modifier" error in exclude.c:1270-1271. Upstream's `get_rule_prefix()` (exclude.c:1536-1572) only ever emits `+`, `-`, or `:` as the leading character; `P` and `R` exist only as parser sugar that lower to plain include/exclude rules carrying the `FILTRULE_RECEIVER_SIDE` flag. The wire encoding for a P rule is `-r pattern`; the wire encoding for an R rule is `+r pattern`. Normalize Protect/Risk in `build_modern_prefix` so the wire stream matches upstream byte-for-byte. The receiver-side flag is forced when serializing Protect/Risk rule types so that hand-built `FilterRuleWireFormat` values without `receiver_side=true` still serialize correctly. Reproduced and verified the fix against upstream rsync 3.4.1 daemon in the rsync-profile podman container with `--filter='P *.log'` push: protected files now survive `--delete`. Removes `standalone:delete-filter-protect` from KNOWN_FAILURES and DASHBOARD_ENTRIES. upstream: exclude.c:1536-1542 upstream: exclude.c:1569-1572 upstream: exclude.c:1201-1206 * docs(checksums): convert restating comments to rustdoc in rolling (#2018) (#3461) Cleans up the `crates/checksums/src/rolling/` test modules: - Drop decorative banner divider in front of the golden-tests block; replace with a short paragraph that describes intent and references upstream checksum.c. - Remove restating-the-code comments (e.g. "Verify it matches manual computation", "Verify rolling produces correct checksum after shift") whose information is already conveyed by the surrounding code or test name. - Sharpen wire-format comments to call out their connection to upstream rsync's packed (s2 << 16) | s1 layout. Production source files in `rolling/` already use proper rustdoc on every public item, so no `///` conversions are required there. SAFETY comments and SIMD weight-table explanations are preserved verbatim. No runtime behaviour changes. * fix(checksums): apply checksum_seed to MD4 strong checksum (upstream parity) (#3465) Upstream rsync's `get_checksum2()` (checksum.c:358-396) appends the negotiated `checksum_seed` after the data when computing MD4 block strong sums: memcpy(buf1, buf, len); if (checksum_seed) { SIVAL(buf1, len, checksum_seed); len += 4; } Our MD4 path was unconditionally calling `Md4::digest(data)` and ignoring the seed, so block-level strong checksums diverged from upstream whenever the session negotiated a non-zero seed. This silently hurt delta-transfer match quality across rsync<->oc-rsync interop and inflated `hash_hits` / `false_alarms` counters in delta-stats output. Changes: * Add `Md4::digest_with_seed(seed, data)` mirroring upstream's append-seed- after-data semantics. A zero seed is a no-op (matches upstream's `if (checksum_seed)` guard). * Add `SignatureAlgorithm::Md4Seeded { seed }` variant. Introducing a new variant rather than mutating the unit `Md4` keeps the 30+ existing call-sites that match on `Md4` working. * `ChecksumFactory::signature_algorithm()` now selects `Md4Seeded` when MD4 (or the legacy `None`/protocol<27 default) runs with a non-zero seed, and keeps unit `Md4` for the seed=0 case so existing wire goldens stay byte- identical. * Extend exhaustive matches in `signature::algorithm`, the parallel checksum executor, and the delta-transfer bench to cover `Md4Seeded`. Tests: * `md4_seeded_appends_seed_after_data` - verifies the byte layout matches building `(data || seed_le_bytes)` and hashing with plain MD4. * `md4_seeded_zero_seed_matches_unseeded` - guards the upstream zero-seed short-circuit. * `md4_seeded_negative_seed_is_le_two_complement` - confirms i32 wire encoding for negative seeds. * `md4_seeded_signature_matches_upstream_format` - parity at the `SignatureAlgorithm` layer. * Three `ChecksumFactory` tests covering MD4/None x seed=0/non-zero. upstream: checksum.c:358-396 `get_checksum2()` MD4 branch * fix(filters): correct R-before-P ordering in delete-filter-risk interop test (#3464) * fix(filters): correct R-before-P ordering in delete-filter-risk interop test The standalone:delete-filter-risk interop test had the filter rules in the wrong order: --filter='P *.log' --filter='P *.sh' --filter='R *.log'. Upstream rsync evaluates filter rules first-match-wins (exclude.c:1038-1065 check_filter()). For R (risk) to override P (protect) on *.log, it must be listed BEFORE the P rule. With R last, the P rule matched first and the *.log files were never deleted. Verified that upstream rsync 3.4.1 itself produces the same "wrong" outcome with the original P-first ordering, confirming the test expectation - not the filter engine - was incorrect. oc-rsync's filter engine already mirrors upstream semantics; the unit test risk_rule_allows_deletion_before_protection in crates/filters/src/tests.rs explicitly documents the required R-before-P order. Reordered both directions (up->oc and oc->up) to use --filter='R *.log' --filter='P *.log' --filter='P *.sh' and updated the function-level comment to cite exclude.c:1201-1207 (R rflags) and exclude.c:1038-1065 (first-match-wins). Verified end-to-end in the rsync-profile container that both upstream->oc and oc->upstream daemon pushes now produce the expected outcome: - source files transferred - risky.log + subdir/nested.log deleted (R overrides P) - keeper.sh preserved (P with no preceding R) - destonly.txt deleted (no protection) Removed the entry from KNOWN_FAILURES and DASHBOARD_ENTRIES in tools/ci/known_failures.conf. upstream: exclude.c:1201-1207 'R' = FILTRULE_INCLUDE|FILTRULE_RECEIVER_SIDE upstream: exclude.c:1038-1065 check_filter() first-match-wins * fix(filters): suppress redundant r/s modifier on Risk/Protect wire prefix The wire encoder unconditionally emitted `s`/`r` modifiers when the sender_side/receiver_side flags were set on a FilterRuleWireFormat. This produced output like `Rr *.log` for a Risk rule (which oc-rsync internally tags as receiver-side). Upstream rsync's parser rejects that with `invalid modifier 'r' at position 1 in filter rule`, breaking the delete-filter-risk interop scenario in oc->upstream direction with exit 12. Upstream's filter parser (exclude.c:1180-1207) sets prefix_specifies_side when the rule type char is R/P (and S/H), and exclude.c:1269-1278 rejects an explicit `r`/`s` modifier in that case. Match upstream's get_rule_prefix (exclude.c:1525-1587), which never emits the redundant flag for those prefixes. Suppress `s`/`r` emission when the rule type is Risk or Protect. Add unit tests at the prefix builder level and wire-level tests including a Risk roundtrip to lock in the format. * docs(protocol): convert restating comments to rustdoc in varint (#1999) (#3466) Remove decorative banner comments from the varint test module and tighten internal comments in `read_varlong` to delete pure restatement while preserving upstream rsync C source references (`io.c:read_varlong`, `memcpy(u.b, b2+1, min_bytes-1)`). No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in zlib (#2029) (#3467) Remove decorative and restating-the-code comments from the zlib module's tests, while preserving every upstream rsync C source reference and comments that explain non-obvious test design rationale. Non-test files (decoder.rs, encoder.rs, helpers.rs, level.rs, mod.rs) already use proper rustdoc and required no changes. * docs: clean up comments in core::timeout (#2014) (#3471) * docs(signature): convert restating comments to rustdoc (#2034) (#3474) Drops inline comments that merely echo the immediately following code in block_size.rs, pipelined_gen.rs, and async_gen.rs. Reworks the few that remain to focus on the WHY (upstream-derived phase 2 short-circuit, sqrt bound derivation, BATCH_SIZE cross-file consistency, disk I/O thread cap). Comment-only change with no functional impact. * docs(metadata): convert restating comments to rustdoc in chmod (#2022) (#3473) Remove section-banner comments and pure-restatement comments from chmod test modules. Convert a stranded `///` doc-comment in `upstream_compatibility` to a proper `//!` module doc. Preserve all comments that reference upstream rsync semantics or that explain the non-obvious conditional-X / assign-empty-perms / octal-validation rules. No functional code changes. * feat(ci): add upstream rsync testsuite harness (#3470) Adds tools/ci/run_upstream_testsuite.sh, which runs upstream rsync's testsuite/*.test scripts against oc-rsync. Mirrors upstream's runtests.sh contract: exports $RSYNC, $TOOLDIR, $srcdir, $suitedir, $scratchdir, $TLS_ARGS, $POSIXLY_CORRECT, and $ECHO_* per test. Difference vs upstream runtests.sh: $RSYNC points at oc-rsync, not upstream. Upstream sources are still needed for helper tools (tls, getgroups, lsh.sh) and config artefacts (config.h, shconfig); the harness fetches and configures them on first run. Known failures live in tools/ci/upstream_testsuite_known_failures.conf and are grouped by category (lsh.sh-routed remote-shell tests, daemon-mode tests, ACL/xattr, devices, chown, hardlinks INC_RECURSE, atimes/crtimes, etc). Tests in this list are reported XFAIL on failure and UPASS on unexpected success so the list stays self-curating. Wires up risk area #4 from the audit: protocol edge-case compatibility against upstream's own conformance suite. Usage: tools/ci/run_upstream_testsuite.sh # all tests WHICHTESTS=00-hello.test tools/ci/... # one test PRESERVE_SCRATCH=yes tools/ci/... # keep per-test dirs * docs: clean up comments in protocol::error_recovery (#1985) (#3472) Remove restatement and decorative banner comments that echo the code without adding value. Replace two banner-style remarks with focused notes that explain non-obvious behavior (over-receive handling, why HashMap re-insert is the desired semantic, the conservative default classification, and the resume-vs-retry policy on transient errors). Scope is limited to crates/protocol/src/error_recovery/. No functional changes; tests and public APIs are unchanged. Refs #1985 * docs(checksums): convert restating comments to rustdoc in strong (#2020) (#3469) Audit of `crates/checksums/src/strong/` (excluding `strategy/`, `md4_tests.rs`, `xxh64_tests.rs` per task scope). Production source files are already well-documented from prior cleanup passes: every public type, trait, function, and method carries rustdoc; upstream rsync references (`upstream: checksum.c:get_checksum2()` and similar) are preserved on the seed-handling paths in `md4.rs` and `md5.rs`; no decorative banners, debug checkpoints, or commented-out code remain in scope. The single substantive change in this pass adds a quirk comment to the private `detect()` helper in `openssl_support.rs` that captures non-obvious upstream behaviour: MD4 may be absent on OpenSSL builds without the legacy provider, so the MD4 probe is best-effort and overall detection succeeds as long as MD5 works. This explains the otherwise-mysterious `if let Some(md4) = ...` whose result is deliberately discarded. No behaviour, signature, or import changes. * docs(compress): convert restating comments to rustdoc in strategy (#2028) (#3468) Remove decorative banner/divider comments from the compress::strategy module. The module's rustdoc was already comprehensive; this cleanup removes the remaining `// ---- Section ----` test-grouping banners that violate the no-decorative-divider rule from <internal-tool-doc>. All upstream rsync C source references and load-bearing internal comments are preserved verbatim. No production code, function bodies, signatures, types, or imports were modified. * docs: add v0.6 known limitations and security hardening notes Adds a "Known Limitations / Architectural Trade-offs" section to README.md covering PBUF_RING 5.19+ requirement, io_uring buffer pool ceiling, SSH double-compression interaction, single-thread delta computation, daemon TLS gap, and Windows IOCP wiring status (#1868). Updates SECURITY.md supported-versions table to 0.6.x and adds a "Hardening Notes" section covering recycle_buffer release-mode bounds-check gap, bgid u16 namespace exhaustion, SSH double-compression amplification surface, daemon TLS-in-front guidance (stunnel / SSH tunnel / reverse proxy), and daemon module hardening defaults. Cross-links the two documents so operators planning a deployment have a single source of truth for the documented trade-offs. * docs: comment cleanup in protocol::flist::hardlink Remove restating-the-code comments from the hardlink test suite and fix two malformed upstream references in types.rs that used `// upstream:` inside rustdoc blocks (rendered literally in generated docs). Preserved: all upstream rsync source references, non-obvious WHY explanations (FxHash collision rationale, swapped-value asymmetry, weak-mixing input categorisation), and module-level documentation. * docs(engine): audit --fuzzy basis-file search algorithm (#2051) Document oc-rsync's --fuzzy / -y basis-file search and scoring against upstream rsync 3.4.1 (generator.c::find_fuzzy() and util1.c::fuzzy_distance()). Identifies six divergences (scoring metric, phase A size+mtime short-circuit, FNAMECMP_FUZZY wire emission, suffix heuristics, already-sent filtering, tie-breaker direction) and three matches (-yy plumbing, --whole-file/--inplace /--append interactions). Recommends Levenshtein-based scoring, phase A short-circuit, and ITEM_BASIS_TYPE_FOLLOWS emission.
3 tasks
oferchen
added a commit
that referenced
this pull request
May 17, 2026
Adds three env-gated sub-scenarios to scripts/windows_throughput_bench.sh that isolate the IOCP hotspots catalogued in the IOCP sync-blocking audit, so future Windows write/read/network improvements can be attributed to specific changes instead of moving the aggregate large_1gib / small_10000 numbers. Enabled by OC_RSYNC_BENCH_DRILLDOWN=1; the existing scenarios run unchanged when the flag is absent. - write_only_iocp: oc-rsync --whole-file --inplace vs cp control to pin the IocpWriter per-IO drain (audit rows #1, #4, #13). - read_only_iocp: oc-rsync --dry-run vs upstream --dry-run over the 1 GiB fixture to pin the IocpReader drain (audit rows #2, #3). - network_only_loopback: rsync push between two short-lived loopback daemons on 127.0.0.1, cancelling out disk bandwidth so only the IocpSocket send/recv path varies (audit rows #8-#11). Documents the invocation, hotspot mapping, and interpretation rules in docs/benchmarks/windows-throughput.md. The drilldown scenarios are not in the required-checks list and have no acceptable-band thresholds; they exist to attribute movement, not to gate merges.
oferchen
added a commit
that referenced
this pull request
May 18, 2026
…ete/context.rs (#2353) (#4365) * docs: design SpillPolicy public API + env-var surface (#2335) (#4340) Introduces design for a 5-knob SpillPolicy struct replacing the single spill_threshold_bytes field on ConcurrentDeltaConfig. Documents defaults, env-var precedence under OC_RSYNC_SPILL_*, ops-friendly CLI flags (--spill-dir, --spill-threshold-bytes), validation rules, a one-release deprecated-forwarding migration story, and a 5-step implementation plan keyed to subtasks STN-2..STN-14. Design only; no runtime changes. * docs(audits): mutex poison recovery classification (#2350, #2351, #2358) (#4341) Inventory all `.lock().expect()` / `.lock().unwrap()` (and `RwLock` peers) sites across the workspace. Classify each as RECOVERABLE (safe to `PoisonError::into_inner()` and proceed), FATAL (mid-operation panic leaves wire state inconsistent, must propagate), or TEST-ONLY (test fixtures and `#[cfg(test)]` env mutexes). Adds tables for the delete module and workspace hotspots, per-crate counts, and follow-up MPE-3..MPE-17 task list anchored on a shared `lock_or_recover` / `lock_or_panic` helper. Identifies `crates/engine/src/local_copy/context_impl/options.rs` as the highest- value single remediation target (7 FATAL batch-writer sites). * feat(engine): add lock_or_recover helpers for poison recovery (#4342) Introduce util::poison with lock_or_recover, read_or_recover, and write_or_recover helpers that promote PoisonError into the inner guard for state that remains valid after a panicking thread unwinds. These helpers will be used by upcoming refactors that replace .lock().expect(...) call sites in modules whose protected state is monotonic or otherwise self-consistent (delete emitter recorder, buffer pool counters, work-queue shards) so a single panicking worker no longer cascades into a full pool abort. Includes unit tests that poison a Mutex and RwLock via a panicking spawned thread and verify the helpers return a usable guard rather than re-panicking. * docs(audits): io_uring data path coverage (#2361) (#4343) Catalogue every SQE construction site in fast_io/io_uring by opcode category. Trace each site upward to confirm whether engine, transfer, or rsync_io route real production data through it. Refute the "metadata-only" framing - the receiver write path, disk-commit thread, and sender source-read path all submit file-data SQEs today. Identify the three production sites where switching to io_uring data SQEs would yield the largest throughput gain (sockets, sub-1MiB source reads, sparse/append fallback) for follow-on IUD-2 and IUD-3 work. * test(fuzz): extend varint decode fuzz target (#2318) (#4344) Adds structured boundary coverage to the protocol varint fuzz target so it does not rely on the corpus to reach extreme inputs: - i32 edges (MIN/MAX, +-1, 0, -0x4000_0000) and i64 edges encoded with write_int/write_varint/write_varlong/write_longint/encode_varint_to_vec, decoded with the matching readers and asserted to roundtrip. - Truncated buffers: every strict prefix of each encoded boundary value is fed through decode_varint/read_varint/read_varlong/read_longint to exercise mid-byte UnexpectedEof paths without panicking. - Both protocol 31 (legacy 4-byte LE via read_int) and protocol 32 (varint) paths driven through read_varint30_int with proto 28-32. - Legacy longint short and long (0xFFFFFFFF-prefixed) forms. The unstructured arbitrary-bytes coverage from the original target is preserved and still runs every iteration. * docs: document Windows NTFS ACL behaviour and lossy cases (#2313) (#4346) Expand the --acls reference, README platform table, and operator migration guide to describe the Tier 1C partial Windows DACL path: deny ACEs dropped, inherited ACEs skipped, SACL gated on --audit-acls, non-rwx access bits collapsed, and unresolvable SIDs dropped. Document --audit-acls, --fail-on-windows-acl-loss, and --windows-acls as planned flags with SDDL wire encoding notes, and cross-link the design doc from all three surfaces. * docs(audits): fuzz coverage gap followups (#2316) (#4347) Decompose the remaining P0/P1 fuzz coverage gaps from the FCV audit into individually filable follow-up tasks. Each entry cites the parse function (file:line), recommends a raw vs structured Arbitrary input shape, and lists the panic / no-panic invariants the harness should assert. Covers the two open P0 gaps (negotiation_prologue, capability_vstring) plus a P0 fill-in (legacy_sniffer reader paths) and the five highest P1 surfaces (compat_flags, filter_list_wire, ndx_codec, incremental_flist, idlist, compress_decoders). * feat(rsync_io): ssh-socketpair-stderr connection primitive (#2372) (#4348) Implements task SSE-3 from docs/design/socketpair-stderr-channel.md. Adds the `socketpair_stderr` module gated behind the new `ssh-socketpair-stderr` Cargo feature (default off). The primitive returns a connected pair of `File`-typed byte streams ready to back the async drain that SSE-4 will layer on top. Unix path uses `UnixStream::pair` (`socketpair(AF_UNIX, SOCK_STREAM, 0)` under the hood with `SOCK_CLOEXEC`) and converts both halves through `OwnedFd` so callers store the parent and child ends behind the same handle type as the existing pipe fallback. A `set_nonblocking` helper toggles `O_NONBLOCK` via the safe stdlib API, ready for the SSE-4 tokio reactor registration. Windows currently returns `io::ErrorKind::Unsupported`; the loopback shim ships under SSE-5 alongside the safe handle bridge in `fast_io`, so the crate keeps `#![deny(unsafe_code)]` intact. Tests gated on Unix + feature flag: 1 KiB round-trip, peer-close EOF, and `set_nonblocking` mode flip surfaced as `WouldBlock`. * docs(design): io_uring data path - receive + send (#2362, #2363) (#4349) Two companion design docs specifying how transfer file data will be routed through io_uring SQEs end-to-end on Linux: - iouring-receive-data-path.md: route disk-commit chunk writes through registered buffers + IORING_OP_WRITE_FIXED, gated by the new iouring-data-writes feature (default off). Shares a pool with the SMR-3a basis reader. - iouring-send-data-path.md: basis re-read via IORING_OP_READ_FIXED feeding directly into IORING_OP_SEND_ZC against the multiplex socket, with the 4-byte MSG_DATA header pre-pended in-slot. Gated by the new iouring-data-sends feature (default off). Both docs spell out ordering, fsync placement, short-write handling, fallback hierarchy, SQPOLL exclusion, and a 5-step phased rollout keyed to follow-up tasks IUD-5..IUD-8. No code changes. * docs(audits): Windows hardlink ACL inheritance (#2311) (#4350) Audits every ACL apply site for files participating in NTFS hardlink cohorts, compares oc-rsync behaviour to upstream rsync 3.4.2 on cygwin, and documents the one divergence: the local-copy --copy-dest Link branch re-applies the source DACL once per follower instead of relying on NTFS inode-level sharing. The writes are idempotent (identical bytes), so correctness holds; the cost is N redundant SetNamedSecurityInfoW round trips and audit log noise. Includes a 3-link cohort test plan covering wire receive, local copy, --copy-dest and cross-cohort isolation. * ci: nightly fuzz coverage report per target (#2322) (#4351) Add `Fuzz Coverage Report` workflow that runs `cargo fuzz coverage` for every target across the `fuzz/`, `crates/protocol/fuzz/`, and `crates/filters/fuzz/` workspaces nightly at 03:30 UTC and uploads the resulting `.lcov` files as 30-day workflow artifacts. The job is informational (`continue-on-error: true`) and emits a per-target line coverage summary to the GitHub step summary, mirroring the SAFETY audit informational pattern from ci.yml. Documents the rollout in docs/audits/fuzz-coverage-matrix.md so the matrix and promotion criteria stay in one place. * bench(windows): per-hotspot drilldown mode (#2305) (#4352) Adds three env-gated sub-scenarios to scripts/windows_throughput_bench.sh that isolate the IOCP hotspots catalogued in the IOCP sync-blocking audit, so future Windows write/read/network improvements can be attributed to specific changes instead of moving the aggregate large_1gib / small_10000 numbers. Enabled by OC_RSYNC_BENCH_DRILLDOWN=1; the existing scenarios run unchanged when the flag is absent. - write_only_iocp: oc-rsync --whole-file --inplace vs cp control to pin the IocpWriter per-IO drain (audit rows #1, #4, #13). - read_only_iocp: oc-rsync --dry-run vs upstream --dry-run over the 1 GiB fixture to pin the IocpReader drain (audit rows #2, #3). - network_only_loopback: rsync push between two short-lived loopback daemons on 127.0.0.1, cancelling out disk bandwidth so only the IocpSocket send/recv path varies (audit rows #8-#11). Documents the invocation, hotspot mapping, and interpretation rules in docs/benchmarks/windows-throughput.md. The drilldown scenarios are not in the required-checks list and have no acceptable-band thresholds; they exist to attribute movement, not to gate merges. * docs(architecture): BGID lifecycle (#2299) (#4353) Add docs/architecture/bgid-lifecycle.md covering the allocation flow, Drop-driven recycling rules, u16 namespace exhaustion math, pre-sized free-list rationale, planned high-water-mark stat (BGE-3), and the planned graceful fallback when exhausted (BGE-6). Cross-references the audit at docs/audits/bgid-lifecycle.md (PR #4331) and the session topology overview. * feat(metadata): Windows DACL/SACL SDDL round-trip (#2307, #2308) (#4354) Adds opt-in SDDL serialisation helpers in crates/metadata/src/acl_windows.rs: - read_dacl_sddl(path): GetNamedSecurityInfoW + ConvertSecurityDescriptor ToStringSecurityDescriptorW for owner/group/DACL. - read_sddl_with_sacl(path): same plus SACL (requires SE_SECURITY_NAME). - write_dacl_sddl(path, sddl): ConvertStringSecurityDescriptorToSecurity DescriptorW + SetNamedSecurityInfoW, applying owner, group, DACL, and SACL components present in the parsed descriptor. DACL is written with PROTECTED_DACL_SECURITY_INFORMATION per design section 5.2 to avoid silently inheriting extra ACEs from the parent directory. All Win32-allocated buffers are wrapped in RAII guards (OwnedSecurity Descriptor, OwnedLocalWString) that release via LocalFree on drop, even on error paths. Unsafe blocks each carry SAFETY notes naming the invariant they uphold. Implements the Windows fidelity payload described in docs/design/windows-ntfs-acl-support.md section 4.2. Re-exports the new functions from crates/metadata/src/lib.rs under the existing cfg(all(feature = "acl", windows)) gate so consumers can drive the round-trip without poking module internals. Tests on a Windows tempdir: - read_dacl_sddl_returns_non_empty_for_temp_file - write_dacl_sddl_round_trips_known_descriptor - write_dacl_sddl_preserves_owner_and_group - write_dacl_sddl_rejects_invalid_input * feat(fast_io): BGID high-water mark + 50%-occupancy warning (#2295) (#4355) Adds an observable peak counter to the io_uring buffer-group-ID allocator so operators can see the worst-case namespace pressure the process has experienced. - `PEAK_USED: AtomicU16` is updated via `fetch_max` after every successful `BgidAllocator::allocate`, including reuses from the free-list. Deallocation never lowers the value. - `bgid_peak_used()` and `bgid_inflight()` expose the stat and the live snapshot for dashboards and capacity tests. - The free-list is pre-sized to 4 096 entries so steady-state daemon churn does not trigger `Vec` reallocations under the free-list mutex. - When the in-flight count crosses 50 % of the 16-bit namespace (32 768) a throttled `tracing::warn!` fires at most once per 30 s so a hot allocate loop cannot flood the log. - Non-Linux stub mirrors the new public accessors with zero returns so cross-platform callers compile without cfg-gating. - Tests cover the 100-allocation peak, the deallocate monotonicity, the pre-sized capacity, and the inflight subtraction. * docs(architecture): drain error recovery contract (#2385) (#4356) * feat(fast_io): IOCP concurrent_ops auto-size based on CPU count (#2302) (#4358) Replace the static IocpConfig::concurrent_ops default (4) with a CPU-derived value: (cpus * 4).clamp(8, 64). This mirrors io_uring's default_sq_entries derivation so wide hosts keep more overlapped WriteFile operations in flight, while a 1-CPU host still gets a healthy 8-deep submission window. The field stays overridable for explicit tuning. The COMPLETION_DRAIN_BATCH = 64 in disk_batch.rs and DEFAULT_BATCH_SIZE = 64 in pump.rs remain fixed (intentionally aligned with the new MAX_CONCURRENT_OPS ceiling) so a single GetQueuedCompletionStatusEx call can reap an entire in-flight cohort; the pump path already grows on demand up to MAX_BATCH_SIZE on ERROR_INSUFFICIENT_BUFFER (#1930). * docs: Mutex poison recovery policy (#2360) (#4359) Capture the engine-wide contract for reacting to a poisoned Mutex / RwLock. Distils the per-site audit at docs/audits/mutex-poison-policy.md and the helpers landed in crates/engine/src/util/poison.rs into one document new code can follow. Defines the four cases (RECOVERABLE, FATAL, TEST-ONLY, UNAUDITED), the decision tree for picking between them, code templates for the RECOVERABLE (lock_or_recover) and FATAL (expect + # Panics rustdoc) paths, the per-crate audit counts (23 fatal / 28 recoverable / 317 test-only across 368 sites), and the MPE-4..MPE-10 promotion path that ends with the contract becoming a CI lint. Cross-references docs/architecture/drain-error-recovery.md for the ATU-series companion and the rsync_io/fast_io call sites that already match the helper recipe. * feat(engine): SpillPolicy struct + ConcurrentDeltaConfig wiring (#2336) (#4360) Introduce a public SpillPolicy aggregate that groups every spill-layer knob (threshold_bytes, dir, reclaim_mode, granularity, compression) under one value, and wire it through ConcurrentDeltaConfig so callers can stage policy changes without touching the DeltaConsumer API. The legacy spill_threshold_bytes / spill_dir fields collapse into spill_policy; deprecated accessors keep existing call sites compiling while pointing readers at the new field. Defaults match the historical behaviour (no spill, no extra plumbing). The new SpillCompression::Zstd variant lives behind the additive spill-compression feature so default builds gain no dependency. * test(metadata): Windows source -> Linux destination ACL round-trip (#2312) (#4366) Pins the cross-platform translation contract from the Windows NTFS ACL design (docs/design/windows-ntfs-acl-support.md, sections 5.1/5.2/5.3): - Forward leg: synthetic Windows-side DACL with owner/group/Everyone allow ACEs plus one named user lowers to user_obj/group_obj/other_obj and a RsyncAcl::names entry, with name preserved. - Reverse leg: POSIX mode 0o755 plus one named user and one named group round-trips through the Windows-side ACE shape and back to the same rwx triplet, kinds, RIDs, and account names. - Lossy path: deny ACEs are dropped on lowering; POSIX bits derive exclusively from surviving allow ACEs. Gate on OC_RSYNC_METADATA_INTEROP=1 to match acl_xattr_roundtrip_linux, and cfg(unix) so Linux/macOS CI runners can pick it up. Uses synthetic SDDL-style fixtures so no Windows host is needed; assertions exercise the documented mapping rules without depending on the in-flight WAS-2, WAS-3, and WAS-4 helpers. * docs(audits): Windows IOCP profiling methodology (#2300) (#4370) WPG-1 deliverable: methodology a Windows operator follows to convert the wall-clock signal from the throughput bench into a ranked hotspot table. Picks ETW + WPA as the default stack (free, ships with the Windows ADK), with VTune and WPR as alternatives. Documents the six profiling steps (release-with-debug build, drilldown bench run, ETW CPU + IO captures, classify, append results), defines the three hotspot buckets (per-IO blocking drain, CQ-depth saturation, bounce-buffer copy) keyed to rows of the sync-blocking audit and to WPG-3/WPG-4 mitigations, and lays out a five-step implementation plan keyed to WPG-4..WPG-6. * docs: document ssh-socketpair-stderr feature (#2377) (#4368) Adds the experimental `ssh-socketpair-stderr` feature to the README cargo features table and a dedicated subsection in the vNEXT operator migration guide. The flag was introduced by SSE-3 (#2372) on top of the design in docs/design/socketpair-stderr-channel.md (#2371) and gates the AF_UNIX socketpair stderr channel that lets the async SSH transport register the drain with epoll/kqueue/tokio AsyncFd and wake it via shutdown(2) instead of consuming a per-connection blocking thread. Migration guide entry explains when to opt in (Linux endpoints running with async-ssh, fan-out clients, deployments wanting the larger socketpair kernel buffer and shutdown-based wake) and when to leave the default (Windows until SSE-5 lands, sync-only SSH builds, operators who prefer the simpler pipe semantics for debugging). Updates the section 4 count, rollback list, and appendix table. Cross-links to the design doc and the SSE-1 stderr handling audit. Task SSE-8. * docs: SpillPolicy user-facing documentation (#2346) (#4378) Document the receiver-side SpillPolicy surface across the three operator-facing docs: - README: add a Receiver memory tuning subsection under the cargo features table cross-linking the public-API design and the spillable buffer internals. - docs/oc-rsync.1.md: document the planned --spill-dir and --spill-threshold-bytes flags (STN-11) under Performance Options, including the OC_RSYNC_SPILL_* env-var bridge until they ship. - docs/operator-migration-guide-vNEXT.md: add a Receiver spill tunability subsection covering all five OC_RSYNC_SPILL_* env vars, defaults, precedence, and when to override. * docs: ssh-socketpair-stderr opt-in feature (SSE-8 #2377) (#4385) * docs(audit): SPL-9 spill mod.rs re-export audit (#2331) (#4390) * refactor(engine): document delete/context mutex panic invariants (#2353) Per the workspace mutex-poison policy (docs/audits/mutex-poison-policy.md, MPE-1/2/9), every `.lock().expect()` site in `crates/engine/src/delete/context.rs` is classified FATAL. The cursor stack encodes upstream's parent-before-child traversal order and the segment-entries buffer is overwritten in place; recovery from a poisoned guard would mis-order deletes or unlink stale names. The strict policy is to keep `.lock().expect("...")` and document the invariant. Changes - Add `# Panics` sections to `observe_directory`, `begin_directory`, `publish_plan_for` naming the invariant and pointing at the audit doc. - Refresh the existing `# Panics` block on `observe_segment_for_delete` to match the new wording. - Normalise the three `cursor.lock().unwrap()` test sites to `cursor.lock().expect("test cursor poisoned")` so diagnostics match the production message. No production swap to `lock_or_recover` here: the audit lists 0 RECOVERABLE sites in this file. The helper now lives in `crates/engine/src/util/poison.rs` (PR #4342) for later MPE-* tickets that target genuinely recoverable state (drain shard buffer, recorder sinks, bgid free lists). No public API change. `cargo fmt --all` clean.
oferchen
added a commit
that referenced
this pull request
May 18, 2026
Adds three env-gated sub-scenarios to scripts/windows_throughput_bench.sh that isolate the IOCP hotspots catalogued in the IOCP sync-blocking audit, so future Windows write/read/network improvements can be attributed to specific changes instead of moving the aggregate large_1gib / small_10000 numbers. Enabled by OC_RSYNC_BENCH_DRILLDOWN=1; the existing scenarios run unchanged when the flag is absent. - write_only_iocp: oc-rsync --whole-file --inplace vs cp control to pin the IocpWriter per-IO drain (audit rows #1, #4, #13). - read_only_iocp: oc-rsync --dry-run vs upstream --dry-run over the 1 GiB fixture to pin the IocpReader drain (audit rows #2, #3). - network_only_loopback: rsync push between two short-lived loopback daemons on 127.0.0.1, cancelling out disk bandwidth so only the IocpSocket send/recv path varies (audit rows #8-#11). Documents the invocation, hotspot mapping, and interpretation rules in docs/benchmarks/windows-throughput.md. The drilldown scenarios are not in the required-checks list and have no acceptable-band thresholds; they exist to attribute movement, not to gate merges.
oferchen
added a commit
that referenced
this pull request
May 18, 2026
Adds three env-gated sub-scenarios to scripts/windows_throughput_bench.sh that isolate the IOCP hotspots catalogued in the IOCP sync-blocking audit, so future Windows write/read/network improvements can be attributed to specific changes instead of moving the aggregate large_1gib / small_10000 numbers. Enabled by OC_RSYNC_BENCH_DRILLDOWN=1; the existing scenarios run unchanged when the flag is absent. - write_only_iocp: oc-rsync --whole-file --inplace vs cp control to pin the IocpWriter per-IO drain (audit rows #1, #4, #13). - read_only_iocp: oc-rsync --dry-run vs upstream --dry-run over the 1 GiB fixture to pin the IocpReader drain (audit rows #2, #3). - network_only_loopback: rsync push between two short-lived loopback daemons on 127.0.0.1, cancelling out disk bandwidth so only the IocpSocket send/recv path varies (audit rows #8-#11). Documents the invocation, hotspot mapping, and interpretation rules in docs/benchmarks/windows-throughput.md. The drilldown scenarios are not in the required-checks list and have no acceptable-band thresholds; they exist to attribute movement, not to gate merges.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68ef7cab7ee48323ac691f0e08c441b4