Add maximum depth when converting V8 object to base::Value

atom/common/native_mate_converters/v8_value_converter.cc

@@ -4,6 +4,7 @@
 
 #include "atom/common/native_mate_converters/v8_value_converter.h"
 
+#include <map>
 #include <string>
 #include <utility>
 
@@ -13,12 +14,70 @@
 
 namespace atom {
 
+namespace {
+
+const int kMaxRecursionDepth = 100;
+
+}  // namespace
+
+// The state of a call to FromV8Value.
+class V8ValueConverter::FromV8ValueState {
+ public:
+  // Level scope which updates the current depth of some FromV8ValueState.
+  class Level {
+   public:
+    explicit Level(FromV8ValueState* state) : state_(state) {
+      state_->max_recursion_depth_--;
+    }
+    ~Level() {
+      state_->max_recursion_depth_++;
+    }
+
+   private:
+    FromV8ValueState* state_;
+  };
+
+  FromV8ValueState() : max_recursion_depth_(kMaxRecursionDepth) {}
+
+  // If |handle| is not in |unique_map_|, then add it to |unique_map_| and
+  // return true.
+  //
+  // Otherwise do nothing and return false. Here "A is unique" means that no
+  // other handle B in the map points to the same object as A. Note that A can
+  // be unique even if there already is another handle with the same identity
+  // hash (key) in the map, because two objects can have the same hash.
+  bool UpdateAndCheckUniqueness(v8::Handle<v8::Object> handle) {
+    typedef HashToHandleMap::const_iterator Iterator;
+    int hash = handle->GetIdentityHash();
+    // We only compare using == with handles to objects with the same identity
+    // hash. Different hash obviously means different objects, but two objects
+    // in a couple of thousands could have the same identity hash.
+    std::pair<Iterator, Iterator> range = unique_map_.equal_range(hash);
+    for (Iterator it = range.first; it != range.second; ++it) {
+      // Operator == for handles actually compares the underlying objects.
+      if (it->second == handle)
+        return false;
+    }
+    unique_map_.insert(std::make_pair(hash, handle));
+    return true;
+  }
+
+  bool HasReachedMaxRecursionDepth() {
+    return max_recursion_depth_ < 0;
+  }
+
+ private:
+  typedef std::multimap<int, v8::Handle<v8::Object> > HashToHandleMap;
+  HashToHandleMap unique_map_;
+
+  int max_recursion_depth_;
+};
+
 V8ValueConverter::V8ValueConverter()
     : date_allowed_(false),
       reg_exp_allowed_(false),
       function_allowed_(false),
-      strip_null_from_objects_(false),
+      strip_null_from_objects_(false) {}
-      avoid_identity_hash_for_testing_(false) {}
 
 void V8ValueConverter::SetDateAllowed(bool val) {
   date_allowed_ = val;
@@ -48,8 +107,8 @@ base::Value* V8ValueConverter::FromV8Value(
     v8::Local<v8::Context> context) const {
   v8::Context::Scope context_scope(context);
   v8::HandleScope handle_scope(context->GetIsolate());
-  HashToHandleMap unique_map;
+  FromV8ValueState state;
-  return FromV8ValueImpl(val, &unique_map);
+  return FromV8ValueImpl(&state, val, context->GetIsolate());
 }
 
 v8::Local<v8::Value> V8ValueConverter::ToV8ValueImpl(
@@ -141,10 +200,16 @@ v8::Local<v8::Value> V8ValueConverter::ToV8Object(
   return result;
 }
 
-base::Value* V8ValueConverter::FromV8ValueImpl(v8::Local<v8::Value> val,
+base::Value* V8ValueConverter::FromV8ValueImpl(
-    HashToHandleMap* unique_map) const {
+    FromV8ValueState* state,
+    v8::Handle<v8::Value> val,
+    v8::Isolate* isolate) const {
   CHECK(!val.IsEmpty());
 
+  FromV8ValueState::Level state_level(state);
+  if (state->HasReachedMaxRecursionDepth())
+    return NULL;
+
   if (val->IsNull())
     return base::Value::CreateNullValue();
 
@@ -170,7 +235,7 @@ base::Value* V8ValueConverter::FromV8ValueImpl(v8::Local<v8::Value> val,
     if (!date_allowed_)
       // JSON.stringify would convert this to a string, but an object is more
       // consistent within this class.
-      return FromV8Object(val->ToObject(), unique_map);
+      return FromV8Object(val->ToObject(), state, isolate);
     v8::Date* date = v8::Date::Cast(*val);
     return new base::FundamentalValue(date->NumberValue() / 1000.0);
   }
@@ -178,35 +243,36 @@ base::Value* V8ValueConverter::FromV8ValueImpl(v8::Local<v8::Value> val,
   if (val->IsRegExp()) {
     if (!reg_exp_allowed_)
       // JSON.stringify converts to an object.
-      return FromV8Object(val->ToObject(), unique_map);
+      return FromV8Object(val->ToObject(), state, isolate);
     return new base::StringValue(*v8::String::Utf8Value(val->ToString()));
   }
 
   // v8::Value doesn't have a ToArray() method for some reason.
   if (val->IsArray())
-    return FromV8Array(val.As<v8::Array>(), unique_map);
+    return FromV8Array(val.As<v8::Array>(), state, isolate);
 
   if (val->IsFunction()) {
     if (!function_allowed_)
       // JSON.stringify refuses to convert function(){}.
       return NULL;
-    return FromV8Object(val->ToObject(), unique_map);
+    return FromV8Object(val->ToObject(), state, isolate);
   }
 
   if (val->IsObject()) {
-    return FromV8Object(val->ToObject(), unique_map);
+    return FromV8Object(val->ToObject(), state, isolate);
   }
 
   LOG(ERROR) << "Unexpected v8 value type encountered.";
   return NULL;
 }
 
-base::Value* V8ValueConverter::FromV8Array(v8::Local<v8::Array> val,
+base::Value* V8ValueConverter::FromV8Array(
-    HashToHandleMap* unique_map) const {
+    v8::Handle<v8::Array> val,
-  if (!UpdateAndCheckUniqueness(unique_map, val))
+    FromV8ValueState* state,
+    v8::Isolate* isolate) const {
+  if (!state->UpdateAndCheckUniqueness(val))
     return base::Value::CreateNullValue();
 
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
   scoped_ptr<v8::Context::Scope> scope;
   // If val was created in a different context than our current one, change to
   // that context, but change back after val is converted.
@@ -228,7 +294,7 @@ base::Value* V8ValueConverter::FromV8Array(v8::Local<v8::Array> val,
     if (!val->HasRealIndexedProperty(i))
       continue;
 
-    base::Value* child = FromV8ValueImpl(child_v8, unique_map);
+    base::Value* child = FromV8ValueImpl(state, child_v8, isolate);
     if (child)
       result->Append(child);
     else
@@ -241,10 +307,11 @@ base::Value* V8ValueConverter::FromV8Array(v8::Local<v8::Array> val,
 
 base::Value* V8ValueConverter::FromV8Object(
     v8::Local<v8::Object> val,
-    HashToHandleMap* unique_map) const {
+    FromV8ValueState* state,
-  if (!UpdateAndCheckUniqueness(unique_map, val))
+    v8::Isolate* isolate) const {
+  if (!state->UpdateAndCheckUniqueness(val))
     return base::Value::CreateNullValue();
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+
   scoped_ptr<v8::Context::Scope> scope;
   // If val was created in a different context than our current one, change to
   // that context, but change back after val is converted.
@@ -281,7 +348,7 @@ base::Value* V8ValueConverter::FromV8Object(
       child_v8 = v8::Null(isolate);
     }
 
-    scoped_ptr<base::Value> child(FromV8ValueImpl(child_v8, unique_map));
+    scoped_ptr<base::Value> child(FromV8ValueImpl(state, child_v8, isolate));
     if (!child.get())
       // JSON.stringify skips properties whose values don't serialize, for
       // example undefined and functions. Emulate that behavior.
@@ -317,24 +384,4 @@ base::Value* V8ValueConverter::FromV8Object(
   return result.release();
 }
 
-bool V8ValueConverter::UpdateAndCheckUniqueness(
-    HashToHandleMap* map,
-    v8::Local<v8::Object> handle) const {
-  typedef HashToHandleMap::const_iterator Iterator;
-
-  int hash = avoid_identity_hash_for_testing_ ? 0 : handle->GetIdentityHash();
-  // We only compare using == with handles to objects with the same identity
-  // hash. Different hash obviously means different objects, but two objects in
-  // a couple of thousands could have the same identity hash.
-  std::pair<Iterator, Iterator> range = map->equal_range(hash);
-  for (Iterator it = range.first; it != range.second; ++it) {
-    // Operator == for handles actually compares the underlying objects.
-    if (it->second == handle)
-      return false;
-  }
-
-  map->insert(std::pair<int, v8::Local<v8::Object> >(hash, handle));
-  return true;
-}
-
 }  // namespace atom

atom/common/native_mate_converters/v8_value_converter.h

@@ -5,8 +5,6 @@
 #ifndef ATOM_COMMON_NATIVE_MATE_CONVERTERS_V8_VALUE_CONVERTER_H_
 #define ATOM_COMMON_NATIVE_MATE_CONVERTERS_V8_VALUE_CONVERTER_H_
 
-#include <map>
-
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "v8/include/v8.h"
@@ -34,7 +32,7 @@ class V8ValueConverter {
                            v8::Local<v8::Context> context) const;
 
  private:
-  typedef std::multimap<int, v8::Local<v8::Object> > HashToHandleMap;
+  class FromV8ValueState;
 
   v8::Local<v8::Value> ToV8ValueImpl(v8::Isolate* isolate,
                                      const base::Value* value) const;
@@ -44,21 +42,16 @@ class V8ValueConverter {
       v8::Isolate* isolate,
       const base::DictionaryValue* dictionary) const;
 
-  base::Value* FromV8ValueImpl(v8::Local<v8::Value> value,
+  base::Value* FromV8ValueImpl(FromV8ValueState* state,
-                               HashToHandleMap* unique_map) const;
+                               v8::Handle<v8::Value> value,
-  base::Value* FromV8Array(v8::Local<v8::Array> array,
+                               v8::Isolate* isolate) const;
-                           HashToHandleMap* unique_map) const;
+  base::Value* FromV8Array(v8::Handle<v8::Array> array,
+                           FromV8ValueState* state,
+                           v8::Isolate* isolate) const;
 
   base::Value* FromV8Object(v8::Local<v8::Object> object,
-                            HashToHandleMap* unique_map) const;
+                            FromV8ValueState* state,
-
+                            v8::Isolate* isolate) const;
-  // If |handle| is not in |map|, then add it to |map| and return true.
-  // Otherwise do nothing and return false. Here "A is unique" means that no
-  // other handle B in the map points to the same object as A. Note that A can
-  // be unique even if there already is another handle with the same identity
-  // hash (key) in the map, because two objects can have the same hash.
-  bool UpdateAndCheckUniqueness(HashToHandleMap* map,
-                                v8::Local<v8::Object> handle) const;
 
   // If true, we will convert Date JavaScript objects to doubles.
   bool date_allowed_;
@@ -73,8 +66,6 @@ class V8ValueConverter {
   // into Values.
   bool strip_null_from_objects_;
 
-  bool avoid_identity_hash_for_testing_;
-
   DISALLOW_COPY_AND_ASSIGN(V8ValueConverter);
 };