Limit depth of parsing of dictionaries returned by JS evaluation.

ios/web/web_state/ui/web_view_js_utils_unittest.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/web/web_state/ui/web_view_js_utils.h"
 
+#include "base/callback_helpers.h"
 #include "base/logging.h"
+#include "base/mac/bind_objc_block.h"
 #include "base/values.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace web {
 
 // Tests that ValueResultFromWKResult converts nil value to nullptr.
 TEST(WebViewJsUtilsTest, ValueResultFromUndefinedWKResult) {
   EXPECT_FALSE(ValueResultFromWKResult(nil));
 }
 
@@ skipping 43 matching lines @@
 TEST(WebViewJsUtilsTest, ValueResultFromNullWKResult) {
   std::unique_ptr<base::Value> value(
       web::ValueResultFromWKResult([NSNull null]));
   EXPECT_TRUE(value);
   EXPECT_EQ(base::Value::TYPE_NULL, value->GetType());
 }
 
 // Tests that ValueResultFromWKResult converts NSDictionaries to properly
 // initialized base::DictionaryValue.
 TEST(WebViewJsUtilsTest, ValueResultFromDictionaryWKResult) {
   NSDictionary* testDictionary =

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

Since you touching this file, could you please s/testDictionary/test_dictionary

[jif, 2016/09/13 09:41:10]

Done.

       @{ @"Key1" : @"Value1",
          @"Key2" : @{@"Key3" : @42} };
 
   std::unique_ptr<base::Value> value(
       web::ValueResultFromWKResult(testDictionary));
   base::DictionaryValue* dictionary = nullptr;
   value->GetAsDictionary(&dictionary);
   EXPECT_NE(nullptr, dictionary);
 
   std::string value1;
   dictionary->GetString("Key1", &value1);
   EXPECT_EQ("Value1", value1);
 
   base::DictionaryValue const* innerDictionary = nullptr;

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

And s/innerDictionary/inner_dictionary

[jif, 2016/09/13 09:41:10]

Done.

   dictionary->GetDictionary("Key2", &innerDictionary);
   EXPECT_NE(nullptr, innerDictionary);
 
   double value3;
   innerDictionary->GetDouble("Key3", &value3);
   EXPECT_EQ(42, value3);
 }
 
+// Tests that an NSDictionary with a cycle does not cause infinite recursion.
+TEST(WebViewJsUtilsTest, ValueResultFromDictionaryWithDepthCheckWKResult) {
+  // Creates a dictionary with a cycle.

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

NIT: s/Creates/Create

We use imperative form for code comments and descriptive for API comments.

[jif, 2016/09/13 09:41:10]

Done. Thanks for the reminder :)

+  NSMutableDictionary* testDictionary =

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

s/testDictionary/test_dictionary

Also obj_c_key, current_dictionary, inner_dictionary, current_depth

[jif, 2016/09/13 09:41:09]

Done.

+      [NSMutableDictionary dictionaryWithCapacity:1];
+  NSMutableDictionary* testDictionary2 =
+      [NSMutableDictionary dictionaryWithCapacity:1];
+  const char* key = "key";
+  NSString* objCKey =
+      [NSString stringWithCString:key encoding:NSASCIIStringEncoding];
+  [testDictionary setObject:testDictionary2 forKey:objCKey];

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

Do you want to use subscript?: 

test_dictionary[obj_c_key] = test_dictionary2;

[jif, 2016/09/13 09:41:09]

Done.

+  [testDictionary2 setObject:testDictionary forKey:objCKey];
+  // Breaks the retain cycle so that the dictionaries are freed.
+  base::ScopedClosureRunner runner(base::BindBlock(^{
+    [testDictionary2 removeAllObjects];
+  }));
+
+  // Checks that parsing the dictionary stopped at a depth of
+  // |kMaximumParsingRecursionDepth|.
+  std::unique_ptr<base::Value> value =
+      web::ValueResultFromWKResult(testDictionary);
+  base::DictionaryValue* currentDictionary = nullptr;
+  base::DictionaryValue* innerDictionary = nullptr;
+
+  value->GetAsDictionary(&currentDictionary);
+  EXPECT_NE(nullptr, currentDictionary);
+
+  int currentDepth = 0;
+  while (currentDepth <= kMaximumParsingRecursionDepth) {

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

Should this be a for loop?:
for (int current_depth = 0; current_depth < kMaximumParsingRecursionDepth;
current_depth)

[Eugene But (OOO till 7-30), 2016/09/12 16:12:00]

How about using very large number instead of kMaximumParsingRecursionDepth? If
someone change kMaximumParsingRecursionDepth to 10000 this test should ideally
catch the stack overflow. 

[jif, 2016/09/13 09:41:09]

If someone changes kMaximumParsingRecursionDepth to 10000, then the stack
overflow will occur when calling ValueResultFromWKResult in line 115-116.

[jif, 2016/09/13 09:41:10]

Done.

+    EXPECT_NE(nullptr, currentDictionary);
+    innerDictionary = nullptr;
+    currentDictionary->GetDictionary(key, &innerDictionary);
+    currentDictionary = innerDictionary;
+    currentDepth++;
+  }
+  EXPECT_EQ(nullptr, currentDictionary);
+}
+
 }  // namespace web