[keys] fixing nested JSProxy for-in enumeration

src/keys.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/keys.h"
 
 #include "src/api-arguments.h"
 #include "src/elements.h"
 #include "src/factory.h"
 #include "src/identity-map.h"
@@ skipping 95 matching lines @@
   return AddKey(handle(key, isolate_), convert);
 }
 
 bool KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
   if (key->IsSymbol()) {
     if (filter_ & SKIP_SYMBOLS) return false;
     if (Handle<Symbol>::cast(key)->is_private()) return false;
     return AddSymbolKey(key);
   }
   if (filter_ & SKIP_STRINGS) return false;
-  // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy).
+  // Make sure we do not add keys to a proxy-level (see AddKeysFromJSProxy).
   DCHECK_LE(0, level_string_length_);
   // In some cases (e.g. proxies) we might get in String-converted ints which
   // should be added to the elements list instead of the properties. For
   // proxies we have to convert as well but also respect the original order.
   // Therefore we add a converted key to both sides
   if (convert == CONVERT_TO_ARRAY_INDEX || convert == PROXY_MAGIC) {
     uint32_t index = 0;
     int prev_length = length_;
     int prev_proto = level_string_length_;
     if ((key->IsString() && Handle<String>::cast(key)->AsArrayIndex(&index)) ||
@@ skipping 11 matching lines @@
         level_string_length_ = prev_proto;
       }
     }
   }
   return AddStringKey(key, convert);
 }
 
 bool KeyAccumulator::AddKey(uint32_t key) { return AddIntegerKey(key); }
 
 bool KeyAccumulator::AddIntegerKey(uint32_t key) {
-  // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy).
+  // Make sure we do not add keys to a proxy-level (see AddKeysFromJSProxy).
   // We mark proxy-levels with a negative length
   DCHECK_LE(0, level_string_length_);
   // Binary search over all but the last level. The last one might not be
   // sorted yet.
   for (size_t i = 1; i < elements_.size(); i++) {
     if (AccumulatorHasKey(elements_[i - 1], key)) return false;
   }
   elements_.back()->push_back(key);
   length_++;
   return true;
@@ skipping 45 matching lines @@
   }
 }
 
 void KeyAccumulator::AddKeys(Handle<JSObject> array_like,
                              AddKeyConversion convert) {
   DCHECK(array_like->IsJSArray() || array_like->HasSloppyArgumentsElements());
   ElementsAccessor* accessor = array_like->GetElementsAccessor();
   accessor->AddElementsToKeyAccumulator(array_like, this, convert);
 }
 
-void KeyAccumulator::AddKeysFromProxy(Handle<JSObject> array_like) {
-  // Proxies define a complete list of keys with no distinction of
-  // elements and properties, which breaks the normal assumption for the
-  // KeyAccumulator.
-  AddKeys(array_like, PROXY_MAGIC);
-  // Invert the current length to indicate a present proxy, so we can ignore
-  // element keys for this level. Otherwise we would not fully respect the order
-  // given by the proxy.
-  level_string_length_ = -level_string_length_;
-}
-
 MaybeHandle<FixedArray> FilterProxyKeys(Isolate* isolate, Handle<JSProxy> owner,
                                         Handle<FixedArray> keys,
                                         PropertyFilter filter) {
   if (filter == ALL_PROPERTIES) {
     // Nothing to do.
     return keys;
   }
   int store_position = 0;
   for (int i = 0; i < keys->length(); ++i) {
     Handle<Name> key(Name::cast(keys->get(i)), isolate);
@@ skipping 10 matching lines @@
       keys->set(store_position, *key);
     }
     store_position++;
   }
   if (store_position == 0) return isolate->factory()->empty_fixed_array();
   keys->Shrink(store_position);
   return keys;
 }
 
 // Returns "nothing" in case of exception, "true" on success.
-Maybe<bool> KeyAccumulator::AddKeysFromProxy(Handle<JSProxy> proxy,
-                                             Handle<FixedArray> keys) {
+Maybe<bool> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> proxy,
+                                               Handle<FixedArray> keys) {
   if (filter_proxy_keys_) {
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(
         isolate_, keys, FilterProxyKeys(isolate_, proxy, keys, filter_),
         Nothing<bool>());
   }
   // Proxies define a complete list of keys with no distinction of
   // elements and properties, which breaks the normal assumption for the
   // KeyAccumulator.
   if (type_ == OWN_ONLY) {
     ownProxyKeys_ = keys;
@@ skipping 374 matching lines @@
       PropertyDetails details = descs->GetDetails(i);
       if ((details.attributes() & filter_) != 0) continue;
       if (filter_ & ONLY_ALL_CAN_READ) {
         if (details.kind() != kAccessor) continue;
         Object* accessors = descs->GetValue(i);
         if (!accessors->IsAccessorInfo()) continue;
         if (!AccessorInfo::cast(accessors)->all_can_read()) continue;
       }
       Name* key = descs->GetKey(i);
       if (key->FilterKey(filter_)) continue;
-      this->AddKey(key, DO_NOT_CONVERT);
+      AddKey(key, DO_NOT_CONVERT);
     }
   } else if (object->IsJSGlobalObject()) {
     GlobalDictionary::CollectKeysTo(
         handle(object->global_dictionary(), isolate_), this, filter_);
   } else {
     NameDictionary::CollectKeysTo(
         handle(object->property_dictionary(), isolate_), this, filter_);
   }
 }
 
 // Returns |true| on success, |false| if prototype walking should be stopped,
 // |nothing| if an exception was thrown.
 Maybe<bool> KeyAccumulator::CollectOwnKeys(Handle<JSReceiver> receiver,
                                            Handle<JSObject> object) {
-  this->NextPrototype();
+  NextPrototype();
   // Check access rights if required.
   if (object->IsAccessCheckNeeded() &&
       !isolate_->MayAccess(handle(isolate_->context()), object)) {
     // The cross-origin spec says that [[Enumerate]] shall return an empty
     // iterator when it doesn't have access...
     if (type_ == INCLUDE_PROTOS) {
       return Just(false);
     }
     // ...whereas [[OwnPropertyKeys]] shall return whitelisted properties.
     DCHECK_EQ(OWN_ONLY, type_);
     filter_ = static_cast<PropertyFilter>(filter_ | ONLY_ALL_CAN_READ);
   }
 
-  this->CollectOwnElementIndices(object);
+  CollectOwnElementIndices(object);
 
   // Add the element keys from the interceptor.
   Maybe<bool> success =
       GetKeysFromInterceptor<v8::IndexedPropertyEnumeratorCallback, kIndexed>(
           receiver, object, this);
   MAYBE_RETURN(success, Nothing<bool>());
 
   if (filter_ == ENUMERABLE_STRINGS) {
     Handle<FixedArray> enum_keys =
         KeyAccumulator::GetEnumPropertyKeys(isolate_, object);
-    this->AddKeys(enum_keys, DO_NOT_CONVERT);
+    AddKeys(enum_keys, DO_NOT_CONVERT);
   } else {
-    this->CollectOwnPropertyNames(object);
+    CollectOwnPropertyNames(object);
   }
 
   // Add the property keys from the interceptor.
   success = GetKeysFromInterceptor<v8::GenericNamedPropertyEnumeratorCallback,
                                    kNamed>(receiver, object, this);
   MAYBE_RETURN(success, Nothing<bool>());
   return Just(true);
 }
 
 // static
@@ skipping 40 matching lines @@
   Handle<JSReceiver> target(proxy->target(), isolate_);
   // 5. Let trap be ? GetMethod(handler, "ownKeys").
   Handle<Object> trap;
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
       isolate_, trap, Object::GetMethod(Handle<JSReceiver>::cast(handler),
                                         isolate_->factory()->ownKeys_string()),
       Nothing<bool>());
   // 6. If trap is undefined, then
   if (trap->IsUndefined()) {
     // 6a. Return target.[[OwnPropertyKeys]]().
-    KeyCollectionType previous_type = type_;
-    type_ = OWN_ONLY;
-    Maybe<bool> result = this->CollectKeys(receiver, target);
-    type_ = previous_type;
-    return result;
+    return CollectOwnJSProxyTargetKeys(proxy, target);
   }
   // 7. Let trapResultArray be Call(trap, handler, «target»).
   Handle<Object> trap_result_array;
   Handle<Object> args[] = {target};
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
       isolate_, trap_result_array,
       Execution::Call(isolate_, trap, handler, arraysize(args), args),
       Nothing<bool>());
   // 8. Let trapResult be ? CreateListFromArrayLike(trapResultArray,
   //    «String, Symbol»).
@@ skipping 34 matching lines @@
                                        target_keys->get(i));
       nonconfigurable_keys_length++;
       // The key was moved, null it out in the original list.
       target_keys->set(i, Smi::FromInt(0));
     } else {
       // 14c. Else,
       // 14c i. Append key as an element of targetConfigurableKeys.
       // (No-op, just keep it in |target_keys|.)
     }
   }
-  this->NextPrototype();  // Prepare for accumulating keys.
+  NextPrototype();  // Prepare for accumulating keys.
   // 15. If extensibleTarget is true and targetNonconfigurableKeys is empty,
   //     then:
   if (extensible_target && nonconfigurable_keys_length == 0) {
     // 15a. Return trapResult.
-    return this->AddKeysFromProxy(proxy, trap_result);
+    return AddKeysFromJSProxy(proxy, trap_result);
   }
   // 16. Let uncheckedResultKeys be a new List which is a copy of trapResult.
   Zone set_zone(isolate_->allocator());
   const int kPresent = 1;
   const int kGone = 0;
   IdentityMap<int> unchecked_result_keys(isolate_->heap(), &set_zone);
   int unchecked_result_keys_size = 0;
   for (int i = 0; i < trap_result->length(); ++i) {
     DCHECK(trap_result->get(i)->IsUniqueName());
     Object* key = trap_result->get(i);
@@ skipping 13 matching lines @@
       isolate_->Throw(*isolate_->factory()->NewTypeError(
           MessageTemplate::kProxyOwnKeysMissing, handle(key, isolate_)));
       return Nothing<bool>();
     }
     // 17b. Remove key from uncheckedResultKeys.
     *found = kGone;
     unchecked_result_keys_size--;
   }
   // 18. If extensibleTarget is true, return trapResult.
   if (extensible_target) {
-    return this->AddKeysFromProxy(proxy, trap_result);
+    return AddKeysFromJSProxy(proxy, trap_result);
   }
   // 19. Repeat, for each key that is an element of targetConfigurableKeys:
   for (int i = 0; i < target_configurable_keys->length(); ++i) {
     Object* key = target_configurable_keys->get(i);
     if (key->IsSmi()) continue;  // Zapped entry, was nonconfigurable.
     // 19a. If key is not an element of uncheckedResultKeys, throw a
     //      TypeError exception.
     int* found = unchecked_result_keys.Find(key);
     if (found == nullptr || *found == kGone) {
       isolate_->Throw(*isolate_->factory()->NewTypeError(
           MessageTemplate::kProxyOwnKeysMissing, handle(key, isolate_)));
       return Nothing<bool>();
     }
     // 19b. Remove key from uncheckedResultKeys.
     *found = kGone;
     unchecked_result_keys_size--;
   }
   // 20. If uncheckedResultKeys is not empty, throw a TypeError exception.
   if (unchecked_result_keys_size != 0) {
     DCHECK_GT(unchecked_result_keys_size, 0);
     isolate_->Throw(*isolate_->factory()->NewTypeError(
         MessageTemplate::kProxyOwnKeysNonExtensible));
     return Nothing<bool>();
   }
   // 21. Return trapResult.
-  return this->AddKeysFromProxy(proxy, trap_result);
+  return AddKeysFromJSProxy(proxy, trap_result);
+}
+
+Maybe<bool> KeyAccumulator::CollectOwnJSProxyTargetKeys(
+    Handle<JSProxy> proxy, Handle<JSReceiver> target) {
+  // TODO(cbruni): avoid creating another KeyAccumulator
+  Handle<FixedArray> keys;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate_, keys, JSReceiver::OwnPropertyKeys(target), Nothing<bool>());
+  NextPrototype();  // Prepare for accumulating keys.
+  bool prev_filter_proxy_keys_ = filter_proxy_keys_;
+  filter_proxy_keys_ = false;
+  Maybe<bool> result = AddKeysFromJSProxy(proxy, keys);
+  filter_proxy_keys_ = prev_filter_proxy_keys_;
+  return result;
 }
 
 }  // namespace internal
 }  // namespace v8