1541161 - Crash in [@ mozilla::net::HttpBaseChannel::SetLoadInfo] due to Services.io.newChannelFromURI

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Oriol Brufau [:Oriol]]

Open browser console and run

var uri = Services.io.newURI("http://example.com", null, null);
var channel = Services.io.newChannelFromURI(uri, null, null, null, null, null);

Result: crash. The assert added in bug 1533159 fails.
https://crash-stats.mozilla.org/report/index/f3db6df4-b9c0-489f-b462-5d4ba0190402
https://crash-stats.mozilla.org/report/index/ade3924e-e839-406e-88b6-867360190402

Just throw an error or something, crashing seems over the top.

Comment 1

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

That's a fair point. I wonder if instead of MOZ_RELEASE_ASSERT we should use a

MOZ_DIAGNOSTIC_ASSERT(aLoadInfo);
if (!aLoadInfo) {
  return NS_ERROR_INVALID_ARG;
}

Also, in nsIOService::NewChannelFromURIWithProxyFlagsInternal we should probably do the same.

(In reply to Oriol Brufau [:Oriol] from comment #0)

Open browser console and run

var uri = Services.io.newURI("http://example.com", null, null);
var channel = Services.io.newChannelFromURI(uri, null, null, null, null, null);

While your point stands, I must point out that calling any Firefox internal code isn't guaranteed to not crash the browser :)

Comment 2

[Christoph Kerschbaumer [:ckerschb, back Sept 8th]]

Yeah, that's probably a good idea after all - I'll incorporate that change.

Comment 3

[Christoph Kerschbaumer [:ckerschb, back Sept 8th]]

Attachment: Bug 1541161: Use MOZ_DIAGNOSTIC_ASSERT and return error in loadinfo is null within NewChannelFromURIWithProxyFlagsInternal. r=valentin

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6dfafa75ea1a
Use MOZ_DIAGNOSTIC_ASSERT and return error in loadinfo is null within NewChannelFromURIWithProxyFlagsInternal. r=valentin

Comment 5

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/6dfafa75ea1a

Comment 6

[Pascal Chevrel:pascalc (PTO until Sept 2)]

Very low volume on beta and may have caused a different crash in 68, doesn't look like a good candidate for uplifting, marking as wontix for 67.