Here is what I intend to implement: On https requests that respond with a redirect the following header will be looked for: X-Target-Digest: sha1:3847273423.... The first digest encountered is used, so if InstallTrigger passes a hash then that is used, if not then the first https response or the second or so on. If a redirect from https to http occurs and no digest has yet been provided then the download is aborted. Downloads that start from http are unaffected.
This implements support for the hash header and adds tests. The testing relies on bug 435743. I removed the separate notification callbacks object we were using, there seems no reason for it and we need to share the hash around anyway and we only create the crypto stream in onStartRequest which always comes after any redirects have occurred.
Attachment #471883 - Flags: review?(robert.bugzilla)
Comment on attachment 471883 [details] [diff] [review] patch rev 1 I started the review of the two bugs out of order... I'll do the other one now. This is very nicely done! >+ // nsIChannelEventSink The CertUtils and the original CertUtils didn't format this comment correctly. Please use the same format as used elsewhere in this file /** * blah * * @see nsIChannelEventSink */
Attachment #471883 - Flags: review?(robert.bugzilla) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
(In reply to comment #1) > On https requests that respond with a redirect the following header will be > looked for: > > X-Target-Digest: sha1:3847273423.... Is this still what we should be sending? Is anything besides sha:<string> accepted?
(In reply to comment #5) > (In reply to comment #1) > > On https requests that respond with a redirect the following header will be > > looked for: > > > > X-Target-Digest: sha1:3847273423.... > > Is this still what we should be sending? Is anything besides sha:<string> > accepted? Yes that is what is supported, the algorithm can be any of md5, sha1, sha256, sha384, sha512.
Dave, which steps have to be done to verify this fix?
(In reply to comment #7) > Dave, which steps have to be done to verify this fix? Unless you want to set up custom headers for an XPI file on a webserver you'll need to wait till AMO update the install service to support this (bug 591028)
(In reply to comment #8) > Unless you want to set up custom headers for an XPI file on a webserver you'll > need to wait till AMO update the install service to support this (bug 591028) So what I would have to add to the .htaccess file and when I want to use an existing extension, do I also have to modify its install.rdf?
I couldn't work out hte magic htaccess to do this but here is a php script that looks for a file in the same directory, adds it's hash to the header and then redirects to it over http. You'll also need a webpage that uses InstallTrigger passing it the url of the redirect script with the name of the XPI as the query string.
Attachment #486684 - Attachment mime type: text/php → text/plain
Thanks Dave! I have now setup a testcase on my domain for that case. It works fine when the 'X-Target-Digest' header gets set. But when I comment it out, we still download the xpi file - at least that's what the messages in the error console are telling me. Here the example: https://www.hskupin.info/mozilla/addons/hash/ That we do not remove the downloading entry in the doorhanger is probably the already known issue.
For me it looks like it does stop the download. Did you clear your cache after you commented it out it might be using the cached data.
Ok, clearing the cache helped. Now I get: "Warning: WARN addons.xpi: Download failed: 302 Moved Temporarily". Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110210 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.