fix: address ignore-existing review findings
CI / lint (pull_request) Successful in 12s
CI / sanitizers (address) (pull_request) Successful in 36s
CI / sanitizers (undefined) (pull_request) Successful in 36s
CI / fuzz-build (pull_request) Successful in 15s
CI / coverage (pull_request) Successful in 31s
CI / build-and-test (pull_request) Successful in 1m15s
CI / valgrind (pull_request) Successful in 33s
CI / lint (pull_request) Successful in 12s
CI / sanitizers (address) (pull_request) Successful in 36s
CI / sanitizers (undefined) (pull_request) Successful in 36s
CI / fuzz-build (pull_request) Successful in 15s
CI / coverage (pull_request) Successful in 31s
CI / build-and-test (pull_request) Successful in 1m15s
CI / valgrind (pull_request) Successful in 33s
This commit is contained in:
+1
-1
@@ -129,7 +129,7 @@ typedef struct Config {
|
|||||||
char* compress_choice;
|
char* compress_choice;
|
||||||
} Config;
|
} Config;
|
||||||
|
|
||||||
#define PROTOCOL_VERSION "2.2.0"
|
#define PROTOCOL_VERSION "2.3.0"
|
||||||
#define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024)
|
#define DEFAULT_CHUNK_SIZE (10 * 1024 * 1024)
|
||||||
|
|
||||||
Config* config_create(void);
|
Config* config_create(void);
|
||||||
|
|||||||
+37
-4
@@ -172,8 +172,17 @@ bool file_set_authorized_root(int fd, const char* canonical_path) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
bool file_path_exists_secure(const char* 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;
|
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) {
|
bool file_stat_secure(const char* path, struct stat* st) {
|
||||||
@@ -302,8 +311,9 @@ bool file_rename_secure(const char* old_path, const char* new_path) {
|
|||||||
return ok;
|
return ok;
|
||||||
}
|
}
|
||||||
|
|
||||||
bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
|
static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||||
bool inplace, bool sparse, const FileMetadata* metadata) {
|
unsigned long long data_size, bool inplace, bool sparse,
|
||||||
|
const FileMetadata* metadata, bool no_replace) {
|
||||||
char* leaf = NULL;
|
char* leaf = NULL;
|
||||||
int dirfd = file_open_secure_parent(path, &leaf, true);
|
int dirfd = file_open_secure_parent(path, &leaf, true);
|
||||||
if (dirfd < 0)
|
if (dirfd < 0)
|
||||||
@@ -334,8 +344,20 @@ bool file_to_disk_secure(const char* path, const void* data, unsigned long long
|
|||||||
if (close(fd) != 0)
|
if (close(fd) != 0)
|
||||||
ok = false;
|
ok = false;
|
||||||
fd = -1;
|
fd = -1;
|
||||||
if (ok && renameat(dirfd, tmp, dirfd, leaf) != 0)
|
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;
|
ok = false;
|
||||||
|
} else {
|
||||||
|
ok = false;
|
||||||
|
}
|
||||||
|
} else if (renameat(dirfd, tmp, dirfd, leaf) != 0) {
|
||||||
|
ok = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
if (!ok)
|
if (!ok)
|
||||||
unlinkat(dirfd, tmp, 0);
|
unlinkat(dirfd, tmp, 0);
|
||||||
}
|
}
|
||||||
@@ -347,6 +369,17 @@ bool file_to_disk_secure(const char* path, const void* data, unsigned long long
|
|||||||
return ok;
|
return ok;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
|
||||||
|
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, true);
|
||||||
|
}
|
||||||
|
|
||||||
bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size,
|
bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size,
|
||||||
bool inplace, bool sparse) {
|
bool inplace, bool sparse) {
|
||||||
if (!path || (!data && data_size != 0) || has_path_traversal(path))
|
if (!path || (!data && data_size != 0) || has_path_traversal(path))
|
||||||
|
|||||||
@@ -32,5 +32,8 @@ bool file_ensure_directory_secure(const char* path);
|
|||||||
bool file_rename_secure(const char* old_path, const char* new_path);
|
bool file_rename_secure(const char* old_path, const char* new_path);
|
||||||
bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
|
bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
|
||||||
bool inplace, bool sparse, const FileMetadata* metadata);
|
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
|
#endif
|
||||||
|
|||||||
@@ -118,8 +118,11 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
bool ok = file_to_disk_secure(disk_path, file->data->data, file->data->size, inplace, sparse,
|
bool ok = config && config->ignore_existing
|
||||||
file->metadata);
|
? file_to_disk_secure_no_replace(disk_path, file->data->data, file->data->size,
|
||||||
|
sparse, file->metadata)
|
||||||
|
: file_to_disk_secure(disk_path, file->data->data, file->data->size, inplace,
|
||||||
|
sparse, file->metadata);
|
||||||
free(parent_copy);
|
free(parent_copy);
|
||||||
free(backup_path);
|
free(backup_path);
|
||||||
free(confined_backup);
|
free(confined_backup);
|
||||||
|
|||||||
@@ -225,6 +225,7 @@ class TestIgnoreExisting:
|
|||||||
with open(existing_file, "wb") as f:
|
with open(existing_file, "wb") as f:
|
||||||
f.write(b"destination content\n")
|
f.write(b"destination content\n")
|
||||||
new_source = os.path.join(SOURCE_DIR, "new.txt")
|
new_source = os.path.join(SOURCE_DIR, "new.txt")
|
||||||
|
try:
|
||||||
with open(new_source, "wb") as f:
|
with open(new_source, "wb") as f:
|
||||||
f.write(b"new file\n")
|
f.write(b"new file\n")
|
||||||
|
|
||||||
@@ -235,6 +236,9 @@ class TestIgnoreExisting:
|
|||||||
assert f.read() == b"destination content\n"
|
assert f.read() == b"destination content\n"
|
||||||
with open(os.path.join(received, "new.txt"), "rb") as f:
|
with open(os.path.join(received, "new.txt"), "rb") as f:
|
||||||
assert f.read() == b"new file\n"
|
assert f.read() == b"new file\n"
|
||||||
|
finally:
|
||||||
|
if os.path.lexists(new_source):
|
||||||
|
os.unlink(new_source)
|
||||||
|
|
||||||
|
|
||||||
class TestDelete:
|
class TestDelete:
|
||||||
|
|||||||
+2
-2
@@ -188,11 +188,11 @@ static void test_config_send_receive() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void test_config_send_receive_version_mismatch() {
|
static void test_config_send_receive_version_mismatch() {
|
||||||
/* Create a config with a different protocol version */
|
/* A peer using the previous wire format must be rejected. */
|
||||||
Config* cfg = config_create();
|
Config* cfg = config_create();
|
||||||
EXPECT_NOT_NULL(cfg);
|
EXPECT_NOT_NULL(cfg);
|
||||||
free(cfg->version);
|
free(cfg->version);
|
||||||
cfg->version = str_dup("0.0");
|
cfg->version = str_dup("2.2.0");
|
||||||
cfg->send_directory = str_dup("/src");
|
cfg->send_directory = str_dup("/src");
|
||||||
cfg->receive_root_directory = str_dup("/dst");
|
cfg->receive_root_directory = str_dup("/dst");
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user