Issue 402 - Use a redirector script for downloads, not a direct link

Note that the script in question is being reviewed under http://codereview.adblockplus.org/5747446760079360

[mathias, 2014-09-15 16:15:55]

LGTM.

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
File modules/updateserver/files/site.conf (right):

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
modules/updateserver/files/site.conf:4: location /
Why is the explicit empty location required?

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
File modules/updateserver/files/sitescripts (right):

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
modules/updateserver/files/sitescripts:3:
adblockplusfirefox_repository=%(sitescripts_var_root)s/adblockplus
(Off-topic: If that's the "new" syntax, the `.sitescripts.example` file in the
`sitescripts` repo should reflect that one as well..)

[Wladimir Palant, 2014-09-15 16:46:47]

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
File modules/updateserver/files/site.conf (right):

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
modules/updateserver/files/site.conf:4: location /
On 2014/09/15 16:15:56, matze wrote:
> Why is the explicit empty location required?

That's a very good question. In fact, the official documentation doesn't really
say what happens if a request doesn't match any location whatsoever.
https://www.linode.com/docs/websites/nginx/basic-nginx-configuration#location...
is a bit more explicit but still only recommends: "Make sure each file and
folder under a domain will match at least one location directive." That's
exactly what I did, I didn't bother checking what happens if I leave that
location directive away.

Now that I actually tested this, nginx seems to always generate an implicit
|location / {}| - so removing it here doesn't seem to have any effect. However,
I think that having it here makes things more obvious. Besides, I don't like
relying on undocumented behavior. What do you think?

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
File modules/updateserver/files/sitescripts (right):

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
modules/updateserver/files/sitescripts:3:
adblockplusfirefox_repository=%(sitescripts_var_root)s/adblockplus
On 2014/09/15 16:15:56, matze wrote:
> (Off-topic: If that's the "new" syntax, the `.sitescripts.example` file in the
> `sitescripts` repo should reflect that one as well..)

The syntax didn't really change here, merely the short project names - they
weren't user-visible earlier so I kept them as short as possible. Still, I agree
than we should adapt the example in the sitescripts repo accordingly.

[mathias, 2014-09-15 17:26:05]

LGTM, again.

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
File modules/updateserver/files/site.conf (right):

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
modules/updateserver/files/site.conf:4: location /
Agreed, leave it as it is. But thank you for the clarification, though ;-)

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
File modules/updateserver/files/sitescripts (right):

http://codereview.adblockplus.org/6041976289886208/diff/5629499534213120/modu...
modules/updateserver/files/sitescripts:3:
adblockplusfirefox_repository=%(sitescripts_var_root)s/adblockplus
Ok, I'll take care of that one.

[Wladimir Palant, 2014-09-17 20:00:32]

Turned out there was a typo in the settings. Uploaded a new patchset here,
already pushed that change however.