Changeset 224287 in webkit


Ignore:
Timestamp:
Nov 1, 2017 12:39:12 PM (7 years ago)
Author:
Chris Dumez
Message:

Regression(r219659): Can no longer log into ifttt.com using Google account
​https://bugs.webkit.org/show_bug.cgi?id=179117

Reviewed by Geoffrey Garen.

LayoutTests/imported/w3c:

Rebaseline WPT tests.

  • web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
  • web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt:

Source/WebCore:

After r219659, it is no longer possible to log into ifttt.com using a Google
account:

It turns out that this change to the HTML specification was not Web-compatible:
See ​https://bugzilla.mozilla.org/show_bug.cgi?id=1412741 & ​https://github.com/whatwg/html/issues/3183

This patch reverts r219659 for now until we agree on what behavior should get
specified.

No new tests, rebaselined existing tests.

  • bindings/js/JSDOMWindowCustom.cpp:

(WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
(WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
(WebCore::JSDOMWindow::getOwnPropertyNames):

  • bindings/js/JSLocationCustom.cpp:

(WebCore::getOwnPropertySlotCommon):
(WebCore::JSLocation::getOwnPropertyNames):

LayoutTests:

Update / rebaseline existing test.

  • http/tests/security/cross-origin-descriptors-expected.txt:
  • http/tests/security/cross-origin-descriptors.html:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r224284 r224287  
     12017-11-01  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r219659): Can no longer log into ifttt.com using Google account
     4        https://bugs.webkit.org/show_bug.cgi?id=179117
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Update / rebaseline existing test.
     9
     10        * http/tests/security/cross-origin-descriptors-expected.txt:
     11        * http/tests/security/cross-origin-descriptors.html:
     12
    1132017-11-01  Frederic Wang  <fwang@igalia.com>
    214

  • trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt

    r219659 r224287  
    77PASS descriptor.get is undefined.
    88PASS descriptor.set is an instance of Function
    9 PASS descriptor.enumerable is true
     9PASS descriptor.enumerable is false
    1010PASS descriptor.configurable is true
    1111* Location.replace
    1212PASS descriptor.value is an instance of Function
    1313PASS descriptor.writable is false
    14 PASS descriptor.enumerable is true
     14PASS descriptor.enumerable is false
    1515PASS descriptor.configurable is true
    1616
    … …  
    1818PASS descriptor.get is an instance of Function
    1919PASS descriptor.set is undefined.
    20 PASS descriptor.enumerable is true
     20PASS descriptor.enumerable is false
    2121PASS descriptor.configurable is true
    2222* Window.self
    2323PASS descriptor.get is an instance of Function
    2424PASS descriptor.set is undefined.
    25 PASS descriptor.enumerable is true
     25PASS descriptor.enumerable is false
    2626PASS descriptor.configurable is true
    2727* Window.location
    2828PASS descriptor.get is an instance of Function
    2929PASS descriptor.set is an instance of Function
    30 PASS descriptor.enumerable is true
     30PASS descriptor.enumerable is false
    3131PASS descriptor.configurable is true
    3232* Window.close
    3333PASS descriptor.value is an instance of Function
    3434PASS descriptor.writable is false
    35 PASS descriptor.enumerable is true
     35PASS descriptor.enumerable is false
    3636PASS descriptor.configurable is true
    3737* Window.closed
    3838PASS descriptor.get is an instance of Function
    3939PASS descriptor.set is undefined.
    40 PASS descriptor.enumerable is true
     40PASS descriptor.enumerable is false
    4141PASS descriptor.configurable is true
    4242* Window.focus
    4343PASS descriptor.value is an instance of Function
    4444PASS descriptor.writable is false
    45 PASS descriptor.enumerable is true
     45PASS descriptor.enumerable is false
    4646PASS descriptor.configurable is true
    4747* Window.blur
    4848PASS descriptor.value is an instance of Function
    4949PASS descriptor.writable is false
    50 PASS descriptor.enumerable is true
     50PASS descriptor.enumerable is false
    5151PASS descriptor.configurable is true
    5252* Window.frames
    5353PASS descriptor.get is an instance of Function
    5454PASS descriptor.set is undefined.
    55 PASS descriptor.enumerable is true
     55PASS descriptor.enumerable is false
    5656PASS descriptor.configurable is true
    5757* Window.length
    5858PASS descriptor.get is an instance of Function
    5959PASS descriptor.set is undefined.
    60 PASS descriptor.enumerable is true
     60PASS descriptor.enumerable is false
    6161PASS descriptor.configurable is true
    6262* Window.top
    6363PASS descriptor.get is an instance of Function
    6464PASS descriptor.set is undefined.
    65 PASS descriptor.enumerable is true
     65PASS descriptor.enumerable is false
    6666PASS descriptor.configurable is true
    6767* Window.opener
    6868PASS descriptor.get is an instance of Function
    6969PASS descriptor.set is undefined.
    70 PASS descriptor.enumerable is true
     70PASS descriptor.enumerable is false
    7171PASS descriptor.configurable is true
    7272* Window.parent
    7373PASS descriptor.get is an instance of Function
    7474PASS descriptor.set is undefined.
    75 PASS descriptor.enumerable is true
     75PASS descriptor.enumerable is false
    7676PASS descriptor.configurable is true
    7777* Window.postMessage
    7878PASS descriptor.value is an instance of Function
    7979PASS descriptor.writable is false
    80 PASS descriptor.enumerable is true
     80PASS descriptor.enumerable is false
    8181PASS descriptor.configurable is true
    8282

  • trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html

    r219659 r224287  
    2828        shouldBeFalse("descriptor.writable");
    2929    }
    30     shouldBeTrue("descriptor.enumerable");
     30    shouldBeFalse("descriptor.enumerable");
    3131    shouldBeTrue("descriptor.configurable");
    3232}

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r224283 r224287  
     12017-11-01  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r219659): Can no longer log into ifttt.com using Google account
     4        https://bugs.webkit.org/show_bug.cgi?id=179117
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Rebaseline WPT tests.
     9
     10        * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
     11        * web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt:
     12
    1132017-10-31  Dean Jackson  <dino@apple.com>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt

    r219659 r224287  
    77PASS [[PreventExtensions]] should throw for cross-origin objects
    88PASS [[GetOwnProperty]] - Properties on cross-origin objects should be reported |own|
    9 PASS [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly
     9FAIL [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly assert_equals: property descriptor for 0 should be enumerable expected true but got false
    1010PASS [[Delete]] Should throw on cross-origin objects
    1111PASS [[DefineOwnProperty]] Should throw for cross-origin objects
    12 PASS Can only enumerate safelisted properties
    13 PASS [[OwnPropertyKeys]] should return all properties from cross-origin objects
     12FAIL Can only enumerate safelisted properties assert_equals: Enumerate all safelisted cross-origin Window properties expected 15 but got 0
     13FAIL [[OwnPropertyKeys]] should return all properties from cross-origin objects assert_array_equals: Object.keys() gives the right answer for cross-origin Window lengths differ, expected 15 got 0
    1414PASS [[OwnPropertyKeys]] should return the right symbol-named properties for cross-origin objects
    1515PASS [[OwnPropertyKeys]] should place the symbols after the property names after the subframe indices

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt

    r219659 r224287  
    11
    22PASS Indexed properties of the window object (non-strict mode)
    3 PASS Ensure indexed properties have the correct configuration
     3FAIL Ensure indexed properties have the correct configuration assert_true: expected true got false
    44FAIL Indexed properties of the window object (non-strict mode) 1 assert_throws: function "() => Object.defineProperty(window, 0, { value: "bar" })" did not throw
    55FAIL Indexed properties of the window object (non-strict mode) 2 assert_throws: function "() => Object.defineProperty(window, 1, { value: "bar" })" did not throw

  • trunk/LayoutTests/js/dom/getOwnPropertyDescriptor-expected.txt

    r219659 r224287  
    129129PASS Object.getOwnPropertyDescriptor(global, 0).hasOwnProperty('get') is false
    130130PASS Object.getOwnPropertyDescriptor(global, 0).hasOwnProperty('set') is false
    131 PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is true
     131PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is false
    132132PASS Object.getOwnPropertyDescriptor(global, 0).configurable is true
    133133PASS Object.getOwnPropertyDescriptor(document.getElementsByTagName('div'), 0).value is document.getElementsByTagName('div')[0]

  • trunk/LayoutTests/js/resources/getOwnPropertyDescriptor.js

    r219659 r224287  
    4545descriptorShouldBe("global", "'window'", {get: 'globalWindowGetter', set: undefined, enumerable: true, configurable: false});
    4646descriptorShouldBe("global", "'XMLHttpRequest'", {writable: true, enumerable: false, configurable: true, value:"XMLHttpRequest"});
    47 descriptorShouldBe("global", "0", {writable: true, enumerable: true, configurable: true, value:"global[0]"});
     47descriptorShouldBe("global", "0", {writable: true, enumerable: false, configurable: true, value:"global[0]"});
    4848descriptorShouldBe("document.getElementsByTagName('div')", "0", {writable: false, enumerable: true, configurable: true, value:"document.getElementsByTagName('div')[0]"});
    4949descriptorShouldBe("document.getElementsByClassName('pass')", "0", {writable: false, enumerable: true, configurable: true, value:"document.getElementsByClassName('pass')[0]"});

  • trunk/Source/WebCore/ChangeLog

    r224283 r224287  
     12017-11-01  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r219659): Can no longer log into ifttt.com using Google account
     4        https://bugs.webkit.org/show_bug.cgi?id=179117
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        After r219659, it is no longer possible to log into ifttt.com using a Google
     9        account:
     10        - Signed into a Google account already
     11        - Visit https://ifttt.com/login
     12        - Click "Continue with Google"
     13        - Select the signed in account
     14
     15        It turns out that this change to the HTML specification was not Web-compatible:
     16        See https://bugzilla.mozilla.org/show_bug.cgi?id=1412741 & https://github.com/whatwg/html/issues/3183
     17
     18        This patch reverts r219659 for now until we agree on what behavior should get
     19        specified.
     20
     21        No new tests, rebaselined existing tests.
     22
     23        * bindings/js/JSDOMWindowCustom.cpp:
     24        (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
     25        (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
     26        (WebCore::JSDOMWindow::getOwnPropertyNames):
     27        * bindings/js/JSLocationCustom.cpp:
     28        (WebCore::getOwnPropertySlotCommon):
     29        (WebCore::JSLocation::getOwnPropertyNames):
     30
    1312017-10-31  Dean Jackson  <dino@apple.com>
    232

  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r223476 r224287  
    9292    if (!frame) {
    9393        if (propertyName == builtinNames.closedPublicName()) {
    94             slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete, jsDOMWindowClosed);
     94            slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, jsDOMWindowClosed);
    9595            return true;
    9696        }
    9797        if (propertyName == builtinNames.closePublicName()) {
    98             slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
     98            slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
    9999            return true;
    100100        }
    … …  
    115115    // Always provide the original function, on a fresh uncached function object.
    116116    if (propertyName == builtinNames.blurPublicName()) {
    117         slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
     117        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
    118118        return true;
    119119    }
    120120    if (propertyName == builtinNames.closePublicName()) {
    121         slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
     121        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
    122122        return true;
    123123    }
    124124    if (propertyName == builtinNames.focusPublicName()) {
    125         slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
     125        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
    126126        return true;
    127127    }
    128128    if (propertyName == builtinNames.postMessagePublicName()) {
    129         slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
     129        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
    130130        return true;
    131131    }
    … …  
    147147            bool shouldExposeSetter = propertyName == builtinNames.locationPublicName();
    148148            CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, entry->propertyGetter(), shouldExposeSetter ? entry->propertyPutter() : nullptr);
    149             slot.setCustomGetterSetter(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), customGetterSetter);
     149            slot.setCustomGetterSetter(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DontEnum), customGetterSetter);
    150150            return true;
    151151        }
    … …  
    164164    // the Moz way.
    165165    if (auto* scopedChild = frame->tree().scopedChild(propertyNameToAtomicString(propertyName))) {
    166         slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete, toJS(exec, scopedChild->document()->domWindow()));
     166        slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, toJS(exec, scopedChild->document()->domWindow()));
    167167        return true;
    168168    }
    … …  
    234234    // These are also allowed cross-orgin, so come before the access check.
    235235    if (frame && index < frame->tree().scopedChildCount()) {
    236         slot.setValue(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), toJS(state, frame->tree().scopedChild(index)->document()->domWindow()));
     236        slot.setValue(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), toJS(state, frame->tree().scopedChild(index)->document()->domWindow()));
    237237        return true;
    238238    }
    … …  
    352352    JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
    353353
    354     addScopedChildrenIndexes(*exec, thisObject->wrapped(), propertyNames);
     354    if (mode.includeDontEnumProperties())
     355        addScopedChildrenIndexes(*exec, thisObject->wrapped(), propertyNames);
    355356
    356357    if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped(), DoNotReportSecurityError)) {
    357         addCrossOriginWindowOwnPropertyNames(*exec, propertyNames);
     358        if (mode.includeDontEnumProperties())
     359            addCrossOriginWindowOwnPropertyNames(*exec, propertyNames);
    358360        return;
    359361    }

  • trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp

    r223476 r224287  
    6363    // We only allow access to Location.replace() cross origin.
    6464    if (propertyName == vm.propertyNames->replace) {
    65         slot.setCustom(&thisObject, static_cast<unsigned>(PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsLocationInstanceFunctionReplace, 1>);
     65        slot.setCustom(&thisObject, static_cast<unsigned>(PropertyAttribute::ReadOnly | PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsLocationInstanceFunctionReplace, 1>);
    6666        return true;
    6767    }
    … …  
    7272        auto* entry = JSLocation::info()->staticPropHashTable->entry(propertyName);
    7373        CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, nullptr, entry->propertyPutter());
    74         slot.setCustomGetterSetter(&thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), customGetterSetter);
     74        slot.setCustomGetterSetter(&thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | PropertyAttribute::DontEnum), customGetterSetter);
    7575        return true;
    7676    }
    … …  
    189189    JSLocation* thisObject = jsCast<JSLocation*>(object);
    190190    if (!BindingSecurity::shouldAllowAccessToFrame(exec, thisObject->wrapped().frame(), DoNotReportSecurityError)) {
    191         addCrossOriginLocationOwnPropertyNames(*exec, propertyNames);
     191        if (mode.includeDontEnumProperties())
     192            addCrossOriginLocationOwnPropertyNames(*exec, propertyNames);
    192193        return;
    193194    }
Note: See TracChangeset for help on using the changeset viewer.