From d21fa49a6704324232902085c7fe9383bc432013 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Wed, 11 Nov 2026 22:21:56 +1100 Subject: [PATCH 1/9] io: handle stream read errors --- clientserver.c | 45 ++++++++- exclude.c | 38 ++++++-- params.c | 59 ++++++++---- testsuite/stdio-error.c | 126 +++++++++++++++++++++++++ testsuite/stdio-read-errors_test.py | 139 ++++++++++++++++++++++++++++ 5 files changed, 377 insertions(+), 30 deletions(-) create mode 100644 testsuite/stdio-error.c create mode 100644 testsuite/stdio-read-errors_test.py diff --git a/clientserver.c b/clientserver.c index 586cc05bf..c547b65f3 100644 --- a/clientserver.c +++ b/clientserver.c @@ -177,6 +177,21 @@ int start_socket_client(char *host, int remote_argc, char *remote_argv[], return ret ? ret : client_run(fd, fd, -1, argc, argv); } +static size_t fread_retry(void *buffer, size_t length, FILE *file) +{ + size_t total = fread(buffer, 1, length, file); + while (total < length && ferror(file) && errno == EINTR) { + clearerr(file); + /* Preserve partial input across EINTR. Clang 21 does not model + * the recoverable position after an interrupted stream read. */ +#ifdef __clang_analyzer__ + __attribute__((suppress)) +#endif + total += fread((char *)buffer + total, 1, length - total, file); + } + return total; +} + static int exchange_protocols(int f_in, int f_out, char *buf, size_t bufsiz, int am_client) { int remote_sub = -1; @@ -191,9 +206,20 @@ static int exchange_protocols(int f_in, int f_out, char *buf, size_t bufsiz, int * symlinks not owned by uid 0 or our euid. */ int motd_fd = open_no_attacker_symlinks(motd, O_RDONLY, 0); FILE *f = motd_fd >= 0 ? fdopen(motd_fd, "r") : NULL; - if (!f && motd_fd >= 0) close(motd_fd); + if (!f && motd_fd >= 0) { + int fdopen_errno = errno; + close(motd_fd); + rsyserr(FLOG, fdopen_errno, "failed to open motd file %s", motd); + return -1; + } while (f && !feof(f)) { - int len = fread(buf, 1, bufsiz - 1, f); + int len = fread_retry(buf, bufsiz - 1, f); + if (ferror(f)) { + int read_errno = errno; + fclose(f); + rsyserr(FLOG, read_errno, "failed to read motd file %s", motd); + return -1; + } if (len > 0) write_buf(f_out, buf, len); } @@ -306,7 +332,11 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char * our euid anywhere in the path. */ int ei_fd = open_no_attacker_symlinks(early_input_file, O_RDONLY, 0); FILE *f = ei_fd >= 0 ? fdopen(ei_fd, "rb") : NULL; - if (!f && ei_fd >= 0) close(ei_fd); + if (!f && ei_fd >= 0) { + int fdopen_errno = errno; + close(ei_fd); + errno = fdopen_errno; + } if (!f || do_fstat(fileno(f), &st) < 0) { rsyserr(FERROR, errno, "failed to open %s", early_input_file); if (f) @@ -331,7 +361,14 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char free(modname); return -1; } - len = fread(line, 1, early_input_len, f); + len = fread_retry(line, early_input_len, f); + if (ferror(f)) { + int read_errno = errno; + fclose(f); + free(modname); + rsyserr(FERROR, read_errno, "failed to read early-input file %s", early_input_file); + return -1; + } if (len > 0) { write_buf(f_out, line, len); early_input_len -= len; diff --git a/exclude.c b/exclude.c index 7476f6a87..47ae8c143 100644 --- a/exclude.c +++ b/exclude.c @@ -1598,6 +1598,21 @@ void parse_filter_str(filter_rule_list *listp, const char *rulestr, } } +static int get_filter_char(FILE *file) +{ + int c = getc(file); + while (c == EOF && ferror(file) && errno == EINTR) { + clearerr(file); + /* EINTR leaves the unread byte in place, including on pipes. + * Clang 21's stream model does not recognise this recovery. */ +#ifdef __clang_analyzer__ + __attribute__((suppress)) +#endif + c = getc(file); + } + return c; +} + void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_rule *template, int xflags) { FILE *fp; @@ -1685,8 +1700,11 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_ if (fd < 0) fp = NULL; - else if (!(fp = fdopen(fd, "rb"))) + else if (!(fp = fdopen(fd, "rb"))) { + int fdopen_errno = errno; close(fd); + errno = fdopen_errno; + } } else fp = stdin; @@ -1762,11 +1780,7 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_ if (pending != EOF) { /* a CR lookahead we could not push back */ ch = pending; pending = EOF; - } else if ((ch = getc(fp)) == EOF) { - if (ferror(fp) && errno == EINTR) { - clearerr(fp); - continue; - } + } else if ((ch = get_filter_char(fp)) == EOF) { break; } if (word_split && isspace(ch)) @@ -1774,9 +1788,7 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_ if (eol_nulls? !ch : (ch == '\n' || ch == '\r')) { if (ch == '\r') { /* CRLF is one line, not two */ int nxt; - while ((nxt = getc(fp)) == EOF - && ferror(fp) && errno == EINTR) - clearerr(fp); + nxt = get_filter_char(fp); if (nxt == EOF) { if (!ferror(fp)) ch = EOF; /* real end of file */ @@ -1796,6 +1808,14 @@ void parse_filter_file(filter_rule_list *listp, const char *fname, const filter_ else overflow = 1; } + if (ferror(fp)) { + if (named_by_file) + rprintf(FERROR, "failed to read filter file %s\n", rule_text(template, src_name)); + else + rsyserr(FERROR, errno, "failed to read filter file %s", src_name); + fclose(fp); + exit_cleanup(RERR_FILEIO); + } if (overflow) { rprintf(FERROR, "discarding over-long filter: %s\n", rule_text_len(NULL, line, 0)); diff --git a/params.c b/params.c index 933db7cb1..7849079bb 100644 --- a/params.c +++ b/params.c @@ -101,6 +101,16 @@ static BOOL (*the_pfunc)(char *, char *); * Functions... */ +static int get_char(FILE *file) +{ + int c; + if (feof(file) || ferror(file)) + return EOF; + while ((c = getc(file)) == EOF && ferror(file) && errno == EINTR) + clearerr(file); + return c; +} + static int EatWhitespace( FILE *InFile ) /* ------------------------------------------------------------------------ ** * Scan past whitespace (see ctype(3C)) and return the first non-whitespace @@ -121,7 +131,7 @@ static int EatWhitespace( FILE *InFile ) { int c; - for( c = getc( InFile ); isspace( c ) && ('\n' != c); c = getc( InFile ) ) + for( c = get_char( InFile ); isspace( c ) && ('\n' != c); c = get_char( InFile ) ) ; return( c ); } /* EatWhitespace */ @@ -146,7 +156,7 @@ static int EatComment( FILE *InFile ) { int c; - for( c = getc( InFile ); ('\n'!=c) && (EOF!=c) && (c>0); c = getc( InFile ) ) + for( c = get_char( InFile ); ('\n'!=c) && (EOF!=c) && (c>0); c = get_char( InFile ) ) ; return( c ); } /* EatComment */ @@ -239,7 +249,7 @@ static BOOL Section( FILE *InFile, BOOL (*sfunc)(char *) ) return( False ); } end = ( (i > 0) && (' ' == bufr[i - 1]) ) ? (i - 1) : (i); - c = getc( InFile ); /* Continue with next line. */ + c = get_char( InFile ); /* Continue with next line. */ break; default: /* All else are a valid name chars. */ @@ -253,12 +263,14 @@ static BOOL Section( FILE *InFile, BOOL (*sfunc)(char *) ) { bufr[i++] = c; end = i; - c = getc( InFile ); + c = get_char( InFile ); } } } /* We arrive here if we've met the EOF before the closing bracket. */ + if (ferror(InFile)) + return False; rprintf(FLOG, "%s Unexpected EOF in the config file: %s\n", func, bufr ); return( False ); } /* Section */ @@ -326,11 +338,13 @@ static BOOL Parameter( FILE *InFile, BOOL (*pfunc)(char *, char *), int c ) return( True ); } end = ( (i > 0) && (' ' == bufr[i - 1]) ) ? (i - 1) : (i); - c = getc( InFile ); /* Read past eoln. */ + c = get_char( InFile ); /* Read past eoln. */ break; case '\0': /* Shouldn't have EOF within param name. */ case EOF: + if (ferror(InFile)) + return False; bufr[i] = '\0'; rprintf(FLOG, "%s Unexpected end-of-file at: %s\n", func, bufr ); return( True ); @@ -359,7 +373,7 @@ static BOOL Parameter( FILE *InFile, BOOL (*pfunc)(char *, char *), int c ) { bufr[i++] = c; end = i; - c = getc( InFile ); + c = get_char( InFile ); } } } @@ -377,7 +391,7 @@ static BOOL Parameter( FILE *InFile, BOOL (*pfunc)(char *, char *), int c ) switch( c ) { case '\r': /* Explicitly remove '\r' because the older */ - c = getc( InFile ); /* version called fgets_slash() which also */ + c = get_char( InFile ); /* version called fgets_slash() which also */ break; /* removes them. */ case '\n': /* Marks end of value unless there's a '\'. */ @@ -388,7 +402,7 @@ static BOOL Parameter( FILE *InFile, BOOL (*pfunc)(char *, char *), int c ) { for( end = i; end >= 0 && isSpace(bufr + end); end-- ) ; - c = getc( InFile ); + c = get_char( InFile ); } break; @@ -396,12 +410,14 @@ static BOOL Parameter( FILE *InFile, BOOL (*pfunc)(char *, char *), int c ) bufr[i++] = c; /* not advance . This allows trimming */ if( !isspace( c ) ) /* of whitespace at the end of the line. */ end = i; - c = getc( InFile ); + c = get_char( InFile ); break; } } bufr[end] = '\0'; /* End of value. */ + if (ferror(InFile)) + return False; return( pfunc( bufr, &bufr[vstart] ) ); /* Pass name & value to pfunc(). */ } /* Parameter */ @@ -586,7 +602,11 @@ static FILE *OpenConfFile( char *FileName ) int cfg_fd = open_no_attacker_symlinks( FileName, O_RDONLY, 0 ); OpenedFile = cfg_fd >= 0 ? fdopen( cfg_fd, "r" ) : NULL; if( !OpenedFile && cfg_fd >= 0 ) + { + int fdopen_errno = errno; close( cfg_fd ); + errno = fdopen_errno; + } } if( NULL == OpenedFile ) { @@ -617,21 +637,26 @@ int pm_process( char *FileName, { int result; FILE *InFile; + BOOL own_buffer = bufr == NULL; char *func = "params.c:pm_process() -"; InFile = OpenConfFile( FileName ); /* Open the config file. */ if( NULL == InFile ) return( False ); - if( NULL != bufr ) /* If we already have a buffer */ - result = Parse( InFile, sfunc, pfunc ); /* (recursive call), then just */ - /* use it. */ - - else /* If we don't have a buffer */ - { /* allocate one, then parse, */ - bSize = BUFR_INC; /* then free. */ + if (own_buffer) + { + bSize = BUFR_INC; bufr = new_array( char, bSize ); - result = Parse( InFile, sfunc, pfunc ); + } + result = Parse( InFile, sfunc, pfunc ); + if (ferror(InFile)) + { + rsyserr(FLOG, errno, "failed to read config file \"%s\"", FileName); + result = 0; + } + if (own_buffer) + { free( bufr ); bufr = NULL; bSize = 0; diff --git a/testsuite/stdio-error.c b/testsuite/stdio-error.c new file mode 100644 index 000000000..3b0750e2e --- /dev/null +++ b/testsuite/stdio-error.c @@ -0,0 +1,126 @@ +#define _GNU_SOURCE +#define _FILE_OFFSET_BITS 64 +#include +#include +#include +#include +#include +#include +#include +#include + +struct reader { + int fd; + long left; + FILE *stream; + struct reader *next; +}; + +static struct reader *readers; +static int failed_fd = -1; + +static int close_fd(int fd) +{ + int (*original)(int) = dlsym(RTLD_NEXT, "close"); + return original(fd); +} + +int close(int fd) +{ + int result = close_fd(fd); + if (fd == failed_fd) { + failed_fd = -1; + errno = EBADF; + } + return result; +} + +static ssize_t read_cookie(void *cookie, char *buffer, size_t length) +{ + struct reader *reader = cookie; + const char *mode = getenv("RSYNC_TEST_READ_ERROR"); + ssize_t result; + if (reader->left == 0) { + int marker = open(getenv("RSYNC_TEST_READ_MARKER"), O_WRONLY | O_CREAT | O_APPEND, 0600); + if (marker >= 0) { + (void)write(marker, "x", 1); + close_fd(marker); + } + if (strcmp(mode, "eof") == 0) + return 0; + if (strcmp(mode, "eintr") == 0) { + reader->left = -1; + errno = EINTR; + } else + errno = EIO; + return -1; + } + if (reader->left > 0 && length > (size_t)reader->left) + length = reader->left; + result = read(reader->fd, buffer, length); + if (result > 0 && reader->left > 0) + reader->left -= result; + return result; +} + +static int close_cookie(void *cookie) +{ + struct reader *reader = cookie, **entry = &readers; + int result = close_fd(reader->fd); + while (*entry && *entry != reader) + entry = &(*entry)->next; + if (*entry) + *entry = reader->next; + free(reader); + return result; +} + +FILE *fdopen(int fd, const char *open_mode) +{ + FILE *(*original)(int, const char *) = dlsym(RTLD_NEXT, "fdopen"); + const char *path = getenv("RSYNC_TEST_READ_PATH"); + const char *error_mode = getenv("RSYNC_TEST_READ_ERROR"); + struct stat actual, target; + struct reader *reader; + FILE *stream; + cookie_io_functions_t functions = {.read = read_cookie, .close = close_cookie}; + if (!path || fstat(fd, &actual) < 0 || stat(path, &target) < 0 + || actual.st_dev != target.st_dev || actual.st_ino != target.st_ino) + return original(fd, open_mode); + if (strcmp(error_mode, "fdopen") == 0) { + int marker = open(getenv("RSYNC_TEST_READ_MARKER"), O_WRONLY | O_CREAT | O_APPEND, 0600); + if (marker >= 0) { + (void)write(marker, "x", 1); + close_fd(marker); + } + failed_fd = fd; + errno = EMFILE; + return NULL; + } + reader = calloc(1, sizeof *reader); + if (!reader) + return NULL; + reader->fd = fd; + reader->left = strtol(getenv("RSYNC_TEST_READ_AFTER"), NULL, 10); + stream = fopencookie(reader, open_mode, functions); + if (!stream) { + free(reader); + return NULL; + } + reader->stream = stream; + reader->next = readers; + readers = reader; + return stream; +} + +/* Keep the backing descriptor available to early-input's fstat(). */ +int fileno(FILE *stream) +{ + struct reader *reader; + int (*original)(FILE *) = dlsym(RTLD_NEXT, "fileno"); + for (reader = readers; reader; reader = reader->next) { + if (reader->stream == stream) + return reader->fd; + } + return original(stream); +} diff --git a/testsuite/stdio-read-errors_test.py b/testsuite/stdio-read-errors_test.py new file mode 100644 index 000000000..8b10993a0 --- /dev/null +++ b/testsuite/stdio-read-errors_test.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python3 + +import errno +import os +import platform +import shlex +import signal +import subprocess +import sys + +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, rsync_argv, start_test_daemon, test_fail, test_skipped + + +def test(): + if platform.system() != 'Linux' or os.environ.get('ASAN_OPTIONS'): + test_skipped('stdio fault injection needs Linux without ASan') + library = SCRATCHDIR / 'stdio-error.so' + fixture = SRCDIR / 'testsuite/stdio-error.c' + build = subprocess.run(['cc', '-shared', '-fPIC', '-o', str(library), str(fixture), '-ldl'], capture_output=True, text=True) + if build.returncode: + test_fail(f'cannot build stdio hook: {build.stderr}') + FROMDIR.mkdir() + (FROMDIR / 'keep').write_text('keep\n') + (FROMDIR / 'drop').write_text('drop\n') + config = SCRATCHDIR / 'rsyncd.conf' + config_text = f'use chroot = no\n[files]\npath = {FROMDIR}\n' + config.write_text(config_text) + url = start_test_daemon(config, 12918) + marker = SCRATCHDIR / 'read-error' + + def invoke(name, arguments, path, after, mode='eio', success=False): + marker.unlink(missing_ok=True) + env = {**os.environ, 'LD_PRELOAD': str(library), 'RSYNC_TEST_READ_PATH': str(path), + 'RSYNC_TEST_READ_AFTER': str(after), 'RSYNC_TEST_READ_ERROR': mode, + 'RSYNC_TEST_READ_MARKER': str(marker)} + with subprocess.Popen(rsync_argv(*arguments), env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + text=True, start_new_session=True) as process: + try: + output, _ = process.communicate(timeout=10) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + output, _ = process.communicate(timeout=5) + test_fail(f'{name}: stalled after a read error: {output}') + if after >= 0 and not marker.exists(): + test_fail(f'{name}: read hook did not fire: {output}') + if process.returncode < 0: + test_fail(f'{name}: terminated by signal {-process.returncode}: {output}') + if (process.returncode == 0) != success: + test_fail(f'{name}: unexpected exit {process.returncode}: {output}') + return output + + invoke('config control', [url], config, -1, success=True) + for after in (0, len('use chroot = no\n'), config_text.index('[files]') + 3, len(config_text) - 3, len(config_text)): + invoke('config error', [url], config, after) + invoke('config fdopen error', [url], config, 0, mode='fdopen') + output = invoke('interrupted config', [url], config, config_text.index('[files]') + 3, mode='eintr', success=True) + if 'files' not in output: + test_fail('interrupted config lost the module name') + config.write_text('') + invoke('empty config', [url], config, -1, success=True) + config.write_text('# comment without a newline') + invoke('config comment', [url], config, -1, success=True) + config.write_text(config_text.rstrip('\n')) + invoke('config without newline', [url], config, -1, success=True) + included = SCRATCHDIR / 'included.conf' + included.write_text(f'[included]\npath = {FROMDIR}\n') + config.write_text(f'use chroot = no\n&include {included}\n' + config_text) + invoke('included config control', [url], included, -1, success=True) + invoke('included config error', [url], included, 3) + + motd = SCRATCHDIR / 'motd' + motd.write_text('message of the day\n') + config.write_text(f'motd file = {motd}\n' + config_text) + invoke('motd control', [url], motd, -1, success=True) + output = invoke('interrupted motd', [url], motd, 4, mode='eintr', success=True) + if motd.read_text() not in output: + test_fail('interrupted MOTD lost data') + invoke('motd error', [url], motd, 4) + invoke('motd fdopen error', [url], motd, 0, mode='fdopen') + motd.write_text('message of the day\n' * 1024) + output = invoke('interrupted long motd', [url], motd, 4099, mode='eintr', success=True) + if motd.read_text() not in output: + test_fail('interrupted MOTD lost data across buffers') + + early = SCRATCHDIR / 'early-input' + early.write_bytes(bytes(range(256))) + captured = SCRATCHDIR / 'early-received' + hook = shlex.join([sys.executable, '-S', '-c', + f'import sys; from pathlib import Path; Path({str(captured)!r}).write_bytes(sys.stdin.buffer.read())']) + config.write_text(config_text + f'early exec = {hook}\n') + arguments = [f'--early-input={early}', f'{url}files/'] + invoke('early-input control', arguments, early, -1, success=True) + invoke('interrupted early-input', arguments, early, 4, mode='eintr', success=True) + if captured.read_bytes() != early.read_bytes(): + test_fail('interrupted early-input lost or duplicated data') + invoke('early-input error', arguments, early, 4) + output = invoke('early-input fdopen error', arguments, early, 0, mode='fdopen') + if os.strerror(errno.EMFILE) not in output: + test_fail(f'early-input fdopen error was overwritten: {output}') + invoke('early-input short file', arguments, early, 4, mode='eof') + early.write_text('') + invoke('empty early-input', arguments, early, -1, success=True) + + rules = SCRATCHDIR / 'rules' + for ending in ('\n', '\r\n'): + rules.write_bytes(f'- drop{ending}'.encode()) + destination = SCRATCHDIR / f'destination-{len(ending)}' + arguments = ['-r', f'--exclude-from={rules}', f'{FROMDIR}/', f'{destination}/'] + invoke('filter control', arguments, rules, -1, success=True) + if not (destination / 'keep').exists() or (destination / 'drop').exists(): + test_fail('filter control copied the wrong files') + invoke('filter error', arguments, rules, len('- drop') + (ending == '\r\n')) + output = invoke('filter fdopen error', arguments, rules, 0, mode='fdopen') + if os.strerror(errno.EMFILE) not in output: + test_fail(f'filter fdopen error was overwritten: {output}') + invoke('interrupted filter', arguments, rules, 3, mode='eintr', success=True) + if (destination / 'drop').exists(): + test_fail('interrupted filter lost a rule') + + protected = SCRATCHDIR / 'protected-destination' + protected.mkdir() + (protected / 'protected').write_text('retained\n') + rules.write_text('- protected\n') + arguments = ['-r', '--delete', f'--exclude-from={rules}', f'{FROMDIR}/', f'{protected}/'] + invoke('filter deletion guard', arguments, rules, 3) + if (protected / 'protected').read_text() != 'retained\n': + test_fail('filter read error allowed deletion') + + nested = SCRATCHDIR / 'private-merge-name' + nested.write_text('- drop\n') + rules.write_text(f'. {nested}\n') + arguments = ['-r', '-f', f'. {rules}', f'{FROMDIR}/', f'{protected}/'] + output = invoke('nested filter error', arguments, nested, 3) + if nested.name in output: + test_fail('nested filter error exposed a filename from file contents') + + +if __name__ == '__main__': + test() From c79c61cfa2aa091a24ffb7177823760c91a0938f Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Wed, 11 Nov 2026 22:21:57 +1100 Subject: [PATCH 2/9] params: terminate truncated section names --- params.c | 1 + testsuite/daemon-config-truncation_test.py | 40 ++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 testsuite/daemon-config-truncation_test.py diff --git a/params.c b/params.c index 7849079bb..20d56d2bc 100644 --- a/params.c +++ b/params.c @@ -271,6 +271,7 @@ static BOOL Section( FILE *InFile, BOOL (*sfunc)(char *) ) /* We arrive here if we've met the EOF before the closing bracket. */ if (ferror(InFile)) return False; + bufr[end] = '\0'; rprintf(FLOG, "%s Unexpected EOF in the config file: %s\n", func, bufr ); return( False ); } /* Section */ diff --git a/testsuite/daemon-config-truncation_test.py b/testsuite/daemon-config-truncation_test.py new file mode 100644 index 000000000..b0256426f --- /dev/null +++ b/testsuite/daemon-config-truncation_test.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python3 + +import socket +import subprocess +from rsyncfns import FROMDIR, SCRATCHDIR, rsync_argv, test_fail + + +FROMDIR.mkdir() +config = SCRATCHDIR / 'rsyncd.conf' +valid = f'use chroot = no\n[valid]\npath = {FROMDIR}\n' +for text in (valid, '[', '[unfinished', '[unfinished\n'): + config.write_text(text) + with socket.socket() as listener: + listener.bind(('127.0.0.1', 0)) + listener.listen(1) + with socket.create_connection(listener.getsockname(), timeout=5) as client: + connection, _ = listener.accept() + with connection, subprocess.Popen(rsync_argv('--daemon', '--no-detach', f'--config={config}'), + stdin=connection, stdout=connection, stderr=subprocess.PIPE) as process: + connection.close() + chunks = [] + try: + try: + client.sendall(b'@RSYNCD: 30.0\n\n') + while data := client.recv(4096): + chunks.append(data) + except (BrokenPipeError, ConnectionResetError): + pass + process.wait(timeout=5) + finally: + if process.poll() is None: + process.kill() + process.wait(timeout=5) + output = b''.join(chunks) + error = process.stderr.read() + if text == valid: + if process.returncode != 255 or b'valid' not in output: + test_fail(f'valid config failed: {output!r} {error!r}') + elif process.returncode != 1: + test_fail(f'incomplete section: exit {process.returncode}: {output!r} {error!r}') From 2b0ae5586e8966268af09a6d103f820e2acbfada Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Wed, 11 Nov 2026 22:21:57 +1100 Subject: [PATCH 3/9] sender: preserve file-open errors --- sender.c | 10 ++-- testsuite/copy-links-open-error_test.py | 48 +++++++++++++++++ testsuite/sender-open-error.c | 68 +++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 testsuite/copy-links-open-error_test.py create mode 100644 testsuite/sender-open-error.c diff --git a/sender.c b/sender.c index c1b903960..c0ef084d3 100644 --- a/sender.c +++ b/sender.c @@ -313,11 +313,13 @@ static int sender_open_copylinks_confined(const char *anchor, const char *relpat if (n < 0) { /* EINVAL: not a symlink -> the resolved target file. Open it * O_NOFOLLOW (a raced symlink flip is still refused). */ - fd = e == EINVAL - ? openat(pdfd, bname, O_RDONLY | O_NOFOLLOW | O_BINARY | extra, 0) - : -1; + if (e == EINVAL) { + fd = openat(pdfd, bname, O_RDONLY | O_NOFOLLOW | O_BINARY | extra, 0); + e = fd < 0 ? errno : 0; + } else + fd = -1; close(pdfd); - if (n < 0 && e != EINVAL) + if (fd < 0) errno = e; return fd; } diff --git a/testsuite/copy-links-open-error_test.py b/testsuite/copy-links-open-error_test.py new file mode 100644 index 000000000..804ba63c9 --- /dev/null +++ b/testsuite/copy-links-open-error_test.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 + +import errno +import os +import platform +import subprocess + +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, forced_protocol, rsync_argv, start_test_daemon, test_fail, test_skipped + + +def test(): + if platform.system() != 'Linux' or os.environ.get('ASAN_OPTIONS'): + test_skipped('open fault injection needs Linux without ASan') + library = SCRATCHDIR / 'sender-open-error.so' + fixture = SRCDIR / 'testsuite/sender-open-error.c' + build = subprocess.run(['cc', '-shared', '-fPIC', '-o', str(library), str(fixture), '-ldl'], capture_output=True, text=True) + if build.returncode: + test_fail(f'cannot build open hook: {build.stderr}') + FROMDIR.mkdir() + (FROMDIR / 'file').write_text('file data\n') + config = SCRATCHDIR / 'rsyncd.conf' + identity = f'uid = {os.getuid()}\ngid = {os.getgid()}\n' if os.geteuid() == 0 else '' + config.write_text(f'use chroot = no\n[files]\npath = {FROMDIR}\n' + identity) + url = start_test_daemon(config, 12919) + marker = SCRATCHDIR / 'open-error' + for error, expected in ((0, 0), (errno.ENOENT, 24), (errno.EACCES, 23)): + if error == errno.ENOENT and (forced_protocol() or 33) < 30: + expected = 0 + marker.unlink(missing_ok=True) + destination = SCRATCHDIR / f'destination-{error}' + env = {**os.environ, 'LD_PRELOAD': str(library), 'LC_ALL': 'C', 'RSYNC_TEST_OPEN_FILE': 'file', + 'RSYNC_TEST_OPEN_ERRNO': str(error), 'RSYNC_TEST_OPEN_MARKER': str(marker)} + result = subprocess.run(rsync_argv('-rL', f'{url}files/', f'{destination}/'), env=env, + capture_output=True, text=True, timeout=15) + if result.returncode != expected: + test_fail(f'open errno {error}: expected exit {expected}, got {result.returncode}: {result.stderr}') + if error and not marker.exists(): + test_fail('open hook did not fire') + if error and (destination / 'file').exists(): + test_fail('failed open produced a destination file') + if error == errno.EACCES and os.strerror(error) not in result.stderr: + test_fail(f'open error was overwritten: {result.stderr}') + if not error and (destination / 'file').read_text() != 'file data\n': + test_fail('open control changed file data') + + +if __name__ == '__main__': + test() diff --git a/testsuite/sender-open-error.c b/testsuite/sender-open-error.c new file mode 100644 index 000000000..8e3f445e8 --- /dev/null +++ b/testsuite/sender-open-error.c @@ -0,0 +1,68 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +static int parent_fd = -1, failed_fd = -1; + +ssize_t readlinkat(int fd, const char *path, char *buffer, size_t length) +{ + ssize_t (*original)(int, const char *, char *, size_t) = dlsym(RTLD_NEXT, "readlinkat"); + ssize_t result = original(fd, path, buffer, length); + if (result < 0 && errno == EINVAL && getenv("RSYNC_TEST_OPEN_FILE") + && strcmp(path, getenv("RSYNC_TEST_OPEN_FILE")) == 0) + parent_fd = fd; + return result; +} + +static int open_file(const char *function, int fd, const char *path, int flags, mode_t mode) +{ + int (*original)(int, const char *, int, ...) = dlsym(RTLD_NEXT, function); + int error = getenv("RSYNC_TEST_OPEN_ERRNO") ? atoi(getenv("RSYNC_TEST_OPEN_ERRNO")) : 0; + if (error && fd == parent_fd && strcmp(path, getenv("RSYNC_TEST_OPEN_FILE")) == 0) { + int marker = open(getenv("RSYNC_TEST_OPEN_MARKER"), O_WRONLY | O_CREAT, 0600); + if (marker >= 0) + close(marker); + failed_fd = fd; + errno = error; + return -1; + } + return original(fd, path, flags, mode); +} + +int openat(int fd, const char *path, int flags, ...) +{ + va_list args; + mode_t mode = 0; + va_start(args, flags); + if ((flags & O_CREAT) || (flags & O_TMPFILE) == O_TMPFILE) + mode = va_arg(args, mode_t); + va_end(args); + return open_file("openat", fd, path, flags, mode); +} + +int openat64(int fd, const char *path, int flags, ...) +{ + va_list args; + mode_t mode = 0; + va_start(args, flags); + if ((flags & O_CREAT) || (flags & O_TMPFILE) == O_TMPFILE) + mode = va_arg(args, mode_t); + va_end(args); + return open_file("openat64", fd, path, flags, mode); +} + +int close(int fd) +{ + int (*original)(int) = dlsym(RTLD_NEXT, "close"); + int result = original(fd); + if (fd == failed_fd) { + failed_fd = -1; + errno = EBADF; + } + return result; +} From aa12fc9265508b1ab40022fd2d574e4ffdb8fe87 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Wed, 11 Nov 2026 22:21:57 +1100 Subject: [PATCH 4/9] daemon: reset cwd after chroot --- NEWS.md | 9 +++++ clientserver.c | 2 + testsuite/chroot-cwd.c | 40 ++++++++++++++++++++ testsuite/daemon-chroot-cwd_test.py | 58 +++++++++++++++++++++++++++++ testsuite/skiplist/cygwin.txt | 3 ++ testsuite/skiplist/macos.txt | 3 ++ 6 files changed, 115 insertions(+) create mode 100644 testsuite/chroot-cwd.c create mode 100644 testsuite/daemon-chroot-cwd_test.py diff --git a/NEWS.md b/NEWS.md index c4f5aeaf8..ccb51b80d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,15 @@ on macOS. Native configuration now proves lookup, creation, permission-revocation and no-follow behaviour before selecting a traversal descriptor. Unverified targets fail closed instead of using raceable paths. +- Rejected config and filter files after read or stream setup errors instead + of accepting partial contents. MOTD and `--early-input` reads now stop on + I/O errors instead of looping; interrupted reads retain their data. +- Fixed an out-of-bounds read while reporting an incomplete daemon config + section header. +- Preserved sender file-open errors when closing a held directory descriptor + during `--copy-links` transfers. +- Changed to `/` immediately after a module chroot, before selecting an inner + module directory. A failed working-directory reset aborts the connection. # NEWS for rsync 3.5.2 (4 Nov 2026) diff --git a/clientserver.c b/clientserver.c index c547b65f3..eafd231fd 100644 --- a/clientserver.c +++ b/clientserver.c @@ -1093,6 +1093,8 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char io_printf(f_out, "@ERROR: chroot failed\n"); return -1; } + if (chdir("/") < 0) + return path_failure(f_out, "/", True); am_chrooted = 1; module_chdir = module_dir; } diff --git a/testsuite/chroot-cwd.c b/testsuite/chroot-cwd.c new file mode 100644 index 000000000..692304066 --- /dev/null +++ b/testsuite/chroot-cwd.c @@ -0,0 +1,40 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include + +static int jailed, marker = -1; + +int chroot(const char *path) +{ + int (*original)(const char *) = dlsym(RTLD_NEXT, "chroot"); + const char *target = getenv("RSYNC_TEST_CHROOT_PATH"); + int selected = target && strcmp(path, target) == 0; + int result; + if (selected) + marker = open(getenv("RSYNC_TEST_CHROOT_MARKER"), O_WRONLY | O_CREAT | O_APPEND, 0600); + result = original(path); + if (selected && result == 0) + jailed = 1; + return result; +} + +int chdir(const char *path) +{ + int (*original)(const char *) = dlsym(RTLD_NEXT, "chdir"); + if (jailed) { + jailed = 0; + if (marker >= 0) { + (void)write(marker, path, strlen(path)); + (void)write(marker, "\n", 1); + } + if (getenv("RSYNC_TEST_CHROOT_DENY") && strcmp(path, "/") == 0) { + errno = EACCES; + return -1; + } + } + return original(path); +} diff --git a/testsuite/daemon-chroot-cwd_test.py b/testsuite/daemon-chroot-cwd_test.py new file mode 100644 index 000000000..9e50f53df --- /dev/null +++ b/testsuite/daemon-chroot-cwd_test.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 + +import ctypes +import os +import platform +import subprocess + +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, rsync_argv, start_test_daemon, test_fail, test_skipped + + +def test(): + if platform.system() != 'Linux' or os.environ.get('ASAN_OPTIONS'): + test_skipped('chroot fault injection needs Linux without ASan') + if 'valgrind' in rsync_argv()[0]: + test_skipped('Valgrind logs are outside the daemon chroot') + pid = os.fork() + if pid == 0: + os._exit(0 if ctypes.CDLL(None, use_errno=True).chroot(b'/') == 0 else 1) + _, status = os.waitpid(pid, 0) + if not os.WIFEXITED(status) or os.WEXITSTATUS(status): + test_skipped('chroot is unavailable') + library = SCRATCHDIR / 'chroot-cwd.so' + fixture = SRCDIR / 'testsuite/chroot-cwd.c' + build = subprocess.run(['cc', '-shared', '-fPIC', '-o', str(library), str(fixture), '-ldl'], capture_output=True, text=True) + if build.returncode: + test_fail(f'cannot build chroot hook: {build.stderr}') + root = SCRATCHDIR / 'root' + inner = root / 'inner' + inner.mkdir(parents=True) + (root / 'outside-inner').write_text('outside\n') + (inner / 'inside').write_text('inside\n') + config = SCRATCHDIR / 'rsyncd.conf' + marker = SCRATCHDIR / 'chdir' + url = start_test_daemon(config, 12920) + for name, path, denied in (('plain', str(root), False), ('inner', f'{root}/./inner', False), + ('denied', f'{root}/./inner', True)): + marker.unlink(missing_ok=True) + config.write_text(f'use chroot = yes\n[files]\npath = {path}\n') + env = {**os.environ, 'LD_PRELOAD': str(library), 'RSYNC_TEST_CHROOT_PATH': str(root), + 'RSYNC_TEST_CHROOT_MARKER': str(marker)} + if denied: + env['RSYNC_TEST_CHROOT_DENY'] = '1' + destination = SCRATCHDIR / name + result = subprocess.run(rsync_argv('-r', f'{url}files/', f'{destination}/'), env=env, + capture_output=True, text=True, timeout=15) + if not marker.exists() or marker.read_text() != '/\n': + test_fail(f'{name}: did not enter the new root immediately') + if (result.returncode != 0) != denied: + test_fail(f'{name}: unexpected exit {result.returncode}: {result.stderr}') + if name == 'plain' and (destination / 'outside-inner').read_text() != 'outside\n': + test_fail('plain chroot selected the wrong directory') + if name == 'inner' and ((destination / 'inside').read_text() != 'inside\n' + or (destination / 'outside-inner').exists()): + test_fail('inner chroot selected the wrong directory') + + +if __name__ == '__main__': + test() diff --git a/testsuite/skiplist/cygwin.txt b/testsuite/skiplist/cygwin.txt index 643aec3c6..49b1f0684 100644 --- a/testsuite/skiplist/cygwin.txt +++ b/testsuite/skiplist/cygwin.txt @@ -22,8 +22,10 @@ chroot-link-dest-inner-module chroot-receiver-write-inner-module chroot-special-inner-module copy-dest-symlink-readleak +copy-links-open-error # Linux interposition copy-xattrs-symlink-race daemon-auth-group +daemon-chroot-cwd # Linux interposition daemon-chroot-munge-default daemon-config-symlink daemon-module-chdir-symlink @@ -69,6 +71,7 @@ search-only-held-dirfd sender-remove-source-root-anchor simd-checksum source-change-size-continues +stdio-read-errors # Linux interposition symlink-dest-backupdir symlink-race-dest symlink-race-relative-dest diff --git a/testsuite/skiplist/macos.txt b/testsuite/skiplist/macos.txt index 1311c90f5..e6f8cf576 100644 --- a/testsuite/skiplist/macos.txt +++ b/testsuite/skiplist/macos.txt @@ -13,8 +13,10 @@ append-abort-mid-write # the mid-write hook is an LD_PRELOAD, Linux-only backup-acl-xattr-cache backup-crossdev-copy chmod-temp-dir +copy-links-open-error # Linux interposition copy-xattrs-symlink-race daemon-auth-group +daemon-chroot-cwd # Linux interposition dir-sgid fake-super-acl-xattr hardlinks-emlink-recovery # LD_PRELOAD hook @@ -38,4 +40,5 @@ sender-remove-source-root-anchor simd-checksum source-change-size-continues sparse +stdio-read-errors # Linux interposition symlink-exclude-xattr From 721ad8d5f7cb16b22d0571a9a9762e54537e2d6d Mon Sep 17 00:00:00 2001 From: Arnaud Rebillout Date: Tue, 10 Nov 2026 11:58:18 +0700 Subject: [PATCH 5/9] github: Add GitHub Workflow for Debian 10 buster --- .github/workflows/debian-10-build.yml | 73 +++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 .github/workflows/debian-10-build.yml diff --git a/.github/workflows/debian-10-build.yml b/.github/workflows/debian-10-build.yml new file mode 100644 index 000000000..8488eb2ec --- /dev/null +++ b/.github/workflows/debian-10-build.yml @@ -0,0 +1,73 @@ +name: Test rsync on Debian 10 Buster + +# This workflow also ensures compatibility with Python 3.7 + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true + +on: + push: + branches: [ master ] + paths-ignore: + - '.github/workflows/*.yml' + - '!.github/workflows/debian-10-build.yml' + pull_request: + types: [opened, synchronize, reopened] + paths-ignore: + - '.github/workflows/*.yml' + - '!.github/workflows/debian-10-build.yml' + schedule: + - cron: '42 8 * * *' + +jobs: + test: + runs-on: ubuntu-latest + container: + image: debian:buster + name: Test rsync on Debian 10 Buster + steps: + - name: point apt sources to archive.debian.org + run: sed -i 's/deb\.debian\.org/archive.debian.org/' /etc/apt/sources.list + - name: populate apt cache + run: apt-get update + - name: install git + # actions/checkout needs git in the container before the checkout step. + run: apt-get -y install git + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: prep + run: | + apt-get -y install \ + build-essential autoconf automake autotools-dev m4 \ + acl libacl1-dev attr libattr1-dev \ + libidn2-dev liblz4-dev libpopt-dev libssl-dev zlib1g-dev \ + python3:native python3-cmarkgfm:native + - name: configure + # Don't use bundled zlib + # Disable md5-asm optimization: we link against openssl so rsync ends up + # using that library's implementation of the hash + # Disable xxhash and zstd: the system libs are too old for the build + run: ./configure --with-rrsync --with-included-zlib=no --disable-md5-asm --disable-xxhash --disable-zstd + - name: make + run: make + - name: install + run: make install + - name: info + run: rsync --version + - name: check + run: RSYNC_EXPECT_SKIPPED=@testsuite/skiplist/common.txt,@testsuite/skiplist/linux.txt make check + - name: save artifact + uses: actions/upload-artifact@v4 + with: + retention-days: 45 + name: debian-10-bin + path: | + rsync + rsync-ssl + rsync.1 + rsync-ssl.1 + rsyncd.conf.5 + rrsync.1 + rrsync From c9db4cb08b8ec2be215ff6ed5be4e3f4d5e13658 Mon Sep 17 00:00:00 2001 From: Arnaud Rebillout Date: Thu, 12 Nov 2026 09:58:14 +0700 Subject: [PATCH 6/9] testsuite: Drop the walrus operator, not in Python 3.7 This was introduced in Python 3.8 --- testsuite/daemon-config-truncation_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testsuite/daemon-config-truncation_test.py b/testsuite/daemon-config-truncation_test.py index b0256426f..ad11152a3 100644 --- a/testsuite/daemon-config-truncation_test.py +++ b/testsuite/daemon-config-truncation_test.py @@ -22,7 +22,10 @@ try: try: client.sendall(b'@RSYNCD: 30.0\n\n') - while data := client.recv(4096): + while True: + data = client.recv(4096) + if not data: + break chunks.append(data) except (BrokenPipeError, ConnectionResetError): pass From 099d879d54f7027ebc7b9e135ca4f706ded7e745 Mon Sep 17 00:00:00 2001 From: Arnaud Rebillout Date: Mon, 9 Nov 2026 16:40:02 +0700 Subject: [PATCH 7/9] testsuite: Workaround missing shlex.join in Python 3.7 shlex.join was added in Python 3.8 This commit adds a wrapper 'join_and_quote', which just invokes shlex.join if it exists, and otherwise basically implements it (the implementation is just a one-line in the Python stdlib, so it's easy to reproduce it here). Then the commit replaces all calls to shlex.join with calls to join_and_quote so that it works with Python 3.7 --- testsuite/niceness_test.py | 4 ++-- testsuite/perftest.py | 4 +++- testsuite/pseudo-paths_test.py | 6 +++--- testsuite/read-batch-pipe_test.py | 4 ++-- testsuite/rrsync-userns-procfs_test.py | 4 ++-- testsuite/rsyncfns.py | 17 ++++++++++++++++- testsuite/stdio-read-errors_test.py | 7 +++---- 7 files changed, 31 insertions(+), 15 deletions(-) diff --git a/testsuite/niceness_test.py b/testsuite/niceness_test.py index 409d7a42e..1453dbb9f 100644 --- a/testsuite/niceness_test.py +++ b/testsuite/niceness_test.py @@ -9,7 +9,7 @@ from rsyncfns import ( FROMDIR, RSYNC, SCRATCHDIR, TODIR, build_rsyncd_conf, hands_setup, - patched_rrsync, rsync_argv, rsync_path_arg, rsh_cmd, run_rsync, + join_and_quote, patched_rrsync, rsync_argv, rsync_path_arg, rsh_cmd, run_rsync, start_test_daemon, test_fail, ) @@ -245,7 +245,7 @@ def forwarded_option(*args): idle_wrapper.write_text( idle_code + 'import sys\nos.execvp(sys.argv[1], sys.argv[1:])\n' ) - idle_command = shlex.join([sys.executable, str(idle_wrapper), *shlex.split(RSYNC)]) + idle_command = join_and_quote([sys.executable, str(idle_wrapper), *shlex.split(RSYNC)]) idle_daemon_rsh = ( f'{idle_command} --config={shlex.quote(str(conf))} --daemon' ) diff --git a/testsuite/perftest.py b/testsuite/perftest.py index d5ac6a2b5..0ac565393 100755 --- a/testsuite/perftest.py +++ b/testsuite/perftest.py @@ -44,6 +44,8 @@ import tempfile import time +from rsyncfns import join_and_quote + # --------------------------------------------------------------------------- # Test-tree generation (ported from gentestdata.py, kept self-contained). # --------------------------------------------------------------------------- @@ -274,7 +276,7 @@ def time_transfer(binary, rsync_args, src, dest, timeout): if r.returncode != 0: raise RuntimeError( f"{binary.label} ({binary.path}) rsync exited {r.returncode}:\n" - f" cmd: {shlex.join(argv)}\n" + f" cmd: {join_and_quote(argv)}\n" f" {(r.stderr or r.stdout).strip()}") return elapsed diff --git a/testsuite/pseudo-paths_test.py b/testsuite/pseudo-paths_test.py index 2c0a5b25c..41dae1616 100644 --- a/testsuite/pseudo-paths_test.py +++ b/testsuite/pseudo-paths_test.py @@ -9,7 +9,7 @@ from pathlib import Path from rsyncfns import ( - SCRATCHDIR, makepath, rmtree, rsync_argv, test_fail, test_skipped, + SCRATCHDIR, join_and_quote, makepath, rmtree, rsync_argv, test_fail, test_skipped, ) if not sys.platform.startswith('linux'): test_skipped('Kernel pseudo-path string is a Linux-specific procfs feature') @@ -42,7 +42,7 @@ (src / 'transfer_me.txt').write_text('sync this\n') -rsync_base_cmd = shlex.join(rsync_argv('-a')) +rsync_base_cmd = join_and_quote(rsync_argv('-a')) src_path = shlex.quote(str(src) + '/') dest_path = shlex.quote(str(dest) + '/') @@ -293,7 +293,7 @@ shutil.copy2(rsync_argv()[0], local_bin) local_bin.chmod(0o777) - cmd_prefix = shlex.join([str(local_bin), '-a']) + cmd_prefix = join_and_quote([str(local_bin), '-a']) s_path = shlex.quote(str(ws_src) + '/') d_path = shlex.quote(str(ws_dest) + '/') diff --git a/testsuite/read-batch-pipe_test.py b/testsuite/read-batch-pipe_test.py index 505678a7e..60c3a5f1e 100644 --- a/testsuite/read-batch-pipe_test.py +++ b/testsuite/read-batch-pipe_test.py @@ -8,7 +8,7 @@ import sys from pathlib import Path -from rsyncfns import SCRATCHDIR, makepath, rmtree, rsync_argv, test_fail, test_skipped +from rsyncfns import SCRATCHDIR, join_and_quote, makepath, rmtree, rsync_argv, test_fail, test_skipped # We require bash specifically because standard POSIX /bin/sh does not # guarantee support for <(...) process substitution syntax. @@ -50,7 +50,7 @@ makepath(dest) # 3. Now we can test reading it via bash process substitution -rsync_base_cmd = shlex.join(rsync_argv('-a')) +rsync_base_cmd = join_and_quote(rsync_argv('-a')) batch_path = shlex.quote(str(batch_file)) dest_path = shlex.quote(str(dest) + '/') diff --git a/testsuite/rrsync-userns-procfs_test.py b/testsuite/rrsync-userns-procfs_test.py index 7bd1cdb06..178b31c0f 100644 --- a/testsuite/rrsync-userns-procfs_test.py +++ b/testsuite/rrsync-userns-procfs_test.py @@ -9,7 +9,7 @@ import tempfile from pathlib import Path -from rsyncfns import makepath, rmtree, rsync_argv, test_fail, test_skipped +from rsyncfns import join_and_quote, makepath, rmtree, rsync_argv, test_fail, test_skipped if not sys.platform.startswith('linux'): @@ -39,7 +39,7 @@ else: rmtree(launch_dir) test_fail(f'cannot locate the rsync executable in {env["RSYNC"]!r}') - env['RSYNC'] = shlex.join(rsync_cmd) + env['RSYNC'] = join_and_quote(rsync_cmd) launcher = [] if os.geteuid() == 0: setpriv = shutil.which('setpriv') diff --git a/testsuite/rsyncfns.py b/testsuite/rsyncfns.py index 0cb473e59..cccadbd88 100644 --- a/testsuite/rsyncfns.py +++ b/testsuite/rsyncfns.py @@ -40,6 +40,21 @@ from exitcodes import Exit # re-exported: tests may `from rsyncfns import Exit` +# --- Python 3.7 compatibility ---------------------------------------------- + +# If shlex.join is not found (Python <= 3.7), implement it. The implementation +# is taken "as is" from Python's stdlib. Otherwise, this function is just a +# wrapper. +if hasattr(shlex, 'join'): + def join_and_quote(split_command: list[str]) -> str: + """Return a shell-escaped string from *split_command*.""" + return shlex.join(split_command) +else: + def join_and_quote(split_command: list[str]) -> str: + """Return a shell-escaped string from *split_command*.""" + return ' '.join(shlex.quote(arg) for arg in split_command) + + # --- environment ----------------------------------------------------------- def _required(name: str) -> str: @@ -918,7 +933,7 @@ def rsync_path_arg(cmd: str = None) -> str: ('valgrind ... /build/rsync') must stay several words. Splitting and re-joining with shlex gives both: each word is quoted only if it needs it. """ - return shlex.join(split_rsync_cmd(RSYNC_PEER if cmd is None else cmd)) + return join_and_quote(split_rsync_cmd(RSYNC_PEER if cmd is None else cmd)) def rsync_argv(*args: str) -> list: diff --git a/testsuite/stdio-read-errors_test.py b/testsuite/stdio-read-errors_test.py index 8b10993a0..89c1509ab 100644 --- a/testsuite/stdio-read-errors_test.py +++ b/testsuite/stdio-read-errors_test.py @@ -3,12 +3,11 @@ import errno import os import platform -import shlex import signal import subprocess import sys -from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, rsync_argv, start_test_daemon, test_fail, test_skipped +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, join_and_quote, rsync_argv, start_test_daemon, test_fail, test_skipped def test(): @@ -85,8 +84,8 @@ def invoke(name, arguments, path, after, mode='eio', success=False): early = SCRATCHDIR / 'early-input' early.write_bytes(bytes(range(256))) captured = SCRATCHDIR / 'early-received' - hook = shlex.join([sys.executable, '-S', '-c', - f'import sys; from pathlib import Path; Path({str(captured)!r}).write_bytes(sys.stdin.buffer.read())']) + hook = join_and_quote([sys.executable, '-S', '-c', + f'import sys; from pathlib import Path; Path({str(captured)!r}).write_bytes(sys.stdin.buffer.read())']) config.write_text(config_text + f'early exec = {hook}\n') arguments = [f'--early-input={early}', f'{url}files/'] invoke('early-input control', arguments, early, -1, success=True) From 8db9a40b60ffa51bd759d796a4167577e2798dff Mon Sep 17 00:00:00 2001 From: Arnaud Rebillout Date: Thu, 12 Nov 2026 11:05:25 +0700 Subject: [PATCH 8/9] testsuite: Workaround missing_ok in Path.unlink in Python 3.7 This was introduced in Python 3.8 This commit adds a wrapper unlink_missing_ok, which just calls Path.unlink(missing_ok=True) for Python 3.8 or above, and otherwise falls back to the try/except method. Then this commit also replaces all occurences to unlink(missing_ok=True) with calls to unlink_missing_ok, so that it works with Python 3.7. --- testsuite/aligned_overflow_test.py | 4 ++-- testsuite/append-abort-mid-write_test.py | 3 ++- ...-nested-singlequote-host-injection_test.py | 7 ++++--- testsuite/copy-links-open-error_test.py | 4 ++-- testsuite/daemon-chroot-cwd_test.py | 4 ++-- .../daemon-leaf-type-race-fchmod_test.py | 6 ++++-- testsuite/rrsync-receiver-file-arg_test.py | 8 ++++---- testsuite/rsync-ssl-socat_test.py | 8 ++++---- testsuite/rsyncfns.py | 14 +++++++++++++ .../source-change-size-continues_test.py | 6 +++--- testsuite/stdio-read-errors_test.py | 4 ++-- testsuite/symlink-dirlink-basis_test.py | 20 ++++++++++++++----- 12 files changed, 58 insertions(+), 30 deletions(-) diff --git a/testsuite/aligned_overflow_test.py b/testsuite/aligned_overflow_test.py index 3007341b4..3a1ca8b00 100755 --- a/testsuite/aligned_overflow_test.py +++ b/testsuite/aligned_overflow_test.py @@ -11,7 +11,7 @@ import subprocess import threading -from rsyncfns import FROMDIR, TODIR, TOOLDIR, rmtree, test_fail +from rsyncfns import FROMDIR, TODIR, TOOLDIR, rmtree, test_fail, unlink_missing_ok RSYNC_BIN = str(TOOLDIR / 'rsync') @@ -175,7 +175,7 @@ def drain_stream(): if t is not None: t.join(timeout=5) logf.close() - bigfile.unlink(missing_ok=True) + unlink_missing_ok(bigfile) out_text = out_log.read_text(encoding='utf-8', errors='replace') print(f"sender exited with rc={rc}", flush=True) diff --git a/testsuite/append-abort-mid-write_test.py b/testsuite/append-abort-mid-write_test.py index b0b1ebe94..105b0e4a4 100644 --- a/testsuite/append-abort-mid-write_test.py +++ b/testsuite/append-abort-mid-write_test.py @@ -29,6 +29,7 @@ from rsyncfns import ( RSYNC, SCRATCHDIR, assert_same, make_data_file, makepath, rmtree, rsync_argv, run_rsync, split_rsync_cmd, test_fail, test_skipped, + unlink_missing_ok, ) @@ -201,7 +202,7 @@ def abort_mid_write(mode): held = SCRATCHDIR / 'midwrite.held' release = SCRATCHDIR / 'midwrite.release' for p in (loaded, held, release): - p.unlink(missing_ok=True) + unlink_missing_ok(p) env = os.environ.copy() env['LD_PRELOAD'] = str(shim) env['RSYNC_MIDWRITE_TARGET'] = str((dest / 'grow.bin').resolve()) diff --git a/testsuite/connect-prog-nested-singlequote-host-injection_test.py b/testsuite/connect-prog-nested-singlequote-host-injection_test.py index 86275ae13..2f5629ae6 100755 --- a/testsuite/connect-prog-nested-singlequote-host-injection_test.py +++ b/testsuite/connect-prog-nested-singlequote-host-injection_test.py @@ -18,7 +18,7 @@ import shlex import subprocess -from rsyncfns import SCRATCHDIR, rmtree, rsync_argv, test_fail, test_skipped +from rsyncfns import SCRATCHDIR, rmtree, rsync_argv, test_fail, test_skipped, unlink_missing_ok REFUSAL = 'unsafe host characters for RSYNC_CONNECT_PROG' @@ -73,7 +73,7 @@ def run(host, prog): ('~root', "is tilde-expanded by the nested shell"), ('%self', "is expanded by a nested fish, where %self is its pid"), ('', "vanishes when a nested shell re-splits, shifting later args")): - seen.unlink(missing_ok=True) + unlink_missing_ok(seen) proc = run(host, f'{probe} %H') if REFUSAL not in proc.stdout: got = seen.read_text() if seen.exists() else '(program not reached)' @@ -90,7 +90,8 @@ def run(host, prog): ('host+alias', 'host+alias'), ('host~alias', 'host~alias'), ('[fe80::1%eth0]', 'fe80::1%eth0')): - seen.unlink(missing_ok=True) + + unlink_missing_ok(seen) proc = run(host, f'{probe} %H') if REFUSAL in proc.stdout: test_fail(f'host {host!r} is legitimate but was refused: {proc.stdout!r}') diff --git a/testsuite/copy-links-open-error_test.py b/testsuite/copy-links-open-error_test.py index 804ba63c9..c554ee44d 100644 --- a/testsuite/copy-links-open-error_test.py +++ b/testsuite/copy-links-open-error_test.py @@ -5,7 +5,7 @@ import platform import subprocess -from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, forced_protocol, rsync_argv, start_test_daemon, test_fail, test_skipped +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, forced_protocol, rsync_argv, start_test_daemon, test_fail, test_skipped, unlink_missing_ok def test(): @@ -26,7 +26,7 @@ def test(): for error, expected in ((0, 0), (errno.ENOENT, 24), (errno.EACCES, 23)): if error == errno.ENOENT and (forced_protocol() or 33) < 30: expected = 0 - marker.unlink(missing_ok=True) + unlink_missing_ok(marker) destination = SCRATCHDIR / f'destination-{error}' env = {**os.environ, 'LD_PRELOAD': str(library), 'LC_ALL': 'C', 'RSYNC_TEST_OPEN_FILE': 'file', 'RSYNC_TEST_OPEN_ERRNO': str(error), 'RSYNC_TEST_OPEN_MARKER': str(marker)} diff --git a/testsuite/daemon-chroot-cwd_test.py b/testsuite/daemon-chroot-cwd_test.py index 9e50f53df..1ea3debe1 100644 --- a/testsuite/daemon-chroot-cwd_test.py +++ b/testsuite/daemon-chroot-cwd_test.py @@ -5,7 +5,7 @@ import platform import subprocess -from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, rsync_argv, start_test_daemon, test_fail, test_skipped +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, rsync_argv, start_test_daemon, test_fail, test_skipped, unlink_missing_ok def test(): @@ -34,7 +34,7 @@ def test(): url = start_test_daemon(config, 12920) for name, path, denied in (('plain', str(root), False), ('inner', f'{root}/./inner', False), ('denied', f'{root}/./inner', True)): - marker.unlink(missing_ok=True) + unlink_missing_ok(marker) config.write_text(f'use chroot = yes\n[files]\npath = {path}\n') env = {**os.environ, 'LD_PRELOAD': str(library), 'RSYNC_TEST_CHROOT_PATH': str(root), 'RSYNC_TEST_CHROOT_MARKER': str(marker)} diff --git a/testsuite/daemon-leaf-type-race-fchmod_test.py b/testsuite/daemon-leaf-type-race-fchmod_test.py index ee2052588..d14270815 100644 --- a/testsuite/daemon-leaf-type-race-fchmod_test.py +++ b/testsuite/daemon-leaf-type-race-fchmod_test.py @@ -26,6 +26,8 @@ import time from pathlib import Path +from rsyncfns import unlink_missing_ok + def racer_main(argv: list[str]) -> int: if len(argv) != 5: @@ -249,8 +251,8 @@ def stop_process(process: subprocess.Popen, timeout: float = 3) -> tuple: old = module / "old" ready_fifo = SCRATCHDIR / "ready.fifo" done_fifo = SCRATCHDIR / "done.fifo" -ready_fifo.unlink(missing_ok=True) -done_fifo.unlink(missing_ok=True) +unlink_missing_ok(ready_fifo) +unlink_missing_ok(done_fifo) os.mkfifo(ready_fifo) os.mkfifo(done_fifo) diff --git a/testsuite/rrsync-receiver-file-arg_test.py b/testsuite/rrsync-receiver-file-arg_test.py index ef757f6d6..dc6a9ffd1 100644 --- a/testsuite/rrsync-receiver-file-arg_test.py +++ b/testsuite/rrsync-receiver-file-arg_test.py @@ -8,7 +8,7 @@ from rsyncfns import ( SCRATCHDIR, makepath, patched_rrsync, proc_self_fd_pins, rmtree, - rsync_argv, test_fail, test_skipped, + rsync_argv, test_fail, test_skipped, unlink_missing_ok, ) if not proc_self_fd_pins(): @@ -72,7 +72,7 @@ def check_contents(path): transfer(source, 'dummy:existing', *option) check_contents(destination) new = restricted / 'new' - new.unlink(missing_ok=True) + unlink_missing_ok(new) transfer(source, 'dummy:new', *option) check_contents(new) @@ -94,8 +94,8 @@ def check_contents(path): def gated_transfer(name, change): ready = base / 'ready' go = base / 'go' - ready.unlink(missing_ok=True) - go.unlink(missing_ok=True) + unlink_missing_ok(ready) + unlink_missing_ok(go) env = {**os.environ, 'RRSYNC_TEST_READY': str(ready), 'RRSYNC_TEST_GO': str(go)} proc = subprocess.Popen(command(source, f'dummy:{name}', '--inplace'), stdout=subprocess.PIPE, stderr=subprocess.PIPE, diff --git a/testsuite/rsync-ssl-socat_test.py b/testsuite/rsync-ssl-socat_test.py index 95a324413..b8e536039 100644 --- a/testsuite/rsync-ssl-socat_test.py +++ b/testsuite/rsync-ssl-socat_test.py @@ -11,7 +11,7 @@ import subprocess import sys -from rsyncfns import SCRATCHDIR, SRCDIR, test_fail +from rsyncfns import SCRATCHDIR, SRCDIR, test_fail, unlink_missing_ok RSYNC_SSL = SRCDIR / 'rsync-ssl' @@ -80,7 +80,7 @@ def fallback_env(**updates): def run_helper(host, **env_updates): - HELPER_ARGV.unlink(missing_ok=True) + unlink_missing_ok(HELPER_ARGV) proc = subprocess.run( [str(RSYNC_SSL), '--HELPER', host, 'rsync', '--server', '--daemon', '.'], env=clean_env(RSYNC_SSL_TYPE='socat', RSYNC_SSL_SOCAT=str(fake_socat), @@ -159,7 +159,7 @@ def run_helper(host, **env_updates): # --- default helper selection keeps existing openssl-first behaviour --------- -OPENSSL_ARGV.unlink(missing_ok=True) +unlink_missing_ok(OPENSSL_ARGV) proc = subprocess.run( [str(RSYNC_SSL), '--HELPER', 'example.org', 'rsync', '--server', '--daemon', '.'], @@ -176,7 +176,7 @@ def run_helper(host, **env_updates): # --- if openssl is unavailable, default selection prefers socat over stunnel - -fallback_helper_argv.unlink(missing_ok=True) +unlink_missing_ok(fallback_helper_argv) proc = subprocess.run( [BASH, str(RSYNC_SSL), '--HELPER', 'fallback.example', 'rsync', '--server', '--daemon', '.'], diff --git a/testsuite/rsyncfns.py b/testsuite/rsyncfns.py index cccadbd88..ba51e0953 100644 --- a/testsuite/rsyncfns.py +++ b/testsuite/rsyncfns.py @@ -55,6 +55,20 @@ def join_and_quote(split_command: list[str]) -> str: return ' '.join(shlex.quote(arg) for arg in split_command) +# If Path.unlink doesn't accept the missing_ok argument, fall back to checking +# the exception returned. For simplicity we check the Python version instead of +# inspecting the signature of the function. +if sys.version_info < (3, 8): + def unlink_missing_ok(file_path: Path) -> None: + try: + file_path.unlink() + except FileNotFoundError: + pass +else: + def unlink_missing_ok(file_path: Path) -> None: + file_path.unlink(missing_ok=True) + + # --- environment ----------------------------------------------------------- def _required(name: str) -> str: diff --git a/testsuite/source-change-size-continues_test.py b/testsuite/source-change-size-continues_test.py index 314784a9d..fb03bbfc0 100644 --- a/testsuite/source-change-size-continues_test.py +++ b/testsuite/source-change-size-continues_test.py @@ -22,7 +22,7 @@ from rsyncfns import ( RSYNC, SCRATCHDIR, assert_same, make_data_file, makepath, rmtree, - rsync_argv, test_fail, test_skipped, split_rsync_cmd, + rsync_argv, test_fail, test_skipped, split_rsync_cmd, unlink_missing_ok, ) @@ -172,8 +172,8 @@ def build_shim(): shim = build_shim() mark = SCRATCHDIR / 'source_shrink_read.mark' load_mark = SCRATCHDIR / 'source_shrink_load.mark' -mark.unlink(missing_ok=True) -load_mark.unlink(missing_ok=True) +unlink_missing_ok(mark) +unlink_missing_ok(load_mark) env = os.environ.copy() env['LD_PRELOAD'] = str(shim) env['RSYNC_SHRINK_TARGET'] = str(changing.resolve()) diff --git a/testsuite/stdio-read-errors_test.py b/testsuite/stdio-read-errors_test.py index 89c1509ab..9f9178753 100644 --- a/testsuite/stdio-read-errors_test.py +++ b/testsuite/stdio-read-errors_test.py @@ -7,7 +7,7 @@ import subprocess import sys -from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, join_and_quote, rsync_argv, start_test_daemon, test_fail, test_skipped +from rsyncfns import FROMDIR, SCRATCHDIR, SRCDIR, join_and_quote, rsync_argv, start_test_daemon, test_fail, test_skipped, unlink_missing_ok def test(): @@ -28,7 +28,7 @@ def test(): marker = SCRATCHDIR / 'read-error' def invoke(name, arguments, path, after, mode='eio', success=False): - marker.unlink(missing_ok=True) + unlink_missing_ok(marker) env = {**os.environ, 'LD_PRELOAD': str(library), 'RSYNC_TEST_READ_PATH': str(path), 'RSYNC_TEST_READ_AFTER': str(after), 'RSYNC_TEST_READ_ERROR': mode, 'RSYNC_TEST_READ_MARKER': str(marker)} diff --git a/testsuite/symlink-dirlink-basis_test.py b/testsuite/symlink-dirlink-basis_test.py index ffe368e27..c38407c1a 100644 --- a/testsuite/symlink-dirlink-basis_test.py +++ b/testsuite/symlink-dirlink-basis_test.py @@ -20,6 +20,7 @@ from rsyncfns import ( RSYNC, SCRATCHDIR, RSYNC_PEER, SRCDIR, TMPDIR, make_data_file, rsync_argv, test_fail, rsync_path_arg, rsh_cmd, + unlink_missing_ok, ) @@ -118,7 +119,7 @@ def assert_same(label: str, a, b) -> None: # Test 4: --backup with directory symlinks. (home / 'real-dir' / 'file').unlink() -(home / 'real-dir' / 'file~').unlink(missing_ok=True) +unlink_missing_ok(home / 'real-dir' / 'file~') make_testfile(srcbase / 'dir' / 'file') push('-KRlptv', 'dir/file', 'localhost:', label="test 4 initial") @@ -139,8 +140,11 @@ def assert_same(label: str, a, b) -> None: # Test 5: --inplace. -(home / 'real-dir' / 'file').unlink() -(home / 'real-dir' / 'file~').unlink(missing_ok=True) +try: + (home / 'real-dir' / 'file').unlink() + (home / 'real-dir' / 'file~').unlink() +except FileNotFoundError: + pass make_testfile(srcbase / 'dir' / 'file') push('-KRlptv', '--inplace', 'dir/file', 'localhost:', label="test 5 initial") @@ -170,7 +174,10 @@ def assert_same(label: str, a, b) -> None: # Test 7: --partial-dir with protocol < 29. -(home / 'real-dir' / 'file').unlink(missing_ok=True) +try: + (home / 'real-dir' / 'file').unlink() +except FileNotFoundError: + pass make_testfile(srcbase / 'dir' / 'file') push('-KRlptv', '--protocol=28', '--partial-dir=.rsync-partial', @@ -187,7 +194,10 @@ def assert_same(label: str, a, b) -> None: # Test 8: protocol < 29 basic update. -(home / 'real-dir' / 'file').unlink(missing_ok=True) +try: + (home / 'real-dir' / 'file').unlink() +except FileNotFoundError: + pass make_testfile(srcbase / 'dir' / 'file') push('-KRlptv', '--protocol=28', 'dir/file', 'localhost:', label="test 8 initial") From dc3cbe7c868b8f3ca19fbb94d28c7b467ae3f1c5 Mon Sep 17 00:00:00 2001 From: Arnaud Rebillout Date: Thu, 12 Nov 2026 17:37:53 +0700 Subject: [PATCH 9/9] testsuite: Fix password-file-first-line for Python 3.7 --- testsuite/password-file-first-line_test.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testsuite/password-file-first-line_test.py b/testsuite/password-file-first-line_test.py index 5af81c280..b2a5e1419 100644 --- a/testsuite/password-file-first-line_test.py +++ b/testsuite/password-file-first-line_test.py @@ -51,12 +51,17 @@ else: args.insert(1, '--password-file=-') stdin = subprocess.PIPE - proc = subprocess.run(rsync_argv(*args), input=contents if source == 'stdin' else None, - stdin=stdin if source == 'file' else None, + run_kwargs = {} + if source == 'stdin': + run_kwargs['input'] = contents + elif source == 'file': + run_kwargs['stdin'] = stdin + proc = subprocess.run(rsync_argv(*args), stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=False, timeout=20, env={**os.environ, 'RSYNC_PASSWORD': - 'wrong-fallback' if succeeds else PASSWORD.decode()}) + 'wrong-fallback' if succeeds else PASSWORD.decode()}, + **run_kwargs) passed = proc.returncode == 0 if passed != succeeds: test_fail(f'{source}/{name}: expected success={succeeds}, '