1450726 - [@ mozilla::net::nsHttpChannel::OnStopRequest ] crash

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Oriol Brufau [:Oriol]]

Enter this code into browser console:

  var ioService = Components.classes["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
  var uri = ioService.newURI("http://example.com", null, null);
  var channel = ioService.newChannelFromURI2(uri, null, null, null, null, null);
  channel.QueryInterface(Ci.nsIHttpChannel);
  channel.asyncOpen({}, null);

Result: crash

Crash report: https://crash-stats.mozilla.com/report/index/5852320f-ca2c-4a97-9fcc-c76820180402

Regression window: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ed8bb5680400049c4dfea4756e751a187e7d5700&tochange=f61cd15b68a01063cf4f10ec7f2ae0748f141ae7

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Missing null check.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1450726 - Missing null check for mLoadInfo

Review commit: https://reviewboard.mozilla.org/r/233054/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233054/

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

I thought we were now getting to the point where we could require LoadInfo on a channel.  Christoph, isn't this something you were working towards?

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Ben Kelly [Part-time, not reviewing][:bkelly] from comment #3)
> I thought we were now getting to the point where we could require LoadInfo
> on a channel.  Christoph, isn't this something you were working towards?

I think we have assertions that our own created channels can't have a null loadinfo. That case is something special. Nevertheless it's not great that we are crashing. Since legacy extensions are gone I think we could return an error if AsyncOpen() has been called without calling AsyncOpen2() first. Or even better, we get Bug 1279428 fixed and landed and completely remove AsyncOpen().

Comment 5

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8964336 [details]
Bug 1450726 - Missing null check for mLoadInfo

https://reviewboard.mozilla.org/r/233054/#review238812

LGTM, thanks for fixing

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6b1a6b5a10a
Missing null check for mLoadInfo r=jkt

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/c6b1a6b5a10a