Avoid leaking params if any entry bad.

Avoid leaking params if any entry bad.

The call to png_set_pCAL can call into png_error for several reasons. This CL
verifies that the params are valid before calling into png_set_pCAL.

BUG=chromium:636214
Committed: https://pdfium.googlesource.com/pdfium/+/8e783a6b2c165b5d3dcdf9e4d4c2526ac18c77c8

[dsinclair, 2016-08-31 14:25:53]

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

[dsinclair, 2016-08-31 14:25:54]

PTAL.

[Lei Zhang, 2016-08-31 15:27:33]

thestig@chromium.org changed reviewers:
+ msarett@google.com

[Lei Zhang, 2016-08-31 15:27:34]

+msarett for libpng.

[dsinclair, 2016-08-31 15:45:31]

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

[commit-bot: I haz the power, 2016-08-31 15:45:38]

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-08-31 16:01:50]

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

[commit-bot: I haz the power, 2016-08-31 16:01:51]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-09-01 03:25:55]

Can we file an issue and suggest this fix to upstream?
https://github.com/glennrp/libpng

[dsinclair, 2016-09-01 18:42:14]

On 2016/09/01 03:25:55, msarett wrote:
> Can we file an issue and suggest this fix to upstream?
> https://github.com/glennrp/libpng

Filed https://github.com/glennrp/libpng/issues/133

I'll keep this open until that issue is resolved and we've either rolled or
pulled the patch into PDFium.

[msarett, 2016-09-01 18:43:55]

On 2016/09/01 18:42:14, dsinclair wrote:
> On 2016/09/01 03:25:55, msarett wrote:
> > Can we file an issue and suggest this fix to upstream?
> > https://github.com/glennrp/libpng
> 
> Filed https://github.com/glennrp/libpng/issues/133
> 
> I'll keep this open until that issue is resolved and we've either rolled or
> pulled the patch into PDFium.

Awesome thanks!

[dsinclair, 2016-09-06 13:45:53]

On 2016/09/01 18:43:55, msarett wrote:
> On 2016/09/01 18:42:14, dsinclair wrote:
> > On 2016/09/01 03:25:55, msarett wrote:
> > > Can we file an issue and suggest this fix to upstream?
> > > https://github.com/glennrp/libpng
> > 
> > Filed https://github.com/glennrp/libpng/issues/133
> > 
> > I'll keep this open until that issue is resolved and we've either rolled or
> > pulled the patch into PDFium.
> 
> Awesome thanks!

A different patch got created to fix the bug in libpng. I've updated the patch
here and applied it to our tree. The pull request was:
https://github.com/glennrp/libpng/pull/135. What is the correct way to do
attribution in these types of .patch files?

[msarett, 2016-09-06 14:04:04]

On 2016/09/06 13:45:53, dsinclair wrote:
> On 2016/09/01 18:43:55, msarett wrote:
> > On 2016/09/01 18:42:14, dsinclair wrote:
> > > On 2016/09/01 03:25:55, msarett wrote:
> > > > Can we file an issue and suggest this fix to upstream?
> > > > https://github.com/glennrp/libpng
> > > 
> > > Filed https://github.com/glennrp/libpng/issues/133
> > > 
> > > I'll keep this open until that issue is resolved and we've either rolled
or
> > > pulled the patch into PDFium.
> > 
> > Awesome thanks!
> 
> A different patch got created to fix the bug in libpng. I've updated the patch
> here and applied it to our tree. The pull request was:
> https://github.com/glennrp/libpng/pull/135. What is the correct way to do
> attribution in these types of .patch files?

Thanks for working with upstream on this!

I think the following is a good way to track cherry-picked commits from
upstream.
Makes it easy to tell that there's no need to reapply when we update the
library.
https://cs.chromium.org/chromium/src/third_party/libpng/README.chromium

I'll defer to how thestig@ wants to track these changes for pdfium.  The pngset
changes, look good to me.

Do you think this is something that I should apply to the chromium version?

[dsinclair, 2016-09-06 14:06:37]

On 2016/09/06 14:04:04, msarett wrote:
> On 2016/09/06 13:45:53, dsinclair wrote:
> > On 2016/09/01 18:43:55, msarett wrote:
> > > On 2016/09/01 18:42:14, dsinclair wrote:
> > > > On 2016/09/01 03:25:55, msarett wrote:
> > > > > Can we file an issue and suggest this fix to upstream?
> > > > > https://github.com/glennrp/libpng
> > > > 
> > > > Filed https://github.com/glennrp/libpng/issues/133
> > > > 
> > > > I'll keep this open until that issue is resolved and we've either rolled
> or
> > > > pulled the patch into PDFium.
> > > 
> > > Awesome thanks!
> > 
> > A different patch got created to fix the bug in libpng. I've updated the
patch
> > here and applied it to our tree. The pull request was:
> > https://github.com/glennrp/libpng/pull/135. What is the correct way to do
> > attribution in these types of .patch files?
> 
> Thanks for working with upstream on this!
> 
> I think the following is a good way to track cherry-picked commits from
> upstream.
> Makes it easy to tell that there's no need to reapply when we update the
> library.
> https://cs.chromium.org/chromium/src/third_party/libpng/README.chromium
> 
> I'll defer to how thestig@ wants to track these changes for pdfium.  The
pngset
> changes, look good to me.
> 
> Do you think this is something that I should apply to the chromium version?


It is possible to craft a PNG which triggers the memory leak, so probably
wouldn't hurt to apply to Chromium version as well.

[Lei Zhang, 2016-09-06 15:33:27]

lgtm

[dsinclair, 2016-09-06 15:36:43]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-09-06 15:36: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-06 15:56:48]

Description was changed from

==========
Avoid leaking params if any entry bad.

The call to png_set_pCAL can call into png_error for several reasons. This CL
verifies that the params are valid before calling into png_set_pCAL.

BUG=chromium:636214
==========

to

==========
Avoid leaking params if any entry bad.

The call to png_set_pCAL can call into png_error for several reasons. This CL
verifies that the params are valid before calling into png_set_pCAL.

BUG=chromium:636214

Committed:
https://pdfium.googlesource.com/pdfium/+/8e783a6b2c165b5d3dcdf9e4d4c2526ac18c...
==========

[commit-bot: I haz the power, 2016-09-06 15:56:49]

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