From 0046fe7e7380b122090f51903cab33bc1c849003 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 21 Nov 2020 15:08:18 +0800 Subject: [PATCH] Prefer Engine::disable_symbol to disable eval. --- RELEASES.md | 1 + doc/src/language/eval.md | 25 +++++------------- doc/src/rust/packages/builtin.md | 1 - src/packages/eval.rs | 14 ---------- src/packages/mod.rs | 2 -- src/parse_error.rs | 9 ++++--- src/parser.rs | 44 +++++++++++++++++--------------- src/syntax.rs | 26 +++++++++++-------- src/token.rs | 30 ++++++++++++---------- tests/eval.rs | 28 ++++++++++++++++++-- 10 files changed, 96 insertions(+), 84 deletions(-) delete mode 100644 src/packages/eval.rs diff --git a/RELEASES.md b/RELEASES.md index 1f0a1fed..bae0c5ef 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -20,6 +20,7 @@ Breaking changes * `Module::set_fn`, `Module::set_raw_fn` and `Module::set_fn_XXX_mut` all take an additional parameter of `FnNamespace`. * `unless` is now a reserved keyword. +* `EvalPackage` is removed in favor of `Engine::disable_symbol`. New features ------------ diff --git a/doc/src/language/eval.md b/doc/src/language/eval.md index fec9569a..0b9eb8b0 100644 --- a/doc/src/language/eval.md +++ b/doc/src/language/eval.md @@ -60,7 +60,13 @@ print(x); -------------- For those who subscribe to the (very sensible) motto of ["`eval` is evil"](http://linterrors.com/js/eval-is-evil), -disable `eval` by overloading it, probably with something that throws. +disable `eval` using [`Engine::disable_symbol`][disable keywords and operators]: + +```rust +engine.disable_symbol("eval"); // disable usage of 'eval' +``` + +`eval` can also be disabled by overloading it, probably with something that throws: ```rust fn eval(script) { throw "eval is evil! I refuse to run " + script } @@ -75,20 +81,3 @@ engine.register_result_fn("eval", |script: String| -> Result<(), Box Result> { - Err("eval is evil!".into()) - } -} diff --git a/src/packages/mod.rs b/src/packages/mod.rs index ede3da61..a4c443ed 100644 --- a/src/packages/mod.rs +++ b/src/packages/mod.rs @@ -6,7 +6,6 @@ use crate::{Module, Shared, StaticVec}; pub(crate) mod arithmetic; mod array_basic; -mod eval; mod fn_basic; mod iter_basic; mod logic; @@ -21,7 +20,6 @@ mod time_basic; pub use arithmetic::ArithmeticPackage; #[cfg(not(feature = "no_index"))] pub use array_basic::BasicArrayPackage; -pub use eval::EvalPackage; pub use fn_basic::BasicFnPackage; pub use iter_basic::BasicIteratorPackage; pub use logic::LogicPackage; diff --git a/src/parse_error.rs b/src/parse_error.rs index 9b851818..010864cf 100644 --- a/src/parse_error.rs +++ b/src/parse_error.rs @@ -32,7 +32,7 @@ pub enum LexError { /// An identifier is in an invalid format. MalformedIdentifier(String), /// Bad symbol encountered when tokenizing the script text. - ImproperSymbol(String), + ImproperSymbol(String, String), } impl Error for LexError {} @@ -47,7 +47,10 @@ impl fmt::Display for LexError { Self::MalformedIdentifier(s) => write!(f, "{}: '{}'", self.desc(), s), Self::UnterminatedString => f.write_str(self.desc()), Self::StringTooLong(max) => write!(f, "{} ({})", self.desc(), max), - Self::ImproperSymbol(s) => f.write_str(s), + Self::ImproperSymbol(s, d) if d.is_empty() => { + write!(f, "Invalid symbol encountered: '{}'", s) + } + Self::ImproperSymbol(_, d) => f.write_str(d), } } } @@ -62,7 +65,7 @@ impl LexError { Self::MalformedNumber(_) => "Invalid number", Self::MalformedChar(_) => "Invalid character", Self::MalformedIdentifier(_) => "Variable name is not proper", - Self::ImproperSymbol(_) => "Invalid symbol encountered", + Self::ImproperSymbol(_, _) => "Invalid symbol encountered", } } /// Convert a `&LexError` into a [`ParseError`]. diff --git a/src/parser.rs b/src/parser.rs index 2a7a4684..10ebf146 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -995,11 +995,8 @@ fn parse_primary( // Access to `this` as a variable is OK Token::Reserved(s) if s == KEYWORD_THIS && *next_token != Token::LeftParen => { if !settings.is_function_scope { - return Err(PERR::BadInput(LexError::ImproperSymbol(format!( - "'{}' can only be used in functions", - s - ))) - .into_err(settings.pos)); + let msg = format!("'{}' can only be used in functions", s); + return Err(PERR::BadInput(LexError::ImproperSymbol(s, msg)).into_err(settings.pos)); } else { let var_name_def = IdentX::new(state.get_interned_string(s), settings.pos); Expr::Variable(Box::new((None, None, 0, var_name_def))) @@ -1045,6 +1042,7 @@ fn parse_primary( LexError::UnexpectedInput(Token::Bang.syntax().to_string()).into_err(token_pos) } else { PERR::BadInput(LexError::ImproperSymbol( + "!".to_string(), "'!' cannot be used to call module functions".to_string(), )) .into_err(token_pos) @@ -1333,6 +1331,7 @@ fn make_assignment_stmt<'a>( } // ??? && ??? = rhs, ??? || ??? = rhs Expr::And(_, _) | Expr::Or(_, _) => Err(PERR::BadInput(LexError::ImproperSymbol( + "=".to_string(), "Possibly a typo of '=='?".to_string(), )) .into_err(pos)), @@ -1438,10 +1437,13 @@ fn make_dot_expr( && [crate::engine::KEYWORD_FN_PTR, crate::engine::KEYWORD_EVAL] .contains(&x.name.as_ref()) => { - return Err(PERR::BadInput(LexError::ImproperSymbol(format!( - "'{}' should not be called in method style. Try {}(...);", - x.name, x.name - ))) + return Err(PERR::BadInput(LexError::ImproperSymbol( + x.name.to_string(), + format!( + "'{}' should not be called in method style. Try {}(...);", + x.name, x.name + ), + )) .into_err(pos)); } // lhs.func!(...) @@ -1932,20 +1934,22 @@ fn ensure_not_statement_expr(input: &mut TokenStream, type_name: &str) -> Result fn ensure_not_assignment(input: &mut TokenStream) -> Result<(), ParseError> { match input.peek().unwrap() { (Token::Equals, pos) => Err(PERR::BadInput(LexError::ImproperSymbol( + "=".to_string(), "Possibly a typo of '=='?".to_string(), )) .into_err(*pos)), - (Token::PlusAssign, pos) - | (Token::MinusAssign, pos) - | (Token::MultiplyAssign, pos) - | (Token::DivideAssign, pos) - | (Token::LeftShiftAssign, pos) - | (Token::RightShiftAssign, pos) - | (Token::ModuloAssign, pos) - | (Token::PowerOfAssign, pos) - | (Token::AndAssign, pos) - | (Token::OrAssign, pos) - | (Token::XOrAssign, pos) => Err(PERR::BadInput(LexError::ImproperSymbol( + (token @ Token::PlusAssign, pos) + | (token @ Token::MinusAssign, pos) + | (token @ Token::MultiplyAssign, pos) + | (token @ Token::DivideAssign, pos) + | (token @ Token::LeftShiftAssign, pos) + | (token @ Token::RightShiftAssign, pos) + | (token @ Token::ModuloAssign, pos) + | (token @ Token::PowerOfAssign, pos) + | (token @ Token::AndAssign, pos) + | (token @ Token::OrAssign, pos) + | (token @ Token::XOrAssign, pos) => Err(PERR::BadInput(LexError::ImproperSymbol( + token.syntax().to_string(), "Expecting a boolean expression, not an assignment".to_string(), )) .into_err(*pos)), diff --git a/src/syntax.rs b/src/syntax.rs index e3e6f1ca..695822e9 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -137,11 +137,14 @@ impl Engine { .map(|v| v.is_keyword() || v.is_reserved()) .unwrap_or(false) => { - return Err(LexError::ImproperSymbol(format!( - "Improper symbol for custom syntax at position #{}: '{}'", - segments.len() + 1, - s - )) + return Err(LexError::ImproperSymbol( + s.to_string(), + format!( + "Improper symbol for custom syntax at position #{}: '{}'", + segments.len() + 1, + s + ), + ) .into_err(Position::NONE) .into()); } @@ -154,11 +157,14 @@ impl Engine { } // Anything else is an error _ => { - return Err(LexError::ImproperSymbol(format!( - "Improper symbol for custom syntax at position #{}: '{}'", - segments.len() + 1, - s - )) + return Err(LexError::ImproperSymbol( + s.to_string(), + format!( + "Improper symbol for custom syntax at position #{}: '{}'", + segments.len() + 1, + s + ), + ) .into_err(Position::NONE) .into()); } diff --git a/src/token.rs b/src/token.rs index c36a5651..cd54059d 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1658,39 +1658,41 @@ impl<'a> Iterator for TokenIterator<'a, '_> { Some((Token::Reserved(s), pos)) => Some((match (s.as_str(), self.engine.custom_keywords.contains_key(&s)) { - ("===", false) => Token::LexError(LERR::ImproperSymbol( + ("===", false) => Token::LexError(LERR::ImproperSymbol(s, "'===' is not a valid operator. This is not JavaScript! Should it be '=='?".to_string(), )), - ("!==", false) => Token::LexError(LERR::ImproperSymbol( + ("!==", false) => Token::LexError(LERR::ImproperSymbol(s, "'!==' is not a valid operator. This is not JavaScript! Should it be '!='?".to_string(), )), - ("->", false) => Token::LexError(LERR::ImproperSymbol( + ("->", false) => Token::LexError(LERR::ImproperSymbol(s, "'->' is not a valid symbol. This is not C or C++!".to_string())), - ("<-", false) => Token::LexError(LERR::ImproperSymbol( + ("<-", false) => Token::LexError(LERR::ImproperSymbol(s, "'<-' is not a valid symbol. This is not Go! Should it be '<='?".to_string(), )), - (":=", false) => Token::LexError(LERR::ImproperSymbol( + (":=", false) => Token::LexError(LERR::ImproperSymbol(s, "':=' is not a valid assignment operator. This is not Go! Should it be simply '='?".to_string(), )), - ("::<", false) => Token::LexError(LERR::ImproperSymbol( + ("::<", false) => Token::LexError(LERR::ImproperSymbol(s, "'::<>' is not a valid symbol. This is not Rust! Should it be '::'?".to_string(), )), - ("(*", false) | ("*)", false) => Token::LexError(LERR::ImproperSymbol( + ("(*", false) | ("*)", false) => Token::LexError(LERR::ImproperSymbol(s, "'(* .. *)' is not a valid comment format. This is not Pascal! Should it be '/* .. */'?".to_string(), )), - ("#", false) => Token::LexError(LERR::ImproperSymbol( + ("#", false) => Token::LexError(LERR::ImproperSymbol(s, "'#' is not a valid symbol. Should it be '#{'?".to_string(), )), // Reserved keyword/operator that is custom. (_, true) => Token::Custom(s), // Reserved operator that is not custom. - (token, false) if !is_valid_identifier(token.chars()) => Token::LexError(LERR::ImproperSymbol( - format!("'{}' is a reserved symbol", token) - )), + (token, false) if !is_valid_identifier(token.chars()) => { + let msg = format!("'{}' is a reserved symbol", token); + Token::LexError(LERR::ImproperSymbol(s, msg)) + }, // Reserved keyword that is not custom and disabled. - (token, false) if self.engine.disabled_symbols.contains(token) => Token::LexError(LERR::ImproperSymbol( - format!("reserved symbol '{}' is disabled", token) - )), + (token, false) if self.engine.disabled_symbols.contains(token) => { + let msg = format!("reserved symbol '{}' is disabled", token); + Token::LexError(LERR::ImproperSymbol(s, msg)) + }, // Reserved keyword/operator that is not custom. (_, false) => Token::Reserved(s), }, pos)), diff --git a/tests/eval.rs b/tests/eval.rs index 7732bd25..568a5e5b 100644 --- a/tests/eval.rs +++ b/tests/eval.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, Scope, INT}; +use rhai::{Engine, EvalAltResult, LexError, ParseErrorType, RegisterFn, Scope, INT}; #[test] fn test_eval() -> Result<(), Box> { @@ -98,10 +98,34 @@ fn test_eval_override() -> Result<(), Box> { fn eval(x) { x } // reflect the script back eval("40 + 2") - "# + "# )?, "40 + 2" ); + let mut engine = Engine::new(); + + // Reflect the script back + engine.register_fn("eval", |script: &str| script.to_string()); + + assert_eq!(engine.eval::(r#"eval("40 + 2")"#)?, "40 + 2"); + + Ok(()) +} + +#[test] +fn test_eval_disabled() -> Result<(), Box> { + let mut engine = Engine::new(); + + engine.disable_symbol("eval"); + + assert!(matches!( + *engine + .compile(r#"eval("40 + 2")"#) + .expect_err("should error") + .0, + ParseErrorType::BadInput(LexError::ImproperSymbol(err, _)) if err == "eval" + )); + Ok(()) }