[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

ap at apple.com ap at apple.com
Wed Apr 7 23:07:14 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 86a47f4f041e6542db65693b9a573e45ab920239
Author: ap at apple.com <ap at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Mon Oct 26 23:11:30 2009 +0000

            Reviewed by Darin Adler.
    
            https://bugs.webkit.org/show_bug.cgi?id=30049
            <rdar://problem/7286002> Manipulating DOM from a script while parsing XHTML can cause a crash
    
            Tests: fast/parser/remove-current-node-parent-x-2.xhtml
                   fast/parser/remove-current-node-parent-x.xhtml
    
            * dom/XMLTokenizer.h: Store the whole stack of parent nodes - element.parentNode() is
            unreliable after DOM manipulation.
    
            * dom/XMLTokenizer.cpp:
            (WebCore::XMLTokenizer::pushCurrentNode): Push the new node onto stack.
            (WebCore::XMLTokenizer::popCurrentNode): This is now called instead of setCurrentNode when
            exiting a node.
            (WebCore::XMLTokenizer::clearCurrentNodeStack): We're aborting; or just done parsing. This
            replaces setCurrentNode(0).
            (WebCore::XMLTokenizer::enterText): Call pushCurrentNode().
            (WebCore::XMLTokenizer::exitText): Call popCurrentNode(), removing a long-standing FIXME
            (not sure if it was ever practical though - how can a parent become null while adding text?)
    
            * dom/XMLTokenizerLibxml2.cpp:
            (WebCore::XMLTokenizer::~XMLTokenizer): Call clearCurrentNodeStack().
            (WebCore::XMLTokenizer::startElementNs): Call pushCurrentNode().
            (WebCore::XMLTokenizer::endElementNs): Call popCurrentNode() to safely get to a parent. Also
            added a check fo script element still being in document - Firefox parses those that aren't,
            but doesn't execute them.
    
            * dom/XMLTokenizerQt.cpp:
            (WebCore::XMLTokenizer::~XMLTokenizer):
            (WebCore::XMLTokenizer::parseStartElement):
            (WebCore::XMLTokenizer::parseEndElement):
            Match libxml2 version changes.
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50110 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d5b166e..55379e1 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2009-10-26  Alexey Proskuryakov  <ap at apple.com>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=30049
+        <rdar://problem/7286002> Manipulating DOM from a script while parsing XHTML can cause a crash
+
+        * fast/parser/remove-current-node-parent-x-2-expected.txt: Added.
+        * fast/parser/remove-current-node-parent-x-2.xhtml: Added.
+        * fast/parser/remove-current-node-parent-x-expected.txt: Added.
+        * fast/parser/remove-current-node-parent-x.xhtml: Added.
+
 2009-10-26  Dan Bernstein  <mitz at apple.com>
 
         Reviewed by Beth Dakin.
diff --git a/LayoutTests/fast/parser/remove-current-node-parent-x-2-expected.txt b/LayoutTests/fast/parser/remove-current-node-parent-x-2-expected.txt
new file mode 100644
index 0000000..8b4e4cf
--- /dev/null
+++ b/LayoutTests/fast/parser/remove-current-node-parent-x-2-expected.txt
@@ -0,0 +1,5 @@
+Test for bug 30049: Manipulating DOM from a script while parsing XHTML can cause a crash.
+
+Should say PASS:
+
+PASS
diff --git a/LayoutTests/fast/parser/remove-current-node-parent-x-2.xhtml b/LayoutTests/fast/parser/remove-current-node-parent-x-2.xhtml
new file mode 100644
index 0000000..cef0c29
--- /dev/null
+++ b/LayoutTests/fast/parser/remove-current-node-parent-x-2.xhtml
@@ -0,0 +1,40 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+</head>
+<body>
+<div>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+    }
+
+    var div = document.getElementsByTagName('div')[0];
+    div.parentNode.removeChild(div);
+
+    function msg(text)
+    {
+        var n = document.createElement("p");
+        n.appendChild(document.createTextNode(text));
+        document.body.appendChild(n);
+    }
+
+    function verify()
+    {
+        msg("Test for bug 30049: Manipulating DOM from a script while parsing XHTML can cause a crash.");
+        msg("Should say PASS:");
+        // Even though a subtree is removed, parsing continues.
+        msg(div.getElementsByTagName("foo").length == 1 ? "PASS" : "FAIL");
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+
+    setTimeout(verify, 100);
+</script>
+<foo/>
+<script>
+    alert("FAIL"); // Firefox compatibility.
+</script>
+</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/parser/remove-current-node-parent-x-expected.txt b/LayoutTests/fast/parser/remove-current-node-parent-x-expected.txt
new file mode 100644
index 0000000..8b4e4cf
--- /dev/null
+++ b/LayoutTests/fast/parser/remove-current-node-parent-x-expected.txt
@@ -0,0 +1,5 @@
+Test for bug 30049: Manipulating DOM from a script while parsing XHTML can cause a crash.
+
+Should say PASS:
+
+PASS
diff --git a/LayoutTests/fast/parser/remove-current-node-parent-x.xhtml b/LayoutTests/fast/parser/remove-current-node-parent-x.xhtml
new file mode 100644
index 0000000..6bf8328
--- /dev/null
+++ b/LayoutTests/fast/parser/remove-current-node-parent-x.xhtml
@@ -0,0 +1,39 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+</head>
+<body>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+    }
+
+    var body = document.getElementsByTagName('body')[0];
+    var newBody = document.createElement("body");
+    body.parentNode.replaceChild(newBody, body);
+
+    function msg(text)
+    {
+        var n = document.createElement("p");
+        n.appendChild(document.createTextNode(text));
+        document.body.appendChild(n);
+    }
+
+    function verify()
+    {
+        msg("Test for bug 30049: Manipulating DOM from a script while parsing XHTML can cause a crash.");
+        msg("Should say PASS:");
+        // Even though a subtree is removed, parsing continues.
+        msg(body.getElementsByTagName("foo").length == 1 ? "PASS" : "FAIL");
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+
+    setTimeout(verify, 100);
+</script>
+<script>
+    alert("FAIL"); // Firefox compatibility.
+</script>
+<foo/>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index da50af4..16583de 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2009-10-26  Alexey Proskuryakov  <ap at apple.com>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=30049
+        <rdar://problem/7286002> Manipulating DOM from a script while parsing XHTML can cause a crash
+
+        Tests: fast/parser/remove-current-node-parent-x-2.xhtml
+               fast/parser/remove-current-node-parent-x.xhtml
+
+        * dom/XMLTokenizer.h: Store the whole stack of parent nodes - element.parentNode() is
+        unreliable after DOM manipulation.
+
+        * dom/XMLTokenizer.cpp:
+        (WebCore::XMLTokenizer::pushCurrentNode): Push the new node onto stack.
+        (WebCore::XMLTokenizer::popCurrentNode): This is now called instead of setCurrentNode when
+        exiting a node.
+        (WebCore::XMLTokenizer::clearCurrentNodeStack): We're aborting; or just done parsing. This
+        replaces setCurrentNode(0).
+        (WebCore::XMLTokenizer::enterText): Call pushCurrentNode().
+        (WebCore::XMLTokenizer::exitText): Call popCurrentNode(), removing a long-standing FIXME
+        (not sure if it was ever practical though - how can a parent become null while adding text?)
+
+        * dom/XMLTokenizerLibxml2.cpp:
+        (WebCore::XMLTokenizer::~XMLTokenizer): Call clearCurrentNodeStack().
+        (WebCore::XMLTokenizer::startElementNs): Call pushCurrentNode().
+        (WebCore::XMLTokenizer::endElementNs): Call popCurrentNode() to safely get to a parent. Also
+        added a check fo script element still being in document - Firefox parses those that aren't,
+        but doesn't execute them.
+
+        * dom/XMLTokenizerQt.cpp:
+        (WebCore::XMLTokenizer::~XMLTokenizer):
+        (WebCore::XMLTokenizer::parseStartElement):
+        (WebCore::XMLTokenizer::parseEndElement):
+        Match libxml2 version changes.
+
 2009-10-26  Dan Bernstein  <mitz at apple.com>
 
         Reviewed by Beth Dakin.
diff --git a/WebCore/dom/XMLTokenizer.cpp b/WebCore/dom/XMLTokenizer.cpp
index 1915bb6..30d39e0 100644
--- a/WebCore/dom/XMLTokenizer.cpp
+++ b/WebCore/dom/XMLTokenizer.cpp
@@ -79,13 +79,41 @@ bool XMLTokenizer::isWMLDocument() const
 }
 #endif
 
-void XMLTokenizer::setCurrentNode(Node* n)
+void XMLTokenizer::pushCurrentNode(Node* n)
 {
-    if (n && n != m_doc)
+    ASSERT(n);
+    ASSERT(m_currentNode);
+    if (n != m_doc)
         n->ref();
+    m_currentNodeStack.append(m_currentNode);
+    m_currentNode = n;
+}
+
+void XMLTokenizer::popCurrentNode()
+{
+    ASSERT(m_currentNode);
+    ASSERT(m_currentNodeStack.size());
+
+    if (m_currentNode != m_doc)
+        m_currentNode->deref();
+
+    m_currentNode = m_currentNodeStack.last();
+    m_currentNodeStack.removeLast();
+}
+
+void XMLTokenizer::clearCurrentNodeStack()
+{
     if (m_currentNode && m_currentNode != m_doc)
         m_currentNode->deref();
-    m_currentNode = n;
+    m_currentNode = 0;
+
+    if (m_currentNodeStack.size()) { // Aborted parsing.
+        for (size_t i = m_currentNodeStack.size() - 1; i != 0; --i)
+            m_currentNodeStack[i]->deref();
+        if (m_currentNodeStack[0] && m_currentNodeStack[0] != m_doc)
+            m_currentNodeStack[0]->deref();
+        m_currentNodeStack.clear();
+    }
 }
 
 void XMLTokenizer::write(const SegmentedString& s, bool /*appendData*/)
@@ -141,7 +169,7 @@ bool XMLTokenizer::enterText()
     RefPtr<Node> newNode = Text::create(m_doc, "");
     if (!m_currentNode->addChild(newNode.get()))
         return false;
-    setCurrentNode(newNode.get());
+    pushCurrentNode(newNode.get());
     return true;
 }
 
@@ -171,10 +199,7 @@ void XMLTokenizer::exitText()
     if (m_view && m_currentNode && !m_currentNode->attached())
         m_currentNode->attach();
 
-    // FIXME: What's the right thing to do if the parent is really 0?
-    // Just leaving the current node set to the text node doesn't make much sense.
-    if (Node* par = m_currentNode->parentNode())
-        setCurrentNode(par);
+    popCurrentNode();
 }
 
 void XMLTokenizer::end()
@@ -188,7 +213,7 @@ void XMLTokenizer::end()
         m_doc->updateStyleSelector();
     }
     
-    setCurrentNode(0);
+    clearCurrentNodeStack();
     if (!m_parsingFragment)
         m_doc->finishedParsing();    
 }
diff --git a/WebCore/dom/XMLTokenizer.h b/WebCore/dom/XMLTokenizer.h
index 147ba46..e1ee09f 100644
--- a/WebCore/dom/XMLTokenizer.h
+++ b/WebCore/dom/XMLTokenizer.h
@@ -124,7 +124,10 @@ public:
         friend bool parseXMLDocumentFragment(const String& chunk, DocumentFragment* fragment, Element* parent);
 
         void initializeParserContext(const char* chunk = 0);
-        void setCurrentNode(Node*);
+
+        void pushCurrentNode(Node*);
+        void popCurrentNode();
+        void clearCurrentNodeStack();
 
         void insertErrorMessageBlock();
 
@@ -148,6 +151,7 @@ public:
         Vector<xmlChar> m_bufferedText;
 #endif
         Node* m_currentNode;
+        Vector<Node*> m_currentNodeStack;
 
         bool m_sawError;
         bool m_sawXSLTransform;
diff --git a/WebCore/dom/XMLTokenizerLibxml2.cpp b/WebCore/dom/XMLTokenizerLibxml2.cpp
index 34b56ee..9aa0961 100644
--- a/WebCore/dom/XMLTokenizerLibxml2.cpp
+++ b/WebCore/dom/XMLTokenizerLibxml2.cpp
@@ -611,7 +611,7 @@ XMLTokenizer::XMLTokenizer(DocumentFragment* fragment, Element* parentElement)
 
 XMLTokenizer::~XMLTokenizer()
 {
-    setCurrentNode(0);
+    clearCurrentNodeStack();
     if (m_parsingFragment && m_doc)
         m_doc->deref();
     if (m_pendingScript)
@@ -798,7 +798,7 @@ void XMLTokenizer::startElementNs(const xmlChar* xmlLocalName, const xmlChar* xm
         return;
     }
 
-    setCurrentNode(newElement.get());
+    pushCurrentNode(newElement.get());
     if (m_view && !newElement->attached())
         newElement->attach();
 
@@ -819,22 +819,29 @@ void XMLTokenizer::endElementNs()
     exitText();
 
     Node* n = m_currentNode;
-    RefPtr<Node> parent = n->parentNode();
     n->finishParsingChildren();
 
     if (!n->isElementNode() || !m_view) {
-        setCurrentNode(parent.get());
+        popCurrentNode();
         return;
     }
 
     Element* element = static_cast<Element*>(n);
+
+    // The element's parent may have already been removed from document.
+    // Parsing continues in this case, but scripts aren't executed.
+    if (!element->inDocument()) {
+        popCurrentNode();
+        return;
+    }
+
     ScriptElement* scriptElement = toScriptElement(element);
     if (!scriptElement) {
-        setCurrentNode(parent.get());
+        popCurrentNode();
         return;
     }
 
-    // don't load external scripts for standalone documents (for now)
+    // Don't load external scripts for standalone documents (for now).
     ASSERT(!m_pendingScript);
     m_requestingScript = true;
 
@@ -862,7 +869,7 @@ void XMLTokenizer::endElementNs()
             m_view->frame()->script()->executeScript(ScriptSourceCode(scriptElement->scriptContent(), m_doc->url(), m_scriptStartLine));
     }
     m_requestingScript = false;
-    setCurrentNode(parent.get());
+    popCurrentNode();
 }
 
 void XMLTokenizer::characters(const xmlChar* s, int len)
diff --git a/WebCore/dom/XMLTokenizerQt.cpp b/WebCore/dom/XMLTokenizerQt.cpp
index 17f7968..9758be7 100644
--- a/WebCore/dom/XMLTokenizerQt.cpp
+++ b/WebCore/dom/XMLTokenizerQt.cpp
@@ -185,7 +185,7 @@ XMLTokenizer::XMLTokenizer(DocumentFragment* fragment, Element* parentElement)
 
 XMLTokenizer::~XMLTokenizer()
 {
-    setCurrentNode(0);
+    clearCurrentNodeStack();
     if (m_parsingFragment && m_doc)
         m_doc->deref();
     if (m_pendingScript)
@@ -566,7 +566,7 @@ void XMLTokenizer::parseStartElement()
         return;
     }
 
-    setCurrentNode(newElement.get());
+    pushCurrentNode(newElement.get());
     if (m_view && !newElement->attached())
         newElement->attach();
 
@@ -579,18 +579,25 @@ void XMLTokenizer::parseEndElement()
     exitText();
 
     Node* n = m_currentNode;
-    RefPtr<Node> parent = n->parentNode();
     n->finishParsingChildren();
 
     if (!n->isElementNode() || !m_view) {
-        setCurrentNode(parent.get());
+        popCurrentNode();
         return;
     }
 
     Element* element = static_cast<Element*>(n);
+
+    // The element's parent may have already been removed from document.
+    // Parsing continues in this case, but scripts aren't executed.
+    if (!element->inDocument()) {
+        popCurrentNode();
+        return;
+    }
+
     ScriptElement* scriptElement = toScriptElement(element);
     if (!scriptElement) {
-        setCurrentNode(parent.get());
+        popCurrentNode();
         return;
     }
 
@@ -622,7 +629,7 @@ void XMLTokenizer::parseEndElement()
             m_view->frame()->script()->executeScript(ScriptSourceCode(scriptElement->scriptContent(), m_doc->url(), m_scriptStartLine));
     }
     m_requestingScript = false;
-    setCurrentNode(parent.get());
+    popCurrentNode();
 }
 
 void XMLTokenizer::parseCharacters()

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list