Diff
Modified: trunk/LayoutTests/ChangeLog (206220 => 206221)
--- trunk/LayoutTests/ChangeLog 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/LayoutTests/ChangeLog 2016-09-21 18:23:33 UTC (rev 206221)
@@ -1,3 +1,15 @@
+2016-09-21 Chris Dumez <[email protected]>
+
+ Object.getOwnPropertyDescriptor() does not work correctly cross origin
+ https://bugs.webkit.org/show_bug.cgi?id=162311
+
+ Reviewed by Gavin Barraclough.
+
+ Add layout test coverage.
+
+ * http/tests/security/cross-origin-descriptors-expected.txt: Added.
+ * http/tests/security/cross-origin-descriptors.html: Added.
+
2016-09-21 Daniel Bates <[email protected]>
REGRESSION (r201090): Setting style.webkitTextSizeAdjust does not change text change on iPad
Added: trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt (0 => 206221)
--- trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt 2016-09-21 18:23:33 UTC (rev 206221)
@@ -0,0 +1,86 @@
+Test the descriptors returns by Object.getOwnPropertyDescriptor() cross origin.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+* Location.href
+PASS descriptor.get is undefined.
+PASS descriptor.set is an instance of Function
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Location.replace
+PASS descriptor.value is an instance of Function
+PASS descriptor.writable is false
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+
+* Window.window
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.self
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.location
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is an instance of Function
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.close
+PASS descriptor.value is an instance of Function
+PASS descriptor.writable is false
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.closed
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.focus
+PASS descriptor.value is an instance of Function
+PASS descriptor.writable is false
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.blur
+PASS descriptor.value is an instance of Function
+PASS descriptor.writable is false
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.frames
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.length
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.top
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.opener
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.parent
+PASS descriptor.get is an instance of Function
+PASS descriptor.set is undefined.
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+* Window.postMessage
+PASS descriptor.value is an instance of Function
+PASS descriptor.writable is false
+PASS descriptor.enumerable is false
+PASS descriptor.configurable is true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html (0 => 206221)
--- trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html 2016-09-21 18:23:33 UTC (rev 206221)
@@ -0,0 +1,72 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="help" href=""
+<link rel="help" href=""
+<script src=""
+</head>
+<body>
+<iframe src=""
+<script>
+description("Test the descriptors returns by Object.getOwnPropertyDescriptor() cross origin.");
+jsTestIsAsync = true;
+
+function testDescriptor(object, propertyName, shouldHaveGetter, shouldHaveSetter)
+{
+ descriptor = Object.getOwnPropertyDescriptor(object, propertyName);
+ if (shouldHaveGetter || shouldHaveSetter) {
+ if (shouldHaveGetter)
+ shouldBeType("descriptor.get", "Function");
+ else
+ shouldBeUndefined("descriptor.get");
+ if (shouldHaveSetter)
+ shouldBeType("descriptor.set", "Function");
+ else
+ shouldBeUndefined("descriptor.set");
+ } else {
+ shouldBeType("descriptor.value", "Function");
+ shouldBeFalse("descriptor.writable");
+ }
+ shouldBeFalse("descriptor.enumerable");
+ shouldBeTrue("descriptor.configurable");
+}
+
+_onload_ = function() {
+ var crossOriginLocationProperties = [
+ ["href", false, true],
+ ["replace", false, false],
+ ];
+ for (var property of crossOriginLocationProperties) {
+ debug("* Location." + property[0]);
+ testDescriptor(frames[0].location, property[0], property[1], property[2]);
+ }
+
+ debug("");
+
+ var crossOriginWindowProperties = [
+ ["window", true, false],
+ ["self", true, false],
+ ["location", true, true],
+ ["close", false, false],
+ ["closed", true, false],
+ ["focus", false, false],
+ ["blur", false, false],
+ ["frames", true, false],
+ ["length", true, false],
+ ["top", true, false],
+ ["opener", true, false],
+ ["parent", true, false],
+ ["postMessage", false, false]
+ ];
+ for (var property of crossOriginWindowProperties) {
+ debug("* Window." + property[0]);
+ testDescriptor(frames[0], property[0], property[1], property[2]);
+ }
+
+ debug("");
+ finishJSTest();
+};
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (206220 => 206221)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2016-09-21 18:23:33 UTC (rev 206221)
@@ -1,3 +1,14 @@
+2016-09-21 Chris Dumez <[email protected]>
+
+ Object.getOwnPropertyDescriptor() does not work correctly cross origin
+ https://bugs.webkit.org/show_bug.cgi?id=162311
+
+ Reviewed by Gavin Barraclough.
+
+ Rebaseline W3C test now that more checks are passing.
+
+ * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
+
2016-09-21 Jer Noble <[email protected]>
Unreviewed gardening; add some whitespace to expected results.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt (206220 => 206221)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt 2016-09-21 18:23:33 UTC (rev 206221)
@@ -7,16 +7,16 @@
PASS [[SetPrototypeOf]] should throw
PASS [[IsExtensible]] should return true for cross-origin objects
PASS [[PreventExtensions]] should throw for cross-origin objects
-FAIL [[GetOwnProperty]] - Properties on cross-origin objects should be reported |own| Blocked a frame with origin "http://localhost:8800" from accessing a frame with origin "http://127.0.0.1:8800". Protocols, domains, and ports must match.
-FAIL [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly assert_equals: property descriptor for location should be configurable expected true but got false
+PASS [[GetOwnProperty]] - Properties on cross-origin objects should be reported |own|
+PASS [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly
PASS [[Delete]] Should throw on cross-origin objects
PASS [[DefineOwnProperty]] Should throw for cross-origin objects
-FAIL [[Enumerate]] should return an empty iterator Blocked a frame with origin "http://localhost:8800" from accessing a frame with origin "http://127.0.0.1:8800". Protocols, domains, and ports must match.
+FAIL [[Enumerate]] should return an empty iterator assert_unreached: Shouldn't have been able to enumerate href on cross-origin Location Reached unreachable code
PASS [[OwnPropertyKeys]] should return all properties from cross-origin objects
PASS A and B jointly observe the same identity for cross-origin Window and Location
PASS Cross-origin functions get local Function.prototype
-FAIL Cross-origin Window accessors get local Function.prototype undefined is not an object (evaluating 'f.name')
+PASS Cross-origin Window accessors get local Function.prototype
PASS Same-origin observers get different functions for cross-origin objects
-FAIL Same-origin obsevers get different accessors for cross-origin Window assert_true: different Window accessors per-incumbent script settings object expected true got false
-FAIL Same-origin observers get different accessors for cross-origin Location Blocked a frame with origin "http://localhost:8800" from accessing a frame with origin "http://127.0.0.1:8800". Protocols, domains, and ports must match.
+PASS Same-origin obsevers get different accessors for cross-origin Window
+PASS Same-origin observers get different accessors for cross-origin Location
Modified: trunk/Source/_javascript_Core/ChangeLog (206220 => 206221)
--- trunk/Source/_javascript_Core/ChangeLog 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-09-21 18:23:33 UTC (rev 206221)
@@ -1,3 +1,30 @@
+2016-09-21 Chris Dumez <[email protected]>
+
+ Object.getOwnPropertyDescriptor() does not work correctly cross origin
+ https://bugs.webkit.org/show_bug.cgi?id=162311
+
+ Reviewed by Gavin Barraclough.
+
+ Add a CustomGetterSetter field to PropertySlot that gets populated
+ by getOwnPropertySlot() and use it in getOwnPropertyDescriptor()
+ to properly populate the descriptor. We used to rely on reifying
+ the properties and then call getDirect() in order to get the
+ CustomGetterSetter. However, this hack was insufficient to support
+ the cross-origin case because we need to control more precisely
+ the visibility of the getter and the setter. For example, Location's
+ href property has both a getter and a setter in the same origin
+ case but only has a setter in the cross-origin case.
+
+ In the future, we can extend the use of PropertySlot's
+ customGetterSetter field to the same origin case and get rid of the
+ reification + getDirect() hack in getOwnPropertyDescriptor().
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::getOwnPropertyDescriptor):
+ * runtime/PropertySlot.cpp:
+ (JSC::PropertySlot::customAccessorGetter):
+ * runtime/PropertySlot.h:
+
2016-09-21 Michael Saboff <[email protected]>
FTL needs to reserve callee stack space in bytes
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (206220 => 206221)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2016-09-21 18:23:33 UTC (rev 206221)
@@ -2842,14 +2842,19 @@
if (auto* proxy = jsDynamicCast<JSProxy*>(this))
thisObject = proxy->target();
- JSValue maybeGetterSetter = thisObject->getDirect(exec->vm(), propertyName);
- if (!maybeGetterSetter) {
- thisObject->reifyAllStaticProperties(exec);
- maybeGetterSetter = thisObject->getDirect(exec->vm(), propertyName);
+ CustomGetterSetter* getterSetter;
+ if (slot.isCustomAccessor())
+ getterSetter = slot.customGetterSetter();
+ else {
+ JSValue maybeGetterSetter = thisObject->getDirect(exec->vm(), propertyName);
+ if (!maybeGetterSetter) {
+ thisObject->reifyAllStaticProperties(exec);
+ maybeGetterSetter = thisObject->getDirect(exec->vm(), propertyName);
+ }
+
+ ASSERT(maybeGetterSetter);
+ getterSetter = jsCast<CustomGetterSetter*>(maybeGetterSetter);
}
-
- ASSERT(maybeGetterSetter);
- auto* getterSetter = jsCast<CustomGetterSetter*>(maybeGetterSetter);
if (getterSetter->getter())
descriptor.setGetter(getCustomGetterSetterFunctionForGetterSetter(exec, propertyName, getterSetter, JSCustomGetterSetterFunction::Type::Getter));
if (getterSetter->setter())
Modified: trunk/Source/_javascript_Core/runtime/PropertySlot.cpp (206220 => 206221)
--- trunk/Source/_javascript_Core/runtime/PropertySlot.cpp 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/_javascript_Core/runtime/PropertySlot.cpp 2016-09-21 18:23:33 UTC (rev 206221)
@@ -41,6 +41,13 @@
return JSValue::decode(m_data.custom.getValue(exec, JSValue::encode(thisValue), propertyName));
}
+JSValue PropertySlot::customAccessorGetter(ExecState* exec, PropertyName propertyName) const
+{
+ if (!m_data.customAccessor.getterSetter->getter())
+ return jsUndefined();
+ return JSValue::decode(m_data.customAccessor.getterSetter->getter()(exec, JSValue::encode(m_thisValue), propertyName));
+}
+
JSValue PropertySlot::getPureResult() const
{
JSValue result;
Modified: trunk/Source/_javascript_Core/runtime/PropertySlot.h (206220 => 206221)
--- trunk/Source/_javascript_Core/runtime/PropertySlot.h 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/_javascript_Core/runtime/PropertySlot.h 2016-09-21 18:23:33 UTC (rev 206221)
@@ -71,7 +71,8 @@
TypeUnset,
TypeValue,
TypeGetter,
- TypeCustom
+ TypeCustom,
+ TypeCustomAccessor,
};
public:
@@ -107,9 +108,11 @@
bool isValue() const { return m_propertyType == TypeValue; }
bool isAccessor() const { return m_propertyType == TypeGetter; }
bool isCustom() const { return m_propertyType == TypeCustom; }
+ bool isCustomAccessor() const { return m_propertyType == TypeCustomAccessor; }
bool isCacheableValue() const { return isCacheable() && isValue(); }
bool isCacheableGetter() const { return isCacheable() && isAccessor(); }
bool isCacheableCustom() const { return isCacheable() && isCustom(); }
+ bool isCacheableCustomAccessor() const { return isCacheable() && isCustomAccessor(); }
void setIsTaintedByOpaqueObject() { m_isTaintedByOpaqueObject = true; }
bool isTaintedByOpaqueObject() const { return m_isTaintedByOpaqueObject; }
@@ -140,6 +143,12 @@
return m_data.custom.getValue;
}
+ CustomGetterSetter* customGetterSetter() const
+ {
+ ASSERT(isCustomAccessor());
+ return m_data.customAccessor.getterSetter;
+ }
+
JSObject* slotBase() const
{
return m_slotBase;
@@ -218,6 +227,34 @@
m_offset = !invalidOffset;
}
+ void setCustomGetterSetter(JSObject* slotBase, unsigned attributes, CustomGetterSetter* getterSetter)
+ {
+ ASSERT(attributes == attributesForStructure(attributes));
+
+ ASSERT(getterSetter);
+ m_data.customAccessor.getterSetter = getterSetter;
+ m_attributes = attributes;
+
+ ASSERT(slotBase);
+ m_slotBase = slotBase;
+ m_propertyType = TypeCustomAccessor;
+ m_offset = invalidOffset;
+ }
+
+ void setCacheableCustomGetterSetter(JSObject* slotBase, unsigned attributes, CustomGetterSetter* getterSetter)
+ {
+ ASSERT(attributes == attributesForStructure(attributes));
+
+ ASSERT(getterSetter);
+ m_data.customAccessor.getterSetter = getterSetter;
+ m_attributes = attributes;
+
+ ASSERT(slotBase);
+ m_slotBase = slotBase;
+ m_propertyType = TypeCustomAccessor;
+ m_offset = !invalidOffset;
+ }
+
void setGetterSlot(JSObject* slotBase, unsigned attributes, GetterSetter* getterSetter)
{
ASSERT(attributes == attributesForStructure(attributes));
@@ -274,6 +311,7 @@
private:
JS_EXPORT_PRIVATE JSValue functionGetter(ExecState*) const;
JS_EXPORT_PRIVATE JSValue customGetter(ExecState*, PropertyName) const;
+ JS_EXPORT_PRIVATE JSValue customAccessorGetter(ExecState*, PropertyName) const;
unsigned m_attributes;
union {
@@ -284,6 +322,9 @@
struct {
GetValueFunc getValue;
} custom;
+ struct {
+ CustomGetterSetter* getterSetter;
+ } customAccessor;
} m_data;
PropertyOffset m_offset;
@@ -302,6 +343,8 @@
return JSValue::decode(m_data.value);
if (m_propertyType == TypeGetter)
return functionGetter(exec);
+ if (m_propertyType == TypeCustomAccessor)
+ return customAccessorGetter(exec, propertyName);
return customGetter(exec, propertyName);
}
Modified: trunk/Source/WebCore/ChangeLog (206220 => 206221)
--- trunk/Source/WebCore/ChangeLog 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/WebCore/ChangeLog 2016-09-21 18:23:33 UTC (rev 206221)
@@ -1,3 +1,25 @@
+2016-09-21 Chris Dumez <[email protected]>
+
+ Object.getOwnPropertyDescriptor() does not work correctly cross origin
+ https://bugs.webkit.org/show_bug.cgi?id=162311
+
+ Reviewed by Gavin Barraclough.
+
+ Object.getOwnPropertyDescriptor() does not work correctly cross origin. In particular:
+ - We return value descriptors for attributes instead of getter/setter descriptors
+ - attributes / operations are wrongly marked as non-configurable
+
+ Corresponding specification:
+ - https://html.spec.whatwg.org/#crossoriginproperties-(-o-)
+ - https://html.spec.whatwg.org/#crossorigingetownpropertyhelper-(-o,-p-)
+
+ Test: http/tests/security/cross-origin-descriptors.html
+
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
+ * bindings/js/JSLocationCustom.cpp:
+ (WebCore::JSLocation::getOwnPropertySlotDelegate):
+
2016-09-21 Alex Christensen <[email protected]>
URLParser: Correctly parse URLs that are just nonspecialscheme:/
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (206220 => 206221)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2016-09-21 18:23:33 UTC (rev 206221)
@@ -101,19 +101,19 @@
// These are the functions we allow access to cross-origin (DoNotCheckSecurity in IDL).
// Always provide the original function, on a fresh uncached function object.
if (propertyName == exec->propertyNames().blur) {
- slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
+ slot.setCustom(thisObject, ReadOnly | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
return true;
}
if (propertyName == exec->propertyNames().close) {
- slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
+ slot.setCustom(thisObject, ReadOnly | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
return true;
}
if (propertyName == exec->propertyNames().focus) {
- slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
+ slot.setCustom(thisObject, ReadOnly | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
return true;
}
if (propertyName == exec->propertyNames().postMessage) {
- slot.setCustom(thisObject, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
+ slot.setCustom(thisObject, ReadOnly | DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
return true;
}
@@ -131,7 +131,9 @@
|| propertyName == exec->propertyNames().opener
|| propertyName == exec->propertyNames().parent
|| propertyName == exec->propertyNames().top) {
- slot.setCacheableCustom(thisObject, ReadOnly | DontDelete | DontEnum, entry->propertyGetter());
+ bool shouldExposeSetter = propertyName == exec->propertyNames().location;
+ CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, entry->propertyGetter(), shouldExposeSetter ? entry->propertyPutter() : nullptr);
+ slot.setCacheableCustomGetterSetter(thisObject, DontEnum | CustomAccessor, customGetterSetter);
return true;
}
Modified: trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp (206220 => 206221)
--- trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp 2016-09-21 18:04:39 UTC (rev 206220)
+++ trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp 2016-09-21 18:23:33 UTC (rev 206221)
@@ -52,12 +52,20 @@
return false;
// We only allow access to Location.replace() cross origin.
- // Make it read-only / non-configurable to prevent writes via defineProperty.
if (propertyName == exec->propertyNames().replace) {
- slot.setCustom(this, ReadOnly | DontDelete | DontEnum, nonCachingStaticFunctionGetter<jsLocationInstanceFunctionReplace, 1>);
+ slot.setCustom(this, ReadOnly | DontEnum, nonCachingStaticFunctionGetter<jsLocationInstanceFunctionReplace, 1>);
return true;
}
+ // Getting location.href cross origin needs to throw. However, getOwnPropertyDescriptor() needs to return
+ // a descriptor that has a setter but no getter.
+ if (slot.internalMethodType() == PropertySlot::InternalMethodType::GetOwnProperty && propertyName == exec->propertyNames().href) {
+ auto* entry = JSLocation::info()->staticPropHashTable->entry(propertyName);
+ CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, nullptr, entry->propertyPutter());
+ slot.setCacheableCustomGetterSetter(this, DontEnum | CustomAccessor, customGetterSetter);
+ return true;
+ }
+
throwSecurityError(*exec, scope, message);
slot.setUndefined();
return true;
