Merge feat/p2-temp-dir: implement --temp-dir
Receiver writes temps into a confined scratch dir and atomically renames into place; EXDEV aborts; inplace/partial bypass; thread-safe temp names. Uses existing wire field; no protocol bump. Reviewed (c-review APPROVE WITH NITS, all fixed); PR #262.
This commit is contained in:
+1
-1
@@ -97,7 +97,7 @@ This document maps rsync's full feature set to FastSync's current implementation
|
||||
| `--backup-dir=DIR` | Backup directory hierarchy | ✅ Implemented | `backup_dir` config field |
|
||||
| `--suffix=SUFFIX` | Backup suffix (default ~) | ✅ Implemented | `suffix` config field |
|
||||
| `--delay-updates` | Put updated files in place at end | ❌ Not Implemented | |
|
||||
| `-T`, `--temp-dir=DIR` | Create temporary files in DIR | ❌ Not Implemented | `-T` is FastSync's timeout alias |
|
||||
| `-T`, `--temp-dir=DIR` | Create temporary files in DIR | ✅ Implemented | `--temp-dir` only; `-T` stays FastSync's `--timeout` alias. Scratch dir is resolved under the receive root; temp copies use a unique name there and are atomically renamed into place. If the scratch dir and destination are on different filesystems the atomic rename fails with EXDEV and the file save fails, which aborts the whole transfer (FastSync has no per-file skip/resume on a save error; rsync's non-atomic copy fallback is deliberately not used). `--inplace` and `--partial-dir` writes bypass the scratch dir |
|
||||
|
||||
## 7. Deletion
|
||||
|
||||
|
||||
@@ -405,6 +405,7 @@ static const OptionEntry OPTION_TABLE[] = {
|
||||
{"--ca", NULL, OPT_STRING, offsetof(Config, tls_ca)},
|
||||
{"--backup-dir", NULL, OPT_STRING, offsetof(Config, backup_dir)},
|
||||
{"--fastsync-server-path", NULL, OPT_STRING, offsetof(Config, fastsync_server_path)},
|
||||
{"--temp-dir", NULL, OPT_STRING, offsetof(Config, temp_dir)},
|
||||
{"--partial-dir", NULL, OPT_STRING, offsetof(Config, partial_dir)},
|
||||
{"--suffix", NULL, OPT_STRING, offsetof(Config, suffix)},
|
||||
{"--compress-choice", "--zc", OPT_STRING, offsetof(Config, compress_choice)},
|
||||
|
||||
@@ -88,6 +88,7 @@ void print_usage(void) {
|
||||
printf(" --stderr=MODE Route logging to stderr: errors or all\n");
|
||||
printf(" --partial Keep partial files on interrupted transfer\n");
|
||||
printf(" --partial-dir <dir> Directory for partial files\n");
|
||||
printf(" --temp-dir <dir> Scratch dir for temp files before atomic install\n");
|
||||
printf(" --fastsync-server-path <path>\n");
|
||||
printf(" Path to fastsync-server on remote (default: fastsync-server)\n");
|
||||
printf(
|
||||
|
||||
+127
-19
@@ -2,6 +2,7 @@
|
||||
#include <fcntl.h>
|
||||
#include <libgen.h>
|
||||
#include <limits.h>
|
||||
#include <stdatomic.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
@@ -30,6 +31,17 @@ static bool write_all(int fd, const void* data, unsigned long long size) {
|
||||
return true;
|
||||
}
|
||||
|
||||
/* Process-wide counter for scratch temp names. A --temp-dir scratch directory
|
||||
is flat: different destinations that share a basename must never race onto
|
||||
the same temp name. Deriving the trailing number from a global atomic
|
||||
sequence keeps every temp name unique across the whole scratch directory
|
||||
even when several threads write concurrently, so the O_EXCL creation loop
|
||||
below almost never needs a retry. */
|
||||
static unsigned long long next_temp_sequence(void) {
|
||||
static atomic_ullong sequence;
|
||||
return atomic_fetch_add_explicit(&sequence, 1, memory_order_relaxed);
|
||||
}
|
||||
|
||||
bool file_checksum(File* file, uint64_t* checksum) {
|
||||
if (!file || !checksum || !file->data)
|
||||
return false;
|
||||
@@ -326,10 +338,36 @@ bool file_rename_secure(const char* old_path, const char* new_path) {
|
||||
return ok;
|
||||
}
|
||||
|
||||
/* Open the configured --temp-dir scratch directory, creating it (and any
|
||||
missing path components) on demand. scratch_path is expected to already be
|
||||
confined below the authorized root by the caller; file_open_secure_parent
|
||||
re-checks that confinement and rejects `..` components, so a scratch
|
||||
directory can never be created or opened outside the destination root.
|
||||
Returns an O_DIRECTORY|O_NOFOLLOW fd, or -1 on error. */
|
||||
static int file_open_scratch_dir(const char* scratch_path) {
|
||||
if (!scratch_path)
|
||||
return -1;
|
||||
char* leaf = NULL;
|
||||
int parent_fd = file_open_secure_parent(scratch_path, &leaf, true);
|
||||
if (parent_fd < 0)
|
||||
return -1;
|
||||
int fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
|
||||
if (fd < 0 && errno == ENOENT) {
|
||||
/* A scratch directory holds transient working copies only; keep it
|
||||
private (0700) so other users cannot race on temp names inside it. */
|
||||
if (mkdirat(parent_fd, leaf, 0700) == 0 || errno == EEXIST)
|
||||
fd = openat(parent_fd, leaf, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
|
||||
}
|
||||
close(parent_fd);
|
||||
free(leaf);
|
||||
return fd;
|
||||
}
|
||||
|
||||
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 preserve_executability,
|
||||
bool update, bool no_replace, bool use_fsync) {
|
||||
bool update, bool no_replace, bool use_fsync,
|
||||
const char* temp_dir) {
|
||||
char* leaf = NULL;
|
||||
int dirfd = file_open_secure_parent(path, &leaf, true);
|
||||
if (dirfd < 0)
|
||||
@@ -337,6 +375,8 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
int fd = -1;
|
||||
bool ok = false;
|
||||
if (inplace) {
|
||||
/* --inplace writes directly into the destination; a scratch --temp-dir
|
||||
does not apply and must never redirect these writes. */
|
||||
fd = openat(dirfd, leaf, O_WRONLY | O_CREAT | O_CLOEXEC | O_NOFOLLOW, 0644);
|
||||
if (fd >= 0) {
|
||||
struct stat destination_stat;
|
||||
@@ -372,7 +412,8 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
}
|
||||
}
|
||||
} else {
|
||||
char tmp[NAME_MAX];
|
||||
/* The --update newer-destination check runs first so a skipped file never
|
||||
creates an empty scratch directory behind it. */
|
||||
if (update && metadata) {
|
||||
/* This check protects the normal atomic path as far as possible. A
|
||||
concurrent replacement can still occur before the final rename. */
|
||||
@@ -384,11 +425,59 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
return true;
|
||||
}
|
||||
}
|
||||
for (unsigned int i = 0; i < 100 && !ok; ++i) {
|
||||
snprintf(tmp, sizeof(tmp), ".%s.tmp.%ld.%u", leaf, (long)getpid(), i);
|
||||
fd = openat(dirfd, tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0600);
|
||||
/* Scratch directory for the temporary working copy. When NULL the temp
|
||||
file is created in the destination directory, exactly as historically. */
|
||||
int scratch_dirfd = -1;
|
||||
if (temp_dir) {
|
||||
scratch_dirfd = file_open_scratch_dir(temp_dir);
|
||||
if (scratch_dirfd < 0) {
|
||||
int saved_errno = errno;
|
||||
log_message(LOG_LEVEL_ERROR, "could not open --temp-dir scratch directory '%s': %s",
|
||||
temp_dir, strerror(saved_errno));
|
||||
close(dirfd);
|
||||
free(leaf);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
/* Temp names can exceed NAME_MAX for basenames near the limit (leaf plus
|
||||
the ".tmp.<pid>.<n>" decoration); heap-size the buffer instead of
|
||||
truncating into a fixed array, which would silently collide in a flat
|
||||
scratch directory. The sizing sentinel is the widest value of each
|
||||
format. */
|
||||
int tmp_size;
|
||||
if (scratch_dirfd >= 0)
|
||||
tmp_size = snprintf(NULL, 0, ".%s.tmp.%ld.%llu", leaf, (long)getpid(), ULLONG_MAX);
|
||||
else
|
||||
tmp_size = snprintf(NULL, 0, ".%s.tmp.%ld.%u", leaf, (long)getpid(), 999U);
|
||||
if (tmp_size < 0) {
|
||||
if (scratch_dirfd >= 0)
|
||||
close(scratch_dirfd);
|
||||
close(dirfd);
|
||||
free(leaf);
|
||||
return false;
|
||||
}
|
||||
char* tmp = malloc((size_t)tmp_size + 1);
|
||||
if (!tmp) {
|
||||
if (scratch_dirfd >= 0)
|
||||
close(scratch_dirfd);
|
||||
close(dirfd);
|
||||
free(leaf);
|
||||
return false;
|
||||
}
|
||||
for (unsigned int i = 0; i < 100; ++i) {
|
||||
/* The temp name is created inside the scratch directory (when one is
|
||||
configured) and, on success, atomically renamed into the destination
|
||||
directory. In a shared scratch directory the atomic sequence number
|
||||
keeps the name unique even for destinations with a common basename. */
|
||||
if (scratch_dirfd >= 0)
|
||||
snprintf(tmp, (size_t)tmp_size + 1, ".%s.tmp.%ld.%llu", leaf, (long)getpid(),
|
||||
next_temp_sequence());
|
||||
else
|
||||
snprintf(tmp, (size_t)tmp_size + 1, ".%s.tmp.%ld.%u", leaf, (long)getpid(), i);
|
||||
fd = openat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp,
|
||||
O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0600);
|
||||
if (fd < 0)
|
||||
continue;
|
||||
continue; /* EEXIST (or a transient open error): try a fresh name. */
|
||||
if (sparse && data_size > 0)
|
||||
ok = ftruncate(fd, (off_t)data_size) == 0;
|
||||
if (ok || (!sparse || data_size == 0))
|
||||
@@ -404,19 +493,37 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
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)
|
||||
if (linkat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp, dirfd, leaf, 0) == 0 ||
|
||||
errno == EEXIST) {
|
||||
if (unlinkat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp, 0) != 0 &&
|
||||
errno != ENOENT)
|
||||
ok = false;
|
||||
} else {
|
||||
if (scratch_dirfd >= 0 && errno == EXDEV)
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"temp dir is on a different filesystem than the destination; cannot "
|
||||
"link file into place (EXDEV); no fallback copy is attempted");
|
||||
ok = false;
|
||||
}
|
||||
} else if (renameat(dirfd, tmp, dirfd, leaf) != 0) {
|
||||
} else if (renameat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp, dirfd, leaf) != 0) {
|
||||
if (scratch_dirfd >= 0 && errno == EXDEV)
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"temp dir is on a different filesystem than the destination; cannot "
|
||||
"atomically install file (EXDEV); no fallback copy is attempted");
|
||||
ok = false;
|
||||
}
|
||||
}
|
||||
if (!ok)
|
||||
unlinkat(dirfd, tmp, 0);
|
||||
unlinkat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp, 0);
|
||||
/* Once the temp fd was created the outcome is permanent: a write,
|
||||
metadata, fsync, close, linkat or renameat failure will not be fixed
|
||||
by retrying under a fresh name, so stop here. Only the open-failure
|
||||
path above retries a new name. */
|
||||
break;
|
||||
}
|
||||
free(tmp);
|
||||
if (scratch_dirfd >= 0)
|
||||
close(scratch_dirfd);
|
||||
}
|
||||
if (fd >= 0)
|
||||
close(fd);
|
||||
@@ -427,36 +534,37 @@ 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,
|
||||
bool preserve_executability) {
|
||||
bool preserve_executability, const char* temp_dir) {
|
||||
return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata,
|
||||
preserve_executability, false, false, false);
|
||||
preserve_executability, false, false, false, temp_dir);
|
||||
}
|
||||
|
||||
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 preserve_executability) {
|
||||
bool preserve_executability, const char* temp_dir) {
|
||||
return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata,
|
||||
preserve_executability, true, false, false);
|
||||
preserve_executability, true, false, false, temp_dir);
|
||||
}
|
||||
|
||||
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 preserve_executability,
|
||||
bool use_fsync) {
|
||||
bool use_fsync, const char* temp_dir) {
|
||||
return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata,
|
||||
preserve_executability, false, false, use_fsync);
|
||||
preserve_executability, false, false, use_fsync, temp_dir);
|
||||
}
|
||||
|
||||
bool file_to_disk_secure_no_replace(const char* path, const void* data,
|
||||
unsigned long long data_size, bool sparse,
|
||||
const FileMetadata* metadata, bool preserve_executability) {
|
||||
const FileMetadata* metadata, bool preserve_executability,
|
||||
const char* temp_dir) {
|
||||
return file_to_disk_secure_impl(path, data, data_size, false, sparse, metadata,
|
||||
preserve_executability, false, true, false);
|
||||
preserve_executability, false, true, false, temp_dir);
|
||||
}
|
||||
|
||||
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))
|
||||
return false;
|
||||
return file_to_disk_secure(path, data, data_size, inplace, sparse, NULL, false);
|
||||
return file_to_disk_secure(path, data, data_size, inplace, sparse, NULL, false, NULL);
|
||||
}
|
||||
|
||||
+15
-4
@@ -31,21 +31,32 @@ bool file_destination_is_newer_secure(const char* path, const FileMetadata* meta
|
||||
int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs);
|
||||
bool file_ensure_directory_secure(const char* path);
|
||||
bool file_rename_secure(const char* old_path, const char* new_path);
|
||||
|
||||
/* The file_to_disk_secure* variants write a temporary copy in the destination
|
||||
directory and atomically rename it over `path`. temp_dir is an absolute,
|
||||
root-confined scratch directory (already validated by the caller): when it
|
||||
is non-NULL the temporary copy is instead created there (with a name unique
|
||||
across the whole scratch directory) and atomically renamed into the
|
||||
destination directory once fully written and fsynced. A rename across
|
||||
filesystems (EXDEV) fails the write with an error; the file is never
|
||||
silently copied into place. Pass NULL for the historical same-directory
|
||||
behavior. --inplace writes never use temp_dir. */
|
||||
bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
|
||||
bool inplace, bool sparse, const FileMetadata* metadata,
|
||||
bool preserve_executability);
|
||||
bool preserve_executability, const char* temp_dir);
|
||||
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 preserve_executability,
|
||||
bool use_fsync);
|
||||
bool use_fsync, const char* temp_dir);
|
||||
/* With update enabled, an existing newer destination is left untouched. The
|
||||
check is descriptor-based for inplace writes; atomic replacement still has
|
||||
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 preserve_executability);
|
||||
bool preserve_executability, const char* temp_dir);
|
||||
bool file_to_disk_secure_no_replace(const char* path, const void* data,
|
||||
unsigned long long data_size, bool sparse,
|
||||
const FileMetadata* metadata, bool preserve_executability);
|
||||
const FileMetadata* metadata, bool preserve_executability,
|
||||
const char* temp_dir);
|
||||
|
||||
#endif
|
||||
|
||||
+38
-11
@@ -37,6 +37,7 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
const char* backup_suffix = (config && config->suffix) ? config->suffix : "~";
|
||||
const char* backup_dir = (config && config->backup_dir) ? config->backup_dir : NULL;
|
||||
const char* partial_dir = (config && config->partial_dir) ? config->partial_dir : NULL;
|
||||
const char* temp_dir = (config && config->temp_dir) ? config->temp_dir : NULL;
|
||||
bool use_partial_root = partial_dir && config && config->partial;
|
||||
char *confined_backup = NULL, *confined_partial = NULL, *disk_path = NULL;
|
||||
char* destination_path = NULL;
|
||||
@@ -52,9 +53,13 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
}
|
||||
|
||||
/* These options arrive from the client. They are names below the server
|
||||
root, never independent filesystem roots. */
|
||||
root, never independent filesystem roots. --temp-dir is confined exactly
|
||||
like --backup-dir/--partial-dir: an absolute or `..`-escaping scratch
|
||||
directory is rejected outright so nothing is ever created outside the
|
||||
authorized destination root. */
|
||||
if ((backup_dir && (backup_dir[0] == '/' || has_path_traversal(backup_dir))) ||
|
||||
(partial_dir && (partial_dir[0] == '/' || has_path_traversal(partial_dir))))
|
||||
(partial_dir && (partial_dir[0] == '/' || has_path_traversal(partial_dir))) ||
|
||||
(temp_dir && (temp_dir[0] == '/' || has_path_traversal(temp_dir))))
|
||||
return FILE_SAVE_ERROR;
|
||||
if (backup_dir && !(confined_backup = path_cat(root_directory, backup_dir)))
|
||||
return FILE_SAVE_ERROR;
|
||||
@@ -151,15 +156,37 @@ FileSaveResult file_save_to_disk_full(const char* root_directory, const File* fi
|
||||
goto fail;
|
||||
metadata = &adjusted_metadata;
|
||||
}
|
||||
bool ok = config && config->ignore_existing
|
||||
? file_to_disk_secure_no_replace(disk_path, file->data->data, file->data->size,
|
||||
sparse, metadata, preserve_executability)
|
||||
: config && config->update
|
||||
? file_to_disk_secure_update(disk_path, file->data->data, file->data->size, inplace,
|
||||
sparse, metadata, preserve_executability)
|
||||
: file_to_disk_secure_with_fsync(disk_path, file->data->data, file->data->size,
|
||||
inplace, sparse, metadata, preserve_executability,
|
||||
config && config->use_fsync);
|
||||
|
||||
/* A configured --temp-dir sends the temporary working copy to a scratch
|
||||
directory resolved below the receive root; the engine then atomically
|
||||
renames the completed file into the final destination directory. The
|
||||
partial-dir flow already keeps its working copy in a separate directory
|
||||
and --inplace writes directly, so neither diverts through the scratch
|
||||
dir (matching rsync, where --inplace/--partial-dir supersede --temp-dir). */
|
||||
char* confined_temp = NULL;
|
||||
bool use_temp_dir = temp_dir != NULL && !inplace && !use_partial_root;
|
||||
if (use_temp_dir) {
|
||||
confined_temp = path_cat(root_directory, temp_dir);
|
||||
if (!confined_temp)
|
||||
goto fail;
|
||||
/* A user-supplied trailing slash would leave the scratch path ending in
|
||||
"/", which has no final component to create/open. Normalize it away. */
|
||||
size_t temp_len = strlen(confined_temp);
|
||||
while (temp_len > 1 && confined_temp[temp_len - 1] == '/')
|
||||
confined_temp[--temp_len] = '\0';
|
||||
}
|
||||
bool ok =
|
||||
config && config->ignore_existing
|
||||
? file_to_disk_secure_no_replace(disk_path, file->data->data, file->data->size, sparse,
|
||||
metadata, preserve_executability, confined_temp)
|
||||
: config && config->update
|
||||
? file_to_disk_secure_update(disk_path, file->data->data, file->data->size, inplace,
|
||||
sparse, metadata, preserve_executability, confined_temp)
|
||||
: file_to_disk_secure_with_fsync(disk_path, file->data->data, file->data->size, inplace,
|
||||
sparse, metadata, preserve_executability,
|
||||
config && config->use_fsync, confined_temp);
|
||||
free(confined_temp);
|
||||
confined_temp = NULL;
|
||||
if (!ok)
|
||||
goto fail;
|
||||
|
||||
|
||||
@@ -1023,7 +1023,6 @@ class TestLargeFile:
|
||||
received = get_dest_received_dir(dest, source)
|
||||
assert filecmp.cmp(source_file, os.path.join(received, "big.bin"), shallow=False)
|
||||
|
||||
|
||||
class TestOneFileSystem:
|
||||
def _make_tree(self, source):
|
||||
clean_dir(source)
|
||||
@@ -1101,3 +1100,128 @@ class TestOneFileSystem:
|
||||
unmount_error = umount.stderr.strip()
|
||||
if unmount_error:
|
||||
pytest.fail(f"test mountpoint {mountpoint} still mounted after umount: {unmount_error}")
|
||||
|
||||
|
||||
def _walk_tmp_files(root):
|
||||
"""Recursively list *.tmp* leftovers under root (empty if root missing)."""
|
||||
leftovers = []
|
||||
if not os.path.isdir(root):
|
||||
return leftovers
|
||||
for base, _, files in os.walk(root):
|
||||
for name in files:
|
||||
if ".tmp." in name:
|
||||
leftovers.append(os.path.join(base, name))
|
||||
return leftovers
|
||||
|
||||
|
||||
class TestTempDir:
|
||||
"""--temp-dir=DIR puts the receiver's temporary working copies in a scratch
|
||||
directory below the destination root and atomically renames each completed
|
||||
file into its final destination. Files sharing a basename across
|
||||
directories exercise the flat scratch namespace."""
|
||||
|
||||
def _make_source(self, name):
|
||||
source = os.path.join(TEST_DATA_DIR, name)
|
||||
clean_dir(source)
|
||||
entries = {
|
||||
"top.txt": b"top level\n",
|
||||
"sub/file.txt": b"nested file\n" * 20,
|
||||
"other/file.txt": b"other nested file\n",
|
||||
"sub/deep.bin": bytes(range(256)) * 8,
|
||||
}
|
||||
for rel, content in entries.items():
|
||||
full = os.path.join(source, rel)
|
||||
os.makedirs(os.path.dirname(full), exist_ok=True)
|
||||
with open(full, "wb") as fh:
|
||||
fh.write(content)
|
||||
return source
|
||||
|
||||
def _assert_clean_scratch(self, scratch):
|
||||
assert os.path.isdir(scratch), f"scratch dir {scratch} was not created"
|
||||
leftovers = _walk_tmp_files(scratch)
|
||||
assert leftovers == [], f"leftover temp files in scratch dir: {leftovers}"
|
||||
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_temp_dir_scratch(self, shared_server, mt):
|
||||
source = self._make_source("tempdir_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "tempdir_dst")
|
||||
clean_dir(dest)
|
||||
flags = ["--temp-dir=scratch"] + (["-m"] if mt else [])
|
||||
result, _ = run_client(source, dest, flags=flags, port=shared_server.port)
|
||||
assert result.returncode == 0, f"temp-dir sync failed: {result.stderr[:200]}"
|
||||
received = get_dest_received_dir(dest, source)
|
||||
mismatches, missing = verify_transfer(source, received)
|
||||
assert not missing, f"Missing: {missing}"
|
||||
assert not mismatches, f"Mismatch: {mismatches}"
|
||||
self._assert_clean_scratch(os.path.join(dest, "scratch"))
|
||||
|
||||
def test_default_behavior_has_no_scratch_dir(self, shared_server):
|
||||
source = self._make_source("tempdir_default_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "tempdir_default_dst")
|
||||
clean_dir(dest)
|
||||
result, _ = run_client(source, dest, port=shared_server.port)
|
||||
assert result.returncode == 0, f"Default sync failed: {result.stderr[:200]}"
|
||||
received = get_dest_received_dir(dest, source)
|
||||
mismatches, missing = verify_transfer(source, received)
|
||||
assert not missing, f"Missing: {missing}"
|
||||
assert not mismatches, f"Mismatch: {mismatches}"
|
||||
assert not os.path.exists(os.path.join(dest, "scratch"))
|
||||
|
||||
def test_temp_dir_ignored_with_inplace(self, shared_server):
|
||||
"""--inplace writes directly into the destination; --temp-dir must not
|
||||
redirect those writes into a scratch dir."""
|
||||
source = self._make_source("tempdir_inplace_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "tempdir_inplace_dst")
|
||||
clean_dir(dest)
|
||||
result, _ = run_client(source, dest,
|
||||
flags=["--inplace", "--temp-dir=scratch"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode == 0, f"inplace+temp-dir sync failed: {result.stderr[:200]}"
|
||||
received = get_dest_received_dir(dest, source)
|
||||
mismatches, missing = verify_transfer(source, received)
|
||||
assert not missing, f"Missing: {missing}"
|
||||
assert not mismatches, f"Mismatch: {mismatches}"
|
||||
assert not os.path.exists(os.path.join(dest, "scratch")), \
|
||||
"--inplace wrote through the scratch dir"
|
||||
|
||||
def test_temp_dir_ignored_with_partial_dir(self, shared_server):
|
||||
"""--partial --partial-dir already stages in a separate directory;
|
||||
--temp-dir must not be used on top of it."""
|
||||
source = self._make_source("tempdir_partial_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "tempdir_partial_dst")
|
||||
clean_dir(dest)
|
||||
result, _ = run_client(source, dest,
|
||||
flags=["--partial", "--partial-dir", ".partial",
|
||||
"--temp-dir=scratch"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode == 0, f"partial+temp-dir sync failed: {result.stderr[:200]}"
|
||||
received = get_dest_received_dir(dest, source)
|
||||
mismatches, missing = verify_transfer(source, received)
|
||||
assert not missing, f"Missing: {missing}"
|
||||
assert not mismatches, f"Mismatch: {mismatches}"
|
||||
partial = os.path.join(dest, ".partial",
|
||||
os.path.relpath(os.path.join(source, "top.txt"), os.path.sep))
|
||||
assert not os.path.exists(partial), "completed file remained under the partial dir"
|
||||
assert not os.path.exists(os.path.join(dest, "scratch")), \
|
||||
"--partial-dir wrote through the scratch dir"
|
||||
|
||||
def test_temp_dir_escape_rejected(self, shared_server):
|
||||
source = self._make_source("tempdir_escape_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "tempdir_escape_dst")
|
||||
clean_dir(dest)
|
||||
# "../escape" would resolve one level above the destination root.
|
||||
outside = os.path.join(TEST_DATA_DIR, "escape")
|
||||
assert not os.path.lexists(outside)
|
||||
|
||||
result, _ = run_client(source, dest, flags=["--temp-dir=../escape"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode != 0, "relative escaping --temp-dir was not rejected"
|
||||
assert not os.path.lexists(outside), "file created outside the destination root"
|
||||
|
||||
clean_dir(dest)
|
||||
abs_escape = os.path.join(TEST_DATA_DIR, "abs_escape_probe")
|
||||
assert not os.path.lexists(abs_escape)
|
||||
result, _ = run_client(source, dest, flags=["--temp-dir", abs_escape],
|
||||
port=shared_server.port)
|
||||
assert result.returncode != 0, "absolute --temp-dir was not rejected"
|
||||
assert not os.path.lexists(abs_escape), "file created outside the destination root"
|
||||
|
||||
+29
-1
@@ -620,7 +620,6 @@ static void test_parse_args_rejects_unimplemented_options() {
|
||||
"-e",
|
||||
"--rsh",
|
||||
"--rsync-path",
|
||||
"--temp-dir",
|
||||
"--compare-dest",
|
||||
"--copy-dest",
|
||||
"--link-dest",
|
||||
@@ -840,6 +839,34 @@ static void test_parse_args_checksum_choice_requires_value() {
|
||||
}
|
||||
}
|
||||
|
||||
/* --temp-dir accepts both the "--temp-dir=DIR" and "--temp-dir DIR" forms. */
|
||||
static void test_parse_args_temp_dir() {
|
||||
Config* cfg = config_create();
|
||||
char* equals_argv[] = {"fastsync", "--temp-dir=scratch", "/src", "/dst"};
|
||||
int positional_args[2];
|
||||
int positional_count = 0;
|
||||
|
||||
EXPECT_EQ_INT(parse_args(cfg, 4, equals_argv, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_STR(cfg->temp_dir, "scratch");
|
||||
EXPECT_EQ_INT(positional_count, 2);
|
||||
config_delete(cfg);
|
||||
|
||||
cfg = config_create();
|
||||
char* space_argv[] = {"fastsync", "--temp-dir", "scratch/sub", "/src", "/dst"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 5, space_argv, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_STR(cfg->temp_dir, "scratch/sub");
|
||||
EXPECT_EQ_INT(positional_count, 2);
|
||||
config_delete(cfg);
|
||||
|
||||
/* A value-taking option may not be passed without a value. */
|
||||
cfg = config_create();
|
||||
char* missing_argv[] = {"fastsync", "--temp-dir"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 2, missing_argv, positional_args, &positional_count), -1);
|
||||
config_delete(cfg);
|
||||
}
|
||||
|
||||
static void test_parse_args_old_args() {
|
||||
Config* cfg = config_create();
|
||||
char* argv[] = {"fastsync", "--old-args", "/src", "/dst"};
|
||||
@@ -1255,4 +1282,5 @@ void test_client_cli() {
|
||||
test_parse_args_partial_progress();
|
||||
test_parse_args_checksum_choice_aliases();
|
||||
test_parse_args_checksum_choice_requires_value();
|
||||
test_parse_args_temp_dir();
|
||||
}
|
||||
|
||||
@@ -381,6 +381,30 @@ static void test_config_string_null_vs_empty_roundtrip() {
|
||||
config_delete(c);
|
||||
}
|
||||
|
||||
/* A --temp-dir value must survive config_send/config_receive unchanged on the
|
||||
receive side (round-trips through the resume-options wire block). */
|
||||
static void test_config_temp_dir_roundtrip() {
|
||||
if (is_running_under_valgrind())
|
||||
return;
|
||||
Config* c = config_create();
|
||||
EXPECT_NOT_NULL(c);
|
||||
c->send_directory = str_dup("/src");
|
||||
c->receive_root_directory = str_dup("/dst");
|
||||
c->temp_dir = str_dup("scratch");
|
||||
EXPECT_TRUE(roundtrip_config_ok(c));
|
||||
config_delete(c);
|
||||
|
||||
/* An empty-STRING wire value is canonicalized back to NULL (never an empty
|
||||
scratch-dir name). */
|
||||
c = config_create();
|
||||
EXPECT_NOT_NULL(c);
|
||||
c->send_directory = str_dup("/src");
|
||||
c->receive_root_directory = str_dup("/dst");
|
||||
c->temp_dir = str_dup("");
|
||||
EXPECT_TRUE(roundtrip_config_ok(c));
|
||||
config_delete(c);
|
||||
}
|
||||
|
||||
static void test_config_is_remote_dest() {
|
||||
/* Valid SSH-style destinations */
|
||||
EXPECT_TRUE(config_is_remote_dest("user@host:/path"));
|
||||
@@ -415,6 +439,7 @@ void test_config() {
|
||||
test_config_send_receive_version_mismatch();
|
||||
test_config_receive_truncated();
|
||||
test_config_string_null_vs_empty_roundtrip();
|
||||
test_config_temp_dir_roundtrip();
|
||||
}
|
||||
test_config_is_remote_dest();
|
||||
}
|
||||
|
||||
+1
-1
@@ -381,7 +381,7 @@ static void test_file_write_to_disk_with_fsync() {
|
||||
const char* path = "test_file_write_to_disk_fsync.txt";
|
||||
const char* content = "fsync file content";
|
||||
EXPECT_TRUE(file_to_disk_secure_with_fsync(path, content, strlen(content), false, false, NULL,
|
||||
false, true));
|
||||
false, true, NULL));
|
||||
struct stat st;
|
||||
EXPECT_EQ_INT(stat(path, &st), 0);
|
||||
EXPECT_EQ_INT((int)st.st_size, (int)strlen(content));
|
||||
|
||||
Reference in New Issue
Block a user