From 55172ff6defb36bae0be02207c0098c86c0ad3ed Mon Sep 17 00:00:00 2001 From: TapTap Date: Fri, 4 Sep 2026 17:59:50 +0200 Subject: [PATCH] Merge remote-tracking branch 'origin/feat/ignore-existing' into dev --- src/client/client_cli.c | 1 + src/client/usage.c | 1 + src/shared/config.c | 10 ++-- src/shared/config.h | 1 + src/shared/file.c | 42 ++++++++++--- src/shared/file.h | 3 + src/shared/file_receive.c | 32 +++++++--- tests/integration/test_features.py | 54 +++++++++++++++++ tests/test_client_cli.c | 14 +++++ tests/test_config.c | 3 + tests/test_file.c | 95 ++++++++++++++++++++++++++++++ 11 files changed, 238 insertions(+), 18 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index cf5890f..956995d 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -230,6 +230,7 @@ static const OptionEntry OPTION_TABLE[] = { {"--checksum", NULL, OPT_FLAG, offsetof(Config, checksum)}, {"--8-bit-output", "-8", OPT_FLAG, offsetof(Config, eight_bit_output)}, {"--existing", NULL, OPT_FLAG, offsetof(Config, existing)}, + {"--ignore-existing", NULL, OPT_FLAG, offsetof(Config, ignore_existing)}, {"--source-dir", NULL, OPT_STRING, offsetof(Config, send_directory)}, {"--dest-dir", NULL, OPT_STRING, offsetof(Config, receive_root_directory)}, diff --git a/src/client/usage.c b/src/client/usage.c index c6fe693..754a57e 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -23,6 +23,7 @@ void print_usage(void) { printf(" --progress Show transfer progress\n"); printf(" -8, --8-bit-output Leave high-bit characters unescaped in output\n"); printf(" --delete Delete files on receiver not in source\n"); + printf(" --ignore-existing Skip files that already exist on receiver\n"); printf(" --exclude Exclude files matching pattern\n"); printf(" --include Only include files matching pattern\n"); printf(" --exclude-from Read exclude patterns from file\n"); diff --git a/src/shared/config.c b/src/shared/config.c index ee6150e..26bff0a 100644 --- a/src/shared/config.c +++ b/src/shared/config.c @@ -75,6 +75,7 @@ static void config_set_defaults(Config* config) { config->human_readable = false; config->eight_bit_output = false; config->existing = false; + config->ignore_existing = false; config->update = false; config->inplace = false; config->use_fsync = false; @@ -131,7 +132,8 @@ static bool validate_received_config(const Config* config) { valid_wire_bool(config->safe_links) && valid_wire_bool(config->copy_unsafe_links) && valid_wire_bool(config->preserve_hard_links) && valid_wire_bool(config->preserve_acls) && valid_wire_bool(config->preserve_xattrs) && valid_wire_bool(config->preserve_devices) && - valid_wire_bool(config->preserve_sparse) && valid_wire_bool(config->existing) && + valid_wire_bool(config->preserve_sparse) && valid_wire_bool(config->ignore_existing) && + valid_wire_bool(config->existing) && valid_wire_bool(config->update) && valid_wire_bool(config->inplace) && valid_wire_bool(config->append) && valid_wire_bool(config->use_fsync) && valid_wire_bool(config->append_verify) && @@ -257,7 +259,7 @@ static bool send_file_options(int fd, const Config* c) { } static bool send_selection_options(int fd, const Config* c) { - return send_int(fd, c->existing) && send_int(fd, c->update) && send_int(fd, c->inplace) && + return send_int(fd, c->ignore_existing) && send_int(fd, c->existing) && send_int(fd, c->update) && send_int(fd, c->inplace) && send_int(fd, c->append) && send_int(fd, c->use_fsync) && send_int(fd, c->append_verify) && send_int(fd, c->delete_excluded) && send_int(fd, c->delete_after) && @@ -328,9 +330,9 @@ static bool receive_file_options(int fd, Config* c) { } static bool receive_selection_options(int fd, Config* c) { - bool* flags[] = {&c->existing, &c->update, &c->inplace, &c->append, + bool* flags[] = {&c->ignore_existing, &c->existing, &c->update, &c->inplace, &c->append, &c->use_fsync, - &c->append_verify, &c->delete_excluded, &c->delete_after}; + &c->append_verify, &c->delete_excluded, &c->delete_after}; for (size_t i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { if (!receive_wire_bool(fd, flags[i])) return false; diff --git a/src/shared/config.h b/src/shared/config.h index 5cb850d..31e4f7d 100644 --- a/src/shared/config.h +++ b/src/shared/config.h @@ -83,6 +83,7 @@ typedef struct Config { // Issue #127: Transfer modes bool existing; + bool ignore_existing; bool update; bool inplace; bool use_fsync; diff --git a/src/shared/file.c b/src/shared/file.c index 377ea0d..09e369b 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -172,8 +172,17 @@ bool file_set_authorized_root(int fd, const char* canonical_path) { } bool file_path_exists_secure(const char* path) { + if (!path) + return false; + char* leaf = NULL; + int parent_fd = file_open_secure_parent(path, &leaf, false); + if (parent_fd < 0) + return false; struct stat st; - return file_stat_secure(path, &st); + bool exists = fstatat(parent_fd, leaf, &st, AT_SYMLINK_NOFOLLOW) == 0; + close(parent_fd); + free(leaf); + return exists; } bool file_stat_secure(const char* path, struct stat* st) { @@ -318,7 +327,8 @@ bool file_rename_secure(const char* old_path, const char* new_path) { static bool file_to_disk_secure_impl(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, - const FileMetadata* metadata, bool update, bool use_fsync) { + const FileMetadata* metadata, bool update, bool no_replace, + bool use_fsync) { char* leaf = NULL; int dirfd = file_open_secure_parent(path, &leaf, true); if (dirfd < 0) @@ -374,8 +384,20 @@ static bool file_to_disk_secure_impl(const char* path, const void* data, if (close(fd) != 0) ok = false; fd = -1; - if (ok && renameat(dirfd, tmp, dirfd, leaf) != 0) - ok = false; + if (ok) { + if (no_replace) { + /* The probe and commit cannot be one operation. A concurrent + creator may win; EEXIST is then the requested skip. */ + if (linkat(dirfd, tmp, dirfd, leaf, 0) == 0 || errno == EEXIST) { + if (unlinkat(dirfd, tmp, 0) != 0 && errno != ENOENT) + ok = false; + } else { + ok = false; + } + } else if (renameat(dirfd, tmp, dirfd, leaf) != 0) { + ok = false; + } + } if (!ok) unlinkat(dirfd, tmp, 0); } @@ -389,21 +411,27 @@ static bool file_to_disk_secure_impl(const char* path, const void* data, bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata) { - return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false, false); + return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false, false, false); } bool file_to_disk_secure_update(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata) { - return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, true, false); + return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, true, false, false); } bool file_to_disk_secure_with_fsync(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata, bool use_fsync) { - return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false, + return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false, false, use_fsync); } +bool file_to_disk_secure_no_replace(const char* path, const void* data, + unsigned long long data_size, bool sparse, + const FileMetadata* metadata) { + return file_to_disk_secure_impl(path, data, data_size, false, sparse, metadata, false, true, false); +} + bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse) { if (!path || (!data && data_size != 0) || has_path_traversal(path)) diff --git a/src/shared/file.h b/src/shared/file.h index 6de7eea..ad54fb9 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -41,5 +41,8 @@ bool file_to_disk_secure_with_fsync(const char* path, const void* data, an unavoidable final rename race without filesystem locking. */ bool file_to_disk_secure_update(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata); +bool file_to_disk_secure_no_replace(const char* path, const void* data, + unsigned long long data_size, bool sparse, + const FileMetadata* metadata); #endif diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index 9609885..90010d9 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -22,7 +22,9 @@ #define MAX_FILE_DATA_SIZE MAX_RECEIVE_FILE_SIZE bool file_save_to_disk(const char* root_directory, const File* file, const Config* config) { - bool backup_enabled = config && config->backup; + /* 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; bool inplace = config && config->inplace; bool sparse = config && config->preserve_sparse; const char* backup_suffix = (config && config->suffix) ? config->suffix : "~"; @@ -73,6 +75,19 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi free(disk_path); return true; } + + /* --ignore-existing checks the final destination before partial files or + overwrite policies can modify it. */ + if (config && config->ignore_existing) { + bool exists = file_path_exists_secure(destination_path); + if (exists) { + free(confined_backup); + free(confined_partial); + free(destination_path); + free(disk_path); + return true; + } + } free(destination_path); destination_path = NULL; @@ -115,12 +130,15 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi } } - bool ok = config && config->update - ? file_to_disk_secure_update(disk_path, file->data->data, file->data->size, inplace, - sparse, file->metadata) - : file_to_disk_secure_with_fsync(disk_path, file->data->data, file->data->size, - inplace, sparse, file->metadata, - config && config->use_fsync); + bool ok = config && config->ignore_existing + ? file_to_disk_secure_no_replace(disk_path, file->data->data, file->data->size, + sparse, file->metadata) + : config && config->update + ? file_to_disk_secure_update(disk_path, file->data->data, file->data->size, + inplace, sparse, file->metadata) + : file_to_disk_secure_with_fsync(disk_path, file->data->data, file->data->size, + inplace, sparse, file->metadata, + config && config->use_fsync); free(parent_copy); free(backup_path); free(confined_backup); diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index 8e80ece..68a5a92 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -497,6 +497,60 @@ class TestExisting: f.write(b"hello world\n") +class TestIgnoreExisting: + def test_ignore_existing_preserves_existing_and_transfers_new(self, shared_server): + clean_dir(DEST_DIR) + result, _ = run_client(SOURCE_DIR, DEST_DIR, port=shared_server.port) + assert result.returncode == 0 + + received = get_dest_received_dir(DEST_DIR, SOURCE_DIR) + existing_file = os.path.join(received, "small.txt") + with open(existing_file, "wb") as f: + f.write(b"destination content\n") + new_source = os.path.join(SOURCE_DIR, "new.txt") + try: + with open(new_source, "wb") as f: + f.write(b"new file\n") + + result, _ = run_client(SOURCE_DIR, DEST_DIR, + flags=["--ignore-existing"], port=shared_server.port) + assert result.returncode == 0, f"Sync failed: {(result.stderr or result.stdout)[:200]}" + with open(existing_file, "rb") as f: + assert f.read() == b"destination content\n" + with open(os.path.join(received, "new.txt"), "rb") as f: + assert f.read() == b"new file\n" + finally: + if os.path.lexists(new_source): + os.unlink(new_source) + + +class TestIgnoreExisting: + def test_ignore_existing_preserves_existing_and_transfers_new(self, shared_server): + clean_dir(DEST_DIR) + result, _ = run_client(SOURCE_DIR, DEST_DIR, port=shared_server.port) + assert result.returncode == 0 + + received = get_dest_received_dir(DEST_DIR, SOURCE_DIR) + existing_file = os.path.join(received, "small.txt") + with open(existing_file, "wb") as f: + f.write(b"destination content\n") + new_source = os.path.join(SOURCE_DIR, "new.txt") + try: + with open(new_source, "wb") as f: + f.write(b"new file\n") + + result, _ = run_client(SOURCE_DIR, DEST_DIR, + flags=["--ignore-existing"], port=shared_server.port) + assert result.returncode == 0, f"Sync failed: {(result.stderr or result.stdout)[:200]}" + with open(existing_file, "rb") as f: + assert f.read() == b"destination content\n" + with open(os.path.join(received, "new.txt"), "rb") as f: + assert f.read() == b"new file\n" + finally: + if os.path.lexists(new_source): + os.unlink(new_source) + + class TestDelete: def test_delete_removes_extra_files(self, shared_server): clean_dir(DEST_DIR) diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index a082461..a458cef 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -224,6 +224,19 @@ static void test_parse_args_size_only() { config_delete(cfg); } +static void test_parse_args_ignore_existing() { + Config* cfg = config_create(); + char* argv[] = {"fastsync", "--ignore-existing", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + + int ret = parse_args(cfg, 4, argv, positional_args, &positional_count); + EXPECT_EQ_INT(ret, 0); + EXPECT_TRUE(cfg->ignore_existing); + + config_delete(cfg); +} + /* Test parse_args rejects port > 65535 */ static void test_parse_args_invalid_port() { Config* cfg = config_create(); @@ -643,6 +656,7 @@ void test_client_cli() { test_parse_args_version(); test_parse_args_valid_port(); test_parse_args_size_only(); + test_parse_args_ignore_existing(); test_parse_args_invalid_port(); test_parse_args_non_numeric_port(); test_parse_args_invalid_server_port(); diff --git a/tests/test_config.c b/tests/test_config.c index e2fa306..efe989d 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -133,6 +133,7 @@ static void test_config_send_receive() { send_cfg->eight_bit_output = true; send_cfg->modify_window = 4; send_cfg->existing = true; + send_cfg->ignore_existing = true; /* Use socketpair for bidirectional communication */ int p[2]; @@ -181,6 +182,8 @@ static void test_config_send_receive() { ok = false; if (!recv_cfg->existing) ok = false; + if (!recv_cfg->ignore_existing) + ok = false; } config_delete(recv_cfg); close(p[0]); diff --git a/tests/test_file.c b/tests/test_file.c index 4fa793a..c5d7358 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -154,6 +154,99 @@ static void test_file_save_to_disk_existing() { config_delete(cfg); unlink(existing_path); + rmdir("test_existing_tmp"); +} + +static void test_file_save_to_disk_ignore_existing() { + const char* path = "test_ignore_existing_tmp/existing.txt"; + EXPECT_TRUE(file_write_to_disk(path, "old", 3, false, false)); + + File* file = file_create("existing.txt"); + EXPECT_NOT_NULL(file); + file->data->data = malloc(3); + EXPECT_NOT_NULL(file->data->data); + memcpy(file->data->data, "new", 3); + file->data->size = 3; + + Config* config = config_create(); + EXPECT_NOT_NULL(config); + config->ignore_existing = true; + EXPECT_TRUE(file_save_to_disk("test_ignore_existing_tmp", file, config)); + + FILE* stream = fopen(path, "rb"); + char content[4] = {0}; + EXPECT_NOT_NULL(stream); + // cppcheck-suppress knownConditionTrueFalse + if (stream) { + EXPECT_EQ_INT((int)fread(content, 1, 3, stream), 3); + fclose(stream); + } + EXPECT_EQ_STR(content, "old"); + + file_destroy(file); + config_delete(config); + unlink(path); + rmdir("test_ignore_existing_tmp"); +} + +static void test_file_save_to_disk_ignore_existing_entry_types() { + const char* root = "test_ignore_existing_entries_tmp"; + const char* directory = "test_ignore_existing_entries_tmp/directory"; + const char* link = "test_ignore_existing_entries_tmp/link"; + const char* target = "test_ignore_existing_entries_tmp/target"; + const char* backup = "test_ignore_existing_entries_tmp/backup.txt~"; + const char* backup_file = "test_ignore_existing_entries_tmp/backup.txt"; + Config* config = config_create(); + File* file = file_create("unused"); + + unlink(link); + unlink(target); + unlink(backup); + unlink(backup_file); + rmdir(directory); + rmdir(root); + EXPECT_NOT_NULL(config); + EXPECT_NOT_NULL(file); + // cppcheck-suppress knownConditionTrueFalse + if (!config || !file) + return; + config->ignore_existing = true; + config->backup = true; + file->data->data = malloc(3); + EXPECT_NOT_NULL(file->data->data); + // cppcheck-suppress knownConditionTrueFalse + if (!file->data->data) { + file_destroy(file); + config_delete(config); + return; + } + memcpy(file->data->data, "new", 3); + file->data->size = 3; + + EXPECT_EQ_INT(mkdir(root, 0755), 0); + EXPECT_EQ_INT(mkdir(directory, 0755), 0); + EXPECT_TRUE(file_write_to_disk(target, "old", 3, false, false)); + EXPECT_EQ_INT(symlink("target", link), 0); + free(file->path); + file->path = str_dup("directory"); + EXPECT_TRUE(file_save_to_disk(root, file, config)); + free(file->path); + file->path = str_dup("link"); + EXPECT_TRUE(file_save_to_disk(root, file, config)); + + free(file->path); + file->path = str_dup("backup.txt"); + EXPECT_TRUE(file_write_to_disk(backup_file, "old", 3, false, false)); + EXPECT_TRUE(file_save_to_disk(root, file, config)); + EXPECT_TRUE(file_path_exists_secure(backup_file)); + EXPECT_FALSE(file_path_exists_secure(backup)); + + file_destroy(file); + config_delete(config); + unlink(link); + unlink(target); + unlink(backup_file); + rmdir(directory); rmdir(root); } @@ -540,6 +633,8 @@ void test_file() { test_file_save_to_disk(); test_file_save_to_disk_with_fsync_config(); test_file_save_to_disk_existing(); + test_file_save_to_disk_ignore_existing(); + test_file_save_to_disk_ignore_existing_entry_types(); test_file_write_to_disk_basic(); test_file_write_to_disk_with_fsync(); test_file_write_to_disk_creates_dirs();