Closed Bug 393309 Opened 12 years ago Closed 12 years ago
Install Trigger::Handle Content when browsing directly to XPI file
Minefield is crashing when I attempt to browse to a non-AMO site that contains an extension. This seems to happen even after I add the site to the whitelist. BuildID: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007082204 Minefield/3.0a8pre I have two crash logs: http://crash-stats.mozilla.com/report/index/67822257-50f4-11dc-b579-001a4bd46e84?date=2007-08-22-21 http://crash-stats.mozilla.com/report/index/f6b8a205-50f8-11dc-8919-001a4bd43ed6?date=2007-08-22-21 == Steps == 1. Browse to https://landfill.bugzilla.org/mozillaQA/latest.xpi == Actual Results == Crash == Expected Results == Get the warning box stating something like "do you want to save this or install it into firefox. Note this site is not trusted etc etc". Note that you can go to https://landfill.bugzilla.org/mozillaQA/ and install the XPI from there, and white list the site. Then when you try directly linking to the XPI again, it will crash on you once more. I'm putting this in extension compatibility because we're in nsInstallInfo in the stack, but it might actually belong in the Download Manager component. I was also able to confirm this crash with the same nightly Aug 22, 2007 on Windows XP: http://crash-stats.mozilla.com/report/index/11109a1c-50fe-11dc-b23f-001a4bd43ef6?date=2007-08-22-22 Since this is a crash on two OS's (and probably on linux too) --> status Critical.
Summary: Crash @nsInstallTrigger::HandleContent when linking directly to XPI file → Crash @nsInstallTrigger::HandleContent when browsing directly to XPI file
Caused by bug 307770, will be fixed by the patch in bug 252830. On another note that breakpad stack is rubbish, it links to the wrong file for the given method.
Is this the same reason FF trunk and SM trunk newer than 20070819 crash when dragging any XPI file from the desktop on a browser window (tested on Windows)? On crash I get a box saying "Unfortunately the crash reporter is unable to submit a crash report. The application did not leave a crash dump file." So no report, sorry.
(In reply to comment #2) > Is this the same reason FF trunk and SM trunk newer than 20070819 crash when > dragging any XPI file from the desktop on a browser window (tested on Windows)? Yes I think that's pretty likely the same.
at least for bug #393682, we are crashing in nsInstallTrigger::HandleContent(), dereferencing referringURI when it is null Here's a stack to the crash: > xpinstal.dll!nsInstallTrigger::HandleContent(const char * aContentType=0x0d086f90, nsIInterfaceRequestor * aWindowContext=0x29aefe28, nsIRequest * aRequest=0x0d086f00) Line 212 + 0x25 bytes C++ docshell.dll!nsDocumentOpenInfo::DispatchContent(nsIRequest * request=0x0d086f00, nsISupports * aCtxt=0x00000000) Line 500 + 0x47 bytes C++ docshell.dll!nsDocumentOpenInfo::OnStartRequest(nsIRequest * request=0x0d086f00, nsISupports * aCtxt=0x00000000) Line 280 + 0x10 bytes C++ necko.dll!nsBaseChannel::OnStartRequest(nsIRequest * request=0x0a6ac040, nsISupports * ctxt=0x00000000) Line 604 + 0x46 bytes C++ necko.dll!nsInputStreamPump::OnStateStart() Line 439 + 0x2c bytes C++ necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x09c0d0c0) Line 395 + 0xb bytes C++ xpcom_core.dll!nsInputStreamReadyEvent::Run() Line 112 C++ xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fa00) Line 491 C++ xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00bce7e8, int mayWait=1) Line 227 + 0x16 bytes C++ gkwidget.dll!nsBaseAppShell::Run() Line 154 + 0xc bytes C++ tkitcmps.dll!nsAppStartup::Run() Line 170 + 0x1c bytes C++ xul.dll!XRE_main(int argc=3, char * * argv=0x00bc9660, const nsXREAppData * aAppData=0x00bc9a50) Line 3069 + 0x25 bytes C++ firefox.exe!main(int argc=3, char * * argv=0x00bc9660) Line 153 + 0x12 bytes C++ firefox.exe!__tmainCRTStartup() Line 586 + 0x19 bytes C firefox.exe!mainCRTStartup() Line 403 C kernel32.dll!7c816fd7() [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll] working on a potential fix.
the comment in the code already points to the issue. from http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsInstallTrigger.cpp#164 164 // It's possible docshell explicitly set a null referrer in the case 165 // of typed, pasted, or bookmarked URLs and the like. In such a case 166 // we get a success return value with null pointer.
12 years ago
Assignee: nobody → sspitzer
Target Milestone: --- → mozilla1.9 M8
That patch will change the whitelist check to use the xpi url in the event of no referer, thus requiring it to pass the whitelist even for manually entered urls and such which is not what we want I think. The patch in bug 252830 will fix this. I'm just wondering though whether it might just be sensible to back out bug 307770 until then. This is showing up as the 8th most popular crasher on breakpad.
> That patch will change the whitelist check to use the xpi url in the event of > no referer, thus requiring it to pass the whitelist even for manually entered > urls and such which is not what we want I think. good point, comparing to fx 2, this is not what we want. since bug #252830 will fix this, should we just wait for that fix? re-assigning to dave, as he has the fix in his other bug.
Assignee: sspitzer → dtownsend
Comment on attachment 278202 [details] [diff] [review] patch obsoleting, based on dave's comment.
This avoids crashing by null checking the URI before calling GetHost. If referringURI is null, we don't need the host anyways, because the only case where a load with a null referringURI is disallowed is if the global xpinstall.disabled pref is false, in which case the UI will only show the "xpinstall disabled, enable it?" notification rather than the "add to whitelist?" notification. This will get reworked in bug 252830, but I think we should just fix this crasher for now until that patch can land.
Assignee: dtownsend → gavin.sharp
Status: NEW → ASSIGNED
Attachment #278424 - Flags: review?
Attachment #278424 - Flags: review? → review?(dtownsend)
(assume that dangling bracket is removed :)
Comment on attachment 278424 [details] [diff] [review] fix the crash Looks good. Will be worth watching breakpad to see if the other case really is happening.
Attachment #278424 - Flags: review?(dtownsend) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Verified this fix with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007082904 Minefield/3.0a8pre Nice work. Also added litmus testcase 4598 (http://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=4598) to catch this regression. --> VERIFIED
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
Breakpad is currently showing 0 crashes for this from todays build so it looks like those stacks pointing to uri are probably faulty.
12 years ago
You need to log in before you can comment on or make changes to this bug.