Closed Bug 532645 Opened 10 years ago Closed 10 years ago

Upgrade libpng to version 1.4.0

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.1 --- wontfix

People

(Reporter: glennrp+bmo, Assigned: glennrp+bmo)

References

()

Details

(Whiteboard: [3.6.x])

Attachments

(2 files, 1 obsolete file)

libpng version 1.2.41 was released today.  This is a code cleanup version with no new features except for a new set of warnings that are intended to help with migration to future versions.  libpng version 1.4.0 should be out in about two weeks.  I recommend testing with 1.2.41 but not checking it in, then update to libpng-1.4.0 and check that in.  libpng-1.4.0 will have some new features that are relevant here, including a more robust defense against DoS attacks using small files that decompress to very large ones.
Assignee: nobody → glennrp+bmo
Depends on: 504805
No longer depends on: 504805
Libpng 1.2.42 will be out soon.  It does not offer anything significant over  version 1.2.40 for Mozilla, so I recommend skipping it as well as 1.2.41 and waiting for 1.4.0.
Status: NEW → ASSIGNED
If all goes according to plan, libpng-1.4.0 will be released on January 3, 2010.
Blocks: 536184
Glenn, is this ready to be reviewed for inclusion into mozilla-central?
Comment on attachment 419883 [details] [diff] [review]
Update libpng in trunk to version 1.4.0

@reed, I guess it is.  No bug reports have come in to the PNG group yet.
Attachment #419883 - Flags: review?(joe)
Comment on attachment 419883 [details] [diff] [review]
Update libpng in trunk to version 1.4.0

>+++ src/modules/libpr0n/encoders/png/nsPNGEncoder.cpp	2010-01-03 21:02:41.000000000 

>   // Stride is the padded width of each row, so it better be longer (I'm afraid
>   // people will not understand what stride means, so check it well)
>   if ((aInputFormat == INPUT_FORMAT_RGB &&
>        aStride < aWidth * 3) ||
>-      ((aInputFormat == INPUT_FORMAT_RGBA || aInputFormat == INPUT_FORMAT_HOSTARGB) &&
>-       aStride < aWidth * 4)) {
>+      ((aInputFormat == INPUT_FORMAT_RGBA || aInputFormat ==
>+                        INPUT_FORMAT_HOSTARGB) &&
>+                        aStride < aWidth * 4)) {

In this part, maybe put the two aInputFormat checks on their own lines, starting with aInputFormat ==, then put the aStride check on its own line, vertically aligned with the second ( in ((aInputFormat?

>@@ -408,17 +432,18 @@ nsPNGEncoder::ParseOptions(const nsAStri

>-      if (PR_sscanf(value, "%u", frameDelay) != 1) { return NS_ERROR_INVALID_ARG; }
>+      if (PR_sscanf(value, "%u", frameDelay) != 1)
>+        { return NS_ERROR_INVALID_ARG; }

May as well fix this bracing style to put opening brace on the if line and closing brace on its own.

r+ on the libpr0n changes, rs=joe on the libpng changes.
Attachment #419883 - Flags: review?(joe) → review+
This patch makes the changes that Joe requested plus other cosmetic changes;  wrapped some long lines, changed

    if (condition) { single statement; }

to

    if (condition)
       single statement;

without brackets.
Attachment #419883 - Attachment is obsolete: true
Attachment #420020 - Flags: review?(joe)
Comment on attachment 420020 [details] [diff] [review]
Update libpng in trunk to version 1.4.0 (v01) (checked in)

Since joe already r+'d this, if you'll toss it to the try server, I can land it for you if everything comes out green.
Unit tests look OK with the v01 patch.  All passed on Linux, one failed on WINNT but other people's trials seem to be failing the same one today so it's probably not related to this patch.
http://hg.mozilla.org/mozilla-central/rev/96a38b690156
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Attachment #420020 - Flags: review?(joe)
Attachment #420020 - Attachment description: Update libpng in trunk to version 1.4.0 (v01) → Update libpng in trunk to version 1.4.0 (v01) (checked in)
Noticed we have a configure check for libpng's version. We should probably bump it, especially since 1.4.0 changes so much.
Attachment #420300 - Flags: review?(glennrp+bmo)
Comment on attachment 420300 [details] [diff] [review]
Bump MOZPNG in configure.in

Right.  The older versions of libpng will work (if patched with APNG), but the comment in "configure" says that MOZPNG is the version number included with mozilla, which is now 10400.  It's used, however, as the minimum acceptable version.
Attachment #420300 - Flags: review?(glennrp+bmo) → review+
Today the Tryserver is building ok on all platforms but I'm getting a tremendous number of failures on the unit tests.
The failures I mentioned in comment #14 seem to be unrelated to libpng.  I've just done another Tryserver run which passed all unit tests.
Depends on: CVE-2010-0205
No longer blocks: 536184
Depends on: 536184
No longer depends on: CVE-2010-0205
Status: RESOLVED → VERIFIED
joe: are we going to land this on the 1.9.2 branch? If not we should at least upgrade to 1.2.40 (bug 504805) which unfortunately was blocking1.9.2-, which effectively caused the UMR fix in bug 492200 to regress between 1.9.1 and 1.9.2 (we didn't land that small patch since we expected to take 504805).

Should we upgrade the 1.9.1 branch to either of these?
blocking1.9.1: --- → ?
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Flags: wanted1.9.2?
Flags: wanted1.9.2+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Whiteboard: [3.6.x]
We should not land this on 1.9.2. We should take the smaller patches on 1.9.2.
Marking "wontfix" for 1.9.1 for now. If we need a security fix from that version of libpng in the future we can look at it case-by-case whether we take a small patch or bite the bullet and upgrade.
blocking1.9.1: ? → ---
You need to log in before you can comment on or make changes to this bug.