RESOLVED FIXED 3653
Safari crash on call to DOMImplementation.createDocumentType 
https://bugs.webkit.org/show_bug.cgi?id=3653
Summary Safari crash on call to DOMImplementation.createDocumentType
Curt Arnold
Reported 2005-06-22 14:26:36 PDT
The following document will crash Safari (2.0/412) and a recent CVS build of WebKit: <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd" > <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <title>implementation</title> <script type='text/javascript'>function loadComplete() { var docType = document.implementation.createDocumentType("", "http://www.example.com/", "myDoc.dtd"); }</script> </head> <body onload="loadComplete()"> </body> </html> The expected behavior is the raising of an exception of some type, either a NOT_IMPLEMENTED_ERR exception since HTML-only implementations are not required to implement createDocumentType or a INVALID_CHARACTER_ERR since "" is not a valid tag name (INVALID_CHARACTER_ERR was stretched a bit to cover any failure to match the name production). Crash report: Date/Time: 2005-06-22 16:24:02.142 -0500 OS Version: 10.4.1 (Build 8B15) Report Version: 3 Command: Safari Path: /Applications/Safari.app/Contents/MacOS/Safari Parent: tcsh [373] Version: 2.0 (412) Build Version: 1 Project Name: WebBrowser Source Version: 4120000 PID: 4354 Thread: 0 Exception: EXC_BAD_ACCESS (0x0001) Codes: KERN_PROTECTION_FAILURE (0x0002) at 0x00000004 Thread 0 Crashed: 0 com.apple.WebCore 0x018f5d48 DOM::DocumentPtr::document() const + 20 (dom_nodeimpl.h:85) 1 com.apple.WebCore 0x018f5e28 DOM::NodeImpl::getDocument() const + 40 (dom_nodeimpl.h:293) 2 com.apple.WebCore 0x0169b7f8 KJS::getDOMNode(KJS::ExecState*, DOM::NodeImpl*) + 116 (kjs_dom.cpp:1654) 3 com.apple.WebCore 0x0169c880 KJS::DOMDOMImplementationProtoFunc::tryCall (KJS::ExecState*, KJS::Object&, KJS::List const&) + 972 (kjs_dom.cpp:1334) 4 com.apple.WebCore 0x0168f42c KJS::DOMFunction::call(KJS::ExecState*, KJS::Object&, KJS::List const&) + 88 (kjs_binding.cpp:76) 5 com.apple.JavaScriptCore 0x0102bd0c KJS::FunctionCallNode::evaluate(KJS::ExecState*) + 932 (nodes.cpp:754)
Attachments
Test case [CRASH] (565 bytes, text/html)
2005-06-22 20:18 PDT, David Kilzer (:ddkilzer) 		no flags
Details
Don't assume that nodes always have a document pointer. (3.08 KB, patch)
2005-06-24 07:23 PDT, Anders Carlsson 		darin: review-
DetailsFormatted DiffDiff
Add always-0 documentptr (3.40 KB, patch)
2005-07-08 18:26 PDT, Anders Carlsson 		hyatt: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2005-06-22 20:18:03 PDT
Confirmed crash on build 412 and TOT (CVS TIP) as of 6/22/2005 8:00 PM CDT.
David Kilzer (:ddkilzer)
Comment 2 2005-06-22 20:18:51 PDT
Created attachment 2563 [details] Test case [CRASH]
Anders Carlsson
Comment 3 2005-06-24 07:23:38 PDT
Created attachment 2627 [details] Don't assume that nodes always have a document pointer. The crash is due to the assumption that dom nodes always have a document pointer. This isn't true for nodes created with createElementType, as they haven't been associated with any document. (I'm not sure if DOMNode::Mark() needs to be fixed aswell)
Darin Adler
Comment 4 2005-06-24 08:14:21 PDT
Comment on attachment 2627 [details] Don't assume that nodes always have a document pointer. I'm not sure about the patch -- if these document type nodes aren't attached to any document, perhaps there will be a lifetime issue for the wrappers? Maybe you'll get two different wrappers for the same document type node, so that if you attached a property to it early on and later got it once it was attached to a document the property would be gone.
Darin Adler
Comment 5 2005-06-24 08:21:17 PDT
Comment on attachment 2627 [details] Don't assume that nodes always have a document pointer. Yes, DOMNode::mark() also needs to be updated to handle this case. If there's no document, it should just take the early return that calls DOMObject::mark(). And DOMNode::getValueProperty needs an update too. I'm thinking that patching each and every place that calls getDocument could be a losing battle -- we might need to change the way document type nodes are created to at least point them at a DocumentPtr that is 0 rather than having the DocumentPtr itself be 0. There could be a single "always-0" DocumentPtr for use in cases like that. Once we do that we could revisit the things patched here -- we wouldn't have to patch anything that checks the result of getDocument() for 0, only the things that assume it's non-0.
Anders Carlsson
Comment 6 2005-07-08 18:26:20 PDT
Created attachment 2872 [details] Add always-0 documentptr Here's a new patch which adds a nullDocumentPtr().
Dave Hyatt
Comment 7 2005-07-09 14:05:01 PDT
Comment on attachment 2872 [details] Add always-0 documentptr r=me
Note You need to log in before you can comment on or make changes to this bug.