"Dangerous Global Object" should be a warning and not a failure

VERIFIED FIXED in 5.12.6

Status

addons.mozilla.org Graveyard
Developer Pages
VERIFIED FIXED
8 years ago
2 years ago

People

(Reporter: krupa, Assigned: basta)

Tracking

Details

(Whiteboard: [Step 2], URL)

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
Created attachment 500119 [details]
test-file

steps to reproduce:
1. Upload the test-file
2. Notice that validation fails with error message: "Dangerous Global Object"

Validation error details:
Error: A dangerous or banned global object was accessed by some JavaScript code.
Error: Accessed object: Function


expected behavior:
This should be a warning and not a failure

Also:
The test-file passes validation in remora
(Assignee)

Comment 1

8 years ago
Should all dangerous globals be warnings? E.g.:

var x = window.eval;
Object.prototype.foo = "bar";
Number = {asdf:1234};

All of these would otherwise throw errors. Should these also be warnings?
Jorge is the man to answer that.  Jorge - comment 1?
(In reply to comment #1)
> Should all dangerous globals be warnings? E.g.:
> 
> var x = window.eval;
> Object.prototype.foo = "bar";
> Number = {asdf:1234};
> 
> All of these would otherwise throw errors. Should these also be warnings?

Yes.

Keep in mind that some of this code could be inserted and executed in the web content space and not the chrome space. Not that it's a good practice even then, but at least it doesn't break Firefox or other add-ons and it's allowed.
(Assignee)

Comment 4

8 years ago
Here's the update:

https://github.com/mattbasta/amo-validator/commit/fe38db3b2f68041e780c339e19beb8119dca2226

There aren't any more errors, they're all warnings.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 5

8 years ago
When I upload the test file, I do not see any warnings about 

Dangerous Global Object
Error: A dangerous or banned global object was accessed by some JavaScript code.
Error: Accessed object: Function

See https://addons.allizom.org/z/en-US/developers/upload/e0e1033e7ba9411ba01d146fa5ceb104
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

8 years ago
Some old-ish code was freaking out and causing problems. Everything should be fixed now:

https://github.com/mattbasta/amo-validator/commit/17ed96b87496149dcf2e206c2f67e9fd5421df0b
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

8 years ago
verified fixed @ https://addons.allizom.org/z/en-US/developers/upload/db443772e7c0499688c533246295ee00
Status: RESOLVED → VERIFIED
(Reporter)

Comment 9

8 years ago
Created attachment 502102 [details]
post-fix screenshot
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.