Merge branch 'fix/parity-review-c' into feat/parity-fixes

RSYNC_COMPAT.md

@@ -80,7 +80,7 @@ Every one of those has an entry below with its remaining caveats.
 
 | Flag | Rsync Description | FastSync Status | Notes |
 |------|-------------------|-----------------|-------|
-| `--stats` | Give transfer stats | ⚠️ Caveat | Prints transfer statistics. Protocol 2.25.0 populates the receiver-only counters the sender cannot observe: `Matched data` (a delta basis's reused bytes) and `Number of deleted files` come from the receiver's `STATUS_STATS` report. The sender now tracks the scanned file list per type and only counts regular files the receiver actually stored, so `Number of files` carries rsync's `(reg: X, dir: Y, link: Z, special: W)` breakdown (directories come from the scanner's captured directory list, present for `-a`/`-t`/`-p`), `Number of regular files transferred` excludes symlinks/specials and up-to-date files, `Total file size` includes symlink target lengths, and `Total transferred file size`/`Literal data` count only transferred files — all differential-tested in the sequential and `--threads` paths. **Remaining divergences:** `Number of created files` is the transferred-regular count (FastSync cannot tell which entries the receiver newly created, so on an update where rsync reports 0 created FastSync can report the transferred file) and lacks the type breakdown; a recursive scan that preserves no directory attribute (`-r` without `-t`/`-p`) captures no directory entries, so the `dir:` category is then omitted; rsync's per-type breakdown on `Number of deleted files` is not reproduced; and `Total bytes sent`/`received` are FastSync wire bytes framed differently from rsync's, so they are not numerically comparable |
+| `--stats` | Give transfer stats | ⚠️ Caveat | Prints transfer statistics. Protocol 2.25.0 populates the receiver-only counters the sender cannot observe: `Matched data` (a delta basis's reused bytes) and `Number of deleted files` come from the receiver's `STATUS_STATS` report. The sender now tracks the scanned file list per type and only counts regular files the receiver actually stored, so `Number of files` carries rsync's `(reg: X, dir: Y, link: Z, special: W)` breakdown (directories come from the scanner's captured directory list, present for `-a`/`-t`/`-p`), `Number of regular files transferred` excludes symlinks/specials and up-to-date files, `Total file size` includes symlink target lengths, and `Total transferred file size`/`Literal data` count only transferred files — all differential-tested in the sequential and `--threads` paths. **Remaining divergences:** `Number of created files` is the transferred-regular count (FastSync cannot tell which entries the receiver newly created, so on an update where rsync reports 0 created FastSync can report the transferred file) and lacks the type breakdown; a recursive scan that preserves no directory attribute (`-r` without `-t`/`-p`) captures no directory entries, so the `dir:` category is then omitted; `Literal data` is exact for a whole-file transfer but an upper bound for a delta transfer (the sender counts each stored file's whole source size rather than only the literal fragments rsync ships, since it does not measure the delta payload it sends); rsync's per-type breakdown on `Number of deleted files` is not reproduced; and `Total bytes sent`/`received` are FastSync wire bytes framed differently from rsync's, so they are not numerically comparable |
 | `-h`, `--human-readable` | Human-readable numbers | ✅ Parity | Formats transfer byte and rate counts using rsync's **decimal** (base-1000) units, matching rsync `-h` (e.g. `1.23M`), not binary units. **A lone `-h` with no transfer arguments prints help instead** (protocol 2.26.0), matching the rsync idiom; `-h` alongside a transfer remains human-readable |
 | `-i`, `--itemize-changes` | Per-file change summary | ✅ Parity | Prints rsync-style `>f+++++++++` lines to stdout only for files actually sent (also under `-j`/`--threads`); unchanged files print nothing, matching single-`-i` behavior |
 | `--progress` | Show progress | ⚠️ Caveat | Protocol 2.25.0 prints rsync-style per-file progress blocks (percent, transferred/total bytes, rate, elapsed, `(xfr#N, to-chk=M/T)`) fed by the receiver's `STATUS_STATS`, in both the sequential and `--threads` send paths. FastSync now also prints rsync's leading `./` transfer-root line and counts that root entry in the `to-chk` denominator, so a **single-file transfer's name lines and deterministic frames are byte-identical to rsync** (differential test). **Remaining divergences:** for a multi-directory tree rsync prints a per-directory name line and its `to-chk` denominator includes every directory/symlink/special entry; FastSync's streaming scan emits only file-name lines and counts just the root plus transferred files (a full flist pre-count would be needed), and the rate/ETA are wall-clock dependent |

src/client/client_cli.c

@@ -23,6 +23,7 @@
 #include <langinfo.h>
 #include <limits.h>
 #include <locale.h>
+#include <math.h>
 #include <time.h>
 #include <signal.h>
 #include <stdbool.h>
@@ -501,6 +502,7 @@ static bool is_accepted_debug_category(const char* name) {
 
 static bool is_accepted_info_category(const char* name) {
   static const char* const categories[] = {
+      "backup",
       "mount",
       "syms",
       "symsafe",
@@ -628,8 +630,6 @@ static int parse_info_flags(const char* value, Config* config) {
       flag = LOG_INFO_FLIST;
     else if (strcmp(name, "nonreg") == 0)
       flag = LOG_INFO_NONREG;
-    else if (strcmp(name, "backup") == 0)
-      flag = LOG_INFO_BACKUP;
     else if (strcmp(name, "progress") == 0)
       flag = LOG_INFO_PROGRESS;
     else if (is_accepted_info_category(name))
@@ -1897,17 +1897,41 @@ static int parse_bwlimit_value(const char* value, unsigned long long* bytes_per_
     return -1;
   }
 
-  long long size = 1;
+  long long base = 1;
   for (int i = 0; i < reps; i++) {
-    if (size > LLONG_MAX / mult) {
+    if (base > LLONG_MAX / mult) {
       log_message(LOG_LEVEL_ERROR, "--bwlimit=%s is too large", value);
       return -1;
     }
-    size *= mult;
+    base *= mult;
   }
-  size = (long long)((double)size * atof(value));
+  /* rsync multiplies the numeric prefix (atof) by mult^reps in a signed
+   * ssize_t, which is undefined on overflow.  Scale in double and range-check
+   * before converting, so a huge value is rejected as "too large" (where
+   * rsync's overflow happens to land on a negative result) without invoking
+   * signed-overflow UB. */
+  double scaled = (double)base * strtod(value, NULL);
+  /* (double)LLONG_MAX rounds up to 2^63, which is itself out of range for the
+   * cast, so reject at >= that bound; LLONG_MIN == -2^63 is exactly
+   * representable and thus castable, so the lower bound stays strict. */
+  if (!isfinite(scaled) || scaled >= (double)LLONG_MAX || scaled < (double)LLONG_MIN) {
+    log_message(LOG_LEVEL_ERROR, "--bwlimit=%s is too large", value);
+    return -1;
+  }
+  long long size = (long long)scaled;
   if ((*arg == '+' || *arg == '-') && arg[1] == '1' && arg != value) {
-    size += atoi(arg);
+    /* The only form accepted here is "+1"/"-1" (a longer number leaves a
+       trailing byte and is rejected below), so apply the delta directly and
+       guard the one overflow direction. */
+    if (*arg == '+') {
+      if (size == LLONG_MAX) {
+        log_message(LOG_LEVEL_ERROR, "--bwlimit=%s is too large", value);
+        return -1;
+      }
+      size += 1;
+    } else {
+      size -= 1;
+    }
     arg += 2;
   }
   if (*arg != '\0' || size < 0) {

src/client/client_send.c

@@ -229,7 +229,10 @@ static void transfer_stats_note_entry(TransferStats* stats, const File* file) {
 }
 
 /* Account for a regular file (or a whole-file append) the receiver actually
-   stored: rsync's transferred-file count and transferred/literal byte totals. */
+   stored: rsync's transferred-file count and transferred/literal byte totals.
+   `literal_data` counts the whole source size, which is exact for a whole-file
+   send but an upper bound for a delta send (the receiver reuses basis blocks
+   the sender never ships); see TransferStats.literal_data in format.h. */
 static void transfer_stats_note_transferred(TransferStats* stats, const File* file) {
   if (stats == NULL || file == NULL)
     return;
@@ -940,8 +943,6 @@ static void source_file_destroy(void* item) {
   }
 }
 
-static const char* delete_display_path(const Config* config, const char* path);
-
 /* Remove only the same regular source file that was sent. */
 static void remove_transferred_sources(const Config* config, ArrayList* paths) {
   if (!config->remove_source_files || !paths)
@@ -1075,20 +1076,7 @@ static bool receive_stats_record(int fd, ReceiverStats* stats, ArrayList* would_
 static const char* delete_display_path(const Config* config, const char* path) {
   if (!config || !path || !config->send_directory)
     return path;
-  const char* root = config->send_directory;
-  while (*root == '/')
-    root++;
-  const char* rel = path;
-  while (*rel == '/')
-    rel++;
-  size_t root_len = strlen(root);
-  while (root_len > 0 && root[root_len - 1] == '/')
-    root_len--;
-  if (root_len == 0)
-    return rel;
-  if (strncmp(rel, root, root_len) == 0 && (rel[root_len] == '/' || rel[root_len] == '\0'))
-    return rel + root_len + (rel[root_len] == '/' ? 1 : 0);
-  return rel;
+  return utils_strip_transfer_root(path, config->send_directory);
 }
 
 /* Send the final STATUS_FINISHED frame and await the receiver's verdict.

src/client/scanner.c

@@ -438,18 +438,7 @@ static void scanner_record_protected(DirectoryScanner* scanner, const char* fs_p
 static void scanner_note_nonreg(const ScannerOptions* options, const char* fs_path) {
   if (!options || !options->note_nonreg || !fs_path)
     return;
-  const char* rel = *fs_path == '/' ? fs_path + 1 : fs_path;
-  const char* root = options->send_directory;
-  if (root != NULL) {
-    while (*root == '/')
-      root++;
-    size_t root_len = strlen(root);
-    while (root_len > 0 && root[root_len - 1] == '/')
-      root_len--;
-    if (root_len > 0 && strncmp(root, rel, root_len) == 0 &&
-        (rel[root_len] == '/' || rel[root_len] == '\0'))
-      rel += root_len + (rel[root_len] == '/' ? 1 : 0);
-  }
+  const char* rel = utils_strip_transfer_root(fs_path, options->send_directory);
   char* escaped = output_escape(rel, options->eight_bit_output);
   printf("skipping non-regular file \"%s\"\n", escaped ? escaped : rel);
   free(escaped);
@@ -947,7 +936,7 @@ static bool scanner_emit_empty_dir(DirectoryScanner* scanner, ArrayList* chunk_d
   if (!scanner->current_path || !scanner->current_rel || scanner->current_rel[0] == '\0')
     return true;
   struct stat st;
-  if (stat(scanner->current_path, &st) != 0 || !S_ISDIR(st.st_mode))
+  if (lstat(scanner->current_path, &st) != 0 || !S_ISDIR(st.st_mode))
     return true;
   File* dir = scanner_build_dir_file(scanner->current_path, &st, &scanner->options);
   if (!dir)

src/shared/config.h

@@ -82,7 +82,7 @@ typedef struct {
 typedef enum SuperMode { SUPER_MODE_AUTO = 0, SUPER_MODE_ON = 1, SUPER_MODE_OFF = 2 } SuperMode;
 
 /* ===========================================================================
- * Config wire-field table (single source of truth for protocol 2.26.0).
+ * Config wire-field table (single source of truth for protocol 2.27.0).
  *
  * Every field below crosses the wire.  The table is the ONLY place a
  * serialized field is named: config.h expands CONFIG_WIRE_FIELDS() to declare

src/shared/file.c

@@ -807,23 +807,22 @@ bool file_ensure_directory_secure(const char* path) {
     } else if (errno == EEXIST) {
       dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
     }
-  } else if (dir_fd < 0 && (errno == ENOTDIR || errno == ELOOP)) {
-    /* rsync replaces a destination non-directory (regular file or symlink)
-       with an incoming directory.  Confined to the already-opened secure
-       parent fd: the leaf is unlinked by name (never followed) and only a
-       non-directory is ever removed, so this cannot escape the authorized
-       root or remove a pre-existing directory tree.  A symlink is left alone:
-       replacing it is not required for FastSync's transferred directories and
-       keeps --keep-dirlinks semantics untouched. */
+  } else if (dir_fd < 0 && errno == ENOTDIR) {
+    /* rsync replaces a destination non-directory (regular file) with an
+       incoming directory.  Confined to the already-opened secure parent fd:
+       the leaf is unlinked by name (never followed) and only a non-directory
+       is ever removed, so this cannot escape the authorized root or remove a
+       pre-existing directory tree.  A symlink is left alone (openat with
+       O_NOFOLLOW reports ELOOP, which takes no branch here), since replacing
+       it is not required for FastSync's transferred directories and keeps
+       --keep-dirlinks semantics untouched. */
     struct stat leaf_st;
     if (fstatat(parent_fd, leaf, &leaf_st, AT_SYMLINK_NOFOLLOW) == 0 && !S_ISDIR(leaf_st.st_mode) &&
         !S_ISLNK(leaf_st.st_mode)) {
       if (unlinkat(parent_fd, leaf, 0) == 0) {
-        if (mkdirat(parent_fd, leaf, (mode_t)(0777 & ~(mode_t)file_process_umask())) == 0) {
+        if (mkdirat(parent_fd, leaf, (mode_t)(0777 & ~(mode_t)file_process_umask())) == 0)
           created = true;
-        } else if (errno != EEXIST) {
-          /* leave dir_fd < 0 so the caller sees the failure */
-        }
+        /* On failure dir_fd stays < 0 below, so the caller still sees it. */
         dir_fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
       }
     }

src/shared/format.h

@@ -88,7 +88,12 @@ typedef struct {
   unsigned long long total_file_size;       /* sum of entry sizes (link target len) */
   unsigned long long transferred_regular;   /* regular files actually stored */
   unsigned long long transferred_file_size; /* source size of those files */
-  unsigned long long literal_data;          /* literal bytes sent for them */
+  /* Whole-file accuracy: the `--stats` "Literal data" row.  The sender counts
+   * the source size of every stored file, so a whole-file transfer matches
+   * rsync.  A delta run actually ships only the literal fragments of the diff
+   * (the rest is matched/copied), so here the value is an upper bound, not
+   * rsync's literal-byte total; see RSYNC_COMPAT.md's `--stats` row. */
+  unsigned long long literal_data;
 } TransferStats;
 
 #endif

src/shared/log.h

@@ -24,17 +24,18 @@ typedef enum {
   /* rsync categories that map to a FastSync event (emitted in rsync's line
    * format): del (deletions), remove (sender-side source removal), name
    * (transferred entry names), flist (file-list header), nonreg (skipped
-   * non-regular files), backup (backed-up files), progress (per-file progress). */
+   * non-regular files), progress (per-file progress).  rsync's `backup`
+   * category is accepted for CLI parity but stays silent: the receiver does the
+   * backing-up and FastSync has no backup event to report from the sender. */
   LOG_INFO_DEL = 1u << 4,
   LOG_INFO_REMOVE = 1u << 5,
   LOG_INFO_NAME = 1u << 6,
   LOG_INFO_FLIST = 1u << 7,
   LOG_INFO_NONREG = 1u << 8,
-  LOG_INFO_BACKUP = 1u << 9,
-  LOG_INFO_PROGRESS = 1u << 10,
+  LOG_INFO_PROGRESS = 1u << 9,
   LOG_INFO_ALL = LOG_INFO_COPY | LOG_INFO_MISC | LOG_INFO_SKIP | LOG_INFO_STATS | LOG_INFO_DEL |
                  LOG_INFO_REMOVE | LOG_INFO_NAME | LOG_INFO_FLIST | LOG_INFO_NONREG |
-                 LOG_INFO_BACKUP | LOG_INFO_PROGRESS,
+                 LOG_INFO_PROGRESS,
 } LogInfoFlag;
 
 void log_message(LogLevel log_level, const char* message, ...);

src/shared/utils.c

@@ -52,6 +52,31 @@ bool path_is_within_root(const char* root, const char* path) {
   return strncmp(root, path, root_len) == 0 && (path[root_len] == '\0' || path[root_len] == '/');
 }
 
+/* Borrowed transfer-relative view of `path`: strip any leading '/' and then a
+ * `root` prefix (its own leading/trailing slashes tolerated), returning a
+ * pointer into `path`.  Non-allocating, so it is safe on the hot scan/print
+ * paths.  A NULL/empty root, or a path not under `root`, leaves only the
+ * leading-slash strip.  `path` must be NUL-terminated and live in the caller. */
+const char* utils_strip_transfer_root(const char* path, const char* root) {
+  if (path == NULL)
+    return NULL;
+  const char* rel = path;
+  while (*rel == '/')
+    rel++;
+  if (root == NULL)
+    return rel;
+  while (*root == '/')
+    root++;
+  size_t root_len = strlen(root);
+  while (root_len > 0 && root[root_len - 1] == '/')
+    root_len--;
+  if (root_len == 0)
+    return rel;
+  if (strncmp(rel, root, root_len) == 0 && (rel[root_len] == '/' || rel[root_len] == '\0'))
+    return rel + root_len + (rel[root_len] == '/' ? 1 : 0);
+  return rel;
+}
+
 /* Open the destination root directory itself, confined to the authorized root.
  * NOTE (do not merge with file_open_secure_parent): this walk opens dest_root
  * (a directory that must already exist) and returns its fd, whereas

src/shared/utils.h

@@ -191,6 +191,11 @@ const char* utils_get_authorized_root_path(void);
  * callers guarantee this); this is containment by string, not by resolved
  * symlinks.  Shared by the utils and file secure-walk root confinement. */
 bool path_is_within_root(const char* root, const char* path);
+/* Non-allocating transfer-relative view of `path`: strip any leading '/' and
+ * then a `root` prefix (leading/trailing slashes tolerated), returning a
+ * borrowed pointer into `path`.  A NULL/empty root, or a path not under
+ * `root`, yields just the leading-slash strip.  `path`/`root` must stay alive. */
+const char* utils_strip_transfer_root(const char* path, const char* root);
 /* True when `path` contains a ".." component.  This is a purely lexical
  * dot-dot check: an absolute path is NOT rejected here, because default
  * (non-relative) transfers legitimately put the sender's absolute source path

tests/integration/test_option_parity.py

@@ -92,9 +92,13 @@ class TestBwlimitParity:
         result, fast_secs = run_client(source, dest, flags=["-a", "--bwlimit=2048"],
                                        port=shared_server.port)
         assert result.returncode == 0, (result.stderr or result.stdout)[:200]
-        assert fast_secs > 1.0, f"fastsync throttled too little: {fast_secs:.2f}s"
-        # Both rendezvous near 2 s; allow a generous band for CI scheduling.
-        assert abs(fast_secs - rsync_secs) < 1.0, (
+        # 4 MiB at 2 MiB/s rendezvous near 2 s.  Use a coarse band on each side
+        # (an unthrottled transfer finishes well under 1.5 s) plus a generous
+        # cross-tolerance so a loaded CI runner cannot flake the parity assert.
+        lo, hi = 1.5, 4.5
+        assert lo <= fast_secs <= hi, f"fastsync throttle out of band: {fast_secs:.2f}s"
+        assert lo <= rsync_secs <= hi, f"rsync throttle out of band: {rsync_secs:.2f}s"
+        assert abs(fast_secs - rsync_secs) < 2.0, (
             f"fastsync {fast_secs:.2f}s vs rsync {rsync_secs:.2f}s"
         )
 

tests/test_client_cli.c

@@ -1348,8 +1348,8 @@ static void test_parse_args_info_name_and_help() {
 
 /* rsync 3.4.1's full --info/--debug vocabulary parses.  The info categories
  * with a FastSync event set their flag; the remaining rsync-only categories
- * (mount/symsafe/syms) parse but stay silent.  Every --debug category listed
- * here is FastSync-silent, so debug_level stays 0. */
+ * (backup/mount/symsafe/syms) parse but stay silent.  Every --debug category
+ * listed here is FastSync-silent, so debug_level stays 0. */
 static void test_parse_args_rsync_flag_vocabulary_accepted() {
   Config* cfg = config_create();
   char* argv[] = {"fastsync", "--info=backup,del,flist,mount,nonreg,progress,remove,symsafe,syms",
@@ -1361,7 +1361,7 @@ static void test_parse_args_rsync_flag_vocabulary_accepted() {
   int positional_count = 0;
 
   EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0);
-  EXPECT_EQ_INT(cfg->info_level, LOG_INFO_BACKUP | LOG_INFO_DEL | LOG_INFO_FLIST | LOG_INFO_NONREG |
+  EXPECT_EQ_INT(cfg->info_level, LOG_INFO_DEL | LOG_INFO_FLIST | LOG_INFO_NONREG |
                                      LOG_INFO_PROGRESS | LOG_INFO_REMOVE);
   EXPECT_EQ_INT(cfg->debug_level, 0);
   config_delete(cfg);
@@ -3932,6 +3932,55 @@ static void test_parse_args_bwlimit_rsync_units() {
   io_set_bwlimit(0);
 }
 
+/* Huge/malformed --bwlimit values must be rejected (not accepted or UB) and
+ * oversized-but-representable ones must still be accepted: the scaling used to
+ * be done with an unchecked signed double->long long cast, which is undefined
+ * when the product leaves long long's range. */
+static void test_parse_args_bwlimit_huge_and_boundary() {
+  struct {
+    const char* value;
+    unsigned long long expected; /* bytes/sec, ignored when !ok */
+    int ok;
+  } cases[] = {
+      /* Malformed / non-numeric prefixes. */
+      {"1e300", 0, 0},
+      {"99999999999999999999999999e3", 0, 0},
+      /* Products that exceed LLONG_MAX at every suffix. */
+      {"99999999999999999999999999", 0, 0},
+      {"99999999999999999999999999K", 0, 0},
+      {"100000000000000000000P", 0, 0},
+      {"99999999999999999999999999999999999999999999999999B", 0, 0},
+      /* `strtod` overflow to +inf must be caught by the isfinite() guard. */
+      {"9999999999999999999999999999999999999999999999999999999999999999999999"
+       "9999999999999999999999999999999999999999999999999999999999999999999999"
+       "99999999999999999999999999999999999999999999999999999999999999999999999",
+       0, 0},
+      /* 2^52 KiB/s: the largest power-of-two scaling that still fits. */
+      {"4503599627370496", 4611686018427387904ULL, 1},
+      /* The +/-1 suffix forms accepted by rsync. */
+      {"1+1", 1024ULL, 1},
+      {"1-1", 1024ULL, 1},
+  };
+  for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
+    Config* cfg = config_create();
+    EXPECT_NOT_NULL(cfg);
+    int positional_args[2];
+    int positional_count = 0;
+    char option[1024];
+    snprintf(option, sizeof(option), "--bwlimit=%s", cases[i].value);
+    char* argv[] = {"fastsync", option, "/src", "/dst"};
+    int rc = parse_args(cfg, 4, argv, positional_args, &positional_count);
+    if (cases[i].ok) {
+      EXPECT_EQ_INT(rc, 0);
+      EXPECT_TRUE(io_get_bwlimit() == cases[i].expected);
+    } else {
+      EXPECT_EQ_INT(rc, -1);
+    }
+    config_delete(cfg);
+    io_set_bwlimit(0);
+  }
+}
+
 /* --dry-run must not emit a batch file, so it is rejected alongside
  * --read-batch/--only-write-batch. */
 static void test_validate_config_dry_run_rejects_write_batch() {
@@ -4662,6 +4711,7 @@ void test_client_cli() {
   test_parse_args_pattern_file_oversized_rejected();
   test_parse_args_unsigned_options_reject_sign();
   test_parse_args_bwlimit_rsync_units();
+  test_parse_args_bwlimit_huge_and_boundary();
   test_validate_config_dry_run_rejects_write_batch();
   test_parse_args_short_clustering();
   test_parse_args_attached_short_values();

tests/test_config.c

@@ -2921,7 +2921,7 @@ static unsigned long long capture_wire_hash(const Config* cfg, size_t* out_len)
   return h;
 }
 
-/* Byte-for-byte wire compatibility guard (protocol 2.26.0).  The expected hash
+/* Byte-for-byte wire compatibility guard (protocol 2.27.0).  The expected hash
  * pins the pre-X-macro byte stream; the refactor MUST NOT change it. */
 static void test_config_wire_golden() {
   if (is_running_under_valgrind())