916644 - Disallow calling WebIDL constructors as functions on the web

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: hackery. Disallow invoking WebIDL constructors without "new" unless you have the system principal.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: hackery. Disallow invoking WebIDL constructors without "new" unless you have the system principal.

Comment 4

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=ebdbc851ae3d

Comment 5

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=14e707d8688c

Comment 6

[Bobby Holley (:bholley)]

Attachment: Disable invoking WebIDL constructors without |new| unless you have the system principal. v2

I DeMorgan-ified the previous patch because I was finding it very difficult to
grok, even though I understood the change it was making.

Comment 7

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=27919b9e2a5d

Comment 8

[Boris Zbarsky [:bzbarsky]]

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.  :(

Comment 9

[Bobby Holley (:bholley)]

(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.

Comment 10

[Bobby Holley (:bholley)]

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

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

Comment 11

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=728bf49ceb83

Comment 12

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=05180848d19c

Comment 13

[Bobby Holley (:bholley)]

(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.

Comment 14

[Bobby Holley (:bholley)]

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

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

Comment 15

[Bobby Holley (:bholley)]

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

Comment 16

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de377011ed9d

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/de377011ed9d

Comment 18

[Boris Zbarsky [:bzbarsky]]

We probably want a followup for fixing the system-caller behavior too...

Comment 19

[Kohei Yoshino]

https://developer.mozilla.org/en-US/Firefox/Releases/30/Site_Compatibility