From 666a8ef7cdfcd941054b54cc4d225da382092c6d Mon Sep 17 00:00:00 2001 From: omrzk <193513049+omrzk@users.noreply.github.com> Date: Fri, 13 Nov 2026 09:20:18 -0600 Subject: [PATCH] Honor the symbolic link option when copying files Inspect source links without following them when COPYFILE_SYM is requested, before rejecting directories. This makes the documented option recreate link text instead of copying the referent, including dangling and directory links. Preserve requested link timestamps and report link-creation errors without overwriting the destination. Cover direct calls, destination directories, normal dereferencing and the existing rsync file caller. --- src/copyfile.c | 26 +++++---- src/rsync.c | 45 ++++++++++++++-- test/.gitignore | 1 + test/Makefile.am | 2 + test/copyfile-symlink.c | 114 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 174 insertions(+), 14 deletions(-) create mode 100644 test/copyfile-symlink.c diff --git a/src/copyfile.c b/src/copyfile.c index 2936daa..593f1a9 100644 --- a/src/copyfile.c +++ b/src/copyfile.c @@ -157,7 +157,13 @@ ssize_t copyfile(const char *src, const char *dst, int len, int opt) if (!buffer) return 0; - if (fisdir(src)) { + if (sym ? lstat(src, &st) : stat(src, &st)) { + saved_errno = errno; + size = -1; + goto exit; + } + + if (S_ISDIR(st.st_mode)) { saved_errno = EISDIR; /* Error: source is a directory */ goto exit; } @@ -165,13 +171,7 @@ ssize_t copyfile(const char *src, const char *dst, int len, int opt) /* Check if target is a directory, then append src filename. */ isdir = adjust_target(src, &dest); - /* Check if the source file is a symlink ... */ - if (stat(src, &st)) { - size = -1; - goto exit; - } - - /* ... only try readlink() if symlink and @sym is set. */ + /* Only try readlink() if symlink and @sym is set. */ if (sym && (st.st_mode & S_IFMT) == S_IFLNK) { size = readlink(src, buffer, BUFSIZ); if (size > 0) { @@ -181,8 +181,16 @@ ssize_t copyfile(const char *src, const char *dst, int len, int opt) } else { buffer[size] = 0; size = !symlink(buffer, dest); + if (!size) + saved_errno = errno; + else if (keep_mtim) { + struct timespec tv[2] = { st.st_atim, st.st_mtim }; + + utimensat(AT_FDCWD, dest, tv, AT_SYMLINK_NOFOLLOW); + } } - } + } else + saved_errno = errno; /* Don't fall back to copy, that is a potentially * dangerous race condition, see CWE-367. */ diff --git a/src/rsync.c b/src/rsync.c index 695fac3..47fd5fc 100644 --- a/src/rsync.c +++ b/src/rsync.c @@ -24,6 +24,7 @@ #include #include /* AT_* macros */ +#include /* dirname() */ #include /* NULL, free() */ #include /* strlen() */ #include /* rindex() */ @@ -159,15 +160,49 @@ int rsync(char *src, char *dst, int opt, int (*filter)(const char *file)) static int copy(char *src, char *dst, int opt) { + char *dest = NULL, *temp = NULL, *file = NULL; + int result = 1, saved_errno, made_temp = 0; + copyfile(src, dst, 0, opt | LITE_FOPT_COPYFILE_SYM); - if (errno) { - if (errno != EEXIST) + if (errno != EEXIST) + return errno ? 1 : 0; + + /* Replace an existing destination only after the new link is ready. */ + if (fisdir(dst)) { + char *base = strrchr(src, '/'); + + if (asprintf(&dest, "%s%s%s", dst, fisslashdir(dst) ? "" : "/", base ? base + 1 : src) < 0) + return 1; + } else { + dest = strdup(dst); + if (!dest) return 1; - - errno = 0; } + if (asprintf(&temp, "%s/.rsync-XXXXXX", dirname(strdupa(dest))) < 0) + goto exit; + if (!mkdtemp(temp)) + goto exit; + made_temp = 1; + if (asprintf(&file, "%s/link", temp) < 0) + goto exit; - return 0; + copyfile(src, file, 0, opt | LITE_FOPT_COPYFILE_SYM); + if (errno) + goto exit; + result = rename(file, dest) != 0; + +exit: + saved_errno = errno; + if (file) + unlink(file); + if (made_temp) + rmdir(temp); + free(file); + free(temp); + free(dest); + errno = saved_errno; + + return result; } /* Creates dir/name @mode ... skipping / if dir already ends so. */ diff --git a/test/.gitignore b/test/.gitignore index 7128afe..6701e84 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -5,6 +5,7 @@ /chomp /copyfile /fcopyfile +/copyfile-symlink /dir /fexist /fisdir diff --git a/test/Makefile.am b/test/Makefile.am index ef46bcc..36e1d2c 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -20,6 +20,7 @@ TESTS = TESTS += chomp TESTS += copyfile TESTS += fcopyfile +TESTS += copyfile-symlink TESTS += dir TESTS += fexist TESTS += fisdir @@ -41,3 +42,4 @@ TESTS += which TESTS += yorn check_PROGRAMS = $(TESTS) +copyfile_symlink_SOURCES = copyfile-symlink.c diff --git a/test/copyfile-symlink.c b/test/copyfile-symlink.c new file mode 100644 index 0000000..d4c55c6 --- /dev/null +++ b/test/copyfile-symlink.c @@ -0,0 +1,114 @@ +#include +#include +#include +#include "check.h" + +static void check_link(const char *path, const char *target) +{ + char buf[256]; + struct stat st; + ssize_t len; + + fail_unless(lstat(path, &st) == 0 && S_ISLNK(st.st_mode)); + len = readlink(path, buf, sizeof(buf)); + fail_unless(len == (ssize_t)strlen(target)); + fail_unless(memcmp(buf, target, len) == 0); +} + +int main(void) +{ + char tmp[] = "/tmp/libite-copylink-XXXXXX"; + const char payload[] = "target contents"; + char buf[sizeof(payload)]; + const char *targets[] = { "file", "dir", "missing", ".", "link" }; + struct timespec times[2] = { { 1234567800, 0 }, { 1234567801, 0 } }; + struct stat src_st, dst_st; + char cwd[4096]; + size_t i; + FILE *fp; + + fail_unless(getcwd(cwd, sizeof(cwd)) && mkdtemp(tmp)); + fail_unless(chdir(tmp) == 0); + fail_unless(mkdir("dir", 0700) == 0); + fail_unless(mkdir("out", 0700) == 0); + fp = fopen("file", "w"); + fail_unless(fp && fwrite(payload, 1, sizeof(payload), fp) == sizeof(payload)); + fail_unless(fclose(fp) == 0); + + for (i = 0; i < NELEMS(targets); i++) { + fail_unless(symlink(targets[i], "link") == 0); + fail_unless(copyfile("link", "copied", 0, LITE_FOPT_COPYFILE_SYM) == 1); + check_link("copied", targets[i]); + fail_unless(copyfile("link", "out", 0, LITE_FOPT_COPYFILE_SYM) == 1); + check_link("out/link", targets[i]); + /* Existing targets must survive a failed symlink creation. */ + errno = 0; + fail_unless(copyfile("link", "file", 0, LITE_FOPT_COPYFILE_SYM) == 0); + fail_unless(errno == EEXIST); + check_link("copied", targets[i]); + fail_unless(unlink("copied") == 0 && unlink("out/link") == 0); + fail_unless(unlink("link") == 0); + } + + fail_unless(symlink("file", "link") == 0); + /* Without SYM, continue to copy target data and preserve regular-file metadata. */ + fail_unless(copyfile("link", "copied", 0, LITE_FOPT_KEEP_MTIME) == sizeof(payload)); + fail_unless(lstat("copied", &dst_st) == 0 && S_ISREG(dst_st.st_mode)); + fail_unless(stat("file", &src_st) == 0 && src_st.st_mtime == dst_st.st_mtime); + fp = fopen("copied", "r"); + fail_unless(fp && fread(buf, 1, sizeof(buf), fp) == sizeof(payload)); + fail_unless(memcmp(buf, payload, sizeof(payload)) == 0 && fclose(fp) == 0); + fail_unless(unlink("copied") == 0); + + fail_unless(utimensat(AT_FDCWD, "link", times, AT_SYMLINK_NOFOLLOW) == 0); + fail_unless(copyfile("link", "copied", 0, LITE_FOPT_COPYFILE_SYM | LITE_FOPT_KEEP_MTIME) == 1); + check_link("copied", "file"); + fail_unless(lstat("copied", &dst_st) == 0 && dst_st.st_mtime == times[1].tv_sec); + fail_unless(unlink("copied") == 0); + /* Exercise the existing rsync file caller, which requests SYM. */ + fail_unless(rsync("link", "copied", 0, NULL) == 0); + check_link("copied", "file"); + /* Reuse the destination after changing the source link. */ + fail_unless(unlink("link") == 0 && symlink("file", "link") == 0); + fail_unless(rsync("link", "out", 0, NULL) == 0); + check_link("out/link", "file"); + fail_unless(unlink("link") == 0 && symlink("copied", "link") == 0); + fail_unless(rsync("link", "out", 0, NULL) == 0); + check_link("out/link", "copied"); + /* A same-path copy must not unlink its source. */ + fail_unless(rsync("link", "link", 0, NULL) == 0); + check_link("link", "copied"); + fail_unless(unlink("out/link") == 0); + /* Replacing a regular destination must also retain the source link. */ + fail_unless(touch("out/link") == 0); + fail_unless(rsync("link", "out", 0, NULL) == 0); + check_link("out/link", "copied"); + fail_unless(unlink("out/link") == 0); + /* A failed replacement must keep both the destination and source. */ + fail_unless(mkdir("out/link", 0700) == 0); + fail_unless(touch("out/link/keep") == 0); + fail_unless(rsync("link", "out", 0, NULL) != 0); + fail_unless(fexist("out/link/keep")); + check_link("link", "copied"); + fail_unless(unlink("out/link/keep") == 0 && rmdir("out/link") == 0); + if (getuid() != 0) { + fail_unless(mkdir("protected", 0700) == 0); + fail_unless(symlink("original", "protected/link") == 0); + fail_unless(chmod("protected", 0500) == 0); + fail_unless(rsync("link", "protected", 0, NULL) != 0); + check_link("protected/link", "original"); + check_link("link", "copied"); + fail_unless(chmod("protected", 0700) == 0); + fail_unless(unlink("protected/link") == 0 && rmdir("protected") == 0); + } + fail_unless(unlink("copied") == 0 && unlink("link") == 0); + + errno = 0; + fail_unless(copyfile("dir", "copied", 0, LITE_FOPT_COPYFILE_SYM) == 0 && errno == EISDIR); + errno = 0; + fail_unless(copyfile("missing", "copied", 0, LITE_FOPT_COPYFILE_SYM) == -1 && errno == ENOENT); + + fail_unless(unlink("file") == 0 && rmdir("dir") == 0 && rmdir("out") == 0); + fail_unless(chdir(cwd) == 0 && rmdir(tmp) == 0); + return 0; +}