1582911 - [LenientThis] behavior does not match spec and other browsers for security error cases

Status: VERIFIED FIXED

(Core :: DOM: Bindings (WebIDL), defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Consider this testcase:

<iframe src="https://example.com"></iframe>
<hr>
<pre><script>
    onload = function() {
      var desc = Object.getOwnPropertyDescriptor(window, "onmouseenter");
      desc.get.call(frames[0]);
    }
</script>

Per spec, this should throw an exception, because in https://heycam.github.io/webidl/#dfn-attribute-getter step 1.1.2.2, which does the security check, comes before step 1.1.2.3, which does the checking for [LenientThis]. In our code the two steps are sort of combined.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Oh, and both Safari and Chrome do throw on that testcase.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I closed this because I realized this testcase is too close to the one for bug 1582857. We will want to land that first, then this one...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1582911 part 1. Fix the exception we throw when binding this-unwrapping fails security checks to match spec. r=peterv

Spec says to throw a SecurityError, not a TypeError.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1582911 part 2. Fix LenientThis behavior to match spec when a security check fails. r=peterv

Comment 5

[Boris Zbarsky [:bzbarsky]]

Given that we already landed bug 1582857, landing this is not a problem.

Comment 6

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

https://hg.mozilla.org/integration/autoland/rev/8e5289a67e6af729629fe80c140945d58b3c84fb
https://hg.mozilla.org/integration/autoland/rev/bb9d14131aea0b7464c4d2dc6a97808349dc7090
https://hg.mozilla.org/mozilla-central/rev/8e5289a67e6a
https://hg.mozilla.org/mozilla-central/rev/bb9d14131aea

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Is this something we're going to want to uplift alongside bug 1582857 or can this ride Fx71 to release?

Comment 8

[Boris Zbarsky [:bzbarsky]]

I think this can just ride the trains. It's a riskier change, too, since it changes what exceptions we throw in common cases not just the weird edge case bug 1582857 affects...

Comment 9

[Rares Doghi, Desktop QA]

Hello Boris, Will I need a debug build in order to reproduce this issue ? After saving the Test case from the description as HTML and loading it in Firefox will I be able to see the exception in Browser Toolbox ? or Dev Tools Console ? or in CMD / Terminal ? What are the steps I can use to reproduce this issue in older builds ?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Steps to reproduce are to save the testcase as HTML, open the devtools console, and load the HTML. There should be an exception (and is one on current builds). Old builds will not have an exception.

Comment 11

[Rares Doghi, Desktop QA]

Verified as fixed in our latest builds, we are seeing the exception in devtools console.