Disallow calling WebIDL constructors as functions on the web

RESOLVED FIXED in mozilla30

Status

()

defect
RESOLVED FIXED
6 years ago
4 months ago

People

(Reporter: bzbarsky, Assigned: bholley)

Tracking

({dev-doc-complete, site-compat})

unspecified
mozilla30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 30+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

See https://www.w3.org/Bugs/Public/show_bug.cgi?id=22808

We can't just do this: our chrome uses this all over.  Including the addon SDK.  A try push with this change is beautifully orange.

So I'm going to see if we can disable this for non-chrome callers only, for now and then get our tests passing.  And if we can, I will then reenable it for non-chrome code, add telemetry, and see what we see in the wild.
https://tbpl.mozilla.org/?tree=Try&rev=f2d188af713c says failures in M1, M2, M3, M5, mochitest-browser-chrome, mochitest-chrome, crashtest, crashtest-ipc, jetpack tests.  And probably the robocop tests (rc2) on Android 2.2.
Attachment #8378065 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Assignee: nobody → bobbyholley
Assignee

Updated

5 years ago
Depends on: 976105
Assignee

Updated

5 years ago
Summary: Measure whether we can disallow calling WebIDL constructors as functions → Disallow calling WebIDL constructors as functions on the web
Assignee

Comment 6

5 years ago
I DeMorgan-ified the previous patch because I was finding it very difficult to
grok, even though I understood the change it was making.
Attachment #8381519 - Flags: review?(bzbarsky)
Assignee

Updated

5 years ago
Attachment #8378066 - Attachment is obsolete: true
Comment on attachment 8381519 [details] [diff] [review]
Disable invoking WebIDL constructors without |new| unless you have the system principal. v2

r=me, but before you can land this you need to fix the gaia code that depends on the current behavior, no?

At the very least, http://mxr.mozilla.org/gaia/search?string=TextEncoder%28&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=gaia shows hits in the mail app, and similar for TextDecoder.  Not sure about other constructors that are worth checking for (e.g. Event() is really hard to search for).

If this passed tests, that mostly tells us that b2g test coverage is terrible, which is not a surprise.  :(
Attachment #8381519 - Flags: review?(bzbarsky) → review+
Assignee

Comment 9

5 years ago
(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #8)
> Comment on attachment 8381519 [details] [diff] [review]
> Disable invoking WebIDL constructors without |new| unless you have the
> system principal. v2
> 
> r=me, but before you can land this you need to fix the gaia code that
> depends on the current behavior, no?

Yeah. The try push in comment 7 is orange on Gaia, though probably not as orange as it should be. I'm looking into it, but it's all bundled up in sinon.js stuff.
Assignee

Updated

5 years ago
Depends on: 976935
Assignee

Updated

5 years ago
Depends on: 976937
Assignee

Comment 10

5 years ago
Try push with a custom gaia including my changes from the dependent bugs:

https://tbpl.mozilla.org/?tree=Try&rev=0bf700091431
Assignee

Comment 13

5 years ago
(In reply to Bobby Holley (:bholley) from comment #12)
> https://tbpl.mozilla.org/?tree=Try&rev=05180848d19c

The gaia stuff here is green with my patches.
Assignee

Comment 14

5 years ago
Gaia changes should be merged in now. Try push without custom gaia:

https://tbpl.mozilla.org/?tree=Try&rev=6831a6cfbebc
Assignee

Comment 15

5 years ago
Looks like Gaia bumps don't git m-c as frequently as I'd thought. Should be there by now:

https://tbpl.mozilla.org/?tree=Try&rev=33eb326fcc42
https://hg.mozilla.org/mozilla-central/rev/de377011ed9d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
We probably want a followup for fixing the system-caller behavior too...
Assignee

Updated

5 years ago
Blocks: 979591
Blocks: 981036
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.