mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-14 19:27:05 -04:00
Never crash: reject deeply nested expressions instead of overflowing the stack
An expression nested about 60 brackets deep overflowed the stack of a 2 MiB thread in the JavaScript parser (release and debug; ~200 levels on 8 MiB), which aborts the whole app: it can't be caught. The array-maths rewrite also nests one call per arithmetic operator, so a long sum went the same way, and the rewriter itself recursed per bracket. rewrite() now measures the nesting (brackets plus chains of prefix operators) before and after rewriting and turns an expression deeper than MAX_NESTING (32) into a RangeError; check_syntax reports the same error. Also replaces the unreachable!() in the bracket-tree builder with a let-chain, and denies the panic lints in the crate. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
10644a83aa
commit
fc8fea031c
3 files changed
+86
-5
No files matched your search
@@ -24,6 +24,8 @@
|
||||
//! the project through memoized host requests (see [`host`]), so nothing borrowed ever enters
|
||||
//! the JS heap. Expressions reading each other nest up to [`MAX_DEPTH`] levels.
|
||||
|
||||
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
|
||||
|
||||
pub mod host;
|
||||
mod noise;
|
||||
pub mod rewrite;
|
||||
@@ -182,6 +184,7 @@ pub fn eval_property(project: &Project, comp: ItemId, layer: LayerId, path: &str
|
||||
|
||||
/// Syntax-check an expression (after the array-maths rewrite) without evaluating it.
|
||||
pub fn check_syntax(text: &str) -> Result<(), String> {
|
||||
rewrite::check_nesting(text)?;
|
||||
runtime::check(text)
|
||||
}
|
||||
|
||||
|
||||
@@ -304,8 +304,11 @@ fn build(toks: Vec<Tok>) -> Vec<Item> {
|
||||
for t in toks {
|
||||
if t.kind == Kind::Punct && matches!(t.text.as_str(), "(" | "[" | "{") {
|
||||
stack.push((t, std::mem::take(&mut cur)));
|
||||
} else if t.kind == Kind::Punct && matches!(t.text.as_str(), ")" | "]" | "}") && stack.last().is_some_and(|(o, _)| closer(&o.text) == t.text) {
|
||||
let (open, parent) = stack.pop().unwrap_or_else(|| unreachable!());
|
||||
} else if t.kind == Kind::Punct
|
||||
&& matches!(t.text.as_str(), ")" | "]" | "}")
|
||||
&& stack.last().is_some_and(|(o, _)| closer(&o.text) == t.text)
|
||||
&& let Some((open, parent)) = stack.pop()
|
||||
{
|
||||
let inner = std::mem::replace(&mut cur, parent);
|
||||
cur.push(Item::Group { open, inner, close: Some(t) });
|
||||
} else {
|
||||
@@ -545,17 +548,67 @@ fn push_piece(out: &mut String, p: Piece) {
|
||||
out.push_str(&p.body);
|
||||
}
|
||||
|
||||
/// Rewrite an expression's source for array-aware arithmetic. Line breaks are preserved.
|
||||
pub fn rewrite(src: &str) -> String {
|
||||
/// The deepest nesting (brackets, plus chains of prefix operators) an expression may have,
|
||||
/// before and after the rewrite (which nests a call per arithmetic operator). The JavaScript
|
||||
/// parser recurses per level and a 2 MiB thread overflows its stack (aborting the app) at
|
||||
/// about 60 levels, so deeper expressions are rejected with an error instead.
|
||||
pub const MAX_NESTING: usize = 32;
|
||||
|
||||
fn nesting(toks: &[Tok]) -> usize {
|
||||
let (mut depth, mut prefix, mut max) = (0usize, 0usize, 0usize);
|
||||
for t in toks {
|
||||
if t.kind == Kind::Punct && matches!(t.text.as_str(), "(" | "[" | "{") {
|
||||
depth += 1;
|
||||
} else if t.kind == Kind::Punct && matches!(t.text.as_str(), ")" | "]" | "}") {
|
||||
depth = depth.saturating_sub(1);
|
||||
}
|
||||
let is_prefix = (t.kind == Kind::Punct && matches!(t.text.as_str(), "!" | "~" | "-" | "+")) || t.is_kw(&["typeof", "void", "delete"]);
|
||||
prefix = if is_prefix { prefix + 1 } else { 0 };
|
||||
max = max.max(depth + prefix);
|
||||
}
|
||||
max
|
||||
}
|
||||
|
||||
/// An error when `src` nests deeper than [`MAX_NESTING`] levels (before or after the rewrite).
|
||||
pub fn check_nesting(src: &str) -> Result<(), String> {
|
||||
let (toks, tail) = tokenize(src);
|
||||
if nesting(&toks) > MAX_NESTING {
|
||||
return Err(too_deep());
|
||||
}
|
||||
let mut out = rewrite_seq(&build(toks));
|
||||
out.push_str(&tail);
|
||||
if nesting(&tokenize(&out).0) > MAX_NESTING {
|
||||
return Err(too_deep());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn too_deep() -> String {
|
||||
format!("Error: expression is nested too deeply (more than {MAX_NESTING} levels of brackets or chained operators)")
|
||||
}
|
||||
|
||||
/// Rewrite an expression's source for array-aware arithmetic. Line breaks are preserved.
|
||||
/// An expression nested deeper than [`MAX_NESTING`] becomes a `throw` of that error.
|
||||
pub fn rewrite(src: &str) -> String {
|
||||
let (toks, tail) = tokenize(src);
|
||||
if nesting(&toks) > MAX_NESTING {
|
||||
return throw_too_deep();
|
||||
}
|
||||
let mut out = rewrite_seq(&build(toks));
|
||||
out.push_str(&tail);
|
||||
if nesting(&tokenize(&out).0) > MAX_NESTING {
|
||||
return throw_too_deep();
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
fn throw_too_deep() -> String {
|
||||
format!("throw new RangeError({:?})", too_deep().trim_start_matches("Error: "))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::rewrite;
|
||||
use super::{MAX_NESTING, check_nesting, rewrite};
|
||||
|
||||
/// Compare ignoring spaces (newlines are significant).
|
||||
fn n(s: &str) -> String {
|
||||
@@ -599,4 +652,25 @@ mod tests {
|
||||
assert_eq!(n(&rewrite("i++ + 1")), n("__add(i++, 1)"));
|
||||
assert_eq!(n(&rewrite("2 ** 3 * 2")), n("__mul(2 ** 3, 2)"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deep_nesting_is_an_error_not_a_stack_overflow() {
|
||||
// Each level costs the JavaScript parser stack: ~60 nested brackets overflowed a 2 MiB
|
||||
// thread and aborted the app; 100 000 overflowed the rewriter itself.
|
||||
for n in [MAX_NESTING + 1, 100, 100_000] {
|
||||
let parens = "(".repeat(n) + "1" + &")".repeat(n);
|
||||
let unclosed = "[".repeat(n);
|
||||
let sum = vec!["x"; n + 1].join(" + ");
|
||||
let nots = "!".repeat(n) + "x";
|
||||
for src in [parens, unclosed, sum, nots] {
|
||||
assert!(rewrite(&src).starts_with("throw new RangeError("), "{n}");
|
||||
assert!(check_nesting(&src).is_err());
|
||||
assert!(crate::check_syntax(&src).unwrap_err().contains("nested too deeply"));
|
||||
}
|
||||
}
|
||||
let ok = "(".repeat(MAX_NESTING - 2) + "a + b" + &")".repeat(MAX_NESTING - 2);
|
||||
assert!(check_nesting(&ok).is_ok());
|
||||
assert!(crate::check_syntax(&ok).is_ok());
|
||||
assert!(check_nesting(&vec!["x"; 20].join(" + ")).is_ok());
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user