Closed Bug 613568 Opened 9 years ago Closed 9 years ago

InstallTrigger.install accepts undefined as a URL and attempts to download the current page as an extension

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: jorgev, Assigned: mossop)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

This was reported in the Add-ons Forum: https://forums.mozilla.org/addons/viewtopic.php?f=7&t=1437&p=5300#p5300

Steps to reproduce:
1) Go to http://wmlbrowser.mozdev.org/installation/wmlbrowser.html
2) Click on the link to install.

Result:
Installation fails with an error. The Error Console shows:
Warning: WARN addons.xpi: Download failed: Downloaded file hash (4ee8e91e2b2151826a4c3ecc6b8e355f4fa606e61ce7037b386e449884910962) did not match provided hash (e52d9e020f2daceb197488b67fc2da01bf3d08778eda41a0a162722a3d48fe92)

Expected result:
The add-on installs correctly.


If you download the file and check the hash, it is correct. The link points to http://downloads.mozdev.org/wmlbrowser/wmlbrowser-0.7.25.xpi, which in run redirects to an FTP site, which is probably the reason this is failing.


Tested on Firefox 4 beta 7 on Mac OS.
Forgot to mention this works fine on 3.6.
blocking2.0: --- → ?
Actually the hash is broken for me (perhaps the mirrors have differing content or something?):

lorenz ~$ wget http://downloads.mozdev.org/wmlbrowser/wmlbrowser-0.7.25.xpi
--2010-11-19 12:53:01--  http://downloads.mozdev.org/wmlbrowser/wmlbrowser-0.7.25.xpi
HTTP request sent, awaiting response... 302 Found
Location: http://mirror.umoss.org/mozdev/wmlbrowser/wmlbrowser-0.7.25.xpi [following]
--2010-11-19 12:53:01--  http://mirror.umoss.org/mozdev/wmlbrowser/wmlbrowser-0.7.25.xpi
HTTP request sent, awaiting response... 200 OK
Length: 92049 (90K) [application/x-xpinstall]
2010-11-19 12:53:03 (135 KB/s) - “wmlbrowser-0.7.25.xpi” saved [92049/92049]

lorenz ~$ openssl dgst -sha1 wmlbrowser-0.7.25.xpi 
SHA1(wmlbrowser-0.7.25.xpi)= edcbf423a9a043d825a1ddd0d814ddfd5c3343a5

The real problem though is that the webpage is broken, because there is a <span> inside the link that is what is being clicked on when the event handler is called, so aEvent.target.href is the href property of the span, not the link, probably giving null. In Firefox 3.6 InstallTrigger decides that this is an illegal call giving the error:

Error: Incorrect arguments to InstallTrigger.Install()
Source File: http://wmlbrowser.mozdev.org/installation/wmlbrowser.html
Line: 129

3.6 falls back to just following the link when the InstallTrigger call throws which is why it appears to work, it just isn't doing a hash check there at all.

In 4.0 it looks like we're assuming null is ok and using it as a relative URL from the main page and so it's trying to download and install http://wmlbrowser.mozdev.org/installation/wmlbrowser.html as the XPI (which of course fails the hash check).

Since this is caused by a bogus InstallTrigger call I suspect it doesn't block, both 3.6 and 4.0 behaviours are kind of sane. But I'm going to look at just what restrictions 3.6 placed on the urls passed and see if it makes sense to reproduce them in 4.0.
Summary: Hash validation fails for download redirecting to FTP → InstallTrigger.install accepts null as a URL and attempts to download the current page as an extension
(In reply to comment #2)
> Actually the hash is broken for me (perhaps the mirrors have differing content
> or something?):
> lorenz ~$ openssl dgst -sha1 wmlbrowser-0.7.25.xpi 
> SHA1(wmlbrowser-0.7.25.xpi)= edcbf423a9a043d825a1ddd0d814ddfd5c3343a5

Actually, the dev is using SHA256, not SHA1. Either way, the hash computed by Firefox does match either of those.
Err yes. The hash is correct so it should work fine if the webpage is fixed.
Duplicate of this bug: 612649
Going to block on this, looks like we should check that the url and iconurl aren't undefined
Assignee: nobody → dtownsend
blocking2.0: ? → beta9+
Summary: InstallTrigger.install accepts null as a URL and attempts to download the current page as an extension → InstallTrigger.install accepts undefined as a URL and attempts to download the current page as an extension
Attached patch patch rev 1 (obsolete) — Splinter Review
The old code actually used JSVAL_IS_VOID on the properties which tests whether they are undefined, null gets accepted so this just duplicates that and tests the main URL case.
Attachment #492762 - Flags: review?(robert.bugzilla)
Status: NEW → ASSIGNED
Whiteboard: [has patch][needs review rs]
Attached patch patch rev 1Splinter Review
Failed to include the test file
Attachment #492762 - Attachment is obsolete: true
Attachment #492787 - Flags: review?(robert.bugzilla)
Attachment #492762 - Flags: review?(robert.bugzilla)
Comment on attachment 492787 [details] [diff] [review]
patch rev 1

>+++ b/toolkit/mozapps/extensions/test/xpinstall/browser_badargs2.js
>@@ -0,0 +1,27 @@
>+// ----------------------------------------------------------------------------
>+// Test whether passing aundefined url InstallTrigger.install throws an
nit: an undefined url causes InstallTrigger.install to throw
Attachment #492787 - Flags: review?(robert.bugzilla) → review+
Whiteboard: [has patch][needs review rs] → [has patch]
Landed: http://hg.mozilla.org/mozilla-central/rev/d744f676a758
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch]
Target Milestone: --- → mozilla2.0b8
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
blocking2.0: beta9+ → betaN+
Dave, is there any way for me to manually verify this fix? I tried to reproduce with a beta7 build on OS X but wasn't successful in doing so.
Keywords: regression
Version: unspecified → Trunk
http://wmlbrowser.mozdev.org/installation/testcase.html should have the old installation code, so it should be reproducible from there I would have thought.
Sadly not, and I'm testing with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7) Gecko/20100101 Firefox/4.0b7. Could this be related to the mirror I'm getting?
A fairly simple test would be to just load a webpage and then put this into the url bar it should display an error in the error console about a missing URL property:

javascript:InstallTrigger.install({addon: { URL: undefined }});
Reproducible for me in Mozilla/5.0 (X11; Linux i686; rv:2.0b7) Gecko/20100101 Firefox/4.0b7 at http://wmlbrowser.mozdev.org/installation/testcase.html, assuming Javascript is enabled, selecting either link.
Verified fixed with the hint from comment 15 with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre ID:20110209030359
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.