diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 27eb93f..43136af 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -2113,7 +2113,8 @@ int main(int argc, char* argv[]) { to nor transfers to a server. --write-batch runs the normal live transfer AND then emits the batch FILE from a separate deterministic scan pass. It drives the single-threaded transfer so the config outlives the run for that - second pass (the -m path takes ownership of the config). */ + second pass (main retains ownership of the config; every send path + borrows it). */ if (config->read_batch) { exit_code = apply_batch_to_dest(config, config->read_batch, config->receive_root_directory); goto cleanup; diff --git a/src/client/client_send.c b/src/client/client_send.c index bfec60a..e026e53 100644 --- a/src/client/client_send.c +++ b/src/client/client_send.c @@ -44,6 +44,10 @@ receiver's RECEIVER_QUEUE_MAX_BYTES). */ #define SENDER_QUEUE_MAX_BYTES (MAX_CONNECTION_MEMORY - 2 * MAX_CHUNK_SIZE) +/* One mebibyte in bytes; the unit used by the --stats/--progress lines. + Always cast to double when dividing so the output stays fractional. */ +#define BYTES_PER_MIB (1024ULL * 1024ULL) + /* Forward declaration for progress-reporting thread used in multithreaded send. */ static int progress_thread_fn(void* arg); @@ -51,10 +55,32 @@ static const char* display_bytes(unsigned long long bytes, bool human_readable, size_t buffer_size) { if (human_readable && format_human_bytes(bytes, buffer, buffer_size)) return buffer; - snprintf(buffer, buffer_size, "%.1f MB", bytes / 1048576.0); + snprintf(buffer, buffer_size, "%.1f MB", (double)bytes / (double)BYTES_PER_MIB); return buffer; } +/* Print the canonical `--stats` line. Shared by the single-threaded and + multithreaded send paths so both honor --stats, --human-readable and --quiet + identically; `start` marks the beginning of the transfer for the rate. */ +static void report_transfer_stats(const Config* config, int total_files, + unsigned long long total_bytes, time_t start) { + if (!config->stats || config->quiet) + return; + double elapsed = difftime(time(NULL), start); + double rate = elapsed > 0.0 ? (double)total_bytes / ((double)BYTES_PER_MIB * elapsed) : 0.0; + if (config->human_readable) { + char total_buffer[32]; + char rate_buffer[32]; + fprintf(stderr, "Stats: %d files, %s, %s/s\n", total_files, + display_bytes(total_bytes, true, total_buffer, sizeof(total_buffer)), + display_bytes((unsigned long long)(rate * (double)BYTES_PER_MIB), true, rate_buffer, + sizeof(rate_buffer))); + } else { + fprintf(stderr, "Stats: %d files, %.1f MB, %.1f MB/s\n", total_files, + (double)total_bytes / (double)BYTES_PER_MIB, rate); + } +} + /* Compiled scanner inputs that are shared read-only across scanner instances * and, in -m mode, across worker threads. `base_filters` owns the compiled * command-line + -C rules; the FileListSet allow-set lives in the Config. @@ -688,7 +714,7 @@ static int send_dry_run_manifest(const Config* config) { 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, total_bytes / 1048576.0); + printf("Total: %d files, %.1f MB\n", file_count, (double)total_bytes / (double)BYTES_PER_MIB); } return 0; } @@ -1425,12 +1451,9 @@ static int send_chunk_with_removal(Client* client, Chunk* chunk, Config* config, return 0; } -int send_chunk(Client* client, Chunk* chunk, Config* config) { - return send_chunk_with_removal(client, chunk, config, NULL); -} - static int send_chunks_multithreaded(void* pipeline_context) { PipelineContextSender* context = (PipelineContextSender*)pipeline_context; + time_t start = time(NULL); Client* client = connect_transfer_client(context->config); if (!client) { if (context->config->transport == TRANSPORT_TCP) @@ -1578,10 +1601,9 @@ static int send_chunks_multithreaded(void* pipeline_context) { int total_files = context->total_files; unsigned long long total_bytes = context->total_bytes; mtx_unlock(&context->mutex_progress); - if (context->config->stats) - fprintf(stderr, "Stats: %d files, %.1f MB\n", total_files, total_bytes / 1048576.0); + report_transfer_stats(context->config, total_files, total_bytes, start); log_info_message(LOG_INFO_STATS, "Transfer summary: %d files, %.1f MB", total_files, - total_bytes / 1048576.0); + (double)total_bytes / (double)BYTES_PER_MIB); disconnect_transfer_client(client); mark_sender_done(context); protocol_session_unbind(); @@ -1754,17 +1776,18 @@ static int load_files_multithreaded(void* pipeline_context) { static void print_transfer_progress(unsigned long long total_bytes, time_t start, const char* suffix, bool human_readable) { double elapsed = difftime(time(NULL), start); - double rate = elapsed > 0.0 ? total_bytes / (1048576.0 * elapsed) : 0.0; + double rate = elapsed > 0.0 ? (double)total_bytes / ((double)BYTES_PER_MIB * elapsed) : 0.0; if (human_readable) { char total_buffer[32]; char rate_buffer[32]; fprintf(stderr, "\rSent %s (%s/s) %s", display_bytes(total_bytes, true, total_buffer, sizeof(total_buffer)), - display_bytes((unsigned long long)(rate * 1048576.0), true, rate_buffer, + display_bytes((unsigned long long)(rate * (double)BYTES_PER_MIB), true, rate_buffer, sizeof(rate_buffer)), suffix); } else { - fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) %s", total_bytes / 1048576.0, rate, suffix); + fprintf(stderr, "\rSent %.1f MB (%.1f MB/s) %s", (double)total_bytes / (double)BYTES_PER_MIB, + rate, suffix); } fflush(stderr); } @@ -2132,23 +2155,9 @@ int send_files(Config* config) { remove_transferred_sources(config, remove_sources); if (config->show_progress && !config->quiet) print_transfer_progress(total_bytes, start, "Done.\n", config->human_readable); - if (config->stats && !config->quiet) { - double elapsed_total = difftime(time(NULL), start); - double rate = elapsed_total > 0 ? total_bytes / (1048576.0 * elapsed_total) : 0; - if (config->human_readable) { - char total_buffer[32]; - char rate_buffer[32]; - fprintf(stderr, "Stats: %d files, %s, %s/s\n", total_files, - display_bytes(total_bytes, true, total_buffer, sizeof(total_buffer)), - display_bytes((unsigned long long)(rate * 1048576.0), true, rate_buffer, - sizeof(rate_buffer))); - } else { - fprintf(stderr, "Stats: %d files, %.1f MB, %.1f MB/s\n", total_files, total_bytes / 1048576.0, - rate); - } - } + report_transfer_stats(config, total_files, total_bytes, start); log_info_message(LOG_INFO_STATS, "Transfer summary: %d files, %.1f MB", total_files, - total_bytes / 1048576.0); + (double)total_bytes / (double)BYTES_PER_MIB); /* --ignore-errors: an unreadable source directory was skipped but the run still completed (and deleted); report the run as errored like rsync does. */ ret = (ok && !had_scan_io) ? 0 : 1; @@ -2237,7 +2246,7 @@ int send_files_multithreaded(Config** config_ptr) { context->missing_args = missing_args; missing_args = NULL; /* owned by the context from here on */ pipeline_context_sender_set_queue_byte_limit(context, SENDER_QUEUE_MAX_BYTES); - *config_ptr = NULL; /* context now owns config through all remaining paths */ + /* The context borrows `config`; the caller (main) still owns and frees it. */ struct timespec now_mono; if (clock_gettime(CLOCK_MONOTONIC, &now_mono) != 0) { now_mono.tv_sec = 0; diff --git a/src/client/client_send.h b/src/client/client_send.h index 0b12973..8c85106 100644 --- a/src/client/client_send.h +++ b/src/client/client_send.h @@ -5,9 +5,10 @@ #include "config.h" #include "transport_tcp.h" -int send_chunk(Client* client, Chunk* chunk, Config* config); +/* Both sender entry points BORROW `config` for the duration of the call; they + * never free it, and the caller retains ownership (freeing it with + * config_delete() once the call returns). */ int send_files(Config* config); -/* Takes ownership only when *config is set to NULL on return. */ int send_files_multithreaded(Config** config); /* Phase 6 residual-batch (client-only). See client_send.c. */ int write_batch_from_source(const Config* config, const char* batch_path); diff --git a/src/client/client_validation.c b/src/client/client_validation.c index f6a8ad0..f19886b 100644 --- a/src/client/client_validation.c +++ b/src/client/client_validation.c @@ -1,6 +1,4 @@ #include "client_validation.h" -#include "charset.h" -#include "delay_updates.h" #include "log.h" #include "usage.h" #include "utils.h" @@ -41,17 +39,6 @@ bool validate_config(const Config* config) { print_usage(); return false; } - if (config_has_basis(config) && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, - "--compare-dest/--copy-dest/--link-dest require per-file incremental checks and " - "cannot be combined with -s (chunk serialization)"); - return false; - } - if (config->use_sendfile && (config->use_chunk_serialization || config->use_compression)) { - log_message(LOG_LEVEL_ERROR, "-f/--sendfile cannot be combined with -c (compression) or -s " - "(chunk serialization)"); - return false; - } if (config->compression_threads > 0 && !config->use_compression) { log_message(LOG_LEVEL_ERROR, "--compress-threads requires compression (-c or -z)"); return false; @@ -60,73 +47,11 @@ bool validate_config(const Config* config) { log_message(LOG_LEVEL_ERROR, "-f/--sendfile is not supported with SSH transport"); return false; } - if (config->use_incremental && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, "--incremental is not supported with -s (chunk serialization)"); - return false; - } /* -4 and -6 are mutually exclusive: a socket address family cannot be both. */ if (config->ipv4 && config->ipv6) { log_message(LOG_LEVEL_ERROR, "-4/--ipv4 and -6/--ipv6 are mutually exclusive"); return false; } - if (config->skip_compress_set && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, - "--skip-compress cannot be combined with -s (chunk serialization)"); - return false; - } - if (config->use_delta && !config->whole_file && !config->use_incremental) { - log_message(LOG_LEVEL_ERROR, "--delta requires --incremental"); - return false; - } - if (config->use_delta && !config->whole_file && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, "--delta cannot be combined with -s (chunk serialization)"); - return false; - } - if (config->use_delta && !config->whole_file && config->use_sendfile) { - log_message(LOG_LEVEL_ERROR, "--delta cannot be combined with -f (sendfile)"); - return false; - } - /* --append / --append-verify resume a shorter existing destination by - transmitting only the tail. The resume needs the per-file STATUS_CHECK - handshake (so the dest length is learned), which chunk serialization -s - disables; and whole-file is the opposite intent (send everything), so the - two would silently make the resume pointless. Both are rejected up front - rather than silently degrading to a full transfer. */ - if ((config->append || config->append_verify) && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, - "--append/--append-verify require the per-file incremental check and cannot be " - "combined with -s (chunk serialization)"); - return false; - } - if ((config->append || config->append_verify) && config->whole_file) { - log_message(LOG_LEVEL_ERROR, - "--append/--append-verify are incompatible with --whole-file (which forces a " - "full transfer)"); - return false; - } - /* --hard-links/-H transmits each later group member as a dedicated per-file - STATUS_HARDLINK frame, which chunk serialization -s does not support; and a - hard-links sibling carries no payload, so the tail-resume of --append is - meaningless for it. Both combinations are rejected up front rather than - silently degrading. */ - if (config->preserve_hard_links && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, - "--hard-links/-H cannot be combined with -s (chunk serialization)"); - return false; - } - /* -X/-A ride the per-file metadata frame; the buffer-based chunk-serialization - wire format does not carry the xattr block, so the pair is rejected up front - (mirroring -H + -s) rather than silently dropping attributes. */ - if ((config->preserve_xattrs || config->preserve_acls) && config->use_chunk_serialization) { - log_message(LOG_LEVEL_ERROR, - "--xattrs/-X and --acls/-A cannot be combined with -s (chunk serialization)"); - return false; - } - if (config->preserve_hard_links && (config->append || config->append_verify)) { - log_message(LOG_LEVEL_ERROR, - "--hard-links/-H cannot be combined with --append/--append-verify"); - return false; - } if (config->log_file_format && !config->log_file) { log_message(LOG_LEVEL_ERROR, "--log-file-format requires --log-file"); return false; @@ -146,28 +71,12 @@ bool validate_config(const Config* config) { log_message(LOG_LEVEL_ERROR, "sending daemon credentials to a non-local server requires --tls"); return false; } - if (config->delay_updates && config->inplace) { - log_message(LOG_LEVEL_ERROR, "--delay-updates does not work with --inplace"); - return false; - } - if (config->delay_updates && delay_updates_staging_name_conflict(config->backup_dir)) { - log_message(LOG_LEVEL_ERROR, - "--backup-dir is reserved when --delay-updates is active (used for the internal " - "staging directory)"); - return false; - } - if (!config_has_valid_delete_timing(config)) { - log_message(LOG_LEVEL_ERROR, - "--delete-before/--delete-during/--delete-delay/--delete-after select the delete " - "timing; at most one may be given and each implies --delete"); - return false; - } - /* --iconv: reject a malformed CONVERT_SPEC or an unsupported charset name at - startup (a probe iconv_open is attempted), so a typo'd charset never fails - the run mid-transfer with per-file errors. */ - if (!charset_spec_valid(config->iconv_spec)) { - log_message(LOG_LEVEL_ERROR, - "--iconv requires LOCAL[,REMOTE] charset names supported by iconv"); + /* Every cross-field invariant the receiver enforces lives in one shared + predicate so the client and the server can never disagree. The client + reports the specific reason here, before any network I/O. */ + const char* invariants_error = config_invariants_error(config); + if (invariants_error) { + log_message(LOG_LEVEL_ERROR, "%s", invariants_error); return false; } /* --protocol: FastSync has exactly one wire format, so the forced version @@ -180,15 +89,5 @@ bool validate_config(const Config* config) { PROTOCOL_VERSION); return false; } - /* --copy-as pushes the source ids through the metadata path (it implies - --preserve). A later --no-preserve would clear use_metadata, leaving the - transfer with nothing to chown while the receiver gate would still pass. - Refuse the combination up front rather than silently chowning nothing. */ - if (config->copy_as_set && !config->use_metadata) { - log_message(LOG_LEVEL_ERROR, - "--copy-as requires metadata preservation and cannot be combined with " - "--no-preserve"); - return false; - } return true; } diff --git a/src/client/scanner.c b/src/client/scanner.c index 7db3f58..3a8722d 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -177,7 +177,7 @@ static bool entry_passes_selection(const FileListSet* file_list, const FilterRul /* Best-effort capture of the file's whitelisted xattrs (-X/-A). A failure to * read xattrs is non-fatal: the file is transferred without them. */ static void scanner_capture_xattrs(const DirectoryScanner* scanner, File* file) { - if (!scanner || !file || !(scanner->preserve_xattrs || scanner->preserve_acls)) + if (!scanner || !file || !(scanner->options.preserve_xattrs || scanner->options.preserve_acls)) return; file->xattrs = xattr_capture_path(file->path); } @@ -263,10 +263,10 @@ static bool excluded_sink_append(ArrayList* list, mtx_t* mtx, const char* rel) { how manifest keep entries are stored), so the receiver's walker prefixes match the destination layout. An allocation failure is a fatal scan error. */ static void scanner_record_excluded(DirectoryScanner* scanner, const char* fs_path) { - if (!scanner->excluded_paths || !fs_path) + if (!scanner->options.excluded_paths || !fs_path) return; const char* rel = *fs_path == '/' ? fs_path + 1 : fs_path; - if (!excluded_sink_append(scanner->excluded_paths, scanner->excluded_mutex, rel)) + if (!excluded_sink_append(scanner->options.excluded_paths, scanner->options.excluded_mutex, rel)) scanner->failed = true; } @@ -274,7 +274,7 @@ static void scanner_record_excluded(DirectoryScanner* scanner, const char* fs_pa * context, returning the context used for this directory's entries. On a parse * error the scanner is marked failed. Returns 0 on success, -1 on failure. */ static int open_directory_filter_context(DirectoryScanner* scanner, const FilterNode* inherited) { - if (!scanner->per_dir_filters) { + if (!scanner->options.per_dir_filters) { scanner->current_node = (FilterNode*)inherited; return 0; } @@ -436,6 +436,11 @@ DirectoryScanner* directory_scanner_create_with_options(const char* root_directo DirectoryScanner* scanner = calloc(1, sizeof(DirectoryScanner)); if (scanner == NULL) return NULL; + /* One copy of the scan inputs; normalize chunk_size as the old field-by-field + copy did. */ + scanner->options = *options; + if (scanner->options.chunk_size == 0) + scanner->options.chunk_size = DESIRED_CHUNK_SIZE; scanner->directories = queue_create(100, dir_entry_destroy); if (!scanner->directories) { free(scanner); @@ -443,31 +448,7 @@ DirectoryScanner* directory_scanner_create_with_options(const char* root_directo } scanner->current_dir = NULL; scanner->current_path = NULL; - scanner->use_metadata = options->use_metadata; - scanner->preserve_atimes = options->preserve_atimes; - scanner->preserve_crtimes = options->preserve_crtimes; - scanner->preserve_xattrs = options->preserve_xattrs; - scanner->preserve_acls = options->preserve_acls; - scanner->chunk_size = options->chunk_size > 0 ? options->chunk_size : DESIRED_CHUNK_SIZE; - scanner->exclude_patterns = options->exclude_patterns; - scanner->exclude_count = options->exclude_count; - scanner->include_patterns = options->include_patterns; - scanner->include_count = options->include_count; - scanner->max_size = options->max_size; - scanner->min_size = options->min_size; - scanner->max_depth = options->max_depth; scanner->current_depth = 0; - scanner->follow_symlinks = options->follow_symlinks; - scanner->copy_links = options->copy_links; - scanner->safe_links = options->safe_links; - scanner->copy_unsafe_links = options->copy_unsafe_links; - scanner->copy_dirlinks = options->copy_dirlinks; - scanner->munge_links = options->munge_links; - scanner->checksum = options->checksum; - scanner->one_file_system = options->one_file_system; - scanner->preserve_devices = options->preserve_devices; - scanner->preserve_specials = options->preserve_specials; - scanner->copy_devices = options->copy_devices; scanner->failed = false; scanner->root_path = str_dup(root_directory); if (!scanner->root_path) { @@ -479,28 +460,14 @@ DirectoryScanner* directory_scanner_create_with_options(const char* root_directo scanner->at_seed_dir = true; scanner->seed_node = NULL; scanner->current_node = NULL; - scanner->file_list = options->file_list; - scanner->base_filters = options->base_filters; - scanner->per_dir_filters = options->per_dir_filters; - scanner->excluded_paths = options->excluded_paths; - scanner->excluded_mutex = options->excluded_mutex; - scanner->ignore_io_errors = options->ignore_io_errors; - scanner->ignore_missing_args = options->ignore_missing_args; scanner->io_error = false; - scanner->dirs_mode = options->dirs; scanner->relative_mode = options->relative && options->file_list != NULL; - scanner->hardlinks = options->hardlinks; - scanner->prune_empty_dirs = options->prune_empty_dirs; - scanner->stop_condition = options->stop_condition; - scanner->capture_dir_times = options->capture_dir_times; - scanner->dir_entries = options->dir_entries; - scanner->dir_entries_mutex = options->dir_entries_mutex; scanner->dirs_root_emitted = false; scanner->list_index = 0; scanner->dirs_batch = NULL; scanner->dirs_batch_size = 0; scanner->filter_nodes = NULL; - if (scanner->base_filters || scanner->per_dir_filters) { + if (scanner->options.base_filters || scanner->options.per_dir_filters) { scanner->filter_nodes = array_list_create(filter_node_destroy); if (!scanner->filter_nodes) { free(scanner->root_path); @@ -509,7 +476,7 @@ DirectoryScanner* directory_scanner_create_with_options(const char* root_directo return NULL; } } - if (scanner->one_file_system) { + if (scanner->options.one_file_system) { struct stat root_stats; if (stat(root_directory, &root_stats) != 0) { log_perror("Could not stat source directory"); @@ -715,7 +682,7 @@ static int open_next_directory(DirectoryScanner* scanner) { scanner->current_rel = NULL; free(scanner->current_path); scanner->current_path = NULL; - if (!scanner->ignore_io_errors || is_root_seed) { + if (!scanner->options.ignore_io_errors || is_root_seed) { scanner->failed = true; return -1; } @@ -729,10 +696,11 @@ static int open_next_directory(DirectoryScanner* scanner) { scanner->current_path = NULL; return -1; } - if (scanner->capture_dir_times && - !scanner_capture_dir_time(scanner->dir_entries, scanner->dir_entries_mutex, + if (scanner->options.capture_dir_times && + !scanner_capture_dir_time(scanner->options.dir_entries, scanner->options.dir_entries_mutex, scanner->root_path, scanner->current_path, scanner->relative_mode, - scanner->preserve_atimes, scanner->preserve_crtimes)) { + scanner->options.preserve_atimes, + scanner->options.preserve_crtimes)) { closedir(scanner->current_dir); scanner->current_dir = NULL; free(scanner->current_path); @@ -773,9 +741,9 @@ static File* dirs_root_dir_file(DirectoryScanner* scanner) { return NULL; } file->is_dir = true; - if (scanner->use_metadata) { - file->metadata = file_metadata_create(scanner->root_path, &st, scanner->preserve_atimes, - scanner->preserve_crtimes); + if (scanner->options.use_metadata) { + file->metadata = file_metadata_create(scanner->root_path, &st, scanner->options.preserve_atimes, + scanner->options.preserve_crtimes); if (!file->metadata) { file_destroy(file); scanner->failed = true; @@ -808,7 +776,7 @@ static File* dirs_file_for_entry(DirectoryScanner* scanner, const char* entry) { missing argument and is skipped here, exactly as the recursive scan skips nothing (missing entries never appear there). Without the flags it stays a hard pre-transfer error. */ - if (scanner->ignore_missing_args) { + if (scanner->options.ignore_missing_args) { log_info_message(LOG_INFO_MISC, "skipping missing --files-from entry '%s'", entry); free(abs_path); return NULL; @@ -822,8 +790,8 @@ static File* dirs_file_for_entry(DirectoryScanner* scanner, const char* entry) { if (S_ISLNK(link_stats.st_mode)) { /* A symlink is transferred (following its referent) only when a link resolution option is active, mirroring the regular scanner. */ - bool resolve = scanner->follow_symlinks || scanner->copy_links || scanner->safe_links || - scanner->copy_unsafe_links; + bool resolve = scanner->options.follow_symlinks || scanner->options.copy_links || + scanner->options.safe_links || scanner->options.copy_unsafe_links; if (!resolve || stat(abs_path, &effective) != 0) { free(abs_path); return NULL; @@ -851,9 +819,9 @@ static File* dirs_file_for_entry(DirectoryScanner* scanner, const char* entry) { return NULL; } } - if (scanner->use_metadata) { - file->metadata = file_metadata_create(file->path, &effective, scanner->preserve_atimes, - scanner->preserve_crtimes); + if (scanner->options.use_metadata) { + file->metadata = file_metadata_create(file->path, &effective, scanner->options.preserve_atimes, + scanner->options.preserve_crtimes); if (!file->metadata) { file_destroy(file); scanner->failed = true; @@ -884,18 +852,18 @@ static bool dirs_source_dir_is_empty(const char* path) { /* The next File from the --dirs generator, or NULL when exhausted. */ static File* dirs_next_file(DirectoryScanner* scanner) { - if (!scanner->file_list) { + if (!scanner->options.file_list) { if (scanner->dirs_root_emitted) return NULL; scanner->dirs_root_emitted = true; /* --prune-empty-dirs: a physically empty source directory's explicit entry would only create an empty destination directory, so it is omitted. */ - if (scanner->prune_empty_dirs && dirs_source_dir_is_empty(scanner->root_path)) + if (scanner->options.prune_empty_dirs && dirs_source_dir_is_empty(scanner->root_path)) return NULL; return dirs_root_dir_file(scanner); } - while (scanner->list_index < scanner->file_list->count) { - const char* entry = scanner->file_list->entries[scanner->list_index++]; + while (scanner->list_index < scanner->options.file_list->count) { + const char* entry = scanner->options.file_list->entries[scanner->list_index++]; File* file = dirs_file_for_entry(scanner, entry); if (scanner->failed) return NULL; @@ -922,8 +890,9 @@ static Chunk* dirs_flush_batch(DirectoryScanner* scanner) { } static Chunk* directory_scanner_next_dirs(DirectoryScanner* scanner) { - while (scanner->dirs_batch == NULL || scanner->dirs_batch_size <= scanner->chunk_size) { - if (scanner->stop_condition && stop_condition_reached(scanner->stop_condition)) { + while (scanner->dirs_batch == NULL || scanner->dirs_batch_size <= scanner->options.chunk_size) { + if (scanner->options.stop_condition && + stop_condition_reached(scanner->options.stop_condition)) { Chunk* leftover = dirs_flush_batch(scanner); if (leftover) chunk_destroy(leftover); @@ -962,7 +931,7 @@ static Chunk* directory_scanner_next_dirs(DirectoryScanner* scanner) { } Chunk* directory_scanner_next(DirectoryScanner* scanner) { - if (scanner && scanner->dirs_mode) + if (scanner && scanner->options.dirs) return directory_scanner_next_dirs(scanner); ArrayList* chunk_data = array_list_create(file_destroy); if (!chunk_data) { @@ -972,7 +941,8 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { unsigned long long chunk_data_size = 0; while (1) { - if (scanner->stop_condition && stop_condition_reached(scanner->stop_condition)) { + if (scanner->options.stop_condition && + stop_condition_reached(scanner->options.stop_condition)) { array_list_delete(chunk_data); return NULL; } @@ -996,34 +966,9 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue; - ScannerOptions options = { - .use_metadata = scanner->use_metadata, - .chunk_size = scanner->chunk_size, - .exclude_patterns = scanner->exclude_patterns, - .exclude_count = scanner->exclude_count, - .include_patterns = scanner->include_patterns, - .include_count = scanner->include_count, - .max_size = scanner->max_size, - .min_size = scanner->min_size, - .max_depth = scanner->max_depth, - .num_threads = 0, - .follow_symlinks = scanner->follow_symlinks, - .copy_links = scanner->copy_links, - .safe_links = scanner->safe_links, - .copy_unsafe_links = scanner->copy_unsafe_links, - .copy_dirlinks = scanner->copy_dirlinks, - .munge_links = scanner->munge_links, - .checksum = scanner->checksum, - .one_file_system = scanner->one_file_system, - .file_list = scanner->file_list, - .base_filters = scanner->base_filters, - .per_dir_filters = scanner->per_dir_filters, - .dirs = false, - .relative = false, - }; ScannerEntry inspected; - int inspection = scanner_inspect_entry(&options, scanner->current_path, scanner->current_path, - entry->d_name, &inspected); + int inspection = scanner_inspect_entry(&scanner->options, scanner->current_path, + scanner->current_path, entry->d_name, &inspected); if (inspection < 0) { scanner->failed = true; break; @@ -1055,16 +1000,17 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { scanner->failed = true; break; } - bool passes_selection = - entry_passes_selection(scanner->file_list, scanner->base_filters, scanner->current_node, - rel, entry->d_name, is_dir, scanner->per_dir_filters); + bool passes_selection = entry_passes_selection( + scanner->options.file_list, scanner->options.base_filters, scanner->current_node, rel, + entry->d_name, is_dir, scanner->options.per_dir_filters); if (!passes_selection) { /* --files-from subset pruning is not a filter exclusion: its delete semantics stay keep-set-only (an unlisted source path is treated as absent, so its destination mirror is a deletable extra). A rule-based exclusion is recorded as a protected prefix. -R + --files-from bare wire paths are never recorded (see ScannerOptions.excluded_paths). */ - bool files_from_prune = scanner->file_list && !file_list_affects(scanner->file_list, rel); + bool files_from_prune = + scanner->options.file_list && !file_list_affects(scanner->options.file_list, rel); if (!files_from_prune && !scanner->relative_mode) scanner_record_excluded(scanner, cur_path); } @@ -1085,12 +1031,13 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { if (is_dir) { free(rel_copy); - if (!scanner_same_filesystem(scanner->one_file_system, scanner->root_dev, stats.st_dev)) { + if (!scanner_same_filesystem(scanner->options.one_file_system, scanner->root_dev, + stats.st_dev)) { free(cur_path); continue; } int next_depth = scanner->current_depth + 1; - if (scanner->max_depth <= 0 || next_depth < scanner->max_depth) { + if (scanner->options.max_depth <= 0 || next_depth < scanner->options.max_depth) { DirEntry* de = dir_entry_create(cur_path, next_depth, scanner->current_node); if (!de || !queue_enqueue(scanner->directories, de)) { dir_entry_destroy(de); @@ -1099,7 +1046,8 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { } free(cur_path); } else { - if (scanner->max_depth > 0 && scanner->current_depth + 1 > scanner->max_depth) { + if (scanner->options.max_depth > 0 && + scanner->current_depth + 1 > scanner->options.max_depth) { free(rel_copy); free(cur_path); continue; @@ -1126,13 +1074,14 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { } /* --devices/--specials: a device/FIFO/socket entry marked for preservation becomes a node to recreate (is_special, no data, rdev captured). */ - scanner_prepare_special(scanner->preserve_devices, scanner->preserve_specials, file, &stats); - if (scanner->hardlinks && S_ISREG(stats.st_mode)) - scanner_assign_hardlink(scanner, scanner->hardlinks, file, &stats); - if (scanner->use_metadata) - file->metadata = file_metadata_create(file->path, &stats, scanner->preserve_atimes, - scanner->preserve_crtimes); - if (scanner->use_metadata && !file->metadata) { + scanner_prepare_special(scanner->options.preserve_devices, scanner->options.preserve_specials, + file, &stats); + if (scanner->options.hardlinks && S_ISREG(stats.st_mode)) + scanner_assign_hardlink(scanner, scanner->options.hardlinks, file, &stats); + if (scanner->options.use_metadata) + file->metadata = file_metadata_create(file->path, &stats, scanner->options.preserve_atimes, + scanner->options.preserve_crtimes); + if (scanner->options.use_metadata && !file->metadata) { free(rel_copy); file_destroy(file); scanner->failed = true; @@ -1147,7 +1096,7 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { break; } chunk_data_size += file->data->size; - if (chunk_data_size > scanner->chunk_size) { + if (chunk_data_size > scanner->options.chunk_size) { free(rel_copy); Chunk* result = chunk_data_to_chunk(chunk_data); if (!result) @@ -1211,7 +1160,7 @@ static int parallel_worker_thread(void* arg) { free(ds->root_path); ds->root_path = str_dup(wa->root_dir); ds->seed_node = wa->ps->root_filter_node; - ds->excluded_mutex = &wa->ps->result_mutex; + ds->options.excluded_mutex = &wa->ps->result_mutex; Chunk* chunk; while ((chunk = directory_scanner_next(ds)) != NULL) { if (!queue_enqueue_multithreaded_cancel(wa->ps->result_queue, chunk, &wa->ps->result_mutex, diff --git a/src/client/scanner.h b/src/client/scanner.h index 950a14e..3db1ce9 100644 --- a/src/client/scanner.h +++ b/src/client/scanner.h @@ -117,37 +117,16 @@ typedef struct { typedef struct FilterNode FilterNode; typedef struct { + /* Scan inputs, copied once at create time. Everything that is also a + ScannerOptions field lives here (with the normalized chunk_size); only + scanner-owned bookkeeping stays as direct members below. */ + ScannerOptions options; Queue* directories; DIR* current_dir; char* current_path; - bool use_metadata; - bool preserve_atimes; - bool preserve_crtimes; - bool preserve_xattrs; - bool preserve_acls; - unsigned long long chunk_size; - char** exclude_patterns; - int exclude_count; - char** include_patterns; - int include_count; - unsigned long long max_size; - unsigned long long min_size; - int max_depth; int current_depth; - bool follow_symlinks; - bool copy_links; - bool safe_links; - bool copy_unsafe_links; - bool copy_dirlinks; - bool munge_links; - bool checksum; - bool one_file_system; dev_t root_dev; bool failed; - /* Phase 4 special/devices (see ScannerOptions). */ - bool preserve_devices; - bool preserve_specials; - bool copy_devices; /* Phase 2 (files-from / filter layer). */ char* root_path; /* transfer root (fs path) for rel computation */ char* current_rel; /* rel path of the open directory ("" == root) */ @@ -155,40 +134,17 @@ typedef struct { FilterNode* seed_node; /* inherited context of the seed dir, or NULL */ FilterNode* current_node; /* filter context of the open directory */ ArrayList* filter_nodes; /* owned FilterNode arena (may be NULL) */ - const FileListSet* file_list; - const FilterRuleList* base_filters; - bool per_dir_filters; - /* --dirs / -R state for the directory-entry generator (dirs_mode replaces + /* --dirs / -R state for the directory-entry generator (options.dirs replaces the recursive scan). */ - bool dirs_mode; bool relative_mode; /* file_list && relative: send bare relative wire paths */ - bool prune_empty_dirs; bool dirs_root_emitted; int list_index; ArrayList* dirs_batch; /* owned when non-NULL */ unsigned long long dirs_batch_size; - /* Excluded-path sink (see ScannerOptions). `excluded_mutex` is shared across - parallel worker threads. */ - ArrayList* excluded_paths; - mtx_t* excluded_mutex; - /* --ignore-errors: continue past unreadable directories (records io_error). */ - bool ignore_io_errors; - /* --ignore-missing-args: --dirs listed-but-missing entries are skipped, not - fatal (see ScannerOptions.ignore_missing_args). */ - bool ignore_missing_args; /* A directory could not be opened (I/O error, e.g. EACCES). With --ignore-errors the scan continues past it and the caller decides what to do; `failed` is reserved for fatal errors that always abort the scan. */ bool io_error; - /* --hard-links (-H): shared link-group detection table (see ScannerOptions). - NULL when -H is off. */ - HardLinkTable* hardlinks; - /* Phase 6: sender stop deadline (from ScannerOptions). */ - const StopCondition* stop_condition; - /* P7 Wave D directory-time capture (see ScannerOptions). */ - bool capture_dir_times; - ArrayList* dir_entries; - mtx_t* dir_entries_mutex; } DirectoryScanner; typedef struct { diff --git a/src/shared/chunk.c b/src/shared/chunk.c index c5baeb4..1f3be3a 100644 --- a/src/shared/chunk.c +++ b/src/shared/chunk.c @@ -207,17 +207,20 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { return NULL; char* data_pointer = data->data; size_t remaining_size = data->size; + /* The element currently being parsed is owned by `files` only after the + * array_list_add() at the end of the iteration; until then the error + * epilogue destroys it directly. Keeping this one pointer nulled after the + * hand-off makes the single cleanup path correct for every failure. */ + File* file = NULL; while (remaining_size > 0) { if ((unsigned int)files->size >= MAX_FILES_PER_CHUNK) { log_message(LOG_LEVEL_ERROR, "Chunk contains too many files"); - array_list_delete(files); - return NULL; + goto error; } if (remaining_size < sizeof(size_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for path length"); - array_list_delete(files); - return NULL; + goto error; } size_t path_len; @@ -227,26 +230,19 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (path_len > SIZE_MAX - 1 || remaining_size < path_len) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for path"); - array_list_delete(files); - return NULL; + goto error; } - if (path_len == SIZE_MAX) { - array_list_delete(files); - return NULL; - } char* path = protocol_alloc(path_len + 1); if (path == NULL) { log_perror("Could not allocate memory for file path"); - array_list_delete(files); - return NULL; + goto error; } memcpy(path, data_pointer, path_len); path[path_len] = '\0'; if (memchr(path, '\0', path_len) != NULL) { free(path); - array_list_delete(files); - return NULL; + goto error; } data_pointer += path_len; remaining_size -= path_len; @@ -260,8 +256,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (local_path == NULL) { log_message(LOG_LEVEL_ERROR, "--iconv: received chunk file name cannot be converted to the local charset"); - array_list_delete(files); - return NULL; + goto error; } path = local_path; path_len = strlen(path); @@ -269,30 +264,23 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (path_len == 0 || has_path_traversal(path)) { free(path); - array_list_delete(files); - return NULL; + goto error; } - File* file = file_create(path); + file = file_create(path); free(path); - if (file == NULL) { - array_list_delete(files); - return NULL; - } + if (file == NULL) + goto error; if (remaining_size < sizeof(int)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for entry type"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } int entry_type; memcpy(&entry_type, data_pointer, sizeof(int)); if (entry_type != 0 && entry_type != 1 && entry_type != 2 && entry_type != 3) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: bad entry type"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } file->is_dir = entry_type == 1; file->is_symlink = entry_type == 2; @@ -303,9 +291,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (file->is_special) { if (remaining_size < 2 * (int32_t)sizeof(int32_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for special rdev"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } int32_t special_major, special_minor; memcpy(&special_major, data_pointer, sizeof(special_major)); @@ -320,9 +306,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (special_major < 0 || special_minor < 0 || special_major > 0xffff || special_minor > 0x00ffffff) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: out-of-range special rdev"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } file->rdev_major = special_major; file->rdev_minor = special_minor; @@ -331,37 +315,32 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (use_metadata) { if (remaining_size < sizeof(int)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for metadata"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } - // Peek at present flag to determine total size needed before reading + /* Peek at the present flag to determine the total record size before + decoding. metadata_from_buf() independently bounds-checks every read + against remaining_size, so a short body can never over-read. */ int present_flag; memcpy(&present_flag, data_pointer, sizeof(int)); if ((present_flag != 0 && present_flag != 1) || (present_flag == 1 && remaining_size < sizeof(int) + FILE_METADATA_WIRE_SIZE)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for metadata body"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } - file->metadata = metadata_from_buf(&data_pointer); - remaining_size -= sizeof(int); + file->metadata = metadata_from_buf((const uint8_t*)data_pointer, remaining_size); + size_t metadata_consumed = sizeof(int); if (present_flag == 1) { - if (file->metadata == NULL) { - file_destroy(file); - array_list_delete(files); - return NULL; - } - remaining_size -= FILE_METADATA_WIRE_SIZE; + if (file->metadata == NULL) + goto error; + metadata_consumed += FILE_METADATA_WIRE_SIZE; } + data_pointer += metadata_consumed; + remaining_size -= metadata_consumed; } if (remaining_size < sizeof(size_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for data size"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } size_t file_data_size; @@ -371,35 +350,26 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (remaining_size < file_data_size) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for file content"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } // Reject individual file data larger than the maximum allowed size. if (file_data_size > MAX_FILE_DATA_SIZE) { log_message(LOG_LEVEL_ERROR, "File data size %zu exceeds maximum %llu", file_data_size, (unsigned long long)MAX_FILE_DATA_SIZE); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } size_t allocation_size = file_data_size > 0 ? file_data_size : 1; void* file_data = protocol_alloc(allocation_size); if (file_data == NULL) { log_perror("Could not allocate memory for file data"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } memcpy(file_data, data_pointer, file_data_size); Data* replacement = data_create(file_data, file_data_size); - if (replacement == NULL) { - file_destroy(file); - array_list_delete(files); - return NULL; - } + if (replacement == NULL) + goto error; data_destroy(file->data); file->data = replacement; data_pointer += file_data_size; @@ -408,9 +378,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { if (file->is_symlink) { if (remaining_size < sizeof(size_t)) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: not enough data for symlink target"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } size_t target_len; memcpy(&target_len, data_pointer, sizeof(size_t)); @@ -418,24 +386,18 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { remaining_size -= sizeof(size_t); if (target_len == 0 || remaining_size < target_len) { log_message(LOG_LEVEL_ERROR, "Invalid chunk format: bad symlink target"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } char* target = protocol_alloc(target_len + 1); if (!target) { log_perror("Could not allocate memory for symlink target"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } memcpy(target, data_pointer, target_len); target[target_len] = '\0'; if (memchr(target, '\0', target_len) != NULL) { free(target); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } /* The symlink target also rides the wire charset; decode it to the local charset like the path (a target is a path). */ @@ -446,9 +408,7 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { log_message(LOG_LEVEL_ERROR, "--iconv: received chunk symlink target cannot be converted to the local " "charset"); - file_destroy(file); - array_list_delete(files); - return NULL; + goto error; } target = local_target; } @@ -457,29 +417,27 @@ Chunk* chunk_deserialize(Data* data, bool use_metadata) { remaining_size -= target_len; } - if (!array_list_add(files, file)) { - file_destroy(file); - array_list_delete(files); - return NULL; - } + if (!array_list_add(files, file)) + goto error; + file = NULL; } File** file_array = (File**)array_list_to_array(files); - if (files->size > 0 && file_array == NULL) { - array_list_delete(files); - return NULL; - } + if (files->size > 0 && file_array == NULL) + goto error; Chunk* chunk = chunk_create(file_array, files->size); - free(file_array); - if (chunk == NULL) { - array_list_delete(files); - return NULL; - } + if (chunk == NULL) + goto error; files->item_destroyer = NULL; array_list_delete(files); - return chunk; + +error: + if (file) + file_destroy(file); + array_list_delete(files); + return NULL; } Data* chunk_compress(Chunk* chunk, int compression_level, bool use_metadata) { diff --git a/src/shared/compression.c b/src/shared/compression.c index d9aa037..7609921 100644 --- a/src/shared/compression.c +++ b/src/shared/compression.c @@ -17,10 +17,6 @@ static char* SKIP_COMPRESSION_EXTENSIONS[] = {".jpg", ".jpeg", ".png", ".gif", ".mp4", ".mkv", ".zip", ".gz", ".xz", ".zst", NULL}; -bool compression_should_skip(const char* path) { - return compression_should_skip_with_suffixes(path, NULL, -1); -} - bool compression_should_skip_with_suffixes(const char* path, char* const* suffixes, int count) { if (!path) return false; diff --git a/src/shared/compression.h b/src/shared/compression.h index 179c2b7..2c3753c 100644 --- a/src/shared/compression.h +++ b/src/shared/compression.h @@ -11,7 +11,6 @@ Data* data_compress_with_threads(Data* data_to_compress, int compression_level, int compression_threads); Data* data_decompress(Data* compressed_data); Data* data_decompress_limited(Data* compressed_data, size_t maximum_size); -bool compression_should_skip(const char* path); bool compression_should_skip_with_suffixes(const char* path, char* const* suffixes, int count); /* Release the calling thread's cached zstd contexts (compressor, decompressor diff --git a/src/shared/config.c b/src/shared/config.c index 6730639..c0f1412 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -252,6 +252,11 @@ static char* config_receive_str_redacted(int fd, ConfigStringBudget* budget) { } static bool validate_received_config(const Config* config) { + /* Cross-field invariants live in one place (config_invariants_error) so the + receiver enforces every combination the client relies on; a hostile peer + can forge a frame that violates any clause of the shared predicate. */ + if (config_invariants_error(config) != NULL) + return false; return valid_wire_bool(config->save_to_disk) && valid_wire_bool(config->use_multithreading) && valid_wire_bool(config->use_chunk_serialization) && valid_wire_bool(config->use_compression) && valid_wire_bool(config->use_metadata) && @@ -275,30 +280,14 @@ static bool validate_received_config(const Config* config) { valid_wire_bool(config->delete_delay) && valid_wire_bool(config->delete_during) && valid_wire_bool(config->relative) && valid_wire_bool(config->prune_empty_dirs) && valid_wire_bool(config->delay_updates) && valid_wire_bool(config->mkpath) && - !(config->delay_updates && config->inplace) && - !(config->delay_updates && delay_updates_staging_name_conflict(config->backup_dir)) && 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) && config_has_valid_delete_timing(config) && - identity_wire_valid(config) && - !(config->skip_compress_set && config->use_chunk_serialization) && - /* --append / --append-verify tail resume needs the per-file check, - which chunk serialization -s disables: reject on the receiver too - so a -s sender cannot negotiate an inert append mode. */ - !((config->append || config->append_verify) && config->use_chunk_serialization) && - !(config->preserve_hard_links && config->use_chunk_serialization) && - !(config->preserve_hard_links && (config->append || config->append_verify)) && - /* The xattr block rides the per-file streaming frame, which -s drops. */ - !((config->preserve_xattrs || config->preserve_acls) && config->use_chunk_serialization) && + 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)) && - /* --copy-as forces ownership through the metadata path; without - metadata it would pass the privilege gate but silently chown - nothing. Refuse the frame instead. */ - (!config->copy_as_set || config->use_metadata) && (!config->use_compression || (config->compression_level >= 1 && config->compression_level <= 22)) && config->chunk_size > 0 && config->chunk_size <= MAX_CHUNK_SIZE && @@ -309,14 +298,6 @@ static bool validate_received_config(const Config* config) { config->skip_compress_count <= MAX_SKIP_COMPRESS_SUFFIXES && config->max_alloc > 0 && (!config->chmod_spec || !*config->chmod_spec || chmod_apply(0, config->chmod_spec, &(mode_t){0})) && - /* The received --iconv CONVERT_SPEC is untrusted input that drives - the receiver's path decoding: reject a malformed spec or an - unsupported charset name so the run is refused up front instead of - every received file name failing mid-transfer. A NULL spec (iconv - disabled) is always accepted. */ - (!config->iconv_spec || charset_spec_valid(config->iconv_spec)) && - /* --super / --no-super: the received tri-state must be one of the - defined values (AUTO/ON/OFF); anything else is a malformed frame. */ config->super_mode >= SUPER_MODE_AUTO && config->super_mode <= SUPER_MODE_OFF; } @@ -348,6 +329,65 @@ bool config_has_valid_delete_timing(const Config* config) { return timing_count <= 1; } +/* The cross-field invariants FastSync relies on, in one place. Every message + * here was previously duplicated (verbatim) in client_validation.c and/or + * config.c; the client reports the returned string for UX and the server + * enforces the same rules at its trust boundary. Pure: no I/O, no logging. + * The order is deliberate (most specific structural conflicts first). */ +const char* config_invariants_error(const Config* config) { + if (!config) + return "Invalid configuration"; + if (config_has_basis(config) && config->use_chunk_serialization) + return "--compare-dest/--copy-dest/--link-dest require per-file incremental checks and cannot " + "be combined with -s (chunk serialization)"; + if (config->use_sendfile && (config->use_chunk_serialization || config->use_compression)) + return "-f/--sendfile cannot be combined with -c (compression) or -s (chunk serialization)"; + if (config->use_incremental && config->use_chunk_serialization) + return "--incremental is not supported with -s (chunk serialization)"; + if (config->skip_compress_set && config->use_chunk_serialization) + return "--skip-compress cannot be combined with -s (chunk serialization)"; + if (config->use_delta && !config->whole_file && !config->use_incremental) + return "--delta requires --incremental"; + if (config->use_delta && !config->whole_file && config->use_chunk_serialization) + return "--delta cannot be combined with -s (chunk serialization)"; + if (config->use_delta && !config->whole_file && config->use_sendfile) + return "--delta cannot be combined with -f (sendfile)"; + /* --append / --append-verify resume a shorter existing destination by + transmitting only the tail. The resume needs the per-file STATUS_CHECK + handshake (so the dest length is learned), which chunk serialization -s + disables; whole-file is the opposite intent (send everything). */ + if ((config->append || config->append_verify) && config->use_chunk_serialization) + return "--append/--append-verify require the per-file incremental check and cannot be " + "combined with -s (chunk serialization)"; + if ((config->append || config->append_verify) && config->whole_file) + return "--append/--append-verify are incompatible with --whole-file (which forces a full " + "transfer)"; + /* -H transmits each later hard-link group member as a dedicated per-file + STATUS_HARDLINK frame, which -s does not support; and a hard-links sibling + carries no payload, so the tail-resume of --append is meaningless. */ + if (config->preserve_hard_links && config->use_chunk_serialization) + return "--hard-links/-H cannot be combined with -s (chunk serialization)"; + /* -X/-A ride the per-file metadata frame; the chunk-serialization wire format + does not carry the xattr block. */ + if ((config->preserve_xattrs || config->preserve_acls) && config->use_chunk_serialization) + return "--xattrs/-X and --acls/-A cannot be combined with -s (chunk serialization)"; + if (config->preserve_hard_links && (config->append || config->append_verify)) + return "--hard-links/-H cannot be combined with --append/--append-verify"; + if (config->delay_updates && config->inplace) + return "--delay-updates does not work with --inplace"; + if (config->delay_updates && delay_updates_staging_name_conflict(config->backup_dir)) + return "--backup-dir is reserved when --delay-updates is active (used for the internal " + "staging directory)"; + if (!config_has_valid_delete_timing(config)) + return "--delete-before/--delete-during/--delete-delay/--delete-after select the delete " + "timing; at most one may be given and each implies --delete"; + if (config->iconv_spec && !charset_spec_valid(config->iconv_spec)) + return "--iconv requires LOCAL[,REMOTE] charset names supported by iconv"; + if (config->copy_as_set && !config->use_metadata) + return "--copy-as requires metadata preservation and cannot be combined with --no-preserve"; + return NULL; +} + bool config_has_basis(const Config* config) { return config && config->basis_count > 0; } diff --git a/src/shared/config.h b/src/shared/config.h index 29b7baf..b0ae890 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -754,6 +754,17 @@ bool config_delete_timing_early(const Config* config); * set (none = the default delete-after commit timing); without deletion no * timing flag may be set (each timing flag implies --delete). */ bool config_has_valid_delete_timing(const Config* config); + +/* Single source of truth for the cross-field ("combination") invariants a + * Config must satisfy. Returns NULL when `config` is consistent, or a static, + * human-readable error string (no trailing period) describing the FIRST + * violation found. No I/O, no logging and no printing, so it is safe to call + * from every trust boundary; the iconv rule does invoke charset_spec_valid + * (which parses via str_dup/iconv_open), so it is not allocation-free. The client calls + * it from validate_config() for up-front UX and the server calls it from + * validate_received_config() so the receiver enforces exactly the same + * invariants it relies on (the server is the trust boundary). */ +const char* config_invariants_error(const Config* config); /* True when at least one --compare-dest/--copy-dest/--link-dest was set. */ bool config_has_basis(const Config* config); /* Append one basis-dir entry. Returns 0 on success, -1 on allocation failure. */ diff --git a/src/shared/file.c b/src/shared/file.c index 6220967..b85e988 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -287,11 +287,6 @@ size_t file_content_to_buffer(File* file) { static int authorized_root_fd = -1; static char* authorized_root_path; -static bool path_is_within_root(const char* root, const char* path) { - size_t root_len = strlen(root); - return strncmp(root, path, root_len) == 0 && (path[root_len] == '\0' || path[root_len] == '/'); -} - bool file_set_authorized_root(int fd, const char* canonical_path) { char* path_copy = canonical_path ? str_dup(canonical_path) : NULL; if (canonical_path && !path_copy) { @@ -362,10 +357,6 @@ void file_set_keep_dirlinks(bool enable) { file_keep_dirlinks = enable; } -bool file_get_keep_dirlinks(void) { - return file_keep_dirlinks; -} - /* --trust-sender (Phase 5) receiver process-wide policy: when set, the receiver * trusts the sender's file list and skips its own redundant up-front re- * validation (empty/".." path rejection, escaping-symlink-target containment). diff --git a/src/shared/file.h b/src/shared/file.h index 554170b..570d703 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -52,7 +52,6 @@ bool file_symlink_at_secure(const char* path, const char* target); /* --keep-dirlinks (-K) receiver process-wide policy: allow an in-root existing * symlink-to-directory to be followed as a directory. */ void file_set_keep_dirlinks(bool enable); -bool file_get_keep_dirlinks(void); /* --trust-sender receiver process-wide policy (Phase 5). When set, the * receiver trusts that the sender already produced a clean file list and skips diff --git a/src/shared/file_store.c b/src/shared/file_store.c index b14da25..5365598 100644 --- a/src/shared/file_store.c +++ b/src/shared/file_store.c @@ -1,129 +1,7 @@ #include -#include -#include -#include -#include -#include -#include #include #include "file_store.h" -#include "metadata.h" -#include "utils.h" - -static int authorized_root_fd = -1; -static char* authorized_root_path; - -static bool path_is_within_root(const char* root, const char* path) { - size_t root_length = strlen(root); - return strncmp(root, path, root_length) == 0 && - (path[root_length] == '\0' || path[root_length] == '/'); -} - -bool file_store_set_authorized_root(int fd, const char* canonical_path) { - char* new_path = canonical_path ? str_dup(canonical_path) : NULL; - if (canonical_path && !new_path) { - authorized_root_fd = -1; - free(authorized_root_path); - authorized_root_path = NULL; - return false; - } - free(authorized_root_path); - authorized_root_path = new_path; - authorized_root_fd = fd; - return true; -} - -int file_store_open_secure_parent(const char* path, char** leaf_out) { - char* copy = str_dup(path); - if (!copy) - return -1; - char* parent = dirname(copy); - const char* slash = strrchr(path, '/'); - char* leaf = str_dup(slash ? slash + 1 : path); - if (!leaf) { - free(copy); - return -1; - } - int fd; - if (authorized_root_fd >= 0) { - if (!authorized_root_path || path[0] != '/' || - !path_is_within_root(authorized_root_path, path)) { - free(copy); - free(leaf); - return -1; - } - fd = dup(authorized_root_fd); - if (fd < 0) { - free(copy); - free(leaf); - return -1; - } - size_t root_length = strlen(authorized_root_path); - char* relative = str_dup(path + root_length); - if (!relative) { - free(copy); - free(leaf); - close(fd); - return -1; - } - free(copy); - copy = relative; - parent = dirname(copy); - } else { - fd = (parent[0] == '/') ? open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC) - : open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); - } - if (fd < 0) { - free(copy); - free(leaf); - return -1; - } - char* save = NULL; - char* component = strtok_r(parent, "/", &save); - while (component) { - if (strcmp(component, "..") == 0) { - close(fd); - free(copy); - free(leaf); - return -1; - } - if (strcmp(component, ".") != 0) { - int next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - if (next < 0 && errno == ENOENT) { - if (mkdirat(fd, component, 0755) == 0 || errno == EEXIST) - next = openat(fd, component, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - } - if (next < 0) { - close(fd); - free(copy); - free(leaf); - return -1; - } - close(fd); - fd = next; - } - component = strtok_r(NULL, "/", &save); - } - free(copy); - *leaf_out = leaf; - return fd; -} - -bool file_store_rename_secure(const char* old_path, const char* new_path) { - char *old_leaf = NULL, *new_leaf = NULL; - int old_parent = file_store_open_secure_parent(old_path, &old_leaf); - int new_parent = file_store_open_secure_parent(new_path, &new_leaf); - bool ok = old_parent >= 0 && new_parent >= 0 && - renameat(old_parent, old_leaf, new_parent, new_leaf) == 0; - if (old_parent >= 0) - close(old_parent); - if (new_parent >= 0) - close(new_parent); - free(old_leaf); - free(new_leaf); - return ok; -} static bool write_all(int fd, const void* data, unsigned long long size) { const unsigned char* p = data; @@ -176,67 +54,3 @@ bool file_store_write_sparse(int fd, const unsigned char* data, unsigned long lo } return ftruncate(fd, (off_t)size) == 0; } - -bool file_store_write_secure(const char* path, const void* data, unsigned long long data_size, - bool inplace, bool sparse, const FileMetadata* metadata, - bool preserve_executability) { - char* leaf = NULL; - int dirfd = file_store_open_secure_parent(path, &leaf); - if (dirfd < 0) - return false; - int fd = -1; - bool ok = false; - if (inplace) { - fd = openat(dirfd, leaf, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0644); - if (fd >= 0) { - if (sparse && data_size > 0) { - if (ftruncate(fd, (off_t)data_size) == 0) - ok = file_store_write_sparse(fd, data, data_size); - } else { - ok = write_all(fd, data, data_size); - } - if (ok && metadata) - ok = file_restore_metadata_fd(fd, metadata, preserve_executability); - } - } else { - int tmp_size = snprintf(NULL, 0, ".%s.tmp.%ld.%u", leaf, (long)getpid(), 99U); - if (tmp_size < 0) { - close(dirfd); - free(leaf); - return false; - } - char* tmp = malloc((size_t)tmp_size + 1); - if (!tmp) { - close(dirfd); - free(leaf); - return false; - } - for (unsigned int i = 0; i < 100 && !ok; ++i) { - snprintf(tmp, (size_t)tmp_size + 1, ".%s.tmp.%ld.%u", leaf, (long)getpid(), i); - fd = openat(dirfd, tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0600); - if (fd < 0) - continue; - if (sparse && data_size > 0) - ok = ftruncate(fd, (off_t)data_size) == 0; - if (ok || (!sparse || data_size == 0)) - ok = (sparse && data_size > 0) - ? file_store_write_sparse(fd, (const unsigned char*)data, data_size) - : write_all(fd, data, data_size); - if (ok && metadata) - ok = file_restore_metadata_fd(fd, metadata, preserve_executability); - if (close(fd) != 0) - ok = false; - fd = -1; - if (ok && renameat(dirfd, tmp, dirfd, leaf) != 0) - ok = false; - if (!ok) - unlinkat(dirfd, tmp, 0); - } - free(tmp); - } - if (fd >= 0) - close(fd); - close(dirfd); - free(leaf); - return ok; -} diff --git a/src/shared/file_store.h b/src/shared/file_store.h index 4bfb39a..9514648 100644 --- a/src/shared/file_store.h +++ b/src/shared/file_store.h @@ -1,15 +1,8 @@ #ifndef FILE_STORE_H #define FILE_STORE_H -#include "file.h" #include -bool file_store_set_authorized_root(int fd, const char* canonical_path); -int file_store_open_secure_parent(const char* path, char** leaf_out); -bool file_store_rename_secure(const char* old_path, const char* new_path); -bool file_store_write_secure(const char* path, const void* data, unsigned long long data_size, - bool inplace, bool sparse, const FileMetadata* metadata, - bool preserve_executability); /* Sparse-aware write (--sparse/-S): every all-zero run of at least * SPARSE_HOLE_MIN bytes is skipped with lseek(SEEK_CUR) so it becomes a real * hole; every other byte is written. The caller pre-sizes the file with diff --git a/src/shared/metadata.c b/src/shared/metadata.c index d1f14e5..a1e5784 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -90,63 +90,65 @@ void metadata_to_buf(char** buf, const FileMetadata* m) { *buf += sizeof(crtime_nsec); } -FileMetadata* metadata_from_buf(char** buf) { +FileMetadata* metadata_from_buf(const uint8_t* buf, size_t len) { + if (buf == NULL || len < sizeof(int32_t)) + return NULL; int32_t present; - memcpy(&present, *buf, sizeof(present)); - *buf += sizeof(present); - if (present != 0 && present != 1) + memcpy(&present, buf, sizeof(present)); + if (present != 1) return NULL; - if (!present) + if (len < sizeof(int32_t) + FILE_METADATA_WIRE_SIZE) return NULL; + const uint8_t* cursor = buf + sizeof(int32_t); FileMetadata* m = protocol_alloc(sizeof(FileMetadata)); if (m == NULL) return NULL; int32_t mode; - memcpy(&mode, *buf, sizeof(mode)); - *buf += sizeof(mode); + memcpy(&mode, cursor, sizeof(mode)); + cursor += sizeof(mode); m->mode = (mode_t)mode; int32_t uid; - memcpy(&uid, *buf, sizeof(uid)); - *buf += sizeof(uid); + memcpy(&uid, cursor, sizeof(uid)); + cursor += sizeof(uid); m->uid = (uid_t)uid; int32_t gid; - memcpy(&gid, *buf, sizeof(gid)); - *buf += sizeof(gid); + memcpy(&gid, cursor, sizeof(gid)); + cursor += sizeof(gid); m->gid = (gid_t)gid; int64_t mtime_sec; - memcpy(&mtime_sec, *buf, sizeof(mtime_sec)); - *buf += sizeof(mtime_sec); + memcpy(&mtime_sec, cursor, sizeof(mtime_sec)); + cursor += sizeof(mtime_sec); m->mtime_sec = (time_t)mtime_sec; int64_t mtime_nsec; - memcpy(&mtime_nsec, *buf, sizeof(mtime_nsec)); - *buf += sizeof(mtime_nsec); + memcpy(&mtime_nsec, cursor, sizeof(mtime_nsec)); + cursor += sizeof(mtime_nsec); m->mtime_nsec = (long)mtime_nsec; int32_t atime_valid; - memcpy(&atime_valid, *buf, sizeof(atime_valid)); - *buf += sizeof(atime_valid); + memcpy(&atime_valid, cursor, sizeof(atime_valid)); + cursor += sizeof(atime_valid); int64_t atime_sec; - memcpy(&atime_sec, *buf, sizeof(atime_sec)); - *buf += sizeof(atime_sec); + memcpy(&atime_sec, cursor, sizeof(atime_sec)); + cursor += sizeof(atime_sec); int64_t atime_nsec; - memcpy(&atime_nsec, *buf, sizeof(atime_nsec)); - *buf += sizeof(atime_nsec); + memcpy(&atime_nsec, cursor, sizeof(atime_nsec)); + cursor += sizeof(atime_nsec); int32_t crtime_valid; - memcpy(&crtime_valid, *buf, sizeof(crtime_valid)); - *buf += sizeof(crtime_valid); + memcpy(&crtime_valid, cursor, sizeof(crtime_valid)); + cursor += sizeof(crtime_valid); int64_t crtime_sec; - memcpy(&crtime_sec, *buf, sizeof(crtime_sec)); - *buf += sizeof(crtime_sec); + memcpy(&crtime_sec, cursor, sizeof(crtime_sec)); + cursor += sizeof(crtime_sec); int64_t crtime_nsec; - memcpy(&crtime_nsec, *buf, sizeof(crtime_nsec)); - *buf += sizeof(crtime_nsec); + memcpy(&crtime_nsec, cursor, sizeof(crtime_nsec)); + cursor += sizeof(crtime_nsec); m->atime_valid = atime_valid != 0; m->atime_sec = (time_t)atime_sec; m->atime_nsec = (long)atime_nsec; m->crtime_valid = crtime_valid != 0; m->crtime_sec = (time_t)crtime_sec; m->crtime_nsec = (long)crtime_nsec; - if (present != 1 || mtime_nsec < 0 || mtime_nsec >= 1000000000LL || mode < 0 || uid < 0 || - gid < 0 || atime_valid < 0 || atime_valid > 1 || crtime_valid < 0 || crtime_valid > 1 || + if (mtime_nsec < 0 || mtime_nsec >= 1000000000LL || mode < 0 || uid < 0 || gid < 0 || + atime_valid < 0 || atime_valid > 1 || crtime_valid < 0 || crtime_valid > 1 || (atime_valid && (atime_nsec < 0 || atime_nsec >= 1000000000LL)) || (crtime_valid && (crtime_nsec < 0 || crtime_nsec >= 1000000000LL))) { free(m); @@ -196,8 +198,7 @@ FileMetadata* metadata_receive(int file_descriptor, int* ok) { *ok = 0; return NULL; } - char* cursor = packed; - FileMetadata* m = metadata_from_buf(&cursor); + FileMetadata* m = metadata_from_buf((const uint8_t*)packed, sizeof(packed)); if (m == NULL) { if (ok) *ok = 0; diff --git a/src/shared/metadata.h b/src/shared/metadata.h index 0540037..82f3ecc 100644 --- a/src/shared/metadata.h +++ b/src/shared/metadata.h @@ -3,6 +3,7 @@ #include "file.h" #include +#include #include #include #include @@ -39,7 +40,13 @@ #define FILE_METADATA_WIRE_SIZE (sizeof(int32_t) * 5 + sizeof(int64_t) * 6) void metadata_to_buf(char** buf, const FileMetadata* m); -FileMetadata* metadata_from_buf(char** buf); +/* Decode one packed metadata record (an int32 present flag followed, when + * present, by FILE_METADATA_WIRE_SIZE field bytes) from `buf`, which has `len` + * readable bytes. Every read is bounds-checked against `len`, so the function + * can never over-read the caller's buffer: a too-short record, an absent + * (present == 0) record and a malformed record all return NULL. A successful + * decode returns a heap-allocated FileMetadata owned by the caller. */ +FileMetadata* metadata_from_buf(const uint8_t* buf, size_t len); bool metadata_send(int file_descriptor, const FileMetadata* m); FileMetadata* metadata_receive(int file_descriptor, int* ok); void file_restore_metadata(const char* path, const FileMetadata* metadata, diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 697e461..14ad46e 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -179,6 +179,9 @@ bool pipeline_context_sender_enqueue_chunk(PipelineContextSender* context, Chunk } void pipeline_context_sender_destroy(PipelineContextSender* context) { + /* `config` is borrowed: the caller retains ownership and frees it after the + pipeline has been destroyed (the worker threads are already joined, so no + config access can outlive this call). */ if (context->manifest) { array_list_delete(context->manifest); } @@ -192,7 +195,6 @@ void pipeline_context_sender_destroy(PipelineContextSender* context) { array_list_delete(context->dir_entries); if (context->dir_entries_mutex_init) mtx_destroy(&context->dir_entries_mutex); - config_delete(context->config); queue_destroy(context->queue_scanner); queue_destroy(context->queue_loader); mtx_destroy(&context->mutex_scanner); diff --git a/src/shared/multiprocessing.h b/src/shared/multiprocessing.h index 73162a4..6de2e98 100644 --- a/src/shared/multiprocessing.h +++ b/src/shared/multiprocessing.h @@ -120,6 +120,8 @@ typedef struct PipelineContextReceiver { DirTimeList dir_times; } PipelineContextReceiver; +/* `config` is borrowed and must outlive the context: destroy does NOT free it, + so the caller owns it and frees it with config_delete() afterwards. */ PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* queue_scanner, Queue* queue_loader); void pipeline_context_sender_destroy(PipelineContextSender* context); diff --git a/src/shared/protocol.c b/src/shared/protocol.c index 3c3eee0..f3dd81b 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -570,10 +570,6 @@ Data* protocol_receive_data_limited(ProtocolSession* session, unsigned long long return result; } -Data* protocol_receive_data(ProtocolSession* session) { - return protocol_receive_data_limited(session, MAX_DATA_PAYLOAD_SIZE); -} - bool protocol_send_int(ProtocolSession* session, int data) { if (!protocol_send_n_data(session, &data, sizeof(int))) return false; diff --git a/src/shared/protocol.h b/src/shared/protocol.h index 60f6dec..b27c2d1 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -172,7 +172,6 @@ char* protocol_receive_str(ProtocolSession* session); bool protocol_send_str_redacted(ProtocolSession* session, const char* data); char* protocol_receive_str_redacted(ProtocolSession* session); bool protocol_send_data(ProtocolSession* session, const Data* data); -Data* protocol_receive_data(ProtocolSession* session); Data* protocol_receive_data_limited(ProtocolSession* session, unsigned long long maximum_size); bool protocol_send_int(ProtocolSession* session, int data); bool protocol_receive_int(ProtocolSession* session, int* data); diff --git a/src/shared/transport_tcp.c b/src/shared/transport_tcp.c index 42dad4c..2758cbe 100644 --- a/src/shared/transport_tcp.c +++ b/src/shared/transport_tcp.c @@ -440,10 +440,6 @@ bool tcp_connect_socket_ex(Client* client, const char* host, int port, return true; } -bool tcp_connect_socket(Client* client, const char* host, int port) { - return tcp_connect_socket_ex(client, host, port, NULL); -} - bool client_connect_ex(Client* client, const char* host, int port, const TcpConnectOptions* opts) { if (!tcp_connect_socket_ex(client, host, port, opts)) return false; diff --git a/src/shared/transport_tcp.h b/src/shared/transport_tcp.h index 4439003..e37b879 100644 --- a/src/shared/transport_tcp.h +++ b/src/shared/transport_tcp.h @@ -57,7 +57,6 @@ bool client_connect_ex(Client* client, const char* host, int port, const TcpConn bool client_connect(Client* client, const char* host, int port); bool tcp_connect_socket_ex(Client* client, const char* host, int port, const TcpConnectOptions* opts); -bool tcp_connect_socket(Client* client, const char* host, int port); void client_disconnect(Client* client); void client_delete(Client* client); void tcp_set_timeouts(int timeout_sec, int contimeout_sec); diff --git a/src/shared/utils.c b/src/shared/utils.c index 080785d..d790172 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -36,11 +36,19 @@ void utils_set_authorized_root_fd(int fd) { (void)utils_set_authorized_root(fd, NULL); } -static bool path_is_within_root(const char* root, const char* path) { +bool path_is_within_root(const char* root, const char* path) { size_t root_len = strlen(root); return strncmp(root, path, root_len) == 0 && (path[root_len] == '\0' || path[root_len] == '/'); } +/* Open the destination root directory itself, confined to the authorized root. + * NOTE (do not merge with file_open_secure_parent): this walk opens dest_root + * (a directory that must already exist) and returns its fd, whereas + * file_open_secure_parent resolves the PARENT of a file path, optionally + * creating missing components and honouring --keep-dirlinks / --copy-as. The + * two differ in create-vs-no-create, in what path component they stop at, and + * in the extra receiver policies they apply, so they are intentionally kept + * separate. Both rely on the shared lexical path_is_within_root check. */ static int open_authorized_destination(const char* dest_root) { if (authorized_root_fd < 0 || !authorized_root_path || !dest_root || !path_is_within_root(authorized_root_path, dest_root)) diff --git a/src/shared/utils.h b/src/shared/utils.h index e97c635..f4a5bd6 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -127,6 +127,12 @@ bool utils_set_authorized_root(int fd, const char* canonical_path); /* The fd-only compatibility form is fail-closed for path-based operations; * callers should use utils_set_authorized_root with the canonical identity. */ void utils_set_authorized_root_fd(int fd); +/* True when `path` is `root` itself or lies directly beneath it: a lexical + * prefix test requiring the byte after `root` to be '\0' or '/'. Both `root` + * and `path` must be absolute canonical paths free of "."/".." components (the + * callers guarantee this); this is containment by string, not by resolved + * symlinks. Shared by the utils and file secure-walk root confinement. */ +bool path_is_within_root(const char* root, const char* path); bool has_path_traversal(const char* path); bool utils_valid_batch_path(const char* path); bool format_human_bytes(unsigned long long bytes, char* buffer, size_t buffer_size); diff --git a/tests/fuzz/fuzz_chunk_deserialize.c b/tests/fuzz/fuzz_chunk_deserialize.c index 444a297..f1c7d2a 100644 --- a/tests/fuzz/fuzz_chunk_deserialize.c +++ b/tests/fuzz/fuzz_chunk_deserialize.c @@ -17,7 +17,11 @@ int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { if (!d) return 0; - Chunk* chunk = chunk_deserialize(d, false); + /* Exercise both the metadata and non-metadata chunk layouts: the + metadata branch (present flag + 4-vs-72 advance) is only reachable with + use_metadata=true, so base the choice on the input rather than hardcoding + false. */ + Chunk* chunk = chunk_deserialize(d, (data[0] & 1) != 0); if (chunk) chunk_destroy(chunk); diff --git a/tests/fuzz/fuzz_metadata_from_buf.c b/tests/fuzz/fuzz_metadata_from_buf.c index 00d051d..57b2a92 100644 --- a/tests/fuzz/fuzz_metadata_from_buf.c +++ b/tests/fuzz/fuzz_metadata_from_buf.c @@ -5,19 +5,19 @@ #include int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { - if (size < sizeof(int) + FILE_METADATA_WIRE_SIZE) - return 0; - - char* buf = malloc(size); + /* Exercise the bounds-checked decoder on EVERY input length, including + * records shorter than a full metadata body; the decoder must reject those + * without reading past `size`. */ + char* buf = malloc(size > 0 ? size : 1); if (!buf) return 0; - memcpy(buf, data, size); + if (size > 0) + memcpy(buf, data, size); - char* original_buf = buf; - FileMetadata* m = metadata_from_buf(&buf); + FileMetadata* m = metadata_from_buf((const uint8_t*)buf, size); if (m) free(m); - free(original_buf); + free(buf); return 0; } diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index bdbec46..7e2ea6e 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -1239,6 +1239,20 @@ class TestProgress: assert "Stats:" in result.stderr assert "KB" in result.stderr + def test_human_readable_stats_multithreaded(self, shared_server): + # The multithreaded sender shares the single-threaded --stats format, + # including --human-readable and the rate suffix. + clean_dir(DEST_DIR) + result, dur = run_client( + SOURCE_DIR, DEST_DIR, + flags=["--threads", "-h", "--stats"], + port=shared_server.port, + ) + assert result.returncode == 0, f"Exit {result.returncode}: {result.stderr[:100]}" + assert "Stats:" in result.stderr + assert "KB" in result.stderr + assert "/s" in result.stderr + def test_human_readable_progress_multithreaded(self, shared_server): clean_dir(DEST_DIR) result, dur = run_client( diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 786efe9..e5375b9 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -122,6 +122,52 @@ static void test_validate_config_delta_sendfile_constraints() { config_delete(cfg); } +/* The client must still reject every combination now enforced by the shared + config_invariants_error() predicate (the server trusts the same rules). */ +static void test_validate_config_unified_invariants() { + Config* cfg = valid_client_config(); + cfg->use_incremental = true; + cfg->use_delta = true; + cfg->use_chunk_serialization = true; + EXPECT_FALSE(validate_config(cfg)); /* delta + chunk */ + config_delete(cfg); + + cfg = valid_client_config(); + cfg->use_delta = true; /* whole_file false */ + EXPECT_FALSE(validate_config(cfg)); /* delta without incremental */ + config_delete(cfg); + + cfg = valid_client_config(); + cfg->use_sendfile = true; + cfg->use_chunk_serialization = true; + EXPECT_FALSE(validate_config(cfg)); /* sendfile + chunk */ + config_delete(cfg); + + cfg = valid_client_config(); + cfg->preserve_hard_links = true; + cfg->use_chunk_serialization = true; + EXPECT_FALSE(validate_config(cfg)); /* hard-links + chunk */ + config_delete(cfg); + + cfg = valid_client_config(); + cfg->preserve_hard_links = true; + cfg->append = true; + EXPECT_FALSE(validate_config(cfg)); /* hard-links + append */ + config_delete(cfg); + + cfg = valid_client_config(); + cfg->append = true; + cfg->whole_file = true; + EXPECT_FALSE(validate_config(cfg)); /* append + whole-file */ + config_delete(cfg); + + cfg = valid_client_config(); + cfg->preserve_xattrs = true; + cfg->use_chunk_serialization = true; + EXPECT_FALSE(validate_config(cfg)); /* xattrs + chunk */ + config_delete(cfg); +} + /* Test main() with --help flag (early return path, no server connection needed) */ static void test_cli_help() { /* We can't easily call main() because it calls send_files which needs a server. @@ -3155,6 +3201,7 @@ void test_client_cli() { test_validate_config_tls_requirements(); test_validate_config_credentials_require_tls_or_loopback(); test_validate_config_delta_sendfile_constraints(); + test_validate_config_unified_invariants(); test_cli_help(); test_cli_archive_flags(); test_cli_dry_run(); diff --git a/tests/test_config.c b/tests/test_config.c index 003d768..c9e672e 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -383,6 +383,7 @@ static void test_pipeline_sender_lifecycle() { EXPECT_EQ_INT((int)pcs->allocation_session.max_alloc, (int)cfg->max_alloc); pipeline_context_sender_destroy(pcs); + config_delete(cfg); /* the context borrows cfg; the caller owns it */ } static void test_pipeline_receiver_lifecycle() { @@ -2042,6 +2043,156 @@ static void test_super_does_not_imply_numeric() { config_delete(c); } +/* The single shared predicate must reject every cross-field combination the + client/server enforce and accept a plain valid config. Because both + validate_config() (client) and validate_received_config() (server) call it, + this table documents the whole invariant set in one place. */ +static void test_config_invariants_error_all_combinations() { + Config* c = config_create(); + EXPECT_NOT_NULL(c); + c->send_directory = str_dup("/src"); + c->receive_root_directory = str_dup("/dst"); + EXPECT_NULL(config_invariants_error(c)); + config_delete(c); + + c = config_create(); + EXPECT_EQ_INT(config_basis_append(c, BASIS_DEST_COMPARE, "sub"), 0); + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* basis + chunk */ + config_delete(c); + + c = config_create(); + c->use_sendfile = true; + c->use_compression = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* sendfile + compression */ + config_delete(c); + + c = config_create(); + c->use_sendfile = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* sendfile + chunk */ + config_delete(c); + + c = config_create(); + c->use_incremental = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* incremental + chunk */ + config_delete(c); + + c = config_create(); + c->skip_compress_set = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* skip-compress + chunk */ + config_delete(c); + + c = config_create(); + c->use_delta = true; /* whole_file false -> active */ + EXPECT_NOT_NULL(config_invariants_error(c)); /* delta without incremental */ + config_delete(c); + + c = config_create(); + c->use_delta = true; + c->use_incremental = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* delta + chunk */ + config_delete(c); + + c = config_create(); + c->use_delta = true; + c->use_incremental = true; + c->use_sendfile = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* delta + sendfile */ + config_delete(c); + + c = config_create(); + c->append = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* append + chunk */ + config_delete(c); + + c = config_create(); + c->append = true; + c->whole_file = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* append + whole-file */ + config_delete(c); + + c = config_create(); + c->preserve_hard_links = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* hard-links + chunk */ + config_delete(c); + + c = config_create(); + c->preserve_xattrs = true; + c->use_chunk_serialization = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* xattrs + chunk */ + config_delete(c); + + c = config_create(); + c->preserve_hard_links = true; + c->append = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* hard-links + append */ + config_delete(c); + + c = config_create(); + c->delay_updates = true; + c->inplace = true; + EXPECT_NOT_NULL(config_invariants_error(c)); /* delay-updates + inplace */ + config_delete(c); + + c = config_create(); + c->delay_updates = true; + c->backup_dir = str_dup(".fastsync-stage"); + EXPECT_NOT_NULL(config_invariants_error(c)); /* delay-updates staging conflict */ + config_delete(c); + + c = config_create(); + c->delete_delay = true; /* a timing flag without --delete */ + EXPECT_NOT_NULL(config_invariants_error(c)); /* invalid delete timing */ + config_delete(c); + + c = config_create(); + c->iconv_spec = str_dup("no-such-charset,utf-8"); + EXPECT_NOT_NULL(config_invariants_error(c)); /* malformed iconv spec */ + config_delete(c); + + c = config_create(); + c->copy_as_set = true; + c->use_metadata = false; + EXPECT_NOT_NULL(config_invariants_error(c)); /* copy-as without metadata */ + config_delete(c); +} + +/* The receiver previously missed several of these; a forged frame that sets + the offending serialized fields must now be refused at the config + handshake. (whole_file is client-only, so its rules cannot appear here.) */ +static void test_config_receive_rejects_unified_invariants() { + if (is_running_under_valgrind()) + return; + struct { + bool incremental, delta, chunk, sendfile, compression; + } cases[] = { + {true, false, true, false, false}, /* --incremental + -s */ + {false, true, true, false, false}, /* --delta + -s */ + {false, true, false, false, false}, /* --delta without --incremental */ + {false, false, false, true, true}, /* --sendfile + compression */ + {false, false, true, true, false}, /* --sendfile + -s */ + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + Config* c = config_create(); + EXPECT_NOT_NULL(c); + c->send_directory = str_dup("/src"); + c->receive_root_directory = str_dup("/dst"); + c->use_incremental = cases[i].incremental; + c->use_delta = cases[i].delta; + c->use_chunk_serialization = cases[i].chunk; + c->use_sendfile = cases[i].sendfile; + c->use_compression = cases[i].compression; + EXPECT_FALSE(roundtrip_config_ok(c)); + config_delete(c); + } +} + void test_config() { test_config_lifecycle(); test_config_ssh_dest(); @@ -2095,6 +2246,8 @@ void test_config() { test_config_receive_rejects_copy_as_without_metadata(); test_config_receive_rejects_oversized_string_budget(); test_config_receive_with_validate_rejects(); + test_config_invariants_error_all_combinations(); + test_config_receive_rejects_unified_invariants(); } test_identity_copy_as_refused(); test_identity_ownership_requested(); diff --git a/tests/test_file.c b/tests/test_file.c index 4d53b70..4561f1a 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -3,7 +3,6 @@ #endif #include "test_file.h" #include "file.h" -#include "file_store.h" #include "file_receive.h" #include "data.h" #include "config.h" @@ -1225,7 +1224,7 @@ void test_trust_sender() { } /* --sparse/-S hole preservation: a buffer with a long zero run written via - * file_store_write_secure(sparse=true) must round-trip its content exactly and + * file_to_disk_secure(sparse=true) must round-trip its content exactly and * have the right logical size, and should additionally be genuinely sparse on * filesystems that support holes. The sparseness assertion is tolerant: if the * filesystem reports no holes (SEEK_HOLE/SEEK_DATA -> ENXIO) we skip the strict @@ -1247,7 +1246,7 @@ static void test_file_write_to_disk_sparse_preserves_holes() { buf[size - 1 - i] = (unsigned char)((i * 7) % 253); } - EXPECT_TRUE(file_store_write_secure(path, buf, size, false, true, NULL, false)); + EXPECT_TRUE(file_to_disk_secure(path, buf, size, false, true, false, NULL, false, NULL)); /* Logical size must equal data_size exactly. */ struct stat st; diff --git a/tests/test_fuzz_smoke.c b/tests/test_fuzz_smoke.c index 50d6c29..22efd50 100644 --- a/tests/test_fuzz_smoke.c +++ b/tests/test_fuzz_smoke.c @@ -128,8 +128,7 @@ static void test_fuzz_metadata_from_buf() { EXPECT_EQ_INT((int)(meta_ptr - meta_buf), (int)meta_buf_size); /* Deserialize from buffer (simulates fuzz_metadata_from_buf) */ - char* buf_copy = meta_buf; - FileMetadata* deserialized = metadata_from_buf(&buf_copy); + FileMetadata* deserialized = metadata_from_buf((const uint8_t*)meta_buf, (size_t)meta_buf_size); EXPECT_NOT_NULL(deserialized); EXPECT_EQ_INT((int)deserialized->mode, (int)meta->mode); EXPECT_EQ_INT((int)deserialized->mtime_sec, (int)meta->mtime_sec); diff --git a/tests/test_metadata.c b/tests/test_metadata.c index b6c7870..68a894f 100644 --- a/tests/test_metadata.c +++ b/tests/test_metadata.c @@ -29,8 +29,8 @@ static void test_metadata_to_from_buf_roundtrip() { char* write_ptr = buf; metadata_to_buf(&write_ptr, &original); - char* read_ptr = buf; - FileMetadata* result = metadata_from_buf(&read_ptr); + FileMetadata* result = + metadata_from_buf((const uint8_t*)buf, FILE_METADATA_WIRE_SIZE + sizeof(int)); EXPECT_NOT_NULL(result); EXPECT_EQ_INT(result->mode, 0755); @@ -45,8 +45,6 @@ static void test_metadata_to_from_buf_roundtrip() { EXPECT_EQ_INT(result->crtime_sec, 1200000000); EXPECT_EQ_INT(result->crtime_nsec, 750000000); - EXPECT_EQ_INT((int)(read_ptr - buf), (int)FILE_METADATA_WIRE_SIZE + (int)sizeof(int)); - free(result); free(buf); } @@ -71,14 +69,46 @@ static void test_metadata_from_buf_null() { int present = 0; memcpy(buf, &present, sizeof(int)); - char* read_ptr = buf; - const FileMetadata* result = metadata_from_buf(&read_ptr); + const FileMetadata* result = + metadata_from_buf((const uint8_t*)buf, FILE_METADATA_WIRE_SIZE + sizeof(int)); EXPECT_NULL(result); free(buf); } +/* The decoder must reject (never over-read) a present record that is even one + * byte shorter than the full int32 flag + FILE_METADATA_WIRE_SIZE body, and + * must reject a buffer too short to even hold the present flag. */ +static void test_metadata_from_buf_bounds() { + char* buf = malloc(FILE_METADATA_WIRE_SIZE + sizeof(int)); + EXPECT_NOT_NULL(buf); + FileMetadata original = {.mode = 0644, + .uid = 1, + .gid = 2, + .mtime_sec = 3, + .mtime_nsec = 4, + .atime_valid = true, + .atime_sec = 5, + .atime_nsec = 6, + .crtime_valid = false}; + char* write_ptr = buf; + metadata_to_buf(&write_ptr, &original); + + EXPECT_NULL(metadata_from_buf((const uint8_t*)buf, 0)); + EXPECT_NULL(metadata_from_buf((const uint8_t*)buf, sizeof(int))); + EXPECT_NULL(metadata_from_buf((const uint8_t*)buf, FILE_METADATA_WIRE_SIZE + sizeof(int) - 1)); + /* A buffer larger than the record decodes using only the record prefix. */ + FileMetadata* decoded = + metadata_from_buf((const uint8_t*)buf, FILE_METADATA_WIRE_SIZE + sizeof(int) + 16); + EXPECT_NOT_NULL(decoded); + EXPECT_EQ_INT(decoded->mode, 0644); + free(decoded); + EXPECT_NULL(metadata_from_buf(NULL, FILE_METADATA_WIRE_SIZE + sizeof(int))); + + free(buf); +} + static void test_metadata_send_receive_roundtrip() { io_set_bwlimit(0); int p[2]; @@ -169,10 +199,8 @@ static void test_metadata_wire_is_one_packed_frame() { EXPECT_EQ_INT(avail, 0); /* The present frame decodes in one shot with the shared codec. */ - char* cursor = (char*)wire; - FileMetadata* decoded = metadata_from_buf(&cursor); + FileMetadata* decoded = metadata_from_buf((const uint8_t*)wire, sizeof(wire)); EXPECT_NOT_NULL(decoded); - EXPECT_EQ_INT((int)(cursor - (char*)wire), (int)sizeof(wire)); EXPECT_EQ_INT(decoded->mode, 0640); EXPECT_EQ_INT(decoded->uid, 42); EXPECT_EQ_INT(decoded->gid, 43); @@ -451,6 +479,7 @@ void test_metadata() { test_metadata_to_from_buf_roundtrip(); test_metadata_to_buf_null(); test_metadata_from_buf_null(); + test_metadata_from_buf_bounds(); test_metadata_send_receive_roundtrip(); test_metadata_send_null(); test_metadata_wire_is_one_packed_frame(); diff --git a/tests/test_multiprocessing.c b/tests/test_multiprocessing.c index 427b5c6..a051897 100644 --- a/tests/test_multiprocessing.c +++ b/tests/test_multiprocessing.c @@ -37,6 +37,7 @@ static void test_sender_create_destroy() { EXPECT_NULL(ctx->manifest); pipeline_context_sender_destroy(ctx); + config_delete(cfg); /* the context borrows cfg; the caller owns it */ } /* Test pipeline_context_receiver_create/destroy with valid arguments */ @@ -80,6 +81,7 @@ static void test_sender_queue_capacities() { EXPECT_EQ_INT(ctx->queue_scanner->capacity, 1); EXPECT_EQ_INT(ctx->queue_loader->capacity, 1); pipeline_context_sender_destroy(ctx); + config_delete(cfg); /* the context borrows cfg; the caller owns it */ } /* Invalid queue capacities must not create unusable pipeline queues. */ @@ -445,8 +447,9 @@ static void test_sender_enqueue_byte_budget() { EXPECT_EQ_INT((int)ctx->queued_bytes, 2000); /* second payload now in flight */ /* pipeline_context_sender_destroy frees the still-queued second chunk and - owns cfg/q_scanner/q_loader from here on. */ + owns q_scanner/q_loader; cfg stays borrowed and is freed by the caller. */ pipeline_context_sender_destroy(ctx); + config_delete(cfg); } void test_multiprocessing() {