359012 - Wrong host added to whitelist when xpi linked to in a frame on a different server

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Kisha Shone]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

I try to download an extension and receive a message saying Firefox prevented it. I click on "Edit Options", and add the site to my "Allow" list. I try to download the extension again and Firefox still blocks it. I closed Firefox and restarted it to see if this helped, but it still doesn't allow me to download an extension from the website even though I have manually added it to my allow list.

Reproducible: Always

Steps to Reproduce:
1. Click "Download" (for any of the extensions).
2. Click "Edit Options" -> "Allow" -> "Close"
3. Click "Download" again

Actual Results:  
I keep receiving a message that Firefox blocked the download.

Expected Results:  
After I add the site to my allow list, Firefox shouldn't block downloads from it anymore.

Theme: default
Extensions:
Adblock Plus 0.7.2.2
Forecastfox 0.9.3.1
Tabbrowser Preferences 1.3.1.1

Comment 1

[Dave Townsend [:mossop]]

Where is it you're trying to download the extension from?

Comment 2

[Kisha Shone]

http://www.graysonmixon.com/extension
I was unable to find the addon I wanted from the usual extensions page, so I went to it's homepage to download it. I'm able to download it on my older version of Firefox (1.5.0.7) just not the new version I installed on my main computer.

Comment 3

[Dave Townsend [:mossop]]

Confirming this. The site links to extensions on the host 68.1.89.194:8080 yet when you attempt to install the warning bar and subsequent add to whitelist all refer to the original host (www.graysonmixon.com) so unless you add "68.1.89.194:8080" manually to the whitelist you cannot install the extensions.

Comment 4

[Robert Strong (they/them - no direct email)]

Sounds like this is xpinstall since it is the code at work at this stage

Comment 5

[Dave Townsend [:mossop]]

Slightly different to what I first thought actually. The webpage is actually a frameset with the main frame being on the server 68.1.89.194:8080, the same server as the xpi is held on. But the whitelist check and add all work from the host of the top level webpage.
 

Comment 6

[Daniel Veditz [:dveditz]]

This is a regression in 2.0, 1.5x worked fine.

Google toolbar is facing a similar problem if people remove the default whitelisted host. In both cases the extension is launched from a subframe on a different host, graysonmixon.com frames http://68.1.89.194:8080/extension/ and the Google toolbar install launches the install by opening a frame on addons.mozilla.org

http://tools.google.com/firefox/toolbar/install.html

In 1.5 the install whitelist infobar offers to add the correct host (68.1.89.194 or addons.mozilla.org), and in 2.0 the infobar wants to add the topmost host.

The xpinstall code did not change in the whitelisting area. It looks like it's checking the source (frame) against the whitelist correctly, it's the notification and/or front-end event handling that's gone wrong.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpinstall/src/nsInstallTrigger.cpp&rev=FIREFOX_2_0_RELEASE#249

Note the above code hasn't changed since the aviary landing, FF1.0

Possible xpinstall broken assumptions:
- did nsIScriptGlobalObjectOwner::GetScriptGlobalObject() change behavior?
- is the wrong aWindowContext being passed in?
- is GetDocShell() returning a different shell?

(there are similar notifications from scripted installs in nsJSInstallTriggerGlobal.cpp -- don't know if those are
similarly broken or if it's just the link click ones that are.

Looks like the infobar widget was reworked in FF2.0, and now instead of using the shell (aSubject) passed by the notification it's using the
shell from the current browser -- that'd match the symptoms we're seeing, too.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&mark=670,709&rev=FIREFOX_2_0_RELEASE#663

Is the fix as simple as passing shell to xpinstallEditPermissions()
on line 709 instead of browser.docShell ?

I'm on vacation and can't drive this any further, I'm assigning
to Neil based on cvs blame. Would like this fixed in 2.0.0.x which
shouldn't be a problem if it's as simple as I'm guessing. But I don't
know the new infobar widget

Comment 7

[Neil Deakin]

Attachment: Change shell reference

Yes, the fix is that simple.

Comment 8

[Asaf Romano (gone)]

Comment on attachment 246183 [details] [diff] [review]
Change shell reference

r=mano

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Not blocking, but wanted, patch should be approved (Neil, can you nominate for approval1.8.1.1?)

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 246183 [details] [diff] [review]
Change shell reference

approved for the 1.8 branch, a=dveditz for drivers

Comment 12

[Phil Ringnalda (:philor)]

Neil, do you remember you have a 1.8.1.2 checkin pending here?

Comment 13

[Carsten Book [:Tomcat]]

Verified fixed for trunk and 1.8.1.2.

Verified with:
Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a2pre) Gecko/20070206 Minefield/3.0a2pre ID:2007020604 [cairo] for trunk

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre ID:2007020604 for 1.8.1.2