Fix stack overflow in object Clone() functions

core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp

Index: core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
diff --git a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
index 8fbaa482bff9df99a6f087ae7d175bc354383a55..f96c9d8f7efc28b2356a0233b305506768b871dd 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
@@ -23,6 +23,8 @@
 namespace {
 
 using ScopedArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>>;
+using ScopedDict =
+    std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>;
 
 void TestArrayAccessors(const CPDF_Array* arr,
                         size_t index,
@@ -744,3 +746,48 @@ TEST(PDFArrayTest, AddReferenceAndGetObjectAt) {
     EXPECT_EQ(indirect_objs[i], arr1->GetDirectObjectAt(i));
   }
 }
+
+TEST(PDFObjectTest, CloneCheckLoop) {

[Lei Zhang, 2016/08/19 16:21:08]

TEST skips SetUp() vs TEST_F?

[Wei Li, 2016/08/19 18:14:57]

Yes, this test should be a standalone unittest, doesn't need all the setups.

+  {
+    // Create an object with a reference loop.
+    ScopedArray arr_obj(new CPDF_Array);
+    // Dictionary object.
+    CPDF_Dictionary* dict_obj = new CPDF_Dictionary;
+    dict_obj->SetAt("arr", arr_obj.get());
+    arr_obj->InsertAt(0, dict_obj);
+
+    // Clone this object to see whether stack overflow will be triggered.
+    ScopedArray cloned_array(arr_obj->Clone()->AsArray());
+    // Cloned object should be the same as the original.
+    ASSERT_TRUE(cloned_array);
+    EXPECT_EQ(1u, cloned_array->GetCount());
+    CPDF_Object* cloned_dict = cloned_array->GetObjectAt(0);

[Lei Zhang, 2016/08/19 16:21:09]

ASSERT_TRUE(cloned_dict);

[Wei Li, 2016/08/19 18:14:57]

Done.

+    ASSERT_TRUE(cloned_dict->IsDictionary());
+    // Recursively referenced object is not cloned.
+    EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectBy("arr"));
+  }
+  {
+    std::unique_ptr<CPDF_IndirectObjectHolder> m_ObjHolder(
+        new CPDF_IndirectObjectHolder());
+    // Create an object with a reference loop.
+    CPDF_Dictionary* dict_obj = new CPDF_Dictionary;
+    CPDF_Array* arr_obj = new CPDF_Array;
+    m_ObjHolder->AddIndirectObject(dict_obj);
+    EXPECT_EQ(1u, dict_obj->GetObjNum());
+    dict_obj->SetAt("arr", arr_obj);
+    arr_obj->InsertAt(0, dict_obj, m_ObjHolder.get());
+    EXPECT_TRUE(arr_obj->GetObjectAt(0)->IsReference());

[Lei Zhang, 2016/08/19 16:21:09]

ASSERT_TRUE, otherwise next line can crash.

ASSERT_TRUE(arr_obj->GetObjectAt(0)) as well.

[Wei Li, 2016/08/19 18:14:57]

Done.

+    EXPECT_EQ(1u, arr_obj->GetObjectAt(0)->AsReference()->GetRefObjNum());
+    EXPECT_EQ(dict_obj, arr_obj->GetObjectAt(0)->AsReference()->GetDirect());
+
+    // Clone this object to see whether stack overflow will be triggered.
+    ScopedDict cloned_dict(ToDictionary(dict_obj->CloneDirectObject()));
+    // Cloned object should be the same as the original.
+    ASSERT_TRUE(cloned_dict);
+    CPDF_Object* cloned_arr = cloned_dict->GetObjectBy("arr");
+    ASSERT_TRUE(cloned_arr && cloned_arr->IsArray());

[Lei Zhang, 2016/08/19 16:21:08]

Separate into 2 ASSERT_TRUE statements.

[Wei Li, 2016/08/19 18:14:57]

Done.

+    EXPECT_EQ(1u, cloned_arr->AsArray()->GetCount());
+    // Recursively referenced object is not cloned.
+    EXPECT_EQ(nullptr, cloned_arr->AsArray()->GetObjectAt(0));
+  }
+}