Stack exhaustion if PDFium returns circular bookmarks.

Stack exhaustion if PDFium returns circular bookmarks.

Ideally, we'd track this in PDFium, but the API doesn't have
a context representing the current "search" onto which to
hang a set of visited bookmarks. Alternatively, we could do
this in pdfium_engine itself, but it may be hard to tell
when a bookmark is the same, since PDFium dutifully cobbles
up a new structure each time around. So restrict the depth
to prevent the crash.

BUG=542403
Committed: https://crrev.com/a60c693cb284a9f96e8a99b8c26f48c7f7217c78
Cr-Commit-Position: refs/heads/master@{#353891}

[Tom Sepez, 2015-10-13 18:55:32]

tsepez@chromium.org changed reviewers:
+ thestig@chromium.org

[Tom Sepez, 2015-10-13 18:55:43]

Lei, for review.

[Lei Zhang, 2015-10-13 20:07:57]

lgtm

https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc...
pdf/pdfium/pdfium_engine.cc:547: pp::VarDictionary TraverseBookmarks(unsigned
int depth,
nit: Can depth be the last param?

https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc...
pdf/pdfium/pdfium_engine.cc:571: const unsigned int kMaxDepth = 128;
How did we pick 128?

[Tom Sepez, 2015-10-13 22:03:27]

On 2015/10/13 20:07:57, Lei Zhang wrote:
> lgtm
> 
> https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc
> File pdf/pdfium/pdfium_engine.cc (right):
> 
>
https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc...
> pdf/pdfium/pdfium_engine.cc:547: pp::VarDictionary TraverseBookmarks(unsigned
> int depth,
> nit: Can depth be the last param?
> 
>
https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc...
> pdf/pdfium/pdfium_engine.cc:571: const unsigned int kMaxDepth = 128;
> How did we pick 128?
Out of the air.  How deep can you imagine a bookmark being?  To me, maybe a
dozen. So multiply by 10 and round.

[Lei Zhang, 2015-10-13 22:05:38]

On 2015/10/13 22:03:27, Tom Sepez wrote:
> On 2015/10/13 20:07:57, Lei Zhang wrote:
> > lgtm
> > 
> >
https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc
> > File pdf/pdfium/pdfium_engine.cc (right):
> > 
> >
>
https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc...
> > pdf/pdfium/pdfium_engine.cc:547: pp::VarDictionary
TraverseBookmarks(unsigned
> > int depth,
> > nit: Can depth be the last param?
> > 
> >
>
https://codereview.chromium.org/1401263005/diff/1/pdf/pdfium/pdfium_engine.cc...
> > pdf/pdfium/pdfium_engine.cc:571: const unsigned int kMaxDepth = 128;
> > How did we pick 128?
> Out of the air.  How deep can you imagine a bookmark being?  To me, maybe a
> dozen. So multiply by 10 and round.

Seems fine. Other route is to make PDFs with more depth and see when Acrobat
Reader fails.

[Tom Sepez, 2015-10-13 22:07:55]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2015-10-13 22:07:56]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1401263005/#ps20001
(title: "Depth goes last.")

[commit-bot: I haz the power, 2015-10-13 22:13:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1401263005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1401263005/20001

[commit-bot: I haz the power, 2015-10-13 23:02:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-13 23:03:04]

Patchset 2 (id:??) landed as
https://crrev.com/a60c693cb284a9f96e8a99b8c26f48c7f7217c78
Cr-Commit-Position: refs/heads/master@{#353891}