Reviewed by Maciej.
- fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4367
Crash when executing setTimeout / Date / document.write Javascript (bugtraq)
Test cases added:
* layout-tests/fast/dom/document-write-infinite-recursion.html: Added.
* layout-tests/fast/dom/document-write-infinite-recursion-expected.txt: Added.
* khtml/xml/dom_docimpl.cpp:
(DocumentImpl::implicitClose): Simplify a bit to make more readable. Remove the
code to delete the tokenizer an extra time -- that can end up deleting the newly
created tokenizer that's still needed for the newly opened page.
(DocumentImpl::write): Added an assertion to catch the badness that caused
infinite recursion so it's easier to recognize next time.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@10513 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/fast/dom/document-write-infinite-recursion-expected.txt b/LayoutTests/fast/dom/document-write-infinite-recursion-expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/LayoutTests/fast/dom/document-write-infinite-recursion-expected.txt
diff --git a/LayoutTests/fast/dom/document-write-infinite-recursion.html b/LayoutTests/fast/dom/document-write-infinite-recursion.html
new file mode 100644
index 0000000..3039f91
--- /dev/null
+++ b/LayoutTests/fast/dom/document-write-infinite-recursion.html
@@ -0,0 +1,27 @@
+<script>
+
+function a()
+{
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+ setTimeout("b()", 0);
+ document.write("a");
+}
+
+function b()
+{
+ setTimeout("c()", 0);
+ document.write("b");
+}
+
+function c()
+{
+ document.write("If you can see this, then the browser survived; older versions of WebKit would have crashed due to infinite recursion.");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+</script>
+<body onload="a()">
diff --git a/WebCore/ChangeLog-2005-12-19 b/WebCore/ChangeLog-2005-12-19
index 9f036249..a56756e 100644
--- a/WebCore/ChangeLog-2005-12-19
+++ b/WebCore/ChangeLog-2005-12-19
@@ -1,5 +1,23 @@
2005-09-10 Darin Adler <darin@apple.com>
+ Reviewed by Maciej.
+
+ - fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4367
+ Crash when executing setTimeout / Date / document.write Javascript (bugtraq)
+
+ Test cases added:
+ * layout-tests/fast/dom/document-write-infinite-recursion.html: Added.
+ * layout-tests/fast/dom/document-write-infinite-recursion-expected.txt: Added.
+
+ * khtml/xml/dom_docimpl.cpp:
+ (DocumentImpl::implicitClose): Simplify a bit to make more readable. Remove the
+ code to delete the tokenizer an extra time -- that can end up deleting the newly
+ created tokenizer that's still needed for the newly opened page.
+ (DocumentImpl::write): Added an assertion to catch the badness that caused
+ infinite recursion so it's easier to recognize next time.
+
+2005-09-10 Darin Adler <darin@apple.com>
+
- add expected success result for newly-enabled test
* layout-tests/dom/html/level2/html/HTMLIFrameElement11-expected.txt: Added.
@@ -794,22 +812,6 @@
* layout-tests/dom/html/level2/core/createDocument08-expected.txt: Updated to expect success.
* layout-tests/dom/html/level2/core/createDocumentType04-expected.txt: Ditto.
-2005-09-05 Darin Adler <darin@apple.com>
-
- Reviewed by John Sullivan.
-
- - fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4493
- add qualifiedName checking for empty string
-
- * khtml/xml/dom_docimpl.cpp:
- (qualifiedNameIsValid): Added.
- (qualifiedNameIsMalformed): Added.
- (DOMImplementationImpl::createDocumentType): Added checks and exceptions using above functions.
- (DOMImplementationImpl::createDocument): Ditto.
-
- * layout-tests/dom/html/level2/core/createDocument08-expected.txt: Updated to expect success.
- * layout-tests/dom/html/level2/core/createDocumentType04-expected.txt: Ditto.
-
2005-09-05 John Sullivan <sullivan@apple.com>
Reviewed by Dave Hyatt.
diff --git a/WebCore/khtml/xml/dom_docimpl.cpp b/WebCore/khtml/xml/dom_docimpl.cpp
index adfc06e..f00d7d6 100644
--- a/WebCore/khtml/xml/dom_docimpl.cpp
+++ b/WebCore/khtml/xml/dom_docimpl.cpp
@@ -1322,97 +1322,75 @@
return;
}
- // First fire the onload.
-
bool wasLocationChangePending = part() && part()->isScheduledLocationChangePending();
bool doload = !parsing() && m_tokenizer && !m_processingLoadEvent && !wasLocationChangePending;
- if (doload) {
- m_processingLoadEvent = true;
+ if (!doload)
+ return;
- // We have to clear the tokenizer, in case someone document.write()s from the
- // onLoad event handler, as in Radar 3206524
- delete m_tokenizer;
- m_tokenizer = 0;
+ m_processingLoadEvent = true;
- // Create a body element if we don't already have one.
- // In the case of Radar 3758785, the window.onload was set in some javascript, but never fired because there was no body.
- // This behavior now matches Firefox and IE.
- HTMLElementImpl *body = this->body();
- if (!body && isHTMLDocument()) {
- NodeImpl *de = documentElement();
- if (de) {
- body = new HTMLBodyElementImpl(docPtr());
- int exceptionCode = 0;
- de->appendChild(body, exceptionCode);
- if (exceptionCode != 0)
- body = 0;
- }
+ // We have to clear the tokenizer, in case someone document.write()s from the
+ // onLoad event handler, as in Radar 3206524.
+ delete m_tokenizer;
+ m_tokenizer = 0;
+
+ // Create a body element if we don't already have one.
+ // In the case of Radar 3758785, the window.onload was set in some javascript, but never fired because there was no body.
+ // This behavior now matches Firefox and IE.
+ HTMLElementImpl *body = this->body();
+ if (!body && isHTMLDocument()) {
+ NodeImpl *de = documentElement();
+ if (de) {
+ body = new HTMLBodyElementImpl(docPtr());
+ int exceptionCode = 0;
+ de->appendChild(body, exceptionCode);
+ if (exceptionCode != 0)
+ body = 0;
}
-
- if (body) {
- dispatchImageLoadEventsNow();
- body->dispatchWindowEvent(loadEvent, false, false);
-#if APPLE_CHANGES
- if (KHTMLPart *p = part())
- KWQ(p)->handledOnloadEvents();
-#endif
-
-#ifdef INSTRUMENT_LAYOUT_SCHEDULING
- if (!ownerElement())
- printf("onload fired at %d\n", elapsedTime());
-#endif
- }
-
- m_processingLoadEvent = false;
}
-
+
+ if (body) {
+ dispatchImageLoadEventsNow();
+ body->dispatchWindowEvent(loadEvent, false, false);
+ if (KHTMLPart *p = part())
+ KWQ(p)->handledOnloadEvents();
+#ifdef INSTRUMENT_LAYOUT_SCHEDULING
+ if (!ownerElement())
+ printf("onload fired at %d\n", elapsedTime());
+#endif
+ }
+
+ m_processingLoadEvent = false;
+
// Make sure both the initial layout and reflow happen after the onload
// fires. This will improve onload scores, and other browsers do it.
// If they wanna cheat, we can too. -dwh
-
- bool isLocationChangePending = part() && part()->isScheduledLocationChangePending();
-
- if (doload && isLocationChangePending && m_startTime.elapsed() < cLayoutScheduleThreshold) {
- // Just bail out. Before or during the onload we were shifted to another page.
- // The old i-Bench suite does this. When this happens don't bother painting or laying out.
- delete m_tokenizer;
- m_tokenizer = 0;
- view()->unscheduleRelayout();
- return;
- }
-
- if (doload) {
- // on an explicit document.close(), the tokenizer might still be waiting on scripts,
- // and in that case we don't want to destroy it because that will prevent the
- // scripts from getting processed.
- // FIXME: this check may no longer be necessary, since now it should be impossible
- // for parsing to be false while stil waiting for scripts
- if (m_tokenizer && !m_tokenizer->isWaitingForScripts()) {
- delete m_tokenizer;
- m_tokenizer = 0;
- }
- if (m_view)
- m_view->part()->checkEmitLoadEvent();
+ if (part() && part()->isScheduledLocationChangePending() && m_startTime.elapsed() < cLayoutScheduleThreshold) {
+ // Just bail out. Before or during the onload we were shifted to another page.
+ // The old i-Bench suite does this. When this happens don't bother painting or laying out.
+ view()->unscheduleRelayout();
+ return;
}
+ if (part())
+ part()->checkEmitLoadEvent();
+
// Now do our painting/layout, but only if we aren't in a subframe or if we're in a subframe
// that has been sized already. Otherwise, our view size would be incorrect, so doing any
// layout/painting now would be pointless.
- if (doload) {
- if (!ownerElement() || (ownerElement()->renderer() && !ownerElement()->renderer()->needsLayout())) {
- updateRendering();
-
- // Always do a layout after loading if needed.
- if (view() && renderer() && (!renderer()->firstChild() || renderer()->needsLayout()))
- view()->layout();
- }
-#if APPLE_CHANGES
- if (renderer() && KWQAccObjectCache::accessibilityEnabled())
- getAccObjectCache()->postNotification(renderer(), "AXLoadComplete");
-#endif
+ if (!ownerElement() || (ownerElement()->renderer() && !ownerElement()->renderer()->needsLayout())) {
+ updateRendering();
+
+ // Always do a layout after loading if needed.
+ if (view() && renderer() && (!renderer()->firstChild() || renderer()->needsLayout()))
+ view()->layout();
}
+#if APPLE_CHANGES
+ if (renderer() && KWQAccObjectCache::accessibilityEnabled())
+ getAccObjectCache()->postNotification(renderer(), "AXLoadComplete");
+#endif
}
void DocumentImpl::setParsing(bool b)
@@ -1469,7 +1447,8 @@
if (!m_tokenizer) {
open();
- write(QString::fromLatin1("<html>"));
+ assert(m_tokenizer);
+ write(QString("<html>"));
}
m_tokenizer->write(text, false);
