Changeset 244330 in webkit
- Timestamp:
- Apr 16, 2019 8:58:59 AM (5 years ago)
- Location:
- trunk
- Files:
-
- 8 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChangeLog
r244324 r244330 1 2019-04-16 Caitlin Potter <caitp@igalia.com> 2 3 [JSC] Filter DontEnum properties in ProxyObject::getOwnPropertyNames() 4 https://bugs.webkit.org/show_bug.cgi?id=176810 5 6 Reviewed by Saam Barati. 7 8 Add tests for the DontEnum filtering, and variations of other tests 9 take the DontEnum-filtering path. 10 11 * stress/proxy-own-keys.js: 12 (i.catch): 13 (set assert): 14 (set add): 15 (let.set new): 16 (get let): 17 1 18 2019-04-15 Saam barati <sbarati@apple.com> 2 19 -
trunk/JSTests/stress/proxy-own-keys.js
r244020 r244330 136 136 called = false; 137 137 } 138 139 for (let i = 0; i < 500; i++) { 140 let threw = false; 141 let foundKey = false; 142 try { 143 for (let k in proxy) 144 foundKey = true; 145 } catch(e) { 146 threw = true; 147 assert(e.toString() === "TypeError: Proxy object's non-extensible 'target' has configurable property 'x' that was not in the result from the 'ownKeys' trap"); 148 assert(!foundKey); 149 } 150 assert(threw); 151 assert(called); 152 called = false; 153 } 138 154 } 139 155 … … 165 181 assert(threw); 166 182 assert(called); 183 called = false; 184 } 185 186 for (let i = 0; i < 500; i++) { 187 let threw = false; 188 let reached = false; 189 try { 190 for (let k in proxy) 191 reached = true; 192 } catch (e) { 193 threw = true; 194 assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"); 195 } 196 assert(threw); 197 assert(called); 198 assert(!reached); 167 199 called = false; 168 200 } … … 668 700 } 669 701 } 702 703 { 704 let error = null; 705 let s1 = Symbol(); 706 let s2 = Symbol(); 707 let target = Object.defineProperties({}, { 708 x: { 709 value: "X", 710 enumerable: true, 711 configurable: true, 712 }, 713 dontEnum1: { 714 value: "dont-enum", 715 enumerable: false, 716 configurable: true, 717 }, 718 y: { 719 get() { return "Y"; }, 720 enumerable: true, 721 configurable: true, 722 }, 723 dontEnum2: { 724 get() { return "dont-enum-accessor" }, 725 enumerable: false, 726 configurable: true 727 }, 728 [s1]: { 729 value: "s1", 730 enumerable: true, 731 configurable: true, 732 }, 733 [s2]: { 734 value: "dont-enum-symbol", 735 enumerable: false, 736 configurable: true, 737 }, 738 }); 739 let checkedOwnKeys = false; 740 let checkedPropertyDescriptor = false; 741 let handler = { 742 ownKeys() { 743 checkedOwnKeys = true; 744 return ["x", "dontEnum1", "y", "dontEnum2", s1, s2]; 745 }, 746 getOwnPropertyDescriptor(t, k) { 747 checkedPropertyDescriptors = true; 748 return Reflect.getOwnPropertyDescriptor(t, k); 749 } 750 }; 751 let proxy = new Proxy(target, handler); 752 for (let i = 0; i < 500; i++) { 753 checkedPropertyDescriptors = false; 754 assert(shallowEq(["x", "dontEnum1", "y", "dontEnum2", s1, s2], Reflect.ownKeys(proxy))); 755 assert(checkedOwnKeys); 756 assert(!checkedPropertyDescriptors); 757 checkedOwnKeys = false; 758 759 let enumerableStringKeys = []; 760 for (let k in proxy) 761 enumerableStringKeys.push(k); 762 assert(shallowEq(["x", "y"], enumerableStringKeys)); 763 assert(checkedOwnKeys); 764 assert(checkedPropertyDescriptors); 765 } 766 } -
trunk/Source/JavaScriptCore/ChangeLog
r244324 r244330 1 2019-04-16 Caitlin Potter <caitp@igalia.com> 2 3 [JSC] Filter DontEnum properties in ProxyObject::getOwnPropertyNames() 4 https://bugs.webkit.org/show_bug.cgi?id=176810 5 6 Reviewed by Saam Barati. 7 8 This adds conditional logic following the invariant checks, to perform 9 filtering in common uses of getOwnPropertyNames. 10 11 While this would ideally only be done in JSPropertyNameEnumerator, adding 12 the filtering to ProxyObject::performGetOwnPropertyNames maintains the 13 invariant that the EnumerationMode is properly followed. 14 15 This was originally rolled out in r244020, as DontEnum filtering code 16 in ObjectConstructor.cpp's ownPropertyKeys() had not been removed. It's 17 now redundant due to being handled in ProxyObject::getOwnPropertyNames(). 18 19 * runtime/PropertyNameArray.h: 20 (JSC::PropertyNameArray::reset): 21 * runtime/ProxyObject.cpp: 22 (JSC::ProxyObject::performGetOwnPropertyNames): 23 1 24 2019-04-15 Saam barati <sbarati@apple.com> 2 25 -
trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
r244136 r244330 921 921 RETURN_IF_EXCEPTION(scope, nullptr); 922 922 923 // https://tc39.github.io/ecma262/#sec-enumerableownproperties924 // If {object} is a Proxy, an explicit and observable [[GetOwnProperty]] op is required to filter out non-enumerable properties.925 // In other cases, filtering has already been performed.926 const bool mustFilterProperty = dontEnumPropertiesMode == DontEnumPropertiesMode::Exclude && object->type() == ProxyObjectType;927 auto filterPropertyIfNeeded = [exec, object, mustFilterProperty](const Identifier& identifier) {928 if (!mustFilterProperty)929 return true;930 PropertyDescriptor descriptor;931 PropertyName name(identifier);932 return object->getOwnPropertyDescriptor(exec, name, descriptor) && descriptor.enumerable();933 };934 935 // If !mustFilterProperty and PropertyNameMode::Strings mode, we do not need to filter out any entries in PropertyNameArray.936 // We can use fast allocation and initialization.937 923 if (propertyNameMode != PropertyNameMode::StringsAndSymbols) { 938 924 ASSERT(propertyNameMode == PropertyNameMode::Strings || propertyNameMode == PropertyNameMode::Symbols); 939 if ( !mustFilterProperty &&properties.size() < MIN_SPARSE_ARRAY_INDEX) {925 if (properties.size() < MIN_SPARSE_ARRAY_INDEX) { 940 926 if (LIKELY(!globalObject->isHavingABadTime())) { 941 927 if (isObjectKeys) { … … 994 980 const auto& identifier = properties[i]; 995 981 ASSERT(!identifier.isSymbol()); 996 bool hasProperty = filterPropertyIfNeeded(identifier); 997 EXCEPTION_ASSERT(!scope.exception() || !hasProperty); 998 if (hasProperty) 999 pushDirect(exec, keys, jsOwnedString(exec, identifier.string())); 982 pushDirect(exec, keys, jsOwnedString(exec, identifier.string())); 1000 983 RETURN_IF_EXCEPTION(scope, nullptr); 1001 984 } … … 1009 992 ASSERT(identifier.isSymbol()); 1010 993 ASSERT(!identifier.isPrivateName()); 1011 bool hasProperty = filterPropertyIfNeeded(identifier); 1012 EXCEPTION_ASSERT(!scope.exception() || !hasProperty); 1013 if (hasProperty) 1014 pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()))); 994 pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()))); 1015 995 RETURN_IF_EXCEPTION(scope, nullptr); 1016 996 } … … 1029 1009 } 1030 1010 1031 bool hasProperty = filterPropertyIfNeeded(identifier); 1032 EXCEPTION_ASSERT(!scope.exception() || !hasProperty); 1033 if (hasProperty) 1034 pushDirect(exec, keys, jsOwnedString(exec, identifier.string())); 1011 pushDirect(exec, keys, jsOwnedString(exec, identifier.string())); 1035 1012 RETURN_IF_EXCEPTION(scope, nullptr); 1036 1013 } … … 1038 1015 // To ensure the order defined in the spec (9.1.12), we append symbols at the last elements of keys. 1039 1016 for (const auto& identifier : propertySymbols) { 1040 bool hasProperty = filterPropertyIfNeeded(identifier); 1041 EXCEPTION_ASSERT(!scope.exception() || !hasProperty); 1042 if (hasProperty) 1043 pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()))); 1017 pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()))); 1044 1018 RETURN_IF_EXCEPTION(scope, nullptr); 1045 1019 } -
trunk/Source/JavaScriptCore/runtime/PropertyNameArray.h
r244020 r244330 54 54 , m_privateSymbolMode(privateSymbolMode) 55 55 { 56 } 57 58 void reset() 59 { 60 m_set.clear(); 61 m_data = PropertyNameArrayData::create(); 56 62 } 57 63 -
trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp
r244020 r244330 1007 1007 } 1008 1008 1009 if (targetIsExensible) 1010 return; 1011 1012 for (UniquedStringImpl* impl : targetConfigurableKeys) { 1013 if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) { 1014 throwVMTypeError(exec, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap")); 1009 if (!targetIsExensible) { 1010 for (UniquedStringImpl* impl : targetConfigurableKeys) { 1011 if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) { 1012 throwVMTypeError(exec, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap")); 1013 return; 1014 } 1015 } 1016 1017 if (uncheckedResultKeys.size()) { 1018 throwVMTypeError(exec, scope, "Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"_s); 1015 1019 return; 1016 1020 } 1017 1021 } 1018 1022 1019 if (uncheckedResultKeys.size()) { 1020 throwVMTypeError(exec, scope, "Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"_s); 1021 return; 1023 if (!enumerationMode.includeDontEnumProperties()) { 1024 // Filtering DontEnum properties is observable in proxies and must occur following the invariant checks above. 1025 auto data = trapResult.releaseData(); 1026 trapResult.reset(); 1027 1028 for (auto propertyName : data->propertyNameVector()) { 1029 PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty); 1030 auto result = getOwnPropertySlotCommon(exec, propertyName, slot); 1031 RETURN_IF_EXCEPTION(scope, void()); 1032 if (!result) 1033 continue; 1034 if (slot.attributes() & PropertyAttribute::DontEnum) 1035 continue; 1036 trapResult.addUnchecked(propertyName.impl()); 1037 } 1022 1038 } 1023 1039 } -
trunk/Source/WebCore/ChangeLog
r244328 r244330 1 2019-04-16 Caitlin Potter <caitp@igalia.com> 2 3 [JSC] Filter DontEnum properties in ProxyObject::getOwnPropertyNames() 4 https://bugs.webkit.org/show_bug.cgi?id=176810 5 6 Reviewed by Saam Barati. 7 8 Previously, there was a comment here indicating uncertainty of whether it 9 was necessary to filter DontEnum properties explicitly or not. It turns 10 out that it was necessary in the case of JSC ProxyObjects. 11 12 This patch adds DontEnum filtering for ProxyObjects, however we continue 13 to explicitly filter them in JSDOMConvertRecord, which needs to use the 14 property descriptor after filtering. This change prevents observably 15 fetching the property descriptor twice per property. 16 17 * bindings/js/JSDOMConvertRecord.h: 18 1 19 2019-04-15 Antoine Quint <graouts@apple.com> 2 20 -
trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h
r244020 r244330 87 87 // 4. Let keys be ? O.[[OwnPropertyKeys]](). 88 88 JSC::PropertyNameArray keys(&vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude); 89 object->methodTable(vm)->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode( ));89 object->methodTable(vm)->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode(JSC::DontEnumPropertiesMode::Include)); 90 90 91 91 RETURN_IF_EXCEPTION(scope, { }); … … 100 100 // 2. If desc is not undefined and desc.[[Enumerable]] is true: 101 101 102 // FIXME: Do we need to check for enumerable / undefined, or is this handled by the default 103 // enumeration mode? 104 102 // It's necessary to filter enumerable here rather than using the default EnumerationMode, 103 // to prevent an observable extra [[GetOwnProperty]] operation in the case of ProxyObject records. 105 104 if (didGetDescriptor && descriptor.enumerable()) { 106 105 // 1. Let typedKey be key converted to an IDL value of type K.
Note: See TracChangeset
for help on using the changeset viewer.