| Summary: | Crash in WebPlatformStrategies::createPingHandle - Deref a null NetworkingContext | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||
| Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | andersca, ap, commit-queue, dbates, kling, koivisto, mkwst, msaboff, simon.fraser, thorton | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Brady Eidson
2015-07-23 09:54:52 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. Created attachment 257354 [details]
Patch v1
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? (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. This test fails on Windows every time (maybe unsupported testRunner functionality?) (In reply to comment #6) > This test fails on Windows every time (maybe unsupported testRunner > functionality?) Probably. Will take a look. 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?) I suspected that addUserStyleSheet wasn't implemented, but looks like it is. (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. |