1117873 - Firefox crash with "Enhanced Steam" extension since Fx35b1

Status: RESOLVED DUPLICATE of bug 1123732

(Core :: Networking: HTTP, defect)

Description

[YF (Yang)]

Run Firefox 35+, install Enhanced Steam extension, open one or more Steam page, the Firefox may crashed.

The crashes seems to be intermittent, I did not find a steps to stable reproduce.


See https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Anet%3A%3AHttpBaseChannel%3A%3ASetupReplacementChannel%28nsIURI*%2C+nsIChannel*%2C+bool%29#tab-comments.

Comment 1

[YF (Yang)]

[Tracking Requested - why for this release]: the causing crashes.

Comment 2

[Ruben Petersen]

This affects a good few users:
http://steamcommunity.com/groups/enhancedsteam/discussions/0/624075036457937895/

(and the extension's author has "not been able to pin down the particular issue that's causing Firefox 35 to crash when Enhanced Steam is enabled")

As per the thread, the issue might not exist on Firefox 36.

Comment 3

[Trunten]

I can confirm the crashing after upgrading to Firefox 35.
Firefox will crash on any of the "Steam" related web sites with the "Enhanced Steam" extension.
Disabling the extension will restore stability to Firefox on the Steam web sites again.
Never had this issue before with other versions of Firefox or the extension.

Comment 4

[kalviskajaks]

There are 2 crash signatures relevant to this bug
1)E10s disabled
https://crash-stats.mozilla.com/report/index/1970cdf5-4ecc-4126-a0a7-365842150115
2)E10S enabled 
https://crash-stats.mozilla.com/report/index/efc71dc6-9c0c-47fa-b48f-41e392150115

Comment 5

[Mikael Magnusson]

I get the crash in firefox 35 and also in seamonkey 2.32, in case that helps narrow it down more.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking for now, though this may turn out to be a tech evangelism issue, it's not a driver for a 35 dot release as there is a workaround (disabling Enhanced Steam).

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

also added this to known issues on 35/36/37

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Tyler - if there isn't already, would it make sense to put up a support article on disabling this extension for people who wonder about crashes?

Comment 9

[jshackles]

Enhanced Steam developer here.  I believe this is related to XMLHttpRequest calls done from the extension context (pageMod) that are performed on pages which have a 302 temporary redirect.  This could have been introduced by fixing bug #1073882 in Firefox 35, as this behavior works as expected in Firefox 34.

I have been able to reliably fix the issue by removing any XMLHttpRequest calls that are done to redirected files, but this limits some of the extension's functionality and could introduce additional browser crashes if files on the remote server are redirected in the future.

I'm happy to provide any additional information I can.

Comment 10

[emtilt]

I can confirm that I experience this crash when using Enhanced Steam 7.2.2 and Firefox 35. There are a large number of users that rely on this add on.

Comment 11

[Trunten]

Still crashes occasionally with 7.2.3 version of the extension.
(In reply to jshackles from comment #9)
> Enhanced Steam developer here...

Comment 12

[:Gijs (out for now; he/him)]

This looks like a nullptr crash, presumably because |uri| is null - meaning one of the redirects has a null URI, which seems odd. mmc/mayhemer, do you have ideas about what's going on here?

Comment 13

[Honza Bambas (:mayhemer)]

STR:
- Nightly debug
- install https://firefox.enhancedsteam.com/latest/enhanced-steam.xpi
- go to http://store.steampowered.com/
- attach the child process

At...

>	xul.dll!mozilla::ipc::PrincipalToPrincipalInfo(0x0a8def24, 0x00240588) Line 132	C++
 	xul.dll!mozilla::net::propagateLoadInfo(0x0d3e4970, {...}) Line 1305	C++
 	xul.dll!mozilla::net::HttpChannelChild::ContinueAsyncOpen() Line 1591	C++
 	xul.dll!mozilla::net::HttpChannelChild::AsyncOpen(0x0d4d9290, 0x00000000) Line 1486	C++
 	xul.dll!nsXMLHttpRequest::Send(0x00000000, {...}) Line 3009	C++
 	xul.dll!nsXMLHttpRequest::Send({...}) Line 423	C++
 	xul.dll!nsXMLHttpRequest::Send(0x04726040, {...}) Line 434	C++
 	xul.dll!nsXMLHttpRequest::Send(0x04726040, {...}, {...}) Line 461	C++
 	xul.dll!mozilla::dom::XMLHttpRequestBinding::send(0x04726040, {...}, 0x0ac4c300, {...}) Line 710	C++
 	xul.dll!mozilla::dom::GenericBindingMethod(0x04726040, 0x00000001, 0x04e1e5c0) Line 2485	C++


0 get_http(url = "http://api.enhancedsteam.com/early_access/", callback = [function]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":285]
    this = [object Window]
1 anonymous() ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":3315]
    this = [object Window]
2 <TOP LEVEL> ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js -> resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js":3306]
    this = [object Window]
3 load(sandbox = [object Window], uri = "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/loader/sandbox.js":66]
4 importScripts(workerSandbox = [object Object], urls = resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/jQuery.min.js,resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/localization.js,resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js, "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/localization.js", "resource://jid1-ydifitekqginxa-at-jetpack/enhanced-steam/data/enhancedsteam.js") ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/content/sandbox.js":290]
    this = null
5 WorkerSandbox(worker = [object Object], window = [object Window]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/content/sandbox.js":252]
    this = [object Object]
6 constructor([object Object], [object Window]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js":146]
7 initialize(options = [object Object]) ["resource://gre/modules/The thread 0x14bc has exited with code 0 (0x0).


...the aPrincipal is an [nsExpandedPrincipal] having no URI (GetURI returns null but doesn't fail).  Since propagateLoadInfo doesn't check result of mozilla::ipc::PrincipalToPrincipalInfo, we store an empty requestingPrincipalInfo and triggeringPrincipalInfo.  This later fails to serialize and I fail with NS_RUNTIMEABORT("unknown union type") - since it's 0 - at PNeckoChild::Write called as Write((__v).triggeringPrincipalInfo(), __msg) from PNeckoChild::Write(const HttpChannelOpenArgs& __v, Message* __msg).



It's the same reason why we fail to have a URI in the redirect chain later (since it's taken from the same principal).


I see following fixes here:
- fix the add-on to not produce XHR loads w/ url-less principals (?)
- fail the XHR load/creation when principals are url-less (jonas?)
- check URL presence in nsHttpChannel (just a logging code anyway, I can do it right now)

Comment 14

[vinod.chandru]

Thanks Honza.
This crashes an extension we've developed as well. Crash Report here: https://crash-stats.mozilla.com/report/index/e57e175b-4477-4741-85c3-ed1302150120 
Since the signature indicates it affects several users, any chance of a dot release with a fix?

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Honza, are you able to write a fix for this? This is the last patch we need to do a dot release of 35. Thanks

Comment 16

[Honza Bambas (:mayhemer)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
> Honza, are you able to write a fix for this? This is the last patch we need
> to do a dot release of 35. Thanks

See bug 1123732.

Comment 17

[Mikael Magnusson]

Based on the patch, I tried compiling firefox with --disable-logging and that does seem to make the crash go away, or at least make it much less rare (I haven't seen it at all yet).

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't know what our security model is on e10s-desktop. Do we trust child processes to use expanded principals? I'm guessing so since they are not that different from "normal" principals?

Mrbkap, can you verify that that is correct?

If so, I think we need to fix PrincipalToPrincipalInfo to be able to properly send Expanded principals across IPC.

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Jonas Sicking (:sicking) from comment #18)
> Mrbkap, can you verify that that is correct?
> 
> If so, I think we need to fix PrincipalToPrincipalInfo to be able to
> properly send Expanded principals across IPC.

Yeah, we definitely need to make PrincipalToPrincipalInfo handle expanded principals. Jetpack frame scripts are going to definitely want to do mashups, even in e10s.

Comment 20

[Trunten]

I no longer have any crashes with (version 7.2.3 of) this extension in Firefox 35.0.1

Comment 21

[Kamil Jozwiak [:kjozwiak]]

This is still happening every single time on fx38 (m-c) with e10s enabled. Once you disable e10s, the crash stops.

* Enhanced Steam v7.2.3
* m-c build used: [build id: 20150128101733]
* https://crash-stats.mozilla.com/report/index/5ccd144a-dddc-4eec-af22-d011c2150128

Comment 22

[kalviskajaks]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #21)
> This is still happening every single time on fx38 (m-c) with e10s enabled.
> Once you disable e10s, the crash stops.
> 
> * Enhanced Steam v7.2.3
> * m-c build used: [build id: 20150128101733]
> *
> https://crash-stats.mozilla.com/report/index/5ccd144a-dddc-4eec-af22-
> d011c2150128

Can confirm. No crashing anymore with e10s disabled. With e10s enabled going to a steam store page, crashes tab instantly every single time. http://store.steampowered.com/app/218620 for example.

Comment 23

[alex_mayorga]

Looks pretty much confirmed and it is even on https://www.mozilla.org/en-US/firefox/36.0beta/releasenotes/ so marking NEW accordingly...

Comment 24

[Sylvestre Ledru [:Sylvestre]]

e10s only, stop tracking.

Comment 25

[YF (Yang)]

Note: It also occurs in without e10s.

Comment 26

[kalviskajaks]

(In reply to YF (Yang) from comment #25)
> Note: It also occurs in without e10s.

Im running latest enhanced steam and nightly i dont experience any crashes without e10s. As far as i know it was supposed to be fixed?

Comment 27

[YF (Yang)]

(In reply to kalviskajaks from comment #26)
> (In reply to YF (Yang) from comment #25)
> > Note: It also occurs in without e10s.
> 
> Im running latest enhanced steam and nightly i dont experience any crashes
> without e10s. As far as i know it was supposed to be fixed?

sorry, I forgot the should have been fixed by bug 1123732.

Comment 28

[Lawrence Mandel [:lmandel] (use needinfo)]

Dropping tracking and marking all versions > 38 as unaffected. 38 will also be unaffected once it moves to Aurora. This needs to be fixed specifically for when e10s is enabled.