- Revision
- 279170
- Author
- [email protected]
- Date
- 2021-06-23 09:25:52 -0700 (Wed, 23 Jun 2021)
Log Message
Add missing exception checks in ScriptModuleLoader and JSDOMPromiseDeferred.
https://bugs.webkit.org/show_bug.cgi?id=221374
rdar://problem/68911404
Reviewed by Yusuke Suzuki.
Based on patch by Frédéric Wang <[email protected]>.
Source/WebCore:
When an import call fails, ScriptModuleLoader::notifyFinished() rejects the
deferred promise with either a call to
DeferredPromise::reject(ExceptionCode, const String&, RejectAsHandled) or a call to
rejectToPropagateNetworkError() / DeferredPromise::rejectWithCallback(). When it
succeeds, it calls DeferredPromise::resolveWithPromise(). These correspond to three
places where we enter the VM. Catch scopes are thus added to handle uncaught
exceptions for DeferredPromise.
Currently, this logic for uncaught exceptions is already duplicated at several
places in the code, and is likely needed for other places too in follow-up work.
This patch however covers all the rejection and resolution calls from
ScriptModuleLoader. Additionally, it handles missing exception checks in
ScriptModuleLoader::evaluate.
Test: js/dom/modules/missing-exception-check-for-import.html
* bindings/js/JSDOMPromiseDeferred.cpp:
(WebCore::DeferredPromise::reject):
(WebCore::DeferredPromise::handleTerminationExceptionIfNeeded):
(WebCore::DeferredPromise::handleUncaughtException):
* bindings/js/JSDOMPromiseDeferred.h:
(WebCore::DeferredPromise::resolveWithCallback):
(WebCore::DeferredPromise::rejectWithCallback):
* bindings/js/ScriptModuleLoader.cpp:
(WebCore::ScriptModuleLoader::evaluate):
LayoutTests:
Add a regression test for import('./'). Depending on the system configuration this can
execute two different rejection methods from ScriptModuleLoader::notifyFinished().
In other words, we may get 2 different error messages. Hence, the test needs to
allow for this variance.
Additional test cases are added for rejectToPropagateNetworkError /
DeferredPromise::rejectWithCallback (e.g. module file not found), for
DeferredPromise::reject(ExceptionCode, const String&, RejectAsHandled) (e.g. module file does
not have _javascript_ MIME type) and DeferredPromise::resolveWithCallback (e.g. module import
is successful).
Also update promise-rejection-might-stack-overflow.html now that the stack overflow error is
reported.
* js/dom/modules/missing-exception-check-for-import-expected.txt: Added.
* js/dom/modules/missing-exception-check-for-import.html: Added.
* js/dom/promise-rejection-might-stack-overflow-expected.txt:
* js/dom/promise-rejection-might-stack-overflow.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (279169 => 279170)
--- trunk/LayoutTests/ChangeLog 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/LayoutTests/ChangeLog 2021-06-23 16:25:52 UTC (rev 279170)
@@ -1,3 +1,32 @@
+2021-06-18 Mark Lam <[email protected]>
+
+ Add missing exception checks in ScriptModuleLoader and JSDOMPromiseDeferred.
+ https://bugs.webkit.org/show_bug.cgi?id=221374
+ rdar://problem/68911404
+
+ Reviewed by Yusuke Suzuki.
+
+ Based on patch by Frédéric Wang <[email protected]>.
+
+ Add a regression test for import('./'). Depending on the system configuration this can
+ execute two different rejection methods from ScriptModuleLoader::notifyFinished().
+ In other words, we may get 2 different error messages. Hence, the test needs to
+ allow for this variance.
+
+ Additional test cases are added for rejectToPropagateNetworkError /
+ DeferredPromise::rejectWithCallback (e.g. module file not found), for
+ DeferredPromise::reject(ExceptionCode, const String&, RejectAsHandled) (e.g. module file does
+ not have _javascript_ MIME type) and DeferredPromise::resolveWithCallback (e.g. module import
+ is successful).
+
+ Also update promise-rejection-might-stack-overflow.html now that the stack overflow error is
+ reported.
+
+ * js/dom/modules/missing-exception-check-for-import-expected.txt: Added.
+ * js/dom/modules/missing-exception-check-for-import.html: Added.
+ * js/dom/promise-rejection-might-stack-overflow-expected.txt:
+ * js/dom/promise-rejection-might-stack-overflow.html:
+
2021-06-21 Darin Adler <[email protected]>
Improve more of the CSS list style implementations
Added: trunk/LayoutTests/js/dom/modules/missing-exception-check-for-import-expected.txt (0 => 279170)
--- trunk/LayoutTests/js/dom/modules/missing-exception-check-for-import-expected.txt (rev 0)
+++ trunk/LayoutTests/js/dom/modules/missing-exception-check-for-import-expected.txt 2021-06-23 16:25:52 UTC (rev 279170)
@@ -0,0 +1,20 @@
+Test module import with validateExceptionChecks.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Import should fail because path is not a file.
+PASS String(error).substr(0, 11) is "TypeError: "
+
+Importing a module script should fail if the file is not found.
+PASS String(error) is "TypeError: Importing a module script failed."
+
+Importing a module script should fail if it does not not a valid _javascript_ MIME type.
+PASS String(error) is "TypeError: 'text/html' is not a valid _javascript_ MIME type."
+
+Importing a simple module script should succeed.
+PASS error is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/dom/modules/missing-exception-check-for-import.html (0 => 279170)
--- trunk/LayoutTests/js/dom/modules/missing-exception-check-for-import.html (rev 0)
+++ trunk/LayoutTests/js/dom/modules/missing-exception-check-for-import.html 2021-06-23 16:25:52 UTC (rev 279170)
@@ -0,0 +1,59 @@
+<!DOCTYPE HTML><!-- webkit-test-runner [ jscOptions=--validateExceptionChecks=true ] -->
+<html>
+ <head>
+ <script src=""
+ </head>
+ <body>
+ <script>
+ description('Test module import with validateExceptionChecks.');
+ window.jsTestIsAsync = true;
+ var error;
+ </script>
+ <script src=""
+ <script>
+ (async function () {
+
+ debug(`Import should fail because path is not a file.`)
+ error = null;
+ try {
+ await import(`./`);
+ } catch (e) {
+ error = e;
+ }
+
+ // Depending on the port, the import may fail due to different rejections.
+ // Hence, we'll discount the rest of the error message in this comparison.
+ shouldBeEqualToString(`String(error).substr(0, 11)`, `TypeError: `);
+
+ debug(`\nImporting a module script should fail if the file is not found.`)
+ error = null;
+ try {
+ await import(`./resources/import-not-found.js`);
+ } catch (e) {
+ error = e;
+ }
+ shouldBeEqualToString(`String(error)`, `TypeError: Importing a module script failed.`);
+
+ debug(`\nImporting a module script should fail if it does not not a valid _javascript_ MIME type.`);
+ error = null;
+ try {
+ await import(`data:text/html,Hello World`);
+ } catch (e) {
+ error = e;
+ }
+ shouldBeEqualToString(`String(error)`, `TypeError: 'text/html' is not a valid _javascript_ MIME type.`);
+
+ debug(`\nImporting a simple module script should succeed.`);
+ error = null;
+ try {
+ await import(`./resources/module-inline-simple.js`);
+ } catch (e) {
+ error = e;
+ }
+ shouldBeNull(`error`);
+
+ finishJSTest();
+ }());
+ </script>
+ </body>
+</html>
Modified: trunk/LayoutTests/js/dom/promise-rejection-might-stack-overflow-expected.txt (279169 => 279170)
--- trunk/LayoutTests/js/dom/promise-rejection-might-stack-overflow-expected.txt 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/LayoutTests/js/dom/promise-rejection-might-stack-overflow-expected.txt 2021-06-23 16:25:52 UTC (rev 279170)
@@ -1,3 +1,5 @@
+CONSOLE MESSAGE: RangeError: Maximum call stack size exceeded.
+PASS Got expected error: 'RangeError: Maximum call stack size exceeded.'
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/js/dom/promise-rejection-might-stack-overflow.html (279169 => 279170)
--- trunk/LayoutTests/js/dom/promise-rejection-might-stack-overflow.html 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/LayoutTests/js/dom/promise-rejection-might-stack-overflow.html 2021-06-23 16:25:52 UTC (rev 279170)
@@ -5,6 +5,9 @@
</head>
<body>
<script src=""
+<script>
+ shouldHaveHadError("RangeError: Maximum call stack size exceeded.")
+</script>
<script src=""
</body>
</html>
Modified: trunk/Source/WebCore/ChangeLog (279169 => 279170)
--- trunk/Source/WebCore/ChangeLog 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/Source/WebCore/ChangeLog 2021-06-23 16:25:52 UTC (rev 279170)
@@ -1,3 +1,39 @@
+2021-06-18 Mark Lam <[email protected]>
+
+ Add missing exception checks in ScriptModuleLoader and JSDOMPromiseDeferred.
+ https://bugs.webkit.org/show_bug.cgi?id=221374
+ rdar://problem/68911404
+
+ Reviewed by Yusuke Suzuki.
+
+ Based on patch by Frédéric Wang <[email protected]>.
+
+ When an import call fails, ScriptModuleLoader::notifyFinished() rejects the
+ deferred promise with either a call to
+ DeferredPromise::reject(ExceptionCode, const String&, RejectAsHandled) or a call to
+ rejectToPropagateNetworkError() / DeferredPromise::rejectWithCallback(). When it
+ succeeds, it calls DeferredPromise::resolveWithPromise(). These correspond to three
+ places where we enter the VM. Catch scopes are thus added to handle uncaught
+ exceptions for DeferredPromise.
+
+ Currently, this logic for uncaught exceptions is already duplicated at several
+ places in the code, and is likely needed for other places too in follow-up work.
+ This patch however covers all the rejection and resolution calls from
+ ScriptModuleLoader. Additionally, it handles missing exception checks in
+ ScriptModuleLoader::evaluate.
+
+ Test: js/dom/modules/missing-exception-check-for-import.html
+
+ * bindings/js/JSDOMPromiseDeferred.cpp:
+ (WebCore::DeferredPromise::reject):
+ (WebCore::DeferredPromise::handleTerminationExceptionIfNeeded):
+ (WebCore::DeferredPromise::handleUncaughtException):
+ * bindings/js/JSDOMPromiseDeferred.h:
+ (WebCore::DeferredPromise::resolveWithCallback):
+ (WebCore::DeferredPromise::rejectWithCallback):
+ * bindings/js/ScriptModuleLoader.cpp:
+ (WebCore::ScriptModuleLoader::evaluate):
+
2021-06-21 Darin Adler <[email protected]>
Improve more of the CSS list style implementations
Modified: trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp (279169 => 279170)
--- trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp 2021-06-23 16:25:52 UTC (rev 279170)
@@ -31,6 +31,8 @@
#include "JSDOMExceptionHandling.h"
#include "JSDOMPromise.h"
#include "JSDOMWindow.h"
+#include "ScriptController.h"
+#include "WorkerGlobalScope.h"
#include <_javascript_Core/BuiltinNames.h>
#include <_javascript_Core/Exception.h>
#include <_javascript_Core/JSONObject.h>
@@ -131,28 +133,28 @@
auto& lexicalGlobalObject = *m_globalObject;
JSC::VM& vm = lexicalGlobalObject.vm();
JSC::JSLockHolder locker(vm);
+ auto scope = DECLARE_CATCH_SCOPE(vm);
if (exception.code() == ExistingExceptionError) {
- auto scope = DECLARE_CATCH_SCOPE(lexicalGlobalObject.vm());
-
EXCEPTION_ASSERT(scope.exception());
-
auto error = scope.exception()->value();
+ bool isTerminating = handleTerminationExceptionIfNeeded(scope, lexicalGlobalObject);
scope.clearException();
- reject<IDLAny>(error, rejectAsHandled);
+ if (!isTerminating)
+ reject<IDLAny>(error, rejectAsHandled);
return;
}
- auto scope = DECLARE_THROW_SCOPE(vm);
auto error = createDOMException(lexicalGlobalObject, WTFMove(exception));
if (UNLIKELY(scope.exception())) {
- ASSERT(vm.hasPendingTerminationException());
+ handleUncaughtException(scope, lexicalGlobalObject);
return;
}
- scope.release();
reject(lexicalGlobalObject, error, rejectAsHandled);
+ if (UNLIKELY(scope.exception()))
+ handleUncaughtException(scope, lexicalGlobalObject);
}
void DeferredPromise::reject(ExceptionCode ec, const String& message, RejectAsHandled rejectAsHandled)
@@ -165,28 +167,28 @@
auto& lexicalGlobalObject = *m_globalObject;
JSC::VM& vm = lexicalGlobalObject.vm();
JSC::JSLockHolder locker(vm);
+ auto scope = DECLARE_CATCH_SCOPE(vm);
if (ec == ExistingExceptionError) {
- auto scope = DECLARE_CATCH_SCOPE(vm);
-
EXCEPTION_ASSERT(scope.exception());
-
auto error = scope.exception()->value();
+ bool isTerminating = handleTerminationExceptionIfNeeded(scope, lexicalGlobalObject);
scope.clearException();
- reject<IDLAny>(error, rejectAsHandled);
+ if (!isTerminating)
+ reject<IDLAny>(error, rejectAsHandled);
return;
}
- auto scope = DECLARE_THROW_SCOPE(vm);
auto error = createDOMException(&lexicalGlobalObject, ec, message);
if (UNLIKELY(scope.exception())) {
- ASSERT(vm.hasPendingTerminationException());
+ handleUncaughtException(scope, lexicalGlobalObject);
return;
}
- scope.release();
reject(lexicalGlobalObject, error, rejectAsHandled);
+ if (UNLIKELY(scope.exception()))
+ handleUncaughtException(scope, lexicalGlobalObject);
}
void DeferredPromise::reject(const JSC::PrivateName& privateName, RejectAsHandled rejectAsHandled)
@@ -265,4 +267,28 @@
fulfillPromiseWithArrayBuffer(WTFMove(promise), ArrayBuffer::tryCreate(data, length).get());
}
+bool DeferredPromise::handleTerminationExceptionIfNeeded(CatchScope& scope, JSDOMGlobalObject& lexicalGlobalObject)
+{
+ auto* exception = scope.exception();
+ VM& vm = scope.vm();
+
+ auto& scriptExecutionContext = *lexicalGlobalObject.scriptExecutionContext();
+ if (is<WorkerGlobalScope>(scriptExecutionContext)) {
+ auto& scriptController = *downcast<WorkerGlobalScope>(scriptExecutionContext).script();
+ bool terminatorCausedException = vm.isTerminationException(exception);
+ if (terminatorCausedException || scriptController.isTerminatingExecution()) {
+ scriptController.forbidExecution();
+ return true;
+ }
+ }
+ return false;
}
+
+void DeferredPromise::handleUncaughtException(CatchScope& scope, JSDOMGlobalObject& lexicalGlobalObject)
+{
+ auto* exception = scope.exception();
+ handleTerminationExceptionIfNeeded(scope, lexicalGlobalObject);
+ reportException(&lexicalGlobalObject, exception);
+};
+
+} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h (279169 => 279170)
--- trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h 2021-06-23 16:25:52 UTC (rev 279170)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013-2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2021 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -147,9 +147,13 @@
ASSERT(deferred());
ASSERT(globalObject());
- JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
- JSC::JSLockHolder locker(lexicalGlobalObject);
+ auto* lexicalGlobalObject = globalObject();
+ JSC::VM& vm = lexicalGlobalObject->vm();
+ JSC::JSLockHolder locker(vm);
+ auto scope = DECLARE_CATCH_SCOPE(vm);
resolve(*lexicalGlobalObject, callback(*globalObject()));
+ if (UNLIKELY(scope.exception()))
+ handleUncaughtException(scope, *lexicalGlobalObject);
}
template<typename Callback>
@@ -160,9 +164,13 @@
ASSERT(deferred());
ASSERT(globalObject());
- JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
- JSC::JSLockHolder locker(lexicalGlobalObject);
+ auto* lexicalGlobalObject = globalObject();
+ JSC::VM& vm = lexicalGlobalObject->vm();
+ JSC::JSLockHolder locker(vm);
+ auto scope = DECLARE_CATCH_SCOPE(vm);
reject(*lexicalGlobalObject, callback(*globalObject()), rejectAsHandled);
+ if (UNLIKELY(scope.exception()))
+ handleUncaughtException(scope, *lexicalGlobalObject);
}
JSC::JSValue promise() const;
@@ -189,6 +197,9 @@
callFunction(lexicalGlobalObject, rejectAsHandled == RejectAsHandled::Yes ? ResolveMode::RejectAsHandled : ResolveMode::Reject, resolution);
}
+ bool handleTerminationExceptionIfNeeded(JSC::CatchScope&, JSDOMGlobalObject& lexicalGlobalObject);
+ void handleUncaughtException(JSC::CatchScope&, JSDOMGlobalObject& lexicalGlobalObject);
+
Mode m_mode;
};
Modified: trunk/Source/WebCore/bindings/js/ScriptModuleLoader.cpp (279169 => 279170)
--- trunk/Source/WebCore/bindings/js/ScriptModuleLoader.cpp 2021-06-23 15:37:56 UTC (rev 279169)
+++ trunk/Source/WebCore/bindings/js/ScriptModuleLoader.cpp 2021-06-23 16:25:52 UTC (rev 279170)
@@ -244,12 +244,18 @@
return JSC::throwTypeError(jsGlobalObject, scope, "Module key is an invalid URL."_s);
if (m_ownerType == OwnerType::Document) {
- if (auto* frame = downcast<Document>(m_context).frame())
- return frame->script().evaluateModule(sourceURL, *moduleRecord, awaitedValue, resumeMode);
+ if (auto* frame = downcast<Document>(m_context).frame()) {
+ auto jsValue = frame->script().evaluateModule(sourceURL, *moduleRecord, awaitedValue, resumeMode);
+ RETURN_IF_EXCEPTION(scope, JSC::jsUndefined());
+ return jsValue;
+ }
} else {
ASSERT(is<WorkerOrWorkletGlobalScope>(m_context));
- if (auto* script = downcast<WorkerOrWorkletGlobalScope>(m_context).script())
- return script->evaluateModule(*moduleRecord, awaitedValue, resumeMode);
+ if (auto* script = downcast<WorkerOrWorkletGlobalScope>(m_context).script()) {
+ auto jsValue = script->evaluateModule(*moduleRecord, awaitedValue, resumeMode);
+ RETURN_IF_EXCEPTION(scope, JSC::jsUndefined());
+ return jsValue;
+ }
}
return JSC::jsUndefined();
}
