Add the ability for the IPCTestingAPI to report when a sync message f…

…ails to be received correctly https://bugs.webkit.org/show_bug.cgi?id=264506 rdar://118188347 Reviewed by Alex Christensen. Add the ability for the IPCTestingAPI to report when a sync message fails to be received correctly. The existing behaviour is that both failed messages and empty replies return an empty IPC object. This change records the decoder validity on failure into the MessageSyncReply so the IPCTestingAPI can raise an exception instead. * Source/WebKit/Platform/IPC/Connection.cpp: (IPC::Connection::dispatchSyncMessage): * Source/WebKit/Platform/IPC/Decoder.cpp: (IPC::Decoder::hasMessageTestingError const): * Source/WebKit/Platform/IPC/Decoder.h: * Source/WebKit/Platform/IPC/Encoder.cpp: (IPC::Encoder::setMessageTestingError): * Source/WebKit/Platform/IPC/Encoder.h: * Source/WebKit/Platform/IPC/MessageFlags.h: * Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp: (WebKit::IPCTestingAPI::jsResultFromReplyDecoder): Canonical link: https://commits.webkit.org/270705@main
WebKit · Nov 14, 2023 · e36c143 · e36c143
1 parent 38f33ee
commit e36c143
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 5 deletions.
diff --git a/LayoutTests/ipc/send-invalid-sync-message-empty-reply-check-exception-expected.txt b/LayoutTests/ipc/send-invalid-sync-message-empty-reply-check-exception-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Sending sync message with incorrect parameters must throw error
+
diff --git a/LayoutTests/ipc/send-invalid-sync-message-empty-reply-check-exception.html b/LayoutTests/ipc/send-invalid-sync-message-empty-reply-check-exception.html
@@ -0,0 +1,19 @@
+<!doctype html><!-- webkit-test-runner [ IPCTestingAPIEnabled=true ] -->
+<title>Test that sending invalid sync messages via the IPC testing API throws the correct errors</title>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<body>
+<script>
+const defaultTimeout = 1000;
+
+promise_test(async t => {
+    if (!window.IPC)
+        return;
+
+    assert_throws_js(TypeError,
+        () => { IPC.sendSyncMessage("UI", 0, IPC.messages.IPCTester_SyncPingEmptyReply.name, defaultTimeout, []); },
+        `failed sync message must throw error`);
+}, "Sending sync message with incorrect parameters must throw error");
+
+</script>
+</body>
diff --git a/Source/WebKit/Platform/IPC/Connection.cpp b/Source/WebKit/Platform/IPC/Connection.cpp
@@ -1109,6 +1109,8 @@ void Connection::dispatchSyncMessage(Decoder& decoder)
 
 #if ENABLE(IPC_TESTING_API)
     ASSERT(decoder.isValid() || m_ignoreInvalidMessageForTesting);
+    if (!decoder.isValid())
+        replyEncoder->setSyncMessageDeserializationFailure();
 #else
     ASSERT(decoder.isValid());
 #endif

diff --git a/Source/WebKit/Platform/IPC/Decoder.cpp b/Source/WebKit/Platform/IPC/Decoder.cpp
@@ -123,6 +123,13 @@ bool Decoder::shouldMaintainOrderingWithAsyncMessages() const
     return m_messageFlags.contains(MessageFlags::MaintainOrderingWithAsyncMessages);
 }
 
+#if ENABLE(IPC_TESTING_API)
+bool Decoder::hasSyncMessageDeserializationFailure() const
+{
+    return m_messageFlags.contains(MessageFlags::SyncMessageDeserializationFailure);
+}
+#endif
+
 #if PLATFORM(MAC)
 void Decoder::setImportanceAssertion(ImportanceAssertion&& assertion)
 {

diff --git a/Source/WebKit/Platform/IPC/Decoder.h b/Source/WebKit/Platform/IPC/Decoder.h
@@ -92,6 +92,9 @@ class Decoder {
     ShouldDispatchWhenWaitingForSyncReply shouldDispatchMessageWhenWaitingForSyncReply() const;
     bool isAllowedWhenWaitingForSyncReply() const { return messageAllowedWhenWaitingForSyncReply(messageName()) || m_isAllowedWhenWaitingForSyncReplyOverride; }
     bool isAllowedWhenWaitingForUnboundedSyncReply() const { return messageAllowedWhenWaitingForUnboundedSyncReply(messageName()); }
+#if ENABLE(IPC_TESTING_API)
+    bool hasSyncMessageDeserializationFailure() const;
+#endif
     bool shouldUseFullySynchronousModeForTesting() const;
     bool shouldMaintainOrderingWithAsyncMessages() const;
     void setIsAllowedWhenWaitingForSyncReplyOverride(bool value) { m_isAllowedWhenWaitingForSyncReplyOverride = value; }

diff --git a/Source/WebKit/Platform/IPC/Encoder.cpp b/Source/WebKit/Platform/IPC/Encoder.cpp
@@ -114,6 +114,13 @@ void Encoder::setFullySynchronousModeForTesting()
     messageFlags().add(MessageFlags::UseFullySynchronousModeForTesting);
 }
 
+#if ENABLE(IPC_TESTING_API)
+void Encoder::setSyncMessageDeserializationFailure()
+{
+    messageFlags().add(MessageFlags::SyncMessageDeserializationFailure);
+}
+#endif
+
 void Encoder::setShouldMaintainOrderingWithAsyncMessages()
 {
     messageFlags().add(MessageFlags::MaintainOrderingWithAsyncMessages);

diff --git a/Source/WebKit/Platform/IPC/Encoder.h b/Source/WebKit/Platform/IPC/Encoder.h
@@ -65,6 +65,9 @@ class Encoder final {
     void setShouldMaintainOrderingWithAsyncMessages();
     bool isAllowedWhenWaitingForSyncReply() const { return messageAllowedWhenWaitingForSyncReply(messageName()) || isFullySynchronousModeForTesting(); }
     bool isAllowedWhenWaitingForUnboundedSyncReply() const { return messageAllowedWhenWaitingForUnboundedSyncReply(messageName()); }
+#if ENABLE(IPC_TESTING_API)
+    void setSyncMessageDeserializationFailure();
+#endif
 
     void wrapForTesting(UniqueRef<Encoder>&&);
 

diff --git a/Source/WebKit/Platform/IPC/MessageFlags.h b/Source/WebKit/Platform/IPC/MessageFlags.h
@@ -32,6 +32,9 @@ enum class MessageFlags : uint8_t {
     DispatchMessageWhenWaitingForUnboundedSyncReply = 1 << 1,
     UseFullySynchronousModeForTesting = 1 << 2,
     MaintainOrderingWithAsyncMessages = 1 << 3,
+#if ENABLE(IPC_TESTING_API)
+    SyncMessageDeserializationFailure = 1 << 4,
+#endif
 };
 
 enum class ShouldDispatchWhenWaitingForSyncReply : uint8_t {
@@ -46,11 +49,14 @@ namespace WTF {
 
 template<> struct EnumTraits<IPC::MessageFlags> {
     using values = EnumValues<
-        IPC::MessageFlags,
-        IPC::MessageFlags::DispatchMessageWhenWaitingForSyncReply,
-        IPC::MessageFlags::DispatchMessageWhenWaitingForUnboundedSyncReply,
-        IPC::MessageFlags::UseFullySynchronousModeForTesting,
-        IPC::MessageFlags::MaintainOrderingWithAsyncMessages
+        IPC::MessageFlags
+        , IPC::MessageFlags::DispatchMessageWhenWaitingForSyncReply
+        , IPC::MessageFlags::DispatchMessageWhenWaitingForUnboundedSyncReply
+        , IPC::MessageFlags::UseFullySynchronousModeForTesting
+        , IPC::MessageFlags::MaintainOrderingWithAsyncMessages
+#if ENABLE(IPC_TESTING_API)
+        , IPC::MessageFlags::SyncMessageDeserializationFailure
+#endif
     >;
 };
 

diff --git a/Source/WebKit/Shared/IPCTester.cpp b/Source/WebKit/Shared/IPCTester.cpp
@@ -213,6 +213,12 @@ void IPCTester::syncPing(IPC::Connection&, uint32_t value, CompletionHandler<voi
     completionHandler(value + 1);
 }
 
+void IPCTester::syncPingEmptyReply(IPC::Connection&, uint32_t value, CompletionHandler<void()>&& completionHandler)
+{
+    UNUSED_PARAM(value);
+    completionHandler();
+}
+
 void IPCTester::stopIfNeeded()
 {
     if (m_testQueue) {

diff --git a/Source/WebKit/Shared/IPCTester.h b/Source/WebKit/Shared/IPCTester.h
@@ -73,6 +73,7 @@ class IPCTester final : public IPC::MessageReceiver {
     void sendAsyncMessageToReceiver(IPC::Connection&, uint32_t);
     void asyncPing(IPC::Connection&, 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 stopIfNeeded();
 

diff --git a/Source/WebKit/Shared/IPCTester.messages.in b/Source/WebKit/Shared/IPCTester.messages.in
@@ -37,6 +37,8 @@ messages -> IPCTester NotRefCounted {
     AsyncPing(uint32_t value) -> (uint32_t nextValue)
     SyncPing(uint32_t value) -> (uint32_t nextValue) Synchronous
 
+    SyncPingEmptyReply(uint32_t value) -> () Synchronous
+
     SendAsyncMessageToReceiver(uint32_t arg0)
 }
 

diff --git a/Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp b/Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp
@@ -2344,6 +2344,11 @@ static JSC::JSObject* jsResultFromReplyDecoder(JSC::JSGlobalObject* globalObject
     auto& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
+    if (decoder.hasSyncMessageDeserializationFailure()) {
+        throwException(globalObject, scope, JSC::createTypeError(globalObject, "Failed to successfully deserialize the message"_s));
+        return nullptr;
+    }
+
     auto arrayBuffer = JSC::ArrayBuffer::create(decoder.buffer());
     JSC::JSArrayBuffer* jsArrayBuffer = nullptr;
     if (auto* structure = globalObject->arrayBufferStructure(arrayBuffer->sharingMode()))