From 9b544a000bf165eddb1890fb44be0cda0c5185c2 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 24 May 2022 14:14:48 +0200 Subject: [PATCH] fix: Don't stackoverflow on deeply nested expressions --- libflux/flux-core/src/ast/check/mod.rs | 50 ++++++++++++++++++++++---- libflux/flux-core/src/formatter/mod.rs | 1 - libflux/flux-core/src/semantic/mod.rs | 14 ++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/libflux/flux-core/src/ast/check/mod.rs b/libflux/flux-core/src/ast/check/mod.rs index 3e9186784e..c8af245333 100644 --- a/libflux/flux-core/src/ast/check/mod.rs +++ b/libflux/flux-core/src/ast/check/mod.rs @@ -11,9 +11,31 @@ use crate::{ /// Inspects an AST node and returns a list of found AST errors plus /// any errors existed before `ast.check()` is performed. pub fn check(node: walk::Node) -> Result<(), Errors> { - let mut errors = Errors::new(); - walk::walk( - &mut |n: walk::Node| { + const MAX_DEPTH: u32 = 1000; + + #[derive(Default)] + struct Check { + depth: u32, + errors: Errors, + } + + impl<'a> walk::Visitor<'a> for Check { + fn visit(&mut self, n: walk::Node<'a>) -> bool { + self.depth += 1; + + let errors = &mut self.errors; + + if self.depth > MAX_DEPTH { + errors.push(located( + n.base().location.clone(), + ErrorKind { + message: String::from("Program is nested to deep"), + }, + )); + + return false; + } + // collect any errors we found prior to ast.check(). for err in n.base().errors.iter() { errors.push(located( @@ -76,9 +98,19 @@ pub fn check(node: walk::Node) -> Result<(), Errors> { } _ => {} } - }, - node, - ); + + true + } + + fn done(&mut self, _: walk::Node<'a>) { + self.depth -= 1; + } + } + + let mut check = Check::default(); + walk::walk(&mut check, node); + let errors = check.errors; + if errors.is_empty() { Ok(()) } else { @@ -97,6 +129,12 @@ pub struct ErrorKind { pub message: String, } +impl ErrorKind { + pub(crate) fn is_fatal(&self) -> bool { + self.message.contains("Program is nested to deep") + } +} + impl AsDiagnostic for ErrorKind { fn as_diagnostic(&self, _source: &dyn crate::semantic::Source) -> diagnostic::Diagnostic<()> { diagnostic::Diagnostic::error().with_message(self.to_string()) diff --git a/libflux/flux-core/src/formatter/mod.rs b/libflux/flux-core/src/formatter/mod.rs index 1b63fed509..d704476b88 100644 --- a/libflux/flux-core/src/formatter/mod.rs +++ b/libflux/flux-core/src/formatter/mod.rs @@ -28,7 +28,6 @@ pub fn format(contents: &str) -> Result { let file = parse_string("".to_string(), contents); let node = ast::walk::Node::File(&file); ast::check::check(node)?; - convert_to_string(&file) } diff --git a/libflux/flux-core/src/semantic/mod.rs b/libflux/flux-core/src/semantic/mod.rs index 93d4e39201..a8e56c9afb 100644 --- a/libflux/flux-core/src/semantic/mod.rs +++ b/libflux/flux-core/src/semantic/mod.rs @@ -555,7 +555,21 @@ impl<'env, I: import::Importer> Analyzer<'env, I> { let mut errors = Errors::new(); if let Err(err) = ast::check::check(ast::walk::Node::Package(ast_pkg)) { + let has_fatal_error = err.iter().any(|e| e.error.is_fatal()); errors.extend(err.into_iter().map(Error::from)); + if has_fatal_error { + return Err(Salvage { + error: FileErrors { + file: ast_pkg.package.clone(), + source: None, + diagnostics: Diagnostics { + errors, + warnings: Errors::new(), + }, + }, + value: None, + }); + } } let mut sem_pkg = {