Send HTTP header to differentiate addon updates from normal file downloads

RESOLVED FIXED in mozilla34

Status

()

enhancement
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: nONoNonO, Assigned: nONoNonO)

Tracking

({dev-doc-needed})

unspecified
mozilla34
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Assignee

Description

7 years ago
Some websites, like sourceforge.net, always show a download page when you select a file for download. This download page is only bypassed if you don't use a known browser, e.g. if you download the file using wget.
If the add-on updater would use a different user-agent string than the browser/mail program, it would be possible for the website to distinguish the download from a normal client request, so the download page can be skipped.
TBH, I'd consider that a bug in sites like sourceforge. 

The user agent is currently useful for easily gathering stats on addon updates, and occasionally to do the opposite of how sourceforge is redirecting - to redirect non-Gecko user agents to a page. Changing the user agent string to something that wouldn't be detected as a known browser by the likes of sourceforge would break that existing functionality (thus breaking updates for some addons that currently update fine).
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Assignee

Comment 2

7 years ago
Well, maybe you can reconsider. You don't have to ditch the User-Agent string altogether, but maybe you can add something to it, like ' Updater' or ' Backend' or whatever to distinguish it from a normal page request. That way you can still detect OS and App from the User-Agent header and see the difference between a normal download (e.g. to download the extension from amo to install it in thunderbird) and an extension update.
If we wanted to allow sites to distinguish between different reasons for the request in some way other than by URL then I think the more appropriate way to do so would be with a custom http header in the request. Regardless of why it is making the request it is still Firefox making it so I don't think altering the UA makes sense.

Comment 4

7 years ago
SourceForge developer here. We actually look at the referer first. If no referer is set, then we check the UA to see if it's any of the known browsers. If it is, we serve up the download landing page; if not then we assume a CLI tool and send the download directly. I'm open to ideas for better ways to handle the problem, including setting a custom header.
IMO what SF does is bogus mystery meat navigation design.

The way to handle the problem is to provide and actual download link that can be followed in any HTTP compliant UA and that has to be fixed at SF. Firefox is not at fault at all.

Comment 6

7 years ago
I agree that Firefox is not at fault here. If Mozilla chooses to keep this bug closed, that's fine by me. With that said, as a site developer, it would be nice for us if Firefox had some way of identifying/distinguishing its non-interactive downloader from its browser. Think of this as an enhancement request, not a bug report.
kadams, if there's no referrer, does sf.net skip the download page? If so couldn't we just fix this by setting the Referer header blank?
(In reply to Luke Crouch [:groovecoder] from comment #7)
> kadams, if there's no referrer, does sf.net skip the download page? If so
> couldn't we just fix this by setting the Referer header blank?

There should already be no Referer header sent. According to comment 4, when that's the case it then checks the User-Agent.


(In reply to Dave Townsend (:Mossop) from comment #3)
> I think the more appropriate way
> to do so would be with a custom http header in the request

That seems reasonable. Strawman proposal:
  Moz-XPI-Update: 1

(Note: Not using an X- prefix, as per RFC6648.)

An alternative (and probably technically more appropriate) would be to set the Accept header to "application/x-xpinstall". But I don't know what that would break :\


There's also the question of whether to make the differentiation between new installs and updates (I don't think there's any privacy implication there, as its already doable via using a different URL). Not all new installs would be affected, just those initiated via InstallTrigger, which should pass through without a landing page anyway - so I'm leaning towards not making the distinction.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WONTFIX → ---
Summary: Add-on Update check should use different user-agent → Send HTTP header to differentiate addon updates from normal file downloads

Comment 9

7 years ago
I'd be thrilled with either the Moz-XPI-Update or an Accept header.
Assignee

Comment 10

7 years ago
Maybe someone from amo has thoughts about differentiating normal downloads from updates and an extra http header...
Assignee

Comment 11

6 years ago
When I look at the download statistics for my extension, I see that addons.mozilla.org differentiates between downloads from the add-ons manager and downloads from the website:
https://addons.mozilla.org/en-US/firefox/addon/forward/statistics/downloads/sources/?last=30

Maybe someone can explain how amo recognizes one from another and sourceforge can do it the same way, without having to wait for an extra Mox-XPI-Update header being built into core.
(In reply to Onno Ekker from comment #11)
> Maybe someone can explain how amo recognizes one from another and
> sourceforge can do it the same way, without having to wait for an extra
> Mox-XPI-Update header being built into core.

The sources there are literally src=_______ in the URLs.  Go to the homepage and hover over one of the links and you'll see the src parameter.

Comment 13

6 years ago
(In reply to Wil Clouser [:clouserw] from comment #12)
> The sources there are literally src=_______ in the URLs.  Go to the homepage
> and hover over one of the links and you'll see the src parameter.

There are a couple interesting things on AMO: the src param in the querystring and the data-hash attribute. That being said, neither seem likely culprits for how AMO is able to distinguish updater from regular browser in download stats. Good catch Onno; if someone can explain how this magic happens, we can do the same on SF.
(In reply to kadams from comment #13)
> (In reply to Wil Clouser [:clouserw] from comment #12)
> > The sources there are literally src=_______ in the URLs.  Go to the homepage
> > and hover over one of the links and you'll see the src parameter.
> 
> There are a couple interesting things on AMO: the src param in the
> querystring and the data-hash attribute. That being said, neither seem
> likely culprits for how AMO is able to distinguish updater from regular
> browser in download stats. Good catch Onno; if someone can explain how this
> magic happens, we can do the same on SF.

It uses the src variable.  The "Addons Manager" header in the statistics page comes from src=api which is added to the URL when you search for something from the interface at Tools->Add-ons in Firefox.  I'm the AMO Engineering Lead.
Assignee

Comment 15

6 years ago
Posted patch AddonUpdateChecker.diff (obsolete) — Splinter Review
This patch adds a Moz-XPI-Update: 1 header to extensions update check.
Assignee

Comment 16

6 years ago
Attachment #806484 - Attachment is obsolete: true
Attachment #806484 - Flags: feedback?(bmcbride)
Assignee

Updated

6 years ago
Attachment #806580 - Flags: review?(bmcbride)
Comment on attachment 806580 [details] [diff] [review]
2nd try at patch, now hopefully according to guidelines

Review of attachment 806580 [details] [diff] [review]:
-----------------------------------------------------------------

This handles it for the request for the update manifest, but not the request for the actual XPI file. You'll need to add the same to AddonInstall.openChannel() in XPIProvider.
Attachment #806580 - Flags: review?(bmcbride) → review-
Assignee

Comment 18

6 years ago
I can't figure out how to add a request header to a nsIHttpChannelInternal channel, which seems to be the type of the channel. As far as I can tell I can only add a request header to a nsIHttpChannel channel.
Assignee

Comment 19

5 years ago
Calling QueryInterface(Ci.nsIHttpChannel) adds the setRequestHeader method to the channel...
Attachment #806580 - Attachment is obsolete: true
Attachment #8475918 - Flags: review?(bmcbride)
Comment on attachment 8475918 [details] [diff] [review]
Add Moz-XPI-Update header also to download request for the XPI itself

Review of attachment 8475918 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ +5246,5 @@
>        this.channel.notificationCallbacks = this;
>        if (this.channel instanceof Ci.nsIHttpChannelInternal)
>          this.channel.forceAllowThirdPartyCookie = true;
> +      this.channel = this.channel.QueryInterface(Ci.nsIHttpChannel); // Bug 789732
> +      this.channel.setRequestHeader("Moz-XPI-Update", "1", true);    // " "

As the above check hints, this isn't guaranteed to be a HTTP channel - it could be FTP, for instance.

Rather than explicitly using QueryInterface, you should just be able to use an instanceof check like above, which will (IIRC) implicitly give you that interface.

Also, no need for comments mentioning the bug this was added by - the VCS history handles blame for you.
Attachment #8475918 - Flags: review?(bmcbride) → review-
Assignee

Comment 21

5 years ago
Thanks for your review and comments. I hope I got it right this time... It's my first patch
Attachment #8475918 - Attachment is obsolete: true
Attachment #8478106 - Flags: review?(bmcbride)
Comment on attachment 8478106 [details] [diff] [review]
Use instanceof instead of QueryInterface

Review of attachment 8478106 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good - thanks for this!
Attachment #8478106 - Flags: review?(bmcbride) → review+
Assignee

Comment 23

5 years ago
I think documentation should be added at https://developer.mozilla.org/en-US/Add-ons/Install_Manifests#updateURL and 
https://developer.mozilla.org/en-US/docs/Extension_Versioning,_Update_and_Compatibility#Update_RDF_Format (updateLink). But that should probably wait until the patch is checked in, so it is clear in which Gecko/Firefox version the functionality will be added.
Can we please run this through Try to avoid any unexpected test failures?
Keywords: checkin-needed
Try run pushed:

remote: You can view the progress of your build at the following URL:
remote:   https://tbpl.mozilla.org/?tree=Try&rev=3e770e83af31
remote: Alternatively, view them on Treeherder (experimental):
remote:   https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3e770e83af31
Try run is all green.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/0783f33c0270

Thanks for the Try push, Irving, and thanks for the patch, Onno!
Assignee: nobody → o.e.ekker
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0783f33c0270
Status: REOPENED → RESOLVED
Closed: 7 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.