339599 - nsISAXXMLReader expects a nsIChannel passed to onStartRequest, but interface takes an nsIRequest

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Todd A. Fisher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: XULRunner 1.8 Branch

The application will crash unless you do something like:

function StreamListener( reader, channel )
{
  this.reader = reader;
  this.channel = channel;
}
StreamListener.prototype = {
  reader:null,
  onStartRequest: function( request,context ){
    dump( "starting\n" ); 
    // XXX: passing channel here to work around bug in nsParser.cpp
    this.reader.onStartRequest( this.channel, context );
  },
  onDataAvailable: function( request, context, input, offset, count ){
    dump( "offset: " + offset + ", count: " + count + "\n" );
    // XXX: passing channel here to work around bug in nsParser.cpp
    this.reader.onDataAvailable( this.channel, context, input, offset, count );
  },
  onStopRequest: function( request, context, status ){
    // XXX: passing channel here to work around bug in nsParser.cpp
    this.reader.onStopRequest( this.channel, context, status );
  }
};

crash happens in:

nsresult nsParser::OnStartRequest(nsIRequest *request, nsISupports* aContext)

...

  nsCOMPtr<nsIChannel> channel = do_QueryInterface(request);
  NS_ASSERTION(channel, "parser needs a channel to find a dtd");
  // the channel is NULL since it wasn't an nsIChannel, it was an nsIRequest

  rv = channel->GetContentType(contentType);
  if (NS_SUCCEEDED(rv))
  {
    mParserContext->SetMimeType(contentType);
  }



Reproducible: Always

Steps to Reproduce:
call nsISAXXMLReader without using an nsIChannel, instead using something like an nsIInputStreamPump.  Like below:

  var reader = CC["@mozilla.org/saxparser/xmlreader;1"].createInstance(CI.nsISAXXMLReader);
  var filestream = CC["@mozilla.org/network/file-input-stream;1"].createInstance(CI.nsIFileInputStream);
  var pump = CC["@mozilla.org/network/input-stream-pump;1"].createInstance(CI.nsIInputStreamPump);
  var ioService = CC["@mozilla.org/network/io-service;1"].getService(CI.nsIIOService);
  var channel = ioService.newChannel( urlSpec,null,null );
  var uri = ioService.newURI( urlSpec, null, null );

  filestream.init(file, 0x01, 0444, 0);

  reader.contentHandler = new RestoreParser( broker, progress );

  reader.parseAsync( null );

  pump.init( filestream, -1, -1, StreamChunkSize, 1, false );
  reader.baseURI = uri;
  pump.asyncRead( reader, null );

Comment 1

[Robert Sayre]

I have fix for that depends on 337451, after that lands, we'll be able to test for a channel in nsSAXXMLReader::InitParser.

Comment 2

[Robert Sayre]

Attachment: don't assume nsIChannel

The ASSERT in the current code seems bogus to me, since it will keep going if channel->GetContentType fails. I tested XML files with and without declarations, and the crashing code in Comment #1 worked fine for me on trunk.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 225225 [details] [diff] [review]
don't assume nsIChannel

It looks like with this patch, we'll end up in DetermineParseMode's catch-all else case, which, while scary, seems like it should work.

Comment 4

[Robert Sayre]

(In reply to comment #3)
> (From update of attachment 225225 [details] [diff] [review] [edit])
> It looks like with this patch, we'll end up in DetermineParseMode's catch-all
> else case, which, while scary, seems like it should work.

For trunk, we could add nsIParser::SetMimeType, and then assert that mParserContext->mMimeType is set in OnStartRequest.

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 225225 [details] [diff] [review]
don't assume nsIChannel

Yeah, this seems like a fragile solution.

Comment 6

[Robert Sayre]

Comment on attachment 225225 [details] [diff] [review]
don't assume nsIChannel

For the branch, I think this is the best we can do :/

Comment 7

[Robert Sayre]

/cvsroot/mozilla/parser/htmlparser/src/nsParser.cpp,v  <--  nsParser.cpp
new revision: 3.393; previous revision: 3.392
done

Comment 8

[Robert Sayre]

retargetted to b2 per beltzner

Comment 9

[Robert Sayre]

/cvsroot/mozilla/parser/htmlparser/src/nsParser.cpp,v  <--  nsParser.cpp
new revision: 3.370.4.4; previous revision: 3.370.4.3
done