From 30c85107256898b009e4da8374a4356fad490237 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Mon, 2 Dec 2024 00:24:48 +1300 Subject: [PATCH] LibWeb: Ensure requests modules is not empty before indexing into it Regression from 5af613aa657ebc03b93891c955f4ee62eaef7e53 Fixes: #2676 --- Libraries/LibWeb/Bindings/MainThreadVM.cpp | 97 ++++++++++--------- .../import-inside-of-a-module.txt | 1 + .../HTML/ModuleLoading/import-in-a-module.js | 5 + .../import-inside-of-a-module.html | 8 ++ 4 files changed, 64 insertions(+), 47 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/HTML/ModuleLoading/import-inside-of-a-module.txt create mode 100644 Tests/LibWeb/Text/input/HTML/ModuleLoading/import-in-a-module.js create mode 100644 Tests/LibWeb/Text/input/HTML/ModuleLoading/import-inside-of-a-module.html diff --git a/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Libraries/LibWeb/Bindings/MainThreadVM.cpp index b6edcb714d5..c98b4860660 100644 --- a/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -471,16 +471,35 @@ ErrorOr initialize_main_thread_vm(HTML::EventLoop::Type type) } // 7. If referrer is a Cyclic Module Record and moduleRequest is equal to the first element of referrer.[[RequestedModules]], then: - if (referrer.has>() && module_request == referrer.get>()->requested_modules().first()) { - // 1. For each ModuleRequest record requested of referrer.[[RequestedModules]]: - for (auto const& module_request : referrer.get>()->requested_modules()) { - // 1. If moduleRequest.[[Attributes]] contains a Record entry such that entry.[[Key]] is not "type", then: - for (auto const& attribute : module_request.attributes) { - if (attribute.key == "type"sv) - continue; + if (referrer.has>()) { + // FIXME: Why do we need to check requested modules is empty here? + if (auto const& requested_modules = referrer.get>()->requested_modules(); !requested_modules.is_empty() && module_request == requested_modules.first()) { + // 1. For each ModuleRequest record requested of referrer.[[RequestedModules]]: + for (auto const& module_request : referrer.get>()->requested_modules()) { + // 1. If moduleRequest.[[Attributes]] contains a Record entry such that entry.[[Key]] is not "type", then: + for (auto const& attribute : module_request.attributes) { + if (attribute.key == "type"sv) + continue; - // 1. Let completion be Completion Record { [[Type]]: throw, [[Value]]: a new SyntaxError exception, [[Target]]: empty }. - auto completion = JS::throw_completion(JS::SyntaxError::create(*module_map_realm, "Module request attributes must only contain a type attribute"_string)); + // 1. Let completion be Completion Record { [[Type]]: throw, [[Value]]: a new SyntaxError exception, [[Target]]: empty }. + auto completion = JS::throw_completion(JS::SyntaxError::create(*module_map_realm, "Module request attributes must only contain a type attribute"_string)); + + // 2. Perform FinishLoadingImportedModule(referrer, moduleRequest, payload, completion). + JS::finish_loading_imported_module(referrer, module_request, payload, completion); + + // 3. Return. + return; + } + } + + // 2. Resolve a module specifier given referencingScript and moduleRequest.[[Specifier]], catching any + // exceptions. If they throw an exception, let resolutionError be the thrown exception. + auto maybe_exception = HTML::resolve_module_specifier(referencing_script, module_request.module_specifier); + + // 3. If the previous step threw an exception, then: + if (maybe_exception.is_exception()) { + // 1. Let completion be Completion Record { [[Type]]: throw, [[Value]]: resolutionError, [[Target]]: empty }. + auto completion = dom_exception_to_throw_completion(main_thread_vm(), maybe_exception.exception()); // 2. Perform FinishLoadingImportedModule(referrer, moduleRequest, payload, completion). JS::finish_loading_imported_module(referrer, module_request, payload, completion); @@ -488,45 +507,29 @@ ErrorOr initialize_main_thread_vm(HTML::EventLoop::Type type) // 3. Return. return; } + + // 4. Let moduleType be the result of running the module type from module request steps given moduleRequest. + auto module_type = HTML::module_type_from_module_request(module_request); + + // 5. If the result of running the module type allowed steps given moduleType and moduleMapRealm is false, then: + if (!HTML::module_type_allowed(*module_map_realm, module_type)) { + // 1. Let completion be Completion Record { [[Type]]: throw, [[Value]]: a new TypeError exception, [[Target]]: empty }. + auto completion = JS::throw_completion(JS::SyntaxError::create(*module_map_realm, MUST(String::formatted("Module type '{}' is not supported", module_type)))); + + // 2. Perform FinishLoadingImportedModule(referrer, moduleRequest, payload, completion). + JS::finish_loading_imported_module(referrer, module_request, payload, completion); + + // 3. Return + return; + } + + // Spec-Note: This step is essentially validating all of the requested module specifiers and type attributes + // when the first call to HostLoadImportedModule for a static module dependency list is made, to + // avoid further loading operations in the case any one of the dependencies has a static error. + // We treat a module with unresolvable module specifiers or unsupported type attributes the same + // as one that cannot be parsed; in both cases, a syntactic issue makes it impossible to ever + // contemplate linking the module later. } - - // 2. Resolve a module specifier given referencingScript and moduleRequest.[[Specifier]], catching any - // exceptions. If they throw an exception, let resolutionError be the thrown exception. - auto maybe_exception = HTML::resolve_module_specifier(referencing_script, module_request.module_specifier); - - // 3. If the previous step threw an exception, then: - if (maybe_exception.is_exception()) { - // 1. Let completion be Completion Record { [[Type]]: throw, [[Value]]: resolutionError, [[Target]]: empty }. - auto completion = dom_exception_to_throw_completion(main_thread_vm(), maybe_exception.exception()); - - // 2. Perform FinishLoadingImportedModule(referrer, moduleRequest, payload, completion). - JS::finish_loading_imported_module(referrer, module_request, payload, completion); - - // 3. Return. - return; - } - - // 4. Let moduleType be the result of running the module type from module request steps given moduleRequest. - auto module_type = HTML::module_type_from_module_request(module_request); - - // 5. If the result of running the module type allowed steps given moduleType and moduleMapRealm is false, then: - if (!HTML::module_type_allowed(*module_map_realm, module_type)) { - // 1. Let completion be Completion Record { [[Type]]: throw, [[Value]]: a new TypeError exception, [[Target]]: empty }. - auto completion = JS::throw_completion(JS::SyntaxError::create(*module_map_realm, MUST(String::formatted("Module type '{}' is not supported", module_type)))); - - // 2. Perform FinishLoadingImportedModule(referrer, moduleRequest, payload, completion). - JS::finish_loading_imported_module(referrer, module_request, payload, completion); - - // 3. Return - return; - } - - // Spec-Note: This step is essentially validating all of the requested module specifiers and type attributes - // when the first call to HostLoadImportedModule for a static module dependency list is made, to - // avoid further loading operations in the case any one of the dependencies has a static error. - // We treat a module with unresolvable module specifiers or unsupported type attributes the same - // as one that cannot be parsed; in both cases, a syntactic issue makes it impossible to ever - // contemplate linking the module later. } // 8. Disallow further import maps given moduleMapRealm. diff --git a/Tests/LibWeb/Text/expected/HTML/ModuleLoading/import-inside-of-a-module.txt b/Tests/LibWeb/Text/expected/HTML/ModuleLoading/import-inside-of-a-module.txt new file mode 100644 index 00000000000..50586a4cbfb --- /dev/null +++ b/Tests/LibWeb/Text/expected/HTML/ModuleLoading/import-inside-of-a-module.txt @@ -0,0 +1 @@ +PASS! (Didn't crash) diff --git a/Tests/LibWeb/Text/input/HTML/ModuleLoading/import-in-a-module.js b/Tests/LibWeb/Text/input/HTML/ModuleLoading/import-in-a-module.js new file mode 100644 index 00000000000..845a7b6839e --- /dev/null +++ b/Tests/LibWeb/Text/input/HTML/ModuleLoading/import-in-a-module.js @@ -0,0 +1,5 @@ +// NOTE: Doesn't matter what this imports, but this imports itself so there is no import error in test logs. +import("./import-in-a-module.js"); + +const returnValue = "PASS! (Didn't crash)"; +export default returnValue; diff --git a/Tests/LibWeb/Text/input/HTML/ModuleLoading/import-inside-of-a-module.html b/Tests/LibWeb/Text/input/HTML/ModuleLoading/import-inside-of-a-module.html new file mode 100644 index 00000000000..60ff2e2e526 --- /dev/null +++ b/Tests/LibWeb/Text/input/HTML/ModuleLoading/import-inside-of-a-module.html @@ -0,0 +1,8 @@ + +