Fix stack overflow in object Clone() functions

Fix stack overflow in object Clone() functions

For some complex objects such as CPDF_Dictionary, CPDF_Array,
CPDF_Stream, and CPDF_Reference, Clone() could be executed with
infinite recursion to cause the stack overflow. Fix this by
checking already cloned objects to avoid recursion.

BUG=pdfium:513
Committed: https://pdfium.googlesource.com/pdfium/+/a470b5e5371d0674d06068ec38d0d3c3279e85e1

[Wei Li, 2016-08-15 22:32:54]

Description was changed from

==========
Fixed stack overflow in object Clone() functions

For some complex objects such as CPDF_Dictionary, CPDF_Array,
CPDF_Stream, and CPDF_Reference, Clone() could be executed with
infinite recursion to cause the stack overflow. Fix this by
checking already cloned objects to avoid recursion.

BUG=pdfium:513
==========

to

==========
Fix stack overflow in object Clone() functions

For some complex objects such as CPDF_Dictionary, CPDF_Array,
CPDF_Stream, and CPDF_Reference, Clone() could be executed with
infinite recursion to cause the stack overflow. Fix this by
checking already cloned objects to avoid recursion.

BUG=pdfium:513
==========

[Wei Li, 2016-08-15 23:20:10]

Patchset #1 (id:1) has been deleted

[Wei Li, 2016-08-16 18:54:27]

Patchset #4 (id:80001) has been deleted

[Wei Li, 2016-08-17 17:15:14]

Patchset #4 (id:100001) has been deleted

[Wei Li, 2016-08-17 17:20:30]

Patchset #4 (id:120001) has been deleted

[Wei Li, 2016-08-17 19:12:47]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 19:13:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 19:21:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-17 19:21:29]

Dry run: Try jobs failed on following builders:
  mac_xfa_rel_gyp on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa_rel_gyp...)

[Wei Li, 2016-08-17 19:53:15]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 19:53:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 19:55:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-17 19:55:30]

Dry run: Try jobs failed on following builders:
  mac_xfa_rel_gyp on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa_rel_gyp...)
  win on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win/builds/1714)

[Wei Li, 2016-08-17 20:06:10]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 20:06:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 20:41:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-17 20:41:49]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2016-08-17 21:41:05]

weili@chromium.org changed reviewers:
+ dsinclair@chromium.org, thestig@chromium.org

[Wei Li, 2016-08-17 21:41:05]

pls review, thanks

[dsinclair, 2016-08-18 14:04:31]

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_array.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_array.cpp:45: CPDF_Object*
CPDF_Array::CloneWithCheck(
Not a huge fan of the name as WithCheck doesn't tell me anything. CloneNonCyclic
(which is terrible, but I can't think of anything better)

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:58: if
(!pdfium::ContainsKey(*pVisited, value))
nit: add {}'s

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp:16:
pair.second->Release();
I believe this should be Destroy(). The Release() method in CPDF_Object won't
actually do anything if there is an object number assigned. So, we'd end up
leaking all of the objects.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_object.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object.cpp:25: CPDF_Object*
CPDF_Object::CloneDeRef(bool bDirect) const {
Oh, I though this was clone dereference. I think this one should be renamed too.

Can this be the public CloneNonCyclic and then CloneWithCheck becomes protected
CloneNOnCyclicInternal? or something?

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object.cpp:40: m_ObjNum = kInvalidObjNum;
The Destroy() call is going to delete the object so no need to reset the
m_ObjNum

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_stream.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_stream.cpp:87: if (pDict &&
!pdfium::ContainsKey(*pVisited, pDict))
nit: {}'s

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_stream.cpp:88: pDict =
ToDictionary(static_cast<const CPDF_Object*>(pDict)->CloneWithCheck(
Why static_cast to Object?

[Lei Zhang, 2016-08-18 15:16:44]

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:57: auto value = it.second;
auto* ?

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:762: EXPECT_EQ(1u,
cloned_array->GetCount());
ASSERT_TRUE(cloned_array); ditto below with |cloned_dict|

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:766: EXPECT_EQ(nullptr,
cloned_dict->AsDictionary()->GetObjectBy("arr"));
Or just EXPECT_FALSE(...);

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_array.h (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_array.h:70: ~CPDF_Array() override;
nit: Separator between ctor/dtor and other methods. Ditto in other headers.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_object.h (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_object.h:44: virtual CPDF_Object* Clone()
const = 0;
It would be good to document what the different Clone methods do.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_object.h:45: virtual CPDF_Object*
CloneDeRef(bool bDirect) const;
Does anyone override this?

[Wei Li, 2016-08-18 16:37:30]

Patchset #9 (id:240001) has been deleted

[Wei Li, 2016-08-18 21:13:23]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-18 21:13:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 21:26:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 21:26:44]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2016-08-18 22:02:30]

ptal, thanks

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_array.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_array.cpp:45: CPDF_Object*
CPDF_Array::CloneWithCheck(
On 2016/08/18 14:04:31, dsinclair wrote:
> Not a huge fan of the name as WithCheck doesn't tell me anything.
CloneNonCyclic
> (which is terrible, but I can't think of anything better)

Couldn't think a better one, go with yours :)

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:57: auto value = it.second;
On 2016/08/18 15:16:44, Lei Zhang wrote:
> auto* ?

Changed to actual type

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:58: if
(!pdfium::ContainsKey(*pVisited, value))
On 2016/08/18 14:04:31, dsinclair wrote:
> nit: add {}'s

Done.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp:16:
pair.second->Release();
On 2016/08/18 14:04:31, dsinclair wrote:
> I believe this should be Destroy(). The Release() method in CPDF_Object won't
> actually do anything if there is an object number assigned. So, we'd end up
> leaking all of the objects.

You are right, done.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_object.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object.cpp:25: CPDF_Object*
CPDF_Object::CloneDeRef(bool bDirect) const {
On 2016/08/18 14:04:31, dsinclair wrote:
> Oh, I though this was clone dereference. I think this one should be renamed
too.
> 
> Can this be the public CloneNonCyclic and then CloneWithCheck becomes
protected
> CloneNOnCyclicInternal? or something?

CloneWithCheck() was originally set to public to avoid the needs to have friend
classes.
Since exposing it seems confusing, put it back to protected section.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object.cpp:40: m_ObjNum = kInvalidObjNum;
On 2016/08/18 14:04:31, dsinclair wrote:
> The Destroy() call is going to delete the object so no need to reset the
> m_ObjNum

Moved to each complex class's dtor to avoid double free for cyclic references.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:762: EXPECT_EQ(1u,
cloned_array->GetCount());
On 2016/08/18 15:16:44, Lei Zhang wrote:
> ASSERT_TRUE(cloned_array); ditto below with |cloned_dict|

Done.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:766: EXPECT_EQ(nullptr,
cloned_dict->AsDictionary()->GetObjectBy("arr"));
On 2016/08/18 15:16:44, Lei Zhang wrote:
> Or just EXPECT_FALSE(...);

Yes, that would work too. Will keep "EXPECT_EQ(nullptr" to be explicit
about the type and value.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_stream.cpp (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_stream.cpp:87: if (pDict &&
!pdfium::ContainsKey(*pVisited, pDict))
On 2016/08/18 14:04:31, dsinclair wrote:
> nit: {}'s

Done.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_stream.cpp:88: pDict =
ToDictionary(static_cast<const CPDF_Object*>(pDict)->CloneWithCheck(
On 2016/08/18 14:04:31, dsinclair wrote:
> Why static_cast to Object?

Original, I only expose CPDF_Object::CloneWithCheck() as public. Now we expose
friend classes thru CPDF_Object also.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_array.h (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_array.h:70: ~CPDF_Array() override;
On 2016/08/18 15:16:44, Lei Zhang wrote:
> nit: Separator between ctor/dtor and other methods. Ditto in other headers.

Done.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_object.h (right):

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_object.h:44: virtual CPDF_Object* Clone()
const = 0;
On 2016/08/18 15:16:44, Lei Zhang wrote:
> It would be good to document what the different Clone methods do.

Done.

https://codereview.chromium.org/2250533002/diff/220001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_object.h:45: virtual CPDF_Object*
CloneDeRef(bool bDirect) const;
On 2016/08/18 15:16:44, Lei Zhang wrote:
> Does anyone override this?

Not now, but it could be.

[Wei Li, 2016-08-19 05:06:48]

Patchset #10 (id:280001) has been deleted

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

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:750: TEST(PDFObjectTest,
CloneCheckLoop) {
TEST skips SetUp() vs TEST_F?

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:764: CPDF_Object* cloned_dict
= cloned_array->GetObjectAt(0);
ASSERT_TRUE(cloned_dict);

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:779:
EXPECT_TRUE(arr_obj->GetObjectAt(0)->IsReference());
ASSERT_TRUE, otherwise next line can crash.

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

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:788: ASSERT_TRUE(cloned_arr &&
cloned_arr->IsArray());
Separate into 2 ASSERT_TRUE statements.

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h (right):

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h:87: ~CPDF_Dictionary()
override;
Ditto, add a separator line.

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_object.h (right):

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_object.h:91: friend class CPDF_Array;
put friends in alphabetical order

[Wei Li, 2016-08-19 17:08:56]

Patchset #10 (id:300001) has been deleted

[Wei Li, 2016-08-19 17:11:26]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 17:11:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 17:15:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 17:15:46]

Dry run: Try jobs failed on following builders:
  linux_xfa on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_xfa/build...)

[Wei Li, 2016-08-19 17:18:15]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 17:18:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wei Li, 2016-08-19 17:44:46]

Patchset #10 (id:320001) has been deleted

[commit-bot: I haz the power, 2016-08-19 17:51:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 17:51:05]

Dry run: This issue passed the CQ dry run.

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

ptal, thanks

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:750: TEST(PDFObjectTest,
CloneCheckLoop) {
On 2016/08/19 16:21:08, Lei Zhang wrote:
> TEST skips SetUp() vs TEST_F?

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

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:764: CPDF_Object* cloned_dict
= cloned_array->GetObjectAt(0);
On 2016/08/19 16:21:09, Lei Zhang wrote:
> ASSERT_TRUE(cloned_dict);

Done.

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:779:
EXPECT_TRUE(arr_obj->GetObjectAt(0)->IsReference());
On 2016/08/19 16:21:09, Lei Zhang wrote:
> ASSERT_TRUE, otherwise next line can crash.
> 
> ASSERT_TRUE(arr_obj->GetObjectAt(0)) as well.

Done.

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:788: ASSERT_TRUE(cloned_arr &&
cloned_arr->IsArray());
On 2016/08/19 16:21:08, Lei Zhang wrote:
> Separate into 2 ASSERT_TRUE statements.

Done.

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h (right):

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h:87: ~CPDF_Dictionary()
override;
On 2016/08/19 16:21:09, Lei Zhang wrote:
> Ditto, add a separator line.

Done.

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
File core/fpdfapi/fpdf_parser/include/cpdf_object.h (right):

https://codereview.chromium.org/2250533002/diff/260001/core/fpdfapi/fpdf_pars...
core/fpdfapi/fpdf_parser/include/cpdf_object.h:91: friend class CPDF_Array;
On 2016/08/19 16:21:09, Lei Zhang wrote:
> put friends in alphabetical order

Done.

[dsinclair, 2016-08-23 13:17:47]

lgtm

[Wei Li, 2016-08-23 16:40:16]

Patchset #11 (id:360001) has been deleted

[Wei Li, 2016-08-23 16:44:31]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 16:44:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-23 16:56:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 16:56:10]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2016-08-24 04:22:24]

The CQ bit was checked by weili@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 04:22:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 04:35:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 04:35:04]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2016-08-24 05:08:09]

The CQ bit was checked by weili@chromium.org

[Wei Li, 2016-08-24 05:08:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2250533002/#ps420001
(title: "rebase again")

[commit-bot: I haz the power, 2016-08-24 05:08:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 05:08:39]

Description was changed from

==========
Fix stack overflow in object Clone() functions

For some complex objects such as CPDF_Dictionary, CPDF_Array,
CPDF_Stream, and CPDF_Reference, Clone() could be executed with
infinite recursion to cause the stack overflow. Fix this by
checking already cloned objects to avoid recursion.

BUG=pdfium:513
==========

to

==========
Fix stack overflow in object Clone() functions

For some complex objects such as CPDF_Dictionary, CPDF_Array,
CPDF_Stream, and CPDF_Reference, Clone() could be executed with
infinite recursion to cause the stack overflow. Fix this by
checking already cloned objects to avoid recursion.

BUG=pdfium:513

Committed:
https://pdfium.googlesource.com/pdfium/+/a470b5e5371d0674d06068ec38d0d3c3279e...
==========

[commit-bot: I haz the power, 2016-08-24 05:08:41]

Committed patchset #13 (id:420001) as
https://pdfium.googlesource.com/pdfium/+/a470b5e5371d0674d06068ec38d0d3c3279e...

[Lei Zhang, 2016-08-24 05:11:24]

lgtm