Microsoft Edge throws a script error: "SCRIPT5078: Cannot redefine non-configurable property 'location'"

Firefox 53 throws a script error: "TypeError: cannot declare global binding `location': property must be configurable or both writable and enumerable"

Thanks for the report. Unless I'm misunderstanding something, this override doesn't cross origin boundaries right? 

haraken, could you please take a look, or help with assigning this to a more suitable person? Thanks.

I think since it's an implementation issue, not a resource request, there is no cross-origin constraint for it. Any script in the webpage can override it, including those cross-origin scripts.

Did i get it wrong about the "cross origin boundaries" ?

As far as I tested, there is no leak over origins.  However, when 'location' is overwritten, crossOriginWin.location is no longer accessible (throws a SecurityError) despite it should be a cross-origin accessible attribute.

jochen@, could you tell us if V8 supports the following situation (data property + unconfigurable + readonly + [PutForwards])?
a) window.location is a data property with C++ callbacks for get and set.
b) window.location is [Unforgeable] / configurable:false.
c) window.location is [PutForwards] and readonly.

window.location is currently implemented as:
  v8::AccessControl = v8::ALL_CAN_READ |
                      v8::ALL_CAN_WRITE |
                      v8::PROHIBITS_OVERWRITING
  v8::PropertyAttribute = v8::DontDelete
and it's possible to define a function named "location" probably because the property looks writable.  A new function "location" overwrites the existing "location" attribute.

I know that V8 supports data property + readonly + [Replaceable] if Blink doesn't set the "set" callback (i.e. making the property writable without the "set" callback).

[Replaceable] readonly data property is implemented as:
  v8::AccessControl = v8::ALL_CAN_READ
  v8::PropertyAttribute = v8::DontDelete |
                          v8::ReadOnly
without "set" callback.

However, is there a good way to support [PutForwards] readonly data property?

Or is it possible that, when a new function is defined, V8 checks if there already exists a property with the same name and if it's configurable or not?

@yukishiino  Thanks for the detailed description, now I can understand the cross origin boundaries you said before. So it leads to the fact that the top window lost control over the location of cross-origin frames or windows, am i right? 

I myself did not noticed that before.

re #c9 , the fact is that a function named "location" shadows the window.location attribute from all the windows including iframes regardless of whether it's same origin or cross origin.

@yukishiino
Ok, I think i've got it right. 
Thanks for your explanation. 

Friendly ping?

Assigned to franzih@.  Assign back to me if there is something Blink can fix.

Could you make window.location ReadOnly rather than DontDelete? 

Unfortunately v8::ReadOnly doesn't work as expected in this case.

case a) v8::PropertyAttribute = v8::ReadOnly
a-1) You can still define "function location() {}".
a-2) You can delete window.location (shouldn't be deletable).
a-3) window.location="http://new/location" doesn't make any navigation (should navigate).

case b) v8::PropertyAttribute = v8::DontDelete | v8::ReadOnly
b-1) You can still define "function location() {}".
b-2) OK: You cannot delete window.location.
b-3) window.location="http://new/location" doesn't make any navigation (should navigate).

case c) v8::PropertyAttribute = v8::DontDelete
c-1) You can still define "function location() {}".
c-2) OK: You cannot delete window.location.
c-3) OK: window.location="http://new/location" makes a navigation.

In all cases, we can still define a function with name="location".

For the deletion/configurability ([Unforgeable]) and navigation ([PutForwards=href]), v8::ReadOnly doesn't work as expected.

I see. The setter is not invoked for declarations. I'll check if we can fix that.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/0e7a2ca0d75fad836435b0836ccc13c2b128e2f3

commit 0e7a2ca0d75fad836435b0836ccc13c2b128e2f3
Author: franzih <franzih@chromium.org>
Date: Thu Dec 15 14:41:07 2016

[runtime] Throw if re-declaring a non-configurable accessor.

If an accessor property is non-configurable, one should not be able
to re-declare it as a function. This specifically applies to special properties like window.location.

BUG= chromium:670596 

Review-Url: https://codereview.chromium.org/2582493002
Cr-Commit-Position: refs/heads/master@{#41725}

[modify] https://crrev.com/0e7a2ca0d75fad836435b0836ccc13c2b128e2f3/src/runtime/runtime-scopes.cc
[modify] https://crrev.com/0e7a2ca0d75fad836435b0836ccc13c2b128e2f3/test/cctest/test-api-accessors.cc

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot