Skip to content

Commit

Permalink
JSExecState::loadModule can dereference null result
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270934
rdar://121268593

Reviewed by Yusuke Suzuki.

Fixes a bug where JSExecState::loadModule always dereferenced the
result of JSC::loadModule, even though JSC::loadModule will return
null if there is an exception. This patch changes the return type
of JSExecState::loadModule to a raw pointer, so callers of it can
detect and handle null results returned from deeper calls.

* Source/WebCore/bindings/js/JSExecState.h:
(WebCore::JSExecState::loadModule):
* Source/WebCore/bindings/js/ScriptController.cpp:
(WebCore::ScriptController::loadModuleScriptInWorld):
* Source/WebCore/workers/WorkerOrWorkletScriptController.cpp:
(WebCore::WorkerOrWorkletScriptController::loadModuleSynchronously):
(WebCore::WorkerOrWorkletScriptController::loadAndEvaluateModule):

Canonical link: https://commits.webkit.org/276190@main
  • Loading branch information
ddegazio committed Mar 15, 2024
1 parent 3ff68ea commit 20cd6e8
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 13 deletions.
8 changes: 4 additions & 4 deletions Source/WebCore/bindings/js/JSExecState.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,16 @@ class JSExecState {
task.run(lexicalGlobalObject);
}

static JSC::JSInternalPromise& loadModule(JSC::JSGlobalObject& lexicalGlobalObject, const URL& topLevelModuleURL, JSC::JSValue parameters, JSC::JSValue scriptFetcher)
static JSC::JSInternalPromise* loadModule(JSC::JSGlobalObject& lexicalGlobalObject, const URL& topLevelModuleURL, JSC::JSValue parameters, JSC::JSValue scriptFetcher)
{
JSExecState currentState(&lexicalGlobalObject);
return *JSC::loadModule(&lexicalGlobalObject, JSC::Identifier::fromString(lexicalGlobalObject.vm(), topLevelModuleURL.string()), parameters, scriptFetcher);
return JSC::loadModule(&lexicalGlobalObject, JSC::Identifier::fromString(lexicalGlobalObject.vm(), topLevelModuleURL.string()), parameters, scriptFetcher);
}

static JSC::JSInternalPromise& loadModule(JSC::JSGlobalObject& lexicalGlobalObject, const JSC::SourceCode& sourceCode, JSC::JSValue scriptFetcher)
static JSC::JSInternalPromise* loadModule(JSC::JSGlobalObject& lexicalGlobalObject, const JSC::SourceCode& sourceCode, JSC::JSValue scriptFetcher)
{
JSExecState currentState(&lexicalGlobalObject);
return *JSC::loadModule(&lexicalGlobalObject, sourceCode, scriptFetcher);
return JSC::loadModule(&lexicalGlobalObject, sourceCode, scriptFetcher);
}

static JSC::JSValue linkAndEvaluateModule(JSC::JSGlobalObject& lexicalGlobalObject, const JSC::Identifier& moduleKey, JSC::JSValue scriptFetcher, NakedPtr<JSC::Exception>& returnedException)
Expand Down
12 changes: 8 additions & 4 deletions Source/WebCore/bindings/js/ScriptController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ void ScriptController::loadModuleScriptInWorld(LoadableModuleScript& moduleScrip
auto& proxy = jsWindowProxy(world);
auto& lexicalGlobalObject = *proxy.window();

auto& promise = JSExecState::loadModule(lexicalGlobalObject, topLevelModuleURL, JSC::JSScriptFetchParameters::create(lexicalGlobalObject.vm(), WTFMove(topLevelFetchParameters)), JSC::JSScriptFetcher::create(lexicalGlobalObject.vm(), { &moduleScript }));
setupModuleScriptHandlers(moduleScript, promise, world);
auto* promise = JSExecState::loadModule(lexicalGlobalObject, topLevelModuleURL, JSC::JSScriptFetchParameters::create(lexicalGlobalObject.vm(), WTFMove(topLevelFetchParameters)), JSC::JSScriptFetcher::create(lexicalGlobalObject.vm(), { &moduleScript }));
if (UNLIKELY(!promise))
return;
setupModuleScriptHandlers(moduleScript, *promise, world);
}

void ScriptController::loadModuleScript(LoadableModuleScript& moduleScript, const URL& topLevelModuleURL, Ref<JSC::ScriptFetchParameters>&& topLevelFetchParameters)
Expand All @@ -201,8 +203,10 @@ void ScriptController::loadModuleScriptInWorld(LoadableModuleScript& moduleScrip
auto& proxy = jsWindowProxy(world);
auto& lexicalGlobalObject = *proxy.window();

auto& promise = JSExecState::loadModule(lexicalGlobalObject, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(lexicalGlobalObject.vm(), { &moduleScript }));
setupModuleScriptHandlers(moduleScript, promise, world);
auto* promise = JSExecState::loadModule(lexicalGlobalObject, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(lexicalGlobalObject.vm(), { &moduleScript }));
if (UNLIKELY(!promise))
return;
setupModuleScriptHandlers(moduleScript, *promise, world);
}

void ScriptController::loadModuleScript(LoadableModuleScript& moduleScript, const ScriptSourceCode& sourceCode)
Expand Down
10 changes: 5 additions & 5 deletions Source/WebCore/workers/WorkerOrWorkletScriptController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ bool WorkerOrWorkletScriptController::loadModuleSynchronously(WorkerScriptFetche

Ref protector { scriptFetcher };
{
auto& promise = JSExecState::loadModule(globalObject, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(vm, { &scriptFetcher }));
auto* promise = JSExecState::loadModule(globalObject, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(vm, { &scriptFetcher }));
scope.assertNoExceptionExceptTermination();
RETURN_IF_EXCEPTION(scope, false);

Expand Down Expand Up @@ -365,7 +365,7 @@ bool WorkerOrWorkletScriptController::loadModuleSynchronously(WorkerScriptFetche
return JSValue::encode(jsUndefined());
});

promise.then(&globalObject, &fulfillHandler, &rejectHandler);
promise->then(&globalObject, &fulfillHandler, &rejectHandler);
}
m_globalScope->eventLoop().performMicrotaskCheckpoint();

Expand Down Expand Up @@ -442,9 +442,9 @@ void WorkerOrWorkletScriptController::loadAndEvaluateModule(const URL& moduleURL

auto parameters = ModuleFetchParameters::create(JSC::ScriptFetchParameters::Type::JavaScript, emptyString(), /* isTopLevelModule */ true);
auto scriptFetcher = WorkerScriptFetcher::create(WTFMove(parameters), credentials, globalScope()->destination(), globalScope()->referrerPolicy());
{
auto& promise = JSExecState::loadModule(globalObject, moduleURL, JSC::JSScriptFetchParameters::create(vm, scriptFetcher->parameters()), JSC::JSScriptFetcher::create(vm, { scriptFetcher.ptr() }));

auto* promise = JSExecState::loadModule(globalObject, moduleURL, JSC::JSScriptFetchParameters::create(vm, scriptFetcher->parameters()), JSC::JSScriptFetcher::create(vm, { scriptFetcher.ptr() }));
if (LIKELY(promise)) {
auto task = createSharedTask<void(std::optional<Exception>&&)>([completionHandler = WTFMove(completionHandler)](std::optional<Exception>&& exception) mutable {
completionHandler(WTFMove(exception));
});
Expand Down Expand Up @@ -537,7 +537,7 @@ void WorkerOrWorkletScriptController::loadAndEvaluateModule(const URL& moduleURL
return JSValue::encode(jsUndefined());
});

promise.then(&globalObject, &fulfillHandler, &rejectHandler);
promise->then(&globalObject, &fulfillHandler, &rejectHandler);
}
m_globalScope->eventLoop().performMicrotaskCheckpoint();
}
Expand Down

0 comments on commit 20cd6e8

Please sign in to comment.