fix(receiver): harden idle-progress definition, single error frame, sendfile timeout
This commit is contained in:
@@ -132,7 +132,7 @@ partial, alternate, and planned behavior.
|
|||||||
| `--existing` | Skip files not already present at the destination; update existing files normally. |
|
| `--existing` | Skip files not already present at the destination; update existing files normally. |
|
||||||
| `--bwlimit <KB/s>` | Bandwidth limit in kilobytes per second |
|
| `--bwlimit <KB/s>` | Bandwidth limit in kilobytes per second |
|
||||||
| `--chunk-size <n>` | Chunk size in bytes (default: 10485760) |
|
| `--chunk-size <n>` | Chunk size in bytes (default: 10485760) |
|
||||||
| `--timeout <sec>` | 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 <sec>` | 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 <sec>` | Connection timeout in seconds (default: 10) |
|
| `--contimeout <sec>` | Connection timeout in seconds (default: 10) |
|
||||||
| `--backup` | Backup existing destination files before overwriting |
|
| `--backup` | Backup existing destination files before overwriting |
|
||||||
| `--backup-dir <dir>` | Target directory for backups (requires `--backup`) |
|
| `--backup-dir <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
|
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
|
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
|
receiver therefore also enforces two wall-clock (`CLOCK_MONOTONIC`) bounds on a
|
||||||
connection: a **1 hour** idle limit (only `STATUS_KEEPALIVE`/`STATUS_ABORT` frames
|
connection: a **1 hour** idle limit and a **24 hour** overall session cap. Only
|
||||||
seen for that long counts as no forward progress) and a **24 hour** overall session
|
frames that move real work (not `STATUS_KEEPALIVE`/`STATUS_ABORT` and not an
|
||||||
cap. Both are deliberately generous so a legitimate long-running transfer is never
|
empty `STATUS_CHECK_BATCH`/`STATUS_DIR_TIMES`) refresh the idle timestamp, so a
|
||||||
aborted; they exist to defeat keepalive slowloris squatting on a connection slot.
|
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
|
### Server
|
||||||
|
|
||||||
@@ -393,7 +397,7 @@ features without changing the meaning of ordinary compatibility options.
|
|||||||
| `--bwlimit <KB/s>` | Apply token-bucket bandwidth limiting. |
|
| `--bwlimit <KB/s>` | Apply token-bucket bandwidth limiting. |
|
||||||
| `--progress` | Show transfer progress and throughput. |
|
| `--progress` | Show transfer progress and throughput. |
|
||||||
| `--stats` | Print transfer statistics. |
|
| `--stats` | Print transfer statistics. |
|
||||||
| `--timeout <seconds>` | 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 <seconds>` | 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 <seconds>` | Set connection timeout. |
|
| `--contimeout <seconds>` | Set connection timeout. |
|
||||||
|
|
||||||
Short-option conflicts with rsync have been resolved for the CLI namespace
|
Short-option conflicts with rsync have been resolved for the CLI namespace
|
||||||
|
|||||||
+27
-8
@@ -203,21 +203,40 @@ bool receiver_time_limit_exceeded(const struct timespec* session_start,
|
|||||||
return false;
|
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
|
/* Refresh the progress timestamp for a forward-moving frame and enforce the
|
||||||
* bounds above. Returns false (after best-effort STATUS_ERROR) when the
|
* bounds above. Returns false when the connection must be dropped; the
|
||||||
* connection must be dropped. */
|
* 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,
|
static bool receiver_note_status(const struct timespec* session_start,
|
||||||
struct timespec* last_progress, Status status,
|
struct timespec* last_progress, Status status, int file_descriptor,
|
||||||
int file_descriptor) {
|
const ReceiverSink* sink) {
|
||||||
struct timespec now;
|
struct timespec now;
|
||||||
clock_gettime(CLOCK_MONOTONIC, &now);
|
if (clock_gettime(CLOCK_MONOTONIC, &now) != 0)
|
||||||
if (status != STATUS_KEEPALIVE && status != STATUS_ABORT)
|
now = *last_progress;
|
||||||
|
if (status_counts_as_progress(status))
|
||||||
*last_progress = now;
|
*last_progress = now;
|
||||||
if (!receiver_time_limit_exceeded(session_start, last_progress, &now))
|
if (!receiver_time_limit_exceeded(session_start, last_progress, &now))
|
||||||
return true;
|
return true;
|
||||||
log_message(LOG_LEVEL_ERROR,
|
log_message(LOG_LEVEL_ERROR,
|
||||||
"Receive session exceeded its time bound (idle %us / total %us); aborting connection",
|
"Receive session exceeded its time bound (idle %us / total %us); aborting connection",
|
||||||
g_max_session_idle_sec, g_max_session_wall_sec);
|
g_max_session_idle_sec, g_max_session_wall_sec);
|
||||||
|
if (!sink || sink->send_error)
|
||||||
send_status(file_descriptor, STATUS_ERROR);
|
send_status(file_descriptor, STATUS_ERROR);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
@@ -248,7 +267,7 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
|
|||||||
struct timespec last_progress;
|
struct timespec last_progress;
|
||||||
clock_gettime(CLOCK_MONOTONIC, &session_start);
|
clock_gettime(CLOCK_MONOTONIC, &session_start);
|
||||||
last_progress = 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;
|
return -1;
|
||||||
bool early_delete = config_delete_timing_early(config);
|
bool early_delete = config_delete_timing_early(config);
|
||||||
/* Parked keep-set for the late/commit timing. Every exit path below frees it
|
/* 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:
|
next_status:
|
||||||
if (!receive_status(file_descriptor, &status))
|
if (!receive_status(file_descriptor, &status))
|
||||||
goto receive_error;
|
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;
|
goto fail;
|
||||||
}
|
}
|
||||||
if (status != STATUS_FINISHED) {
|
if (status != STATUS_FINISHED) {
|
||||||
|
|||||||
+5
-4
@@ -609,10 +609,11 @@ void handler(int file_descriptor) {
|
|||||||
if (gate_ctx.super_mode_override != -1)
|
if (gate_ctx.super_mode_override != -1)
|
||||||
config->super_mode = (SuperMode)gate_ctx.super_mode_override;
|
config->super_mode = (SuperMode)gate_ctx.super_mode_override;
|
||||||
protocol_set_8_bit_output(config->eight_bit_output);
|
protocol_set_8_bit_output(config->eight_bit_output);
|
||||||
/* Honor the negotiated --timeout for every protocol frame from here on (the
|
/* Server-side per-message protocol deadline for every frame from here on.
|
||||||
* config handshake itself used the built-in 60 s window). A positive value
|
* `timeout` is not serialized, so this is the server's own config (the server
|
||||||
* also tightens the socket SO_RCVTIMEO/SO_SNDTIMEO already applied by the
|
* has no --timeout CLI and defaults it to 0): the built-in 60 s window stays
|
||||||
* transport; 0 leaves both built-in defaults in place. */
|
* 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);
|
protocol_session_set_io_timeout(&session, config->timeout);
|
||||||
if (!authorized_root) {
|
if (!authorized_root) {
|
||||||
log_message(LOG_LEVEL_ERROR, "No server-side destination root configured");
|
log_message(LOG_LEVEL_ERROR, "No server-side destination root configured");
|
||||||
|
|||||||
@@ -145,7 +145,7 @@ bool file_send_sendfile_with_skip(File* file, int file_descriptor, bool use_meta
|
|||||||
off_t offset = 0;
|
off_t offset = 0;
|
||||||
struct timespec deadline;
|
struct timespec deadline;
|
||||||
clock_gettime(CLOCK_MONOTONIC, &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) {
|
while ((unsigned long long)offset < file_size) {
|
||||||
struct timespec now;
|
struct timespec now;
|
||||||
clock_gettime(CLOCK_MONOTONIC, &now);
|
clock_gettime(CLOCK_MONOTONIC, &now);
|
||||||
|
|||||||
@@ -87,6 +87,12 @@ void protocol_session_set_io_timeout(ProtocolSession* session, int sec) {
|
|||||||
session->io_timeout_sec = 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) {
|
void protocol_session_set_max_alloc(ProtocolSession* session, unsigned long long max_alloc) {
|
||||||
if (!session)
|
if (!session)
|
||||||
session = bound_session ? bound_session : &legacy_io_session;
|
session = bound_session ? bound_session : &legacy_io_session;
|
||||||
|
|||||||
@@ -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
|
* 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. */
|
* protocol_receive_status_timed and is unaffected by this setter. */
|
||||||
void protocol_session_set_io_timeout(ProtocolSession* session, int sec);
|
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_alloc(size_t size);
|
||||||
void* protocol_realloc(void* ptr, size_t size);
|
void* protocol_realloc(void* ptr, size_t size);
|
||||||
void protocol_session_set_8_bit_output(ProtocolSession* session, bool enabled);
|
void protocol_session_set_8_bit_output(ProtocolSession* session, bool enabled);
|
||||||
|
|||||||
@@ -62,19 +62,28 @@ static void test_receiver_aborts_idle_keepalive() {
|
|||||||
protocol_session_bind(&session);
|
protocol_session_bind(&session);
|
||||||
|
|
||||||
Status keepalive = STATUS_KEEPALIVE;
|
Status keepalive = STATUS_KEEPALIVE;
|
||||||
EXPECT_EQ_INT((int)write(sv[0], &keepalive, sizeof(keepalive)), (int)sizeof(keepalive));
|
ssize_t wrote = write(sv[0], &keepalive, sizeof(keepalive));
|
||||||
int result = receiver_process_pending(config, sv[1], &sink, NULL);
|
int result = -2;
|
||||||
EXPECT_EQ_INT(result, -1);
|
if (wrote == (ssize_t)sizeof(keepalive))
|
||||||
|
result = receiver_process_pending(config, sv[1], &sink, NULL);
|
||||||
Status reply = STATUS_OK;
|
Status reply = STATUS_OK;
|
||||||
EXPECT_EQ_INT((int)read(sv[0], &reply, sizeof(reply)), (int)sizeof(reply));
|
ssize_t got = -1;
|
||||||
EXPECT_EQ_INT((int)reply, (int)STATUS_ERROR);
|
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();
|
protocol_session_unbind();
|
||||||
config_delete(config);
|
config_delete(config);
|
||||||
close(sv[0]);
|
close(sv[0]);
|
||||||
close(sv[1]);
|
close(sv[1]);
|
||||||
receiver_reset_time_limits();
|
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) {
|
void test_receiver_timeout(void) {
|
||||||
|
|||||||
Reference in New Issue
Block a user