265135 - Can't install extensions from update.mozilla.org (whitelist)

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Tracy Walker [:tracy]]

seen with Windows FF branch build 2004-10-19-07-0.9

-Using the extensions manager, goto the u.m.o page from Get More Extensions
-select an extension to install. then in the install page, click Install Now

tested results: The notification "To protect your computer, Firefox has
prevented this site (update.mozilla,org) from installing software on your
computer" appears.

expected results:

extension download dialog comes up.

-click on Edit Options
notice that u.m.o is already whitelisted as an allowed site.
-delete it, then add it back
-attempt Install Now again

tested results:  Notification again.

expected results:

extension download dialog comes up.

note: http://extensionroom.mozdev.org will work if you goto Edit Options and set
to allow the site

Comment 1

[Peter van der Woude [:Peter6]]

*** Bug 265133 has been marked as a duplicate of this bug. ***

Comment 2

[edward ash]

Already TRIED deleting the listing for allowed, then RE-allowing it...it didnt
work...

I'll try again..

Comment 3

[edward ash]

(In reply to comment #2)
> Already TRIED deleting the listing for allowed, then RE-allowing it...it didnt
> work...
> 
> I'll try again..

TRIED it, didnt work...
"Firefox could no download the file at
http//ftp.mozilla.org/pub/mozilla.org/extentions/textzoom/textzoom_1.6.0.xpi
Because download error"

It only happens on UNSIGNED....

Comment 4

[sairuh (rarely reading bugmail)]

(In reply to comment #0)
> The notification "To protect your computer, Firefox has
> prevented this site (update.mozilla,org) from installing software on your
> computer" appears.

when using 2004101809-0.9+ (y'day's) on linux fc2, I didn't get this
notification. (unable to test with today's build due to bug 265103.)

does anyone else see this bug using y'day's build (as opposed to today's)?

Comment 5

[sairuh (rarely reading bugmail)]

cannot seem to repro this on Mac OS X 10.3.5 using 2004101907-0.9+ bits.

Comment 6

[sairuh (rarely reading bugmail)]

> cannot seem to repro this on Mac OS X 10.3.5 using 2004101907-0.9+ bits.

whups, scratch that; was looking at the wrong thing (themes instead of
extensions, which do work). I do see the same problem as Tracy on Mac: just get
the notification bar, etc.

this had worked with 2004101807-0.9+.

Comment 7

[Tracy Walker [:tracy]]

confirmed this regressed between 2004-10-18-07-0.9 and 2004-10-19-07-0.9 windows
builds.

Comment 8

[Peter van der Woude [:Peter6]]

checkin

2004-10-18 14:26 DVeditz bug 264560: tighten up referrer requirements for XPI
whitelist ?

Comment 9

[Curtis Duhn]

Here's another data point:  I reproduced this under the Windows 2004-10-19-07
build, although it seemed to fix itself once I navigated to another extension.  

Here's what I did:

1. Went to u.m.o.
2. Navigated to OutSidebar (my extension, which I'm testing for compatibility)
3. Clicked Install Now
4. Got the "To protect your computer..." notification.  Noticed that u.m.o was
already listed in my whitelist.
5. Clicked "Add" for u.m.o. anyway.
6. Clicked Install Now again
7. Got the "To protect your computer..." notification again.  u.m.o. still in
whitelist.
8. Navigated to the IE View extension.
9. Clicked Install Now
10. Software Installation dialog popped up as expected.
11. Canceled and navigated back to OutSidebar.
12. Clicked Install Now
13. Software Installation dialog popped up as expected.

Comment 10

[Daniel Veditz [:dveditz]]

Taking bug, it is most likely my patch which fiddled with the core of this
logic, though it shouldn't have affected this case.

Comment 11

[Curtis Duhn]

I tried and failed to reproduce my own behavior as described above, but in the
process I discovered this repeatable case:

Installing this way works:

1. Launch firefox.
2. Navigate to u.m.o by typing it into your URL bar.
3. Select an extension.
4. Click Install Now
5. The Software Installation dialog pops up as expected.

Installing this way doesn't work:

1. Launch firefox.
2. Select Tools->Extensions->Get More Extensions, which opens u.m.o in a new window.
3. Select an extension.
4. Click Install Now
5. The "To protect your computer..." notification pops up.

Comment 12

[edward ash]

(In reply to comment #11)
> I tried and failed to reproduce my own behavior as described above, but in the
> process I discovered this repeatable case:
> 
> Installing this way works:
> 
> 1. Launch firefox.
> 2. Navigate to u.m.o by typing it into your URL bar.
> 3. Select an extension.
> 4. Click Install Now
> 5. The Software Installation dialog pops up as expected.
> 

Thats funny..
It worked..
But could only DL 1 extention..
Going around and see if it will do again.

Comment 13

[edward ash]

> 
> Thats funny..
> It worked..
> But could only DL 1 extention..
> Going around and see if it will do again.
> 

NOPE,
Only worked 1 time...
Even after restart, cold boot.

Comment 14

[edward ash]

> 
> NOPE,
> Only worked 1 time...
> Even after restart, cold boot.

http://ftp35.newaol.com/pub/mozilla.org/extensions

when did this become your upload site????
FTP to your DL location and it swaps to HERE....

Comment 15

[edward ash]

I also went to some home sites,
Had to ALLOW them, and got 1 set, then the option stopped working also..
Even after reset.

Comment 16

[Sébastien Vauclair]

Seeing it on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3)
Gecko/20041019 Firefox/1.0 (MOOX M3)

Can install extensions from u.m.o using http:// protocol, but not https://

Comment 17

[edward ash]

Security setting on the page is LOCKED.

http://update.mozilla.org/extensions/install.php/chatzilla-0.9.65.xpi?id=16&vid=735
this is the link from the site.

Extention DL window says:
http://ftp.mozilla.org/pub/mozilla.org/extensions/chatzilla/chatzilla-0.9.65.xpi

Even got here, one time:
http://ftp35.newaol.com/pub/mozilla.org/extensions

The HTTP, and HTTPS thing worked 1 TIME.  Now wont work.

Comment 18

[Ben Goodger (use ben at mozilla dot org for email)]

if worst comes to the worst, wolf is preparing a patch to make umo use
InstallTrigger.install() to install extensions, however this may break aspects
of the site such as download counts and should only be used as a backup if
dveditz' attempt to implement darin's suggested use of nsIHTTPChannelInternal
doesn't pan out.

Comment 19

[alanjstr]

(In reply to comment #18)
> if worst comes to the worst, wolf is preparing a patch to make umo use
> InstallTrigger.install() to install extensions, however this may break aspects
> of the site such as download counts and should only be used as a backup if
> dveditz' attempt to implement darin's suggested use of nsIHTTPChannelInternal
> doesn't pan out.

Well, I'd like to see InstallTrigger so that we can do bug 246812.  Can we do
some sort of internal redirect to InstallTrigger that would still count it?

Comment 20

[edward ash]

> 
> Well, I'd like to see InstallTrigger so that we can do bug 246812.  Can we do
> some sort of internal redirect to InstallTrigger that would still count it?

The funny part.
1 of threee things happens.
It Loads then give error.  Security??
If something has allready tried to load, and you try again on different, the
process bar shows it gets halfway and Pauses ALONG time, ?Or it dont even try,
if it was tried before(same DL).

Comment 21

[Darin Fisher]

Attachment: Patch to make nsHttpChannel and nsFTPChannel support nsIProperties

This patch enables anyone to set and get arbitrary properties on necko channels
(HTTP and FTP at least, for now).  It depends on the patch for bug 263957. 
Otherwise, properties are not preserved across HTTP redirects.

Comment 22

[Darin Fisher]

adding dependency on bug 263957 since i think we want our whitelist to work
across HTTP redirects.

Comment 23

[Wolf [:wolf]]

Attachment: Make UMO use InstallTrigger.install()

This is the patch Ben mentioned in Comment 18. It changes UMO from using direct
XPI links (via redirect) to using InstallTrigger on the URL. This breaks (or
severely skews) download count statistics though. :-/ but if it's needed, use
it. :-)

Comment 24

[Darin Fisher]

Attachment: Revised version of Necko nsIProperties patch

This is a revised version of the Necko nsIProperties patch to deal with changes
to my patch for bug 263957.

Comment 25

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 162866 [details] [diff] [review]
Revised version of Necko nsIProperties patch

note to all: this patch alone will not fix this bug.

nsFTPChannel.h
+    

no need to add trailing whitespace :-)

nsHttpChannel.cpp
+		 for (PRUint32 i=0; i<count; ++i) {

missing space around = and <

can you simply do newChannel->mProperties = mProperties; ? that may have
undesired consequences...

+		 do_CreateInstance(NS_PROPERTIES_CONTRACTID, (nsIChannel *)
this);

make this NS_STATIC_CAST?

I'm not sure QI is the best way to expose this. well, it certainly will do for
now. r=biesi.


I've been wondering for some time... should necko have an nsBaseChannel class,
which by itself would do things like manage loadgroups, loadflags, would
implement (overridable) Open using NS_ImplementOpen, etc - i.e. common base
stuff many channels need? it would probably come in handy here as well. just
something to think about.

man, this tiny textarea sucks.

Comment 26

[Asa Dotzler [:asa]]

Comment on attachment 162866 [details] [diff] [review]
Revised version of Necko nsIProperties patch

a=asa pending sr=.

Comment 27

[Darin Fisher]

> can you simply do newChannel->mProperties = mProperties; ? that may have
> undesired consequences...

yeah, that's a problem because what if properties of the old channel are modified?


> I've been wondering for some time... should necko have an nsBaseChannel class,
> which by itself would do things like manage loadgroups, loadflags, would
> implement (overridable) Open using NS_ImplementOpen, etc - i.e. common base
> stuff many channels need? it would probably come in handy here as well. just
> something to think about.

yes, this is something i definitely want to do at some point.

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 162866 [details] [diff] [review]
Revised version of Necko nsIProperties patch

sr=bzbarsky.  I wish we could make this code less ugly... :(

Comment 29

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #27)
> > can you simply do newChannel->mProperties = mProperties; ? that may have
> > undesired consequences...
> 
> yeah, that's a problem because what if properties of the old channel are modified?

right... would be nice if nsIProperties had a clone method...

Comment 30

[Daniel Veditz [:dveditz]]

Attachment: Combined patch for aviary

Darin's Necko patch is only part of the solution, and the one here is a trunk
patch that fails to apply to the aviary branch.

This patch combines his with docshell/xpinstall changes to get this over on
firefox.

Comment 31

[Christian :Biesinger (don't email me, ping me on IRC)]

+      props->Set("docshell.internalReferrer", aReferrerURI);

that reminds me... a downside of exposing this via QueryInterface is that there
is no meaningful place to describe its usage. What definitely should be
documented is the intended namespacing of the attributes (as shown here).

Comment 32

[Darin Fisher]

Comment on attachment 163062 [details] [diff] [review]
Combined patch for aviary

>Index: docshell/base/nsDocShell.cpp

>+    nsCOMPtr<nsIProperties> props(do_QueryInterface(channel));
>+    if (props)
>+    {
>+      // save true referrer for those who need it (e.g. xpinstall whitelisting)
>+      // Currently only http and ftp channels support this.
>+      props->Set("docshell.internalReferrer", aReferrerURI);
>+    }

I'd leave out the comment about only HTTP and FTP supporting this
since that is bound to change at some point.  Instead, I'd say that
some channels may not support this.  And, you could add "but HTTP
and FTP do" or something like that.  The comment "as is" is sure to
bit-rot shortly.


>Index: xpinstall/src/nsInstallTrigger.cpp

>+    if (channelprops && 
>+        NS_SUCCEEDED(channelprops->Has(kReferrerProperty, &useReferrer)) &&
>+        useReferrer)
>+    {
>+        // channel may have the property but set to null. In that case we know
>+        // it was a typed URL or bookmark and can bypass site whitelisting, as
>+        // opposed to not knowing the origin and going with the fallback plan.
>+        channelprops->Get(kReferrerProperty,
>+                          NS_GET_IID(nsIURI),
>+                          getter_AddRefs(referringURI));
>+    }

why waste code with "Has()" ... maybe just call Get and if there's a result,
use it otherwise don't use it?

Comment 33

[Johnny Stenback (:jst)]

Comment on attachment 163062 [details] [diff] [review]
Combined patch for aviary

>+    nsCOMPtr<nsISupports>           mProperties;

While mProperties is required to be of type nsISupports here since that's
required by the COM aggregation rules (as darin informed me), it's a bit odd
(and certainly unusual in Mozilla code), and might be worthy of a comment. Same
thing in nsHttpChannel.

sr=jst

Comment 34

[Darin Fisher]

FYI, the patch for bug 263957 (on which this bug depends) has been commited to
the aviary1.0 and 1.7 branches.

Comment 35

[Asa Dotzler [:asa]]

Comment on attachment 163062 [details] [diff] [review]
Combined patch for aviary

a=asa for aviary checkin.

Comment 36

[Daniel Veditz [:dveditz]]

Attachment: Fix null-referrer crash

Typing in the url bar or hitting a bookmarked xpi were crashing because
nsProperties didn't entirely support null values. Now that I'm sure what value
I'll get in the null-referrer case I can get rid of the Has() call that annoyed
Darin.

Comment 37

[Daniel Veditz [:dveditz]]

Comment on attachment 163106 [details] [diff] [review]
Fix null-referrer crash

Carrying over sr/a

Comment 38

[Henrik Skupin [:whimboo][⌚️UTC+2]]

*** Bug 264536 has been marked as a duplicate of this bug. ***

Comment 39

[Chris Beck]

Hi, I'm the reporter of 264536 ... I would just like to point out that my build
is not a nightly ... I am using 1.0PR with extensions.  Perhaps one of the
security extensions had this as an unintended side-effect?  Did anything happen
on the server or the network devices (firewalls,load-balancers,switches,routers)
in front of it?

Comment 40

[Darin Fisher]

Comment on attachment 163106 [details] [diff] [review]
Fix null-referrer crash

>Index: xpcom/ds/nsProperties.cpp

> nsProperties::Get(const char* prop, const nsIID & uuid, void* *result)
...
>+    return (value) ? value->QueryInterface(uuid, result) : NS_ERROR_NO_INTERFACE;
> }

ok, at first glance i thought NS_ERROR_NULL_POINTER would be better, but
from the point-of-view of the callers, NS_ERROR_NO_INTERFACES indicates
that a property exists, but that that property doesn't support the
requested interface.  i guess that makes sense given that a null pointer
supports no interfaces.

still, NS_ERROR_NULL_POINTER might be clearer...

you could even return NS_OK and *result=NULL, but then that would seem to
suggest that the requested interface is supported -- ok, i don't like that.

NS_ERROR_NULL_POINTER or NS_ERROR_NO_INTERFACE ... hmm... ok, i'm fine
with NS_ERROR_NO_INTERFACE.

r=darin

Comment 41

[Darin Fisher]

ah, and given the documentation in the frozen nsIProperties.idl, i like
NS_ERROR_NO_INTERFACE for sure ;-)

Comment 42

[[e-mail]]

FYI nsISupportsArray::QueryElementAt returns NS_ERROR_FAILURE for null.

Comment 43

[Darin Fisher]

> FYI nsISupportsArray::QueryElementAt returns NS_ERROR_FAILURE for null.

but nsISupportsArray isn't frozen, nsIProperties is :-/

Comment 44

[alanjstr]

*** Bug 259715 has been marked as a duplicate of this bug. ***

Comment 45

[sairuh (rarely reading bugmail)]

tested with 2004102409-0.9+ (linux fc2) and 2004102406-0.9+ (mac os x 10.3.5),
extension installation from https://update.mozilla.org now works.

Comment 46

[Daniel Veditz [:dveditz]]

Fixed 1.7.x

Comment 47

[edward ash]

OK,
If this is fixed.
what was fixed...
the site or firefox.

If it was fire fox, where to get the update..

Comment 48

[Steffen Wilberg]

Firefox was broken since around 10/18, and is fixed again since 10/23. Get a new
nightly from here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-0.11/

Or try 1.0RC1: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/1.0rc1/

By the way, this needs trunk checkin, bug 263957 is fixed.

Comment 49

[Chris Beck]

Word on the street (neowin, mozillazine) is that PR1 is still broken.
As a point of interest, this broke long before 10/18.  I am using PR1!

https://bugzilla.mozilla.org/show_bug.cgi?id=264536

Comment 50

[Peter van der Woude [:Peter6]]

No, it is fixed, use FF1.0RC1

Comment 51

[Chris Beck]

Umm, yeah, sorry, I meant RC1.
Neowin quotes Asa as mentioning that at least the application update is still
broken.
http://www.neowin.net/comments.php?id=25210&category=main
http://weblogs.mozillazine.org/asa/archives/006789.html

Comment 52

[Daniel Veditz [:dveditz]]

Checked in to the trunk on October 28

Comment 53

[Frank Wein [:mcsmurf]]

Comment on attachment 163062 [details] [diff] [review]
Combined patch for aviary

kill obsolete review request