Last Comment Bug 745178 - Update libpng to version 1.5.10
: Update libpng to version 1.5.10
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla15
Assigned To: Glenn Randers-Pehrson
:
: Milan Sreckovic [:milan]
Mentors:
http://libpng.sf.net/index.html
Depends on: 648690 771394
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-13 06:33 PDT by Glenn Randers-Pehrson
Modified: 2012-07-05 17:39 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v00 update bundled libpng to version 1.5.10 (86.24 KB, patch)
2012-04-13 06:33 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Splinter Review
v01: Update libpng to version 1.5.10 (86.36 KB, patch)
2012-04-13 07:28 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Splinter Review
v02 update bundled libpng to version 1.5.10 (87.33 KB, patch)
2012-04-13 08:48 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Splinter Review
v03: update libpng to version 1.5.10 (fix unknown handling in pngpread.c) (88.22 KB, patch)
2012-04-23 17:48 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Splinter Review
v04: update libpng to version 1.5.10 (88.22 KB, patch)
2012-04-23 18:03 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Splinter Review
v05: update libpng to version 1.5.10 (88.04 KB, patch)
2012-04-23 20:53 PDT, Glenn Randers-Pehrson
ryanvm: review+
Details | Diff | Splinter Review

Description Glenn Randers-Pehrson 2012-04-13 06:33:07 PDT
Created attachment 614760 [details] [diff] [review]
v00 update bundled libpng to version 1.5.10

Libpng-1.5.10 has been out for a couple of weeks and seems to be stable.  The main reason for the release was to fix CVE-2011-3048, but we aren't vulnerable to that.  It also ignores multiple iCCP chunks which could give us a slight performance benefit.
Comment 1 Glenn Randers-Pehrson 2012-04-13 06:51:18 PDT
A new feature of libpng-1.5.10 is to check for invalid palette index and iissue a warning if one is found.  Previously we allowed invalid indexes and rendered any pixels with such an invalid index to black.  The v00 patch does not enable this feature, for speed consideration (otherwise an extra pass is made over each row), so we continue to render any such pixels as black without a warning.  This is harmless.
Comment 2 Ryan VanderMeulen [:RyanVM] 2012-04-13 07:03:53 PDT
I'll update the apng diff and give it a Try push tonight.
Comment 3 Glenn Randers-Pehrson 2012-04-13 07:28:35 PDT
Created attachment 614769 [details] [diff] [review]
v01: Update libpng to version 1.5.10

Added checkin header, updated MOZCHANGES
Comment 4 Max Stepin 2012-04-13 08:05:23 PDT
Ryan, I found a typo that probably existed for years...

In png_get_next_frame_fcTL() we are testing the pointer to x_offset twice, but no y_offset
Comment 5 Glenn Randers-Pehrson 2012-04-13 08:23:38 PDT
See also bug#745202 which disables the unnecessary palette-index testing when using the systems libpng, when PR_LOGGING is disabled.  @Max, I'll post a v02 patch fixing the typo, thanks.
Comment 6 Glenn Randers-Pehrson 2012-04-13 08:48:22 PDT
Created attachment 614797 [details] [diff] [review]
v02 update bundled libpng to version 1.5.10
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-04-14 05:33:52 PDT
I had to fix up an ifdef in png.h, but other than that, it passes on try.
https://tbpl.mozilla.org/?tree=Try&rev=82bbfa4bd308

Also, there's a new build warning in pngpread.c. Here's the MSVC version:
pngpread.c(690) : warning C4013: 'MOZ_PNG_handle_unknown' undefined; assuming extern returning int
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-04-23 16:40:44 PDT
Glenn, how should we proceed on the new warning?
Comment 9 Glenn Randers-Pehrson 2012-04-23 17:14:10 PDT
It's an error in libpng.

At line 361 in pngpread.c (APNG patched) or line 258 (unpatched)
I think we need the following instead of the two lines that are there
(splitting the "else if" into two parts):

#ifdef PNG_HANDLE_UNKNOWN_SUPPORTED
   else
#ifdef PNG_HANDLE_AS_UNKNOWN_SUPPORTED
   if (png_chunk_unknown_handling(png_ptr, chunk_name))
#endif
Comment 10 Glenn Randers-Pehrson 2012-04-23 17:23:23 PDT
Also the "else { .. } " block beginning around line 680 needs to be
enclosed in #ifdef PNG_HANDLE_UNKNOWN_SUPPORTED / #endif

I'm preparing a new patch.
Comment 11 Glenn Randers-Pehrson 2012-04-23 17:48:30 PDT
Created attachment 617715 [details] [diff] [review]
v03: update libpng to version 1.5.10 (fix unknown handling in pngpread.c)
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-04-23 17:52:29 PDT
This still has a broken #endif in png.h (same one I alluded to in comment 7).
+#if defined(PNG_READ_CHECK_FOR_INVALID_INDEX_SUPPORTED) || \
+    defined(PNG_WRITE_CHECK_FOR_INVALID_INDEX_SUPPORTED)
+PNG_EXPORT(234, void, png_set_check_for_invalid_index, (png_structp png_ptr,
+    int allowed));
+
++#endif

No need for another patch, I'll fix it locally.
Comment 13 Glenn Randers-Pehrson 2012-04-23 18:03:36 PDT
Created attachment 617723 [details] [diff] [review]
v04: update libpng to version 1.5.10

Fixed "+#endif" in png.h
Comment 14 Glenn Randers-Pehrson 2012-04-23 20:53:19 PDT
Created attachment 617763 [details] [diff] [review]
v05: update libpng to version 1.5.10

declares png_handle_unknown() in pngpriv.h
Comment 15 Ryan VanderMeulen [:RyanVM] 2012-04-24 15:23:24 PDT
Comment on attachment 617763 [details] [diff] [review]
v05: update libpng to version 1.5.10

v05 looks good on Try.
https://tbpl.mozilla.org/?tree=Try&rev=aba02a77a80b
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-04-24 17:04:46 PDT
Comment on attachment 617763 [details] [diff] [review]
v05: update libpng to version 1.5.10

Got r+ from joe over IRC.
Comment 17 Ryan VanderMeulen [:RyanVM] 2012-04-24 17:05:07 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f22a06ee6a6e

Note You need to log in before you can comment on or make changes to this bug.