From 1462efb97704e240e735f60c2baa8efedf54f4bd Mon Sep 17 00:00:00 2001 From: gold-silver-copper <157451540+gold-silver-copper@users.noreply.github.com> Date: Sat, 7 Nov 2026 21:39:32 -0800 Subject: [PATCH 1/3] test(basics): a helper timeout too long for a deadline is refused --- plugins/rig-basics/src/helpers/tests.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/plugins/rig-basics/src/helpers/tests.rs b/plugins/rig-basics/src/helpers/tests.rs index 73f896fbc6..cda808337e 100644 --- a/plugins/rig-basics/src/helpers/tests.rs +++ b/plugins/rig-basics/src/helpers/tests.rs @@ -575,6 +575,27 @@ fn the_helpers_table_reads_its_keys_and_refuses_zero() { assert!(read("[helpers]\ntimeout_secs = 0\n").is_err()); } +/// A timeout too long for a deadline is refused, naming the value: a +/// call's `timeout` when called, `[helpers] timeout_secs` when read. +#[test] +fn a_timeout_too_long_for_a_deadline_is_refused() { + let default = HelperSettings::default().timeout_secs; + let asked = timeout(Some(u64::MAX), default); + assert!( + asked + .as_ref() + .is_err_and(|why| why.contains(&u64::MAX.to_string())), + "a call's timeout too long for a deadline: {asked:?}" + ); + let most = i64::MAX.to_string(); + let read = Settings::from_toml(&format!("[helpers]\ntimeout_secs = {most}\n")) + .get::("helpers"); + assert!( + read.as_ref().is_err_and(|why| why.contains(&most)), + "a timeout_secs too long for a deadline: {read:?}" + ); +} + /// A helper past its deadline fails its task, naming where the deadline /// came from. #[test] From 764a8f329beb323d7bdfc73cbca04f5c05bd33d8 Mon Sep 17 00:00:00 2001 From: gold-silver-copper <157451540+gold-silver-copper@users.noreply.github.com> Date: Sat, 7 Nov 2026 21:40:00 -0800 Subject: [PATCH 2/3] fix(basics): a helper timeout too long for a deadline is refused A helper call's `timeout`, or `[helpers] timeout_secs`, so large that now plus it overflowed the clock made no deadline at all: the helper could run forever. Such a value is now refused naming it, a call's `timeout` when the call is made and `timeout_secs` when settings are read, as 0 already is. --- plugins/rig-basics/src/helpers.rs | 23 +++++++++++++++++++++-- plugins/rig-basics/src/helpers/tests.rs | 4 ++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/plugins/rig-basics/src/helpers.rs b/plugins/rig-basics/src/helpers.rs index 996b1dc2a1..8e5c3cab53 100644 --- a/plugins/rig-basics/src/helpers.rs +++ b/plugins/rig-basics/src/helpers.rs @@ -83,6 +83,7 @@ pub struct HelpersPlugin; #[serde(default, deny_unknown_fields)] struct HelperSettings { /// Seconds, 600 by default. + #[serde(deserialize_with = "deadline_secs")] timeout_secs: NonZeroU64, /// Tasks at once, 4 by default. at_once: NonZeroUsize, @@ -410,17 +411,35 @@ fn start( } /// How long each helper of a call may take: the call's `timeout`, at least -/// 1 second and as long as it says, else `default`. +/// 1 second and short enough for a deadline, else `default`. fn timeout(asked: Option, default: NonZeroU64) -> Result { match asked { Some(0) => Err(format!( "`timeout` must be at least 1 second; leave it out for {default} s (`[helpers] \ timeout_secs`)" )), - asked => Ok(Duration::from_secs(asked.unwrap_or(default.get()))), + asked => deadline(asked.unwrap_or(default.get())), } } +/// `secs` as a helper's time limit, refused when a deadline that far from +/// now does not fit the clock, which would read as no deadline. +fn deadline(secs: u64) -> Result { + let limit = Duration::from_secs(secs); + match Instant::now().checked_add(limit) { + Some(_) => Ok(limit), + None => Err(format!("a timeout of {secs} s is too long for a deadline")), + } +} + +/// A `timeout_secs` that [`deadline`] takes. +fn deadline_secs<'de, D: serde::Deserializer<'de>>(read: D) -> Result { + let secs = NonZeroU64::deserialize(read)?; + deadline(secs.get()) + .map(|_| secs) + .map_err(serde::de::Error::custom) +} + /// Runs a call's children, on a thread of its own. struct Runner { command: HelperCommand, diff --git a/plugins/rig-basics/src/helpers/tests.rs b/plugins/rig-basics/src/helpers/tests.rs index cda808337e..5adca15501 100644 --- a/plugins/rig-basics/src/helpers/tests.rs +++ b/plugins/rig-basics/src/helpers/tests.rs @@ -549,8 +549,8 @@ fn a_helper_in_tilde_is_in_the_home_directory() { ); } -/// A call's `timeout` is each helper's deadline as given, however long; a -/// call without one gets `[helpers] timeout_secs`, 600 s by default. +/// A call's `timeout` is each helper's deadline as given; a call without +/// one gets `[helpers] timeout_secs`, 600 s by default. #[test] fn a_helpers_deadline_is_the_calls_timeout_or_the_setting() { let default = HelperSettings::default().timeout_secs; From 99a327c887b2ca206cb9fb5033ef1af1e7b3da8f Mon Sep 17 00:00:00 2001 From: gold-silver-copper <157451540+gold-silver-copper@users.noreply.github.com> Date: Sat, 7 Nov 2026 22:30:16 -0800 Subject: [PATCH 3/3] fix(basics): a helper timeout past a fixed ceiling is refused Checking a timeout against the clock when it is read let a value just under the limit pass and overflow for a helper started later, and the limit moved with uptime. A timeout is now refused past u32::MAX seconds, about 136 years, which every platform's clock adds to now. A deadline that still does not fit refuses the helper instead of meaning none. --- plugins/rig-basics/src/helpers.rs | 25 +++++++++++++------------ plugins/rig-basics/src/helpers/tests.rs | 14 +++++++------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/plugins/rig-basics/src/helpers.rs b/plugins/rig-basics/src/helpers.rs index 8e5c3cab53..831b7bae86 100644 --- a/plugins/rig-basics/src/helpers.rs +++ b/plugins/rig-basics/src/helpers.rs @@ -411,7 +411,7 @@ fn start( } /// How long each helper of a call may take: the call's `timeout`, at least -/// 1 second and short enough for a deadline, else `default`. +/// 1 second and at most [`MOST_SECS`], else `default`. fn timeout(asked: Option, default: NonZeroU64) -> Result { match asked { Some(0) => Err(format!( @@ -422,22 +422,21 @@ fn timeout(asked: Option, default: NonZeroU64) -> Result } } -/// `secs` as a helper's time limit, refused when a deadline that far from -/// now does not fit the clock, which would read as no deadline. +/// The longest a helper may take, about 136 years, which any clock adds to now. +const MOST_SECS: u64 = u32::MAX as u64; +/// `secs` as a helper's time limit, refused past [`MOST_SECS`]. fn deadline(secs: u64) -> Result { - let limit = Duration::from_secs(secs); - match Instant::now().checked_add(limit) { - Some(_) => Ok(limit), - None => Err(format!("a timeout of {secs} s is too long for a deadline")), + if secs > MOST_SECS { + return Err(format!("a timeout of {secs} s is over {MOST_SECS} s")); } + Ok(Duration::from_secs(secs)) } /// A `timeout_secs` that [`deadline`] takes. fn deadline_secs<'de, D: serde::Deserializer<'de>>(read: D) -> Result { let secs = NonZeroU64::deserialize(read)?; - deadline(secs.get()) - .map(|_| secs) - .map_err(serde::de::Error::custom) + deadline(secs.get()).map_err(serde::de::Error::custom)?; + Ok(secs) } /// Runs a call's children, on a thread of its own. @@ -576,6 +575,9 @@ impl Runner { grace: Some(STOP_WINDOW), ..Spawn::default() }; + let Some(deadline) = Instant::now().checked_add(self.timeout) else { + return (Err("no deadline".to_owned()), String::new()); + }; // Unsettled from before it can spend until what it spent is known. self.reports.unsettled.fetch_add(1, Ordering::SeqCst); let mut process = match launcher.spawn(&self.command.program, args, how) { @@ -599,8 +601,7 @@ impl Runner { } }; let mut read = Read::new(self.reports.clone()); - let deadline = Instant::now().checked_add(self.timeout); - let ended = process.run(deadline, |piece| { + let ended = process.run(Some(deadline), |piece| { read.piece(piece); true }); diff --git a/plugins/rig-basics/src/helpers/tests.rs b/plugins/rig-basics/src/helpers/tests.rs index 5adca15501..098cb4b92b 100644 --- a/plugins/rig-basics/src/helpers/tests.rs +++ b/plugins/rig-basics/src/helpers/tests.rs @@ -580,18 +580,18 @@ fn the_helpers_table_reads_its_keys_and_refuses_zero() { #[test] fn a_timeout_too_long_for_a_deadline_is_refused() { let default = HelperSettings::default().timeout_secs; - let asked = timeout(Some(u64::MAX), default); + let most = u64::from(u32::MAX); + assert_eq!(timeout(Some(most), default), Ok(Duration::from_secs(most))); + let past = most.saturating_add(1).to_string(); + let asked = timeout(Some(most.saturating_add(1)), default); assert!( - asked - .as_ref() - .is_err_and(|why| why.contains(&u64::MAX.to_string())), + asked.as_ref().is_err_and(|why| why.contains(&past)), "a call's timeout too long for a deadline: {asked:?}" ); - let most = i64::MAX.to_string(); - let read = Settings::from_toml(&format!("[helpers]\ntimeout_secs = {most}\n")) + let read = Settings::from_toml(&format!("[helpers]\ntimeout_secs = {past}\n")) .get::("helpers"); assert!( - read.as_ref().is_err_and(|why| why.contains(&most)), + read.as_ref().is_err_and(|why| why.contains(&past)), "a timeout_secs too long for a deadline: {read:?}" ); }