285166 - createElement crashes [@ NS_NewHTMLElement] [@ nsCharTraits<unsigned short>::length]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Frank Wein [:mcsmurf]]

To reproduce:
1. Load the testcase from URL (it requires some XML file, so i uploaded it there).
2. Click on Mail
3. Observe crash

Stacktrace:
nsCharTraits<unsigned short>::length(const unsigned short * 0x00000000) line 310
NS_NewHTMLElement(nsIContent * * 0x0012ee0c, nsINodeInfo * 0x00000000) line 992
+ 14 bytes
NS_NewElement(nsIContent * * 0x0012ee0c, int 0x00000003, nsINodeInfo *
0x05c9b0e0) line 244 + 11 bytes
nsDocument::CreateElement(nsDocument * const 0x00000000, nsINodeInfo *
0x05c9b0e0, int 0x00000003, nsIContent * * 0x0012eefc) line 4421 + 21 bytes
nsDocument::CreateElem(nsDocument * const 0x00000000, nsIAtom * 0x0792a5e0,
nsIAtom * 0x00000000, int 0x00000000, int 0x00000001, nsIContent * * 0x05c9b0e0)
line 4412 + 12 bytes
nsHTMLDocument::CreateElement(nsHTMLDocument * const 0x00000000, const nsAString
& {...}, nsIDOMElement * * 0x0012ef44) line 1277
XPTC_InvokeByIndex(nsISupports * 0x06e61088, unsigned int 0x0000001f, unsigned
int 0x00000002, nsXPTCVariant * 0x0012ef34) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
0x9254d260) line 2067 + 22 bytes
XPC_WN_CallMethod(JSContext * 0x02e718a8, JSObject * 0x0254d260, unsigned int
0x00000001, long * 0x01a40560, long * 0x01a40410) line 1287 + 10 bytes
js_Invoke(JSContext * 0x00000001, unsigned int 0x00000001, unsigned int
0x00000000) line 1293 + 17 bytes
js_Interpret(JSContext * 0x02e718a8, unsigned char * 0x027b32d8, long *
0x0012f3a8) line 3568
js_Execute(JSContext * 0x00f868b8, JSObject * 0x02e4d488, JSScript * 0x06da0f08,
JSStackFrame * 0x00000000, unsigned int 0x00000000, long * 0x0012f460) line 1524
JS_EvaluateUCScriptForPrincipals(JSContext * 0x02e718a8, JSObject * 0x02e4d488,
JSPrincipals * 0x03e1c294, const unsigned short * 0x0012f4c0, unsigned int
0x0000000a, const char * 0x0012f558, unsigned int 0x00000001, long * 0x0012f460)
line 3739 + 15 bytes
nsJSContext::EvaluateString(nsJSContext * const 0x00000000, const nsAString &
{...}, void * 0x02e4d488, nsIPrincipal * 0xffffffff, const char * 0x0012f558,
unsigned int 0x00000001, const char * 0x00000000, nsAString * 0x0012f5f0, int *
0x0012f600) line 1035 + 69 bytes
nsJSThunk::EvaluateScript(nsJSThunk * const 0x00000000, nsIChannel * 0x03e1c290)
line 255 + 82 bytes

Note if you comment in the line
//if(xIn.childNodes[i].nodeType != 1) continue;
it doesn't crash.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Pretty minimal testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

With that testcase I get:

###!!! ASSERTION: Don't pass invalid names to nsDocument::CreateElem, check
caller.: 'NS_SUCCEEDED(rv)', file
/home/bzbarsky/mozilla/xlib/mozilla/content/base/src/nsDocument.cpp, line 4398
###!!! ASSERTION: What? Reverse mapping of id to string broken!!!: 'tag', file
/home/bzbarsky/mozilla/xlib/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 990

then crash when we pass null (per second assert) to do_GetAtom.

So what happens here is that createElement() gets passed "#text" as the
nodename.  In quirks mode we don't do a qname check, then we assert in
nsDocument::CreateElem (which does a check via the parser service, not via
nsContentUtils::CheckQName; why, I don't know; the two do the same thing).

Then we take this string, map it to a HTML tag using nsHTMLTags::LookupTag()
(called by HTMLAtomTagToId), which returns eHTMLTag_text.  Mapping that back to
a tag atom returns null, however.

Things are worse with __moz_text -- that one is valid as a tagname _and_ will
get mapped to eHTMLTag_text.

So we either need to fix HTMLAtomTagToId to never return eHTMLTag_text (and
audito all callers), or we need to fix the content sink caller
(NS_NewHTMLElement) to handle this situation...

Note also that the standards version -- HTMLCaseSensitiveAtomTagToId -- is
likely broken too (for the __moz_text thing, at least).

Peter, this (well, the code we start out in) looks like code you touched most
recently, no?

Comment 3

[Boris Zbarsky [:bzbarsky]]

I do think we want to fix this one way or another for Gecko 1.8.

Comment 4

[Peter Van der Beken [:peterv]]

I'll take this.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

One thing we should do while we're at it is to make texts and comments have
Tag() atoms that aren't valid element-names. I'm pretty sure this is causing
bugs if you were to create an <__moz_text> element.

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: Quick fix

Here's a quick fix for this bug. I started on a better fix, but I keep finding
more and more silly stuff so it's starting to grow and I'd rather put it in a
separate bug.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 177539 [details] [diff] [review]
Quick fix

sr=bzbarsky, but yeah, let's get that followup filed.

Comment 8

[Peter Van der Beken [:peterv]]

Hmm, I just realized that both instances of |id >= nsHTMLTag(NS_HTML_TAG_MAX)|
should be |id > NS_HTML_TAG_MAX|

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to comment #7)
> (From update of attachment 177539 [details] [diff] [review] [edit])
> sr=bzbarsky, but yeah, let's get that followup filed.

Bug 286300 filed.

Comment 10

[Peter Van der Beken [:peterv]]

Checked in. The assertion is annoying, but it'll be hard to remove it just for
this case (createElement in quirks mode).

Comment 11

[Dave Townsend [:mossop]]

*** Bug 304533 has been marked as a duplicate of this bug. ***

Comment 12

[Bob Clary [:bc] (inactive)]

content/html/document/crashtests/285166-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3