Bug 163476 – Crash in ASCIICaseInsensitiveHash::hash() when a response has a null MIME type

• Component: New Bugs
• Status: RESOLVED FIXED

[Andy Estes]

Crash in ASCIICaseInsensitiveHash::hash() when a response has a null MIME type

[Andy Estes]

rdar://problem/26941395

[Andy Estes]

Created attachment 291689 [details]
Patch

[Tim Horton]

Comment on attachment 291689 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/LoadDataWithNilMIMEType.mm:38
> +    [webView loadData:[@"test" dataUsingEncoding:NSUTF8StringEncoding] MIMEType:mimeType characterEncodingName:@"UTF-8" baseURL:[NSURL URLWithString:@"about:blank"]];

How are we actually getting the nulls in here? MIMEType is non-nullable, so we could theoretically instead raise an exception, but it seems like this was happening in Safari, so maybe it's possible to get a null MIME type in a response from the network? I wonder if we can test that.

[Andy Estes]

(In reply to comment #3)
> Comment on attachment 291689 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291689&action=review
> 
> > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/LoadDataWithNilMIMEType.mm:38
> > +    [webView loadData:[@"test" dataUsingEncoding:NSUTF8StringEncoding] MIMEType:mimeType characterEncodingName:@"UTF-8" baseURL:[NSURL URLWithString:@"about:blank"]];
> 
> How are we actually getting the nulls in here? MIMEType is non-nullable, so
> we could theoretically instead raise an exception, but it seems like this
> was happening in Safari, so maybe it's possible to get a null MIME type in a
> response from the network? I wonder if we can test that.

We see this crash in a third-party app, and the call stack shows it's loading substitute data, so I suspect this API test reflects what's happening (I don't know of any other API that will cause a substitute data load).

This does also happen in Safari for non-substitute data loads, so I tried to write a HTTP test for this that omits the Content-Type header. Unfortunately, CFNetwork will guess a MIME type if the server doesn't specify one, and it wasn't clear to me how to stop CFNetwork from guessing.

[Build Bot]

Comment on attachment 291689 [details]
Patch

Attachment 291689 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2287627

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 291696 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Andy Estes]

(In reply to comment #5)
> Comment on attachment 291689 [details]
> Patch
> 
> Attachment 291689 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/2287627
> 
> Number of test failures exceeded the failure limit.

Rearranging WebPage::shouldUseCustomContentProviderForResponse() to call canPluginHandleResponse() before checking m_mimeTypesWithCustomContentProviders caused these crashes. They look like this:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x000000010dcd60ee WebKit::WebPage::canPluginHandleResponse(WebCore::ResourceResponse const&) + 30
1   com.apple.WebKit              	0x000000010dcd6262 WebKit::WebPage::shouldUseCustomContentProviderForResponse(WebCore::ResourceResponse const&) + 18
2   com.apple.WebKit              	0x000000010dc9aaaf WebKit::WebFrameLoaderClient::transitionToCommittedForNewPage() + 167
3   com.apple.WebCore             	0x000000011000899b WebCore::FrameLoader::transitionToCommitted(WebCore::CachedPage*) + 619 (RefPtr.h:74)
4   com.apple.WebCore             	0x0000000110007c78 WebCore::FrameLoader::commitProvisionalLoad() + 360 (FrameLoader.cpp:1799)
5   com.apple.WebCore             	0x000000010febe346 WebCore::DocumentLoader::finishedLoading(double) + 182 (DocumentLoader.cpp:158)
6   com.apple.WebCore             	0x000000010fec2c10 WebCore::DocumentLoader::maybeLoadEmpty() + 784 (utility:765)
7   com.apple.WebCore             	0x000000010fec2f3a WebCore::DocumentLoader::startLoadingMainResource() + 714 (DocumentLoader.cpp:1512)
8   com.apple.WebCore             	0x000000010fffa669 WebCore::FrameLoader::init() + 953 (FrameLoader.cpp:292)
9   com.apple.WebKit              	0x000000010dc93b5b WebKit::WebFrame::createWithCoreMainFrame(WebKit::WebPage*, WebCore::Frame*) + 183
10  com.apple.WebKit              	0x000000010dcc9d30 WebKit::WebPage::WebPage(unsigned long long, WebKit::WebPageCreationParameters const&) + 3490
11  com.apple.WebKit              	0x000000010dcc8f50 WebKit::WebPage::create(unsigned long long, WebKit::WebPageCreationParameters const&) + 52
12  com.apple.WebKit              	0x000000010dd3df85 WebKit::WebProcess::createWebPage(unsigned long long, WebKit::WebPageCreationParameters const&) + 85
13  com.apple.WebKit              	0x000000010dd4cb17 void IPC::handleMessage<Messages::WebProcess::CreateWebPage, WebKit::WebProcess, void (WebKit::WebProcess::*)(unsigned long long, WebKit::WebPageCreationParameters const&)>(IPC::Decoder&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(unsigned long long, WebKit::WebPageCreationParameters const&)) + 264

WebPage::m_mainFrame is NULL because we are in the middle of calling WebFrame::createWithCoreMainFrame() in WebPage's constructor :(

I guess I just need to null-check m_mainFrame, but that seems unfortunate.

[Andy Estes]

Created attachment 291894 [details]
Patch

[Tim Horton]

Comment on attachment 291894 [details]
Patch

Assuming the test failure is a flake (but wait for EWS to see).

[WebKit Commit Bot]

Comment on attachment 291894 [details]
Patch

Clearing flags on attachment: 291894

Committed r207443: <http://trac.webkit.org/changeset/207443>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 163616

[Andy Estes]

Created attachment 292091 [details]
Patch

[Andreas Kling]

Comment on attachment 292091 [details]
Patch

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

r=me

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4607
> +    return m_mimeTypesWithCustomContentProviders.contains(mimeType) && !canPluginHandleResponse(response);

I would personally put a comment here noting that canPluginHandleResponse() is called last since it may involve synchronous IPC.

[Andy Estes]

Created attachment 292104 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 292104 [details]
Patch

Clearing flags on attachment: 292104

Committed r207563: <http://trac.webkit.org/changeset/207563>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.