[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.16-1409-g5afdf4d
ap at apple.com
ap at apple.com
Thu Dec 3 13:19:45 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit 27c1c8bc00d8204c9ce10f34207593c9e28cbadf
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