diff options
author | David Sherret <dsherret@users.noreply.github.com> | 2023-03-15 21:41:13 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-15 21:41:13 -0400 |
commit | c1eba16b84c5bba4f7fdf05beb6ccf5e0fd1da16 (patch) | |
tree | 8ce89ad74652f0f47daead27adc5b61ff4fa455a | |
parent | 48a0b7f98f568bb5c3a15b487459569e38e4c671 (diff) |
fix(repl): do not panic deleting `Deno` or deleting all its properties (#18211)
Closes #18194
Closes #12092
-rw-r--r-- | cli/tests/integration/repl_tests.rs | 30 | ||||
-rw-r--r-- | cli/tools/repl/editor.rs | 11 | ||||
-rw-r--r-- | cli/tools/repl/session.rs | 155 |
3 files changed, 135 insertions, 61 deletions
diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index efbde55c3..aafb3bb48 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -223,6 +223,36 @@ fn pty_assign_global_this() { } #[test] +fn pty_assign_deno_keys_and_deno() { + util::with_pty(&["repl"], |mut console| { + console.write_line( + "Object.keys(Deno).forEach((key)=>{try{Deno[key] = undefined} catch {}})", + ); + console.write_line("delete globalThis.Deno"); + console.write_line("console.log('testing ' + 'this out')"); + console.write_line("close();"); + + let output = console.read_all_output(); + assert_not_contains!(output, "panicked"); + assert_contains!(output, "testing this out"); + }); +} + +#[test] +fn pty_internal_repl() { + util::with_pty(&["repl"], |mut console| { + console.write_line("globalThis"); + console.write_line("__\t\t"); + console.write_line("close();"); + let output = console.read_all_output(); + assert_contains!(output, "__defineGetter__"); + // should not contain the internal repl variable + // in the `globalThis` or completions output + assert_not_contains!(output, "__DENO_"); + }); +} + +#[test] fn pty_emoji() { // windows was having issues displaying this util::with_pty(&["repl"], |mut console| { diff --git a/cli/tools/repl/editor.rs b/cli/tools/repl/editor.rs index bf38573f4..98e528614 100644 --- a/cli/tools/repl/editor.rs +++ b/cli/tools/repl/editor.rs @@ -39,6 +39,7 @@ use std::sync::Arc; use super::cdp; use super::channel::RustylineSyncMessageSender; +use super::session::REPL_INTERNALS_NAME; // Provides helpers to the editor like validation for multi-line edits, completion candidates for // tab completion. @@ -159,7 +160,7 @@ impl EditorHelper { } fn is_word_boundary(c: char) -> bool { - if c == '.' { + if matches!(c, '.' | '_' | '$') { false } else { char::is_ascii_whitespace(&c) || char::is_ascii_punctuation(&c) @@ -207,7 +208,11 @@ impl Completer for EditorHelper { let candidates = self .get_expression_property_names(sub_expr) .into_iter() - .filter(|n| !n.starts_with("Symbol(") && n.starts_with(prop_name)) + .filter(|n| { + !n.starts_with("Symbol(") + && n.starts_with(prop_name) + && n != &*REPL_INTERNALS_NAME + }) .collect(); Ok((pos - prop_name.len(), candidates)) @@ -217,7 +222,7 @@ impl Completer for EditorHelper { .get_expression_property_names("globalThis") .into_iter() .chain(self.get_global_lexical_scope_names()) - .filter(|n| n.starts_with(expr)) + .filter(|n| n.starts_with(expr) && n != &*REPL_INTERNALS_NAME) .collect::<Vec<_>>(); // sort and remove duplicates diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 3cd9730a7..4438f6ddc 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -22,41 +22,69 @@ use deno_graph::npm::NpmPackageReqReference; use deno_graph::source::Resolver; use deno_runtime::deno_node; use deno_runtime::worker::MainWorker; +use once_cell::sync::Lazy; use super::cdp; -static PRELUDE: &str = r#" -Object.defineProperty(globalThis, "_", { +/// We store functions used in the repl on this object because +/// the user might modify the `Deno` global or delete it outright. +pub static REPL_INTERNALS_NAME: Lazy<String> = Lazy::new(|| { + let now = std::time::SystemTime::now(); + let seconds = now + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(); + // use a changing variable name to make it hard to depend on this + format!("__DENO_REPL_INTERNALS_{seconds}__") +}); + +fn get_prelude() -> String { + format!( + r#" +Object.defineProperty(globalThis, "{0}", {{ + enumerable: false, + writable: false, + value: {{ + lastEvalResult: undefined, + lastThrownError: undefined, + inspectArgs: Deno[Deno.internal].inspectArgs, + noColor: Deno.noColor, + }}, +}}); +Object.defineProperty(globalThis, "_", {{ configurable: true, - get: () => Deno[Deno.internal].lastEvalResult, - set: (value) => { - Object.defineProperty(globalThis, "_", { + get: () => {0}.lastEvalResult, + set: (value) => {{ + Object.defineProperty(globalThis, "_", {{ value: value, writable: true, enumerable: true, configurable: true, - }); + }}); console.log("Last evaluation result is no longer saved to _."); - }, -}); + }}, +}}); -Object.defineProperty(globalThis, "_error", { +Object.defineProperty(globalThis, "_error", {{ configurable: true, - get: () => Deno[Deno.internal].lastThrownError, - set: (value) => { - Object.defineProperty(globalThis, "_error", { + get: () => {0}.lastThrownError, + set: (value) => {{ + Object.defineProperty(globalThis, "_error", {{ value: value, writable: true, enumerable: true, configurable: true, - }); + }}); console.log("Last thrown error is no longer saved to _error."); - }, -}); + }}, +}}); globalThis.clear = console.clear.bind(console); -"#; +"#, + *REPL_INTERNALS_NAME + ) +} pub enum EvaluationOutput { Value(String), @@ -161,7 +189,7 @@ impl ReplSession { }; // inject prelude - repl_session.evaluate_expression(PRELUDE).await?; + repl_session.evaluate_expression(&get_prelude()).await?; Ok(repl_session) } @@ -307,22 +335,27 @@ impl ReplSession { &mut self, error: &cdp::RemoteObject, ) -> Result<(), AnyError> { - self.post_message_with_event_loop( - "Runtime.callFunctionOn", - Some(cdp::CallFunctionOnArgs { - function_declaration: "function (object) { Deno[Deno.internal].lastThrownError = object; }".to_string(), - object_id: None, - arguments: Some(vec![error.into()]), - silent: None, - return_by_value: None, - generate_preview: None, - user_gesture: None, - await_promise: None, - execution_context_id: Some(self.context_id), - object_group: None, - throw_on_side_effect: None - }), - ).await?; + self + .post_message_with_event_loop( + "Runtime.callFunctionOn", + Some(cdp::CallFunctionOnArgs { + function_declaration: format!( + r#"function (object) {{ {}.lastThrownError = object; }}"#, + *REPL_INTERNALS_NAME + ), + object_id: None, + arguments: Some(vec![error.into()]), + silent: None, + return_by_value: None, + generate_preview: None, + user_gesture: None, + await_promise: None, + execution_context_id: Some(self.context_id), + object_group: None, + throw_on_side_effect: None, + }), + ) + .await?; Ok(()) } @@ -334,9 +367,10 @@ impl ReplSession { .post_message_with_event_loop( "Runtime.callFunctionOn", Some(cdp::CallFunctionOnArgs { - function_declaration: - "function (object) { Deno[Deno.internal].lastEvalResult = object; }" - .to_string(), + function_declaration: format!( + r#"function (object) {{ {}.lastEvalResult = object; }}"#, + *REPL_INTERNALS_NAME + ), object_id: None, arguments: Some(vec![evaluate_result.into()]), silent: None, @@ -360,28 +394,33 @@ impl ReplSession { // TODO(caspervonb) we should investigate using previews here but to keep things // consistent with the previous implementation we just get the preview result from // Deno.inspectArgs. - let inspect_response = self.post_message_with_event_loop( - "Runtime.callFunctionOn", - Some(cdp::CallFunctionOnArgs { - function_declaration: r#"function (object) { - try { - return Deno[Deno.internal].inspectArgs(["%o", object], { colors: !Deno.noColor }); - } catch (err) { - return Deno[Deno.internal].inspectArgs(["%o", err]); - } - }"#.to_string(), - object_id: None, - arguments: Some(vec![evaluate_result.into()]), - silent: None, - return_by_value: None, - generate_preview: None, - user_gesture: None, - await_promise: None, - execution_context_id: Some(self.context_id), - object_group: None, - throw_on_side_effect: None - }), - ).await?; + let inspect_response = self + .post_message_with_event_loop( + "Runtime.callFunctionOn", + Some(cdp::CallFunctionOnArgs { + function_declaration: format!( + r#"function (object) {{ + try {{ + return {0}.inspectArgs(["%o", object], {{ colors: !{0}.noColor }}); + }} catch (err) {{ + return {0}.inspectArgs(["%o", err]); + }} + }}"#, + *REPL_INTERNALS_NAME + ), + object_id: None, + arguments: Some(vec![evaluate_result.into()]), + silent: None, + return_by_value: None, + generate_preview: None, + user_gesture: None, + await_promise: None, + execution_context_id: Some(self.context_id), + object_group: None, + throw_on_side_effect: None, + }), + ) + .await?; let response: cdp::CallFunctionOnResponse = serde_json::from_value(inspect_response)?; |