352346 - toXMLString() called on text node in XMLList fails to escape '<' and '>' and '&' in the text content

Status: RESOLVED FIXED

(Rhino Graveyard :: E4X, defect)

Description

[Martin Honnen]

This test with the shell of Rhino 1.6 R 4 (and XML beans 1.0.4)

Rhino 1.6 release 4 2006 09 09
js> var xmlList = <><![CDATA[<strong>Kibo & Xibo</strong>]]></>;
js> xmlList[0].nodeKind()
text
js> xmlList.toXMLString()
<strong>Kibo & Xibo</strong>

shows that the toXMLString method fails to properly escape '<' and '>' and '&' in the text content in the XML list. Result should be

&lt;strong&gt;Kibo &amp; Xibo&lt;/strong&gt;

instead as Spidermonkey does.

In terms of the XML specification escaping '>' as '&gt;' is not a must but for symmetry and clearness usually '>' is escaped. '<' and '&' however must be escaped so Rhino has a bug there.

Comment 1

[Martin Honnen]

I have looked the E4X specification (2nd edition) and it clearly requires toXMLString to always escape '<', '>', '&' for text nodes respectively element content. The requirement is in section 10.2.1.1 EscapeElementValue.

The Rhino E4X implementors of toXMLString however have choosen not do do any escaping on text nodes as
<http://lxr.mozilla.org/mozilla/source/js/rhino/xmlimplsrc/org/mozilla/javascript/xmlimpl/XML.java#2897>
simply checks

if (curs.isText())
{
  result = curs.getChars();
}

and that getChars method of the XmlCursor in XML beans does not seem to be intended to give you XML markup with proper escaping but rather plain text:
<http://xmlbeans.apache.org/docs/1.0.4/reference/org/apache/xmlbeans/XmlCursor.html#getChars()>

So this bug is not specific to text nodes in XMLList objects but simply happens when toXMLString is directly called on an XML object of node kind 'text'.

Comment 2

[Attila Szegedi]

Ok, any idea for a patch? Just escape the return value of "curs.getChars()" in

if (curs.isText())
{
  result = curs.getChars();
}

would do? Does XmlBeans or Rhino already have code for XML escaping somewhere?

Comment 3

[Martin Honnen]

(In reply to comment #2)
> Ok, any idea for a patch? Just escape the return value of "curs.getChars()" in
> 
> if (curs.isText())
> {
>   result = curs.getChars();
> }
> 
> would do? 

It would be start, at least as this bug is concerned. E4X wants toXMLString called on an attribute to apply escaping too so that would need to be fixed too.

> Does XmlBeans or Rhino already have code for XML escaping somewhere?

Rhino has
<http://lxr.mozilla.org/mozilla/source/js/rhino/xmlimplsrc/org/mozilla/javascript/xmlimpl/XML.java#508>
dumpNode which then makes uses of cursor.xmlText in some cases. Using that it might be possible to at least get well-formed stuff from toXMLString(). It does not look as if XML beans escpapes '>', XML does not require that, but the E4X specification spells out that '>' should be escaped as '&gt;'.

And I am more guessing what might help than really understanding the XML beans API.

Are those guys that implemented E4X for Rhino no longer interested in maintaining/fixing it? The file lists contributors 
  Ethan Hugg
  Terry Lucas
  Milen Nankov

Comment 4

[David Caldwell]

This bug is fixed in the upcoming non-XMLBeans implementation of E4X for Rhino.  See bug 355677.

Comment 5

[David Caldwell]

The submitted test case gives the submitter's expected results if XMLBeans is not present.  See bug 355677.

Comment 6

[Norris Boyd]

Adding target milestone of 1.6R6 based on the date this bug was resolved FIXED.