289169 - DOMParser freezes when parsing elements with fixed attributes.

Status: RESOLVED FIXED

(Core :: XML, defect, P2)

Description

[Alberto González Palomo]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050217
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050217

Parsing elements which are declared with type FIXED, either in an external DTD
(that gets loaded since it's in a chrome:// XUL application), or in the internal
subset as in the example below, makes the whole application to freeze, to the
extent where the only way I can find of stopping it is killing the process with
"killall mozilla-bin".
It works fine with Mozilla 1.7.x, up to 1.7.6.
Fails with any 1.8ax and 1.8bx I've tried, up to 1.8b1.

The bug #284296 seems a bit similar, but not the same:
https://bugzilla.mozilla.org/show_bug.cgi?id=284296


Reproducible: Always

Steps to Reproduce:
1. Evaluate the following into the JavaScript console:

(new DOMParser()).parseFromString('<?xml version="1.0"?>\n<!DOCTYPE omdoc
[\n<!ATTLIST title xmlns CDATA #FIXED "http://purl.org/DC">\n<!ATTLIST creator
xmlns CDATA #FIXED "http://purl.org/DC">\n]>\n<omdoc><metadata><title>Some
title</title><creator role="aut">Some Author</creator></metadata></omdoc>',
'text/xml')

Actual Results:  
Mozilla freezes.


Expected Results:  
Display "[object XMLDocument]" as result.

Comment 1

[Hermann Schwab]

wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050329

forgot to include the string in ():

Error: missing ; before statement
Source File: javascript: new DOMParser()).parseFromString('<?xml
version="1.0"?>\n<!DOCTYPE omdoc[\n<!ATTLIST title xmlns CDATA #FIXED
"http://purl.org/DC">\n<!ATTLIST creatorxmlns CDATA #FIXED
"http://purl.org/DC">\n]>\n<omdoc><metadata><title>Sometitle</title><creator
role="aut">Some Author</creator></metadata></omdoc>','text/xml'
Line: 1, Column: 16
Source Code:
 new DOMParser()).parseFromString('<?xml version="1.0"?>\n<!DOCTYPE
omdoc[\n<!ATTLIST title xmlns CDATA #FIXED "http://purl.org/DC">\n<!ATTLIST
creatorxmlns CDATA #FIXED
"http://purl.org/DC">\n]>\n<omdoc><metadata><title>Sometitle</title><creator
role="au


string in parens (as in comment 0) evaluated:
[object XMLDocument]

Comment 2

[Alberto González Palomo]

>[...]
> forgot to include the string in ():
> 
> Error: missing ; before statement
> Source File: javascript: new DOMParser()).parseFromString('<?xml
>[...]

    Thanks for testing it. However, there is a little difference to what
I wrote: please note the missing opening parenthesis before "new DOMParser".
    I've tried with removing the parenthesis so that it reads:

new DOMParser().parseFromString(...)

    But it still freezes for me.

> string in parens (as in comment 0) evaluated:
> [object XMLDocument]

    As I saw that your userAgent string said "1.8b2", I've just downloaded
the latest nightly build for MacOS X, which identifies itself as:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050404

    The window title bar includes the following at the end:

Mozilla {Build ID: 2005040409}

    Both of the following expressions in the JavaScript console freeze the
whole application as described before. The first one is the original expression
in comment 0, copied from my notes after testing it again:

(new DOMParser()).parseFromString('<?xml version="1.0"?>\n<!DOCTYPE omdoc
[\n<!ATTLIST title xmlns CDATA #FIXED "http://purl.org/DC">\n<!ATTLIST creator
xmlns CDATA #FIXED "http://purl.org/DC">\n]>\n<omdoc><metadata><title>Some
title</title><creator role="aut">Some Author</creator></metadata></omdoc>',
'text/xml')

new DOMParser().parseFromString('<?xml version="1.0"?>\n<!DOCTYPE omdoc
[\n<!ATTLIST title xmlns CDATA #FIXED "http://purl.org/DC">\n<!ATTLIST creator
xmlns CDATA #FIXED "http://purl.org/DC">\n]>\n<omdoc><metadata><title>Some
title</title><creator role="aut">Some Author</creator></metadata></omdoc>',
'text/xml')

    This is a simplified test case from the code I'm using in my application.
    In there, a variable is assigned to the created DOMParser object, and
then used, like this:

  var parser;
  parser = new DOMParser();
  xml_document = parser.parseFromString(document_as_string, "text/xml");

    Where "document_as_string" has the string content that has to be parsed,
which includes the elements in the test expression, with the difference that
the attribute declarations are in an external file. If I put them in the
internal subset, as in the test expression, the result is exactly the same.

    Thank you very much for your reply. It's good to know that in some builds
it works. Perhaps this is particular to the MacOS X version.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Peter, Simon, can you reproduce on mac? This worksforme on Linux, but it _could_
be platform-specific, I guess...

Comment 4

[Simon Fraser [no longer active]]

I tried in Camino. It's getting stuck in

poolAppend
poolStoreString
storeAtts
doContent
contentProcessor
doProlog
prologProcessor
MOZ_XML_Parse
nsExpatDriver::ParseBuffer(char const*, unsigned int, int)
nsExpatDriver::ConsumeToken(nsScanner&, int&)
nsParser::Tokenize(int)
nsParser::ResumeParse(int, int, int)
nsParser::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned
int, unsigned int)
nsDOMParser::ParseFromStream(nsIInputStream*, char const*, int, char const*,
nsIDOMDocument**)
nsDOMParser::ParseFromString(unsigned short const*, char const*, nsIDOMDocument**)
_XPTC_InvokeByIndex
XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)
XPC_WN_CallMethod(JSContext*, JSObject*, unsigned int, long*, long*)
js_Invoke
js_Interpret
js_Execute
JS_EvaluateUCScriptForPrincipals
nsJSContext::EvaluateString(nsAString const&, void*, nsIPrincipal*, char const*,
unsigned int, char const*, nsAString*, int*)
nsJSThunk::EvaluateScript(nsIChannel*)
nsJSChannel::InternalOpen(int, nsIStreamListener*, nsISupports*, nsIInputStream**)
nsDocumentOpenInfo::Open(nsIChannel*)
nsURILoader::OpenURI(nsIChannel*, int, nsIInterfaceRequestor*)
nsDocShell::DoChannelLoad(nsIChannel*, nsIURILoader*)
nsDocShell::DoURILoad(nsIURI*, nsIURI*, int, nsISupports*, char const*,
nsIInputStream*, nsIInputStream*, int, nsIDocShell**, nsIRequest**)
nsDocShell::InternalLoad(nsIURI*, nsIURI*, nsISupports*, unsigned int, unsigned
short const*, char const*, nsIInputStream*, nsIInputStream*, unsigned int,
nsISHEntry*, int, nsIDocShell**, nsIRequest**)
nsWebShell::OnLinkClickSync(nsIContent*, nsLinkVerb, nsIURI*, unsigned short
const*, nsIInputStream*, nsIInputStream*, nsIDocShell**, nsIRequest**)
HandlePLEvent(OnLinkClickEvent*)
PL_HandleEvent

Maybe the code assumes something about the behavior of realloc() that is
different on Mac?

Oddly, tracing also shows both realloc() and PR_Realloc() being called from
poolGrow(), though I'm not sure I trust the symbol lookup for PR_Realloc(). The
only way I could see this happening is if we suck in this header somehow:
http://lxr.mozilla.org/mozilla/source/cck/expat/xmltok/xmldef.h#35

Comment 5

[Simon Fraser [no longer active]]

I debugged through this. poolGrow() pulls a block off the free list whose size
is 0. Subsequent calls to poolGrow() fall into the realloc clause, where
PR_realloc() is called via pool->mem->realloc_fcn(), and 'blockSize' is
calculated as zero each time, so no progress is made. I'm not sure why this
would differ between platforms.

Comment 6

[Peter Van der Beken [:peterv]]

I figured this out, the bug is in storeAtts (in the block starting at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/expat/lib/xmlparse.c&rev=1.30&root=/cvsroot#2734).
 In that block we expand prefixed attribute names and xmlns attributes to pass
along the namespace uri. At
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/expat/lib/xmlparse.c&rev=1.30&root=/cvsroot#2774
we start looping over the attribute name/value pairs. There's a small
optimization at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/expat/lib/xmlparse.c&rev=1.30&root=/cvsroot#2844
and
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/expat/lib/xmlparse.c&rev=1.30&root=/cvsroot#2894
to bail out if there are no more prefixed attribute names or xmlns attributes.
Note that in that case we don't increment i, even though we've just handled the
attribute at i.
Normally the type of attribute is stored in memory just before the attribute
name (at ((XML_Char *)(appAtts[i]))[-1]). We reset that to 0 when we've looped
over the attribute. For prefixed attribute names and xmlns attributes we then
reset appAtts[i] to a string we constructed in the tempPool. If we then bail out
early without incrementing i (see above) when we loop over the remaining
attributes
(http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/expat/lib/xmlparse.c&rev=1.30&root=/cvsroot#2903)
we set ((XML_Char *)(appAtts[i]))[-1] to 0. Since we just set appAtts[i] to a
string from the tempPool we end up overwriting some stuff in the tempPool. In
this case on OS X we overwrote the size member of the tempPool with 0. That's bad.
The fix I'm going to attach just increments i before bailing out. I'll try to
push that change upstream, but I propose we check this one in already in our tree.

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 8

[Boris Zbarsky [:bzbarsky]]

I doubt I'll get to this before July...

Comment 9

[Peter Van der Beken [:peterv]]

My corresponding Expat patch got applied in the Expat CVS. See
https://sourceforge.net/tracker/?func=detail&atid=110127&aid=1221160&group_id=10127

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 186251 [details] [diff] [review]
v1

r+sr=jst

Comment 11

[Peter Van der Beken [:peterv]]

Comment on attachment 186251 [details] [diff] [review]
v1

Simple fix for a freeze.

Comment 12

[Alberto González Palomo]

(In reply to comment #11)
> (From update of attachment 186251 [details] [diff] [review] [edit])
> Simple fix for a freeze.
> 

    Thank you very much, all of you!
    My XUL application is an editor that loads XML files, and it absolutely
needs this part working right. If it wasn't for your work on diagnosing and
fixing this bug, I would have had to do some awful tricks to work around it,
or perhaps abandon.
    Now I know that in future releases it will work fine.

    You made my day.