AddOn-Validation never finishes when submitting a new AddOn

RESOLVED FIXED in 6.3.9

Status

addons.mozilla.org Graveyard
Add-on Validation
P1
major
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: Florian Paul Schmidt, Assigned: basta)

Tracking

({regression})

unspecified
6.3.9
regression

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.1 (KHTML, like Gecko) Ubuntu/11.04 Chromium/14.0.835.202 Chrome/14.0.835.202 Safari/535.1

Steps to reproduce:

I created a new extension in a file called easycrypt.xpi. Then i headed over to the mozilla add on site and tried to submit the new addon. 


Actual results:

Step 2, the automated validation after uploading the xpi never finished with a result. This happened every time I tried submitting the addon over the last few days


Expected results:

The Validation should show a result, e.g. that the validation succeeded or that it failed
(Reporter)

Comment 1

6 years ago
Created attachment 578895 [details]
The XPI that i tried to submit
(Reporter)

Comment 2

6 years ago
I tried waiting for up to an hour for the validation to finish. Never happened..
(Reporter)

Comment 3

6 years ago
I just tried it using the "standalone" addon validator, i.e. not during the submit process.. Same result.

Comment 4

6 years ago
This is happening for me too!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 707497
Duplicate of this bug: 707509

Updated

6 years ago
Severity: normal → major
Keywords: regression
Sorry for the inconvenience and thanks for the bug reports. Some task workers were down and now they're back up but they have a *lot* of queued tasks to work on. Please hold off on re-validating until the machines have caught up (should happen soon).  bug 707522 has some details.
Addon validation is fixed for me. Please re-open or report back if you continue to have problems.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Comment 9

6 years ago
Problem persists for me. But now i at least sometimes get an error message:

Your add-on failed validation with 1 error.
There was a problem contacting the server.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 10

6 years ago
(In reply to Florian Paul Schmidt from comment #9)
> Problem persists for me. But now i at least sometimes get an error message:
> 
> Your add-on failed validation with 1 error.
> There was a problem contacting the server.

Same problem here.
Hi all, addon validation should be mostly back to normal now.

However, the easycrypt.xpi is triggering a bug in our system that uses a lot of resources. We have a timeout in place now in case resource consumption gets too high.  Matt is working on a real fix for validating addons like easycrypt.xpi. More info as it happens.
Assignee: nobody → mattbasta
Priority: -- → P1
Target Milestone: --- → 6.3.4

Comment 12

6 years ago
(In reply to Kumar McMillan [:kumar] from comment #11)
> Hi all, addon validation should be mostly back to normal now.

Still not working! When I try upload new addon I see:

Your add-on failed validation with 1 error.
Received an empty response from the server; status: 500

This is very annoying.
(Reporter)

Comment 13

6 years ago
Hi, as the author of said easycrypt.xpi I am dying to know what evil thing my addon does that fubars AMO :D Feel free to remove this comment if it is inappropriate and contact me via e.g. email if you feel so inclined..
I can't tell you what it does to stall the validator (except that on my machine it causes it to consume vast amounts of RAM and bring my system to its knees), but I can tell you that you should be using the builtin crypto functionality rather than including third party, pure-JS libraries.
Incidentally, before you do submit, you need to clean up any unused files and duplicate files (minified versions of the crypto library you're not using and editor backup files), and to load the crypto library, if you're insistent on continuing to use it, into a private namespace via the subscript loader.
(Reporter)

Comment 16

6 years ago
OK, thanks for the comments. I will continue to use the crypto-js library, but will do as you recommended about stripping the xpi and using the subscript loader..
(In reply to Leszek Zyczkowski from comment #12)
> Received an empty response from the server; status: 500

This probably means that your addon is maxing out validation too. Leszek, can you attach your xpi so we can take a look while fixing the issue?

To clarify my comment from earlier about mostly back to normal: addon validation should be working as long as you're not triggering the bug in our traverser.
Created attachment 579704 [details]
Foxtrick 0.8

I ran the mobile compatibility bump today and got this result in the Passes list:
https://addons.mozilla.org/en-US/developers/addon/foxtrick/validation-result/89498
Kumar asked me to post it here.
(Reporter)

Comment 19

6 years ago
Just letting you guys know that after the recommended changes it validates and submits OK.. Thanks for the help..

Comment 20

6 years ago
(In reply to Kumar McMillan [:kumar] from comment #17)

> This probably means that your addon is maxing out validation too. Leszek,
> can you attach your xpi so we can take a look while fixing the issue?

When I adding my comment validator when I check out addon before upload working, when I try upload addon to AMO validator displayed reported bug.

Now working properly.
(In reply to Jorge Villalobos [:jorgev] from comment #18)
> Created attachment 579704 [details]
> Foxtrick 0.8
> 
> I ran the mobile compatibility bump today and got this result in the Passes
> list:
> https://addons.mozilla.org/en-US/developers/addon/foxtrick/validation-result/
> 89498

Huh? This should not have been in the passing result list! I filed bug 708501

Comment 23

6 years ago
Same issue: http://screencast.com/t/bhQRgU6HrC attaching XPI as well.

Comment 24

6 years ago
Created attachment 580661 [details]
Integrated Gmail 2.8 (Fails to validate, server errror)
It should be noticed that older versions (already uploaded and validated) don't validate anymore, failing with a timeout. 
Example: https://addons.mozilla.org/en-US/developers/addon/noscript/file/138376/validation

Is there any ETA for this bug?
If not, is there any work-around to be implemented in the XPI not to trigger it?
This seems to be affecting a lot of prominent add-ons. Can we roll back to a working version of the validator until this is fixed?
(Assignee)

Comment 27

6 years ago
What's concerning is that--besides the FX10 compat stuff--there haven't been any major changes to the JS engine recently. I don't know what version we would roll back to.

I'm going to be investigating this tomorrow and hopefully will have a solution.
(In reply to Kris Maglione [:kmag] from comment #26)
> This seems to be affecting a lot of prominent add-ons. Can we roll back to a
> working version of the validator until this is fixed?

Just to clarify, there are two issues going on.

One is that the first add-on attached to this bug causes an infinite loop in the validator. I couldn't find an easy rollback point to stop that from happening. Matt is working on a fix for that asap.

The second issue is that we limited validation runs to 60 seconds to prevent the infinite loop from happening. My hunch is that all these other bug reports are for validation runs that are taking longer than 60 seconds but do not result in an infinite loop. We can mitigate some of that by increasing the timeout. I'll make the config change for that tomorrow (Monday).
The timeout was just increased from 30 seconds to 90 seconds in production while we work on the real fix.

Comment 30

6 years ago
That has resolved the issue for me. Thanks!
Target Milestone: 6.3.4 → 6.3.6
(Assignee)

Comment 31

6 years ago
https://github.com/mozilla/amo-validator/pull/97

I traced the issue back to a line of code that deals with JS arrays. JS has a (horrible) feature that lets you assign an arbitrary value like so:

var x = [];
x[4] = 1;

x is then equal to [null, null, null, null, 1]

The validator dealt with this by giving arrays null JSWrappers for each empty value (JSWrapper(value=None)). Where the breakdown occurred was in des_min.js, which was arbitrarily setting the value of an array at an index (which the static analysis had determined to be a likely value for the index) somewhere around 250,000,000.

The fix was to cap the indexes that can be assigned in this case to 100,000 and to use None instead of JSWrapper(value=None).
Just to be pedantic, that's not strictly true. JS arrays are not real arrays. They're ordinary JS objects with a few array-ish methods in their prototype and a length property which always returns the value of the highest numerical property name + 1. Assigning to a numerical value beyond the length of the array doesn't create any new properties, or even behave as if it had done so. for (x in array) still gives you only one iteration, x=4; map(), forEach(), and so forth only give you iterations for the indices actually in the array.
(Assignee)

Comment 33

6 years ago
That may be so, but converting an array to a string will return the array as if it does indeed contain that may entries:

var x = []
x[4] = 1;

x + "" would yield ",,,,1", which is why we treat them like actual arrays. I suppose it would be wise to convert them to an implementation based more on what *actual* JS interpreters use than how the arrays behave.
Target Milestone: 6.3.6 → 6.3.7
Target Milestone: 6.3.7 → 6.3.8
will get the pull request looked at this week
Target Milestone: 6.3.8 → 6.3.9
(Assignee)

Comment 35

6 years ago
Merged:

https://github.com/mozilla/amo-validator/commit/c2d332c7e47df24001c6e45329b06c51cc6a3606
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.