Bug 167920 – Symbols exposed on cross-origin Window / Location objects should be configurable

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Symbols exposed on cross-origin Window / Location objects should be configurable:
- https://html.spec.whatwg.org/#crossorigingetownpropertyhelper-(-o,-p-) (Step 1)

Firefox behaves as per specification.

[Chris Dumez]

Created attachment 300779 [details]
Patch

[Chris Dumez]

With this bug and Bug 167917 fixed, 100% pass rate on:
- http://w3c-test.org/html/browsers/origin/cross-origin-objects/cross-origin-objects.html

[Ryosuke Niwa]

Comment on attachment 300779 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300779&action=review

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:100
>      if (propertyName == exec->propertyNames().toStringTagSymbol || propertyName == exec->propertyNames().hasInstanceSymbol || propertyName == exec->propertyNames().isConcatSpreadableSymbol) {

Are these only symbols that could ever exist on the window object?
It seems a bit fragile to have a hard-coded list here.

[Chris Dumez]

(In reply to comment #3)
> Comment on attachment 300779 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300779&action=review
> 
> > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:100
> >      if (propertyName == exec->propertyNames().toStringTagSymbol || propertyName == exec->propertyNames().hasInstanceSymbol || propertyName == exec->propertyNames().isConcatSpreadableSymbol) {
> 
> Are these only symbols that could ever exist on the window object?
> It seems a bit fragile to have a hard-coded list here.

These are the only ones that we expose on Window / Location objects that are *cross-origin*. They are listed in the spec here:
https://html.spec.whatwg.org/#crossorigingetownpropertyhelper-(-o,-p-) (Step 1).

[Chris Dumez]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 300779 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=300779&action=review
> > 
> > > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:100
> > >      if (propertyName == exec->propertyNames().toStringTagSymbol || propertyName == exec->propertyNames().hasInstanceSymbol || propertyName == exec->propertyNames().isConcatSpreadableSymbol) {
> > 
> > Are these only symbols that could ever exist on the window object?
> > It seems a bit fragile to have a hard-coded list here.
> 
> These are the only ones that we expose on Window / Location objects that are
> *cross-origin*. They are listed in the spec here:
> https://html.spec.whatwg.org/#crossorigingetownpropertyhelper-(-o,-p-) (Step
> 1).

This is how the HTML specification and our code operates for cross-origin Window / Location objects, we explicitly whitelist things we want to expose. The list of properties exposed cross-origin on these objects is also hard-coded.

[Ryosuke Niwa]

Comment on attachment 300779 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300779&action=review

>>>> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:100
>>>>      if (propertyName == exec->propertyNames().toStringTagSymbol || propertyName == exec->propertyNames().hasInstanceSymbol || propertyName == exec->propertyNames().isConcatSpreadableSymbol) {
>>> 
>>> Are these only symbols that could ever exist on the window object?
>>> It seems a bit fragile to have a hard-coded list here.
>> 
>> These are the only ones that we expose on Window / Location objects that are *cross-origin*. They are listed in the spec here:
>> https://html.spec.whatwg.org/#crossorigingetownpropertyhelper-(-o,-p-) (Step 1).
> 
> This is how the HTML specification and our code operates for cross-origin Window / Location objects, we explicitly whitelist things we want to expose. The list of properties exposed cross-origin on these objects is also hard-coded.

We should probably add that URL as a comment.

> Source/WebCore/bindings/js/JSLocationCustom.cpp:-57
>      if (propertyName == state->propertyNames().toStringTagSymbol || propertyName == state->propertyNames().hasInstanceSymbol || propertyName == state->propertyNames().isConcatSpreadableSymbol) {
> -        slot.setUndefined();

Ditto here.
Also, perhaps we should extract this check into a helper function so that there's a single list instead of being duplicated in two places.

[Chris Dumez]

Created attachment 300785 [details]
Patch

[Chris Dumez]

Comment on attachment 300785 [details]
Patch

Clearing flags on attachment: 300785

Committed r211772: <http://trac.webkit.org/changeset/211772>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.