* SUMMARY Crash under WebCore::PageConsoleClient::addMessage attempting to log insecure content message in ImageDocument * CRASH SNIPPET Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000 Exception Note: EXC_CORPSE_NOTIFY Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 WebCore::PageConsoleClient::addMessage(JSC::MessageSource, JSC::MessageLevel, WTF::String const&, unsigned long, WebCore::Document*) + 83 1 WebCore::MixedContentChecker::logWarning(bool, WTF::String const&, WebCore::URL const&) const + 329 2 WebCore::MixedContentChecker::canDisplayInsecureContent(WebCore::SecurityOrigin*, WebCore::MixedContentChecker::ContentType, WebCore::URL const&) const + 192 3 WebCore::CachedResourceLoader::checkInsecureContent(WebCore::CachedResource::Type, WebCore::URL const&) const + 134 4 WebCore::CachedResourceLoader::canRequest(WebCore::CachedResource::Type, WebCore::URL const&, WebCore::ResourceLoaderOptions const&, bool) + 681 5 WebCore::CachedResourceLoader::requestResource(WebCore::CachedResource::Type, WebCore::CachedResourceRequest&) + 355 6 WebCore::CachedResourceLoader::requestFont(WebCore::CachedResourceRequest&, bool) + 67 7 WebCore::CSSFontFaceSrcValue::cachedFont(WebCore::Document*, bool) + 852 8 WebCore::CSSFontSelector::addFontFaceRule(WebCore::StyleRuleFontFace const*) + 1559 9 WebCore::RuleSet::addChildRules(WTF::Vector<WTF::RefPtr<WebCore::StyleRuleBase>, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::MediaQueryEvaluator const&, WebCore::StyleResolver*, bool, WebCore::AddRuleFlags) + 398 10 WebCore::RuleSet::addRulesFromSheet(WebCore::StyleSheetContents*, WebCore::MediaQueryEvaluator const&, WebCore::StyleResolver*) + 193 11 WebCore::DocumentRuleSets::initUserStyle(WebCore::DocumentStyleSheetCollection&, WebCore::MediaQueryEvaluator const&, WebCore::StyleResolver&) + 505 12 WebCore::StyleResolver::StyleResolver(WebCore::Document&, bool) + 1257 13 WebCore::Document::createStyleResolver() + 82 14 WebCore::Style::attachRenderTree(WebCore::Element&, WebCore::RenderStyle&, WebCore::RenderTreePosition&, WTF::PassRefPtr<WebCore::RenderStyle>) + 669 15 WebCore::Style::resolveTree(WebCore::Element&, WebCore::RenderStyle&, WebCore::RenderTreePosition&, WebCore::Style::Change) + 713 16 WebCore::Style::resolveTree(WebCore::Document&, WebCore::Style::Change) + 328 17 WebCore::Document::recalcStyle(WebCore::Style::Change) + 281 18 WebCore::ImageDocument::imageUpdated() + 26 19 WebCore::ImageDocument::updateDuringParsing() + 150 20 WebCore::DocumentLoader::commitData(char const*, unsigned long) + 579 21 WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 50 22 WebCore::DocumentLoader::commitLoad(char const*, int) + 145 23 WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int) + 160 24 WebCore::CachedRawResource::addDataBuffer(WebCore::SharedBuffer&) + 145 25 WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::PassRefPtr<WebCore::SharedBuffer>, long long, WebCore::DataPayloadType) + 210 26 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType) + 35 27 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::MessageDecoder&) + 257 28 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 102 29 IPC::Connection::dispatchOneMessage() + 114 * NOTES Nothing in MixedContentChecker looks suspicious in its call to Document::addConsoleMessage given the surrounding code. But it is espeically suspicous that the crash happened inside of this signature of addMessage, which eventually just calls through to the others, so the crash likely happened before it got to pass through. This looks particularly suspicious: ScriptableDocumentParser* parser = document->scriptableDocumentParser(); … if (!parser->shouldAssociateConsoleMessagesWithTextPosition()) return; Because apparently Document::scriptableDocumentParser can return nullptr. ScriptableDocumentParser* Document::scriptableDocumentParser() const { return parser() ? parser()->asScriptableDocumentParser() : nullptr; } An early return should be fine.
<rdar://problem/21423510>
I was unable to write a test for this, but no doubt it is possible. Any pointers would be very helpful. I attempted to write a test in LayoutTests/http/tests/security/mixedContent loading an SVG document that loads an insecure CSS font. It seems I wasn't able to trigger the ImageDocument path. Likewise loading the SVG directly over https didn't reproduce the issue for me. That said, I believe the fix is rather easy to follow from the crash log, and just makes sense. So I'm proposing the fix without a test.
Created attachment 255080 [details] [PATCH] Proposed Fix
(In reply to comment #2) > I was unable to write a test for this, but no doubt it is possible. Any > pointers would be very helpful. I attempted to write a test in > LayoutTests/http/tests/security/mixedContent loading an SVG document that > loads an insecure CSS font. It seems I wasn't able to trigger the > ImageDocument path. Likewise loading the SVG directly over https didn't > reproduce the issue for me. ImageDocuments are solely for PDFs or bitmap images loaded as main resources. You'll never be able to trigger this for an SVG. I think the key to writing a test will be installing a user stylesheet (as indicated in the backtrace) that would trigger the insecure load, and then redirecting the test to an image-as-main-resource.
Removing CQ+ because I believe Joe was going to explore a layout test with the hint on how to do so.
Created attachment 255151 [details] [PATCH] Proposed Fix - With Test Thanks Brady! That worked perfectly. Now with a test!
Comment on attachment 255151 [details] [PATCH] Proposed Fix - With Test View in context: https://bugs.webkit.org/attachment.cgi?id=255151&action=review > LayoutTests/http/tests/security/mixedContent/insecure-content-in-image-document.html:17 > + testRunner.addUserStyleSheet(` > + @font-face { > + font-family: SpaceOnly; > + src: url('http://127.0.0.1:8080/security/resources/SpaceOnly.otf'); > + } > + * { > + font-family: SpaceOnly; > + } > + `, true); I believe all ports support ES6_TEMPLATE_LITERAL_SYNTAX, but it is behind a compile time flag... I could convert this to a regular string if necessary.
Comment on attachment 255151 [details] [PATCH] Proposed Fix - With Test Attachment 255151 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5638392255610880 New failing tests: http/tests/security/mixedContent/insecure-content-in-image-document.html
Created attachment 255154 [details] Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
> New failing tests: > http/tests/security/mixedContent/insecure-content-in-image-document.html I'm starting to think the bots just don't like me... I cannot reproduce this failure.
Created attachment 255207 [details] [PATCH] Proposed Fix - Doubled timeout in test Doubling the timeout from 100ms to 200ms to see if that could be related the issue seen on the bots. If it is timing, some event to know when the sub-window completed loading would be useful.
Comment on attachment 255207 [details] [PATCH] Proposed Fix - Doubled timeout in test Attachment 255207 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6418407721271296 New failing tests: http/tests/security/mixedContent/insecure-content-in-image-document.html
Created attachment 255210 [details] Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 255223 [details] [PATCH] Proposed Fix - Alternate Test Approach Trying a different test approach, no setTimeout.
Comment on attachment 255223 [details] [PATCH] Proposed Fix - Alternate Test Approach Attachment 255223 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5704227661807616 New failing tests: http/tests/security/mixedContent/insecure-content-in-image-document.html
Created attachment 255226 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
(In reply to comment #15) > Comment on attachment 255223 [details] > [PATCH] Proposed Fix - Alternate Test Approach > > Attachment 255223 [details] did not pass mac-ews (mac): > Output: http://webkit-queues.appspot.com/results/5704227661807616 > > New failing tests: > http/tests/security/mixedContent/insecure-content-in-image-document.html Ahh interesting. WK1 doesn't even try to load the PDF document? I can reproduce locally if this test through WK1... I'm thinking of just skipping on WK1, given that there is coverage on WK2.
(In reply to comment #17) > (In reply to comment #15) > > Comment on attachment 255223 [details] > > [PATCH] Proposed Fix - Alternate Test Approach > > > > Attachment 255223 [details] did not pass mac-ews (mac): > > Output: http://webkit-queues.appspot.com/results/5704227661807616 > > > > New failing tests: > > http/tests/security/mixedContent/insecure-content-in-image-document.html > > Ahh interesting. WK1 doesn't even try to load the PDF document? I can > reproduce locally if this test through WK1... I'm thinking of just skipping > on WK1, given that there is coverage on WK2. Ugh, now I'm able to reproduce if I just repeat the test for multiple iterations only the first succeed and the rest fail... Is there any reliable way to load a PDF in tests?
> Ugh, now I'm able to reproduce if I just repeat the test for multiple > iterations only the first succeed and the rest fail... Since even this test is unreliable when run multiple times in WK2, I'm going to drop the test... I've spent too much time trying to get this test to work given the simplicity of the fix, and that is has been reproduced and verified. =(
Created attachment 255241 [details] [PATCH] For Landing - Without Test
Comment on attachment 255241 [details] [PATCH] For Landing - Without Test Clearing flags on attachment: 255241 Committed r185781: <http://trac.webkit.org/changeset/185781>
All reviewed patches have been landed. Closing bug.