Closed Bug 613568 Opened 10 years ago Closed 10 years ago
Trigger .install accepts undefined as a URL and attempts to download the current page as an extension
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.
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
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.
Status: NEW → ASSIGNED
Whiteboard: [has patch][needs review rs]
Failed to include the test file
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]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
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.
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?
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.