Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behavior of Window's [SetPrototypeOf] ? #1727

Closed
cdumez opened this issue Aug 30, 2016 · 13 comments
Closed

Behavior of Window's [SetPrototypeOf] ? #1727

cdumez opened this issue Aug 30, 2016 · 13 comments
Assignees

Comments

@cdumez
Copy link

cdumez commented Aug 30, 2016

The HTML specification merely says to return false:
https://html.spec.whatwg.org/#windowproxy-setprototypeof

7.4.2 [[SetPrototypeOf]] ( V )

Return false.

However, this does not seem to match browsers?

Firefox:

  • Cross-Origin: Throws an Error
  • Same Origin: Throws a TypeError

Chrome:

  • Cross-Origin: Throws a SecurityError
  • Same Origin: Sets the prototype

Safari TP:

  • Cross-Origin: Ignores and logs an error message
  • Same Origin: Sets the prototype
@cdumez
Copy link
Author

cdumez commented Aug 30, 2016

@annevk @domenic @bzbarsky

@cdumez cdumez changed the title Behavior of Window's [SetPrototype] ? Behavior of Window's [SetPrototypeOf] ? Aug 30, 2016
@cdumez
Copy link
Author

cdumez commented Aug 30, 2016

http://w3c-test.org/html/browsers/origin/cross-origin-objects/cross-origin-objects.sub.html seems to expect that an exception be thrown for the cross-origin case.

@bzbarsky
Copy link
Contributor

However, this does not seem to match browsers?

It matches Firefox for the same-origin case, no? At least assuming you're using Object.setPrototypeOf: http://www.ecma-international.org/ecma-262/6.0/#sec-object.setprototypeof step 7 says to throw a TypeError if O.[[SetPrototypeOf]] returns false.

Chrome hasn't updated to this part of the spec yet, sounds like; it's a somewhat recent change to the spec (both HTML and ES; see https://tc39.github.io/ecma262/#sec-immutable-prototype-exotic-objects-setprototypeof-v and https://tc39.github.io/ecma262/#sec-properties-of-the-object-prototype-object which makes trying to set the prototype of Object.prototype throw in the same way). The idea is to make the prototype chain of the global immutable in general.

For the cross-origin case, this is a matter of the clear WindowProxy spec being fairly new and hence Firefox not yet implementing it. We should change the test to test that a TypeError is thrown, probably....

@annevk
Copy link
Member

annevk commented Aug 30, 2016

After web-platform-tests/wpt#3610 lands I can update the (cross-origin) test to actually check for specific exceptions. Okay with you @bholley?

@annevk
Copy link
Member

annevk commented Aug 30, 2016

Also fix web-platform-tests/wpt#3610 (comment) as part of fixing the test. (Assuming everyone is okay with fixing the test.)

@domenic
Copy link
Member

domenic commented Aug 30, 2016

Right, I think the spec is correct-ish here, but again it's just the problem of confusing the behavior of Object.setPrototypeOf with the [[SetPrototypeOf]] internal method. Returning false from the latter translates to throwing a TypeError from the former. (And there is no strict/sloppy distinction here.)

I think what remains is to decide whether we want to throw a TypeError cross-origin, or a DOMException SecurityError. That question is probably the same for all internal methods (so #1726/#1728 need to align with whatever we decide).

We will always through a TypeError same-origin; as @bzbarsky points out that is somewhat of a recent change, but an important one for security.

@domenic
Copy link
Member

domenic commented Aug 30, 2016

Edge does not throw (but also fails the "Basic sanity-checking") when running http://w3c-test.org/html/browsers/origin/cross-origin-objects/cross-origin-objects.sub.html

@cdumez
Copy link
Author

cdumez commented Aug 30, 2016

Well, all browsers will fail Basic Sanity-checking if the test is hosted on port 80:
assert_equals: Need to run the top-level test from port 80 expected "80" but got ""

This is the test that needs fixing.

@bholley
Copy link

bholley commented Aug 30, 2016

I'll defer to @bzbarsky on the details of exactly what sort of exceptions are thrown.

Checking this in the test is good, but please make sure to do it in such a way that a browser that throws the wrong sort of exception still gets credit for throwing an exception, and merely fails an additional "right kind of exception" test.

@cdumez
Copy link
Author

cdumez commented Aug 30, 2016

I am updating WebKit [1] to throw a TypeError in the cross-origin case as per:

For now, we still allow setting the prototype in the same origin case, similarly to Chrome.

[1] https://bugs.webkit.org/show_bug.cgi?id=161396

@annevk
Copy link
Member

annevk commented Sep 13, 2016

Given #1728 (comment) it seems this can be closed as "worksforme"? That is, since the behavior is supposed to be consistent for same-origin and cross-origin, we'll leave this as a TypeError?

annevk added a commit to web-platform-tests/wpt that referenced this issue Sep 13, 2016
This adds a new test to test for specific cross-origin object
exceptions as discussed in whatwg/html#1727.

Once this test is more widely implemented the cross-origin-objects.html
resource can be replaced by it (as indicated within the resource).
@annevk
Copy link
Member

annevk commented Sep 13, 2016

I created a WPT PR to test for the specific exceptions. As requested I've done it as a separate resource for now so browsers can separately check whether they throw at all and whether they throw the correct exception. Once a couple of browsers start throwing the correct exceptions I suggest we only use that resource going forward. It's not worth the hassle to maintain it twice.

@cdumez
Copy link
Author

cdumez commented Sep 13, 2016

Yes, the specification seems correct here.

@cdumez cdumez closed this as completed Sep 13, 2016
annevk added a commit to web-platform-tests/wpt that referenced this issue Nov 29, 2016
This adds a new test to test for specific cross-origin object
exceptions as discussed in whatwg/html#1727.

Once this test is more widely implemented the cross-origin-objects.html
resource can be replaced by it (as indicated within the resource).
foolip pushed a commit to web-platform-tests/wpt that referenced this issue Nov 29, 2016
This adds a new test to test for specific cross-origin object
exceptions as discussed in whatwg/html#1727.

Once this test is more widely implemented the cross-origin-objects.html
resource can be replaced by it (as indicated within the resource).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants