Skip to content

Commit 623cfe1

Browse files
cdumezcdumez
committed
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: - Signed into a Google account already - Visit https://ifttt.com/login - Click "Continue with Google" - Select the signed in 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 & whatwg/html#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: Canonical link: https://commits.webkit.org/195238@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224287 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent c6d13a9 commit 623cfe1

11 files changed

Lines changed: 93 additions & 36 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2017-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+
113
2017-11-01 Frederic Wang <fwang@igalia.com>
214

315
Make iOS Find UI reveal matches in scrollable elements

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,78 +6,78 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
66
* Location.href
77
PASS descriptor.get is undefined.
88
PASS descriptor.set is an instance of Function
9-
PASS descriptor.enumerable is true
9+
PASS descriptor.enumerable is false
1010
PASS descriptor.configurable is true
1111
* Location.replace
1212
PASS descriptor.value is an instance of Function
1313
PASS descriptor.writable is false
14-
PASS descriptor.enumerable is true
14+
PASS descriptor.enumerable is false
1515
PASS descriptor.configurable is true
1616

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

8383
PASS successfullyParsed is true

‎LayoutTests/http/tests/security/cross-origin-descriptors.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
shouldBeType("descriptor.value", "Function");
2828
shouldBeFalse("descriptor.writable");
2929
}
30-
shouldBeTrue("descriptor.enumerable");
30+
shouldBeFalse("descriptor.enumerable");
3131
shouldBeTrue("descriptor.configurable");
3232
}
3333

‎LayoutTests/imported/w3c/ChangeLog‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2017-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+
113
2017-10-31 Dean Jackson <dino@apple.com>
214

315
transferFromImageBitmap should update canvas dimensions

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ PASS [[SetPrototypeOf]] should return false
66
PASS [[IsExtensible]] should return true for cross-origin objects
77
PASS [[PreventExtensions]] should throw for cross-origin objects
88
PASS [[GetOwnProperty]] - Properties on cross-origin objects should be reported |own|
9-
PASS [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly
9+
FAIL [[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
1010
PASS [[Delete]] Should throw on cross-origin objects
1111
PASS [[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
12+
FAIL Can only enumerate safelisted properties assert_equals: Enumerate all safelisted cross-origin Window properties expected 15 but got 0
13+
FAIL [[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
1414
PASS [[OwnPropertyKeys]] should return the right symbol-named properties for cross-origin objects
1515
PASS [[OwnPropertyKeys]] should place the symbols after the property names after the subframe indices
1616
PASS A and B jointly observe the same identity for cross-origin Window and Location

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

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

‎LayoutTests/js/dom/getOwnPropertyDescriptor-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ PASS Object.getOwnPropertyDescriptor(global, 'XMLHttpRequest').configurable is t
128128
PASS Object.getOwnPropertyDescriptor(global, 0).value is global[0]
129129
PASS Object.getOwnPropertyDescriptor(global, 0).hasOwnProperty('get') is false
130130
PASS Object.getOwnPropertyDescriptor(global, 0).hasOwnProperty('set') is false
131-
PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is true
131+
PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is false
132132
PASS Object.getOwnPropertyDescriptor(global, 0).configurable is true
133133
PASS Object.getOwnPropertyDescriptor(document.getElementsByTagName('div'), 0).value is document.getElementsByTagName('div')[0]
134134
PASS Object.getOwnPropertyDescriptor(document.getElementsByTagName('div'), 0).hasOwnProperty('get') is false

‎LayoutTests/js/resources/getOwnPropertyDescriptor.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ descriptorShouldBe("global", "'Infinity'", {writable: false, enumerable: false,
4444
var globalWindowGetter = Object.getOwnPropertyDescriptor(global, 'window').get;
4545
descriptorShouldBe("global", "'window'", {get: 'globalWindowGetter', set: undefined, enumerable: true, configurable: false});
4646
descriptorShouldBe("global", "'XMLHttpRequest'", {writable: true, enumerable: false, configurable: true, value:"XMLHttpRequest"});
47-
descriptorShouldBe("global", "0", {writable: true, enumerable: true, configurable: true, value:"global[0]"});
47+
descriptorShouldBe("global", "0", {writable: true, enumerable: false, configurable: true, value:"global[0]"});
4848
descriptorShouldBe("document.getElementsByTagName('div')", "0", {writable: false, enumerable: true, configurable: true, value:"document.getElementsByTagName('div')[0]"});
4949
descriptorShouldBe("document.getElementsByClassName('pass')", "0", {writable: false, enumerable: true, configurable: true, value:"document.getElementsByClassName('pass')[0]"});
5050
var canvas = document.createElement("canvas");

‎Source/WebCore/ChangeLog‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
2017-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+
131
2017-10-31 Dean Jackson <dino@apple.com>
232

333
transferFromImageBitmap should update canvas dimensions

‎Source/WebCore/bindings/js/JSDOMWindowCustom.cpp‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,11 @@ static bool jsDOMWindowGetOwnPropertySlotRestrictedAccess(JSDOMWindow* thisObjec
9191
// access below; we should try to find a way to merge the two.
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
}
101101

@@ -114,19 +114,19 @@ static bool jsDOMWindowGetOwnPropertySlotRestrictedAccess(JSDOMWindow* thisObjec
114114
// These are the functions we allow access to cross-origin (DoNotCheckSecurity in IDL).
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
}
132132

@@ -146,7 +146,7 @@ static bool jsDOMWindowGetOwnPropertySlotRestrictedAccess(JSDOMWindow* thisObjec
146146
|| propertyName == builtinNames.topPublicName()) {
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
}
152152

@@ -163,7 +163,7 @@ static bool jsDOMWindowGetOwnPropertySlotRestrictedAccess(JSDOMWindow* thisObjec
163163
// properties that are in Moz but not IE. Since we have some of these, we have to do it
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
}
169169

@@ -233,7 +233,7 @@ bool JSDOMWindow::getOwnPropertySlotByIndex(JSObject* object, ExecState* state,
233233
// (1) First, indexed properties.
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
}
239239

@@ -351,10 +351,12 @@ void JSDOMWindow::getOwnPropertyNames(JSObject* object, ExecState* exec, Propert
351351
{
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
}
360362
Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);

0 commit comments

Comments
 (0)