diff --git a/crates/monty-cli/src/main.rs b/crates/monty-cli/src/main.rs index 27307256..567f7749 100644 --- a/crates/monty-cli/src/main.rs +++ b/crates/monty-cli/src/main.rs @@ -88,16 +88,17 @@ struct Cli { impl Cli { /// Builds `ResourceLimits` from the parsed CLI arguments. /// - /// Returns `None` when no resource flags were provided, which lets the + /// Returns `Ok(None)` when no resource flags were provided, which lets the /// caller fall back to `NoLimitTracker` for zero-overhead execution. - fn resource_limits(&self) -> Option { + /// Returns `Err` if a supplied flag cannot be converted into a valid limit. + fn resource_limits(&self) -> Result, String> { if self.max_allocations.is_none() && self.max_duration.is_none() && self.max_memory.is_none() && self.gc_interval.is_none() && self.max_recursion_depth.is_none() { - return None; + return Ok(None); } let mut limits = ResourceLimits::new(); @@ -105,7 +106,9 @@ impl Cli { limits = limits.max_allocations(n); } if let Some(secs) = self.max_duration { - limits = limits.max_duration(Duration::from_secs_f64(secs)); + limits = limits.max_duration( + Duration::try_from_secs_f64(secs).map_err(|err| format!("invalid --max-duration: {err}"))?, + ); } if let Some(bytes) = self.max_memory { limits = limits.max_memory(bytes); @@ -116,7 +119,7 @@ impl Cli { if let Some(depth) = self.max_recursion_depth { limits = limits.max_recursion_depth(Some(depth)); } - Some(limits) + Ok(Some(limits)) } } @@ -126,7 +129,14 @@ fn main() -> ExitCode { let cli = Cli::parse(); let type_check_enabled = cli.type_check; - let limits = cli.resource_limits(); + + let limits = match cli.resource_limits() { + Ok(limits) => limits, + Err(err) => { + eprintln!("{BOLD_RED}error{RESET}: {err}"); + return ExitCode::FAILURE; + } + }; // Build mount table early to fail fast on bad -m args. let mount_table = match build_mount_table(&cli.mounts) { diff --git a/crates/monty-js/__test__/limits.spec.ts b/crates/monty-js/__test__/limits.spec.ts index 44071f63..08dce848 100644 --- a/crates/monty-js/__test__/limits.spec.ts +++ b/crates/monty-js/__test__/limits.spec.ts @@ -76,6 +76,12 @@ len(result) t.true(error.message.includes('MemoryError')) }) +test('allocation limit accepts values above u32 max', (t) => { + const m = new Monty('1 + 1') + const limits: ResourceLimits = { maxAllocations: 2 ** 33 } + t.is(m.run({ limits }), 2) +}) + // ============================================================================= // Memory limit tests // ============================================================================= @@ -93,6 +99,12 @@ len(result) t.true(error.message.includes('MemoryError')) }) +test('memory limit accepts values above u32 max', (t) => { + const m = new Monty('1 + 1') + const limits: ResourceLimits = { maxMemory: 2 ** 33 } + t.is(m.run({ limits }), 2) +}) + // ============================================================================= // Limits with inputs tests // ============================================================================= diff --git a/crates/monty-js/src/limits.rs b/crates/monty-js/src/limits.rs index ea688dd1..a2a9f2c1 100644 --- a/crates/monty-js/src/limits.rs +++ b/crates/monty-js/src/limits.rs @@ -6,48 +6,103 @@ use std::time::Duration; use monty::{ResourceLimits, DEFAULT_MAX_RECURSION_DEPTH}; +use napi::{Error, Result, Status}; use napi_derive::napi; /// Resource limits configuration from JavaScript. /// /// All limits are optional. Omit a key to disable that limit. +/// Numeric limits are received as JS `number`s, so the boundary uses `f64` +/// and validates them before converting into Rust `usize` values. #[napi(object, js_name = "ResourceLimits")] #[derive(Debug, Clone, Copy, Default)] pub struct JsResourceLimits { /// Maximum number of heap allocations allowed. - pub max_allocations: Option, + pub max_allocations: Option, /// Maximum execution time in seconds. pub max_duration_secs: Option, /// Maximum heap memory in bytes. - pub max_memory: Option, + pub max_memory: Option, /// Run garbage collection every N allocations. - pub gc_interval: Option, + pub gc_interval: Option, /// Maximum function call stack depth (default: 1000). - pub max_recursion_depth: Option, + pub max_recursion_depth: Option, } -impl From for ResourceLimits { - fn from(js_limits: JsResourceLimits) -> Self { - let max_recursion_depth = js_limits - .max_recursion_depth - .map(|v| v as usize) - .or(Some(DEFAULT_MAX_RECURSION_DEPTH)); +/// Extracts a Rust resource-limit configuration from a JS resource-limit object. +/// +/// This mirrors the Python binding convention: validate host-provided limits at +/// the boundary, then convert them into Monty's internal `ResourceLimits`. +/// +/// Returns `Err` for invalid JS number inputs that cannot be represented as +/// `usize` or for invalid duration values rejected by +/// `std::time::Duration::try_from_secs_f64`. +pub fn extract_limits(js_limits: JsResourceLimits) -> Result { + let max_recursion_depth = js_limits + .max_recursion_depth + .map(|v| js_number_to_usize(v, "maxRecursionDepth")) + .transpose()? + .or(Some(DEFAULT_MAX_RECURSION_DEPTH)); - let mut limits = Self::new().max_recursion_depth(max_recursion_depth); + let mut limits = ResourceLimits::new().max_recursion_depth(max_recursion_depth); - if let Some(max) = js_limits.max_allocations { - limits = limits.max_allocations(max as usize); - } - if let Some(secs) = js_limits.max_duration_secs { - limits = limits.max_duration(Duration::from_secs_f64(secs)); - } - if let Some(max) = js_limits.max_memory { - limits = limits.max_memory(max as usize); - } - if let Some(interval) = js_limits.gc_interval { - limits = limits.gc_interval(interval as usize); - } + if let Some(max) = js_limits.max_allocations { + limits = limits.max_allocations(js_number_to_usize(max, "maxAllocations")?); + } + if let Some(secs) = js_limits.max_duration_secs { + limits = limits.max_duration( + Duration::try_from_secs_f64(secs).map_err(|err| Error::new(Status::InvalidArg, err.to_string()))?, + ); + } + if let Some(max) = js_limits.max_memory { + limits = limits.max_memory(js_number_to_usize(max, "maxMemory")?); + } + if let Some(interval) = js_limits.gc_interval { + limits = limits.gc_interval(js_number_to_usize(interval, "gcInterval")?); + } - limits + Ok(limits) +} + +impl TryFrom for ResourceLimits { + type Error = Error; + + fn try_from(js_limits: JsResourceLimits) -> Result { + extract_limits(js_limits) + } +} + +/// Converts a JavaScript `number` used for a size/count limit into `usize`. +/// +/// JavaScript numbers are IEEE-754 doubles, so integers above `2^53 - 1` +/// cannot be represented exactly. Rejecting values outside the safe integer +/// range avoids silently rounding resource limits at the napi boundary. +/// +/// Returns `Err` for non-finite, negative, fractional, or out-of-range inputs. +/// This helper does not panic. +fn js_number_to_usize(value: f64, name: &str) -> Result { + const JS_MAX_SAFE_INTEGER: u64 = (1_u64 << 53) - 1; + + match value { + v if !v.is_finite() => Err(Error::new( + Status::InvalidArg, + format!("{name} must be a finite number"), + )), + v if v < 0.0 => Err(Error::new(Status::InvalidArg, format!("{name} must be non-negative"))), + v if v.fract() != 0.0 => Err(Error::new(Status::InvalidArg, format!("{name} must be an integer"))), + v if v > JS_MAX_SAFE_INTEGER as f64 => Err(Error::new( + Status::InvalidArg, + format!("{name} must be a safe integer (<= {JS_MAX_SAFE_INTEGER})"), + )), + v => { + #[expect(clippy::cast_possible_truncation, clippy::cast_sign_loss)] + let value = v as u64; + usize::try_from(value).map_err(|_| { + Error::new( + Status::InvalidArg, + format!("{name} must fit in Rust usize on this platform"), + ) + }) + } } } diff --git a/crates/monty-js/src/monty_cls.rs b/crates/monty-js/src/monty_cls.rs index a705815c..154cc53d 100644 --- a/crates/monty-js/src/monty_cls.rs +++ b/crates/monty-js/src/monty_cls.rs @@ -226,7 +226,7 @@ impl Monty { } let result = if let Some(limits) = options.limits { - let tracker = LimitedTracker::new(limits.into()); + let tracker = LimitedTracker::new(limits.try_into()?); self.runner.run(input_values, tracker, print_writer) } else { let tracker = NoLimitTracker; @@ -334,8 +334,14 @@ impl Monty { }}; } - if let Some(limits) = limits { - let tracker = LimitedTracker::new(limits.into()); + if let Some(resource_limits) = limits { + let tracker = match resource_limits.try_into() { + Ok(r) => LimitedTracker::new(r), + Err(err) => { + put_back(mount_table); + return Err(err); + } + }; run_loop!(tracker) } else { run_loop!(NoLimitTracker) @@ -383,8 +389,14 @@ impl Monty { let print_callback_ref = options.print_callback.as_ref().map(Function::create_ref).transpose()?; // Start execution with appropriate tracker - if let Some(limits) = options.limits { - let tracker = LimitedTracker::new(limits.into()); + if let Some(resource_limits) = options.limits { + let tracker = match resource_limits.try_into() { + Ok(r) => LimitedTracker::new(r), + Err(err) => { + put_back_mount_state(mount_state); + return Err(err); + } + }; let progress = match runner.start(input_values, tracker, print_writer) { Ok(p) => p, Err(exc) => { @@ -548,22 +560,21 @@ impl MontyRepl { /// /// @param options - Optional configuration (scriptName, limits) #[napi(constructor)] - #[must_use] - pub fn new(options: Option) -> Self { + pub fn new(options: Option) -> Result { let options = options.unwrap_or_default(); let script_name = options.script_name.unwrap_or_else(|| "main.py".to_string()); let repl = if let Some(limits) = options.limits { - let tracker = LimitedTracker::new(limits.into()); + let tracker = LimitedTracker::new(limits.try_into()?); EitherRepl::Limited(CoreMontyRepl::new(&script_name, tracker)) } else { EitherRepl::NoLimit(CoreMontyRepl::new(&script_name, NoLimitTracker)) }; - Self { + Ok(Self { repl: Some(repl), script_name, - } + }) } /// Returns the script name for this REPL session. diff --git a/crates/monty-python/src/limits.rs b/crates/monty-python/src/limits.rs index 73ca933d..87289d7f 100644 --- a/crates/monty-python/src/limits.rs +++ b/crates/monty-python/src/limits.rs @@ -12,7 +12,7 @@ use std::{ }; use monty::{DEFAULT_MAX_RECURSION_DEPTH, ExcType, MontyException, ResourceError, ResourceTracker}; -use pyo3::{prelude::*, types::PyDict}; +use pyo3::{exceptions::PyValueError, prelude::*, types::PyDict}; use crate::exceptions::exc_py_to_monty; @@ -32,6 +32,7 @@ pub(crate) type CancellationFlag = Arc; /// (except `max_recursion_depth` which defaults to 1000). /// /// Raises `TypeError` if a value is present but has the wrong type. +/// Raises `ValueError` if `max_duration_secs` is not a valid duration value. pub fn extract_limits(dict: &Bound<'_, PyDict>) -> PyResult { let max_allocations = extract_optional_usize(dict, "max_allocations")?; let max_duration_secs = extract_optional_f64(dict, "max_duration_secs")?; @@ -46,7 +47,8 @@ pub fn extract_limits(dict: &Bound<'_, PyDict>) -> PyResult