Bug 163476 - Crash in ASCIICaseInsensitiveHash::hash() when a response has a null MIME type
Summary: Crash in ASCIICaseInsensitiveHash::hash() when a response has a null MIME type
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on: 163616
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-14 17:23 PDT by Andy Estes
Modified: 2016-10-19 13:33 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.37 KB, patch)
2016-10-14 18:00 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (317.95 KB, application/zip)
2016-10-14 18:46 PDT, Build Bot
no flags Details
Patch (17.59 KB, patch)
2016-10-17 16:27 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (10.65 KB, patch)
2016-10-19 12:09 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (10.73 KB, patch)
2016-10-19 13:17 PDT, Andy Estes
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2016-10-14 17:23:12 PDT
Crash in ASCIICaseInsensitiveHash::hash() when a response has a null MIME type
Comment 1 Andy Estes 2016-10-14 17:23:39 PDT
rdar://problem/26941395
Comment 2 Andy Estes 2016-10-14 18:00:48 PDT
Created attachment 291689 [details]
Patch
Comment 3 Tim Horton 2016-10-14 18:05:49 PDT
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.
Comment 4 Andy Estes 2016-10-14 18:11:57 PDT
(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.
Comment 5 Build Bot 2016-10-14 18:46:45 PDT
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.
Comment 6 Build Bot 2016-10-14 18:46:49 PDT
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
Comment 7 Andy Estes 2016-10-17 15:27:07 PDT
(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.
Comment 8 Andy Estes 2016-10-17 16:27:11 PDT
Created attachment 291894 [details]
Patch
Comment 9 Tim Horton 2016-10-17 16:54:25 PDT
Comment on attachment 291894 [details]
Patch

Assuming the test failure is a flake (but wait for EWS to see).
Comment 10 WebKit Commit Bot 2016-10-17 17:31:16 PDT
Comment on attachment 291894 [details]
Patch

Clearing flags on attachment: 291894

Committed r207443: <http://trac.webkit.org/changeset/207443>
Comment 11 WebKit Commit Bot 2016-10-17 17:31:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Commit Bot 2016-10-18 13:28:36 PDT
Re-opened since this is blocked by bug 163616
Comment 13 Andy Estes 2016-10-19 12:09:25 PDT
Created attachment 292091 [details]
Patch
Comment 14 Andreas Kling 2016-10-19 13:10:29 PDT
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.
Comment 15 Andy Estes 2016-10-19 13:17:44 PDT
Created attachment 292104 [details]
Patch
Comment 16 WebKit Commit Bot 2016-10-19 13:33:10 PDT
Comment on attachment 292104 [details]
Patch

Clearing flags on attachment: 292104

Committed r207563: <http://trac.webkit.org/changeset/207563>
Comment 17 WebKit Commit Bot 2016-10-19 13:33:16 PDT
All reviewed patches have been landed.  Closing bug.