Changeset 184846 in webkit


Ignore:
Timestamp:
May 24, 2015 4:22:06 PM (9 years ago)
Author:
weinig@apple.com
Message:

Crash when using a removed ScriptMessageHandler
<rdar://problem/20888499>
​https://bugs.webkit.org/show_bug.cgi?id=145359

Reviewed by Dan Bernstein.

Source/WebCore:

Added tests:

WKUserContentController.ScriptMessageHandlerBasicRemove
WKUserContentController.ScriptMessageHandlerCallRemovedHandler

  • page/UserMessageHandler.cpp:

(WebCore::UserMessageHandler::~UserMessageHandler):
(WebCore::UserMessageHandler::postMessage):
(WebCore::UserMessageHandler::name):

  • page/UserMessageHandler.h:

(WebCore::UserMessageHandler::create):

  • page/UserMessageHandler.idl:
  • page/UserMessageHandlerDescriptor.cpp:

(WebCore::UserMessageHandlerDescriptor::UserMessageHandlerDescriptor):

  • page/UserMessageHandlerDescriptor.h:

(WebCore::UserMessageHandlerDescriptor::client):
(WebCore::UserMessageHandlerDescriptor::invalidateClient):
Add support for invalidating the descriptor and throw an exception if someone tries
to post a message using an invalidated descriptor.

  • page/UserMessageHandlersNamespace.cpp:

(WebCore::UserMessageHandlersNamespace::handler):
Add logic to remove message handlers if their descriptor has been invalidated.

Source/WebKit2:

  • WebProcess/UserContent/WebUserContentController.cpp:

(WebKit::WebUserMessageHandlerDescriptorProxy::~WebUserMessageHandlerDescriptorProxy):
Invalidate the descriptor when the message handler client (as implemented by WebUserMessageHandlerDescriptorProxy)
goes away. This will happen if a script message handler is removed at the API level or the WebUserContentController
is destroyed (which will happen if all the pages get destroyed).

Tools:

  • TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm:

Add tests for removing script message handlers.

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r184845 r184846  
     12015-05-24  Sam Weinig  <sam@webkit.org>
     2
     3        Crash when using a removed ScriptMessageHandler
     4        <rdar://problem/20888499>
     5        https://bugs.webkit.org/show_bug.cgi?id=145359
     6
     7        Reviewed by Dan Bernstein.
     8
     9        Added tests:
     10            WKUserContentController.ScriptMessageHandlerBasicRemove
     11            WKUserContentController.ScriptMessageHandlerCallRemovedHandler
     12
     13        * page/UserMessageHandler.cpp:
     14        (WebCore::UserMessageHandler::~UserMessageHandler):
     15        (WebCore::UserMessageHandler::postMessage):
     16        (WebCore::UserMessageHandler::name):
     17        * page/UserMessageHandler.h:
     18        (WebCore::UserMessageHandler::create):
     19        * page/UserMessageHandler.idl:
     20        * page/UserMessageHandlerDescriptor.cpp:
     21        (WebCore::UserMessageHandlerDescriptor::UserMessageHandlerDescriptor):
     22        * page/UserMessageHandlerDescriptor.h:
     23        (WebCore::UserMessageHandlerDescriptor::client):
     24        (WebCore::UserMessageHandlerDescriptor::invalidateClient):
     25        Add support for invalidating the descriptor and throw an exception if someone tries
     26        to post a message using an invalidated descriptor.
     27
     28        * page/UserMessageHandlersNamespace.cpp:
     29        (WebCore::UserMessageHandlersNamespace::handler):
     30        Add logic to remove message handlers if their descriptor has been invalidated.
     31
    1322015-05-23  Dan Bernstein  <mitz@apple.com>
    233

  • trunk/Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp

    r177143 r184846  
    8787
    8888    WebCore::JSMainThreadNullState state;
    89     handler->postMessage(WebCore::SerializedScriptValue::create(String::fromUTF8(message)));
     89    WebCore::ExceptionCode ec = 0;
     90    handler->postMessage(WebCore::SerializedScriptValue::create(String::fromUTF8(message)), ec);
     91    if (ec)
     92        return FALSE;
     93
    9094    return TRUE;
    9195}

  • trunk/Source/WebCore/page/UserMessageHandler.cpp

    r169023 r184846  
    2929#if ENABLE(USER_MESSAGE_HANDLERS)
    3030
     31#include "ExceptionCode.h"
    3132#include "Frame.h"
    3233#include "SerializedScriptValue.h"
    … …  
    4445}
    4546
    46 void UserMessageHandler::postMessage(PassRefPtr<SerializedScriptValue> value)
     47void UserMessageHandler::postMessage(PassRefPtr<SerializedScriptValue> value, ExceptionCode& ec)
    4748{
    48     m_descriptor->client().didPostMessage(*this, value.get());
     49    // Check to see if the descriptor has been removed. This can happen if the host application has
     50    // removed the named message handler at the WebKit2 API level.
     51    if (!m_descriptor->client()) {
     52        ec = INVALID_ACCESS_ERR;
     53        return;
     54    }
     55
     56    m_descriptor->client()->didPostMessage(*this, value.get());
    4957}
    5058

  • trunk/Source/WebCore/page/UserMessageHandler.h

    r177259 r184846  
    3535namespace WebCore {
    3636
     37typedef int ExceptionCode;
     38
    3739class UserMessageHandler : public RefCounted<UserMessageHandler>, public FrameDestructionObserver {
    3840public:
    … …  
    4345    virtual ~UserMessageHandler();
    4446
    45     void postMessage(PassRefPtr<SerializedScriptValue>);
     47    void postMessage(PassRefPtr<SerializedScriptValue>, ExceptionCode&);
    4648
    4749    const AtomicString& name();

  • trunk/Source/WebCore/page/UserMessageHandler.idl

    r169023 r184846  
    2727    Conditional=USER_MESSAGE_HANDLERS
    2828] interface UserMessageHandler {
    29     void postMessage(SerializedScriptValue message);
     29    [RaisesException] void postMessage(SerializedScriptValue message);
    3030};

  • trunk/Source/WebCore/page/UserMessageHandlerDescriptor.cpp

    r169023 r184846  
    3636    : m_name(name)
    3737    , m_world(world)
    38     , m_client(client)
     38    , m_client(&client)
    3939{
    4040}

  • trunk/Source/WebCore/page/UserMessageHandlerDescriptor.h

    r184066 r184846  
    5757    const AtomicString& name();
    5858    DOMWrapperWorld& world();
    59    
    60     Client& client() const { return m_client; }
     59
     60    Client* client() const { return m_client; }
     61    void invalidateClient() { m_client = nullptr; }
    6162
    6263private:
    6364    WEBCORE_EXPORT explicit UserMessageHandlerDescriptor(const AtomicString&, DOMWrapperWorld&, Client&);
    64    
     65
    6566    AtomicString m_name;
    6667    Ref<DOMWrapperWorld> m_world;
    67     Client& m_client;
     68    Client* m_client;
    6869};
    6970

  • trunk/Source/WebCore/page/UserMessageHandlersNamespace.cpp

    r180291 r184846  
    4747UserMessageHandler* UserMessageHandlersNamespace::handler(const AtomicString& name, DOMWrapperWorld& world)
    4848{
    49     // First, check if we have a handler instance already.
    50     for (auto& handler : m_messageHandlers) {
    51         if (handler->name() == name && &handler->world() == &world)
    52             return &handler.get();
    53     }
    54 
    55     // Second, attempt to create a handler instance from a descriptor.
    5649    if (!frame())
    5750        return nullptr;
    … …  
    7063
    7164    RefPtr<UserMessageHandlerDescriptor> descriptor = userMessageHandlerDescriptors->get(std::make_pair(name, &world));
    72     if (!descriptor)
     65    if (!descriptor) {
     66        m_messageHandlers.removeFirstMatching([&name, &world](Ref<UserMessageHandler>& handler) {
     67            return handler->name() == name && &handler->world() == &world;
     68        });
    7369        return nullptr;
     70    }
     71
     72    for (auto& handler : m_messageHandlers) {
     73        if (handler->name() == name && &handler->world() == &world)
     74            return &handler.get();
     75    }
    7476
    7577    m_messageHandlers.append(UserMessageHandler::create(*frame(), *descriptor));

  • trunk/Source/WebKit2/ChangeLog

    r184834 r184846  
     12015-05-24  Sam Weinig  <sam@webkit.org>
     2
     3        Crash when using a removed ScriptMessageHandler
     4        <rdar://problem/20888499>
     5        https://bugs.webkit.org/show_bug.cgi?id=145359
     6
     7        Reviewed by Dan Bernstein.
     8
     9        * WebProcess/UserContent/WebUserContentController.cpp:
     10        (WebKit::WebUserMessageHandlerDescriptorProxy::~WebUserMessageHandlerDescriptorProxy):
     11        Invalidate the descriptor when the message handler client (as implemented by WebUserMessageHandlerDescriptorProxy)
     12        goes away. This will happen if a script message handler is removed at the API level or the WebUserContentController
     13        is destroyed (which will happen if all the pages get destroyed).
     14
    1152015-05-23  Dan Bernstein  <mitz@apple.com>
    216

  • trunk/Source/WebKit2/WebProcess/UserContent/WebUserContentController.cpp

    r184066 r184846  
    118118    virtual ~WebUserMessageHandlerDescriptorProxy()
    119119    {
     120        m_descriptor->invalidateClient();
    120121    }
    121122

  • trunk/Tools/ChangeLog

    r184845 r184846  
     12015-05-24  Sam Weinig  <sam@webkit.org>
     2
     3        Crash when using a removed ScriptMessageHandler
     4        <rdar://problem/20888499>
     5        https://bugs.webkit.org/show_bug.cgi?id=145359
     6
     7        Reviewed by Dan Bernstein.
     8
     9        * TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm:
     10        Add tests for removing script message handlers.
     11
    1122015-05-23  Dan Bernstein  <mitz@apple.com>
    213

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/UserContentController.mm

    r177506 r184846  
    6363@end
    6464
    65 TEST(WKUserContentController, ScriptMessageHandlerSimple)
     65TEST(WKUserContentController, ScriptMessageHandlerBasicPost)
    6666{
    6767    RetainPtr<ScriptMessageHandler> handler = adoptNS([[ScriptMessageHandler alloc] init]);
    … …  
    8686
    8787    EXPECT_WK_STREQ(@"Hello", (NSString *)[lastScriptMessage body]);
     88}
     89
     90TEST(WKUserContentController, ScriptMessageHandlerBasicRemove)
     91{
     92    RetainPtr<ScriptMessageHandler> handler = adoptNS([[ScriptMessageHandler alloc] init]);
     93    RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     94    RetainPtr<WKUserContentController> userContentController = [configuration userContentController];
     95    [userContentController addScriptMessageHandler:handler.get() name:@"handlerToRemove"];
     96    [userContentController addScriptMessageHandler:handler.get() name:@"handlerToPost"];
     97
     98    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
     99
     100    RetainPtr<SimpleNavigationDelegate> delegate = adoptNS([[SimpleNavigationDelegate alloc] init]);
     101    [webView setNavigationDelegate:delegate.get()];
     102
     103    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
     104
     105    [webView loadRequest:request];
     106
     107    TestWebKitAPI::Util::run(&isDoneWithNavigation);
     108
     109    // Test that handlerToRemove was succesfully added.
     110    [webView evaluateJavaScript:
     111        @"if (window.webkit.messageHandlers.handlerToRemove) {"
     112         "    window.webkit.messageHandlers.handlerToPost.postMessage('PASS');"
     113         "} else {"
     114         "    window.webkit.messageHandlers.handlerToPost.postMessage('FAIL');"
     115         "}" completionHandler:nil];
     116
     117    TestWebKitAPI::Util::run(&receivedScriptMessage);
     118    receivedScriptMessage = false;
     119
     120    EXPECT_WK_STREQ(@"PASS", (NSString *)[lastScriptMessage body]);
     121
     122    [userContentController removeScriptMessageHandlerForName:@"handlerToRemove"];
     123
     124    // Test that handlerToRemove has been removed.
     125    [webView evaluateJavaScript:
     126        @"if (window.webkit.messageHandlers.handlerToRemove) {"
     127         "    window.webkit.messageHandlers.handlerToPost.postMessage('FAIL');"
     128         "} else {"
     129         "    window.webkit.messageHandlers.handlerToPost.postMessage('PASS');"
     130         "}" completionHandler:nil];
     131
     132    TestWebKitAPI::Util::run(&receivedScriptMessage);
     133    receivedScriptMessage = false;
     134
     135    EXPECT_WK_STREQ(@"PASS", (NSString *)[lastScriptMessage body]);
     136}
     137
     138TEST(WKUserContentController, ScriptMessageHandlerCallRemovedHandler)
     139{
     140    RetainPtr<ScriptMessageHandler> handler = adoptNS([[ScriptMessageHandler alloc] init]);
     141    RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     142    RetainPtr<WKUserContentController> userContentController = [configuration userContentController];
     143    [userContentController addScriptMessageHandler:handler.get() name:@"handlerToRemove"];
     144    [userContentController addScriptMessageHandler:handler.get() name:@"handlerToPost"];
     145
     146    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
     147
     148    RetainPtr<SimpleNavigationDelegate> delegate = adoptNS([[SimpleNavigationDelegate alloc] init]);
     149    [webView setNavigationDelegate:delegate.get()];
     150
     151    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
     152
     153    [webView loadRequest:request];
     154
     155    TestWebKitAPI::Util::run(&isDoneWithNavigation);
     156
     157    [webView evaluateJavaScript:@"var handlerToRemove = window.webkit.messageHandlers.handlerToRemove;" completionHandler:nil];
     158
     159    [userContentController removeScriptMessageHandlerForName:@"handlerToRemove"];
     160
     161    // Test that we throw an exception if you try to use a message handler that has been removed.
     162    [webView evaluateJavaScript:
     163        @"try {"
     164         "    handlerToRemove.postMessage('FAIL');"
     165         "} catch (e) {"
     166         "    window.webkit.messageHandlers.handlerToPost.postMessage('PASS');"
     167         "}" completionHandler:nil];
     168
     169    TestWebKitAPI::Util::run(&receivedScriptMessage);
     170    receivedScriptMessage = false;
     171
     172    EXPECT_WK_STREQ(@"PASS", (NSString *)[lastScriptMessage body]);
    88173}
    89174

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebExtensionTest.cpp

    r183265 r184846  
    105105        WebKitDOMUserMessageHandlersNamespace* messageHandlers = webkit_dom_webkit_namespace_get_message_handlers(webkit);
    106106        if (WebKitDOMUserMessageHandler* handler = webkit_dom_user_message_handlers_namespace_get_handler(messageHandlers, "dom"))
    107             webkit_dom_user_message_handler_post_message(handler, "DocumentLoaded");
     107            webkit_dom_user_message_handler_post_message(handler, "DocumentLoaded", nullptr);
    108108    }
    109109
Note: See TracChangeset for help on using the changeset viewer.