The default bug view has changed. See this FAQ.

change extensions.update.url to not live behind https://addons.mozilla.org/

VERIFIED FIXED in mozilla1.9beta4

Status

()

Toolkit
Add-ons Manager
P2
normal
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: mrz, Assigned: reed)

Tracking

Trunk
mozilla1.9beta4
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
blocking1.8.1.13 -
wanted1.8.1.x -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
I want to separate VersionCheck to live behind a seperate hostname.  

Currently, it's impossible to separate normal AMO traffic and VersionCheck traffic since both live behind https://addons.mozilla.org/ (L7 content switching is possible but expensive, CPU-wise)

Both have vastly different usage profiles but because both are behind the addons.mozilla.org hostname, both go through the same load balancers and same backend servers.  Because of this, VersionCheck has the ability to take down all of AMO instead of just itself (and it has).  

Marked as blocking Fx3 to get this in for b4, otherwise, as the user base grows, we'll have a harder time scaling AMO and VersionCheck. 

(would love to get this in for Fx 2.0.0.13)

For IT, just need to add a ServerAlias under addons.mozilla.org's vhost.
Flags: blocking-firefox3?
(Assignee)

Updated

9 years ago
Component: Software Update → Extension/Theme Manager
QA Contact: software.update → extension.manager

Comment 1

9 years ago
Just to add my vote of support here, something we should really do before ff3 goes final if possible.  Will help us to scale addons as we grow.  Let us know if there is anything we can do to help test or otherwise to push this through.
(Assignee)

Comment 2

9 years ago
So, what hostname do you want to use?

http://mxr.mozilla.org/firefox/source/browser/app/profile/firefox.js shows that the following subdomains are used for various things that are on or part of AMO:
* https://addons.mozilla.org
* https://services.addons.mozilla.org
* https://%LOCALE%.add-ons.mozilla.com
* https://pfs.mozilla.org
Flags: blocking1.8.1.13?
This would require a late-l10n change to implement. We can either change the url in all locales, or we could just stop localising the pref which I have been wanting for a while since there is no reason why it should be and has caused issues in the past.
(Reporter)

Comment 4

9 years ago
(In reply to comment #2)
> So, what hostname do you want to use?

-not- addons.mozilla.org or anything that is shared with any other service.  

So https://versioncheck.mozilla.org/update/VersionCheck.php...  would be a good guess.
(Reporter)

Comment 5

9 years ago
(In reply to comment #3)
> This would require a late-l10n change to implement. 

I'm just talking about the hostname, not anything to do with l10n.  Change:

https://addons.mozilla.org/update/VersionCheck.php?reqVersion=%REQ_VERSION%&id=%ITEM_ID%&version=%ITEM_VERSION%&maxAppVersion=%ITEM_MAXAPPVERSION%&status=%ITEM_STATUS%&appID=%APP_ID%&appVersion=%APP_VERSION%&appOS=%APP_OS%&appABI=%APP_ABI%&locale=%APP_LOCALE%

to:

https://versionscheck.mozilla.org/update/VersionCheck.php?reqVersion=%REQ_VERSION%&id=%ITEM_ID%&version=%ITEM_VERSION%&maxAppVersion=%ITEM_MAXAPPVERSION%&status=%ITEM_STATUS%&appID=%APP_ID%&appVersion=%APP_VERSION%&appOS=%APP_OS%&appABI=%APP_ABI%&locale=%APP_LOCALE%
The url is currently held in the toolkit locale so it would require changes to locale strings.

Comment 7

9 years ago
Wouldn't this be as simple as shipping with a new about:config entry for extensions.update.url?
(Assignee)

Comment 8

9 years ago
Hmm, update.mozilla.org was used years ago and is now unused... It already points to AMO. You could reuse it, possibly. :)

(In reply to comment #5)
> (In reply to comment #3)
> > This would require a late-l10n change to implement. 
> 
> I'm just talking about the hostname, not anything to do with l10n.  Change:

Yes, we know, but it does have to do with l10n, as the locales have this preference localized (for some reason). It can't be easily changed globally without either changing all the locales or changing the pref to be non-localized.

See http://mxr.mozilla.org/l10n/source/en-GB/toolkit/chrome/mozapps/extensions/extensions.properties#43 for an example. :(
I would certainly prefer making the preference a regular one so we don't have to alter every locale everytime we want to change it, but even that is an l10n change since we would no longer be using a string.

Thinking though, given that this is in the locale in Firefox 2, I'm not sure what the rules are on being able to change it there, Pike?
(Assignee)

Comment 10

9 years ago
(In reply to comment #9)
> I would certainly prefer making the preference a regular one so we don't have
> to alter every locale everytime we want to change it, but even that is an l10n
> change since we would no longer be using a string.
> 
> Thinking though, given that this is in the locale in Firefox 2, I'm not sure
> what the rules are on being able to change it there, Pike?

What would it matter, as you would just ignore the localized version of the string? You could, of course, remove it on the trunk, but wouldn't you be able to just ignore it on the branch and fallback to the firefox.js version of the pref?

Comment 11

9 years ago
Just another vote here - we should do this for beta4

Comment 12

9 years ago
I think that we could perfectly remove that string for Firefox and Thunderbird, but I remember that this pref was asked to stay in l10n by the non-moco projects, i.e., SeaMonkey. Not sure, I might be confusing the pref, too.

Anyway, from a late-l10n perspective, I have no problems with removing strings, as that's not going to turn any build broken, the worst thing happening is leaving in dead code in a localization.

As for SeaMonkey, CCing KaiRo, they might be able to just set the pref to something localizable for SeaMonkey, and we use a static pattern.
It's sort of paradoxical, but not using a localized string actually makes it easier for SeaMonkey to localize - either way, they have to store their localized URL in a different .properties file, because a locale that uses a different site for SM will still be using AMO for Fx. If Toolkit doesn't have a localized string then SM changes their pref from a URL-pattern to a chrome URI for their .properties file, and puts the URL-pattern in there, and the url formatter deals, while if Toolkit provides a localized string, then because nobody can actually use it, SM changes their pref from one chrome URI to another chrome URI, and every locale that does both Fx/Tb and SM has to have two different URL patterns, one of which they can change and one of which they are absolutely required to not ever touch. Either way, SM has to do the exact same thing, the difference is just that removing the localized string from Toolkit makes it less confusing.

Comment 14

9 years ago
If it's going through urlformatter, then I think it's no problem in any way, as the app can override the pref, and the pref in the app can always be a localized pref, as urlformatter handles that automagically, allowing it to be either.
IMHO, this should be set together with the pref'd urlformatted values for "Get *" links in EM, so that any app can easily set their own service instead of AMO - I guess Songbird and others probably want that anyways.
(In reply to comment #14)
> IMHO, this should be set together with the pref'd urlformatted values for "Get
> *" links in EM, so that any app can easily set their own service instead of AMO
> - I guess Songbird and others probably want that anyways.

All of the getExtensions etc. prefs are currently plain prefs in the application's preference files (they used to be localised but changed fairly recently).

So from what I see the general feeling is that we can just switch to an app pref for the url. That works for seamonkey and other apps and schrep and pike approve so getting approval for the l10n change shouldn't be an issue.

So decide on the final hostname, get it set up and the patch to do this is simple.

(Reporter)

Comment 16

9 years ago
(In reply to comment #15)
> So decide on the final hostname, get it set up and the patch to do this is
> simple.

For the sake of being verbose, versioncheck.mozilla.org works for me.  Sort of identifies what it's being used for (you have no idea how many people think we're attacking them when they see responses from fxfeeds.mozilla.org!).
Who is the authority on URLs?  If no one has any complaints about the URL in Comment #16 we can make this happen right away.
If we're only using this for checking the versions of add-ons it seems more appropriate to use versioncheck.addons.mozilla.org.  That would be more consistent with the current URLs in comment 2 and the unmentioned facebook.a.m.o.
Works for me if we have the *.addons.mozilla.org cert.

Initially the setup should just be:

versioncheck.addons.mozilla.org CNAME addons.mozilla.org

Add versioncheck.addons.mozilla.org to ServerAlias in the addon.mozilla.org vhost.

Does that sounds correct to everyone?
(Reporter)

Comment 20

9 years ago
(In reply to comment #19)
> Works for me if we have the *.addons.mozilla.org cert.
> 
> Initially the setup should just be:
> 
> versioncheck.addons.mozilla.org CNAME addons.mozilla.org

That wouldn't work - the cert under addons.mozilla.org is *.mozilla.org, not *.addons.mozilla.org. Needs to be a seperate VIP but with the same backend servers (the vhost addition is still valid).

The interesting thing with the seperate VIP is that we can actually see how  much traffic version check is vs. just AMO.
Flags: blocking-firefox3? → blocking-firefox3+
(Reporter)

Updated

9 years ago
Blocks: 416550
(Assignee)

Updated

9 years ago
No longer blocks: 416550
Depends on: 416550
(Assignee)

Updated

9 years ago
Summary: change extension.update.url to not live behind https://addons.mozilla.org/ → change extensions.update.url to not live behind https://addons.mozilla.org/
(Assignee)

Comment 21

9 years ago
Created attachment 302322 [details] [diff] [review]
trunk patch - v1

Mossop, something like this?
Assignee: nobody → reed
Status: NEW → ASSIGNED
Attachment #302322 - Flags: review?(dtownsend)
(Assignee)

Comment 22

9 years ago
Comment on attachment 302322 [details] [diff] [review]
trunk patch - v1

>-      var dsURI = gPref.getComplexValue(PREF_EM_ITEM_UPDATE_URL.replace(/%UUID%/, aItem.id),
>-                                        Ci.nsIPrefLocalizedString).data;
>+      var dsURI = gPref.getCharPref(PREF_EM_ITEM_UPDATE_URL.replace(/%UUID%/, aItem.id);

Pretend there's an extra ) at the end of that line. :)
Comment on attachment 302322 [details] [diff] [review]
trunk patch - v1

Almost but you're overthinking it. We don't want to be unlocalizing the PREF_EM_ITEM_UPDATE_URL case
Attachment #302322 - Flags: review?(dtownsend) → review-
(Assignee)

Comment 24

9 years ago
Created attachment 302337 [details] [diff] [review]
trunk patch - v2
Attachment #302322 - Attachment is obsolete: true
Attachment #302337 - Flags: review?(dtownsend)
(Assignee)

Comment 25

9 years ago
Created attachment 302339 [details] [diff] [review]
trunk patch - v2.1

No, I don't hate SeaMonkey.
Attachment #302337 - Attachment is obsolete: true
Attachment #302339 - Flags: review?(dtownsend)
Attachment #302337 - Flags: review?(dtownsend)
Comment on attachment 302339 [details] [diff] [review]
trunk patch - v2.1

>Index: toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>     if (!dsURI)
>       dsURI = aItem.updateRDF;
>     if (!dsURI) {
>-      dsURI = gPref.getComplexValue(PREF_UPDATE_DEFAULT_URL,
>-                                    Ci.nsIPrefLocalizedString).data;
>+      dsURI = gPref.getCharPref(PREF_UPDATE_DEFAULT_URL);
>     }

Nit: drop the braces around the single line statement.

r+ from me assuming the hostname is correct, if an IT guy could confirm that explicitly here it'd be good.

Please do not check in until the service is actually up and running.
Attachment #302339 - Flags: review?(dtownsend) → review+
(Assignee)

Comment 27

9 years ago
Comment on attachment 302339 [details] [diff] [review]
trunk patch - v2.1

Requesting approval since this bug has late-l10n. Pike has said before that string removals aren't really considered late-l10n (see bug 388652, comment #13), but since he's on vacation, I'll just go ahead and request approval just in case.
Attachment #302339 - Flags: approval1.9?
Comment on attachment 302339 [details] [diff] [review]
trunk patch - v2.1

a=beltzner
Attachment #302339 - Flags: approval1.9? → approval1.9+
(Reporter)

Comment 29

9 years ago
(In reply to comment #26)
> r+ from me assuming the hostname is correct, if an IT guy could confirm that
> explicitly here it'd be good.

Agreed - versioncheck.addons.mozilla.org will be setup.
IT - can you let me know when you plan on doing the switchover so I can be around?
The VIP will be active right away, but the actual switchover will happen when this patch is released.

Updated

9 years ago
Priority: -- → P2
(Reporter)

Comment 32

9 years ago
I'd like to see this pushed in for Fx 2.0.0.13 if possible.  Minor updates in the Fx2 cycle are still going to be a pain point that IT will have to deal with until Fx3 is out the door and users have moved off Fx2.  
Flags: wanted1.8.1.x?

Comment 33

9 years ago
(In reply to comment #32)
> I'd like to see this pushed in for Fx 2.0.0.13 if possible.  Minor updates in
> the Fx2 cycle are still going to be a pain point that IT will have to deal with
> until Fx3 is out the door and users have moved off Fx2.  
> 

I'd prefer not.  FF3 is coming and folks will move fast.  Trust me.  
Glad I saw a mention of this new URL in IRC scrollback and looked for a bug...

This will break automatic add-on detection (currently only used by Rock Your Firefox) due to not being able to use the addons.mozilla.org cookie and may also require update ping counter script changes depending on if IT setup the logs to go to the same place they're going now under addons.mozilla.org. I briefly tested to see if a query string I entered in versioncheck.amo appeared in the logs and couldn't find it.

I can fix the RYF detection next week, but that means a couple RYF pages will have to be served from versioncheck.addons.mozilla.org and some netscaler cache rules may have to be changed dealing with that cookie.
(Assignee)

Comment 35

9 years ago
fligtar, are there bugs tracking the issues you mentioned so I will know when it is safe for me to land this?
Depends on: 417248
(In reply to comment #35)
> fligtar, are there bugs tracking the issues you mentioned so I will know when
> it is safe for me to land this?
> 

Filed bug 417248 for the RYF changes. Can someone in IT comment as to whether the versioncheck.addons.mozilla.org logs are going somewhere new or to addons.mozilla.org as usual? If they are going to the same place, there's nothing to be done there.
They will be going to /data/stats/logs/im-log01/versioncheck.addons.mozilla.org and /data/stats/logs/nladm01/versioncheck.addons.mozilla.org.
So we'll need to add 2 more counter crons for the AMO update ping counter and make similar changes to your counter, right?
Depends on: 418919
It would be really nice to get this in for l10n code freeze for b4, but not required.
Target Milestone: --- → Firefox 3 beta4
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Comment 40

9 years ago
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.281; previous revision: 1.280
done
Checking in calendar/sunbird/app/profile/sunbird.js;
/cvsroot/mozilla/calendar/sunbird/app/profile/sunbird.js,v  <--  sunbird.js
new revision: 1.53; previous revision: 1.52
done
Checking in composer/app/profile/composer-prefs.js;
/cvsroot/mozilla/composer/app/profile/composer-prefs.js,v  <--  composer-prefs.js
new revision: 1.4; previous revision: 1.3
done
Checking in mail/app/profile/all-thunderbird.js;
/cvsroot/mozilla/mail/app/profile/all-thunderbird.js,v  <--  all-thunderbird.js
new revision: 1.108; previous revision: 1.107
done
Checking in suite/browser/browser-prefs.js;
/cvsroot/mozilla/suite/browser/browser-prefs.js,v  <--  browser-prefs.js
new revision: 1.83; previous revision: 1.82
done
Checking in toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v  <--  extensions.properties
new revision: 1.50; previous revision: 1.49
done
Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.275; previous revision: 1.274
done
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
We agree w/Schrep, not blocking FF2 releases.
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x-
Flags: blocking1.8.1.13?
Flags: blocking1.8.1.13-
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4) Gecko/2008030317 Firefox/3.0b4

extensions.update.url = https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=%REQ_VERSION%&id=%ITEM_ID%&version=%ITEM_VERSION%&maxAppVersion=%ITEM_MAXAPPVERSION%&status=%ITEM_STATUS%&appID=%APP_ID%&appVersion=%APP_VERSION%&appOS=%APP_OS%&appABI=%APP_ABI%&locale=%APP_LOCALE%
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.