From 6f974eff19977ed91eade46b5f15cd571b3c27dd Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 11:56:05 +0200 Subject: [PATCH 1/6] feat(dry-run): server-contacting --dry-run (protocol 2.21.0) --dry-run now handshakes with a remote/daemon receiver and reports what WOULD transfer/skip based on receiver state, mutating nothing on either side. - Serialize Config.dry_run into the wire config frame and append STATUS_DRY_RUN_TRANSFER to the status enum (no renumbering); bump PROTOCOL_VERSION/CMake VERSION/CHANGELOG/golden wire to 2.21.0. - Receiver: receive_incremental_check_ex runs the normal read-only decision and answers STATUS_OK (skip) or STATUS_DRY_RUN_TRANSFER (would transfer) with no basis materialization/append/delta/full transfer. All mutation sites are guarded by !dry_run: file store, manifest deletes, --mkpath root creation, --delay-updates staging, publication, directory-time application, and outcome acks. - Client: send_dry_run_remote connects, sends the config, checks each regular file and prints the would-transfer set + trailer; no file data or delete manifest is sent. Plain local destinations keep the client-side manifest. --- CHANGELOG.md | 15 ++ CMakeLists.txt | 2 +- README.md | 2 +- RSYNC_COMPAT.md | 2 +- src/client/client_cli.c | 1 + src/client/client_send.c | 195 +++++++++++++++++++++- src/server/receiver.c | 34 +++- src/server/receiver_pipeline.c | 12 +- src/server/server.c | 21 ++- src/shared/config.c | 12 +- src/shared/config.h | 36 +++- src/shared/file_receive.c | 51 +++++- src/shared/file_receive.h | 7 + src/shared/protocol.c | 2 + src/shared/protocol.h | 10 +- tests/integration/test_fault_injection.py | 2 +- tests/integration/test_features.py | 165 ++++++++++++++++++ tests/integration/test_preflight.py | 6 +- tests/test_client_cli.c | 9 +- tests/test_config.c | 10 +- tests/test_server.c | 84 ++++++++++ 21 files changed, 632 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe26d5a..4ceec06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ run the same version because the handshake is strict. ## [Unreleased] +### Added + +- **Server-contacting `--dry-run` (protocol 2.21.0).** `--dry-run` now performs + a real handshake with a remote/daemon receiver and reports exactly what WOULD + change based on receiver state (existing destination files, mtimes, checksums, + basis dirs). The wire config carries the dry-run intent (`Config.dry_run`) and + the receiver answers each per-file check with `STATUS_DRY_RUN_TRANSFER` (would + transfer) or `STATUS_OK` (already up to date); the sender prints the + would-transfer set and its trailer without sending any file data. The receiver + performs the normal read-only incremental decision but mutates nothing: no temp + files, writes, renames, deletes, metadata/xattr/chown, or directory creation. + A plain local destination (no explicit `--server-port`/remote) keeps the + original client-side dry-run. Would-delete reporting for `--delete*` is + deferred to a follow-up; dry-run never deletes. + ### Security - Enforce the daemon's per-module `max connections` cap and add a global diff --git a/CMakeLists.txt b/CMakeLists.txt index 479cbca..39c0301 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.22) -project(FastFileTransfer VERSION 2.20.0) +project(FastFileTransfer VERSION 2.21.0) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(CMAKE_C_STANDARD 11) diff --git a/README.md b/README.md index d19ed99..b7d8a0e 100644 --- a/README.md +++ b/README.md @@ -582,7 +582,7 @@ before the module list, before authentication, and the connecting peer address ## Protocol and Security -FastSync protocol version `2.20.0` is shared by the client and server. The +FastSync protocol version `2.21.0` is shared by the client and server. The current protocol is sender-driven and includes configuration negotiation, including the maximum allocation limit, incremental checks, checksums, manifests, keep-alives, abort handling, per-file remove-source results, and diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index df03870..0aab6aa 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -93,7 +93,7 @@ This document maps rsync's full feature set to FastSync's current implementation | Flag | Rsync Description | FastSync Status | Notes | |------|-------------------|-----------------|-------| -| `-n`, `--dry-run` | Trial run with no changes | ✅ Implemented | `dry_run` config field | +| `-n`, `--dry-run` | Trial run with no changes | ✅ Implemented | Server-contacting since protocol 2.21.0: with a remote/daemon destination (or an explicit `--server-port`) the client handshakes with the receiver, which runs the normal read-only per-file check and answers `STATUS_DRY_RUN_TRANSFER`/`STATUS_OK` without mutating anything. A plain local destination keeps the client-side manifest. Would-delete reporting for `--delete*` is deferred (dry-run never deletes). | | `-b`, `--backup` | Make backups of overwritten files | ✅ Implemented | Backup before overwrite | | `--backup-dir=DIR` | Backup directory hierarchy | ✅ Implemented | `backup_dir` config field | | `--suffix=SUFFIX` | Backup suffix (default ~) | ✅ Implemented | `suffix` config field | diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 3fa2004..dd5f2ad 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -1389,6 +1389,7 @@ static int set_server_port_option(Config* config, const char* value, const char* return -1; } config->server_port = port; + config->server_port_set = true; return 0; } diff --git a/src/client/client_send.c b/src/client/client_send.c index e220fd4..9168ed1 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -481,6 +481,21 @@ static void disconnect_transfer_client(Client* client) { client_delete(client); } +/* True when --dry-run should contact a receiver rather than running the + * client-side local manifest. A remote (SSH host:path), daemon + * (host::module/path), or an explicit --server-port/--port selects the + * server-contacting path; a plain local destination keeps the original + * client-side behavior (which never dials the default 127.0.0.1:8080). */ +static bool dry_run_targets_server(const Config* config) { + if (!config) + return false; + if (config->transport == TRANSPORT_SSH) + return true; + if (config->module && config->module[0] != '\0') + return true; + return config->server_port_set; +} + static bool add_chunk_to_manifest(ArrayList* manifest, const Chunk* chunk) { if (!manifest) return true; @@ -1024,6 +1039,12 @@ static int incremental_check(Client* client, File* file, const Config* config, *resume_offset = offset; return 3; } + /* Server-contacting --dry-run: the receiver decided the file is not up to + date and answered "would transfer" WITHOUT expecting any data. The caller + only uses this in the dry-run path; a non-dry-run sender never receives it + because the receiver only emits it when the wire config sets dry_run. */ + if (s == STATUS_DRY_RUN_TRANSFER) + return 4; if (s != STATUS_NEXT) { log_message(LOG_LEVEL_ERROR, "Unexpected server status"); send_status(client->file_descriptor, STATUS_ERROR); @@ -1163,6 +1184,174 @@ static int send_append(const Client* client, File* file, Config* config, return ok ? 0 : -1; } +/* Server-contacting --dry-run. Connects to the configured remote/daemon and + * runs the normal per-file incremental decision WITHOUT transmitting any file + * data: the receiver (which also sees dry_run=true on the wire) answers + * STATUS_OK for an up-to-date file and STATUS_DRY_RUN_TRANSFER for a file it + * would otherwise write, mutating nothing on either side. The would-transfer + * set and the same trailer as the local dry-run are printed. A + * --compare-dest exact basis hit with no destination copy is reported as a + * skip by the receiver. + * + * Only regular files take the receiver-consulted check; directory / symlink / + * special / hard-link-sibling entries have no per-file content check, so they + * are reported conservatively as would-transfer and their frames are never + * sent (which is what keeps the receiver mutation-free). --delete* is + * deliberately NOT transmitted in dry-run, so no deletion can occur; the + * would-delete manifest report is a documented follow-up. + * + * Returns 0 on success, 1 on error. */ +static int send_dry_run_remote(Config* config) { + int from_skipped = 0; + ArrayList* missing_args = NULL; + if (config->delete_missing_args) { + missing_args = array_list_create(free); + if (!missing_args) + return 1; + } + if (!files_from_list_check(config, missing_args, &from_skipped)) { + if (missing_args) + array_list_delete(missing_args); + return 1; + } + if (missing_args) + array_list_delete(missing_args); + /* Alternate basis dirs force the whole-file per-file check on the real + receiver; refuse an oversize source up front exactly as send_files does so + dry-run reports the same clear diagnostic instead of aborting mid-stream. */ + if (config_has_basis(config) && !basis_oversize_preflight(config)) + return 1; + /* Would-delete reporting requires a receiver-side read-only extras walk that + is not implemented yet; be explicit that --delete is a no-op in dry-run + rather than silently ignoring it. */ + if ((config->use_delete || config->delete_missing_args) && !config->quiet) + log_message(LOG_LEVEL_WARNING, + "--dry-run: would-delete reporting is not available in this release; nothing is " + "deleted"); + + /* A live session may follow, so arm graceful abort handling. */ + client_set_abort_armed(true); + Client* client = connect_transfer_client(config); + if (!client) { + if (config->transport == TRANSPORT_TCP) + log_message(LOG_LEVEL_ERROR, "could not connect to server%s", + config->use_tls ? " via TLS" : ""); + client_set_abort_armed(false); + return 1; + } + ProtocolSession session; + protocol_session_init(&session, client->file_descriptor, client->file_descriptor); + protocol_session_set_io_timeout(&session, config->timeout); + protocol_session_set_ssl(&session, (SSL*)client->ssl); + protocol_session_bind(&session); + + int ret = 1; + PreparedScanner prepared; + memset(&prepared, 0, sizeof(prepared)); + DirectoryScanner* scanner = NULL; + if (!config_send(client->file_descriptor, config)) + goto dry_fail; + receive_daemon_motd(client, config); + if (!prepare_scanner(config, 0, &prepared)) + goto dry_fail; + scanner = directory_scanner_create_with_options(config->send_directory, &prepared.options); + if (!scanner) + goto dry_fail; + + int file_count = 0; + unsigned long long total_bytes = 0; + char size_buffer[32]; + if (!config->quiet) + printf("Dry run: files to be transferred\n"); + Chunk* chunk; + while ((chunk = directory_scanner_next(scanner)) != NULL) { + for (int i = 0; i < chunk->element_count; i++) { + File* f = chunk->items[i]; + if (!f) + continue; + unsigned long long fsize = f->data ? f->data->size : 0; + bool would; + if (f->is_dir || f->is_symlink || f->is_special || + (f->link_group != 0 && !f->link_first && f->hardlink_target != NULL)) { + /* No receiver-side content check exists for these frame types; a real + run would (re)create them, so report would-transfer and send no + frame (the receiver must stay mutation-free). */ + would = true; + } else if (fsize > MAX_RECEIVE_WHOLE_FILE_SIZE && !config->use_incremental && + !config_has_basis(config)) { + /* A non-incremental run streams a >whole-file-limit source without the + STATUS_CHECK handshake, so no read-only receiver decision is possible + (and none is needed: a real run would transfer it). */ + would = true; + } else { + DeltaSignature* sig = NULL; + unsigned long long resume_offset = 0; + int rc = incremental_check(client, f, config, &sig, &resume_offset); + delta_signature_destroy(sig); + if (rc < 0) { + chunk_destroy(chunk); + goto dry_fail; + } + if (rc == 1) + continue; /* up to date; nothing to report */ + if (rc != 4) { + log_message(LOG_LEVEL_ERROR, "Unexpected receiver reply during dry-run"); + chunk_destroy(chunk); + goto dry_fail; + } + would = true; + } + if (would) { + if (!config->quiet) { + char* escaped_path = output_escape(file_wire_path(f), config->eight_bit_output); + if (!escaped_path) { + chunk_destroy(chunk); + goto dry_fail; + } + if (config->human_readable) + printf(" %s (%s)\n", escaped_path, + display_bytes(fsize, true, size_buffer, sizeof(size_buffer))); + else + printf(" %s (%llu bytes)\n", escaped_path, fsize); + free(escaped_path); + } + total_bytes += fsize; + file_count++; + } + } + chunk_destroy(chunk); + } + bool io_error = directory_scanner_had_io_error(scanner); + if (directory_scanner_failed(scanner)) + goto dry_fail; + if (io_error) + log_message(LOG_LEVEL_WARNING, "source scan hit an unreadable directory"); + /* Terminate the stream so the receiver emits its success frame; no data + frame and no delete manifest are ever sent in dry-run. */ + if (!send_status(client->file_descriptor, STATUS_FINISHED)) + goto dry_fail; + Status status; + if (!receive_status(client->file_descriptor, &status) || status != STATUS_OK) + goto dry_fail; + if (!config->quiet) { + if (config->human_readable) + printf("Total: %d files, %s\n", file_count, + display_bytes(total_bytes, true, size_buffer, sizeof(size_buffer))); + else + printf("Total: %d files, %.1f MB\n", file_count, (double)total_bytes / (double)BYTES_PER_MIB); + } + ret = io_error ? 1 : 0; + +dry_fail: + if (scanner) + directory_scanner_destroy(scanner); + prepared_scanner_destroy(&prepared); + disconnect_transfer_client(client); + protocol_session_unbind(); + client_set_abort_armed(false); + return ret; +} + // Send a single file directly (non-incremental path). static bool send_file_direct(File* file, int fd, bool use_metadata, int compression_level, const Config* config) { @@ -1931,7 +2120,8 @@ int send_files(Config* config) { if (config->list_only) return send_list_only(config); if (config->dry_run) - return send_dry_run_manifest(config); + return dry_run_targets_server(config) ? send_dry_run_remote(config) + : send_dry_run_manifest(config); ArrayList* missing_args = NULL; int skipped = 0; if (config->delete_missing_args) { @@ -2243,7 +2433,8 @@ int send_files_multithreaded(Config** config_ptr) { if (config->list_only) return send_list_only(config); if (config->dry_run) - return send_dry_run_manifest(config); + return dry_run_targets_server(config) ? send_dry_run_remote(config) + : send_dry_run_manifest(config); ArrayList* missing_args = NULL; int skipped = 0; if (config->delete_missing_args) { diff --git a/src/server/receiver.c b/src/server/receiver.c index a0a3071..2fd803e 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -288,10 +288,19 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver goto fail; } if (status == STATUS_CHECK) { - bool skipped; - File* file = receive_incremental_check(file_descriptor, config, &skipped); - if (!skipped && (!file || !sink->store_file(file, sink->context))) + bool skipped = false; + bool would_transfer = false; + File* file = receive_incremental_check_ex(file_descriptor, config, &skipped, &would_transfer); + if (config->dry_run) { + /* Server-contacting --dry-run: the reply has already been sent + (STATUS_OK = up to date, STATUS_DRY_RUN_TRANSFER = would transfer) and + nothing may be stored. Both flags false means a genuine protocol + error (STATUS_ERROR already sent or sent by receive_error below). */ + if (!skipped && !would_transfer) + goto receive_error; + } else if (!skipped && (!file || !sink->store_file(file, sink->context))) { goto receive_error; + } } else if (status == STATUS_CHUNK) { Chunk* chunk = receive_chunk_data(file_descriptor, config); if (!chunk || !receiver_process_chunk(chunk, sink)) @@ -323,6 +332,15 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver DeleteManifest* manifest = receive_manifest_entries(file_descriptor); if (!manifest) goto fail; /* receive_manifest_entries already sent STATUS_ERROR */ + if (config->dry_run) { + /* Server-contacting --dry-run mutates nothing, so a keep-set manifest + is consumed and discarded. The early-delete mode still needs its ACK + so a sender blocked on the delete handshake is not left hanging. */ + delete_manifest_free(manifest); + if (early_delete && !send_status(file_descriptor, STATUS_OK)) + goto fail; + goto next_status; + } if (early_delete) { /* --delete-before / --delete-during: the manifest is authoritative the moment it arrives, before any file data. Delete now and acknowledge @@ -441,7 +459,11 @@ typedef struct { static bool receiver_save_file(File* file, void* context_pointer) { ReceiverSaveContext* context = context_pointer; FileSaveResult result = FILE_SAVE_ERROR; - if (!context->config->save_to_disk) { + if (context->config->dry_run) { + /* Defense in depth: a dry-run receiver mutates nothing even if a data + frame reaches the sink (the sender is not supposed to send one). */ + result = FILE_SAVE_SKIPPED; + } else if (!context->config->save_to_disk) { /* Nothing is stored; report the file as not-written so a --remove-source-files sender keeps its source. */ result = FILE_SAVE_SKIPPED; @@ -469,6 +491,10 @@ static bool receiver_save_file(File* file, void* context_pointer) { static bool receiver_send_success_frame(int fd, void* context_pointer) { ReceiverSaveContext* context = context_pointer; + /* Server-contacting --dry-run: nothing was staged or written, so there is + nothing to publish and no directory times to stamp. */ + if (context->config->dry_run) + return receiver_send_final_success(fd, context->config, &context->outcomes); /* --delay-updates: the whole protocol stream (including manifest/delete handling, which ran inside receiver_process) has succeeded and every staged file was fully written. Publish them atomically now, before the diff --git a/src/server/receiver_pipeline.c b/src/server/receiver_pipeline.c index abe8719..7c500da 100644 --- a/src/server/receiver_pipeline.c +++ b/src/server/receiver_pipeline.c @@ -196,7 +196,11 @@ int write_thread(void* pipeline_context) { } size_t file_bytes = file->data ? file->data->size : 0; FileSaveResult result = FILE_SAVE_SKIPPED; - if (save_to_disk) { + /* Server-contacting --dry-run: never write. The receiver thread does not + enqueue anything on the dry-run path, but this keeps the writer thread + provably mutation-free if a data frame ever reached it. */ + bool dry_run = context->config->dry_run; + if (save_to_disk && !dry_run) { result = file_save_to_disk_full(root_directory, file, context->config); if (result == FILE_SAVE_ERROR) { file_destroy(file); @@ -215,7 +219,7 @@ int write_thread(void* pipeline_context) { /* P7 Wave D: a directory's times are never applied inline (a later child write would clobber them); accumulate the metadata here and let the caller apply it once every writer has drained. */ - if (result != FILE_SAVE_ERROR && file->is_dir && file->metadata && + if (!dry_run && result != FILE_SAVE_ERROR && file->is_dir && file->metadata && dir_times_should_capture(context->config) && !dir_time_list_add(&context->dir_times, file->path, file->metadata)) { file_destroy(file); @@ -234,8 +238,8 @@ int write_thread(void* pipeline_context) { which sources were actually written versus skipped on the receiver. Explicit directory entries and recreated device/special nodes have no source and are never acknowledged (mirrors receiver.c). */ - if (context->config->remove_source_files && !file->is_dir && !file->is_special && !file->skip && - !receiver_outcomes_append(&context->outcomes, (unsigned char)result)) { + if (!dry_run && context->config->remove_source_files && !file->is_dir && !file->is_special && + !file->skip && !receiver_outcomes_append(&context->outcomes, (unsigned char)result)) { file_destroy(file); pipeline_context_receiver_note_bytes_released(context, file_bytes); mtx_lock(&context->mutex); diff --git a/src/server/server.c b/src/server/server.c index 35e0bd2..8adb60a 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -756,9 +756,12 @@ void handler(int file_descriptor) { skipped via its implied --ignore-missing-args, but nothing is deleted). */ config->delete_missing_args = config->delete_missing_args && allow_delete; /* --mkpath: create the destination root (and its missing leading components) - before anything else; without it the root must pre-exist. A failure here - aborts the connection cleanly before any file data is exchanged. */ - if (!ensure_receive_root(config)) { + * before anything else; without it the root must pre-exist. A failure here + * aborts the connection cleanly before any file data is exchanged. A + * server-contacting --dry-run must NOT create anything: the root is only + * read for the would-transfer/skip decision (an absent root simply means + * "everything would transfer"). */ + if (!config->dry_run && !ensure_receive_root(config)) { char* escaped_root = output_escape(config->receive_root_directory, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "destination root is not available: %s", escaped_root ? escaped_root : ""); @@ -767,8 +770,9 @@ void handler(int file_descriptor) { } /* A --delay-updates transfer stages under a private 0700 directory inside the receive root. Create it up front (wiping leftovers of any previously - interrupted delayed transfer) so a fully-skipped run also starts clean. */ - if (config->delay_updates) { + interrupted delayed transfer) so a fully-skipped run also starts clean. + A dry-run stages nothing, so the staging tree is never created. */ + if (config->delay_updates && !config->dry_run) { config->delay_context = delay_updates_context_create(config->receive_root_directory); if (!config->delay_context || !delay_updates_prepare(config->delay_context)) { log_message(LOG_LEVEL_ERROR, "Failed to initialize --delay-updates staging area"); @@ -864,12 +868,13 @@ void handler(int file_descriptor) { thrd_join(receiver, &receiver_result); thrd_join(writer, &writer_result); bool transfer_ok = receiver_result == thrd_success && writer_result == thrd_success; - if (transfer_ok) { + if (transfer_ok && !config->dry_run) { /* Commit-style (late) deletion: receive_thread handed the keep-set manifest here instead of deleting while write_thread might still be draining, so by now every file is on disk and the whole transfer is known to have succeeded. Remove the extras before publishing a - --delay-updates run; the walker skips the staging directory. */ + --delay-updates run; the walker skips the staging directory. A + server-contacting --dry-run deletes nothing (no manifest is sent). */ if (context->deferred_manifest) { if (!manifest_delete_all(config, context->deferred_manifest)) { transfer_ok = false; @@ -878,7 +883,7 @@ void handler(int file_descriptor) { context->deferred_manifest = NULL; } } - if (transfer_ok) { + if (transfer_ok && !config->dry_run) { /* --delay-updates: receive_thread has finished the whole protocol stream (including manifest/delete handling) and write_thread has drained its queue, so every staged file is complete. Publish atomically before the diff --git a/src/shared/config.c b/src/shared/config.c index 8ac4938..3f9d3a2 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -21,7 +21,6 @@ static void config_set_defaults(Config* config) { config->scanner_threads = 0; config->metadata_explicitly_disabled = false; config->show_progress = false; - config->dry_run = false; config->compression_threads = 0; config->ssh_port = 22; config->transport = TRANSPORT_TCP; @@ -42,6 +41,7 @@ static void config_set_defaults(Config* config) { config->tls_ca = NULL; config->server_host = str_dup("127.0.0.1"); config->server_port = 8080; + config->server_port_set = false; /* 0 means "--timeout not given": the transport keeps its own built-in 30 s * socket timeout (tcp_set_timeouts ignores non-positive values) and the * protocol layer keeps its built-in 60 s per-message deadline. A positive @@ -190,11 +190,11 @@ static bool validate_received_config(const Config* config) { valid_wire_bool(config->delay_updates) && valid_wire_bool(config->mkpath) && valid_wire_bool(config->partial) && valid_wire_bool(config->delete_before) && valid_wire_bool(config->checksum) && valid_wire_bool(config->eight_bit_output) && - checksum_algo_valid(config->checksum_algo) && identity_wire_valid(config) && - valid_wire_bool(config->preserve_atimes) && valid_wire_bool(config->preserve_crtimes) && - valid_wire_bool(config->omit_dir_times) && valid_wire_bool(config->omit_link_times) && - valid_wire_bool(config->munge_links) && valid_wire_bool(config->keep_dirlinks) && - valid_wire_bool(config->fake_super) && + valid_wire_bool(config->dry_run) && checksum_algo_valid(config->checksum_algo) && + identity_wire_valid(config) && valid_wire_bool(config->preserve_atimes) && + valid_wire_bool(config->preserve_crtimes) && valid_wire_bool(config->omit_dir_times) && + valid_wire_bool(config->omit_link_times) && valid_wire_bool(config->munge_links) && + valid_wire_bool(config->keep_dirlinks) && valid_wire_bool(config->fake_super) && (!config->copy_as_set || (config->copy_as_uid >= 0 && config->copy_as_gid >= 0)) && (!config->use_compression || (config->compression_level >= 1 && config->compression_level <= 22)) && diff --git a/src/shared/config.h b/src/shared/config.h index 6e2e425..6180598 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -76,7 +76,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.20.0). + * Config wire-field table (single source of truth for protocol 2.21.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 @@ -109,6 +109,11 @@ typedef enum SuperMode { SUPER_MODE_AUTO = 0, SUPER_MODE_ON = 1, SUPER_MODE_OFF * =========================================================================== */ #define CONFIG_WIRE_HEADER_FIELDS(X) X(version, char*, str_dup(PROTOCOL_VERSION), STR) +/* dry_run (--dry-run) is CLIENT-INTENT that now CROSSES the wire (protocol + * 2.21.0): the receiver needs it to answer what WOULD transfer/skip without + * touching disk. The client-only launch behavior (no server contact for a + * local destination) is decided separately in client_send.c before the frame + * is ever sent. */ #define CONFIG_WIRE_CORE_FIELDS(X) \ X(eight_bit_output, bool, false, BOOL_8BIT) \ X(max_alloc, unsigned long long, DEFAULT_MAX_ALLOC, RAW_MAXALLOC) \ @@ -122,7 +127,8 @@ typedef enum SuperMode { SUPER_MODE_AUTO = 0, SUPER_MODE_ON = 1, SUPER_MODE_OFF X(use_executability, bool, false, BOOL) \ X(compression_level, int, 5, INT) \ X(chunk_size, unsigned long long, DEFAULT_CHUNK_SIZE, RAW) \ - X(use_sendfile, bool, false, BOOL) + X(use_sendfile, bool, false, BOOL) \ + X(dry_run, bool, false, BOOL) #define CONFIG_WIRE_DELTA_FIELDS(X) \ X(use_delete, bool, false, BOOL) \ @@ -261,7 +267,6 @@ typedef struct Config { int scanner_threads; bool metadata_explicitly_disabled; bool show_progress; - bool dry_run; int compression_threads; int ssh_port; TransportType transport; @@ -281,6 +286,12 @@ typedef struct Config { bool use_tls; char* server_host; int server_port; + /* True when --server-port/--port was explicitly given. CLIENT-ONLY (never + * serialized): --dry-run uses it to decide whether a real server handshake + * was requested, so a plain local destination (no explicit port) keeps the + * existing client-side dry-run behavior instead of dialing the default + * 127.0.0.1:8080. */ + bool server_port_set; char* tls_cert; char* tls_key; char* tls_ca; @@ -756,8 +767,23 @@ typedef struct Config { * The bump is therefore a deliberate lockstep-release marker, not a * desynchronization fix — the strict same-version handshake still rejects a * mixed 2.19/2.20 deployment. The chunk codec, which already used the packed - * metadata_to_buf()/metadata_from_buf() form, is unchanged. */ -#define PROTOCOL_VERSION "2.20.0" + * metadata_to_buf()/metadata_from_buf() form, is unchanged. + * + * Server-contacting Dry-run Wave: 2.20.0 -> 2.21.0. + * + * WHY the bump, grounded in the wire: this wave makes --dry-run contact the + * receiver and report exactly what WOULD change. The binary config frame + * gains one serialized bool (Config->dry_run) appended to CONFIG_WIRE_CORE_ + * FIELDS after use_sendfile, and the frame stream gains one terminal status + * (STATUS_DRY_RUN_TRANSFER) sent in reply to a per-file STATUS_CHECK when the + * file is not already up to date. The receiver performs the normal read-only + * incremental decision but no mutation; the sender then skips the data. Any + * config-frame layout or frame-sequence change must bump the protocol version: + * a 2.20 peer would desynchronize on the extra trailing byte and the unknown + * status, and the strict same-version handshake (config_receive rejects a + * mismatched version before parsing anything else) is what keeps a 2.21 client + * and a 2.20 server from ever reaching that state. */ +#define PROTOCOL_VERSION "2.21.0" #define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024) /* Upper bound on total basis-dir entries (rsync caps --link-dest at 20). */ #define MAX_BASIS_DIRS 64 diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 477fa68..6fb154c 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -1647,7 +1647,14 @@ static File* receive_full_file(int fd, const Config* config, const char* path) { return file; } -File* receive_incremental_check(int fd, const Config* config, bool* skipped) { +/* Core implementation. `would_transfer` (may be NULL) is set true only on the + * server-contacting --dry-run path, when the file is not up to date and the + * receiver answered STATUS_DRY_RUN_TRANSFER; the caller then knows no File is + * returned and nothing was stored. */ +File* receive_incremental_check_ex(int fd, const Config* config, bool* skipped, + bool* would_transfer) { + if (would_transfer) + *would_transfer = false; if (!config || !skipped) { send_status(fd, STATUS_ERROR); return NULL; @@ -1799,6 +1806,44 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return NULL; } + /* ---- Server-contacting --dry-run ---- + * The destination does not already hold this file. In dry-run the receiver + * must NOT materialize anything (no basis link/copy, no append/delta/full + * transfer) and the sender must NOT send any data, so answer + * STATUS_DRY_RUN_TRANSFER and return immediately. The one exception is a + * --compare-dest exact hit with no destination copy: a real run would + * suppress the data without changing the destination, so it reports as a + * skip (STATUS_OK) exactly as the full path below would. Everything read + * here (destination file, basis candidates) is read-only. */ + if (config->dry_run) { + bool skip_via_compare = false; + if (config_has_basis(config) && !config->ignore_times) { + BasisMatch basis; + basis_match_find(config, check_path, check_size, (time_t)check_mtime, (long)check_mtime_nsec, + check_digest, check_digest_len, false, &basis); + if (basis.hit && basis.type == BASIS_DEST_COMPARE && !has_old_file) + skip_via_compare = true; + basis_match_free(&basis); + } + Status reply = skip_via_compare ? STATUS_OK : STATUS_DRY_RUN_TRANSFER; + if (!send_status(fd, reply)) { + free(old_data); + close(old_fd); + free(full_path); + free(check_path); + return NULL; + } + if (skip_via_compare) + *skipped = true; + else if (would_transfer) + *would_transfer = true; + free(old_data); + close(old_fd); + free(full_path); + free(check_path); + return NULL; + } + /* ---- Alternate basis directories ---- */ if (config_has_basis(config)) { BasisMatch basis; @@ -2181,6 +2226,10 @@ File* receive_incremental_check(int fd, const Config* config, bool* skipped) { return file; } +File* receive_incremental_check(int fd, const Config* config, bool* skipped) { + return receive_incremental_check_ex(fd, config, skipped, NULL); +} + File* file_receive(const Config* config, int file_descriptor) { char* path = receive_wire_str(file_descriptor); if (path == NULL) diff --git a/src/shared/file_receive.h b/src/shared/file_receive.h index ec6ccfa..83fa910 100644 --- a/src/shared/file_receive.h +++ b/src/shared/file_receive.h @@ -24,6 +24,13 @@ File* file_receive_symlink(int file_descriptor, const Config* config); File* file_receive_special(int file_descriptor); bool file_special_rdev_valid(int32_t major, int32_t minor, mode_t mode); File* receive_incremental_check(int fd, const Config* config, bool* skipped); +/* Extended variant used by the receiver. `would_transfer` (may be NULL) is set + * true only on the server-contacting --dry-run path when the file is not up to + * date: the receiver has already sent STATUS_DRY_RUN_TRANSFER and returns NULL + * without storing anything. On that path `*skipped` is true for an up-to-date + * (STATUS_OK) file and both flags are false for a genuine error. */ +File* receive_incremental_check_ex(int fd, const Config* config, bool* skipped, + bool* would_transfer); /* P7 Wave D directory-time accumulator. The receiver collects the metadata of * every directory it creates/receives (STATUS_MKDIR with metadata and/or the diff --git a/src/shared/protocol.c b/src/shared/protocol.c index c65c4f8..c945765 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -446,6 +446,8 @@ static const char* status_to_string(Status status) { return "AUTH_OK"; case STATUS_AUTH_FAILED: return "AUTH_FAILED"; + case STATUS_DRY_RUN_TRANSFER: + return "DRY_RUN_TRANSFER"; default: return "UNKNOWN"; } diff --git a/src/shared/protocol.h b/src/shared/protocol.h index e55a742..f278aed 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -132,7 +132,15 @@ enum NET_STATUS { STATUS_AUTH_CHALLENGE, STATUS_AUTH_RESPONSE, STATUS_AUTH_OK, - STATUS_AUTH_FAILED + STATUS_AUTH_FAILED, + /* Server-contacting --dry-run (protocol 2.21.0). Sent by the receiver in + * response to a per-file STATUS_CHECK when the wire config carries + * dry_run=true and the file is NOT already up to date: it tells the sender + * the file WOULD be transferred, and the sender must NOT transmit any data + * (the receiver reads none in dry-run). STATUS_OK keeps its meaning in this + * path ("already up to date / nothing to do"). Appended after + * STATUS_AUTH_FAILED so no existing status is renumbered. */ + STATUS_DRY_RUN_TRANSFER }; void io_set_fds(int read_fd, int write_fd); diff --git a/tests/integration/test_fault_injection.py b/tests/integration/test_fault_injection.py index 8978daf..fc0cc1d 100644 --- a/tests/integration/test_fault_injection.py +++ b/tests/integration/test_fault_injection.py @@ -36,7 +36,7 @@ from common import ( # noqa: E402 verify_transfer, ) -PROTOCOL_VERSION = b"2.20.0" +PROTOCOL_VERSION = b"2.21.0" STATUS_MANIFEST = 5 STATUS_OK = 0 diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index 7e2ea6e..bf72dbc 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -370,6 +370,171 @@ class TestDryRun: assert not mismatches, f"Mismatch: {mismatches}" +def _snapshot_tree(root): + """Return {relpath: (size, mtime_ns, content_bytes)} for a directory tree. + + Used to prove a dry-run left the destination byte-for-byte and + timestamp-for-timestamp unchanged. Returns an empty dict for a missing + root so "nothing was created" is also observable.""" + snapshot = {} + if not os.path.exists(root): + return snapshot + for dirpath, _dirnames, filenames in os.walk(root): + for name in filenames: + path = os.path.join(dirpath, name) + rel = os.path.relpath(path, root) + st = os.lstat(path) + if stat.S_ISLNK(st.st_mode): + snapshot[rel] = ("symlink", os.readlink(path), st.st_mtime_ns) + continue + with open(path, "rb") as fh: + data = fh.read() + snapshot[rel] = (st.st_size, st.st_mtime_ns, data) + return snapshot + + +class TestRemoteDryRun: + """Server-contacting --dry-run (protocol 2.21.0): contacts the receiver, + reports what WOULD transfer/skip based on receiver state, and mutates + nothing on either side.""" + + def _seed(self, source): + clean_dir(source) + os.makedirs(os.path.join(source, "nested"), exist_ok=True) + with open(os.path.join(source, "keep.txt"), "wb") as f: + f.write(b"unchanged content\n") + with open(os.path.join(source, "changed.txt"), "wb") as f: + f.write(b"original content\n") + with open(os.path.join(source, "nested", "deep.txt"), "wb") as f: + f.write(b"deep file\n") + + @pytest.mark.ci + def test_remote_dry_run_reports_changes_and_mutates_nothing(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_dst") + self._seed(source) + clean_dir(dest) + + # Populate the destination with a real transfer, then make exactly one + # file differ (content+size) and add a brand-new file. + result, _ = run_client(source, dest, port=shared_server.port) + assert result.returncode == 0, f"seed transfer failed: {result.stderr[:200]}" + received = get_dest_received_dir(dest, source) + + with open(os.path.join(source, "changed.txt"), "wb") as f: + f.write(b"a much longer replacement payload\n") + with open(os.path.join(source, "added.txt"), "wb") as f: + f.write(b"newly added\n") + + before = _snapshot_tree(received) + # --checksum makes the up-to-date decision content-based (the seed + # transfer did not preserve mtimes), so keep.txt/deep.txt report skip. + result, _ = run_client(source, dest, flags=["--dry-run", "--checksum"], + port=shared_server.port) + assert result.returncode == 0, f"remote dry-run failed: {result.stderr[:300]}" + assert "Dry run:" in result.stdout, result.stdout[:200] + assert "changed.txt" in result.stdout, result.stdout + assert "added.txt" in result.stdout, result.stdout + assert "keep.txt" not in result.stdout, ( + f"up-to-date file must not be reported as would-transfer: {result.stdout}" + ) + assert "deep.txt" not in result.stdout, result.stdout + assert _snapshot_tree(received) == before, "remote dry-run mutated the destination" + + @pytest.mark.ci + def test_remote_dry_run_into_empty_dest_creates_nothing(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_empty_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_empty_dst") + self._seed(source) + clean_dir(dest) + received = get_dest_received_dir(dest, source) + assert not os.path.exists(received) + + result, _ = run_client(source, dest, flags=["--dry-run"], port=shared_server.port) + assert result.returncode == 0, f"exit {result.returncode}: {result.stderr[:300]}" + assert "keep.txt" in result.stdout + assert "changed.txt" in result.stdout + assert "deep.txt" in result.stdout + # Nowhere may the receiver have created the destination mirror. + assert not os.path.exists(received), "dry-run created directories on the receiver" + assert _snapshot_tree(received) == {} + + @pytest.mark.ci + def test_remote_dry_run_mkpath_does_not_create_root(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_mk_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_mk_dst") + self._seed(source) + shutil.rmtree(dest, ignore_errors=True) + assert not os.path.exists(dest) + + result, _ = run_client(source, dest, flags=["--dry-run", "--mkpath"], + port=shared_server.port) + assert result.returncode == 0, f"exit {result.returncode}: {result.stderr[:300]}" + assert "changed.txt" in result.stdout + assert not os.path.exists(dest), "dry-run --mkpath created the destination root" + + @pytest.mark.ci + def test_remote_dry_run_with_delete_does_not_delete(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_del_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_del_dst") + self._seed(source) + clean_dir(dest) + result, _ = run_client(source, dest, port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + received = get_dest_received_dir(dest, source) + extra = os.path.join(received, "extra.txt") + with open(extra, "wb") as f: + f.write(b"must survive a dry-run delete\n") + before = _snapshot_tree(received) + + for flags in (["--dry-run", "--delete"], ["--dry-run", "--delete-after"]): + result, _ = run_client(source, dest, flags=flags, port=shared_server.port) + assert result.returncode == 0, f"{flags}: {result.stderr[:300]}" + assert os.path.exists(extra), f"{flags} deleted an extra in dry-run" + assert _snapshot_tree(received) == before, f"{flags} mutated the destination" + + @pytest.mark.ci + def test_remote_dry_run_quiet_is_silent(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_quiet_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_quiet_dst") + self._seed(source) + clean_dir(dest) + result, _ = run_client(source, dest, flags=["-q", "--dry-run"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert result.stdout == "" + assert result.stderr == "" + + @pytest.mark.ci + def test_remote_dry_run_threaded_routes_to_server(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_mt_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_mt_dst") + self._seed(source) + clean_dir(dest) + result, _ = run_client(source, dest, flags=["--dry-run", "--threads"], + port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert "changed.txt" in result.stdout + assert _snapshot_tree(get_dest_received_dir(dest, source)) == {} + + @pytest.mark.ci + def test_normal_transfer_unaffected_by_dry_run(self, shared_server): + """A real transfer after dry-run still installs the changes.""" + source = os.path.join(TEST_DATA_DIR, "remote_dry_normal_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_normal_dst") + self._seed(source) + clean_dir(dest) + run_client(source, dest, port=shared_server.port) + received = get_dest_received_dir(dest, source) + with open(os.path.join(source, "changed.txt"), "wb") as f: + f.write(b"updated payload for the real transfer\n") + run_client(source, dest, flags=["--dry-run"], port=shared_server.port) + + result, _ = run_client(source, dest, port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + with open(os.path.join(received, "changed.txt"), "rb") as f: + assert f.read() == b"updated payload for the real transfer\n" + + class TestRemoveSourceFiles: def test_removes_only_transferred_regular_files(self, shared_server): source = os.path.join(TEST_DATA_DIR, "remove_source") diff --git a/tests/integration/test_preflight.py b/tests/integration/test_preflight.py index 4cbfda3..1995b91 100644 --- a/tests/integration/test_preflight.py +++ b/tests/integration/test_preflight.py @@ -94,14 +94,14 @@ def _seed_protocol_source(source): class TestProtocol: @pytest.mark.ci def test_protocol_current_version_accepted(self, shared_server): - """--protocol=2.20.0 (the current PROTOCOL_VERSION) is accepted and the + """--protocol=2.21.0 (the current PROTOCOL_VERSION) is accepted and the transfer completes normally.""" source = os.path.join(TEST_DATA_DIR, "proto_ok_src") dest = os.path.join(TEST_DATA_DIR, "proto_ok_dst") shutil.rmtree(dest, ignore_errors=True) os.makedirs(dest) _seed_protocol_source(source) - result, _ = run_client(source, dest, flags=["--protocol=2.20.0"], + result, _ = run_client(source, dest, flags=["--protocol=2.21.0"], port=shared_server.port) assert result.returncode == 0, \ f"--protocol current run failed: {(result.stderr or result.stdout)[:400]}" @@ -118,7 +118,7 @@ class TestProtocol: shutil.rmtree(dest, ignore_errors=True) os.makedirs(dest) _seed_protocol_source(source) - for bad in ("2.19.0", "2.18.0", "2.17.0", "2.15.0", "2.16.0", "216", "31"): + for bad in ("2.20.0", "2.19.0", "2.18.0", "2.17.0", "2.15.0", "2.16.0", "216", "31"): result, _ = run_client(source, dest, flags=[f"--protocol={bad}"], port=shared_server.port) assert result.returncode != 0, f"--protocol={bad} should be rejected" diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index b2a7021..ca0a990 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -306,7 +306,7 @@ static void test_parse_args_protocol_accept_current() { Config* cfg = valid_client_config(); EXPECT_NOT_NULL(cfg); char* argv_equals[] = {"fastsync", "--source-dir", "/src", - "--dest-dir", "/dst", "--protocol=2.20.0"}; + "--dest-dir", "/dst", "--protocol=2.21.0"}; int positional_args[2]; int positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 6, argv_equals, positional_args, &positional_count), 0); @@ -316,7 +316,7 @@ static void test_parse_args_protocol_accept_current() { cfg = valid_client_config(); EXPECT_NOT_NULL(cfg); char* argv_space[] = {"fastsync", "--source-dir", "/src", "--dest-dir", - "/dst", "--protocol", "2.20.0"}; + "/dst", "--protocol", "2.21.0"}; positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 7, argv_space, positional_args, &positional_count), 0); EXPECT_EQ_STR(cfg->version, PROTOCOL_VERSION); @@ -326,8 +326,9 @@ static void test_parse_args_protocol_accept_current() { /* Any --protocol value other than the current PROTOCOL_VERSION must end in * failure (parse_args simply stores it; validate_config rejects it up front). */ static void test_parse_args_protocol_rejects_other_versions() { - static const char* const bad_versions[] = { - "2.17", "2.16", "2.15.0", "2.16.0", "2.17.0", "2.18.0", "2.19.0", "216", "31", "abc", ""}; + static const char* const bad_versions[] = {"2.17", "2.16", "2.15.0", "2.16.0", + "2.17.0", "2.18.0", "2.19.0", "2.20.0", + "216", "31", "abc", ""}; for (size_t i = 0; i < sizeof(bad_versions) / sizeof(bad_versions[0]); i++) { Config* cfg = valid_client_config(); EXPECT_NOT_NULL(cfg); diff --git a/tests/test_config.c b/tests/test_config.c index db35101..80e1198 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -2347,6 +2347,7 @@ static void golden_config_populate(Config* c) { c->compression_level = 7; c->chunk_size = 65536; c->use_sendfile = false; + c->dry_run = true; c->use_delete = true; c->use_incremental = true; c->size_only = false; @@ -2443,11 +2444,12 @@ static void golden_config_populate(Config* c) { c->copy_as_gid = 222; } -/* The pinned golden frame (protocol 2.20.0). The values below are the only +/* The pinned golden frame (protocol 2.21.0). The values below are the only * thing that ties the generated table to the historical wire format; update - * them ONLY with a PROTOCOL_VERSION bump and a documented reason. */ -#define GOLDEN_WIRE_LEN 633 -#define GOLDEN_WIRE_HASH 9160991280011164139ULL + * them ONLY with a PROTOCOL_VERSION bump and a documented reason. The 2.21.0 + * bump appends the serialized dry_run bool to CONFIG_WIRE_CORE_FIELDS. */ +#define GOLDEN_WIRE_LEN 637 +#define GOLDEN_WIRE_HASH 13228626061067899189ULL static unsigned long long fnv1a_64(const unsigned char* buf, size_t len) { unsigned long long h = 1469598103934665603ULL; diff --git a/tests/test_server.c b/tests/test_server.c index 82b842e..b2246e8 100644 --- a/tests/test_server.c +++ b/tests/test_server.c @@ -6,6 +6,7 @@ #include "protocol.h" #include "test_utils.h" #include "utils.h" +#include #include #include #include @@ -376,6 +377,88 @@ static void test_incremental_check_size_mismatch_full_transfer() { } } +/* Server-contacting --dry-run: with the wire config's dry_run set, a file that + is NOT up to date makes the receiver answer STATUS_DRY_RUN_TRANSFER and + return immediately; no data body is read and the destination file is left + byte-for-byte unchanged (no temp file, no write, no rename). */ +static void test_incremental_check_dry_run_reports_transfer_without_writing() { + Config* cfg = config_create(); + EXPECT_NOT_NULL(cfg); + cfg->dry_run = true; + char* root = make_check_root("dryw"); + EXPECT_NOT_NULL(root); + cfg->receive_root_directory = str_dup(root); + write_check_file(root, "file.txt", "0123456789abcdef"); + + char path[1024]; + snprintf(path, sizeof(path), "%s/file.txt", root); + struct stat st; + EXPECT_EQ_INT(stat(path, &st), 0); + + int p[2]; + EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0); + io_set_fds(p[0], p[1]); + io_set_bwlimit(0); + + pid_t pid = fork(); + if (pid == 0) { + alarm(30); + close(p[1]); + io_set_fds(p[0], p[0]); + bool skipped = false; + bool would_transfer = false; + File* file = receive_incremental_check_ex(p[0], cfg, &skipped, &would_transfer); + bool ok = file == NULL && !skipped && would_transfer; + file_destroy(file); + config_delete(cfg); + close(p[0]); + _exit(ok ? 0 : 1); + } else { + close(p[0]); + io_set_fds(p[1], p[1]); + EXPECT_TRUE(send_str(p[1], "file.txt")); + unsigned long long size = (unsigned long long)st.st_size + 1; + long long mtime = (long long)st.st_mtime; + long long mtime_nsec = 0; +#ifdef __linux__ + mtime_nsec = (long long)st.st_mtim.tv_nsec; +#endif + EXPECT_TRUE(send_n_data(p[1], &size, sizeof(size))); + EXPECT_TRUE(send_n_data(p[1], &mtime, sizeof(mtime))); + EXPECT_TRUE(send_n_data(p[1], &mtime_nsec, sizeof(mtime_nsec))); + Status s; + EXPECT_TRUE(receive_status(p[1], &s)); + EXPECT_EQ_INT(s, STATUS_DRY_RUN_TRANSFER); + + int status; + waitpid(pid, &status, 0); + close(p[1]); + config_delete(cfg); + /* The destination file must be untouched and no temp sibling may appear. */ + char buf[32] = {0}; + int fd = open(path, O_RDONLY); + EXPECT_TRUE(fd >= 0); + ssize_t got = read(fd, buf, sizeof(buf) - 1); + EXPECT_EQ_INT((int)got, 16); + EXPECT_EQ_STR(buf, "0123456789abcdef"); + close(fd); + DIR* d = opendir(root); + EXPECT_NOT_NULL(d); + int entries = 0; + const struct dirent* e; + while ((e = readdir(d)) != NULL) { + if (strcmp(e->d_name, ".") != 0 && strcmp(e->d_name, "..") != 0) + entries++; + } + closedir(d); + EXPECT_EQ_INT(entries, 1); + unlink(path); + rmdir(root); + free(root); + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0); + } +} + /* Issue #256: when a received delta claims a result above the whole-file cap, receive_delta_file must mark the operation failed so the caller aborts with STATUS_ERROR instead of emitting STATUS_NEXT and waiting for a body that @@ -771,6 +854,7 @@ void test_server() { test_receive_incremental_check_rejects_invalid_nanoseconds(); test_incremental_check_quick_skip_by_mtime(); test_incremental_check_size_mismatch_full_transfer(); + test_incremental_check_dry_run_reports_transfer_without_writing(); test_incremental_check_delta_oversize_reports_failure(); test_late_manifest_abort_frees_keepset(); test_late_manifest_eof_frees_keepset(); From 99df0a8a6d94f54a40ca9585040525285be951e8 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 12:57:23 +0200 Subject: [PATCH 2/6] fix(server): fail-closed dry-run destination-root precondition A wire dry_run bit must not relax the destination-root precondition: previously handlers skipped ensure_receive_root entirely in dry-run, so a client could dry-run against a nonexistent/regular-file root a real session rejects. Split the existence check (receive_root_exists, never creates) from the create path and apply the precondition unconditionally: dry-run runs the existence/directory check only, reports the failure, and creates nothing (no --mkpath). Also allow a `read only = yes` daemon module for a dry-run session (a server-contacting dry-run IS a read-only wire operation) while still refusing it for real writes, and update the stale read-only comments. --- src/server/server.c | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/server/server.c b/src/server/server.c index 8adb60a..f62c822 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -217,12 +217,24 @@ static bool path_is_within(const char* root, const char* path) { root is rejected up front instead of being silently invented by a later write. Both paths are confined to the authorized root by the secure file helpers. */ +/* Existence-only half of the precondition: the destination root must already + resolve to a directory below the authorized root. Never creates anything, so + a server-contacting --dry-run can apply the exact same fail-closed check a + real run would without mutating the tree. */ +static bool receive_root_exists(const Config* config) { + if (!config || !config->receive_root_directory) + return false; + return file_directory_exists_secure(config->receive_root_directory); +} + +/* Full precondition for a real run: --mkpath creates the root (and missing + leading components), otherwise it must already exist as a directory. */ static bool ensure_receive_root(const Config* config) { if (!config || !config->receive_root_directory) return false; if (config->mkpath) return file_ensure_directory_secure(config->receive_root_directory); - return file_directory_exists_secure(config->receive_root_directory); + return receive_root_exists(config); } static bool configure_authorization(const char* root) { @@ -263,9 +275,11 @@ typedef enum { } ModuleAuthResult; /* Looks up the daemon module selected by the client's config frame and rejects - * a `read only` one (every FastSync network transfer writes; there is no - * read-only wire operation yet). Returns the module, or NULL with *error set - * to the caller-facing rejection message. */ + * a `read only` one for a real write transfer. A server-contacting --dry-run + * IS a read-only wire operation (it reports what would transfer/skip and + * mutates nothing), so a `read only` module is the safest possible dry-run + * target and is accepted. Returns the module, or NULL with *error set to the + * caller-facing rejection message. */ static const DaemonModule* module_gate_lookup_module(const Config* config, const char** error) { const DaemonModule* module = daemon_conf_find_module(g_daemon_conf, config->module); if (module == NULL) { @@ -276,7 +290,7 @@ static const DaemonModule* module_gate_lookup_module(const Config* config, const *error = "requested daemon module does not exist"; return NULL; } - if (module->read_only) { + if (module->read_only && !config->dry_run) { log_message(LOG_LEVEL_ERROR, "daemon module '%s' is read only; refusing write transfer", config->module); *error = "requested daemon module is read only"; @@ -556,9 +570,10 @@ static const char* module_gate_install_root(const Config* config, const DaemonMo * becomes the authorized root via configure_authorization -- exactly the same * root confinement the standalone server applies to its single * --destination-root, but per-module and NEVER client-chosen. The module is - * refused (with a clear log) when it is unknown, when it is `read only` (every - * FastSync network transfer writes; there is no read-only wire operation yet), - * when it requests client-chosen ownership without the module's + * refused (with a clear log) when it is unknown, when it is `read only` for a + * real write transfer (a server-contacting --dry-run is a read-only wire + * operation and may target a `read only` module), when it requests + * client-chosen ownership without the module's * `client owner = yes` opt-in (P7 Wave E hardening), or when the presented * daemon credentials fail for a module that declares `auth users`. Wave A * refused every auth-required module (auth was not yet implemented); Wave B @@ -756,12 +771,14 @@ void handler(int file_descriptor) { skipped via its implied --ignore-missing-args, but nothing is deleted). */ config->delete_missing_args = config->delete_missing_args && allow_delete; /* --mkpath: create the destination root (and its missing leading components) - * before anything else; without it the root must pre-exist. A failure here - * aborts the connection cleanly before any file data is exchanged. A - * server-contacting --dry-run must NOT create anything: the root is only - * read for the would-transfer/skip decision (an absent root simply means - * "everything would transfer"). */ - if (!config->dry_run && !ensure_receive_root(config)) { + * before anything else; without it the root must pre-exist. The precondition + * is UNCONDITIONAL: a server-contacting --dry-run must reject exactly the + * root a real session would reject, so a client cannot set the wire dry_run + * bit to relax it. Dry-run only runs the existence/directory check (never + * --mkpath) so it creates nothing while still failing closed. A failure here + * aborts the connection cleanly before any file data is exchanged. */ + bool root_ok = config->dry_run ? receive_root_exists(config) : ensure_receive_root(config); + if (!root_ok) { char* escaped_root = output_escape(config->receive_root_directory, log_get_8_bit_output()); log_message(LOG_LEVEL_ERROR, "destination root is not available: %s", escaped_root ? escaped_root : ""); From a1eaa933577b6a3c15fd412ebc895f19a8b94ac1 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 12:57:26 +0200 Subject: [PATCH 3/6] fix(client): route explicit remote dry-run targets, fail closed on stray status --dry-run --server-host=H (or TLS / source-bind --address) silently ran the client-side manifest even though a real run contacts the server. Add a client-only, never-serialized server_host_set bit (alongside the existing server_port_set) and extend dry_run_targets_server so every explicit remote target contacts the receiver. Also make incremental_check return the dry-run code (4) only when the session actually requested dry-run; a stray STATUS_DRY_RUN_TRANSFER from a hostile/buggy peer is now a logged protocol error (STATUS_ERROR) instead of falling through to send file data and desync. Both normal send_single_file callers handle rc == 4 explicitly as an abort. --- src/client/client_cli.c | 5 ++++ src/client/client_send.c | 52 ++++++++++++++++++++++++++++++++-------- src/shared/config.c | 1 + src/shared/config.h | 6 +++++ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index dd5f2ad..4014e8b 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -1106,6 +1106,11 @@ static bool cli_handle_table_option(CliParseCtx* ctx) { } config->use_metadata = true; } + /* Remember that --server-host was explicitly given (the field itself + defaults to 127.0.0.1, so a value check cannot distinguish it). Used + by --dry-run to route an explicit remote target to the server. */ + if (entry->offset == offsetof(Config, server_host)) + config->server_host_set = true; } } else if (apply_table_option(config, entry, NULL) != 0) { ctx->exit_code = -1; diff --git a/src/client/client_send.c b/src/client/client_send.c index 9168ed1..b4779fc 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -482,10 +482,11 @@ static void disconnect_transfer_client(Client* client) { } /* True when --dry-run should contact a receiver rather than running the - * client-side local manifest. A remote (SSH host:path), daemon - * (host::module/path), or an explicit --server-port/--port selects the - * server-contacting path; a plain local destination keeps the original - * client-side behavior (which never dials the default 127.0.0.1:8080). */ + * client-side local manifest. Any target a real run would reach over the wire + * selects the server-contacting path: a remote (SSH host:path), a daemon + * (host::module/path), an explicit --server-host, --server-port/--port, TLS, or + * a source-bind --address. A plain local destination (none of these) keeps the + * original client-side behavior, which never dials the default 127.0.0.1:8080. */ static bool dry_run_targets_server(const Config* config) { if (!config) return false; @@ -493,7 +494,13 @@ static bool dry_run_targets_server(const Config* config) { return true; if (config->module && config->module[0] != '\0') return true; - return config->server_port_set; + if (config->server_host_set || config->server_port_set) + return true; + if (config->use_tls) + return true; + if (config->address != NULL) + return true; + return false; } static bool add_chunk_to_manifest(ArrayList* manifest, const Chunk* chunk) { @@ -1040,11 +1047,18 @@ static int incremental_check(Client* client, File* file, const Config* config, return 3; } /* Server-contacting --dry-run: the receiver decided the file is not up to - date and answered "would transfer" WITHOUT expecting any data. The caller - only uses this in the dry-run path; a non-dry-run sender never receives it - because the receiver only emits it when the wire config sets dry_run. */ - if (s == STATUS_DRY_RUN_TRANSFER) - return 4; + date and answered "would transfer" WITHOUT expecting any data. Treat it as + the dry-run code ONLY when this session actually requested dry-run. A + hostile/buggy peer that emits it outside dry-run is a protocol error: fail + closed (and send STATUS_ERROR) rather than fall through to the normal path, + which would transmit file data the receiver is not reading and desync. */ + if (s == STATUS_DRY_RUN_TRANSFER) { + if (config->dry_run) + return 4; + log_message(LOG_LEVEL_ERROR, "Unexpected DRY_RUN_TRANSFER status outside a --dry-run session"); + send_status(client->file_descriptor, STATUS_ERROR); + return -1; + } if (s != STATUS_NEXT) { log_message(LOG_LEVEL_ERROR, "Unexpected server status"); send_status(client->file_descriptor, STATUS_ERROR); @@ -1478,6 +1492,16 @@ static int send_single_file(Client* client, File* file, Config* config, bool use } return arc == 0 ? 0 : -1; } + // rc == 4: the receiver answered DRY_RUN_TRANSFER, which is only valid in + // incremental_check's dedicated dry-run consumer. send_single_file never + // runs a dry-run session, so this is a protocol error: abort instead of + // falling through and sending data the receiver is not reading. + if (rc == 4) { + log_message(LOG_LEVEL_ERROR, "Receiver answered DRY_RUN_TRANSFER in a non-dry-run transfer"); + delta_signature_destroy(sig); + send_status(client->file_descriptor, STATUS_ERROR); + return -1; + } // rc == 0: unchanged file, skip // rc == 2: server sent delta signature but sendfile doesn't support delta delta_signature_destroy(sig); @@ -1519,6 +1543,14 @@ static int send_single_file(Client* client, File* file, Config* config, bool use } return arc == 0 ? 0 : -1; } + if (rc == 4) { + /* See the sendfile branch above: DRY_RUN_TRANSFER is only valid in the + dedicated dry-run consumer, never in the normal per-file send path. */ + log_message(LOG_LEVEL_ERROR, "Receiver answered DRY_RUN_TRANSFER in a non-dry-run transfer"); + delta_signature_destroy(sig); + send_status(client->file_descriptor, STATUS_ERROR); + return -1; + } if (rc == 2 && config->use_delta && !config->whole_file) { int drc = send_delta(client, file, sig, config); delta_signature_destroy(sig); diff --git a/src/shared/config.c b/src/shared/config.c index 3f9d3a2..9e83e10 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -42,6 +42,7 @@ static void config_set_defaults(Config* config) { config->server_host = str_dup("127.0.0.1"); config->server_port = 8080; config->server_port_set = false; + config->server_host_set = false; /* 0 means "--timeout not given": the transport keeps its own built-in 30 s * socket timeout (tcp_set_timeouts ignores non-positive values) and the * protocol layer keeps its built-in 60 s per-message deadline. A positive diff --git a/src/shared/config.h b/src/shared/config.h index 6180598..890fd10 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -292,6 +292,12 @@ typedef struct Config { * existing client-side dry-run behavior instead of dialing the default * 127.0.0.1:8080. */ bool server_port_set; + /* True when --server-host was explicitly given. CLIENT-ONLY (never + * serialized), and distinct from the "127.0.0.1" default: --dry-run uses it + * to route an explicit remote target to the server so it reports receiver + * state exactly like a real run, instead of silently running the client-side + * manifest. */ + bool server_host_set; char* tls_cert; char* tls_key; char* tls_ca; From 5b8aca57994605f2c2a867c6bec7eb773eedebf1 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 12:57:30 +0200 Subject: [PATCH 4/6] fix(receive): enforce dry-run no-mutation centrally --dry-run --read-batch=FILE still wrote to the destination because batch_read_apply -> file_save_to_disk_full bypassed the per-caller !dry_run guards. Guard file_save_to_disk_full and manifest_delete_all directly (return SKIPPED/no-op) so every save/delete path is mutation-free in dry-run, and keep the per-caller guards. Reject --dry-run combined with --read-batch/--only-write-batch at CLI validation with a clear error (a dry-run of a local batch apply is not meaningful). --- src/client/client_validation.c | 11 +++++++++++ src/shared/file_receive.c | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/client/client_validation.c b/src/client/client_validation.c index f19886b..ef2a6e2 100644 --- a/src/client/client_validation.c +++ b/src/client/client_validation.c @@ -22,6 +22,17 @@ bool validate_config(const Config* config) { "--write-batch, --only-write-batch, and --read-batch are mutually exclusive"); return false; } + /* A dry-run of a local batch apply is not meaningful: --read-batch bypasses + the client-side scan/server decision entirely, so dry-run would have no + wire state to report (and must not be used as a mutation escape hatch). + --only-write-batch likewise never contacts a receiver. Reject both up + front instead of silently ignoring --dry-run. */ + if (config->dry_run && (read_batch || only_write_batch)) { + log_message(LOG_LEVEL_ERROR, + "--dry-run cannot be combined with --read-batch or --only-write-batch; " + "a dry-run of a local batch apply is not meaningful"); + return false; + } if (read_batch) { if (!config->receive_root_directory) { log_message(LOG_LEVEL_ERROR, "--read-batch requires a destination directory"); diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 6fb154c..9438b1c 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -569,6 +569,13 @@ static FileSaveResult file_save_write_device(const char* root_directory, const F FileSaveResult file_save_to_disk_full(const char* root_directory, const File* file, const Config* config) { + /* Central no-mutation guard: a server-contacting --dry-run (or a local batch + apply that somehow carries dry_run) must never touch the destination, no + matter which caller reached this primitive. The per-caller guards remain, + but this is the last line of defense for every save path. Report SKIPPED + so a --remove-source-files sender correctly keeps its source. */ + if (config && config->dry_run) + return FILE_SAVE_SKIPPED; /* Backups are incompatible with ignore-existing: moving the entry first would make a concurrent no-replace commit overwrite its old name. */ bool backup_enabled = config && config->backup && !config->ignore_existing; @@ -2941,6 +2948,11 @@ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest bool manifest_delete_all(const Config* config, DeleteManifest* manifest) { if (!config || !manifest) return false; + /* Central no-mutation guard: a dry-run never deletes. No manifest is sent on + the dry-run path, but a hostile/buggy peer could; treat it as a no-op so + the receiver can never remove anything. */ + if (config->dry_run) + return true; if (config->delete_missing_args && !manifest_delete_missing_args(config, manifest)) return false; if (config->use_delete && !manifest_delete_extras(config, manifest)) From 07f7555c1de5b8b4c88e844686bb1930c2538f98 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 12:57:33 +0200 Subject: [PATCH 5/6] fix(server): skip dry-run per-file outcome bookkeeping receiver_save_file appended to context->outcomes for --remove-source-files without the !dry_run guard the multithreaded pipeline has, so a hostile dry-run client could grow outcomes unbounded (raw, uncharged realloc) and force a per-frame ack. Guard the append on !dry_run. --- src/server/receiver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/server/receiver.c b/src/server/receiver.c index 2fd803e..6e24e88 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -479,8 +479,12 @@ static bool receiver_save_file(File* file, void* context_pointer) { file_destroy(file); return false; } - if (result != FILE_SAVE_ERROR && context->config->remove_source_files && !file->is_dir && - !file->is_special && !file->skip && + /* A dry-run receiver mutates nothing AND records no per-file outcomes: a + hostile dry-run client that streamed data frames anyway must not be able to + grow `outcomes` without bound (receiver_outcomes_append reallocs uncharged) + or force a per-frame ack. */ + if (!context->config->dry_run && result != FILE_SAVE_ERROR && + context->config->remove_source_files && !file->is_dir && !file->is_special && !file->skip && !receiver_outcomes_append(&context->outcomes, (unsigned char)result)) { file_destroy(file); return false; From 6269ae54e5bad0e5c7bbb294b372da045a05d3af Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 12:57:37 +0200 Subject: [PATCH 6/6] test(dry-run): strengthen no-mutation coverage and refresh docs Extend _snapshot_tree to record mode, inode, xattrs, directories and special nodes, and add coverage proving a server-contacting --dry-run leaves the destination structurally identical for --delay-updates, --backup, symlinks, hardlinks, FIFOs, and daemon modules (including a read-only module). Add a regression test for the --read-batch --dry-run refusal and for a missing/non-directory receive root failing a dry-run exactly like a real run. Fix stale version comments (2.20.0/633 -> 2.21.0/637) and RSYNC_COMPAT's current --protocol value, and add a unit assertion that --server-port/--port (and --server-host) set the dry-run routing bit. --- RSYNC_COMPAT.md | 4 +- tests/integration/test_daemon.py | 30 +++++ tests/integration/test_features.py | 210 +++++++++++++++++++++++++++-- tests/test_client_cli.c | 28 ++++ tests/test_config.c | 4 +- 5 files changed, 258 insertions(+), 18 deletions(-) diff --git a/RSYNC_COMPAT.md b/RSYNC_COMPAT.md index 0aab6aa..f985b9e 100644 --- a/RSYNC_COMPAT.md +++ b/RSYNC_COMPAT.md @@ -679,7 +679,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved | `--stop-after=MINS` | Stop after N minutes | ✅ Implemented | Client-only sender stop deadline (Phase 6): computing `--stop-after=MINS` (a positive minute count; 0/negative/garbage rejected) and `--stop-at=TIME` (`HH:MM`, `HH:MM:SS`, or `now+N[smhd]`; a past time stops immediately). The transfer stops ELEGANTLY at the next chunk boundary: everything already fully sent is kept and applied, the run returns 0, and --delete (late/delete-after timing) does NOT wipe the destination — when the scan is cut short the partial keep-set manifest is suppressed with a warning (the delete walk is skipped rather than acting on an incomplete keep-set, so unscanned source mirrors survive). `--delete-before`/`--delete-during` still run their complete pre-scan (which ignores the deadline). Local client-only fields: never serialized into the wire config frame, so no PROTOCOL_VERSION bump. `--stop-after` uses CLOCK_MONOTONIC; `--stop-at` uses the wall clock. Works single-threaded and under `-j`/`--threads` (multithreaded). Divergence: rsync computes `--stop-after` from the run start; FastSync likewise. When both are given, the earlier of the two deadlines wins (checked per iteration). See the Phase-6 stop notes below | | `--stop-at=TIME` | Stop at specified time | ✅ Implemented | Same feature as `--stop-after` (deadline transfer stop), absolute wall-clock form (`HH:MM[:SS]` or `now+N[smhd]`). See the row above and the Phase-6 stop notes | | `--fsync` | Fsync every written file before publication | ✅ Implemented | | -| `--protocol=NUM` | Force older protocol version | ✅ Implemented | Forces the wire protocol version for this transfer. FastSync has exactly ONE wire format (`PROTOCOL_VERSION`, currently 2.20.0) with no downgrade/backward-compat code paths, so `--protocol=2.20.0` is accepted (it sets the version claim the client sends, which the server already requires to match exactly) and **every other value is rejected up front** with a clear error before any connection — it does not and cannot speak an older or virtual wire format. Divergence from rsync (which negotiates a range and downgrades to an integer 0..31): FastSync's honest contract is force-to-the-one-supported-value; a genuine downgrade would require a per-version compatibility layer that does not exist. Client-only; the server-side exact-match check is unchanged. `--protocol=2.19.0`/`2.18.0`/`2.18`/`2.17.0`/`2.16.0`/`2.15.0`/`216`/`31`/garbage are all rejected. See the Phase-6 protocol note below | +| `--protocol=NUM` | Force older protocol version | ✅ Implemented | Forces the wire protocol version for this transfer. FastSync has exactly ONE wire format (`PROTOCOL_VERSION`, currently 2.21.0) with no downgrade/backward-compat code paths, so `--protocol=2.21.0` is accepted (it sets the version claim the client sends, which the server already requires to match exactly) and **every other value is rejected up front** with a clear error before any connection — it does not and cannot speak an older or virtual wire format. Divergence from rsync (which negotiates a range and downgrades to an integer 0..31): FastSync's honest contract is force-to-the-one-supported-value; a genuine downgrade would require a per-version compatibility layer that does not exist. Client-only; the server-side exact-match check is unchanged. `--protocol=2.20.0`/`2.19.0`/`2.18.0`/`2.18`/`2.17.0`/`2.16.0`/`2.15.0`/`216`/`31`/garbage are all rejected. See the Phase-6 protocol note below | | `--iconv=CONVERT_SPEC` | Charset conversion | ✅ Implemented | Charset conversion of FILE NAMES (not content) at the protocol boundary via iconv(3): `--iconv=LOCAL[,REMOTE]` — the sender converts each local filename LOCAL→REMOTE before transmitting, and the receiver converts each wire filename REMOTE→LOCAL before creating/writing. The full CONVERT_SPEC is serialized into the config frame as a new trailing string field so the peer knows the wire charset; **PROTOCOL_VERSION bumped 2.15.0 → 2.16.0**. `LOCAL[,REMOTE]` parse: single charset ⇒ LOCAL==REMOTE (identity both ways); garbage rejected up front. Validation probes BOTH directions (a spec that only opens one way is refused, as is a NUL-emitting target charset like utf-16/utf-32/ucs-2, since filenames cannot contain NUL). An unrepresentable name (EILSEQ/EINVAL) fails that path cleanly with a logged `--iconv: cannot convert file name ...` and is never written mangled/truncated. Conversion is applied at EVERY wire-path site (regular/MKDIR/hardlink path+target/symlink path+target/SPECIAL, the delete manifest, the incremental-check path, and the `-s`/`chunk_serialize` embedded blob path), on both client and server (`--iconv` is also a server/daemon option). Zero overhead when unset. See the Phase-6 iconv notes below | | `--checksum-seed=NUM` | Set checksum seed | ✅ Implemented | Sets the seed for FastSync's whole-file xxHash64 digest (full 64-bit seed) and for the delta path's per-block xxHash32 strong checksum (low 32 bits of the seed). An explicit seed deterministically changes every computed digest on BOTH endpoints (sender and receiver share the seed via the config frame, protocol 2.10.0), so identical runs with the same seed skip the same files and a changed seed changes the digests — the explicit-seed path that makes xxHash comparisons deterministic. `--checksum-choice=md5` has no seed and ignores it (documented). The value is a strict decimal 0..2⁶⁴-1 (blank, signed, or non-numeric values are rejected). Like rsync, a seed only matters where a digest is actually computed (`--checksum` or a basis-dir run, or a delta transfer); it does not by itself enable `--checksum`/`--delta`. Divergence from rsync: the default is seed 0, and FastSync never randomizes the seed (rsync uses a random per-transfer seed when `--checksum-seed` is unset); FastSync's unset default therefore reproduces its historical byte-for-byte behavior | | `--secluded-args`, `-s` | Use protocol to send args | ⛔ Impossible/Divergence | Accepted for CLI compatibility (including the rsync short `-s`, Phase 7 Wave A) but a documented **no-op / divergence**. rsync's `-s` protects arguments from shell expansion by shipping them over the protocol; FastSync never passes remote arguments through a shell expansion boundary in the first place — its SSH transport builds the remote argv as **single-quote-escaped shell words** (`ssh_build_remote_command`), so the injection/leak that `-s` guards against does not exist and there is nothing to "seclude". Implementing a true arg-send protocol would mean replacing the argv-based SSH launch with an in-band argument channel, a large redesign of the transport that buys no security here. Chunk serialization remains the long-only `--chunk-serialization`. | @@ -795,7 +795,7 @@ These are the hardest compatibility items because they require durable formats o **Phase 6, Wave B (iconv) shipping note (PROTOCOL 2.15.0 → 2.16.0):** `--iconv=LOCAL[,REMOTE]` converts file NAMES at the wire boundary (never content). The full CONVERT_SPEC is serialized into the config frame as a new trailing string field (empty→NULL canonicalized), so both ends share the same wire charset interpretation; this required the PROTOCOL bump because the frame is a strict ordered sequence and a peer that does not parse the new trailing field would desynchronize. Each end derives LOCAL (its own charset) and REMOTE (the wire charset): the sender opens LOCAL→REMOTE and converts every transmitted filename; the receiver opens REMOTE→LOCAL and converts every received filename before creating/writing. Conversion is applied at every wire-path site (regular/MKDIR/hardlink path+target/symlink path+target/SPECIAL, the delete manifest keep/protected/missing entries, the incremental-check path, and the embedded `-s`/chunk-blob path). A name it cannot convert (EILSEQ/EINVAL) is failed cleanly with a logged `--iconv: cannot convert file name ...` and is never written truncated/mangled. Validation probes both directions up front (both the sender local→remote and the receiver remote→local, and, for a server/daemon with its own `--iconv`, the client-REMOTE→server-LOCAL pair) so an unusable spec is rejected before the connection rather than mid-transfer, and NUL-emitting target charsets (utf-16/utf-32/ucs-2) are refused because filenames cannot contain NUL. Divergence documented upstream: the receiver does NOT half-swap; the wire charset always comes from the sender's REMOTE half, so a server whose local charset differs from the client's LOCAL must declare it with its own `--iconv`. Conversion is process-global and runs on a single thread per process (sender thread / receiver-loop thread), initialized before worker threads start and freed after they join. -**Phase 6, Wave C (protocol-version) shipping note (no PROTOCOL_VERSION change):** `--protocol=NUM` lets the client force the wire protocol version for a transfer. FastSync's protocol is a single lockstep format: the config frame is a strict ordered sequence and the server requires the client's version string to equal `PROTOCOL_VERSION` exactly (`config_receive_with_validate`, src/shared/config.c) — there are no older-format code paths and no downgrade/negotiation machinery, so a lower/higher/virtual version can never be spoken. The honest contract is therefore: `--protocol=2.20.0` (the current `PROTOCOL_VERSION`, as of the packed-metadata wave) is accepted and stored into the client's `version` claim (which `config_send` already transmits), and every other value — `2.19.0`, `2.18.0`, `2.18`, `2.17.0`, `2.16.0`, `2.15.0`, `3.0.0`, rsync-integer spellings like `216`/`31`, garbage, empty — is rejected up front in `validate_config()` before any connection, with a clear error that FastSync supports only its current wire protocol and cannot speak an older or virtual one. Implementation is client-only: a server-side `--protocol` is intentionally not added because the server has no negotiation (it only enforces exact match), and it could only ever be the current version. This preserves (and slightly tightens) existing validation: the client now also refuses to launch with a version it cannot actually speak, rather than only the server rejecting it later. A genuine downgrade would require a per-version compatibility layer for every frame/feature added since (append 2.10, preallocate 2.11, hardlinks 2.12, devices/specials/symlink-trust/xattr 2.13, remote-option 2.14, daemon module/auth 2.15, iconv 2.16, dir/symlink times 2.17, privilege flags --super/--copy-as 2.18, SCRAM daemon auth 2.19, packed metadata 2.20) and is intentionally out of scope — documented divergences from rsync's integer-negotiated downgrade remain. +**Phase 6, Wave C (protocol-version) shipping note (no PROTOCOL_VERSION change):** `--protocol=NUM` lets the client force the wire protocol version for a transfer. FastSync's protocol is a single lockstep format: the config frame is a strict ordered sequence and the server requires the client's version string to equal `PROTOCOL_VERSION` exactly (`config_receive_with_validate`, src/shared/config.c) — there are no older-format code paths and no downgrade/negotiation machinery, so a lower/higher/virtual version can never be spoken. The honest contract is therefore: `--protocol=2.21.0` (the current `PROTOCOL_VERSION`, as of the server-contacting dry-run wave) is accepted and stored into the client's `version` claim (which `config_send` already transmits), and every other value — `2.20.0`, `2.19.0`, `2.18.0`, `2.18`, `2.17.0`, `2.16.0`, `2.15.0`, `3.0.0`, rsync-integer spellings like `216`/`31`, garbage, empty — is rejected up front in `validate_config()` before any connection, with a clear error that FastSync supports only its current wire protocol and cannot speak an older or virtual one. Implementation is client-only: a server-side `--protocol` is intentionally not added because the server has no negotiation (it only enforces exact match), and it could only ever be the current version. This preserves (and slightly tightens) existing validation: the client now also refuses to launch with a version it cannot actually speak, rather than only the server rejecting it later. A genuine downgrade would require a per-version compatibility layer for every frame/feature added since (append 2.10, preallocate 2.11, hardlinks 2.12, devices/specials/symlink-trust/xattr 2.13, remote-option 2.14, daemon module/auth 2.15, iconv 2.16, dir/symlink times 2.17, privilege flags --super/--copy-as 2.18, SCRAM daemon auth 2.19, packed metadata 2.20) and is intentionally out of scope — documented divergences from rsync's integer-negotiated downgrade remain. **Phase-1/2 selection-and-update status correction (docs):** `-I/--ignore-times`, `--size-only`, `-@/--modify-window`, `--existing`, `--ignore-existing`, `-u/--update`, `-W/--whole-file`, and `--compress-threads` were previously listed as not-implemented in this document but are in fact fully implemented and tested on `dev`. This pass corrects the matrix to match the code. The realistic model of these is that FastSync is a *sender-driven* whole-tree copy, so the size+mtime quick-check and all three receiver-policy skips (`--existing`, `--ignore-existing`, `-u`) are evaluated against the **destination** on the receiver side, and their booleans cross the wire in the config frame. `-I`/`--size-only`/`--modify-window` modify the `--incremental` per-file `STATUS_CHECK` handshake's match predicate (`-I` disables the mtime leg and forces transfer; `--size-only` drops only the mtime leg; `--modify-window` adds tolerance to `metadata_mtime_matches`); they require `--incremental` (or a basis dir) to have a handshake to affect, mirroring how they only matter where a quick-check exists in rsync. `--existing`/`--ignore-existing`/`-u` are receiver write-time policies (skipping the write / newer-destination guard) applied across the regular-file, `--delay-updates`-staged, hardlink-sibling, and special/device paths; `-u` implies `-M` metadata and uses a second-then-nanosecond strict `>` newer check; both correctly influence `--remove-source-files` (a skipped source is not removed). `-W/--whole-file` disables block-level delta (opt-in via `--delta`), folded into the wire `use_delta` so no protocol bump was needed, and makes `--fuzzy` inert; `--append`/`--append-verify` are rejected with `-W`. `--compress-threads=NUM` (1..64, client-only, never crosses the wire) sizes the zstd compression worker pool. No code was changed by this correction; the implementation had landed in earlier merge waves (feat/ignore-times, feat/ignore-existing via the newer `file_to_disk_secure_no_replace`/`linkat EEXIST` path, feat/size-only, feat/modify-window, feat/whole-file, feat/update, compression-threads). diff --git a/tests/integration/test_daemon.py b/tests/integration/test_daemon.py index a7870e9..00be064 100644 --- a/tests/integration/test_daemon.py +++ b/tests/integration/test_daemon.py @@ -43,6 +43,9 @@ from common import ( _find_free_port, _wait_for_port, ) +# The dry-run no-mutation contract is asserted with the same structural snapshot +# (mode/inode/mtime/xattr/content) the feature suite uses. +from test_features import _snapshot_tree SOURCE_DIR = os.path.join(TEST_DATA_DIR, "daemon_source") MODULE_ROOT = os.path.join(TEST_DATA_DIR, "daemon_modules") @@ -355,6 +358,33 @@ class TestDaemonRejection: assert result.returncode != 0 assert self._tree_files() == before, "read-only rejection wrote under the module root" + @pytest.mark.ci + def test_read_only_module_allows_dry_run(self, daemon): + """A server-contacting --dry-run IS a read-only wire operation, so a + `read only = yes` module is the safest dry-run target and must accept it + while writing nothing.""" + result, _ = run_client(SOURCE_DIR, "127.0.0.1::readonly", flags=["--dry-run"], + port=daemon.port) + assert result.returncode == 0, (result.stderr or result.stdout)[:300] + assert "Dry run:" in result.stdout, result.stdout[:200] + assert _tree_file_count(READONLY_MODULE) == 0, "read-only dry-run wrote a file" + + @pytest.mark.ci + def test_module_dry_run_mutates_nothing(self, daemon): + """A daemon-module dry-run reports would-transfer entries but leaves the + module tree structurally identical (mode/inode/mtime/xattr/content).""" + result = _push("127.0.0.1::files", daemon.port) + assert result.returncode == 0, result.stderr or result.stdout + before = _snapshot_tree(FILES_MODULE) + # --ignore-times forces every regular file to be reported as + # would-transfer, so the dry-run exercises the receiver decision rather + # than an all-skip shortcut -- while still mutating nothing. + result, _ = run_client(SOURCE_DIR, "127.0.0.1::files", + flags=["--dry-run", "--ignore-times"], port=daemon.port) + assert result.returncode == 0, (result.stderr or result.stdout)[:300] + assert "Dry run:" in result.stdout, result.stdout[:200] + assert _snapshot_tree(FILES_MODULE) == before, "daemon dry-run mutated the module root" + def test_unknown_module_rejected(self, daemon): result = _push("127.0.0.1::no-such-module", daemon.port) assert result.returncode != 0 diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index bf72dbc..96372cb 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -370,26 +370,55 @@ class TestDryRun: assert not mismatches, f"Mismatch: {mismatches}" -def _snapshot_tree(root): - """Return {relpath: (size, mtime_ns, content_bytes)} for a directory tree. +def _snapshot_xattrs(path): + """Return a stable, comparable tuple of (name, value) xattr pairs. - Used to prove a dry-run left the destination byte-for-byte and - timestamp-for-timestamp unchanged. Returns an empty dict for a missing - root so "nothing was created" is also observable.""" + Returns None when the platform/filesystem does not expose xattrs so both + snapshots agree on "unavailable" instead of one being treated as changed.""" + try: + names = os.listxattr(path, follow_symlinks=False) + except (AttributeError, OSError): + return None + if not names: + return () + pairs = [] + for name in sorted(names): + try: + value = os.getxattr(path, name, follow_symlinks=False) + except OSError: + value = None + pairs.append((name, value)) + return tuple(pairs) + + +def _snapshot_tree(root): + """Return a structural snapshot of a directory tree. + + Every entry (including directories) is recorded as + (inode, mtime_ns, mode, xattrs, kind-specific payload) so a dry-run that + touched a mode, inode, mtime, xattr, or content is observable. Regular + files carry their size+bytes, symlinks their target, and special entries + (FIFO/socket/device) their size only -- opening a special file could block. + Returns an empty dict for a missing root so "nothing was created" is also + observable.""" snapshot = {} if not os.path.exists(root): return snapshot - for dirpath, _dirnames, filenames in os.walk(root): - for name in filenames: + for dirpath, dirnames, filenames in os.walk(root): + for name in list(dirnames) + filenames: path = os.path.join(dirpath, name) rel = os.path.relpath(path, root) st = os.lstat(path) + entry = [st.st_ino, st.st_mtime_ns, stat.S_IMODE(st.st_mode), _snapshot_xattrs(path)] if stat.S_ISLNK(st.st_mode): - snapshot[rel] = ("symlink", os.readlink(path), st.st_mtime_ns) - continue - with open(path, "rb") as fh: - data = fh.read() - snapshot[rel] = (st.st_size, st.st_mtime_ns, data) + entry.append(("symlink", os.readlink(path))) + elif stat.S_ISREG(st.st_mode): + with open(path, "rb") as fh: + data = fh.read() + entry += [st.st_size, data] + else: + entry.append(st.st_size) + snapshot[rel] = tuple(entry) return snapshot @@ -461,6 +490,10 @@ class TestRemoteDryRun: @pytest.mark.ci def test_remote_dry_run_mkpath_does_not_create_root(self, shared_server): + """A wire dry_run cannot make --mkpath create anything, and it cannot + relax the precondition either: a nonexistent root is rejected (a real + run without the created root is impossible in dry-run) while nothing is + created.""" source = os.path.join(TEST_DATA_DIR, "remote_dry_mk_src") dest = os.path.join(TEST_DATA_DIR, "remote_dry_mk_dst") self._seed(source) @@ -469,8 +502,7 @@ class TestRemoteDryRun: result, _ = run_client(source, dest, flags=["--dry-run", "--mkpath"], port=shared_server.port) - assert result.returncode == 0, f"exit {result.returncode}: {result.stderr[:300]}" - assert "changed.txt" in result.stdout + assert result.returncode != 0, "dry-run --mkpath accepted a nonexistent receive root" assert not os.path.exists(dest), "dry-run --mkpath created the destination root" @pytest.mark.ci @@ -534,6 +566,156 @@ class TestRemoteDryRun: with open(os.path.join(received, "changed.txt"), "rb") as f: assert f.read() == b"updated payload for the real transfer\n" + @pytest.mark.ci + def test_remote_dry_run_delay_updates_mutates_nothing(self, shared_server): + """--delay-updates stages under the receive root; a dry-run must neither + create that staging tree nor publish anything (mode/inode/mtime intact).""" + source = os.path.join(TEST_DATA_DIR, "remote_dry_delay_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_delay_dst") + self._seed(source) + clean_dir(dest) + result, _ = run_client(source, dest, flags=["--delay-updates"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + + with open(os.path.join(source, "changed.txt"), "wb") as f: + f.write(b"changed for delay-updates dry-run\n") + before = _snapshot_tree(dest) + result, _ = run_client(source, dest, flags=["--dry-run", "--delay-updates"], + port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert "changed.txt" in result.stdout, result.stdout + assert _snapshot_tree(dest) == before, "delay-updates dry-run mutated the destination" + + @pytest.mark.ci + def test_remote_dry_run_backup_mutates_nothing(self, shared_server): + """--backup would rename the old file aside; a dry-run must not.""" + source = os.path.join(TEST_DATA_DIR, "remote_dry_backup_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_backup_dst") + self._seed(source) + clean_dir(dest) + result, _ = run_client(source, dest, port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + + with open(os.path.join(source, "changed.txt"), "wb") as f: + f.write(b"changed for backup dry-run\n") + before = _snapshot_tree(dest) + result, _ = run_client(source, dest, flags=["--dry-run", "--backup"], + port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert "changed.txt" in result.stdout, result.stdout + assert _snapshot_tree(dest) == before, "--backup dry-run mutated the destination" + + @pytest.mark.ci + def test_remote_dry_run_symlink_mutates_nothing(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_symlink_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_symlink_dst") + self._seed(source) + os.symlink("changed.txt", os.path.join(source, "link")) + clean_dir(dest) + result, _ = run_client(source, dest, flags=["-a"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + received = get_dest_received_dir(dest, source) + assert os.path.islink(os.path.join(received, "link")) + + # Re-point the source link so the entry is genuinely stale, then prove a + # dry-run leaves the destination link target, inode, and mtime untouched. + os.unlink(os.path.join(source, "link")) + os.symlink("keep.txt", os.path.join(source, "link")) + before = _snapshot_tree(dest) + result, _ = run_client(source, dest, flags=["-a", "--dry-run"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert _snapshot_tree(dest) == before, "symlink dry-run mutated the destination" + assert os.readlink(os.path.join(received, "link")) == "changed.txt" + + @pytest.mark.ci + def test_remote_dry_run_hardlink_mutates_nothing(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_hardlink_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_hardlink_dst") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "h1.txt"), "wb") as f: + f.write(b"hardlinked payload\n") + os.link(os.path.join(source, "h1.txt"), os.path.join(source, "h2.txt")) + result, _ = run_client(source, dest, flags=["-H"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + received = get_dest_received_dir(dest, source) + assert os.stat(os.path.join(received, "h1.txt")).st_ino == \ + os.stat(os.path.join(received, "h2.txt")).st_ino + + # Change the shared inode; both names are now stale in the destination. + with open(os.path.join(source, "h1.txt"), "wb") as f: + f.write(b"changed hardlinked payload\n") + before = _snapshot_tree(dest) + result, _ = run_client(source, dest, flags=["-H", "--dry-run"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert _snapshot_tree(dest) == before, "hardlink dry-run mutated the destination" + + @pytest.mark.ci + def test_remote_dry_run_fifo_special_mutates_nothing(self, shared_server): + source = os.path.join(TEST_DATA_DIR, "remote_dry_fifo_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_fifo_dst") + clean_dir(source) + clean_dir(dest) + with open(os.path.join(source, "plain.txt"), "wb") as f: + f.write(b"plain\n") + os.mkfifo(os.path.join(source, "existing.fifo")) + result, _ = run_client(source, dest, flags=["--specials"], port=shared_server.port) + assert result.returncode == 0, result.stderr[:200] + received = get_dest_received_dir(dest, source) + assert stat.S_ISFIFO(os.lstat(os.path.join(received, "existing.fifo")).st_mode) + + os.mkfifo(os.path.join(source, "new.fifo")) + before = _snapshot_tree(dest) + result, _ = run_client(source, dest, flags=["--specials", "--dry-run"], + port=shared_server.port) + assert result.returncode == 0, result.stderr[:300] + assert not os.path.exists(os.path.join(received, "new.fifo")), \ + "dry-run created a FIFO on the receiver" + assert _snapshot_tree(dest) == before, "special-node dry-run mutated the destination" + + @pytest.mark.ci + def test_read_batch_with_dry_run_is_refused(self, shared_server): + """A dry-run of a local batch apply is meaningless (and must not become a + mutation escape hatch): the CLI rejects the combination up front.""" + source = os.path.join(TEST_DATA_DIR, "remote_dry_batch_src") + dest = os.path.join(TEST_DATA_DIR, "remote_dry_batch_dst") + self._seed(source) + clean_dir(dest) + result, _ = run_client(source, dest, flags=["--read-batch=/nonexistent.batch", "--dry-run"], + port=shared_server.port) + assert result.returncode != 0, "read-batch + dry-run was accepted" + combined = (result.stderr or "") + (result.stdout or "") + assert "cannot be combined" in combined or "--dry-run" in combined, combined[:300] + + @pytest.mark.ci + def test_remote_dry_run_bad_root_fails_like_real_run(self, shared_server): + """A wire dry_run must not relax the destination-root precondition: a + missing or non-directory root that fails a real run fails a dry-run too, + and the dry-run must not create/replace anything.""" + source = os.path.join(TEST_DATA_DIR, "remote_dry_badroot_src") + self._seed(source) + + missing = os.path.join(TEST_DATA_DIR, "remote_dry_badroot_missing") + shutil.rmtree(missing, ignore_errors=True) + real, _ = run_client(source, missing, port=shared_server.port) + assert real.returncode != 0, "real run accepted a missing receive root" + assert not os.path.exists(missing), "real run created the missing root" + dry, _ = run_client(source, missing, flags=["--dry-run"], port=shared_server.port) + assert dry.returncode != 0, "dry-run accepted a missing receive root a real run rejects" + assert not os.path.exists(missing), "dry-run created the missing receive root" + + fileroot = os.path.join(TEST_DATA_DIR, "remote_dry_badroot_file") + shutil.rmtree(fileroot, ignore_errors=True) + with open(fileroot, "wb") as f: + f.write(b"i am a regular file, not a directory\n") + real, _ = run_client(source, fileroot, port=shared_server.port) + assert real.returncode != 0, "real run accepted a regular-file receive root" + dry, _ = run_client(source, fileroot, flags=["--dry-run"], port=shared_server.port) + assert dry.returncode != 0, "dry-run accepted a regular-file receive root a real run rejects" + with open(fileroot, "rb") as f: + assert f.read() == b"i am a regular file, not a directory\n", \ + "dry-run clobbered a regular-file receive root" + class TestRemoveSourceFiles: def test_removes_only_transferred_regular_files(self, shared_server): diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index ca0a990..538e2ff 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -590,6 +590,9 @@ static void test_parse_args_port_alias() { int positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 5, argv_space, positional_args, &positional_count), 0); EXPECT_EQ_INT(cfg->server_port, 9000); + /* The default port is 8080; the explicit bit is what lets --dry-run tell an + explicit remote target from the default and route to the server. */ + EXPECT_TRUE(cfg->server_port_set); config_delete(cfg); cfg = config_create(); @@ -597,6 +600,7 @@ static void test_parse_args_port_alias() { positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 4, argv_inline, positional_args, &positional_count), 0); EXPECT_EQ_INT(cfg->server_port, 9001); + EXPECT_TRUE(cfg->server_port_set); config_delete(cfg); cfg = config_create(); @@ -604,6 +608,29 @@ static void test_parse_args_port_alias() { positional_count = 0; EXPECT_EQ_INT(parse_args(cfg, 4, argv_long, positional_args, &positional_count), 0); EXPECT_EQ_INT(cfg->server_port, 9002); + EXPECT_TRUE(cfg->server_port_set); + config_delete(cfg); +} + +/* An explicit --server-host must set its own routing bit (the field itself + * defaults to 127.0.0.1, so a value check cannot distinguish an explicit host + * from the default); --dry-run uses it to route to the server. */ +static void test_parse_args_server_host_sets_routing_bit() { + Config* cfg = config_create(); + int positional_args[2]; + int positional_count = 0; + EXPECT_FALSE(cfg->server_host_set); + char* argv_space[] = {"fastsync", "--server-host", "example.test", "/src", "/dst"}; + EXPECT_EQ_INT(parse_args(cfg, 5, argv_space, positional_args, &positional_count), 0); + EXPECT_EQ_STR(cfg->server_host, "example.test"); + EXPECT_TRUE(cfg->server_host_set); + config_delete(cfg); + + cfg = config_create(); + char* argv_inline[] = {"fastsync", "--server-host=example.test", "/src", "/dst"}; + positional_count = 0; + EXPECT_EQ_INT(parse_args(cfg, 4, argv_inline, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->server_host_set); config_delete(cfg); } @@ -3301,6 +3328,7 @@ void test_client_cli() { test_parse_args_non_numeric_port(); test_parse_args_invalid_server_port(); test_parse_args_port_alias(); + test_parse_args_server_host_sets_routing_bit(); test_parse_args_threads(); test_client_abort_flag(); test_parse_args_invalid_compression_level(); diff --git a/tests/test_config.c b/tests/test_config.c index 80e1198..d9a442a 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -2399,7 +2399,7 @@ static void golden_config_populate(Config* c) { c->modify_window = 3; c->compress_choice = str_dup("zstd"); /* "u=rwx,go=rx" is the same 11 bytes as the original "u=rwX,go=rX" (so the - * frame stays 633 bytes) but X is not in FastSync's chmod grammar, and the + * frame stays 637 bytes) but X is not in FastSync's chmod grammar, and the * receive-side golden validates the frame. */ c->chmod_spec = str_dup("u=rwx,go=rx"); c->skip_compress_set = true; @@ -2531,7 +2531,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.20.0). The expected hash +/* Byte-for-byte wire compatibility guard (protocol 2.21.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())