Fixup integer conversion logic.

Fixup integer conversion logic.

In bc8a64029f898286c3dcad3a6cecdc98ef30b139 we updated the FX_atonum logic
to correctly handle integer overflow. This causes issues when parsing the
Permissions flag of encrypted documents as that flag isn't encoded like other
numbers.

The Permissions flag is a unsigned value, and has to be treated as such since
the sign bit is always set. The current logic will detect an overflow of the
int value and return 0. The old logic would have detected the overflow and
returned the negative result regardless.

This CL updates the logic to do the string to int conversion as a uint32_t
and then verifies the uint32_t value, if a sign was provided, fits within
the int range, otherwise it converts it to an int and lets it be positive or
negative as needed.

BUG=pdfium:539
Committed: https://pdfium.googlesource.com/pdfium/+/6f1025492801aaa93fca2c0ed7c40a3389ad8cd1

[dsinclair, 2016-07-21 17:34:36]

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

[dsinclair, 2016-07-21 17:34:36]

PTAL.

[dsinclair, 2016-07-21 17:35:19]

Description was changed from

==========
Fixup integer conversion logic.

In bc8a64029f898286c3dcad3a6cecdc98ef30b139 we updated the FX_atonum logic
to correctly handle integer overflow. This causes issues when parsing the
Permissions flag of encrypted documents as that flag isn't encoded like other
numbers.

The Permissions flag is a unsigned value, and has to be treated as such since
the sign bit is always set. The current logic will detect an overflow of the
int value and return 0. The old logic would have detected the overflow and
returned the negative result regardless.

This CL updates the logic to do the string to int conversion as a uint32_t
and then verifies the uint32_t value, if a sign was provided, fits within
the int range, otherwise it converts it to an int and lets it be positive or
negative as needed.

BUG=539
==========

to

==========
Fixup integer conversion logic.

In bc8a64029f898286c3dcad3a6cecdc98ef30b139 we updated the FX_atonum logic
to correctly handle integer overflow. This causes issues when parsing the
Permissions flag of encrypted documents as that flag isn't encoded like other
numbers.

The Permissions flag is a unsigned value, and has to be treated as such since
the sign bit is always set. The current logic will detect an overflow of the
int value and return 0. The old logic would have detected the overflow and
returned the negative result regardless.

This CL updates the logic to do the string to int conversion as a uint32_t
and then verifies the uint32_t value, if a sign was provided, fits within
the int range, otherwise it converts it to an int and lets it be positive or
negative as needed.

BUG=pdfium:539
==========

[Lei Zhang, 2016-07-21 20:39:31]

https://codereview.chromium.org/2168173002/diff/20001/core/fxcrt/fx_basic_uti...
File core/fxcrt/fx_basic_util.cpp (right):

https://codereview.chromium.org/2168173002/diff/20001/core/fxcrt/fx_basic_uti...
core/fxcrt/fx_basic_util.cpp:37: // actually a unsigned value. We treat use
uint32_t so we can deal with the
"We treat use uint32_t" does not parse English bad.

https://codereview.chromium.org/2168173002/diff/20001/core/fxcrt/fx_basic_uti...
core/fxcrt/fx_basic_util.cpp:71: value = -value;
Do we need to worry about the value = INT_MIN case?

[Tom Sepez, 2016-07-21 21:29:41]

tsepez@chromium.org changed reviewers:
- tsepez@chromium.org

[dsinclair, 2016-07-25 13:47:59]

https://codereview.chromium.org/2168173002/diff/20001/core/fxcrt/fx_basic_uti...
File core/fxcrt/fx_basic_util.cpp (right):

https://codereview.chromium.org/2168173002/diff/20001/core/fxcrt/fx_basic_uti...
core/fxcrt/fx_basic_util.cpp:37: // actually a unsigned value. We treat use
uint32_t so we can deal with the
On 2016/07/21 20:39:31, Lei Zhang wrote:
> "We treat use uint32_t" does not parse English bad.

Done.

https://codereview.chromium.org/2168173002/diff/20001/core/fxcrt/fx_basic_uti...
core/fxcrt/fx_basic_util.cpp:71: value = -value;
On 2016/07/21 20:39:31, Lei Zhang wrote:
> Do we need to worry about the value = INT_MIN case?


Fixed up some issues around boundary conversions. We dont' check INT_MIN, but
INT_MAX + 1 as INT_MIN is one bigger in the negative. We're comparing againts a
uint, so I have to do the comparison in the positive.

[Lei Zhang, 2016-07-25 17:40:17]

Windows bots are red. :\

[dsinclair, 2016-07-27 14:32:01]

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

[commit-bot: I haz the power, 2016-07-27 14:32:09]

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-07-27 14:44:59]

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

[commit-bot: I haz the power, 2016-07-27 14:45:00]

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

[dsinclair, 2016-07-27 14:52:55]

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

[commit-bot: I haz the power, 2016-07-27 14:52:59]

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-07-27 15:03:21]

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

[commit-bot: I haz the power, 2016-07-27 15:03:22]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-07-27 15:03:55]

win bots happy, ptal.

[Lei Zhang, 2016-07-28 04:43:57]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-07-28 04:43:57]

lgtm

[commit-bot: I haz the power, 2016-07-28 04:44:08]

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-07-28 04:44:28]

Description was changed from

==========
Fixup integer conversion logic.

In bc8a64029f898286c3dcad3a6cecdc98ef30b139 we updated the FX_atonum logic
to correctly handle integer overflow. This causes issues when parsing the
Permissions flag of encrypted documents as that flag isn't encoded like other
numbers.

The Permissions flag is a unsigned value, and has to be treated as such since
the sign bit is always set. The current logic will detect an overflow of the
int value and return 0. The old logic would have detected the overflow and
returned the negative result regardless.

This CL updates the logic to do the string to int conversion as a uint32_t
and then verifies the uint32_t value, if a sign was provided, fits within
the int range, otherwise it converts it to an int and lets it be positive or
negative as needed.

BUG=pdfium:539
==========

to

==========
Fixup integer conversion logic.

In bc8a64029f898286c3dcad3a6cecdc98ef30b139 we updated the FX_atonum logic
to correctly handle integer overflow. This causes issues when parsing the
Permissions flag of encrypted documents as that flag isn't encoded like other
numbers.

The Permissions flag is a unsigned value, and has to be treated as such since
the sign bit is always set. The current logic will detect an overflow of the
int value and return 0. The old logic would have detected the overflow and
returned the negative result regardless.

This CL updates the logic to do the string to int conversion as a uint32_t
and then verifies the uint32_t value, if a sign was provided, fits within
the int range, otherwise it converts it to an int and lets it be positive or
negative as needed.

BUG=pdfium:539

Committed:
https://pdfium.googlesource.com/pdfium/+/6f1025492801aaa93fca2c0ed7c40a3389ad...
==========

[commit-bot: I haz the power, 2016-07-28 04:44:29]

Committed patchset #7 (id:120001) as
https://pdfium.googlesource.com/pdfium/+/6f1025492801aaa93fca2c0ed7c40a3389ad...