From 227d001092828e49e0e75e220a65245d5e961d55 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 10:10:21 +0200 Subject: [PATCH 1/2] feat(p7-devices): finalize device/special-file statuses (devices/copy/write -> implemented, specials -> Impossible/Divergence for sockets) + coverage --- README.md | 8 ++- RSYNC_COMPAT.md | 10 ++-- tests/integration/test_features.py | 91 ++++++++++++++++++++++++++++-- tests/test_file.c | 3 + 4 files changed, 101 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 438e04b..50daa96 100644 --- a/README.md +++ b/README.md @@ -63,8 +63,12 @@ replacement for every rsync feature or protocol mode. - Archive mode does not yet provide all of rsync's `-rlptgoD` behavior. - Symlink transfer is incomplete; link targets are not yet recreated in all modes. -- Owner/group, ACL, xattr, hard-link, device, and special-file handling is - incomplete or unavailable. +- Owner/group, ACL, xattr, and hard-link handling is incomplete or + unavailable. +- Device and special-file preservation is implemented with documented + divergences: recreated device nodes require `CAP_MKNOD` on the receiver (a + non-root receiver skips the entry), and sockets cannot be recreated (FIFOs + are). - Sparse-file handling does not yet preserve all holes correctly. - `--partial`, `--partial-dir`, `-P`, `--append`, and `--append-verify` are not yet full rsync-style resumable transfers. Interrupted files are not retained diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 9961680..e88723e 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -248,10 +248,10 @@ why plain `--append` works on the normal atomic path, not only with `--inplace`. | `-X`, `--xattrs` | Preserve extended attributes | ✅ Implemented | Preserves unprivileged `user.*` extended attributes (Linux `listxattr`/`getxattr` on capture, `fsetxattr` on the written destination fd). Both capture (sender) and application (receiver) are restricted to the `user.*` namespace and the two POSIX ACL xattrs, so a client can **never** force a `security.*`/`trusted.*`/privileged attribute onto the destination; the receiver independently re-validates every incoming name against this whitelist and rejects anything else. Payloads are bounded (per-name ≤255B, per-value ≤1MiB, per-file count ≤256 total bytes ≤4MiB) on both ends, and an oversized/malformed frame is a clean protocol rejection (no OOM). Applied fd-relative to the exact written file. Implies metadata transmission. Incompatible with `-s` (chunk serialization), rejected up front (see the notes); a `--link-dest`/`-H` hard-link copy fallback re-applies the attributes so they are not dropped when a link is refused | | `-H`, `--hard-links` | Preserve hard links | ✅ Implemented | Files on the source that share an inode (`st_dev`+`st_ino`, e.g. a `cp -al` tree) are re-created as hard links to one another on the destination, so duplicate links stay deduplicated and only the first member's data is sent (later members are transmitted as payload-less `STATUS_HARDLINK` frames). The receiver links each sibling to the first member's installed file with an atomic link + rename; on `link()` failure it falls back to a byte-identical local copy of the first member, never a partial/corrupt file. Requires the sequential scan for ordering (the first member is always emitted and installed before any sibling is linked). Works single-threaded and under `-j`/`--threads`, `--inplace`, `--delay-updates` (links staged and published by rename) and `--partial`. Crosses the wire (`preserve_hard_links` bool; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0**, peers must match). Incompatible with `-s` (chunk serialization) and `--append`/`--append-verify`, rejected up front with a distinct error. See the Phase-4 hard-links notes below | | `-D` | Same as --devices --specials | ✅ Implemented | Implies `--devices --specials`. `-D` was unassigned in FastSync (verified: no collision), so it is free to imply both device-node and special-file preservation. See the `--devices`/`--specials` rows and the Phase-4 devices notes below | -| `--devices` | Preserve device files | ⚠️ Partial | Recreates char/block device nodes on the destination via `mknod` instead of transferring content. Type + rdev are validated strictly (S_IFMT from the transmitted mode; major/minor range-checked, non-negative), and creation is **privilege-gated**: `mknod` needs `CAP_MKNOD`, so a non-root receiver (CI runs via setpriv as non-root) logs a warning and **skips the device entry safely** — the whole transfer never aborts just because the node could not be made. The node is created fd-relative below the receive root (`mknodat` on the confined secure parent), so it can never be placed outside the authorized root, never follows a symlink, and never replaces an existing directory. Only a char/block mode is honored. Crosses the wire (a new `STATUS_SPECIAL` frame carries the path + metadata mode + rdev; `PROTOCOL_VERSION` bumped **2.12.0 → 2.13.0**). Divergence: per-entry skip (not a hard error) when the receiver lacks `CAP_MKNOD`, documented in the Phase-4 devices notes | -| `--specials` | Preserve special files | ⚠️ Partial | Recreates **FIFOs** on the destination via `mkfifo` (unprivileged, so this is a real, assertable behavior under CI). Sockets cannot be recreated by any standard filesystem call and are skipped with an explicit note (best-effort / unsupported, matching the plan). FIFO creation is privileged-gated only in the sense of graceful skip on any permission failure. Node creation is confined below the receive root (`mkfifoat` on the secure fd-relative parent; no `..`, no symlink follow). Crosses the wire like `--devices` (the `STATUS_SPECIAL` frame; `PROTOCOL_VERSION` bumped **2.12.0 → 2.13.0**). See the Phase-4 devices notes | -| `--copy-devices` | Copy device contents as file | ⚠️ Partial | Copy a device's CONTENT into an ordinary regular file on the destination instead of recreating the node — non-privileged and safe. FastSync scans a device/FIFO as a regular file: its reported size (`st_size`, typically 0 for char devices and FIFOs) is copied, so a FIFO or a non-readable device becomes an empty (or size-bounded) regular file without ever blocking or reading unbounded pseudo-device streams. The run always succeeds and never crashes on such input. **Deliberate, safe divergence from rsync's dd-like unbounded device read.** See the Phase-4 devices notes | -| `--write-devices` | Write to devices as files | ⚠️ Partial | Write the received data directly into an **existing** device node on the destination instead of creating a regular file. Restricted and best-effort: the destination must already exist and be a char/block device (opened only under the confined receive root, with `O_NOFOLLOW` + `O_NONBLOCK`); a missing, symlinked, FIFO-with-no-reader (`ENXIO`), non-device destination, or any write failure is **skipped with a warning** rather than allowed, so a run can never clobber the system, never blocks on a special-file target, and never aborts on an unusable target. See the Phase-4 devices notes | +| `--devices` | Preserve device files | ✅ Implemented | Recreates char/block device nodes on the destination via `mknod` instead of transferring content. Type + rdev are validated strictly (S_IFMT from the transmitted mode; major/minor range-checked, non-negative), and creation is **privilege-gated**: `mknod` needs `CAP_MKNOD`, so a non-root receiver (CI runs via setpriv as non-root) logs a warning and **skips the device entry safely** — the whole transfer never aborts just because the node could not be made. The node is created fd-relative below the receive root (`mknodat` on the confined secure parent), so it can never be placed outside the authorized root, never follows a symlink, and never replaces an existing directory. Only a char/block mode is honored. Crosses the wire (a new `STATUS_SPECIAL` frame carries the path + metadata mode + rdev; `PROTOCOL_VERSION` bumped **2.12.0 → 2.13.0**). Divergence: per-entry skip (not a hard error) when the receiver lacks `CAP_MKNOD`, documented in the Phase-4 devices notes | +| `--specials` | Preserve special files | ⛔ Impossible/Divergence | **FIFO recreation works**: FIFOs are recreated on the destination via `mkfifo` (unprivileged, so this is a real, assertable behavior under CI). **Only socket recreation is impossible**: a socket entry can be created only by `bind(2)` on a live socket, not by any filesystem call, so a source socket is skipped with an explicit note. That one unsupported node kind is why the flag is classified Impossible/Divergence even though FIFO recreation itself works; its normal path is otherwise complete. FIFO creation is privileged-gated only in the sense of graceful skip on any permission failure. Node creation is confined below the receive root (`mkfifoat` on the secure fd-relative parent; no `..`, no symlink follow). Crosses the wire like `--devices` (the `STATUS_SPECIAL` frame; `PROTOCOL_VERSION` bumped **2.12.0 → 2.13.0**). See the Phase-4 devices notes | +| `--copy-devices` | Copy device contents as file | ✅ Implemented | Copy a device's CONTENT into an ordinary regular file on the destination instead of recreating the node — non-privileged and safe. FastSync scans a device/FIFO as a regular file: its reported size (`st_size`, typically 0 for char devices and FIFOs) is copied, so a FIFO or a non-readable device becomes an empty (or size-bounded) regular file without ever blocking or reading unbounded pseudo-device streams. The run always succeeds and never crashes on such input. **Deliberate, safe divergence from rsync's dd-like unbounded device read.** See the Phase-4 devices notes | +| `--write-devices` | Write to devices as files | ✅ Implemented | Write the received data directly into an **existing** device node on the destination instead of creating a regular file. Restricted and best-effort: the destination must already exist and be a char/block device (opened only under the confined receive root, with `O_NOFOLLOW` + `O_NONBLOCK`); a missing, symlinked, FIFO-with-no-reader (`ENXIO`), non-device destination, or any write failure is **skipped with a warning** rather than allowed, so a run can never clobber the system, never blocks on a special-file target, and never aborts on an unusable target. See the Phase-4 devices notes | | `-U`, `--atimes` | Preserve access times | ✅ Implemented | Captures the source access time (from the scanner's pre-read stat, so it is not clobbered by reading the file for transfer) and transmits it over the wire; the receiver restores it together with the mtime via `futimens`/`utimensat`. Implies metadata transmission (the times travel inside the `-M` metadata payload), but does not enable ownership application (that stays opt-in via the identity flags). Wire: new `atime` fields on the metadata frame + a `preserve_atimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** | | `-N`, `--crtimes` | Preserve create times | ⚠️ Partial | Captures the source birth time via `statx(STATX_BTIME)` on Linux and transmits it (recorded as a wire field), but there is **no portable way to set a birth time** (`utimensat` can only set atime/mtime), so the receiver explicitly does NOT apply it: it logs a debug note and continues — never failing the transfer and never pretending it worked. On platforms without `statx` it parses as a documented no-op (flag accepted; nothing is captured). Implies metadata transmission. Wire: new `crtime` fields + a `preserve_crtimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** (see the Phase-4 metadata-time notes) | | `-O`, `--omit-dir-times` | Omit dirs from --times | 🔄 Compatibility No-op | Accepted and parsed for CLI compatibility, and the config boolean crosses the wire, but it has **no effect**: FastSync never preserves directory mtimes in the first place (directories are created via `mkdir` with no metadata, a documented divergence under `-d`/recursive), so there is nothing for an "omit" to suppress. It never breaks a normal run | @@ -813,7 +813,7 @@ These are the last compatibility items and the closing phase toward rsync flag p **Wave B — Output & filesystem completion.** `-S`/`--sparse` (`⚠️→✅`): real hole preservation (skip zero runs / `SEEK_HOLE` read, `ftruncate` sizing) instead of accepted-and-stored. `-P` (`⚠️→✅`): interrupted-file retention enabling true resumable `--partial` transfers (today only `--progress` is honored). `--block-size=SIZE` (`⚠️→✅`): make the delta checksum block-size genuinely configurable/honored rather than merely parsed as `--delta-block`. `--fake-super` replay (`⚠️→✅` or `Impossible/Divergence`): the FastSync-native `user.fastsync.stat` xattr already records uid/gid/mode/mtime → parse and re-apply it on a later privileged run (possible → implement). `--stderr=client` (`⚠️`): FastSync has no client-side rsync message channel → implement a minimal message classification **or** mark **Impossible/Divergence** (decide in-wave). `-N`/`--crtimes` (`⚠️→Impossible/Divergence`): birth-times cannot be set by any portable fs call → promote from "Partial" to explicit **Impossible/Divergence** (capture + transmit stays). -**Wave C — Devices & special files (finalize statuses + tests).** `--devices`, `--specials`, `--copy-devices`, `--write-devices` (`⚠️`) are already functionally implemented with documented, safety-driven divergences (CAP_MKNOD per-entry skip; FIFO-recreate-with-no-socket; size-bounded content copy; confined best-effort device write). During this wave each is promoted to its final status with coverage tests: `--specials` **sockets** cannot be recreated by any standard filesystem call → mark **Impossible/Divergence**; the rest are complete → **✅**. +**Wave C — Devices & special files (finalize statuses + tests) (✅ implemented).** The four special-file rows are finalized with coverage tests. `--devices`, `--copy-devices`, and `--write-devices` are **✅ Implemented**, each with a documented, safety-driven divergence: device-node creation is privilege-gated, so a receiver without `CAP_MKNOD` skips that entry with a warning (a per-entry skip, never a transfer failure); `--copy-devices` copies a device/FIFO's reported size into an ordinary regular file (a size-bounded safe divergence from rsync's unbounded dd-like read); `--write-devices` writes only into an existing char/block node under the confined receive root and skips every unusable target rather than clobbering or aborting. `--specials` is classified **⛔ Impossible/Divergence** for one reason only: **FIFO recreation works** (unprivileged `mkfifo`, asserted under CI), but **sockets cannot be recreated by any standard filesystem call**, so a source socket is skipped with an explicit note. Tests assert FIFO recreation, the safe socket skip, the regular-file result of `--copy-devices`, the skipped/missing and non-device `--write-devices` targets, and (root-gated) real device-node creation; a root runner additionally drops the receiver to an unprivileged user to assert the `CAP_MKNOD` skip is graceful. **Wave D — Times superstructure & arg-protection no-ops (`🔄`).** `-O`/`--omit-dir-times`, `-J`/`--omit-link-times` (`🔄`) are no-ops *only because* FastSync never preserves directory/symlink times in the first place. To make them real (honoring "all possible flags"): **add directory-mtime and symlink-mtime/owner preservation**, so `-O`/`-J` become meaningful modifiers — an intentional behavior addition that reverses the old "never preserves dir times" divergence. If instead this is judged out of scope at Wave-D time, mark both **Impossible/Divergence**. `--secluded-args` (`🔄→Impossible/Divergence`): a true arg-send protocol is large and FastSync already builds remote SSH argv securely (single-quote-escaped shell words, injection-safe), so there is no argument-leak to close; document the already-safe behavior. diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index df3b0f6..9552486 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -3,6 +3,7 @@ import filecmp import os import random import shutil +import socket import stat import subprocess import sys @@ -14,7 +15,8 @@ from common import ( PROJECT_ROOT, BUILD_DIR, TEST_DATA_DIR, run_client, CountingProxy, generate_test_files, verify_transfer, clean_dir, make_result, - get_dest_received_dir, CLIENT_CMD, ServerManager, + get_dest_received_dir, CLIENT_CMD, SERVER_CMD, ServerManager, + _find_free_port, _wait_for_port, ) SOURCE_DIR = os.path.join(TEST_DATA_DIR, "feature_source") @@ -64,14 +66,44 @@ class TestDeviceSpecial: received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) assert stat.S_ISFIFO(os.stat(os.path.join(received, "pipe.fifo")).st_mode) - def test_copy_devices_non_crash(self, shared_server): - """--copy-devices treats a special/device source as a regular-file copy; - a FIFO (st_size 0) must transfer without hanging or crashing.""" + def test_specials_socket_source_skipped_safely(self, shared_server): + """A socket cannot be recreated by any standard filesystem call, so + --specials must skip it with a note and still complete the run (the + adjacent regular file transfers normally; no socket node appears).""" + self._setup() + sock_path = os.path.join(DEVICE_SOURCE, "source.sock") + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + s.bind(sock_path) + result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, + flags=["--specials"], port=shared_server.port) + finally: + s.close() + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) + with open(os.path.join(received, "plain.txt")) as f: + assert f.read() == "regular content\n" + assert not os.path.lexists(os.path.join(received, "source.sock")), ( + "socket source must be skipped, not materialized" + ) + + def test_copy_devices_fifo_becomes_regular_file(self, shared_server): + """--copy-devices treats a special source as an ordinary regular-file + copy: a FIFO (st_size 0) becomes a zero-length REGULAR file on the + destination (never a FIFO, never a hang), and the run succeeds.""" self._setup() os.mkfifo(os.path.join(DEVICE_SOURCE, "device_copy.fifo")) result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, flags=["--copy-devices"], port=shared_server.port) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) + copied = os.path.join(received, "device_copy.fifo") + assert os.path.lexists(copied), "copy-devices source was not transferred" + st = os.lstat(copied) + assert stat.S_ISREG(st.st_mode), ( + f"copy-devices must produce a regular file, got mode {oct(st.st_mode)}" + ) + assert st.st_size == 0, f"expected a size-bounded 0-byte copy, got {st.st_size}" def test_write_devices_non_crash(self, shared_server): """--write-devices writes into an existing device only; when the @@ -85,6 +117,57 @@ class TestDeviceSpecial: flags=["--write-devices"], port=shared_server.port) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + def test_write_devices_regular_file_target_skipped(self, shared_server): + """--write-devices only ever writes into an existing char/block node: a + pre-existing REGULAR file at the destination path is left byte-identical + (not clobbered) and the run still succeeds.""" + self._setup() + received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) + os.makedirs(received, exist_ok=True) + target = os.path.join(received, "plain.txt") + with open(target, "wb") as f: + f.write(b"pre-existing local content\n") + result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, + flags=["--write-devices"], port=shared_server.port) + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + with open(target, "rb") as f: + assert f.read() == b"pre-existing local content\n", ( + "write-devices clobbered a non-device destination" + ) + + @pytest.mark.setpriv + def test_devices_nonroot_receiver_skips_safely(self): + """A receiver without CAP_MKNOD must skip a device entry with a warning + and never abort. A root runner drops the receiver (server) to nobody + via setpriv; on a non-root runner (or without setpriv) the test skips.""" + if os.geteuid() != 0 or shutil.which("setpriv") is None: + pytest.skip("requires root + setpriv to run the receiver unprivileged") + self._setup() + os.mknod(os.path.join(DEVICE_SOURCE, "chardev"), stat.S_IFCHR | 0o666, + os.makedev(1, 3)) + # The unprivileged receiver must be able to create the destination tree. + os.makedirs(DEVICE_DEST, exist_ok=True) + os.chmod(DEVICE_DEST, 0o777) + port = _find_free_port() + server = subprocess.Popen( + ["setpriv", "--reuid=65534", "--regid=65534", "--clear-groups"] + + SERVER_CMD + ["-p", str(port), "--allow-unauthenticated"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + try: + _wait_for_port(port) + result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, + flags=["--devices"], port=port) + finally: + server.terminate() + server.wait(timeout=5) + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:300]}" + received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) + with open(os.path.join(received, "plain.txt")) as f: + assert f.read() == "regular content\n" + assert not os.path.lexists(os.path.join(received, "chardev")), ( + "a receiver without CAP_MKNOD must skip the device node, not create it" + ) + @pytest.mark.skipif(os.geteuid() != 0, reason="requires root to create device nodes") def test_devices_recreates_real_char_device(self, shared_server): """Root-only: a source char device node is recreated on the destination diff --git a/tests/test_file.c b/tests/test_file.c index d771b75..817138e 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -33,6 +33,7 @@ static void test_file_special_rdev_valid() { mode_t fake_char = S_IFCHR | 0600; mode_t fake_blk = S_IFBLK | 0600; mode_t fake_fifo = S_IFIFO | 0600; + mode_t fake_sock = S_IFSOCK | 0600; /* char/block devices: accept a legal pair, reject negative / oversized. */ EXPECT_TRUE(file_special_rdev_valid(1, 3, fake_char)); EXPECT_TRUE(file_special_rdev_valid(0xffff, 0x00ffffff, fake_blk)); @@ -43,6 +44,8 @@ static void test_file_special_rdev_valid() { /* FIFOs/sockets must carry an empty rdev. */ EXPECT_TRUE(file_special_rdev_valid(0, 0, fake_fifo)); EXPECT_FALSE(file_special_rdev_valid(1, 0, fake_fifo)); + EXPECT_TRUE(file_special_rdev_valid(0, 0, fake_sock)); + EXPECT_FALSE(file_special_rdev_valid(0, 1, fake_sock)); EXPECT_FALSE(file_special_rdev_valid(0, 0, (mode_t)(S_IFREG | 0600))); } From 8ca2b74e793492079de69413b562784800f4375f Mon Sep 17 00:00:00 2001 From: TapTap Date: Sat, 12 Sep 2026 11:00:26 +0200 Subject: [PATCH 2/2] fix(p7-devices): sendfile falls back to buffered read for non-regular sources (--copy-devices no longer hangs); test/doc hardening --- RSYNC_COMPAT.md | 2 +- src/client/client_send.c | 18 +++++++- tests/integration/test_features.py | 66 +++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index e88723e..93ddfb4 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -250,7 +250,7 @@ why plain `--append` works on the normal atomic path, not only with `--inplace`. | `-D` | Same as --devices --specials | ✅ Implemented | Implies `--devices --specials`. `-D` was unassigned in FastSync (verified: no collision), so it is free to imply both device-node and special-file preservation. See the `--devices`/`--specials` rows and the Phase-4 devices notes below | | `--devices` | Preserve device files | ✅ Implemented | Recreates char/block device nodes on the destination via `mknod` instead of transferring content. Type + rdev are validated strictly (S_IFMT from the transmitted mode; major/minor range-checked, non-negative), and creation is **privilege-gated**: `mknod` needs `CAP_MKNOD`, so a non-root receiver (CI runs via setpriv as non-root) logs a warning and **skips the device entry safely** — the whole transfer never aborts just because the node could not be made. The node is created fd-relative below the receive root (`mknodat` on the confined secure parent), so it can never be placed outside the authorized root, never follows a symlink, and never replaces an existing directory. Only a char/block mode is honored. Crosses the wire (a new `STATUS_SPECIAL` frame carries the path + metadata mode + rdev; `PROTOCOL_VERSION` bumped **2.12.0 → 2.13.0**). Divergence: per-entry skip (not a hard error) when the receiver lacks `CAP_MKNOD`, documented in the Phase-4 devices notes | | `--specials` | Preserve special files | ⛔ Impossible/Divergence | **FIFO recreation works**: FIFOs are recreated on the destination via `mkfifo` (unprivileged, so this is a real, assertable behavior under CI). **Only socket recreation is impossible**: a socket entry can be created only by `bind(2)` on a live socket, not by any filesystem call, so a source socket is skipped with an explicit note. That one unsupported node kind is why the flag is classified Impossible/Divergence even though FIFO recreation itself works; its normal path is otherwise complete. FIFO creation is privileged-gated only in the sense of graceful skip on any permission failure. Node creation is confined below the receive root (`mkfifoat` on the secure fd-relative parent; no `..`, no symlink follow). Crosses the wire like `--devices` (the `STATUS_SPECIAL` frame; `PROTOCOL_VERSION` bumped **2.12.0 → 2.13.0**). See the Phase-4 devices notes | -| `--copy-devices` | Copy device contents as file | ✅ Implemented | Copy a device's CONTENT into an ordinary regular file on the destination instead of recreating the node — non-privileged and safe. FastSync scans a device/FIFO as a regular file: its reported size (`st_size`, typically 0 for char devices and FIFOs) is copied, so a FIFO or a non-readable device becomes an empty (or size-bounded) regular file without ever blocking or reading unbounded pseudo-device streams. The run always succeeds and never crashes on such input. **Deliberate, safe divergence from rsync's dd-like unbounded device read.** See the Phase-4 devices notes | +| `--copy-devices` | Copy device contents as file | ✅ Implemented | Copy a device's CONTENT into an ordinary regular file on the destination instead of recreating the node — non-privileged and safe. FastSync scans a device/FIFO as a regular file: its reported size (`st_size`, typically 0 for char devices and FIFOs) is copied, so a FIFO or a non-readable device becomes an empty (or size-bounded) regular file. The default data path is size-bounded and never blocks (it sends exactly `st_size` bytes, never an unbounded pseudo-device stream); with `--sendfile`, a non-regular source (FIFO/device) is detected from its `stat` mode and falls back to that same buffered read, so `--copy-devices --sendfile` cannot hang either. The run always succeeds and never crashes on such input. **Deliberate, safe divergence from rsync's dd-like unbounded device read.** See the Phase-4 devices notes | | `--write-devices` | Write to devices as files | ✅ Implemented | Write the received data directly into an **existing** device node on the destination instead of creating a regular file. Restricted and best-effort: the destination must already exist and be a char/block device (opened only under the confined receive root, with `O_NOFOLLOW` + `O_NONBLOCK`); a missing, symlinked, FIFO-with-no-reader (`ENXIO`), non-device destination, or any write failure is **skipped with a warning** rather than allowed, so a run can never clobber the system, never blocks on a special-file target, and never aborts on an unusable target. See the Phase-4 devices notes | | `-U`, `--atimes` | Preserve access times | ✅ Implemented | Captures the source access time (from the scanner's pre-read stat, so it is not clobbered by reading the file for transfer) and transmits it over the wire; the receiver restores it together with the mtime via `futimens`/`utimensat`. Implies metadata transmission (the times travel inside the `-M` metadata payload), but does not enable ownership application (that stays opt-in via the identity flags). Wire: new `atime` fields on the metadata frame + a `preserve_atimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** | | `-N`, `--crtimes` | Preserve create times | ⚠️ Partial | Captures the source birth time via `statx(STATX_BTIME)` on Linux and transmits it (recorded as a wire field), but there is **no portable way to set a birth time** (`utimensat` can only set atime/mtime), so the receiver explicitly does NOT apply it: it logs a debug note and continues — never failing the transfer and never pretending it worked. On platforms without `statx` it parses as a documented no-op (flag accepted; nothing is captured). Implies metadata transmission. Wire: new `crtime` fields + a `preserve_crtimes` config boolean; `PROTOCOL_VERSION` bumped **2.11.0 → 2.12.0** (see the Phase-4 metadata-time notes) | diff --git a/src/client/client_send.c b/src/client/client_send.c index 55a0e74..ea9bee2 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -1259,6 +1259,19 @@ static int send_single_file(Client* client, File* file, Config* config, bool use return 0; } +/* Sendfile calls a blocking open() on the source (file_send_sendfile_with_skip + * -> file_open_for_read), which never returns for a FIFO/device with no writer. + * Only a regular file may take the zero-copy sendfile path; a non-regular source + * (FIFO/device copied by --copy-devices) must use the buffered, size-bounded + * read path instead. `stat` follows symlinks, so a dereferenced symlink to a + * regular file keeps the sendfile fast path. */ +static bool source_is_regular_file(const File* file) { + if (!file || !file->path) + return false; + struct stat st; + return stat(file->path, &st) == 0 && S_ISREG(st.st_mode); +} + static int send_chunk_with_removal(Client* client, Chunk* chunk, Config* config, ArrayList* remove_sources) { if (config->use_chunk_serialization) { @@ -1337,8 +1350,9 @@ static int send_chunk_with_removal(Client* client, Chunk* chunk, Config* config, continue; } bool stream = f->data->data == NULL && f->data->size > 0; - bool use_sendfile = - (config->use_sendfile && !config->use_compression) || (stream && !config->use_compression); + bool use_sendfile = ((config->use_sendfile && !config->use_compression) || + (stream && !config->use_compression)) && + source_is_regular_file(f); SourceFile* source = remove_sources ? source_file_create(f) : NULL; int rc = send_single_file(client, f, config, config->use_incremental, use_sendfile); if (rc == 1) { diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index 9552486..f000a90 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -16,7 +16,7 @@ from common import ( run_client, CountingProxy, generate_test_files, verify_transfer, clean_dir, make_result, get_dest_received_dir, CLIENT_CMD, SERVER_CMD, ServerManager, - _find_free_port, _wait_for_port, + _find_free_port, _wait_for_port, _wait_proc, ) SOURCE_DIR = os.path.join(TEST_DATA_DIR, "feature_source") @@ -25,6 +25,31 @@ DEVICE_SOURCE = os.path.join(TEST_DATA_DIR, "device_source") DEVICE_DEST = os.path.join(TEST_DATA_DIR, "device_dest") +def _start_captured_server(prefix=None, extra_args=None): + """Start a plain-TCP server with captured stdout/stderr for one test. + + Returns (proc, port). The caller owns `proc` and must terminate it via + `_wait_proc` so a server that ignores SIGTERM is killed instead of leaving + a zombie or raising TimeoutExpired. The shared session server discards its + output, so tests that lock in a receiver-side warning need their own. The + server's SIGTERM handler exits via `_exit`, which does not flush stdio, so + `stdbuf -oL` keeps stdout line-buffered and the warning observable.""" + port = _find_free_port() + cmd = ["stdbuf", "-oL"] + (prefix or []) + SERVER_CMD + ["-p", str(port), "--allow-unauthenticated"] + if extra_args: + cmd += extra_args + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + _wait_for_port(port) + return proc, port + + +def _stop_captured_server(proc): + """Terminate a captured server and return its (stdout, stderr) text.""" + proc.terminate() + _wait_proc(proc) + return proc.communicate() + + class TestDeviceSpecial: """Phase 4: --devices / --specials / -D / --copy-devices / --write-devices. @@ -66,19 +91,22 @@ class TestDeviceSpecial: received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) assert stat.S_ISFIFO(os.stat(os.path.join(received, "pipe.fifo")).st_mode) - def test_specials_socket_source_skipped_safely(self, shared_server): + @pytest.mark.ci + def test_specials_socket_source_skipped_safely(self): """A socket cannot be recreated by any standard filesystem call, so --specials must skip it with a note and still complete the run (the adjacent regular file transfers normally; no socket node appears).""" self._setup() sock_path = os.path.join(DEVICE_SOURCE, "source.sock") s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + server, port = _start_captured_server() try: s.bind(sock_path) result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, - flags=["--specials"], port=shared_server.port) + flags=["--specials"], port=port) finally: s.close() + out, err = _stop_captured_server(server) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) with open(os.path.join(received, "plain.txt")) as f: @@ -86,15 +114,23 @@ class TestDeviceSpecial: assert not os.path.lexists(os.path.join(received, "source.sock")), ( "socket source must be skipped, not materialized" ) + assert "socket not recreated" in (out + err), ( + f"receiver did not log the documented socket skip: out={out!r} err={err!r}" + ) - def test_copy_devices_fifo_becomes_regular_file(self, shared_server): + @pytest.mark.ci + @pytest.mark.parametrize("flags", [["--copy-devices"], ["--copy-devices", "--sendfile"]]) + def test_copy_devices_fifo_becomes_regular_file(self, shared_server, flags): """--copy-devices treats a special source as an ordinary regular-file copy: a FIFO (st_size 0) becomes a zero-length REGULAR file on the - destination (never a FIFO, never a hang), and the run succeeds.""" + destination (never a FIFO, never a hang), and the run succeeds. The + --sendfile variant previously blocked forever in the sendfile open(); + the non-regular source now falls back to the buffered read path, so it + must complete within the bounded-time assertion below.""" self._setup() os.mkfifo(os.path.join(DEVICE_SOURCE, "device_copy.fifo")) - result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, - flags=["--copy-devices"], port=shared_server.port) + result, dur = run_client(DEVICE_SOURCE, DEVICE_DEST, + flags=flags, port=shared_server.port) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) copied = os.path.join(received, "device_copy.fifo") @@ -104,6 +140,7 @@ class TestDeviceSpecial: f"copy-devices must produce a regular file, got mode {oct(st.st_mode)}" ) assert st.st_size == 0, f"expected a size-bounded 0-byte copy, got {st.st_size}" + assert dur < 60, f"{' '.join(flags)} hung on a FIFO source" def test_write_devices_non_crash(self, shared_server): """--write-devices writes into an existing device only; when the @@ -117,6 +154,7 @@ class TestDeviceSpecial: flags=["--write-devices"], port=shared_server.port) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:200]}" + @pytest.mark.ci def test_write_devices_regular_file_target_skipped(self, shared_server): """--write-devices only ever writes into an existing char/block node: a pre-existing REGULAR file at the destination path is left byte-identical @@ -148,18 +186,13 @@ class TestDeviceSpecial: # The unprivileged receiver must be able to create the destination tree. os.makedirs(DEVICE_DEST, exist_ok=True) os.chmod(DEVICE_DEST, 0o777) - port = _find_free_port() - server = subprocess.Popen( - ["setpriv", "--reuid=65534", "--regid=65534", "--clear-groups"] + - SERVER_CMD + ["-p", str(port), "--allow-unauthenticated"], - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + server, port = _start_captured_server( + prefix=["setpriv", "--reuid=65534", "--regid=65534", "--clear-groups"]) try: - _wait_for_port(port) result, _ = run_client(DEVICE_SOURCE, DEVICE_DEST, flags=["--devices"], port=port) finally: - server.terminate() - server.wait(timeout=5) + out, err = _stop_captured_server(server) assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:300]}" received = get_dest_received_dir(DEVICE_DEST, DEVICE_SOURCE) with open(os.path.join(received, "plain.txt")) as f: @@ -167,6 +200,9 @@ class TestDeviceSpecial: assert not os.path.lexists(os.path.join(received, "chardev")), ( "a receiver without CAP_MKNOD must skip the device node, not create it" ) + assert "cannot create device node" in (out + err), ( + f"receiver did not log the documented CAP_MKNOD skip: out={out!r} err={err!r}" + ) @pytest.mark.skipif(os.geteuid() != 0, reason="requires root to create device nodes") def test_devices_recreates_real_char_device(self, shared_server):