Guard against overflow when calculating font weight.

Guard against overflow when calculating font weight.

This CL uses the safe math libraries when calculating the font weight from
the StemV value as very large values for StemV can cause the signed int to
overflow.

BUG=chromium:641418
Committed: https://pdfium.googlesource.com/pdfium/+/f7252a074ed013e2ad3cc11e08eba90502262ce0

[dsinclair, 2016-08-29 19:15:23]

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

[commit-bot: I haz the power, 2016-08-29 19:15:34]

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

[dsinclair, 2016-08-29 19:15:42]

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

[dsinclair, 2016-08-29 19:15:43]

PTAL.

https://codereview.chromium.org/2293633002/diff/1/core/fpdfapi/fpdf_font/cpdf...
File core/fpdfapi/fpdf_font/cpdf_simplefont.cpp (right):

https://codereview.chromium.org/2293633002/diff/1/core/fpdfapi/fpdf_font/cpdf...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:191: safeStemV.ValueOrDefault(140),
m_ItalicAngle, 0);
I made up 140 in here as it seemed to be the number we used in other bits ...

[npm, 2016-08-29 19:23:39]

npm@chromium.org changed reviewers:
+ npm@chromium.org

[npm, 2016-08-29 19:23:39]

https://codereview.chromium.org/2293633002/diff/1/core/fpdfapi/fpdf_font/cpdf...
File core/fpdfapi/fpdf_font/cpdf_simplefont.cpp (right):

https://codereview.chromium.org/2293633002/diff/1/core/fpdfapi/fpdf_font/cpdf...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:185:
pdfium::base::CheckedNumeric<int> safeStemV(m_StemV);
Maybe you can also do something similar in CPDF_CIDFont::LoadSubstFont()

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

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

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

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-08-29 19:40:55]

https://codereview.chromium.org/2293633002/diff/1/core/fpdfapi/fpdf_font/cpdf...
File core/fpdfapi/fpdf_font/cpdf_simplefont.cpp (right):

https://codereview.chromium.org/2293633002/diff/1/core/fpdfapi/fpdf_font/cpdf...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:185:
pdfium::base::CheckedNumeric<int> safeStemV(m_StemV);
On 2016/08/29 19:23:39, npm wrote:
> Maybe you can also do something similar in CPDF_CIDFont::LoadSubstFont()

Done.

[dsinclair, 2016-08-29 19:41:34]

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

[commit-bot: I haz the power, 2016-08-29 19:41:46]

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

[npm, 2016-08-29 19:52:44]

Description was changed from

==========
Guard against overflow when calculating font weight.

This CL uses the safe math libraries when calculating the font weight from
the StemV value as very large values for StemV can cause the signed int to
overflow.

BUG=chromium:641418
==========

to

==========
Guard against overflow when calculating font weight.

This CL uses the safe math libraries when calculating the font weight from
the StemV value as very large values for StemV can cause the signed int to
overflow.

BUG=chromium:641418
==========

[npm, 2016-08-29 19:52:44]

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

[commit-bot: I haz the power, 2016-08-29 20:05:05]

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

[commit-bot: I haz the power, 2016-08-29 20:05:06]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2016-08-29 23:05:49]

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/cpdf_cidfont.cpp (right):

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_cidfont.cpp:775: safeStemV.ValueOrDefault(140),
m_ItalicAngle,
Maybe it is worth defining a constant for the default value if not using the
existing one.

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/cpdf_simplefont.cpp (right):

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:189: safeStemV = safeStemV * 5 + 140;
Shouldn't multiple 4 here?

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:191: safeStemV.ValueOrDefault(140),
m_ItalicAngle, 0);
Maybe the default value should be FXFONT_FW_NORMAL or 0?

[dsinclair, 2016-08-30 14:26:27]

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

[commit-bot: I haz the power, 2016-08-30 14:26:28]

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

[dsinclair, 2016-08-30 14:26:39]

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/cpdf_cidfont.cpp (right):

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_cidfont.cpp:775: safeStemV.ValueOrDefault(140),
m_ItalicAngle,
On 2016/08/29 23:05:48, Wei Li wrote:
> Maybe it is worth defining a constant for the default value if not using the
> existing one.

Went with FXFONT_FW_NORMAL.

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/cpdf_simplefont.cpp (right):

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:189: safeStemV = safeStemV * 5 + 140;
On 2016/08/29 23:05:48, Wei Li wrote:
> Shouldn't multiple 4 here?

Doh. Good catch, thanks.

https://codereview.chromium.org/2293633002/diff/20001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/cpdf_simplefont.cpp:191: safeStemV.ValueOrDefault(140),
m_ItalicAngle, 0);
On 2016/08/29 23:05:48, Wei Li wrote:
> Maybe the default value should be FXFONT_FW_NORMAL or 0?

Went with normal, thanks for pointing it out.

[commit-bot: I haz the power, 2016-08-30 14:53:38]

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

[commit-bot: I haz the power, 2016-08-30 14:53:39]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2016-08-30 16:43:26]

lgtm

[dsinclair, 2016-08-30 17:26:37]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-08-30 17:26:49]

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-30 17:27:06]

Description was changed from

==========
Guard against overflow when calculating font weight.

This CL uses the safe math libraries when calculating the font weight from
the StemV value as very large values for StemV can cause the signed int to
overflow.

BUG=chromium:641418
==========

to

==========
Guard against overflow when calculating font weight.

This CL uses the safe math libraries when calculating the font weight from
the StemV value as very large values for StemV can cause the signed int to
overflow.

BUG=chromium:641418

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

[commit-bot: I haz the power, 2016-08-30 17:27:07]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/f7252a074ed013e2ad3cc11e08eba9050226...