Added: trunk/JSTests/stress/proxy-get-own-property-names-should-not-clear-previous-results.js (0 => 245643)
--- trunk/JSTests/stress/proxy-get-own-property-names-should-not-clear-previous-results.js (rev 0)
+++ trunk/JSTests/stress/proxy-get-own-property-names-should-not-clear-previous-results.js 2019-05-22 20:34:55 UTC (rev 245643)
@@ -0,0 +1,21 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+a = {defineProperties:Object};
+function opt() {
+ a.__proto__ = new Proxy(Object,{ownKeys:opt});
+ return 1;
+}
+for(var i=0;i<400;i=i+1) {
+ var prop = null;
+ var count = 0;
+ for (t in a) {
+ opt();
+ prop = t;
+ ++count;
+ }
+ shouldBe(prop, "defineProperties");
+ shouldBe(count, 1);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (245642 => 245643)
--- trunk/Source/_javascript_Core/ChangeLog 2019-05-22 20:22:44 UTC (rev 245642)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-05-22 20:34:55 UTC (rev 245643)
@@ -1,3 +1,33 @@
+2019-05-22 Yusuke Suzuki <[email protected]>
+
+ Don't clear PropertyNameArray in Proxy code
+ https://bugs.webkit.org/show_bug.cgi?id=197691
+
+ Reviewed by Saam Barati.
+
+ ProxyObject::performGetOwnPropertyNames clears the given PropertyNameArray to filter out non-enumerable keys.
+ But this does not assume that PropertyNameArray already contains the keys collected in the different objects.
+ We have an assumption that PropertyNameArray is always increasing, and JSPropertyNameEnumerator relies on this.
+ Since ProxyObject::performGetOwnPropertyNames clears the passed PropertyNameArray which contained the other
+ keys collected at some point of prototype hierarchy, this breaks JSPropertyNameEnumerator. Let's see the example.
+
+ var object = { __proto__: someProxy, someKey: 42 };
+ // Here, we first collect "someKey" in object. And using the same PropertyNameArray to add more keys from __proto__.
+ // But Proxy accidentally clears the passed PropertyNameArray, so "someKey" becomes missing.
+ for (var key in object);
+
+ This patch fixes ProxyObject::performGetOwnPropertyNames. Using separate PropertyNameArray to collect keys, and
+ filtering and adding them to the passed PropertyNameArray later. We also remove PropertyNameArray::reset method
+ since this breaks JSPropertyNameEnumerator's assumption.
+
+ We also fix the issue by changing seenKeys' HashSet<UniquedStringImpl*> to HashSet<RefPtr<UniquedStringImpl>>.
+ They can be deallocated if it is not added to trapResult later and it is toString-ed result from 'toPropertyKey()'.
+
+ * runtime/PropertyNameArray.h:
+ (JSC::PropertyNameArray::reset): Deleted.
+ * runtime/ProxyObject.cpp:
+ (JSC::ProxyObject::performGetOwnPropertyNames):
+
2019-05-22 Ross Kirsling <[email protected]>
[ESNext] Implement support for Numeric Separators
Modified: trunk/Source/_javascript_Core/runtime/ProxyObject.cpp (245642 => 245643)
--- trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2019-05-22 20:22:44 UTC (rev 245642)
+++ trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2019-05-22 20:34:55 UTC (rev 245643)
@@ -893,7 +893,7 @@
return thisObject->performDefineOwnProperty(exec, propertyName, descriptor, shouldThrow);
}
-void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray& trapResult, EnumerationMode enumerationMode)
+void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode enumerationMode)
{
NO_TAIL_CALLS();
@@ -917,7 +917,7 @@
JSObject* target = this->target();
if (ownKeysMethod.isUndefined()) {
scope.release();
- target->methodTable(vm)->getOwnPropertyNames(target, exec, trapResult, enumerationMode);
+ target->methodTable(vm)->getOwnPropertyNames(target, exec, propertyNames, enumerationMode);
return;
}
@@ -927,58 +927,61 @@
JSValue arrayLikeObject = call(exec, ownKeysMethod, callType, callData, handler, arguments);
RETURN_IF_EXCEPTION(scope, void());
- PropertyNameMode propertyNameMode = trapResult.propertyNameMode();
- RuntimeTypeMask resultFilter = 0;
- switch (propertyNameMode) {
- case PropertyNameMode::Symbols:
- resultFilter = TypeSymbol;
- break;
- case PropertyNameMode::Strings:
- resultFilter = TypeString;
- break;
- case PropertyNameMode::StringsAndSymbols:
- resultFilter = TypeSymbol | TypeString;
- break;
- }
- ASSERT(resultFilter);
- RuntimeTypeMask dontThrowAnExceptionTypeFilter = TypeString | TypeSymbol;
+ PropertyNameArray trapResult(&vm, propertyNames.propertyNameMode(), propertyNames.privateSymbolMode());
HashSet<UniquedStringImpl*> uncheckedResultKeys;
- HashSet<UniquedStringImpl*> seenKeys;
+ {
+ HashSet<RefPtr<UniquedStringImpl>> seenKeys;
- auto addPropName = [&] (JSValue value, RuntimeType type) -> bool {
- static const bool doExitEarly = true;
- static const bool dontExitEarly = false;
+ RuntimeTypeMask resultFilter = 0;
+ switch (propertyNames.propertyNameMode()) {
+ case PropertyNameMode::Symbols:
+ resultFilter = TypeSymbol;
+ break;
+ case PropertyNameMode::Strings:
+ resultFilter = TypeString;
+ break;
+ case PropertyNameMode::StringsAndSymbols:
+ resultFilter = TypeSymbol | TypeString;
+ break;
+ }
+ ASSERT(resultFilter);
- Identifier ident = value.toPropertyKey(exec);
- RETURN_IF_EXCEPTION(scope, doExitEarly);
+ auto addPropName = [&] (JSValue value, RuntimeType type) -> bool {
+ static const bool doExitEarly = true;
+ static const bool dontExitEarly = false;
- // If trapResult contains any duplicate entries, throw a TypeError exception.
- //
- // Per spec[1], filtering by type should occur _after_ [[OwnPropertyKeys]], so duplicates
- // are tracked in a separate hashtable from uncheckedResultKeys (which only contain the
- // keys filtered by type).
- //
- // [1] Per https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeysmust not contain any duplicate names"_s);
- if (!seenKeys.add(ident.impl()).isNewEntry) {
- throwTypeError(exec, scope, "Proxy handler's 'ownKeys' trap result must not contain any duplicate names"_s);
- return doExitEarly;
- }
+ Identifier ident = value.toPropertyKey(exec);
+ RETURN_IF_EXCEPTION(scope, doExitEarly);
- if (!(type & resultFilter))
+ // If trapResult contains any duplicate entries, throw a TypeError exception.
+ //
+ // Per spec[1], filtering by type should occur _after_ [[OwnPropertyKeys]], so duplicates
+ // are tracked in a separate hashtable from uncheckedResultKeys (which only contain the
+ // keys filtered by type).
+ //
+ // [1] Per https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeysmust not contain any duplicate names"_s);
+ if (!seenKeys.add(ident.impl()).isNewEntry) {
+ throwTypeError(exec, scope, "Proxy handler's 'ownKeys' trap result must not contain any duplicate names"_s);
+ return doExitEarly;
+ }
+
+ if (!(type & resultFilter))
+ return dontExitEarly;
+
+ uncheckedResultKeys.add(ident.impl());
+ trapResult.add(ident.impl());
return dontExitEarly;
+ };
- uncheckedResultKeys.add(ident.impl());
- trapResult.addUnchecked(ident.impl());
- return dontExitEarly;
- };
+ RuntimeTypeMask dontThrowAnExceptionTypeFilter = TypeString | TypeSymbol;
+ createListFromArrayLike(exec, arrayLikeObject, dontThrowAnExceptionTypeFilter, "Proxy handler's 'ownKeys' method must return an array-like object containing only Strings and Symbols"_s, addPropName);
+ RETURN_IF_EXCEPTION(scope, void());
+ }
- createListFromArrayLike(exec, arrayLikeObject, dontThrowAnExceptionTypeFilter, "Proxy handler's 'ownKeys' method must return an array-like object containing only Strings and Symbols"_s, addPropName);
- RETURN_IF_EXCEPTION(scope, void());
-
bool targetIsExensible = target->isExtensible(exec);
RETURN_IF_EXCEPTION(scope, void());
- PropertyNameArray targetKeys(&vm, propertyNameMode, trapResult.privateSymbolMode());
+ PropertyNameArray targetKeys(&vm, propertyNames.propertyNameMode(), propertyNames.privateSymbolMode());
target->methodTable(vm)->getOwnPropertyNames(target, exec, targetKeys, enumerationMode);
RETURN_IF_EXCEPTION(scope, void());
Vector<UniquedStringImpl*> targetConfigurableKeys;
@@ -1026,10 +1029,7 @@
if (!enumerationMode.includeDontEnumProperties()) {
// Filtering DontEnum properties is observable in proxies and must occur following the invariant checks above.
- auto data = ""
- trapResult.reset();
-
- for (auto propertyName : data->propertyNameVector()) {
+ for (auto propertyName : trapResult) {
PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty);
auto result = getOwnPropertySlotCommon(exec, propertyName, slot);
RETURN_IF_EXCEPTION(scope, void());
@@ -1037,8 +1037,11 @@
continue;
if (slot.attributes() & PropertyAttribute::DontEnum)
continue;
- trapResult.addUnchecked(propertyName.impl());
+ propertyNames.add(propertyName.impl());
}
+ } else {
+ for (auto propertyName : trapResult)
+ propertyNames.add(propertyName.impl());
}
}
