From b10563cb2083f7af9d4320662d6aa1897b6db23e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 18 Jan 2022 00:13:14 +0100 Subject: [PATCH] fix(runtime): don't crash when window is deleted (#13392) This commit fixes an error when user deletes "window" global JS variable. Instead of relying on "window" or "globalThis" to dispatch "load" and "unload" events, we are default to global scope of the worker. --- cli/tests/integration/run_tests.rs | 5 +++++ cli/tests/testdata/delete_window.js | 1 + ext/web/02_event.js | 6 +++++- runtime/worker.rs | 17 ++++++++++++++--- 4 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 cli/tests/testdata/delete_window.js diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 2f32fd67f6..74a03e0555 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2480,6 +2480,11 @@ itest!(import_assertions_type_check { exit_code: 1, }); +itest!(delete_window { + args: "run delete_window.js", + output_str: Some("true\n"), +}); + itest!(colors_without_global_this { args: "run colors_without_globalThis.js", output_str: Some("true\n"), diff --git a/cli/tests/testdata/delete_window.js b/cli/tests/testdata/delete_window.js new file mode 100644 index 0000000000..f2f16820d8 --- /dev/null +++ b/cli/tests/testdata/delete_window.js @@ -0,0 +1 @@ +console.log(delete globalThis.window); diff --git a/ext/web/02_event.js b/ext/web/02_event.js index f9a69c625e..7ba5afa276 100644 --- a/ext/web/02_event.js +++ b/ext/web/02_event.js @@ -970,7 +970,11 @@ webidl.requiredArguments(arguments.length, 1, { prefix: "Failed to execute 'dispatchEvent' on 'EventTarget'", }); - const self = this ?? globalThis; + // If `this` is not present, then fallback to global scope. We don't use + // `globalThis` directly here, because it could be deleted by user. + // Instead use saved reference to global scope when the script was + // executed. + const self = this ?? window; const { listeners } = self[eventTargetData]; if (!(event.type in listeners)) { diff --git a/runtime/worker.rs b/runtime/worker.rs index ad31cacc66..7a3a6c1c34 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -313,7 +313,13 @@ impl MainWorker { &mut self, script_name: &str, ) -> Result<(), AnyError> { - self.execute_script(script_name, "window.dispatchEvent(new Event('load'))") + self.execute_script( + script_name, + // NOTE(@bartlomieju): not using `globalThis` here, because user might delete + // it. Instead we're using global `dispatchEvent` function which will + // used a saved reference to global scope. + "dispatchEvent(new Event('load'))", + ) } /// Dispatches "unload" event to the JavaScript runtime. @@ -323,8 +329,13 @@ impl MainWorker { &mut self, script_name: &str, ) -> Result<(), AnyError> { - self - .execute_script(script_name, "window.dispatchEvent(new Event('unload'))") + self.execute_script( + script_name, + // NOTE(@bartlomieju): not using `globalThis` here, because user might delete + // it. Instead we're using global `dispatchEvent` function which will + // used a saved reference to global scope. + "dispatchEvent(new Event('unload'))", + ) } }