Bug 165157 – Web Inspector: Clicking on link in Web Inspector can cause UIProcess to crash

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Summary:
Clicking on link in Web Inspector can cause UIProcess to crash

Test:
<a href="/http://example.com">Link</a>

Steps to Reproduce:
1. Inspect the link on the test page on a file:/// domain
2. Click on the "/http://example.com" link in the Element's DOM Tree
  => ASSERT / Crash

Crash:
Exception Type:        EXC_BREAKPOINT (SIGTRAP)
Exception Codes:       0x0000000000000002, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Application Specific Information:
Invalid WebProcess IPC Message:
Message "WebPageProxy.DecidePolicyForNavigationAction"

Application Specific Signatures:
Invalid Web Process IPC Message ID "WebPageProxy.DecidePolicyForNavigationAction"

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.Safari.framework    	0x00000001051d2bc7 invalidMessageFunction(OpaqueWKString const*) + 293 (AppController.mm:743)
1   com.apple.WebKit              	0x00000001081a46ca WebKit::WebProcessPool::didReceiveInvalidMessage(IPC::StringReference const&, IPC::StringReference const&) + 162 (Ref.h:58)
2   com.apple.WebKit              	0x00000001081b0f95 WebKit::WebProcessProxy::didReceiveInvalidMessage(IPC::Connection&, IPC::StringReference, IPC::StringReference) + 185 (WebProcessProxy.cpp:537)
3   com.apple.WebKit              	0x0000000107f7e425 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 211 (Connection.cpp:936)
4   com.apple.WebKit              	0x0000000107f81005 IPC::Connection::dispatchOneMessage() + 175 (memory:2722)

Notes:
Here we have a link like this:

    <a href="/http://example.com">Link</a>

Which should resolve to an absolute URL like:

    "<document base url>/http://example.com"

However Web Inspector thinks it is a complete URL:

    "/http://example.com"

Which when put into a <a> inside Web Inspector becomes:

    "file:///http://example.com"

When clicked in Web Inspector, Web Inspector tries to open a new tab for that URL. The UI Process sanity checks the URL and does not allow file URLs that aren't already allowed. Its defense is to crash.

The issue here was the absolute URL that Web Inspector created was incorrect because it incorrectly thought "/http://example.com" was a complete URL. The defense mechanism seems fine.

[Joseph Pecoraro]

<rdar://problem/27896562>

[Joseph Pecoraro]

Created attachment 295635 [details]
[PATCH] Proposed Fix

[Blaze Burg]

Comment on attachment 295635 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=295635&action=review

r=me

> LayoutTests/inspector/unit-tests/url-utilities.html:196
> +            // FIXME: <https://webkit.org/b/165155> Web Inspector: Use URL constructor to better handle all kinds of URLs

Do we want to leave this stuff in the test case? If it is currently broken in these cases maybe we can rebaseline the results to pass later.

[Joseph Pecoraro]

(In reply to comment #3)
> Comment on attachment 295635 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295635&action=review
> 
> r=me
> 
> > LayoutTests/inspector/unit-tests/url-utilities.html:196
> > +            // FIXME: <https://webkit.org/b/165155> Web Inspector: Use URL constructor to better handle all kinds of URLs
> 
> Do we want to leave this stuff in the test case? If it is currently broken
> in these cases maybe we can rebaseline the results to pass later.

Leave them in failing at the moment with a FIXME? Yeah, that sounds good. I'll do it.

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/209143>