Bug 8509 - document.importNode(null) crashes
Summary: document.importNode(null) crashes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Geoffrey Garen
URL: javascript:document.importNode(null)
Keywords: EasyFix
Depends on:
Blocks:
 
Reported: 2006-04-21 00:04 PDT by Maciej Stachowiak
Modified: 2019-02-06 09:02 PST (History)
1 user (show)

See Also:


Attachments
Fix (4.19 KB, patch)
2006-04-21 11:49 PDT, Geoffrey Garen
sullivan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.