From dff6f4c4f06f655309783673f3ec5ce7d4d03810 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Thu, 9 Aug 2012 21:17:56 -0500 Subject: [PATCH] [libpng16Cleanup of png_set_filler(). This function does very different things on read and write. In libpng 1.6 the two cases can be distinguished and considerable code cleanup, and extra error checking, is possible. This makes calls on the write side that have no effect be ignored with a png_app_error(), which can be disabled in the app using png_set_benign_errors(), and removes the spurious use of usr_channels on the read side. --- ANNOUNCE | 7 +++++ CHANGES | 7 +++++ pngpriv.h | 2 +- pngstruct.h | 2 ++ pngtrans.c | 89 +++++++++++++++++++++++++++++++++++++++++------------ 5 files changed, 86 insertions(+), 21 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 471c8cff3..c8e0d6f63 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -415,6 +415,13 @@ Version 1.6.0beta27 [August 10, 2012] it with strncpy(). Eliminated use of png_sizeof(); use sizeof() instead. Use a consistent style for (sizeof type) and (sizeof (array)) + Cleanup of png_set_filler(). This function does very different things on + read and write. In libpng 1.6 the two cases can be distinguished and + considerable code cleanup, and extra error checking, is possible. This + makes calls on the write side that have no effect be ignored with a + png_app_error(), which can be disabled in the app using + png_set_benign_errors(), and removes the spurious use of usr_channels + on the read side. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/CHANGES b/CHANGES index f8c5d13ee..2362c9b8a 100644 --- a/CHANGES +++ b/CHANGES @@ -4166,6 +4166,13 @@ Version 1.6.0beta27 [August 10, 2012] it with strncpy(). Eliminated use of png_sizeof(); use sizeof() instead. Use a consistent style for (sizeof type) and (sizeof (array)) + Cleanup of png_set_filler(). This function does very different things on + read and write. In libpng 1.6 the two cases can be distinguished and + considerable code cleanup, and extra error checking, is possible. This + makes calls on the write side that have no effect be ignored with a + png_app_error(), which can be disabled in the app using + png_set_benign_errors(), and removes the spurious use of usr_channels + on the read side. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/pngpriv.h b/pngpriv.h index 1bcb05728..c543c6197 100644 --- a/pngpriv.h +++ b/pngpriv.h @@ -1577,7 +1577,7 @@ PNG_INTERNAL_FUNCTION(void,png_formatted_warning,(png_const_structrp png_ptr, PNG_INTERNAL_FUNCTION(void,png_app_warning,(png_const_structrp png_ptr, png_const_charp message),PNG_EMPTY); /* The application provided invalid parameters to an API function or called - * an API function at the wrong time, libpng can completely recovered. + * an API function at the wrong time, libpng can completely recover. */ PNG_INTERNAL_FUNCTION(void,png_app_error,(png_const_structrp png_ptr, diff --git a/pngstruct.h b/pngstruct.h index 8a63ff91f..d1bcf322f 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -252,7 +252,9 @@ struct png_struct_def png_byte usr_bit_depth; /* bit depth of users row: write only */ png_byte pixel_depth; /* number of bits per pixel */ png_byte channels; /* number of channels in file */ +#ifdef PNG_WRITE_SUPPORTED png_byte usr_channels; /* channels at start of write: write only */ +#endif png_byte sig_bytes; /* magic bytes read/written from start of file */ png_byte maximum_pixel_depth; /* pixel depth used for the row buffers */ diff --git a/pngtrans.c b/pngtrans.c index bbf97a642..058280e1f 100644 --- a/pngtrans.c +++ b/pngtrans.c @@ -122,32 +122,79 @@ png_set_filler(png_structrp png_ptr, png_uint_32 filler, int filler_loc) if (png_ptr == NULL) return; + /* In libpng 1.6 it is possible to determine whether this is a read or write + * operation and therefore to do more checking here for a valid call. + */ + if (png_ptr->mode & PNG_IS_READ_STRUCT) + { +# ifdef PNG_READ_FILLER_SUPPORTED + /* On read png_set_filler is always valid, regardless of the base PNG + * format, because other transformations can give a format where the + * filler code can execute (basically an 8 or 16-bit component RGB or G + * format.) + * + * NOTE: usr_channels is not used by the read code! (This has led to + * confusion in the past.) The filler is only used in the read code. + */ + png_ptr->filler = (png_uint_16)filler; +# else + png_app_error(png_ptr, "png_set_filler not supported on read"); + PNG_UNUSED(filler); /* not used in the write case */ + return; +# endif + } + + else /* write */ + { +# ifdef PNG_WRITE_FILLER_SUPPORTED + /* On write the usr_channels parameter must be set correctly at the + * start to record the number of channels in the app-supplied data. + */ + switch (png_ptr->color_type) + { + case PNG_COLOR_TYPE_RGB: + png_ptr->usr_channels = 4; + break; + + case PNG_COLOR_TYPE_GRAY: + if (png_ptr->bit_depth >= 8) + { + png_ptr->usr_channels = 2; + break; + } + + else + { + /* There simply isn't any code in libpng to strip out bits + * from bytes when the components are less than a byte in + * size! + */ + png_app_error(png_ptr, + "png_set_filler is invalid for low bit depth gray output"); + return; + } + + default: + png_app_error(png_ptr, + "png_set_filler: inappropriate color type"); + return; + } +# else + png_app_error(png_ptr, "png_set_filler not supported on write"); + return; +# endif + } + + /* Here on success - libpng supports the operation, set the transformation + * and the flag to say where the filler channel is. + */ png_ptr->transformations |= PNG_FILLER; - png_ptr->filler = (png_uint_16)filler; if (filler_loc == PNG_FILLER_AFTER) png_ptr->flags |= PNG_FLAG_FILLER_AFTER; else png_ptr->flags &= ~PNG_FLAG_FILLER_AFTER; - - /* This should probably go in the "do_read_filler" routine. - * I attempted to do that in libpng-1.0.1a but that caused problems - * so I restored it in libpng-1.0.2a - */ - - if (png_ptr->color_type == PNG_COLOR_TYPE_RGB) - { - png_ptr->usr_channels = 4; - } - - /* Also I added this in libpng-1.0.2a (what happens when we expand - * a less-than-8-bit grayscale to GA?) */ - - if (png_ptr->color_type == PNG_COLOR_TYPE_GRAY && png_ptr->bit_depth >= 8) - { - png_ptr->usr_channels = 2; - } } /* Added to libpng-1.2.7 */ @@ -160,7 +207,9 @@ png_set_add_alpha(png_structrp png_ptr, png_uint_32 filler, int filler_loc) return; png_set_filler(png_ptr, filler, filler_loc); - png_ptr->transformations |= PNG_ADD_ALPHA; + /* The above may fail to do anything. */ + if (png_ptr->transformations & PNG_FILLER) + png_ptr->transformations |= PNG_ADD_ALPHA; } #endif