65015 2011-07-22 00:41:28 -0700 If Range::insertNode is passed an empty document fragment, it creates a broken DOM tree 2019-02-06 09:03:05 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 skylined darin ap cdumez darin eric rniwa oldest_to_newest 440870 0 101702 skylined 2011-07-22 00:41:28 -0700 Created attachment 101702 Repro Chromium: https://code.google.com/p/chromium/issues/detail?id=90147 Repro: <body onload="go()"></body> <script> function go() { var oBodyImported = document.importNode(document.body,false); document.body.appendChild(oBodyImported); var oRange1 = document.createRange(); var oRange2 = document.createRange(); document.body=document.activeElement; var oDocumentFragment = document.createDocumentFragment(); oRange1.setStartBefore(oBodyImported); oRange1.insertNode(oDocumentFragment); oRange2.setStart(document.body,0); oRange2.surroundContents(oBodyImported); ''+oRange1; } </script> The range oRange1 ends up containing a NULL node, among valid nodes. This is probably not supposed to happen, as some of the range's methods that access the nodes do not handle this correctly. In the repro, we end up executing this code: String Range::toString(ExceptionCode& ec) const { if (!m_start.container()) { ec = INVALID_STATE_ERR; return String(); } StringBuilder builder; Node* pastLast = pastLastNode(); for (Node* n = firstNode(); n != pastLast; n = n->traverseNextNode()) { if (n->nodeType() == Node::TEXT_NODE || n->nodeType() == Node::CDATA_SECTION_NODE) { String data = static_cast<CharacterData*>(n)->data(); int length = data.length(); int start = (n == m_start.container()) ? min(max(0, m_start.offset()), length) : 0; int end = (n == m_end.container()) ? min(max(start, m_end.offset()), length) : length; builder.append(data.characters() + start, end - start); } } return builder.toString(); } "n" goes through the nodes, which includes the NULL node. This causes a NULL ptr when the code tries to access n->nodeType(). The fact that the code loops through the nodes using n->traverseNextNode() signals that NULL nodes are probably not supposed to be part of a range. 441311 1 darin 2011-07-22 18:17:10 -0700 What is a NULL node? 441313 2 darin 2011-07-22 18:18:12 -0700 I think I understand. The issue is not with ranges. The issue is that insertNode does the wrong thing when passed an empty document fragment. 441317 3 darin 2011-07-22 18:26:47 -0700 I think all that’s needed to fix this bug is code to return from Range::insertNode function and do no further work if newNode->firstChild() is zero inside the DOCUMENT_FRAGMENT_NODE section of the code. 442174 4 skylined 2011-07-26 04:32:24 -0700 @Darin; yes, with "NULL node" I meant a NULL ptr where a ptr to a node is expected. You analysis sounds correct to me, but I don't know the code, so don't take my word for it. 442559 5 darin 2011-07-26 15:05:09 -0700 I think I’ll fix this myself, but if someone else wants to tackle it first, they are welcome to. 454350 6 104621 darin 2011-08-20 10:55:32 -0700 Created attachment 104621 Patch 454351 7 darin 2011-08-20 11:12:01 -0700 Committed r93481: <http://trac.webkit.org/changeset/93481> 1502901 8 lforschler 2019-02-06 09:03:05 -0800 Mass moving XML DOM bugs to the "DOM" Component. 101702 2011-07-22 00:41:28 -0700 2011-07-22 00:41:28 -0700 Repro repro.html text/html 555 skylined PGJvZHkgb25sb2FkPSJnbygpIj48L2JvZHk+CjxzY3JpcHQ+CiAgZnVuY3Rpb24gZ28oKSB7CiAg ICB2YXIgb0JvZHlJbXBvcnRlZCA9IGRvY3VtZW50LmltcG9ydE5vZGUoZG9jdW1lbnQuYm9keSxm YWxzZSk7CiAgICBkb2N1bWVudC5ib2R5LmFwcGVuZENoaWxkKG9Cb2R5SW1wb3J0ZWQpOwogICAg dmFyIG9SYW5nZTEgPSBkb2N1bWVudC5jcmVhdGVSYW5nZSgpOwogICAgdmFyIG9SYW5nZTIgPSBk b2N1bWVudC5jcmVhdGVSYW5nZSgpOwogICAgZG9jdW1lbnQuYm9keT1kb2N1bWVudC5hY3RpdmVF bGVtZW50OwogICAgdmFyIG9Eb2N1bWVudEZyYWdtZW50ID0gZG9jdW1lbnQuY3JlYXRlRG9jdW1l bnRGcmFnbWVudCgpOwogICAgb1JhbmdlMS5zZXRTdGFydEJlZm9yZShvQm9keUltcG9ydGVkKTsK ICAgIG9SYW5nZTEuaW5zZXJ0Tm9kZShvRG9jdW1lbnRGcmFnbWVudCk7CiAgICBvUmFuZ2UyLnNl dFN0YXJ0KGRvY3VtZW50LmJvZHksMCk7CiAgICBvUmFuZ2UyLnN1cnJvdW5kQ29udGVudHMob0Jv ZHlJbXBvcnRlZCk7CiAgICAnJytvUmFuZ2UxOwogIH0KPC9zY3JpcHQ+ 104621 2011-08-20 10:55:32 -0700 2011-08-20 11:05:03 -0700 Patch bug-65015-20110820105531.patch text/plain 5739 darin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDkzNDgwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDgtMjAgIERhcmluIEFk bGVyICA8ZGFyaW5AYXBwbGUuY29tPgorCisgICAgICAgIElmIFJhbmdlOjppbnNlcnROb2RlIGlz IHBhc3NlZCBhbiBlbXB0eSBkb2N1bWVudCBmcmFnbWVudCwgaXQgY3JlYXRlcyBhIGJyb2tlbiBE T00gdHJlZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NjUwMTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBU ZXN0OiBmYXN0L2RvbS9SYW5nZS9pbnNlcnROb2RlLWVtcHR5LWZyYWdtZW50LWNyYXNoLmh0bWwK KworICAgICAgICAqIGRvbS9SYW5nZS5jcHA6IChXZWJDb3JlOjpSYW5nZTo6aW5zZXJ0Tm9kZSk6 IERvbid0IGFkanVzdCB0aGUgcmFuZ2UgYWZ0ZXIgaW5zZXJ0aW9uCisgICAgICAgIGlmIHdlIGRp ZG4ndCBhZGQgYW55dGhpbmcuIE90aGVyd2lzZSB0aGUgY29kZSB3aWxsIHB1dCBhIHdyb25nICJj aGlsZCBiZWZvcmUiIHZhbHVlIGludG8KKyAgICAgICAgdGhlIHJhbmdlIGVuZCBib3VuZGFyeSBw b2ludC4KKwogMjAxMS0wOC0yMCAgS2VuIEJ1Y2hhbmFuICA8a2VucmJAY2hyb21pdW0ub3JnPgog CiAgICAgICAgIE9PQiBSZWFkIGluIFdlYkNvcmU6OlNWR0FuaW1hdGlvbkVsZW1lbnQKSW5kZXg6 IFNvdXJjZS9XZWJDb3JlL2RvbS9SYW5nZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNv cmUvZG9tL1JhbmdlLmNwcAkocmV2aXNpb24gOTM0ODApCisrKyBTb3VyY2UvV2ViQ29yZS9kb20v UmFuZ2UuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zLDcgKzMsNyBAQAogICogKEMpIDIwMDAgR3Vu bnN0ZWluIEx5ZSAoZ3VubnN0ZWluQG5ldGNvbS5ubykKICAqIChDKSAyMDAwIEZyZWRlcmlrIEhv bGxqZW4gKGZyZWRlcmlrLmhvbGxqZW5AaGlnLm5vKQogICogKEMpIDIwMDEgUGV0ZXIgS2VsbHkg KHBta0Bwb3N0LmNvbSkKLSAqIENvcHlyaWdodCAoQykgMjAwNCwgMjAwNSwgMjAwNiwgMjAwNywg MjAwOCwgMjAwOSBBcHBsZSBJbmMuIEFsbCByaWdodHMgcmVzZXJ2ZWQuCisgKiBDb3B5cmlnaHQg KEMpIDIwMDQsIDIwMDUsIDIwMDYsIDIwMDcsIDIwMDgsIDIwMDksIDIwMTAsIDIwMTEgQXBwbGUg SW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgogICoKICAqIFRoaXMgbGlicmFyeSBpcyBmcmVlIHNv ZnR3YXJlOyB5b3UgY2FuIHJlZGlzdHJpYnV0ZSBpdCBhbmQvb3IKICAqIG1vZGlmeSBpdCB1bmRl ciB0aGUgdGVybXMgb2YgdGhlIEdOVSBMaWJyYXJ5IEdlbmVyYWwgUHVibGljCkBAIC0xMDQ4LDcg KzEwNDgsNyBAQCB2b2lkIFJhbmdlOjppbnNlcnROb2RlKFBhc3NSZWZQdHI8Tm9kZT4gCiAKICAg ICAgICAgLy8gVGhpcyBzcGVjaWFsIGNhc2UgZG9lc24ndCBzZWVtIHRvIG1hdGNoIHRoZSBET00g c3BlY2lmaWNhdGlvbiwgYnV0IGl0J3MgY3VycmVudGx5IHJlcXVpcmVkCiAgICAgICAgIC8vIHRv IHBhc3MgQWNpZDMuIFdlIG1pZ2h0IGxhdGVyIGRlY2lkZSB0byByZW1vdmUgdGhpcy4KLSAgICAg ICAgaWYgKGNvbGxhcHNlZCkKKyAgICAgICAgaWYgKGNvbGxhcHNlZCAmJiBudW1OZXdDaGlsZHJl bikKICAgICAgICAgICAgIG1fZW5kLnNldChtX3N0YXJ0LmNvbnRhaW5lcigpLCBzdGFydE9mZnNl dCArIG51bU5ld0NoaWxkcmVuLCBsYXN0Q2hpbGQuZ2V0KCkpOwogICAgIH0KIH0KSW5kZXg6IExh eW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJ KHJldmlzaW9uIDkzNDgwKQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkp CkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMDgtMjAgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUu Y29tPgorCisgICAgICAgIElmIFJhbmdlOjppbnNlcnROb2RlIGlzIHBhc3NlZCBhbiBlbXB0eSBk b2N1bWVudCBmcmFnbWVudCwgaXQgY3JlYXRlcyBhIGJyb2tlbiBET00gdHJlZQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NjUwMTUKKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGZhc3QvZG9tL1JhbmdlL2lu c2VydE5vZGUtZW1wdHktZnJhZ21lbnQtY3Jhc2gtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAg ICAgKiBmYXN0L2RvbS9SYW5nZS9pbnNlcnROb2RlLWVtcHR5LWZyYWdtZW50LWNyYXNoLmh0bWw6 IEFkZGVkLgorCiAyMDExLTA4LTIwICBCYWxhenMgS2VsZW1lbiAgPGtiYWxhenNAd2Via2l0Lm9y Zz4KIAogICAgICAgICBldmVudFNlbmRlci5rZXlEb3duIGlzIHVuaW1wbGVtZW50ZWQgKFdLVFIp CkluZGV4OiBMYXlvdXRUZXN0cy9mYXN0L2RvbS9SYW5nZS9pbnNlcnROb2RlLWVtcHR5LWZyYWdt ZW50LWNyYXNoLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9mYXN0L2Rv bS9SYW5nZS9pbnNlcnROb2RlLWVtcHR5LWZyYWdtZW50LWNyYXNoLWV4cGVjdGVkLnR4dAkocmV2 aXNpb24gMCkKKysrIExheW91dFRlc3RzL2Zhc3QvZG9tL1JhbmdlL2luc2VydE5vZGUtZW1wdHkt ZnJhZ21lbnQtY3Jhc2gtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDE5IEBA CitUZXN0IFJhbmdlLmluc2VydE5vZGUoZnJhZ21lbnQpIHdoZW4gdGhlIGZyYWdtZW50IGlzIGVt cHR5IGFuZCB0aGUgcmFuZ2UgaXMgY29sbGFwc2VkCisKK09uIHN1Y2Nlc3MsIHlvdSB3aWxsIHNl ZSBhIHNlcmllcyBvZiAiUEFTUyIgbWVzc2FnZXMsIGZvbGxvd2VkIGJ5ICJURVNUIENPTVBMRVRF Ii4KKworCitQQVNTIHIudG9TdHJpbmcoKSBpcyAiIgorUEFTUyBwLmNoaWxkTm9kZXMubGVuZ3Ro IGlzIDIKK1BBU1MgcC5jaGlsZE5vZGVzWzBdIGlzIHQxCitQQVNTIHAuY2hpbGROb2Rlc1sxXSBp cyB0MgorUEFTUyByLmNvbGxhcHNlZCBpcyB0cnVlCitQQVNTIHIuc3RhcnRDb250YWluZXIgaXMg cAorUEFTUyByLnN0YXJ0T2Zmc2V0IGlzIDEKK1BBU1Mgci5lbmRDb250YWluZXIgaXMgcAorUEFT UyByLmVuZE9mZnNldCBpcyAxCitQQVNTIHIudG9TdHJpbmcoKSBpcyAiIgorUEFTUyBzdWNjZXNz ZnVsbHlQYXJzZWQgaXMgdHJ1ZQorCitURVNUIENPTVBMRVRFCisKClByb3BlcnR5IGNoYW5nZXMg b246IExheW91dFRlc3RzL2Zhc3QvZG9tL1JhbmdlL2luc2VydE5vZGUtZW1wdHktZnJhZ21lbnQt Y3Jhc2gtZXhwZWN0ZWQudHh0Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KQWRkZWQ6IHN2bjplb2wtc3R5bGUKICAgKyBu YXRpdmUKCkluZGV4OiBMYXlvdXRUZXN0cy9mYXN0L2RvbS9SYW5nZS9pbnNlcnROb2RlLWVtcHR5 LWZyYWdtZW50LWNyYXNoLmh0bWwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvZmFzdC9kb20v UmFuZ2UvaW5zZXJ0Tm9kZS1lbXB0eS1mcmFnbWVudC1jcmFzaC5odG1sCShyZXZpc2lvbiAwKQor KysgTGF5b3V0VGVzdHMvZmFzdC9kb20vUmFuZ2UvaW5zZXJ0Tm9kZS1lbXB0eS1mcmFnbWVudC1j cmFzaC5odG1sCShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDQzIEBACis8aHRtbD4KKyAgICA8aGVh ZD4KKyAgICAgICAgPGxpbmsgcmVsPSJzdHlsZXNoZWV0IiBocmVmPSIuLi8uLi9qcy9yZXNvdXJj ZXMvanMtdGVzdC1zdHlsZS5jc3MiPgorICAgICAgICA8c2NyaXB0IHNyYz0iLi4vLi4vanMvcmVz b3VyY2VzL2pzLXRlc3QtcHJlLmpzIj48L3NjcmlwdD4KKyAgICA8L2hlYWQ+CisgICAgPGJvZHk+ CisgICAgICAgIDxwIGlkPSJkZXNjcmlwdGlvbiI+PC9wPgorICAgICAgICA8ZGl2IGlkPSJjb25z b2xlIj48L2Rpdj4KKyAgICAgICAgPHNjcmlwdD4KKyAgICAgICAgICAgIGRlc2NyaXB0aW9uKCdU ZXN0IFJhbmdlLmluc2VydE5vZGUoZnJhZ21lbnQpIHdoZW4gdGhlIGZyYWdtZW50IGlzIGVtcHR5 IGFuZCB0aGUgcmFuZ2UgaXMgY29sbGFwc2VkJyk7CisKKyAgICAgICAgICAgIHZhciBwID0gZG9j dW1lbnQuY3JlYXRlRWxlbWVudCgncCcpOworICAgICAgICAgICAgdmFyIHQxID0gZG9jdW1lbnQu Y3JlYXRlVGV4dE5vZGUoJzEyMzQ1Jyk7CisgICAgICAgICAgICBwLmFwcGVuZENoaWxkKHQxKTsK KyAgICAgICAgICAgIHZhciB0MiA9IGRvY3VtZW50LmNyZWF0ZVRleHROb2RlKCdBQkNERScpOwor ICAgICAgICAgICAgcC5hcHBlbmRDaGlsZCh0Mik7CisgICAgICAgICAgICBkb2N1bWVudC5ib2R5 LmFwcGVuZENoaWxkKHApOworICAgICAgICAgICAgdmFyIHIgPSBkb2N1bWVudC5jcmVhdGVSYW5n ZSgpOworICAgICAgICAgICAgci5zZXRTdGFydChwLCAxKTsKKyAgICAgICAgICAgIHIuc2V0RW5k KHAsIDEpOworICAgICAgICAgICAgc2hvdWxkQmVFcXVhbFRvU3RyaW5nKCJyLnRvU3RyaW5nKCki LCAiIik7CisKKyAgICAgICAgICAgIHZhciBkZiA9IGRvY3VtZW50LmNyZWF0ZURvY3VtZW50RnJh Z21lbnQoKTsKKyAgICAgICAgICAgIHIuaW5zZXJ0Tm9kZShkZik7CisKKyAgICAgICAgICAgIHNo b3VsZEJlKCJwLmNoaWxkTm9kZXMubGVuZ3RoIiwgIjIiKTsKKyAgICAgICAgICAgIHNob3VsZEJl KCJwLmNoaWxkTm9kZXNbMF0iLCAidDEiKTsKKyAgICAgICAgICAgIHNob3VsZEJlKCJwLmNoaWxk Tm9kZXNbMV0iLCAidDIiKTsKKworICAgICAgICAgICAgc2hvdWxkQmVUcnVlKCJyLmNvbGxhcHNl ZCIpOworICAgICAgICAgICAgc2hvdWxkQmUoInIuc3RhcnRDb250YWluZXIiLCAicCIpOworICAg ICAgICAgICAgc2hvdWxkQmUoInIuc3RhcnRPZmZzZXQiLCAiMSIpOworICAgICAgICAgICAgc2hv dWxkQmUoInIuZW5kQ29udGFpbmVyIiwgInAiKTsKKyAgICAgICAgICAgIHNob3VsZEJlKCJyLmVu ZE9mZnNldCIsICIxIik7CisgICAgICAgICAgICBzaG91bGRCZUVxdWFsVG9TdHJpbmcoInIudG9T dHJpbmcoKSIsICIiKQorCisgICAgICAgICAgICBkb2N1bWVudC5ib2R5LnJlbW92ZUNoaWxkKHAp OworCisgICAgICAgICAgICB2YXIgc3VjY2Vzc2Z1bGx5UGFyc2VkID0gdHJ1ZTsKKyAgICAgICAg PC9zY3JpcHQ+CisgICAgICAgIDxzY3JpcHQgc3JjPSIuLi8uLi9qcy9yZXNvdXJjZXMvanMtdGVz dC1wb3N0LmpzIj48L3NjcmlwdD4KKyAgICA8L2JvZHk+Cis8L2h0bWw+CgpQcm9wZXJ0eSBjaGFu Z2VzIG9uOiBMYXlvdXRUZXN0cy9mYXN0L2RvbS9SYW5nZS9pbnNlcnROb2RlLWVtcHR5LWZyYWdt ZW50LWNyYXNoLmh0bWwKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fXwpBZGRlZDogc3ZuOm1pbWUtdHlwZQogICArIHRleHQv aHRtbApBZGRlZDogc3ZuOmVvbC1zdHlsZQogICArIG5hdGl2ZQoK