diff --git a/README.md b/README.md index ee7ac3a..34059c2 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ partial, alternate, and planned behavior. | `--existing` | Skip files not already present at the destination; update existing files normally. | | `--bwlimit ` | Bandwidth limit in kilobytes per second | | `--chunk-size ` | Chunk size in bytes (default: 10485760) | -| `--timeout ` | I/O timeout in seconds. Applied to both the socket (`SO_RCVTIMEO`/`SO_SNDTIMEO`, built-in default 30 s) and the per-message protocol poll deadline (built-in default 60 s). `0` (the default/unset sentinel) keeps both built-ins; a positive value overrides both. | +| `--timeout ` | Positive I/O timeout in seconds, applied to both the socket (`SO_RCVTIMEO`/`SO_SNDTIMEO`, built-in default 30 s) and the per-message protocol poll deadline (built-in default 60 s). Omit the option to keep both built-ins; `0` is rejected. The server side keeps the built-in 60 s protocol window (the value is not sent on the wire). | | `--contimeout ` | Connection timeout in seconds (default: 10) | | `--backup` | Backup existing destination files before overwriting | | `--backup-dir ` | Target directory for backups (requires `--backup`) | @@ -155,10 +155,14 @@ partial, alternate, and planned behavior. send/receive (the `poll()` deadline), so a peer that stops mid-frame is dropped. It does not, by itself, stop a peer that keeps sending well-formed frames forever. The receiver therefore also enforces two wall-clock (`CLOCK_MONOTONIC`) bounds on a -connection: a **1 hour** idle limit (only `STATUS_KEEPALIVE`/`STATUS_ABORT` frames -seen for that long counts as no forward progress) and a **24 hour** overall session -cap. Both are deliberately generous so a legitimate long-running transfer is never -aborted; they exist to defeat keepalive slowloris squatting on a connection slot. +connection: a **1 hour** idle limit and a **24 hour** overall session cap. Only +frames that move real work (not `STATUS_KEEPALIVE`/`STATUS_ABORT` and not an +empty `STATUS_CHECK_BATCH`/`STATUS_DIR_TIMES`) refresh the idle timestamp, so a +peer cannot hold a connection slot by emitting cheap empty frames; a peer that +fabricates minimal non-empty frames can still occupy a slot until the 24 hour +cap, since no bound can require actual payload without risking a legitimate +long operation. Both are deliberately generous so a legitimate long-running +transfer is never aborted. ### Server @@ -393,7 +397,7 @@ features without changing the meaning of ordinary compatibility options. | `--bwlimit ` | Apply token-bucket bandwidth limiting. | | `--progress` | Show transfer progress and throughput. | | `--stats` | Print transfer statistics. | -| `--timeout ` | Set the socket **and** per-message protocol I/O timeout. `0` keeps the built-in 30 s socket / 60 s protocol defaults; a positive value overrides both. | +| `--timeout ` | Set the socket **and** per-message protocol I/O timeout (positive seconds). Omit to keep the built-in 30 s socket / 60 s protocol defaults. | | `--contimeout ` | Set connection timeout. | Short-option conflicts with rsync have been resolved for the CLI namespace diff --git a/src/server/receiver.c b/src/server/receiver.c index 04e9f3d..a0a3071 100644 --- a/src/server/receiver.c +++ b/src/server/receiver.c @@ -203,22 +203,41 @@ bool receiver_time_limit_exceeded(const struct timespec* session_start, return false; } +/* A frame proves forward progress only when it cannot be fabricated for free. + * KEEPALIVE/ABORT are pure liveness, and CHECK_BATCH/DIR_TIMES may carry zero + * entries, so a peer must not be able to hold a connection slot forever by + * merely emitting empty frames. */ +static bool status_counts_as_progress(Status status) { + switch (status) { + case STATUS_KEEPALIVE: + case STATUS_ABORT: + case STATUS_CHECK_BATCH: + case STATUS_DIR_TIMES: + return false; + default: + return true; + } +} + /* Refresh the progress timestamp for a forward-moving frame and enforce the - * bounds above. Returns false (after best-effort STATUS_ERROR) when the - * connection must be dropped. */ + * bounds above. Returns false when the connection must be dropped; the + * terminal STATUS_ERROR is sent only when the sink owns error reporting (the + * -m sink sets send_error=false so the main thread emits exactly one). */ static bool receiver_note_status(const struct timespec* session_start, - struct timespec* last_progress, Status status, - int file_descriptor) { + struct timespec* last_progress, Status status, int file_descriptor, + const ReceiverSink* sink) { struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - if (status != STATUS_KEEPALIVE && status != STATUS_ABORT) + if (clock_gettime(CLOCK_MONOTONIC, &now) != 0) + now = *last_progress; + if (status_counts_as_progress(status)) *last_progress = now; if (!receiver_time_limit_exceeded(session_start, last_progress, &now)) return true; log_message(LOG_LEVEL_ERROR, "Receive session exceeded its time bound (idle %us / total %us); aborting connection", g_max_session_idle_sec, g_max_session_wall_sec); - send_status(file_descriptor, STATUS_ERROR); + if (!sink || sink->send_error) + send_status(file_descriptor, STATUS_ERROR); return false; } @@ -248,7 +267,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver struct timespec last_progress; clock_gettime(CLOCK_MONOTONIC, &session_start); last_progress = session_start; - if (!receiver_note_status(&session_start, &last_progress, status, file_descriptor)) + if (!receiver_note_status(&session_start, &last_progress, status, file_descriptor, sink)) return -1; bool early_delete = config_delete_timing_early(config); /* Parked keep-set for the late/commit timing. Every exit path below frees it @@ -349,7 +368,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver next_status: if (!receive_status(file_descriptor, &status)) goto receive_error; - if (!receiver_note_status(&session_start, &last_progress, status, file_descriptor)) + if (!receiver_note_status(&session_start, &last_progress, status, file_descriptor, sink)) goto fail; } if (status != STATUS_FINISHED) { diff --git a/src/server/server.c b/src/server/server.c index 13b854c..da3e65d 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -609,10 +609,11 @@ void handler(int file_descriptor) { if (gate_ctx.super_mode_override != -1) config->super_mode = (SuperMode)gate_ctx.super_mode_override; protocol_set_8_bit_output(config->eight_bit_output); - /* Honor the negotiated --timeout for every protocol frame from here on (the - * config handshake itself used the built-in 60 s window). A positive value - * also tightens the socket SO_RCVTIMEO/SO_SNDTIMEO already applied by the - * transport; 0 leaves both built-in defaults in place. */ + /* Server-side per-message protocol deadline for every frame from here on. + * `timeout` is not serialized, so this is the server's own config (the server + * has no --timeout CLI and defaults it to 0): the built-in 60 s window stays + * in effect. A client's --timeout tightens only that client's own protocol + * I/O and the server's socket read/write timeout is the transport default. */ protocol_session_set_io_timeout(&session, config->timeout); if (!authorized_root) { log_message(LOG_LEVEL_ERROR, "No server-side destination root configured"); diff --git a/src/shared/file_send.c b/src/shared/file_send.c index e7bcffb..f15b77a 100644 --- a/src/shared/file_send.c +++ b/src/shared/file_send.c @@ -145,7 +145,7 @@ bool file_send_sendfile_with_skip(File* file, int file_descriptor, bool use_meta off_t offset = 0; struct timespec deadline; clock_gettime(CLOCK_MONOTONIC, &deadline); - deadline.tv_sec += 60; + deadline.tv_sec += protocol_get_io_timeout_sec(); while ((unsigned long long)offset < file_size) { struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); diff --git a/src/shared/protocol.c b/src/shared/protocol.c index 7604379..3c3eee0 100644 --- a/src/shared/protocol.c +++ b/src/shared/protocol.c @@ -87,6 +87,12 @@ void protocol_session_set_io_timeout(ProtocolSession* session, int sec) { session->io_timeout_sec = sec; } +int protocol_get_io_timeout_sec(void) { + const ProtocolSession* session = bound_session ? bound_session : &legacy_io_session; + int sec = session->io_timeout_sec; + return sec > 0 ? sec : RECEIVE_TIMEOUT_SEC; +} + void protocol_session_set_max_alloc(ProtocolSession* session, unsigned long long max_alloc) { if (!session) session = bound_session ? bound_session : &legacy_io_session; diff --git a/src/shared/protocol.h b/src/shared/protocol.h index 5015971..60f6dec 100644 --- a/src/shared/protocol.h +++ b/src/shared/protocol.h @@ -152,6 +152,10 @@ void protocol_session_set_max_alloc(ProtocolSession* session, unsigned long long * An explicit long deadline (e.g. the delete-ack wait) is applied per-call by * protocol_receive_status_timed and is unaffected by this setter. */ void protocol_session_set_io_timeout(ProtocolSession* session, int sec); +/* Effective per-message I/O deadline (seconds) for the currently-bound session, + * falling back to the built-in default. Used by the plaintext sendfile path + * which bypasses the protocol send primitive. */ +int protocol_get_io_timeout_sec(void); void* protocol_alloc(size_t size); void* protocol_realloc(void* ptr, size_t size); void protocol_session_set_8_bit_output(ProtocolSession* session, bool enabled); diff --git a/tests/test_receiver_timeout.c b/tests/test_receiver_timeout.c index 1bb65c8..8066b5c 100644 --- a/tests/test_receiver_timeout.c +++ b/tests/test_receiver_timeout.c @@ -62,19 +62,28 @@ static void test_receiver_aborts_idle_keepalive() { protocol_session_bind(&session); Status keepalive = STATUS_KEEPALIVE; - EXPECT_EQ_INT((int)write(sv[0], &keepalive, sizeof(keepalive)), (int)sizeof(keepalive)); - int result = receiver_process_pending(config, sv[1], &sink, NULL); - EXPECT_EQ_INT(result, -1); - + ssize_t wrote = write(sv[0], &keepalive, sizeof(keepalive)); + int result = -2; + if (wrote == (ssize_t)sizeof(keepalive)) + result = receiver_process_pending(config, sv[1], &sink, NULL); Status reply = STATUS_OK; - EXPECT_EQ_INT((int)read(sv[0], &reply, sizeof(reply)), (int)sizeof(reply)); - EXPECT_EQ_INT((int)reply, (int)STATUS_ERROR); + ssize_t got = -1; + if (result == -1) + got = read(sv[0], &reply, sizeof(reply)); + /* Tear down the binding/descriptors BEFORE asserting: an EXPECT_* failure + * returns immediately, and a dangling bound_session would poison later + * fd-level protocol I/O tests. */ protocol_session_unbind(); config_delete(config); close(sv[0]); close(sv[1]); receiver_reset_time_limits(); + + EXPECT_EQ_INT((int)wrote, (int)sizeof(keepalive)); + EXPECT_EQ_INT(result, -1); + EXPECT_EQ_INT((int)got, (int)sizeof(reply)); + EXPECT_EQ_INT((int)reply, (int)STATUS_ERROR); } void test_receiver_timeout(void) {