diff --git a/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyNames4.js b/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyNames4.js index 090a3d7410..cf36191a54 100644 --- a/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyNames4.js +++ b/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyNames4.js @@ -2,4 +2,4 @@ load(libdir + "asserts.js"); var handler = { ownKeys : () => [ 'foo', 'foo' ] }; for (let p of [new Proxy({}, handler), Proxy.revocable({}, handler).proxy]) - assertDeepEq(Object.getOwnPropertyNames(p), ['foo', 'foo']); + assertThrowsInstanceOf(() => Object.getOwnPropertyNames(p), TypeError); diff --git a/js/src/jit-test/tests/proxy/testDirectProxyKeys4.js b/js/src/jit-test/tests/proxy/testDirectProxyKeys4.js index f20d5368de..cad6b89247 100644 --- a/js/src/jit-test/tests/proxy/testDirectProxyKeys4.js +++ b/js/src/jit-test/tests/proxy/testDirectProxyKeys4.js @@ -2,4 +2,4 @@ load(libdir + "asserts.js"); var handler = { ownKeys: () => [ 'foo', 'foo' ] }; for (let p of [new Proxy({}, handler), Proxy.revocable({}, handler).proxy]) - assertDeepEq(Object.keys(p), []); // Properties are not enumerable. + assertThrowsInstanceOf(() => Object.keys(p), TypeError); diff --git a/js/src/js.msg b/js/src/js.msg index 5125cb12b8..499e7a1623 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -424,6 +424,7 @@ MSG_DEF(JSMSG_CANT_SET_NW_NC, 0, JSEXN_TYPEERR, "proxy can't successful MSG_DEF(JSMSG_CANT_SET_WO_SETTER, 0, JSEXN_TYPEERR, "proxy can't succesfully set an accessor property without a setter") MSG_DEF(JSMSG_CANT_SKIP_NC, 0, JSEXN_TYPEERR, "proxy can't skip a non-configurable property") MSG_DEF(JSMSG_ONWKEYS_STR_SYM, 0, JSEXN_TYPEERR, "proxy [[OwnPropertyKeys]] must return an array with only string and symbol elements") +MSG_DEF(JSMSG_OWNKEYS_DUPLICATE, 0, JSEXN_TYPEERR, "proxy [[OwnPropertyKeys]] cannot report duplicate keys") MSG_DEF(JSMSG_MUST_REPORT_SAME_VALUE, 0, JSEXN_TYPEERR, "proxy must report the same value for a non-writable, non-configurable property") MSG_DEF(JSMSG_MUST_REPORT_UNDEFINED, 0, JSEXN_TYPEERR, "proxy must report undefined for a non-configurable accessor property without a getter") MSG_DEF(JSMSG_OBJECT_ACCESS_DENIED, 0, JSEXN_ERR, "Permission denied to access object") diff --git a/js/src/proxy/ScriptedProxyHandler.cpp b/js/src/proxy/ScriptedProxyHandler.cpp index d396b7805c..ede79aa275 100644 --- a/js/src/proxy/ScriptedProxyHandler.cpp +++ b/js/src/proxy/ScriptedProxyHandler.cpp @@ -696,7 +696,7 @@ CreateFilteredListFromArrayLike(JSContext* cx, HandleValue v, AutoIdVector& prop } -// ES8 rev 0c1bd3004329336774cbc90de727cd0cf5f11e93 9.5.11 Proxy.[[OwnPropertyKeys]]() +// ES2018 9.5.11 Proxy.[[OwnPropertyKeys]]() bool ScriptedProxyHandler::ownPropertyKeys(JSContext* cx, HandleObject proxy, AutoIdVector& props) const { @@ -732,27 +732,45 @@ ScriptedProxyHandler::ownPropertyKeys(JSContext* cx, HandleObject proxy, AutoIdV return false; // Step 9. + Rooted> uncheckedResultKeys(cx, GCHashSet(cx)); + if (!uncheckedResultKeys.init(trapResult.length())) + return false; + + for (size_t i = 0, len = trapResult.length(); i < len; i++) { + MOZ_ASSERT(!JSID_IS_VOID(trapResult[i])); + + auto ptr = uncheckedResultKeys.lookupForAdd(trapResult[i]); + if (ptr) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_OWNKEYS_DUPLICATE); + return false; + } + + if (!uncheckedResultKeys.add(ptr, trapResult[i])) + return false; + } + + // Step 10. bool extensibleTarget; if (!IsExtensible(cx, target, &extensibleTarget)) return false; - // Steps 10-11. + // Steps 11-12. AutoIdVector targetKeys(cx); if (!GetPropertyKeys(cx, target, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, &targetKeys)) return false; - // Steps 12-13. + // Steps 13-14. AutoIdVector targetConfigurableKeys(cx); AutoIdVector targetNonconfigurableKeys(cx); - // Step 14. + // Step 15. Rooted desc(cx); for (size_t i = 0; i < targetKeys.length(); ++i) { - // Step 14a. + // Step 15a. if (!GetOwnPropertyDescriptor(cx, target, targetKeys[i], &desc)) return false; - // Steps 14b-c. + // Steps 15b-c. if (desc.object() && !desc.configurable()) { if (!targetNonconfigurableKeys.append(targetKeys[i])) return false; @@ -762,24 +780,10 @@ ScriptedProxyHandler::ownPropertyKeys(JSContext* cx, HandleObject proxy, AutoIdV } } - // Step 15. + // Step 16. if (extensibleTarget && targetNonconfigurableKeys.empty()) return props.appendAll(trapResult); - // Step 16. - // The algorithm below always removes all occurences of the same key - // at once, so we can use a set here. - Rooted> uncheckedResultKeys(cx, GCHashSet(cx)); - if (!uncheckedResultKeys.init(trapResult.length())) - return false; - - for (size_t i = 0, len = trapResult.length(); i < len; i++) { - MOZ_ASSERT(!JSID_IS_VOID(trapResult[i])); - - if (!uncheckedResultKeys.put(trapResult[i])) - return false; - } - // Step 17. for (size_t i = 0; i < targetNonconfigurableKeys.length(); ++i) { MOZ_ASSERT(!JSID_IS_VOID(targetNonconfigurableKeys[i])); diff --git a/js/src/tests/ecma_6/Proxy/ownkeys-trap-duplicates.js b/js/src/tests/ecma_6/Proxy/ownkeys-trap-duplicates.js index eef84d5713..ddfd99d87a 100644 --- a/js/src/tests/ecma_6/Proxy/ownkeys-trap-duplicates.js +++ b/js/src/tests/ecma_6/Proxy/ownkeys-trap-duplicates.js @@ -6,9 +6,8 @@ var gTestfile = 'ownkeys-trap-duplicates.js'; var BUGNUMBER = 1293995; var summary = - "Scripted proxies' [[OwnPropertyKeys]] should not throw if the trap " + - "implementation returns duplicate properties and the object is " + - "non-extensible or has non-configurable properties"; + "Scripted proxies' [[OwnPropertyKeys]] should throw if the trap " + + "implementation returns duplicate properties"; print(BUGNUMBER + ": " + summary); @@ -16,13 +15,17 @@ print(BUGNUMBER + ": " + summary); * BEGIN TEST * **************/ -var target = Object.preventExtensions({ a: 1 }); +var target = {}; var proxy = new Proxy(target, { ownKeys(t) { return ["a", "a"]; } }); -assertDeepEq(Object.getOwnPropertyNames(proxy), ["a", "a"]); +assertThrowsInstanceOf(() => Object.getOwnPropertyNames(proxy), TypeError); + +target = Object.preventExtensions({ a: 1 }); +proxy = new Proxy(target, { ownKeys(t) { return ["a", "a"]; } }); +assertThrowsInstanceOf(() => Object.getOwnPropertyNames(proxy), TypeError); target = Object.freeze({ a: 1 }); proxy = new Proxy(target, { ownKeys(t) { return ["a", "a"]; } }); -assertDeepEq(Object.getOwnPropertyNames(proxy), ["a", "a"]); +assertThrowsInstanceOf(() => Object.getOwnPropertyNames(proxy), TypeError); /******************************************************************************/