78700 2012-02-15 05:23:42 -0800 SVG TRef/Use NULL ptr 2012-05-16 00:43:35 -0700 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) PC Windows Vista RESOLVED FIXED P1 Normal --- 1 skylined zimmermann abarth cc-bugs cgarcia eric fmalita gustavo jamesr japhet levin+threading macpherson menard ojan rakuco rwlbuis webkit.review.bot zimmermann zoltan oldest_to_newest 557179 0 skylined 2012-02-15 05:23:42 -0800 http://code.google.com/p/chromium/issues/detail?id=114358 <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> <g id="g"> <animate id="animate"> </animate> <tref xlink:href="#animate"> </tref> </g> <use xlink:href="#g"> </use> </svg> src\third_party\webkit\source\webcore\svg\svgtrefelement.cpp void SVGTRefElement::buildPendingResource() { <<<snip>>> m_eventListener = SubtreeModificationEventListener::create(this, id); ASSERT(target->parentNode()); target->parentNode()->addEventListener(eventNames().DOMSubtreeModifiedEvent, m_eventListener.get(), false); } src\third_party\webkit\source\webcore\dom\node.cpp bool Node::addEventListener(const AtomicString& eventType, PassRefPtr<EventListener> listener, bool useCapture) { <<<snip>>> for (HashSet<SVGElementInstance*>::const_iterator it = instances.begin(); it != end; ++it) { ASSERT((*it)->shadowTreeElement()); ASSERT((*it)->correspondingElement() == this); RefPtr<EventListener> listenerForCurrentShadowTreeElement = listenerForShadowTree; bool result = tryAddEventListener((*it)->shadowTreeElement(), eventType, listenerForCurrentShadowTreeElement.release(), useCapture); <<<snip>>> (*it) points to an SVGUseElement which doesn't have a shadowTreeElement, causing the NULL ptr. 557184 1 zimmermann 2012-02-15 05:31:36 -0800 (In reply to comment #0) > <<<snip>>> > m_eventListener = SubtreeModificationEventListener::create(this, id); > ASSERT(target->parentNode()); > target->parentNode()->addEventListener(eventNames().DOMSubtreeModifiedEvent, m_eventListener.get(), false); > (*it) points to an SVGUseElement which doesn't have a shadowTreeElement, causing the NULL ptr. Oh dear, apparently <tref> doesn't even check if the target is valid, just attaching its event listener. This is a bad idea. CC'ing Rob, who wrote the current <tref> implementation. Reminds me of a similar <use> bug which is in the process of being fixed: white-list allowed targets, instead of black-listing disallowed ones. 557509 2 fmalita 2012-02-15 13:33:53 -0800 FWIW my patch for http://www.webkit.org/b/74858 also fixes this particular crash (as the event listeners are moved onto the target instead of its parent). Sounds like we still need to add target validation though. 557629 3 zimmermann 2012-02-15 15:22:16 -0800 (In reply to comment #2) > FWIW my patch for http://www.webkit.org/b/74858 also fixes this particular crash (as the event listeners are moved onto the target instead of its parent). Sounds like we still need to add target validation though. Just looked at this again, Rob did it just correct according to SVG 1.1 2nd edition: "xlink:href of <tref>: An IRI reference to an element whose character data content shall be used as character data for this ‘tref’ element." There's no white or black-listing desired, any element should work. So probably this is just a dup of your bug. 568612 4 129693 zimmermann 2012-03-01 06:11:30 -0800 Created attachment 129693 Patch 568614 5 webkit.review.bot 2012-03-01 06:14:14 -0800 Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API 568615 6 webkit.review.bot 2012-03-01 06:15:27 -0800 Attachment 129693 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1 WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitDefines.h" Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:71: Use 0 instead of NULL. [readability/null] [5] Total errors found: 1 in 176 files If any of these errors are false positives, please file a bug against check-webkit-style. 568623 7 zimmermann 2012-03-01 06:26:27 -0800 (In reply to comment #6) > If any of these errors are false positives, please file a bug against check-webkit-style. Not mine. Bug was already filed. 579195 8 129693 morrita 2012-03-15 01:26:36 -0700 Comment on attachment 129693 Patch Adding test doesn't hurt. Could you update the ChangeLog to put diff to the head? 625091 9 zimmermann 2012-05-16 00:43:35 -0700 Committed r117229: <http://trac.webkit.org/changeset/117229> 129693 2012-03-01 06:11:30 -0800 2012-03-15 01:26:36 -0700 Patch bug-78700-20120301151128.patch text/plain 1776 zimmermann U3VidmVyc2lvbiBSZXZpc2lvbjogMTA5MzQyCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNjUzNjMzYWJmZTZlNmM5YTQyY2Zk MDc1Y2Q0NWY2YzdjMWNlNmJiZC4uYjA1YTlhOWU3MTM4ODYwNzU4OTg5N2Y1MThhZjkzMjg5YjYw M2MyMiAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE1IEBACiAyMDEyLTAzLTAxICBOaWtvbGFzIFppbW1lcm1h bm4gIDxuemltbWVybWFubkByaW0uY29tPgogCisgICAgICAgIFNWRyBUUmVmL1VzZSBOVUxMIHB0 cgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Nzg3MDAK KworICAgICAgICBBZGQgdGVzdGNhc2UgY292ZXJpbmcgYSBwcm9ibGVtIGZpeGVkIGluIHRydW5r LgorCisgICAgICAgICogc3ZnL2N1c3RvbS9idWc3ODcwMC1leHBlY3RlZC50eHQ6IEFkZGVkLgor ICAgICAgICAqIHN2Zy9jdXN0b20vYnVnNzg3MDAuc3ZnOiBBZGRlZC4KKworMjAxMi0wMy0wMSAg Tmlrb2xhcyBaaW1tZXJtYW5uICA8bnppbW1lcm1hbm5AcmltLmNvbT4KKwogICAgICAgICBJbnRy b2R1Y2UgU01JTCBvdmVycmlkZVN0eWxlLCB0byBtYWtlIFNWRyBzdG9wIG11dGF0aW5nIENTUyBz dHlsZXMgZGlyZWN0bHkKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTc5NzkwCiAKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL3N2Zy9jdXN0b20vYnVnNzg3 MDAtZXhwZWN0ZWQudHh0IGIvTGF5b3V0VGVzdHMvc3ZnL2N1c3RvbS9idWc3ODcwMC1leHBlY3Rl ZC50eHQKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMC4uNDE3MGVjOWE0MDg0NGY4MmZkN2IxYzEyZmM5NWE3YzE3MWEwMTk1 MwotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL3N2Zy9jdXN0b20vYnVnNzg3MDAtZXhw ZWN0ZWQudHh0CkBAIC0wLDAgKzEsMiBAQAorVGhpcyB0ZXN0IHBhc3NlcyBpZiB0aGVyZSBpcyBu byBjcmFzaAorCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9zdmcvY3VzdG9tL2J1Zzc4NzAwLnN2 ZyBiL0xheW91dFRlc3RzL3N2Zy9jdXN0b20vYnVnNzg3MDAuc3ZnCm5ldyBmaWxlIG1vZGUgMTAw NjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLmNjZTdh ZGEwMDFhNjRkZTQ5NGFjZTYwMzk0NDdiYWU4MzhmNjczMDAKLS0tIC9kZXYvbnVsbAorKysgYi9M YXlvdXRUZXN0cy9zdmcvY3VzdG9tL2J1Zzc4NzAwLnN2ZwpAQCAtMCwwICsxLDE1IEBACis8c3Zn IHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93 d3cudzMub3JnLzE5OTkveGxpbmsiPgorICA8ZGVmcz48dGV4dD5UaGlzIHRlc3QgcGFzc2VzIGlm IHRoZXJlIGlzIG5vIGNyYXNoPC90ZXh0PjwvZGVmcz4KKyAgPHNjcmlwdD4KKyAgICBpZiAod2lu ZG93LmxheW91dFRlc3RDb250cm9sbGVyKQorICAgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIuZHVt cEFzVGV4dCgpOworICA8L3NjcmlwdD4KKyAgPGcgaWQ9ImciPgorICAgIDxhbmltYXRlIGlkPSJh bmltYXRlIj4KKyAgICA8L2FuaW1hdGU+CisgICAgPHRyZWYgeGxpbms6aHJlZj0iI2FuaW1hdGUi PgorICAgIDwvdHJlZj4KKyAgPC9nPgorICA8dXNlIHhsaW5rOmhyZWY9IiNnIj4KKyAgPC91c2U+ Cis8L3N2Zz4K