IPC testing API is unable to reply with nullopt optional

https://bugs.webkit.org/show_bug.cgi?id=275409 rdar://129669346 Reviewed by Cameron McCormack. Decoding std::optional would return empty JSValue (JSValue()) if the optional was std::nullopt. This would be interpreted as a decoding failure. Fix this by returning the more natural value, jsUndefined value. Fix the result array construction to not use jsUndefined to signify that the value should be a array buffer containing the data. Use empty JSValue to signify this. Use exception scope to detect when JS exception would cancel the decoding as a decoding failure. Decoding failures would use the already cleared exception as the result JS object, causing a crash. Fix by first constructing the JS value, then clearing the exception from the scope. Add decoder for WebCore::ExceptionData, to be used in testing the message this problem was noted. * LayoutTests/ipc/async-with-reply-optional.html: Added. * Source/WebKit/Platform/IPC/JSIPCBinding.cpp: (IPC::jsValueForDecodedArgumentValue): (IPC::putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined): Deleted. * Source/WebKit/Platform/IPC/JSIPCBinding.h: (IPC::jsValueForDecodedArgumentValue): (IPC::putJSValueForDecodeArgumentInArray): * Source/WebKit/Shared/IPCTester.cpp: (WebKit::IPCTester::asyncOptionalExceptionData): * Source/WebKit/Shared/IPCTester.h: * Source/WebKit/Shared/IPCTester.messages.in: * Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp: (WebKit::IPCTestingAPI::jsSendWithAsyncReply): Canonical link: https://commits.webkit.org/279999@main
WebKit · webkit-commit-queue · Jun 14, 2024 · Jun 14, 2024 · Jun 14, 2024 · 8ebae84b5572e076d84b8ead18258c48a72e9280
commit 8ebae84b5572e076d84b8ead18258c48a72e9280
diff --git a/LayoutTests/ipc/async-with-reply-optional-expected.txt b/LayoutTests/ipc/async-with-reply-optional-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Test that async messages can reply with optionals
+
diff --git a/LayoutTests/ipc/async-with-reply-optional.html b/LayoutTests/ipc/async-with-reply-optional.html
@@ -0,0 +1,43 @@
+<!doctype html><!-- webkit-test-runner [ IPCTestingAPIEnabled=true ] -->
+<title>Test that async messages can reply with optionals</title>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script src="../resources/ipc.js"></script>
+<body>
+<script>
+async function runTest() {
+    const testerID = 0;
+    const IPCTester_AsyncOptionalExceptionData = IPC.messages.IPCTester_AsyncOptionalExceptionData.name;
+    const IPCTester_AsyncOptionalExceptionDataReply = IPC.messages.IPCTester_AsyncOptionalExceptionDataReply.name;
+
+    for (const processTarget of IPC.processTargets) {
+        // Test starts here.
+        let results = { };
+
+        let connection = IPC.connectionForProcessTarget(processTarget);
+        let result1 = IPC.sendWithPromisedReply(processTarget, testerID, IPCTester_AsyncOptionalExceptionData, [{type: 'bool', value: 0}]);
+        let result2 = IPC.sendWithPromisedReply(processTarget, testerID, IPCTester_AsyncOptionalExceptionData, [{type: 'bool', value: 1}]);
+        let result = await result1;
+        assert_equals(typeof result, "object");
+        assert_equals(typeof result.arguments[0], "undefined");
+        assert_equals(result.arguments[1].type, "String");
+        assert_equals(result.arguments[1].value, "b");
+        result = await result2;
+        assert_equals(typeof result, "object");
+        assert_equals(result.arguments[0].type, "ExceptionData");
+        assert_equals(result.arguments[0].code, "WrongDocumentError");
+        assert_equals(result.arguments[0].message, "m");
+        assert_equals(result.arguments[1].type, "String");
+        assert_equals(result.arguments[1].value, "a");
+    }
+    done();
+}
+
+setup({ single_test: true });
+if (window.IPC)
+    runTest();
+else
+    done();
+
+</script>
+</body>
@@ -30,6 +30,8 @@
 
 #include <JavaScriptCore/JSCInlines.h>
 #include <JavaScriptCore/ObjectConstructor.h>
+#include <WebCore/DOMException.h>
+#include <WebCore/ExceptionData.h>
 #include <WebCore/FloatRect.h>
 #include <WebCore/IntRect.h>
 #include <WebCore/RegistrableDomain.h>
@@ -194,20 +196,21 @@ JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, W
     return jsValueForDecodedArgumentRect(globalObject, value, "FloatRect"_s);
 }
 
-bool putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined(JSC::JSGlobalObject* globalObject, JSC::JSArray* array, unsigned index, JSC::JSValue value, std::span<const uint8_t> buffer)
+template<>
+JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, WebCore::ExceptionData&& exceptionData)
 {
-    auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
-
-    if (value.isUndefined()) {
-        auto arrayBuffer = JSC::ArrayBuffer::create(buffer);
-        if (auto* structure = globalObject->arrayBufferStructure(arrayBuffer->sharingMode()))
-            value = JSC::JSArrayBuffer::create(Ref { globalObject->vm() }, structure, WTFMove(arrayBuffer));
-    }
-
-    array->putDirectIndex(globalObject, index, value);
-    RETURN_IF_EXCEPTION(scope, false);
-
-    return true;
+    auto& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+    auto* object = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype());
+    RETURN_IF_EXCEPTION(scope, JSC::JSValue());
+    object->putDirect(vm, JSC::Identifier::fromString(vm, "type"_s), JSC::jsNontrivialString(vm, "ExceptionData"_s));
+    RETURN_IF_EXCEPTION(scope, JSC::JSValue());
+    auto& message = exceptionData.message;
+    object->putDirect(vm, JSC::Identifier::fromString(vm, "message"_s), message.isNull() ? JSC::jsNull() : JSC::jsString(vm, message));
+    RETURN_IF_EXCEPTION(scope, JSC::JSValue());
+    object->putDirect(vm, JSC::Identifier::fromString(vm, "code"_s), JSC::jsNontrivialString(vm, WebCore::DOMException::description(exceptionData.code).name));
+    RETURN_IF_EXCEPTION(scope, JSC::JSValue());
+    return object;
 }
 
 }

@@ -50,6 +50,7 @@ namespace WebCore {
 class FloatRect;
 class IntRect;
 class RegistrableDomain;
+struct ExceptionData;
 
 }
 
@@ -60,7 +61,8 @@ class Semaphore;
 template<typename T, std::enable_if_t<!std::is_arithmetic<T>::value && !std::is_enum<T>::value>* = nullptr>
 JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, T&&)
 {
-    return JSC::jsUndefined();
+    // Report that we don't recognize this type.
+    return JSC::JSValue();
 }
 
 template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, String&&);
@@ -109,6 +111,7 @@ JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, A
 
 template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, WebCore::IntRect&&);
 template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, WebCore::FloatRect&&);
+template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, WebCore::ExceptionData&&);
 
 template<typename U>
 JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, OptionSet<U>&& value)
@@ -126,12 +129,10 @@ template<typename U>
 JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, std::optional<U>&& value)
 {
     if (!value)
-        return JSC::JSValue();
+        return JSC::jsUndefined();
     return jsValueForDecodedArgumentValue(globalObject, std::forward<U>(*value));
 }
 
-bool putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined(JSC::JSGlobalObject*, JSC::JSArray*, unsigned index, JSC::JSValue, std::span<const uint8_t> buffer);
-
 template<typename... Elements>
 std::optional<JSC::JSValue> putJSValueForDecodeArgumentInArray(JSC::JSGlobalObject*, IPC::Decoder&, JSC::JSArray*, size_t currentIndex, std::tuple<Elements...>*);
 
@@ -150,12 +151,19 @@ std::optional<JSC::JSValue> putJSValueForDecodeArgumentInArray(JSC::JSGlobalObje
     if (!value)
         return std::nullopt;
 
+    auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
     auto jsValue = jsValueForDecodedArgumentValue(globalObject, WTFMove(*value));
-    if (jsValue.isEmpty())
-        return jsValue;
-
-    auto span = decoder.span().subspan(startingBufferOffset, decoder.currentBufferOffset() - startingBufferOffset);
-    putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined(globalObject, array, currentIndex, jsValue, span);
+    RETURN_IF_EXCEPTION(scope, std::nullopt);
+    if (jsValue.isEmpty()) {
+        // Create array buffers out of types we don't recognize.
+        auto span = decoder.span().subspan(startingBufferOffset, decoder.currentBufferOffset() - startingBufferOffset);
+        auto arrayBuffer = JSC::ArrayBuffer::create(span);
+        if (auto* structure = globalObject->arrayBufferStructure(arrayBuffer->sharingMode()))
+            jsValue = JSC::JSArrayBuffer::create(Ref { globalObject->vm() }, structure, WTFMove(arrayBuffer));
+        RETURN_IF_EXCEPTION(scope, std::nullopt);
+    }
+    array->putDirectIndex(globalObject, currentIndex, jsValue);
+    RETURN_IF_EXCEPTION(scope, std::nullopt);
 
     std::tuple<Elements...>* dummyArguments = nullptr;
     return putJSValueForDecodeArgumentInArray<Elements...>(globalObject, decoder, array, currentIndex + 1, dummyArguments);

diff --git a/Source/WebKit/Shared/IPCTester.cpp b/Source/WebKit/Shared/IPCTester.cpp
@@ -35,6 +35,7 @@
 #include "IPCTesterReceiverMessages.h"
 #include "IPCUtilities.h"
 
+#include <WebCore/ExceptionData.h>
 #include <atomic>
 #include <dlfcn.h>
 #include <stdio.h>
@@ -218,6 +219,15 @@ void IPCTester::syncPingEmptyReply(IPC::Connection&, uint32_t value, CompletionH
     completionHandler();
 }
 
+void IPCTester::asyncOptionalExceptionData(IPC::Connection&, bool sendEngaged, CompletionHandler<void(std::optional<WebCore::ExceptionData>, String)>&& completionHandler)
+{
+    if (sendEngaged) {
+        completionHandler(WebCore::ExceptionData { WebCore::ExceptionCode::WrongDocumentError, "m"_s }, "a"_s);
+        return;
+    }
+    completionHandler(std::nullopt, "b"_s);
+}
+
 void IPCTester::stopIfNeeded()
 {
     if (m_testQueue) {

diff --git a/Source/WebKit/Shared/IPCTester.h b/Source/WebKit/Shared/IPCTester.h
@@ -74,6 +74,7 @@ class IPCTester final : public IPC::MessageReceiver {
     void asyncPing(uint32_t value, CompletionHandler<void(uint32_t)>&&);
     void syncPing(IPC::Connection&, uint32_t value, CompletionHandler<void(uint32_t)>&&);
     void syncPingEmptyReply(IPC::Connection&, uint32_t value, CompletionHandler<void()>&&);
+    void asyncOptionalExceptionData(IPC::Connection&, bool sendEngaged, CompletionHandler<void(std::optional<WebCore::ExceptionData>, String)>&&);
 
     void stopIfNeeded();
 

diff --git a/Source/WebKit/Shared/IPCTester.messages.in b/Source/WebKit/Shared/IPCTester.messages.in
@@ -40,6 +40,7 @@ messages -> IPCTester NotRefCounted {
     SyncPingEmptyReply(uint32_t value) -> () Synchronous
 
     SendAsyncMessageToReceiver(uint32_t arg0)
+    AsyncOptionalExceptionData(bool sendEngaged) -> (std::optional<WebCore::ExceptionData> exceptionData, String other)
 }
 
 #endif
@@ -527,8 +527,8 @@ static JSValueRef jsSendWithAsyncReply(IPC::Connection& connection, uint64_t des
                 jsResult = jsResultFromReplyDecoder(globalObject, messageName, *replyDecoder);
             JSValueRef arguments[1] = { nullptr };
             if (auto* exception = scope.exception()) {
-                scope.clearException();
                 arguments[0] = toRef(globalObject, exception);
+                scope.clearException();
             } else
                 arguments[0] = toRef(globalObject, jsResult);
             JSObjectCallAsFunction(context, callback, callback, 1, arguments, nullptr);
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@

		PASS Test that async messages can reply with optionals