document.importNode(null) crashes

• Component: DOM
• Status: RESOLVED FIXED

[Maciej Stachowiak]

Doing document.importNode(null) causes a crash with the following backtrace:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x01de03af in WebCore::Document::importNode (this=0xf103c00, importedNode=0x0, deep=false, ec=@0xbfffe518) at /Volumes/Data/mjs/Work/src/Safari/OpenSource/WebCore/dom/Document.cpp:457
(gdb) bt
#0  0x01de03af in WebCore::Document::importNode (this=0xf103c00, importedNode=0x0, deep=false, ec=@0xbfffe518) at /Volumes/Data/mjs/Work/src/Safari/OpenSource/WebCore/dom/Document.cpp:457
#1  0x01f569ab in WebCore::JSDocumentProtoFunc::callAsFunction (this=0x230ac490, exec=0xbfffe8e8, thisObj=0x230ac3b0, args=@0xbfffe748) at /Users/mjs/Work/symroots/Debug/DerivedSources/WebCore/JSDocument.cpp:301
#2  0x015ce96c in KJS::JSObject::call (this=0x230ac490, exec=0xbfffe8e8, thisObj=0x230ac3b0, args=@0xbfffe748) at /Volumes/Data/mjs/Work/src/Safari/OpenSource/JavaScriptCore/kjs/object.cpp:96
#3  0x015c58cb in KJS::FunctionCallDotNode::evaluate (this=0x231a8900, exec=0xbfffe8e8) at /Volumes/Data/mjs/Work/src/Safari/OpenSource/JavaScriptCore/kjs/nodes.cpp:758
#4  0x015c312e in KJS::ExprStatementNode::execute (this=0x231a8a00, exec=0xbfffe8e8) at /Volumes/Data/mjs/Work/src/Safari/OpenSource/JavaScriptCore/kjs/nodes.cpp:1712

Looks like just lack of a null check.

[Geoffrey Garen]

I'm on it.

[Geoffrey Garen]

Created attachment 7878 [details]
Fix

[John Sullivan]

Comment on attachment 7878 [details]
Fix

Looks good.

[Darin Adler]

Comment on attachment 7878 [details]
Fix

Geoff, there's a reason I used this style:

         case DOCUMENT_TYPE_NODE:
         case DOCUMENT_FRAGMENT_NODE:
         case NOTATION_NODE:
-            break;
+        default:
+            ec = NOT_SUPPORTED_ERR;
+            return 0;
     }
-
-    ec = NOT_SUPPORTED_ERR;
-    return 0;
 }

If you don't have a "default" in your switch statement, gcc will give you a warning if you leave any enum values out. So I often write switch statements in a slightly awkward way, just to avoid putting a default in. Here your patch is undoing that.

[Geoffrey Garen]

Reverted code Darin mentioned, then landed.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.