Check first page number in CPDF_HintTables::ReadPageHintTable().

Check first page number in CPDF_HintTables::ReadPageHintTable().

BUG=642655
Committed: https://pdfium.googlesource.com/pdfium/+/73e9c31d7eb8ecc59086c29e8e3e5bb4062b4424

[Lei Zhang, 2016-08-31 21:55:55]

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

[Tom Sepez, 2016-08-31 21:58:10]

https://codereview.chromium.org/2298753003/diff/1/core/fpdfapi/fpdf_parser/cp...
File core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp (right):

https://codereview.chromium.org/2298753003/diff/1/core/fpdfapi/fpdf_parser/cp...
core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp:168: if (nFirstPageNum < 0)
Is there a reason this isn't just unsigned all the way down ...

[Lei Zhang, 2016-08-31 23:44:53]

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

[commit-bot: I haz the power, 2016-08-31 23:45:06]

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

[Lei Zhang, 2016-08-31 23:45:51]

I added more checks in all the places that use the first page number.

https://codereview.chromium.org/2298753003/diff/1/core/fpdfapi/fpdf_parser/cp...
File core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp (right):

https://codereview.chromium.org/2298753003/diff/1/core/fpdfapi/fpdf_parser/cp...
core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp:168: if (nFirstPageNum < 0)
On 2016/08/31 21:58:10, Tom Sepez wrote:
> Is there a reason this isn't just unsigned all the way down ...

PDF number objects have signed values.

[commit-bot: I haz the power, 2016-08-31 23:57:14]

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

[commit-bot: I haz the power, 2016-08-31 23:57:15]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-09-01 00:06:51]

lgtm

https://codereview.chromium.org/2298753003/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp (right):

https://codereview.chromium.org/2298753003/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp:357: bool
CPDF_HintTables::GetPagePos(int index,
nit: can index at least be unsigned?

https://codereview.chromium.org/2298753003/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp:392:
CPDF_DataAvail::DownloadHints* pHints) {
nit: same here.

[Lei Zhang, 2016-09-01 00:11:30]

https://codereview.chromium.org/2298753003/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp (right):

https://codereview.chromium.org/2298753003/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp:357: bool
CPDF_HintTables::GetPagePos(int index,
On 2016/09/01 00:06:51, Tom Sepez wrote:
> nit: can index at least be unsigned?

Maybe. I will in a separate CL. Need to check the callers. (and their callers
for CheckPage()).

[Lei Zhang, 2016-09-01 00:11:41]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-09-01 00:11:51]

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-09-01 00:12:05]

Description was changed from

==========
Check first page number in CPDF_HintTables::ReadPageHintTable().

BUG=642655
==========

to

==========
Check first page number in CPDF_HintTables::ReadPageHintTable().

BUG=642655

Committed:
https://pdfium.googlesource.com/pdfium/+/73e9c31d7eb8ecc59086c29e8e3e5bb4062b...
==========

[commit-bot: I haz the power, 2016-09-01 00:12:06]

Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/73e9c31d7eb8ecc59086c29e8e3e5bb4062b...