Exit infinite loops for cross reference loading

Exit infinite loops for cross reference loading

When cross reference sections form a loop, our code will enter an
infinite loop. Add detection and exit code for v4 cross reference loading.
V5 loading was done previously.

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/aab8f462ea3368ed4825172231131e1a10db978d

[Wei Li, 2016-01-05 03:19:31]

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

[Wei Li, 2016-01-05 03:19:31]

PTAL, thanks.

[Lei Zhang, 2016-01-05 03:30:27]

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:386: if
(seen_xrefpos.find(xrefpos) != seen_xrefpos.end())
I added pdfium::ContainsKey() recently. You can use that here.

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:431: if
(seen_xrefpos.find(xrefpos) != seen_xrefpos.end())
Ditto.

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:441: if
(!CheckDirectType(pDict.get(), "Prev", PDFOBJ_NUMBER))
Can you just see if GetDirectInteger() returns 0 instead? If yes, change 396 as
well.

[Lei Zhang, 2016-01-05 03:30:57]

Otherwise lgtm

[Wei Li, 2016-01-05 19:52:00]

thanks, PTAL

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:386: if
(seen_xrefpos.find(xrefpos) != seen_xrefpos.end())
On 2016/01/05 03:30:27, Lei Zhang wrote:
> I added pdfium::ContainsKey() recently. You can use that here.

Done.

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:431: if
(seen_xrefpos.find(xrefpos) != seen_xrefpos.end())
On 2016/01/05 03:30:27, Lei Zhang wrote:
> Ditto.

Done.

https://codereview.chromium.org/1558093002/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:441: if
(!CheckDirectType(pDict.get(), "Prev", PDFOBJ_NUMBER))
On 2016/01/05 03:30:27, Lei Zhang wrote:
> Can you just see if GetDirectInteger() returns 0 instead? If yes, change 396
as
> well.

Yes, GetDirectInteger() will return 0. The loading will stop there. So I removed
the type checking. This behavior is a little different from the previous one
which aborts the function and the caller may trigger a cross reference rebuild
immediately. This change will be consistent with v5 loading logic. It takes the
cross reference loading as much as it can.

[Lei Zhang, 2016-01-05 20:18:56]

lgtm

https://codereview.chromium.org/1558093002/diff/20001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1558093002/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:380: // When m_pTrailer
doesn't have Prev entry or Prev entry value is not
|m_pTrailer|

https://codereview.chromium.org/1558093002/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:381: // numerical,
GetDirectInteger returns 0. Loading will end.
GetDirectInteger()

[Wei Li, 2016-01-05 20:34:06]

Description was changed from

==========
Exit infinite loops for cross reference loading

When cross reference sections form a loop, our code will enter an
infinite loop. Add detection and exit code for v4 cross reference loading.
V5 loading was done previously.
==========

to

==========
Exit infinite loops for cross reference loading

When cross reference sections form a loop, our code will enter an
infinite loop. Add detection and exit code for v4 cross reference loading.
V5 loading was done previously.

R=thestig@chromium.org

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

[Wei Li, 2016-01-05 20:34:07]

Committed patchset #3 (id:40001) manually as
aab8f462ea3368ed4825172231131e1a10db978d (presubmit successful).