From 07772cba074392b6c697a2f9bb7102d7fb1207c3 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Fri, 14 Oct 2011 18:19:47 -0500 Subject: [PATCH] [libpng15] Fixes for multiple calls to png_read_update_info. These fixes attend to most of the errors revealed in pngvalid, however doing the gamma work twice results in inaccuracies that can't be easily fixed. There is now a warning in the code if this is going to happen. --- ANNOUNCE | 4 +++ CHANGES | 4 +++ png.c | 70 +++++++++++++++++++++++++++++++++++++++-- pngpriv.h | 1 + pngread.c | 49 +++-------------------------- pngrtran.c | 56 +++++++++++++++++++++------------ pngstruct.h | 10 +++--- pngvalid.c | 2 +- test-pngvalid-full.sh | 6 ++-- test-pngvalid-simple.sh | 6 ++-- 10 files changed, 130 insertions(+), 78 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 909036e5b..ec5b38fb8 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -74,6 +74,10 @@ Version 1.5.6beta06 [October 14, 2011] Removed two redundant tests for unitialized row. Fixed a relatively harmless memory overwrite in compressed text writing with a 1 byte zlib buffer. + Fixes for multiple calls to png_read_update_info. These fixes attend to + most of the errors revealed in pngvalid, however doing the gamma work + twice results in inaccuracies that can't be easily fixed. There is now + a warning in the code if this is going to happen. Send comments/corrections/commendations to png-mng-implement at lists.sf.net: (subscription required; visit diff --git a/CHANGES b/CHANGES index 7c414c00a..dd7967ab2 100644 --- a/CHANGES +++ b/CHANGES @@ -3635,6 +3635,10 @@ Version 1.5.6beta06 [October 14, 2011] Removed two redundant tests for unitialized row. Fixed a relatively harmless memory overwrite in compressed text writing with a 1 byte zlib buffer. + Fixes for multiple calls to png_read_update_info. These fixes attend to + most of the errors revealed in pngvalid, however doing the gamma work + twice results in inaccuracies that can't be easily fixed. There is now + a warning in the code if this is going to happen. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/png.c b/png.c index 0a96d0d8c..d5d7adbc7 100644 --- a/png.c +++ b/png.c @@ -645,13 +645,13 @@ png_get_copyright(png_const_structp png_ptr) #else # ifdef __STDC__ return PNG_STRING_NEWLINE \ - "libpng version 1.5.6beta06 - October 12, 2011" PNG_STRING_NEWLINE \ + "libpng version 1.5.6beta06 - October 14, 2011" PNG_STRING_NEWLINE \ "Copyright (c) 1998-2011 Glenn Randers-Pehrson" PNG_STRING_NEWLINE \ "Copyright (c) 1996-1997 Andreas Dilger" PNG_STRING_NEWLINE \ "Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc." \ PNG_STRING_NEWLINE; # else - return "libpng version 1.5.6beta06 - October 12, 2011\ + return "libpng version 1.5.6beta06 - October 14, 2011\ Copyright (c) 1998-2011 Glenn Randers-Pehrson\ Copyright (c) 1996-1997 Andreas Dilger\ Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc."; @@ -2663,6 +2663,60 @@ png_build_8bit_table(png_structp png_ptr, png_bytepp ptable, table[i] = (png_byte)i; } +/* Used from png_read_destroy and below to release the memory used by the gamma + * tables. + */ +void /* PRIVATE */ +png_destroy_gamma_table(png_structp png_ptr) +{ + png_free(png_ptr, png_ptr->gamma_table); + png_ptr->gamma_table = NULL; + + if (png_ptr->gamma_16_table != NULL) + { + int i; + int istop = (1 << (8 - png_ptr->gamma_shift)); + for (i = 0; i < istop; i++) + { + png_free(png_ptr, png_ptr->gamma_16_table[i]); + } + png_free(png_ptr, png_ptr->gamma_16_table); + png_ptr->gamma_16_table = NULL; + } + +#if defined(PNG_READ_BACKGROUND_SUPPORTED) || \ + defined(PNG_READ_ALPHA_MODE_SUPPORTED) || \ + defined(PNG_READ_RGB_TO_GRAY_SUPPORTED) + png_free(png_ptr, png_ptr->gamma_from_1); + png_ptr->gamma_from_1 = NULL; + png_free(png_ptr, png_ptr->gamma_to_1); + png_ptr->gamma_to_1 = NULL; + + if (png_ptr->gamma_16_from_1 != NULL) + { + int i; + int istop = (1 << (8 - png_ptr->gamma_shift)); + for (i = 0; i < istop; i++) + { + png_free(png_ptr, png_ptr->gamma_16_from_1[i]); + } + png_free(png_ptr, png_ptr->gamma_16_from_1); + png_ptr->gamma_16_from_1 = NULL; + } + if (png_ptr->gamma_16_to_1 != NULL) + { + int i; + int istop = (1 << (8 - png_ptr->gamma_shift)); + for (i = 0; i < istop; i++) + { + png_free(png_ptr, png_ptr->gamma_16_to_1[i]); + } + png_free(png_ptr, png_ptr->gamma_16_to_1); + png_ptr->gamma_16_to_1 = NULL; + } +#endif /* READ_BACKGROUND || READ_ALPHA_MODE || RGB_TO_GRAY */ +} + /* We build the 8- or 16-bit gamma tables here. Note that for 16-bit * tables, we don't make a full table if we are reducing to 8-bit in * the future. Note also how the gamma_16 tables are segmented so that @@ -2673,6 +2727,18 @@ png_build_gamma_table(png_structp png_ptr, int bit_depth) { png_debug(1, "in png_build_gamma_table"); + /* Remove any existing table; this copes with multiple calls to + * png_read_update_info. The warning is because building the gamma tables + * multiple times is a performance hit - it's harmless but the ability to call + * png_read_update_info() multiple times is new in 1.5.6 so it seems sensible + * to warn if the app introduces such a hit. + */ + if (png_ptr->gamma_table != NULL || png_ptr->gamma_16_table != NULL) + { + png_warning(png_ptr, "gamma table being rebuilt"); + png_destroy_gamma_table(png_ptr); + } + if (bit_depth <= 8) { png_build_8bit_table(png_ptr, &png_ptr->gamma_table, diff --git a/pngpriv.h b/pngpriv.h index 0f06d11cd..ca36fe18a 100644 --- a/pngpriv.h +++ b/pngpriv.h @@ -1568,6 +1568,7 @@ PNG_EXTERN png_uint_16 png_gamma_16bit_correct PNGARG((unsigned int value, png_fixed_point gamma_value)); PNG_EXTERN png_byte png_gamma_8bit_correct PNGARG((unsigned int value, png_fixed_point gamma_value)); +PNG_EXTERN void png_destroy_gamma_table(png_structp png_ptr); PNG_EXTERN void png_build_gamma_table PNGARG((png_structp png_ptr, int bit_depth)); #endif diff --git a/pngread.c b/pngread.c index 53b017263..89cf14567 100644 --- a/pngread.c +++ b/pngread.c @@ -1032,6 +1032,10 @@ png_read_destroy(png_structp png_ptr, png_infop info_ptr, if (end_info_ptr != NULL) png_info_destroy(png_ptr, end_info_ptr); +#ifdef PNG_READ_GAMMA_SUPPORTED + png_destroy_gamma_table(png_ptr); +#endif + png_free(png_ptr, png_ptr->zbuf); png_free(png_ptr, png_ptr->big_row_buf); png_free(png_ptr, png_ptr->prev_row); @@ -1042,15 +1046,6 @@ png_read_destroy(png_structp png_ptr, png_infop info_ptr, png_free(png_ptr, png_ptr->quantize_index); #endif -#ifdef PNG_READ_GAMMA_SUPPORTED - png_free(png_ptr, png_ptr->gamma_table); -#endif - -#ifdef PNG_READ_BACKGROUND_SUPPORTED - png_free(png_ptr, png_ptr->gamma_from_1); - png_free(png_ptr, png_ptr->gamma_to_1); -#endif - if (png_ptr->free_me & PNG_FREE_PLTE) png_zfree(png_ptr, png_ptr->palette); png_ptr->free_me &= ~PNG_FREE_PLTE; @@ -1068,42 +1063,6 @@ png_read_destroy(png_structp png_ptr, png_infop info_ptr, png_ptr->free_me &= ~PNG_FREE_HIST; #endif -#ifdef PNG_READ_GAMMA_SUPPORTED - if (png_ptr->gamma_16_table != NULL) - { - int i; - int istop = (1 << (8 - png_ptr->gamma_shift)); - for (i = 0; i < istop; i++) - { - png_free(png_ptr, png_ptr->gamma_16_table[i]); - } - png_free(png_ptr, png_ptr->gamma_16_table); - } - -#ifdef PNG_READ_BACKGROUND_SUPPORTED - if (png_ptr->gamma_16_from_1 != NULL) - { - int i; - int istop = (1 << (8 - png_ptr->gamma_shift)); - for (i = 0; i < istop; i++) - { - png_free(png_ptr, png_ptr->gamma_16_from_1[i]); - } - png_free(png_ptr, png_ptr->gamma_16_from_1); - } - if (png_ptr->gamma_16_to_1 != NULL) - { - int i; - int istop = (1 << (8 - png_ptr->gamma_shift)); - for (i = 0; i < istop; i++) - { - png_free(png_ptr, png_ptr->gamma_16_to_1[i]); - } - png_free(png_ptr, png_ptr->gamma_16_to_1); - } -#endif -#endif - inflateEnd(&png_ptr->zstream); #ifdef PNG_PROGRESSIVE_READ_SUPPORTED diff --git a/pngrtran.c b/pngrtran.c index 737970011..e87776ad9 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -1641,8 +1641,9 @@ png_init_read_transformations(png_structp png_ptr) /* if (png_ptr->background_gamma_type!=PNG_BACKGROUND_GAMMA_UNKNOWN) */ else /* color_type != PNG_COLOR_TYPE_PALETTE */ { - png_fixed_point g = PNG_FP_1; - png_fixed_point gs = PNG_FP_1; + int gs_sig, g_sig; + png_fixed_point g = PNG_FP_1; /* Correction to linear */ + png_fixed_point gs = PNG_FP_1; /* Correction to screen */ switch (png_ptr->background_gamma_type) { @@ -1666,34 +1667,45 @@ png_init_read_transformations(png_structp png_ptr) png_error(png_ptr, "invalid background gamma type"); } - png_ptr->background_1.gray = png_gamma_correct(png_ptr, - png_ptr->background.gray, g); + g_sig = png_gamma_significant(g); + gs_sig = png_gamma_significant(gs); - png_ptr->background.gray = png_gamma_correct(png_ptr, - png_ptr->background.gray, gs); + if (g_sig) + png_ptr->background_1.gray = png_gamma_correct(png_ptr, + png_ptr->background.gray, g); + + if (gs_sig) + png_ptr->background.gray = png_gamma_correct(png_ptr, + png_ptr->background.gray, gs); if ((png_ptr->background.red != png_ptr->background.green) || (png_ptr->background.red != png_ptr->background.blue) || (png_ptr->background.red != png_ptr->background.gray)) { /* RGB or RGBA with color background */ - png_ptr->background_1.red = png_gamma_correct(png_ptr, - png_ptr->background.red, g); + if (g_sig) + { + png_ptr->background_1.red = png_gamma_correct(png_ptr, + png_ptr->background.red, g); - png_ptr->background_1.green = png_gamma_correct(png_ptr, - png_ptr->background.green, g); + png_ptr->background_1.green = png_gamma_correct(png_ptr, + png_ptr->background.green, g); - png_ptr->background_1.blue = png_gamma_correct(png_ptr, - png_ptr->background.blue, g); + png_ptr->background_1.blue = png_gamma_correct(png_ptr, + png_ptr->background.blue, g); + } - png_ptr->background.red = png_gamma_correct(png_ptr, - png_ptr->background.red, gs); + if (gs_sig) + { + png_ptr->background.red = png_gamma_correct(png_ptr, + png_ptr->background.red, gs); - png_ptr->background.green = png_gamma_correct(png_ptr, - png_ptr->background.green, gs); + png_ptr->background.green = png_gamma_correct(png_ptr, + png_ptr->background.green, gs); - png_ptr->background.blue = png_gamma_correct(png_ptr, - png_ptr->background.blue, gs); + png_ptr->background.blue = png_gamma_correct(png_ptr, + png_ptr->background.blue, gs); + } } else @@ -1705,6 +1717,9 @@ png_init_read_transformations(png_structp png_ptr) png_ptr->background.red = png_ptr->background.green = png_ptr->background.blue = png_ptr->background.gray; } + + /* The background is now in screen gamma: */ + png_ptr->background_gamma_type = PNG_BACKGROUND_GAMMA_SCREEN; } /* color_type != PNG_COLOR_TYPE_PALETTE */ }/* png_ptr->transformations & PNG_BACKGROUND */ @@ -3404,7 +3419,8 @@ png_build_grayscale_palette(int bit_depth, png_colorp palette) #ifdef PNG_READ_TRANSFORMS_SUPPORTED -#ifdef PNG_READ_BACKGROUND_SUPPORTED +#if (defined PNG_READ_BACKGROUND_SUPPORTED) ||\ + (defined PNG_READ_ALPHA_MODE_SUPPORTED) /* Replace any alpha or transparency with the supplied background color. * "background" is already in the screen gamma, while "background_1" is * at a gamma of 1.0. Paletted files have already been taken care of. @@ -4112,7 +4128,7 @@ png_do_compose(png_row_infop row_info, png_bytep row, png_structp png_ptr) } } } -#endif +#endif /* PNG_READ_BACKGROUND_SUPPORTED || PNG_READ_ALPHA_MODE_SUPPORTED */ #ifdef PNG_READ_GAMMA_SUPPORTED /* Gamma correct the image, avoiding the alpha channel. Make sure diff --git a/pngstruct.h b/pngstruct.h index ec62a78cb..658a25b66 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -157,19 +157,21 @@ struct png_struct_def png_uint_32 flush_rows; /* number of rows written since last flush */ #endif -#if defined(PNG_READ_GAMMA_SUPPORTED) || defined(PNG_READ_BACKGROUND_SUPPORTED) +#ifdef PNG_READ_GAMMA_SUPPORTED int gamma_shift; /* number of "insignificant" bits in 16-bit gamma */ png_fixed_point gamma; /* file gamma value */ png_fixed_point screen_gamma; /* screen gamma value (display_exponent) */ -#endif -#if defined(PNG_READ_GAMMA_SUPPORTED) || defined(PNG_READ_BACKGROUND_SUPPORTED) png_bytep gamma_table; /* gamma table for 8-bit depth files */ + png_uint_16pp gamma_16_table; /* gamma table for 16-bit depth files */ +#if defined(PNG_READ_BACKGROUND_SUPPORTED) || \ + defined(PNG_READ_ALPHA_MODE_SUPPORTED) || \ + defined(PNG_READ_RGB_TO_GRAY_SUPPORTED) png_bytep gamma_from_1; /* converts from 1.0 to screen */ png_bytep gamma_to_1; /* converts from file to 1.0 */ - png_uint_16pp gamma_16_table; /* gamma table for 16-bit depth files */ png_uint_16pp gamma_16_from_1; /* converts from 1.0 to screen */ png_uint_16pp gamma_16_to_1; /* converts from file to 1.0 */ +#endif /* READ_BACKGROUND || READ_ALPHA_MODE || RGB_TO_GRAY */ #endif #if defined(PNG_READ_GAMMA_SUPPORTED) || defined(PNG_sBIT_SUPPORTED) diff --git a/pngvalid.c b/pngvalid.c index cad41253c..c8952207d 100644 --- a/pngvalid.c +++ b/pngvalid.c @@ -7748,7 +7748,7 @@ gamma_component_validate(PNG_CONST char *name, PNG_CONST validate_info *vi, case ALPHA_MODE_OFFSET + PNG_ALPHA_BROKEN: case ALPHA_MODE_OFFSET + PNG_ALPHA_OPTIMIZED: # endif /* ALPHA_MODE_SUPPORTED */ - do_compose = (alpha >= 0 && alpha < 1); + do_compose = (alpha > 0 && alpha < 1); use_input = (alpha != 0); break; diff --git a/test-pngvalid-full.sh b/test-pngvalid-full.sh index d8c6faa23..cf0e41a78 100755 --- a/test-pngvalid-full.sh +++ b/test-pngvalid-full.sh @@ -9,11 +9,11 @@ echo "============ pngvalid-full.sh ==============" >> pngtest-log.txt echo "Running test-pngvalid-full.sh" for gamma in threshold transform sbit 16-to-8 background alpha-mode "transform --expand16" "background --expand16" "alpha-mode --expand16" do - if ./pngvalid --gamma-$gamma >> pngtest-log.txt 2>&1 + if ./pngvalid "$@" --gamma-$gamma >> pngtest-log.txt 2>&1 then - echo " PASS:" pngvalid "--gamma-$gamma" + echo " PASS: pngvalid" "$@" "--gamma-$gamma" else - echo " FAIL:" pngvalid "--gamma-$gamma" + echo " FAIL: pngvalid" "$@" "--gamma-$gamma" err=1 fi done diff --git a/test-pngvalid-simple.sh b/test-pngvalid-simple.sh index 9eb0c0a06..7d4d9d83c 100755 --- a/test-pngvalid-simple.sh +++ b/test-pngvalid-simple.sh @@ -18,11 +18,11 @@ for opts in "--standard" "--standard --progressive-read" \ "--size" "--size --progressive-read" \ "--transform" do - if ./pngvalid $opts >> pngtest-log.txt 2>&1 + if ./pngvalid "$@" $opts >> pngtest-log.txt 2>&1 then - echo " PASS:" pngvalid $opts + echo " PASS: pngvalid" "$@" $opts else - echo " FAIL:" pngvalid $opts + echo " FAIL: pngvalid" "$@" $opts err=1 fi done