Closed Bug 368117 Opened 18 years ago Closed 17 years ago

AUS should accept version 3 URLs

Categories

(AUS Graveyard :: General, defect)

Product:
AUS Graveyard
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
4.x (triaged)

People

(Reporter: morgamic, Assigned: morgamic)

References

Details

Attachments

(1 file)

v1, added passthru for schema3 with acceptance tests
7.73 KB, patch
preed
: review+
oremj
: review+
Details | Diff | Splinter Review 
AUS cannot accept version 2 URLs that contain OS_VERSION information from 1.5.* clients as of bug 360127.  Bug 341190 is a bug that calls for OS_VERSION information from those clients.

In order to allow for this new information, the URL version would have to be bumped to 3.  See bug 341190 comment #46.
mike, thanks for logging this bug.

as stated in today's meeting:

if we want bug #341190 on the 1.5.0.x branch (and we do), we'd need the AUS side to support version 3 urls first.

otherwise, 1.5.0.x users would be blocked, by the fix for bug #360127, from getting updates.

it doesn't look like this is going to happen for 1.5.0.10, but hopefully for 1.5.0.11.

the reason not for 1.5.0.10 is that with the major update stuff, now is not the time to twiddle AUS.

i'd argue that we should switch everything to version 3 for 1.5.0.11, fx 2.0.0.x, and the trunk.  (morgamic, what do you think?)  if we all agree, I can log a new bug about version 3 for all branches, which would include backporting bug #341190 to the MOZILLA_1_8_0_BRANCH (for 1.5.0.11)
Blocks: 341190
> I can log a new bug about version 3 for all branches

see bug #368200
Mike,

For the upcoming fx distributions work, I'd like to modify the aus update url to include two extra fields: distribution, and distribution version.

So I'd like to use version 3 of the url for that (note that the previous client bug to bump the version to 3 with no other changes has been wontfixed).

We don't need to do anything on the server-side, other than accept version 3 and ignore the two extra fields for now.

What do you think?
Sounds fine -- I can have a patch by Friday.  Does that work?
Blocks: 392496
That would work great, thanks!

Bug 392496 is the client-side of this, btw.
Hey Mike,
When do you expect this change to land?  The client-side patch is ready to go, and I'd like it to land for M8 if possible.
Hey Dan, sorry I'm late on this.  I have a patch ready to go, working on tests.
Status: NEW → ASSIGNED
Tests that were added cover the schema 3 URL, and verifies that when re-running the basic update test case we can assume the same output.
Attachment #280004 - Flags: review?
Comment on attachment 280004 [details] [diff] [review]
v1, added passthru for schema3 with acceptance tests

Paul can you take a quick look?
Attachment #280004 - Flags: review? → review?(preed)
Comment on attachment 280004 [details] [diff] [review]
v1, added passthru for schema3 with acceptance tests

Asking oremj for a quick review -- build is pretty overloaded this week.
Attachment #280004 - Flags: review?(preed) → review?(oremj)
Comment on attachment 280004 [details] [diff] [review]
v1, added passthru for schema3 with acceptance tests

nm, preed was already looking at it :)
Attachment #280004 - Flags: review?(oremj) → review?(preed)
Comment on attachment 280004 [details] [diff] [review]
v1, added passthru for schema3 with acceptance tests

Looks good; on the test side, do we want to do any validation of... well... anything in particular? Maybe that version 3 URLs have a distribution channel set? (Not as familiar with the PyFit syntax... as I probably should be.)
Attachment #280004 - Flags: review?(preed) → review+
Right now, the app has no logic for using dist or distVersion, but when finding the vars from parsing the URL, it already does an existence check.

When we add dist/distVersion logic later we can add whatever business rules are appropriate and test those.  That sound good for now?
Comment on attachment 280004 [details] [diff] [review]
v1, added passthru for schema3 with acceptance tests

I'm not super familiar with this code, but everything looks pretty straightforward and correct.
Attachment #280004 - Flags: review+
sorry for jumping in (where not invited), but I have a question:

does this patch treat the following two scenarios as a frankenfox, and return an empty snippet (or log frankenfox errors, what ever we are doing these days):

https://aus2.mozilla.org/update/3/Firefox/3.0a8pre/2007083015/Darwin_x86-gcc3/en-US/default/Darwin%208.10.1/update.xml?force=1 (version 3, but missing both new fields)

https://aus2.mozilla.org/update/3/Firefox/3.0a8pre/2007083015/Darwin_x86-gcc3/en-US/default/Darwin%208.10.1/XXX/update.xml?force=1 (version 3, but missing one of the new fields)
(In reply to comment #15)
> does this patch treat the following two scenarios as a frankenfox, and return
> an empty snippet (or log frankenfox errors, what ever we are doing these days):
No.  Are we expecting frankenfox to exist past the initially affected versions?  I thought it was a one-off fix.

Have you verified that these URLs are possible?

So I can add logic to stop the update if:
- schema is 3 but no dist OR distVersion exist
Seth, so quick question is that with M8 ready to go, do we want to hold this patch up for frankenfox or deal w/ frankenfox later?
Mike, thanks for answering my comment.

> Are we expecting frankenfox to exist past the initially affected versions?

No, we are not expecting more frankenfoxen.  (But, then again, we weren't expecting it the first time.)

> Have you verified that these URLs are possible?

Good point.  

They would be possible if someone created a firefox build and set the aus url pref to something "bad".  This happened with some frankenfoxen that were cck builds created by ibm.

But, these frankenfoxen could happen (

https://aus2.mozilla.org/update/3/Firefox/1.5.0.x/2007083015/Darwin_x86-gcc3/en-US/default/Darwin%208.10.1/default/default/update.xml?force=1

(version 3 url with version 1.5.0.x of the browser)

https://aus2.mozilla.org/update/2/Firefox/3.0/2007083015/Darwin_x86-gcc3/en-US/default/Darwin%208.10.1/update.xml?force=1

a version of firefox 3 but with aus url version 2.  

But note, that would be legal for 3.0apre builds. (any trunk builds before thunder's fix.)

> do we want to hold this patch up for frankenfox or deal w/ frankenfox later?

No, I would not hold up this patch for frankenfox.  But we may want to log a spin off bug and deal with it later.

Does that seem reasonable?
Seth, that seems reasonable.  I filed bug 396073 to track the frankenfox stuff.

As for the basic schema 3 patch, that's landed on AUS2_STAGING.  I'm going to to a comparison test between prod/staging and talk to stephend before pushing to prod.
> I filed bug 396073 to track the frankenfox stuff.

thanks for logging that bug and for fixing this one.

Are we holding up m8 for this fix and the fix for bug #392496?
Hey guys, did my tests after I figured out why aus2-staging was messed up.  It was throttling! *tears of poetic coincidence*

So... we have stage vs. prod comparison using random URL sample set using v2 URLs:
http://people.mozilla.com/~morgamic/v3_patch_v2_urls.txt

To me this looks fine, and running spot checks on v3 URLs yields the expected: they work on stage but not in prod.

So, I'm for pushing this Friday AM unless anybody objects (bit late right now).  Thoughts?

I've got this tagged for production and it's ready to go -- can someone from QA or build sign off on this?
Talked to build and we're going to push this live.  Prod push bug is 397516.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
(In reply to comment #24)
> Talked to build and we're going to push this live.  Prod push bug is 397516.

Is there a reason why the push bug is in a confidential group? Seems like it should be open so if there's a problem, it would be easy to see.
It was just conf by default.  Wasn't intentional and I can't remove it.  Will look next time.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: