709648 - add-on does not match the add-on expected: caused by deleting version, & submitting a modified xpi with same version number

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect, P2)

Description

[Reşat SABIQ (Reshat)]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20111104165243

Steps to reproduce:

This issue is probably happening for a number of add-ons. I'm listing only 1 that i'm interested in.
1. Go to https://addons.mozilla.org/en-US/firefox/addon/blend-in
2. Try installing the add-on


Actual results:

Weird error mentioned in the summary. Apparently having something to do with a checksum being wrong on AMO.


Expected results:

Successful install

Comment 1

[Reşat SABIQ (Reshat)]

I'm marking this as major:
a. Apparently 40% or so of users encountered this error and uninstalled the extension, even though the error is entirely AMO's fault, not the extension's.
b. A lot of the remaining users appear to be using the previous version 7.0.1.1, which had a bug in it rendering it dysfunctional. Upgrading to at least version 8.0 is required for those users to regain functionality.
b.1. Because this is a privacy addon, some users might now be have an illusion of improved privacy: illusion because 7.0.1.1 on Firefox is not functional, and they can't upgrade to 8.0 because of a bug on AMO. Logging this bug is the best i can do to help those users out.

Thanks.

Comment 2

[Wil Clouser [:clouserw]]

I didn't hear any other reports about this but we did have major outages last week which may have caused oddities such as this.

This kind of thing happens when the hash AMO gets from the site doesn't match the hash of the file it downloaded.  Hypothetically it could have been preventing something malicious, in reality it was probably something to do with either the outages or if you had just uploaded a new version the site may have still been advertising the old hash while the new file was propogating to mirrors.  If someone got the new file with the old hash it would fail.

I'm marking this as worksforme because of the severity of the outages.  If it happens again please tell us the hash (search for 'data-hash' on the page) and the  file you get.  If you can give us the exact URL to the file you get (after the redirects) that would help also.

Comment 3

[Reşat SABIQ (Reshat)]

(In reply to Wil Clouser [:clouserw] from comment #2)
> I'm marking this as worksforme because of the severity of the outages.  If
> it happens again please tell us the hash (search for 'data-hash' on the
> page) and the  file you get.  If you can give us the exact URL to the file
> you get (after the redirects) that would help also.
For it to be WORKSFORME, you'd need to be able to download and install the addon, wouldn't you? It hasn't worked for a month, and it ain't working now either.

URL:
https://statse.webtrendslive.com/dcsis0ifv10000gg3ag82u4rf_7b1e/dcs.gif?&dcsdat=1324164230130&dcssip=addons.mozilla.org&dcsuri=/firefox/downloads/latest/309621/addon-309621-latest.xpi&dcsref=https://addons.mozilla.org/en-US/firefox/addon/blend-in/versions/&dcsqry=%3Fsrc=dp-btn-primary&WT.co_f=24.94.182.145-4031172096.30178699&WT.vtid=24.94.182.145-4031172096.30178699&WT.vtvs=1324192328518&WT.tz=-6&WT.bh=17&WT.ul=tr&WT.cd=24&WT.sr=1360x768&WT.jo=Yes&WT.ti=Download:%20%20%20%20%20%20%20%20%20%20Add%20to%20Firefox%20%20&WT.js=Yes&WT.jv=1.8&WT.ct=unknown&WT.bs=1360x666&WT.fv=11.1&WT.slv=Not%20enabled&WT.tv=8.6.2&WT.dl=20&WT.ssl=1&WT.es=addons.mozilla.org/en-US/firefox/addon/blend-in/&WT.vt_f_tlh=1324164223&WT.nv=install%20%20clickHijack

wget gives:
--2011-12-17 17:27:21--  https://statse.webtrendslive.com/dcsis0ifv10000gg3ag82u                    4rf_7b1e/dcs.gif?&dcsdat=1324164230299&dcssip=addons.mozilla.org&dcsuri=/firefox                    /downloads/latest/309621/addon-309621-latest.xpi&dcsref=https://addons.mozilla.o                    rg/en-US/firefox/addon/blend-in/versions/&dcsqry=%3Fsrc=dp-btn-primary&WT.co_f=2                    4.94.182.145-4031172096.30178699&WT.vtid=24.94.182.145-4031172096.30178699&WT.vt                    vs=1324192328518&WT.tz=-6&WT.bh=17&WT.ul=tr&WT.cd=24&WT.sr=1360x768&WT.jo=Yes&WT                    .ti=Download:%20%20%20%20%20%20%20%20%20%20Add%20to%20Firefox%20%20&WT.js=Yes&WT                    .jv=1.8&WT.ct=unknown&WT.bs=1360x666&WT.fv=11.1&WT.slv=Not%20enabled&WT.tv=8.6.2                    &WT.dl=20&WT.ssl=1&WT.es=addons.mozilla.org/en-US/firefox/addon/blend-in/&WT.vt_                    f_tlh=1324164230&WT.nv=install%20%20clickHijack
Resolving statse.webtrendslive.com (statse.webtrendslive.com)... 66.150.117.25
Connecting to statse.webtrendslive.com (statse.webtrendslive.com)|66.150.117.25|                    :443... connected.
ERROR: The certificate of `statse.webtrendslive.com' is not trusted.
ERROR: The certificate of `statse.webtrendslive.com' hasn't got a known issuer.

data-hash="sha256:2d4998a058c228a8c5c08f28821ca91182bbd1cd8ae3df9080834054092b5257"
I get the same sha256 on my machine for the file i submitted.

P.S. I've been a fan, but i'm sorry i can't hold this back anymore: Mozilla has been disappointing in the app, and AMO experience since mid this year (and now the same goes for mozilla bugzilla as well). I think rushing releases every 40 days might be one of the reasons for this, not sure though.

Comment 4

[Reşat SABIQ (Reshat)]

I got the above URL from Web Console.
This one is wget with the URL copied from AMO UI (download succeeds, but checksum is wrong, and size is 2 bytes smaller):
1.
--2011-12-17 17:48:47--  https://addons.mozilla.org/firefox/downloads/latest/309621/addon-309621-latest.xpi?src=dp-btn-primary
Resolving addons.mozilla.org... 63.245.217.112
Connecting to addons.mozilla.org|63.245.217.112|:443... connected.
HTTP request sent, awaiting response... 301 MOVED PERMANENTLY
Location: https://addons.mozilla.org/firefox/downloads/latest/blend-in/addon-blend-in-latest.xpi?src=dp-btn-primary [following]
--2011-12-17 17:48:53--  https://addons.mozilla.org/firefox/downloads/latest/blend-in/addon-blend-in-latest.xpi?src=dp-btn-primary
Reusing existing connection to addons.mozilla.org:443.
HTTP request sent, awaiting response... 302 FOUND
Location: https://addons.mozilla.org/firefox/downloads/file/137126/blend_in-8.0-tb+fx.xpi?src=dp-btn-primary [following]
--2011-12-17 17:48:53--  https://addons.mozilla.org/firefox/downloads/file/137126/blend_in-8.0-tb+fx.xpi?src=dp-btn-primary
Reusing existing connection to addons.mozilla.org:443.
HTTP request sent, awaiting response... 302 FOUND
Location: http://releases.mozilla.org/pub/mozilla.org/addons/309621/blend_in-8.0-tb+fx.xpi [following]
--2011-12-17 17:48:53--  http://releases.mozilla.org/pub/mozilla.org/addons/309621/blend_in-8.0-tb+fx.xpi
Resolving releases.mozilla.org... 129.101.198.59, 2001:6b0:e:2018::1337, 2001:4f8:4:b:230:48ff:fedf:7f3a
Connecting to releases.mozilla.org|129.101.198.59|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 6917 (6,8K) [application/x-xpinstall]
Saving to: `addon-309621-latest.xpi?src=dp-btn-primary'

100%[===========================================================================================================================================================================>] 6.917       --.-K/s   in 0,08s  

2. 
f977d7b5d3aac7dc718597a2ddcf3b787efcc9dc4ac8990c16bb8214ebb6991d  addon-309621-latest.xpi?src=dp-btn-primary

3. Size 6917 instead 6919.

P.S. I also didn't get that AMO updated the maxVersion on version 7.0.1 5-6 days ago: there was 7.0.1.1  and 8.0 after it whose maxVersion wasn't updated. I think even the bugs are confused about these bugs.

Comment 5

[Reşat SABIQ (Reshat)]

Wow, i didn't expect this, the xpi served up actually had everything the same as the xpi i submitted, except the .js file in it which was from the prior version 7.0.1.1. Diff:
119,120c119,120
<         var thunderbird = generalPrefs.getCharPref(EXTRA_THUNDERBIRD_CONFIG_NAME);
<         if (thunderbird !=null) {
---
>         try {
>             var thunderbird = generalPrefs.getCharPref(EXTRA_THUNDERBIRD_CONFIG_NAME);
122a123
>
126c127
<         }
---
>         } catch(e) {}        

So, to wrap up:
AMO appears to have merged most of my 8.0. xpi with the js file from 7.0.1.1 xpi and serves it up as my 8.0 xpi. No idea what that's all about. If it wasn't so messed up, i'd probably laugh cause it's actually funny in a twisted kind of way.

Comment 6

[Reşat SABIQ (Reshat)]

I thought about this a bit more, and i think i finally know the root cause. The sha256sum of the file currently served is the same as that of the file that was submitted as version 8.0 initially. A day or two after that initial submission, and before the version was reviewed, a little js bug was fixed, the initial 8.0 version was deleted, and the modified xpi was submitted with the same version number.

What's apparently happened is:
a. the DB has been updated with the sha256sum of the latest xpi file
b. the filesystem(s) serving up the actual xpi file have not been updated with the latest xpi file

The result is a checksum mismatch.

P.S. 
1) I now remember that the thought of something possibly going wrong in this scenario did cross my mind. However, for other addons this workflow worked previously. I don't believe a statement can be made that this workflow always leads to problems. However, if now things have changed and this workflow is no longer supported, this should probably be made known, and a validation should be added that prevents submitting a modified xpi using the same version number as a previously submitted xpi.
2) I think it would also be nice to have a recommendation about what to do in this case. 
2.1) Delete the version, and resubmit the xpi again as same version?
2.2) Delete the version, and resubmit the xpi again as a newer version?
2.3) Don't delete the version, just submit the same xpi again as a newer version?

Regards.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Confirming.

Another example: https://addons.mozilla.org/de/editors/review/geizkragende-sparbar?num=76

I granted preliminary review to 1.1.6 today, the developers deleted that version later and upload a new 1.1.6 xpi. The file download serves the old xpi, the file viewer uses the new one (install.rdf's maxVersion is 8.* or 9.*).

Comment 9

[Andrew Williamson [:eviljeff]]

I haven't got a specific example to hand but I can confirm experiencing it a number of times (and thought it had already been logged on bugzilla)

Comment 10

[Jorge Villalobos [:jorgev] (he/him)]

The drop in usage stats is bug 708539, so I don't think this should be major. It is a problem that shows up every now and then, as Andrew mentioned.

It just seems like we're not saving a hash on a new upload if there's already one in the DB. The hash should always be recalculated and saved when a new file is uploaded.

Comment 11

[Andrew Williamson [:eviljeff]]

(In reply to Archaeopteryx [:aryx] from comment #8)
> Confirming.
> 
> Another example:
> https://addons.mozilla.org/de/editors/review/geizkragende-sparbar?num=76
> 
> I granted preliminary review to 1.1.6 today, the developers deleted that
> version later and upload a new 1.1.6 xpi. The file download serves the old
> xpi, the file viewer uses the new one (install.rdf's maxVersion is 8.* or
> 9.*).

I think this is a different bug actually.  The file persisting when the version is deleting and being served later is different than the hash not being recalculated (or at least the symptom is different - the underlying cause may be connected).

Comment 12

[Reşat SABIQ (Reshat)]

(In reply to Archaeopteryx [:aryx] from comment #8)
> Confirming.
> 
> Another example:
> https://addons.mozilla.org/de/editors/review/geizkragende-sparbar?num=76
> 
> I granted preliminary review to 1.1.6 today, the developers deleted that
> version later and upload a new 1.1.6 xpi. The file download serves the old
> xpi, the file viewer uses the new one (install.rdf's maxVersion is 8.* or
> 9.*).
Thanks.

(In reply to Jorge Villalobos [:jorgev] from comment #10)
> ... I don't think this should be
> major.
When addon usage completely stops for the whole update, and users can either think that the developer might be doing something bad, and or that AMO is not working right, that's a major issue in my book.
I was even thinking that critical might be appropriate, given loss of data (the entire extension update not being unusable, etc.):
critical 	crashes, loss of data, severe memory leak
major 	major loss of function
normal 	regular issue, some loss of functionality under specific circumstances
> It just seems like we're not saving a hash on a new upload if there's
> already one in the DB. The hash should always be recalculated and saved when
> a new file is uploaded.
Per comment 6, and comment 8, the issue that's been confirmed is actually that checksum is updated, and the file on the filesystem(s) isn't.

Comment 13

[Reşat SABIQ (Reshat)]

(In reply to Reşat SABIQ (Reshat) from comment #12)
> When addon usage completely stops for the whole update...
I think i need to clarify this worst-case scenario a bit: version 7.0.1.1 of Blend In addon added support for Thunderbird, but caused a small bug for Firefox installations that made addon unusable on Firefox. Version 8 of Blend-in fixed that, but because of this bug, users never got it. The result is that users of the addon on Firefox, were left with unusable addon from 11/6 until 12/29 (based on version review dates): 53 days. But if this bug had been fixed (in time), the users would have gotten the updated version (with the fix) much sooner, on 11/17: 11 days.

And until this bug is fixed, other addons can and probably are running into the same worst-case scenario.

P.S. Work-around, IF the addon developer is aware of the bug, AND has time on their hands, they can upload a version with an incremented version number that's different from any previously used version (even if it it has the same contents as a previously used version).

Comment 14

[Andrew Williamson [:eviljeff]]

(In reply to Reşat SABIQ (Reshat) from comment #12)
> (In reply to Jorge Villalobos [:jorgev] from comment #10)
> > ... I don't think this should be
> > major.
> When addon usage completely stops for the whole update, and users can either
> think that the developer might be doing something bad, and or that AMO is
> not working right, that's a major issue in my book.
> I was even thinking that critical might be appropriate, given loss of data
> (the entire extension update not being unusable, etc.):
> critical 	crashes, loss of data, severe memory leak
> major 	major loss of function
> normal 	regular issue, some loss of functionality under specific
> circumstances

The severity ratings are for the whole site; so while under certain circumstances it can be a major issue for one add-on, its not a major issue in general.

> > It just seems like we're not saving a hash on a new upload if there's
> > already one in the DB. The hash should always be recalculated and saved when
> > a new file is uploaded.
> Per comment 6, and comment 8, the issue that's been confirmed is actually
> that checksum is updated, and the file on the filesystem(s) isn't.

Its saved somewhere, as we the new version with our internal fileviewer.  It might be cached somewhere else though so the wrong xpi is served.

Comment 15

[Andreas Wagner [:TheOne] [use NI]]

This also affects reviewing add-ons. 
v.1.7 of https://addons.mozilla.org/en-US/editors/review/youtube-netplayed-plugin had been rejected, then deleted by the author and reuploaded with a another xpi but the same version number.

Both the file viewer and the download served the old xpi file.

Should we handle this in a separate bug?

Comment 16

[Jorge Villalobos [:jorgev] (he/him)]

This is becoming a major problem on our side, as many developers like deleting versions and reuploading.

Comment 17

[Wil Clouser [:clouserw]]

Kumar: A lot of the above comments are conjecture but if you use the summary as the steps to reproduce, can you check:

1) That we're serving the new file correctly.  I think our code may be checking for a file existing on disk before copying, which, in this case, wouldn't replace the old file and so would continue serving the old one.  Note that the new one should exist both locally and on the mirror URL so it gets rsync'd out.  We should continue to serve a freshly uploaded file from our servers for the regular MIRROR_DELAY

2) That we're replacing sha hashes as appropriate for newly uploaded files.

I'm not sure if we can roll this into your abstraction or not but since you're in there already you're the best suited to look at this. Thanks.

Comment 18

[Andy McKay]

I think this is a timing issue with the mirror and file name. If I delete a version and reupload a new file, the two files will have the same file name but different hashes. So I can see how it's possible to have a mismatch with mirrors between the file name and hash. I just haven't found a bit in the code that would cause that.

A real quick workaround would be to include the pk of the file row in the filename so instead. But this is a workaround to cope with the other issues. Looking at the timing in a bit more detail.

Comment 19

[Andy McKay]

If a version was deleted from the status and versions page on the devhub the files were not deleted because the signal was not called. If you deleted the file explicitly (click the version, delete the file) then the files were deleted. 

Added in signal so that deleting the version deleting the files and ensures the files get deleted from the mirror.

This should help, but I'd still like to add the pk into the filename.

https://github.com/mozilla/zamboni/commit/02b6cf