287409 - loading xml documents from file input stream using nsIDOMParser::parseFromStream fails

Status: VERIFIED FIXED

(Core :: XML, defect, P2)

Description

[Nickolay_Ponomarev]

Loading XML documents using nsIDOMParser from file input stream fails. The error
message is as follows:

XML Parsing Error: no element found
Location: [...]/test.xul
Line Number: 1, Column: 1

To reproduce, create a simple XML file named test.xml in your profile and open
the attached testcase. The testcase is a XUL file with this script:
----
// get profile directory
var file = Components.classes["@mozilla.org/file/directory_service;1"].
  getService(Components.interfaces.nsIProperties).
  get("ProfD", Components.interfaces.nsIFile);
file.append("test.xml");
   
var fileInputStream =
Components.classes["@mozilla.org/network/file-input-stream;1"].
  createInstance(Components.interfaces.nsIFileInputStream);

fileInputStream.init(file, 1, 0, 0);

var domParser = new DOMParser();
var doc = domParser.parseFromStream(fileInputStream, "UTF-8",
fileInputStream.available(), "text/xml");
if(doc.firstChild.firstChild)
  alert(doc.firstChild.firstChild.nodeValue);
----

As far as I can see, this happens because nsDOMParser::ParseFromStream() relies
on nsParser to parse the stream passed to it, and nsParser makes use of stream's
ReadSegments() method, that is not implemented for filestreams.

The strange message is because in nsDOMParser::ParseFromStream() the return
value of nsParser::OnDataAvailable() (which is the one that fails) is ignored.
Therefore, nsDOMParser thinks everything was parsed correctly, but the tree is
empty.

The workaround is to read the file into a string and call parseFromString.

Comment 1

[Nickolay_Ponomarev]

Attachment: testcase

Comment 2

[Nickolay_Ponomarev]

oops. You first need to save the testcase to your disk.

Comment 3

[Peter Van der Beken [:peterv]]

Shaver, this sounds similar to what you ran into, no?

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

the code at
http://robert.accettura.com/archives/2004/12/24/reporter-status-update/ probably
fails due to the same issue, only that it's JAR code that doesn't implement
readsegments
(http://lxr.mozilla.org/seamonkey/source/modules/libjar/nsJARChannel.cpp#174)

Comment 5

[Robert Accettura [:raccettura]]

Attachment: bz's last try

bz attempted to help me out a few weeks ago with the attached code.

{As I recall}
Seems to fail on windows, and work on linux.

Comment 6

[Peter Van der Beken [:peterv]]

Is this a regression? We switched to ReadSegments in bug 197114, more than a
year ago.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #6)
> Is this a regression? We switched to ReadSegments in bug 197114, more than a
> year ago.

Isn't the issue here in
http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/nsParser.cpp#2605,
unrelated to the external entity references from that bug?

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK.  So the reason that loading data from file:// currently works at all is that
nsFileChannel::AsyncOpen wraps the raw file stream up in async stream (using an
input transport), and that implements ReadSegments (it's just an nsPipe).

The DOMParser should probably wrap a buffered stream around the stream it's given.

That said, is it worth making our file streams buffered (eg via mmap() or
something)?

Comment 9

[Robert Accettura [:raccettura]]

bz:  is this what was busting that buildconfig code?

If so, having this fixed somewhat soon would be very welcome, as I'd like to be
capturing buildconfig when reporter gets rolled out.

Comment 10

[Darin Fisher]

> That said, is it worth making our file streams buffered (eg via mmap() or
> something)?

A nsIInputStream implementation is not required to implement readSegments. 
Fixing nsFileInputStream as you suggest is not a solution to the real problem. 
The DOM parser should deal with lack of a readSegments implementation.  It is
fairly easy to write code that checks to see if readSegments is supported (just
implement a writer callback that consumes no data and have it set a boolean flag
so you can tell that it was tickled).

Comment 11

[Darin Fisher]

BTW, I regret that nsIInputStream was frozen without an isBuffered method to
avoid messy readSegments checking code such as what I suggested above.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> is this what was busting that buildconfig code?

Almost certainly.

> A nsIInputStream implementation is not required to implement readSegments. 

I realize that.  The real question is whether there would be any sort of
efficiency win...  I guess there wouldn't be if we usually wrap them up anyway...

> The DOM parser should deal with lack of a readSegments implementation.

One question is whether the DOMParser should deal or the nsParser should
deal....  The latter is what's really assuming all streams passed to
OnDataAvailable have a ReadSegments that works.  That's not part of the
nsIStreamListener interface definition as far as I can tell, so perhaps it
shouldn't be assuming that?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Fix DOMParser to use the new API darin added in bug 287750.

I decided it wasn't worth passing in knowledge about the string and byte
stream; just testing whether the stream is buffered is fast enough.

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 178650 [details] [diff] [review]
Fix

sr=jst

Comment 15

[Darin Fisher]

Comment on attachment 178650 [details] [diff] [review]
Fix

nit: s/COMPtr/nsCOMPtr/ in the comment

r=darin

Comment 16

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 17

[Nickolay_Ponomarev]

verified using 20050401 win Firefox build, thanks!

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

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