Bug 8509

Summary: document.importNode(null) crashes
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: DOMAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez
Priority: P1 Keywords: EasyFix
Version: 420+   
Hardware: Macintosh   
OS: OS X 10.4   
URL: javascript:document.importNode(null)
Attachments:
Description Flags
Fix sullivan: review+

Description Maciej Stachowiak 2006-04-21 00:04:50 PDT
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.
Comment 1 Geoffrey Garen 2006-04-21 10:44:27 PDT
I'm on it.
Comment 2 Geoffrey Garen 2006-04-21 11:49:21 PDT
Created attachment 7878 [details]
Fix
Comment 3 John Sullivan 2006-04-21 16:27:48 PDT
Comment on attachment 7878 [details]
Fix

Looks good.
Comment 4 Darin Adler 2006-04-21 22:20:52 PDT
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.
Comment 5 Geoffrey Garen 2006-04-22 00:29:34 PDT
Reverted code Darin mentioned, then landed.
Comment 6 Lucas Forschler 2019-02-06 09:02:55 PST
Mass moving XML DOM bugs to the "DOM" Component.