RESOLVED FIXED 147227
Crash in WebPlatformStrategies::createPingHandle - Deref a null NetworkingContext
https://bugs.webkit.org/show_bug.cgi?id=147227
Summary Crash in WebPlatformStrategies::createPingHandle - Deref a null NetworkingCon...
Brady Eidson
Reported 2015-07-23 09:54:52 PDT
Crash in WebPlatformStrategies::createPingHandle - Deref a null NetworkingContext 0 com.apple.WebKit 0x00007fff9152d6e8 WebKit::WebFrameNetworkingContext::webFrameLoaderClient() const + 6 1 com.apple.WebKit 0x00007fff9159b224 WebKit::WebPlatformStrategies::createPingHandle(WebCore::NetworkingContext*, WebCore::ResourceRequest&, bool) + 64 2 com.apple.WebCore 0x00007fff97c7a696 WebCore::PingLoader::startPingLoad(WebCore::Frame&, WebCore::ResourceRequest&) + 566 3 com.apple.WebCore 0x00007fff97c7b0f6 WebCore::PingLoader::sendViolationReport(WebCore::Frame&, WebCore::URL const&, WTF::PassRefPtr<WebCore::FormData>) + 918 4 com.apple.WebCore 0x00007fff974f7fe2 WebCore::ContentSecurityPolicy::reportViolation(WTF::String const&, WTF::String const&, WTF::String const&, WebCore::URL const&, WTF::Vector<WebCore::URL, 0ul, WTF::CrashOnOverflow, 16ul> const&, WTF::String const&, WTF::String const&, WTF::OrdinalNumber const&, JSC::ExecState*) const + 3554 5 com.apple.WebCore 0x00007fff974f71c8 WebCore::CSPDirectiveList::reportViolation(WTF::String const&, WTF::String const&, WTF::String const&, WebCore::URL const&, WTF::String const&, WTF::OrdinalNumber const&, JSC::ExecState*) const + 280 ... 30 com.apple.WebCore 0x00007fff9713f482 WebCore::FrameLoader::init() + 770 31 com.apple.WebKit 0x00007fff91392fe9 WebKit::WebFrame::createSubframe(WebKit::WebPage*, WTF::String const&, WebCore::HTMLFrameOwnerElement*) + 381 32 com.apple.WebKit 0x00007fff9152a639 WebKit::WebFrameLoaderClient::createFrame(WebCore::URL const&, WTF::String const&, WebCore::HTMLFrameOwnerElement*, WTF::String const&, bool, int, int) + 63 33 com.apple.WebCore 0x00007fff97ece1db WebCore::SubframeLoader::loadSubframe(WebCore::HTMLFrameOwnerElement&, WebCore::URL const&, WTF::String const&, WTF::String const&) + 283 UserStyleSheet, with a font-src rule, injected into the initial empty document for a new frame, but with CSP blocking the font load, and triggering a violation report. Wow! Easy to repro with a layout test, though. Radar - <rdar://problem/21949735>
Attachments
Patch v1 (4.29 KB, patch)
2015-07-23 10:01 PDT, Brady Eidson
ap: review+
Brady Eidson
Comment 1 2015-07-23 09:56:00 PDT
In WK2, it would actually be possible to bypass the need for the networking context and start the load in the Networking Process. But it's a bit unclear what we should be doing in the initial-empty-document case. Previously, this precise violation report would not start a ping because of the null networking context, so for now let's restore that behavior.
Brady Eidson
Comment 2 2015-07-23 10:01:55 PDT
Created attachment 257354 [details] Patch v1
Alexey Proskuryakov
Comment 3 2015-07-23 10:32:04 PDT
Comment on attachment 257354 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=257354&action=review Hrmpf. Nice. > LayoutTests/http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher-expected.txt:3 > +CONSOLE MESSAGE: Refused to load the font 'http://127.0.0.1:8000/security/contentSecurityPolicy/example_font.woff' because it violates the following Content Security Policy directive: "font-src http://webkit.org". > + > +CONSOLE MESSAGE: Refused to load the font 'http://127.0.0.1:8000/security/contentSecurityPolicy/example_font.woff' because it violates the following Content Security Policy directive: "font-src http://webkit.org". Why is this logged twice, do we have a bug? > LayoutTests/http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.html:8 > + testRunner.addUserStyleSheet("@font-face { font-family: ExampleFont; src: url(example_font.woff); }", true); I wonder if this can also be reproduced with something like <iframe src="http://www.apple.com"></iframe> <script> frames[0].document.write(theStylesheet); </script> > Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:255 > + // We shouldn't be sending ping loads during that process anyways. What do other browsers do? I wonder if "shouldn't" may be too assertive. It feels like we shouldn't apply user stylesheets in initial documents, what do you think?
Brady Eidson
Comment 4 2015-07-23 11:11:01 PDT
(In reply to comment #3) > Comment on attachment 257354 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257354&action=review > > Hrmpf. Nice. > > > LayoutTests/http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher-expected.txt:3 > > +CONSOLE MESSAGE: Refused to load the font 'http://127.0.0.1:8000/security/contentSecurityPolicy/example_font.woff' because it violates the following Content Security Policy directive: "font-src http://webkit.org". > > + > > +CONSOLE MESSAGE: Refused to load the font 'http://127.0.0.1:8000/security/contentSecurityPolicy/example_font.woff' because it violates the following Content Security Policy directive: "font-src http://webkit.org". > > Why is this logged twice, do we have a bug? No - the style sheet is injected into both frames, so the CSP fires twice. > > > LayoutTests/http/tests/security/contentSecurityPolicy/user-style-sheet-font-crasher.html:8 > > + testRunner.addUserStyleSheet("@font-face { font-family: ExampleFont; src: url(example_font.woff); }", true); > > I wonder if this can also be reproduced with something like > > <iframe src="http://www.apple.com"></iframe> > <script> > frames[0].document.write(theStylesheet); > </script> > That wouldn't do it, because initial document creation is synchronous, and completed by the time you get to document.write. By the time you get there, you'd have the networking context and wouldn't repro the crash. > > Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:255 > > + // We shouldn't be sending ping loads during that process anyways. > > What do other browsers do? I wonder if "shouldn't" may be too assertive. > > It feels like we shouldn't apply user stylesheets in initial documents, what > do you think? Perhaps it is too assertive. I'll relax it before landing. I postulated at least one other solution in the radar but it seems much higher risk. I'm not sure whether or not user sheets should apply to the initial empty document - I'd want Simon, Antti, Andreas, etc to comment. As is, this patch restores precisely the behavior we had before https://trac.webkit.org/changeset/186530 - I'm pretty confident in that interim step while we explore what we should *really* be doing here.
Brady Eidson
Comment 5 2015-07-23 13:20:31 PDT
Alexey Proskuryakov
Comment 6 2015-07-23 21:59:27 PDT
This test fails on Windows every time (maybe unsupported testRunner functionality?)
Brady Eidson
Comment 7 2015-07-23 22:32:21 PDT
(In reply to comment #6) > This test fails on Windows every time (maybe unsupported testRunner > functionality?) Probably. Will take a look.
Brady Eidson
Comment 8 2015-07-23 22:36:14 PDT
Hmmmm, the message comes from: - (void)webView:(WebView *)sender addMessageToConsole:(NSDictionary *)dictionary withSource:(NSString *)source ...in Mac DRT, and: "willAddMessageToConsole" ...in WK2 WKTR. There is a "CONSOLE MESSAGE" string in DRT Win, in: HRESULT UIDelegate::webViewAddMessageToConsole(IWebView* /*sender*/, BSTR message, int lineNumber, BSTR url, BOOL isError) I guess that's not getting called (or its bailing out early?)
Alexey Proskuryakov
Comment 9 2015-07-23 22:52:51 PDT
I suspected that addUserStyleSheet wasn't implemented, but looks like it is.
Brady Eidson
Comment 10 2015-07-23 22:57:11 PDT
(In reply to comment #9) > I suspected that addUserStyleSheet wasn't implemented, but looks like it is. Is CSP not working in WebKitWin? That would explain it, too.
Note You need to log in before you can comment on or make changes to this bug.