From 8fe2eac47fe79c128eae4b7466d9955ae4be9701 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Mon, 30 Nov 2015 21:00:22 -0800 Subject: [PATCH 1/2] Separate png_compress_IDAT into w/c I.e. write/compress, also remove some trailing spaces and clean up pnglibconf stuff. Signed-off-by: John Bowler --- pngstruct.h | 28 +-- pngwutil.c | 311 ++++++++++++++++++---------------- scripts/options.awk | 2 +- scripts/pnglibconf.dfa | 6 +- scripts/pnglibconf.h.prebuilt | 5 +- 5 files changed, 189 insertions(+), 163 deletions(-) diff --git a/pngstruct.h b/pngstruct.h index cae87cf10..43b39fae5 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -566,7 +566,7 @@ struct png_struct_def * wherein !(r==g==b). */ #endif /* RGB_TO_GRAY */ -#endif /* TRANFORM_MECH */ +#endif /* TRANSFORM_MECH */ #ifdef PNG_READ_SUPPORTED /* These, and IDAT_size below, control how much input and output (at most) is @@ -599,12 +599,7 @@ struct png_struct_def int zlib_set_window_bits; int zlib_set_mem_level; int zlib_set_strategy; - - unsigned int zbuffer_start; /* Bytes written from start */ - png_uint_32 zbuffer_len; /* Length of data in list */ - png_compression_bufferp zbuffer_list; /* Created on demand during write */ - png_compression_bufferp *zbuffer_end; /* 'next' field of current buffer */ -#endif +#endif /* WRITE */ /* ERROR HANDLING */ #ifdef PNG_SETJMP_SUPPORTED @@ -748,14 +743,23 @@ struct png_struct_def * zlib expects a 'zstream' as the fundamental control structure, it allows * all the parameters to be passed as one pointer. */ - png_uint_32 zowner; /* ID (chunk type) of zstream owner, 0 if none */ z_stream zstream; /* decompression structure */ - unsigned int zstream_start:1; /* before first byte of stream */ - unsigned int zstream_ended:1; /* no more zlib output available */ - unsigned int zstream_error:1; /* zlib error message has been output */ + png_uint_32 zowner; /* ID (chunk type) of zstream owner, 0 if none */ +# ifdef PNG_WRITE_SUPPORTED + png_compression_bufferp zbuffer_list; /* Created on demand during write */ + png_compression_bufferp *zbuffer_end; /* 'next' field of current buffer */ + png_uint_32 zbuffer_len; /* Length of data in list */ + unsigned int zbuffer_start; /* Bytes written from start */ +# endif /* WRITE */ +# ifdef PNG_READ_SUPPORTED + unsigned int zstream_ended:1; /* no more zlib output available [read] */ + unsigned int zstream_error:1; /* zlib error message has been output [read] */ +# endif /* READ */ +# ifdef PNG_PROGRESSIVE_READ_SUPPORTED unsigned int zstream_eod :1; /* all the required uncompressed data has been * received; set by the zstream using code for - * its own purposes. */ + * its own purposes. [progressive read] */ +# endif /* PROGRESSIVE_READ */ /* MNG SUPPORT */ #ifdef PNG_MNG_FEATURES_SUPPORTED diff --git a/pngwutil.c b/pngwutil.c index 468859763..6d01aa1c8 100644 --- a/pngwutil.c +++ b/pngwutil.c @@ -1907,11 +1907,148 @@ png_start_IDAT(png_structrp png_ptr) } static void -png_compress_IDAT(png_structrp png_ptr, png_const_voidp input, uInt input_len, - int flush) +png_write_IDAT(png_structrp png_ptr, int end_of_image) { + png_compression_bufferp *listp = &png_ptr->zbuffer_list; + png_compression_bufferp list; + png_uint_32 output_len = png_ptr->zbuffer_len; + png_uint_32 start = png_ptr->zbuffer_start; + png_uint_32 size = png_ptr->IDAT_size; + const png_uint_32 min_size = (end_of_image ? 1U : size); + + affirm(output_len >= min_size); + + /* png_struct::zbuffer_end points to the pointer to the next (unused) + * compression buffer. We don't need those blocks to produce output so + * free them now to save space. This also ensures that *zbuffer_end is + * NULL so can be used to store the list head when wrapping the list. + */ + png_free_buffer_list(png_ptr, png_ptr->zbuffer_end); + + /* Write at least one chunk, of size 'size', write chunks until + * 'output_len' is less than 'min_size'. + */ + list = *listp; + + /* First, if this is the very first IDAT (PNG_HAVE_IDAT not set) + * optimize the CINFO field: + */ +# ifdef PNG_WRITE_OPTIMIZE_CMF_SUPPORTED + if ((png_ptr->mode & PNG_HAVE_IDAT) == 0U) + { + affirm(start == 0U); + optimize_cmf(list->output, png_image_size(png_ptr)); + } +# endif /* WRITE_OPTIMIZE_CMF */ + + do /* write chunks */ + { + /* 'size' is the size of the chunk to write, limited to IDAT_size: + */ + if (size > output_len) /* Z_FINISH */ + size = output_len; + + debug(size >= min_size); + png_write_chunk_header(png_ptr, png_IDAT, size); + + do /* write the data of one chunk */ + { + /* The chunk data may be split across multiple compression + * buffers. This loop moves 'list' down the available + * compression buffers. + */ + png_uint_32 avail = PNG_ROW_BUFFER_SIZE - start; /* in *list */ + + if (avail > output_len) /* valid bytes */ + avail = output_len; + + if (avail > size) /* bytes needed for chunk */ + avail = size; + + affirm(list != NULL && avail > 0U && + start+avail <= PNG_ROW_BUFFER_SIZE); + png_write_chunk_data(png_ptr, list->output+start, avail); + output_len -= avail; + size -= avail; + start += avail; + + if (start == PNG_ROW_BUFFER_SIZE) + { + /* End of the buffer. If all the compressed data has been + * consumed (output_len == 0) this will set list to NULL + * because of the png_free_buffer_list call above. At this + * point 'size' should be 0 too and the loop will terminate. + */ + start = 0U; + listp = &list->next; + list = *listp; /* May be NULL at the end */ + } + } while (size > 0); + + png_write_chunk_end(png_ptr); + size = png_ptr->IDAT_size; /* For the next chunk */ + } while (output_len >= min_size); + + png_ptr->mode |= PNG_HAVE_IDAT; + png_ptr->zbuffer_len = output_len; + + if (output_len > 0U) /* Still got stuff to write */ + { + affirm(!end_of_image && list != NULL); + + /* If any compression buffers have been completely written move them + * to the end of the list so that they can be re-used and move + * 'list' to the head: + */ + if (listp != &png_ptr->zbuffer_list) /* list not at start */ + { + debug(list != png_ptr->zbuffer_list /* obviously */ && + listp != png_ptr->zbuffer_end /* because *end == NULL */); + *png_ptr->zbuffer_end = png_ptr->zbuffer_list; + *listp = NULL; + png_ptr->zbuffer_list = list; + } + + /* 'list' is now at the start, so 'start' can be stored. */ + png_ptr->zbuffer_start = start; + png_ptr->zbuffer_len = output_len; + } + + else /* output_len == 0U; all compressed data has been written */ + { + if (end_of_image) + { + png_ptr->zowner = 0U; /* release z_stream */ + png_ptr->mode |= PNG_AFTER_IDAT; + } + + /* Else: this is unlikely but possible; the compression code managed + * to exactly fill an IDAT chunk with the data for this block of row + * bytes so nothing is left in the buffer list. Simply reset the + * output pointers to the start of the list. This code is executed + * on Z_FINISH as well just to make the state safe. + */ + png_ptr->zstream.next_out = NULL; + png_ptr->zstream.avail_out = 0U; + png_ptr->zbuffer_start = 0U; + png_ptr->zbuffer_len = 0U; + png_ptr->zbuffer_end = &png_ptr->zbuffer_list; + } /* output_len == 0 */ +} + +typedef struct +{ + png_compression_bufferp *zbuffer_end; /* 'next' field of current buffer */ + png_uint_32 zbuffer_len; /* Length of data in list */ +} png_IDAT_compression_state; + +static int +png_compress_IDAT_test(png_structrp png_ptr, png_IDAT_compression_state *state, + z_stream *zstream, png_const_voidp input, uInt input_len, int flush) +{ + png_uint_32 output_len = 0U; int ret; - + /* The stream must have been claimed: */ affirm(png_ptr->zowner == png_IDAT); @@ -1919,22 +2056,34 @@ png_compress_IDAT(png_structrp png_ptr, png_const_voidp input, uInt input_len, * buffer list. next_in must be set here, avail_in comes from the input_len * parameter: */ - { - png_uint_32 output_len = 0U; + zstream->next_in = PNGZ_INPUT_CAST(png_voidcast(const Bytef*, input)); + ret = png_compress(png_ptr, zstream, &state->zbuffer_end, input_len, + &output_len, flush); + implies(ret == Z_OK || ret == Z_FINISH, zstream->avail_in == 0U); + zstream->next_in = NULL; + zstream->avail_in = 0U; /* safety */ - png_ptr->zstream.next_in = - PNGZ_INPUT_CAST(png_voidcast(const Bytef*,input)); - ret = png_compress(png_ptr, &png_ptr->zstream, &png_ptr->zbuffer_end, - input_len, &output_len, flush); - implies(ret == Z_OK || ret == Z_FINISH, png_ptr->zstream.avail_in == 0U); - png_ptr->zstream.next_in = NULL; - png_ptr->zstream.avail_in = 0U; /* safety */ + /* If IDAT_size is set to PNG_UINT_31_MAX the length will be larger, but + * not enough to overflow a png_uint_32. + */ + state->zbuffer_len += output_len; + return ret; - /* If IDAT_size is set to PNG_UINT_31_MAX the length will be larger, but - * not enough to overflow a png_uint_32. - */ - png_ptr->zbuffer_len += output_len; - } +} + +static void +png_compress_IDAT(png_structp png_ptr, png_const_voidp input, uInt input_len, + int flush) +{ + png_IDAT_compression_state state; + int ret; + + state.zbuffer_end = png_ptr->zbuffer_end; + state.zbuffer_len = png_ptr->zbuffer_len; + ret = png_compress_IDAT_test(png_ptr, &state, &png_ptr->zstream, input, + input_len, flush); + png_ptr->zbuffer_end = state.zbuffer_end; + png_ptr->zbuffer_len = state.zbuffer_len; /* Check the return code. */ if (ret == Z_OK || ret == Z_STREAM_END) @@ -1956,133 +2105,7 @@ png_compress_IDAT(png_structrp png_ptr, png_const_voidp input, uInt input_len, * written this function call is complete. */ if (flush == Z_FINISH || png_ptr->zbuffer_len >= png_ptr->IDAT_size) - { - png_compression_bufferp *listp = &png_ptr->zbuffer_list; - png_compression_bufferp list; - png_uint_32 output_len = png_ptr->zbuffer_len; - png_uint_32 start = png_ptr->zbuffer_start; - png_uint_32 size = png_ptr->IDAT_size; - const png_uint_32 min_size = (flush == Z_FINISH ? 1U : size); - - affirm(output_len >= min_size); - - /* png_struct::zbuffer_end points to the pointer to the next (unused) - * compression buffer. We don't need those blocks to produce output so - * free them now to save space. This also ensures that *zbuffer_end is - * NULL so can be used to store the list head when wrapping the list. - */ - png_free_buffer_list(png_ptr, png_ptr->zbuffer_end); - - /* Write at least one chunk, of size 'size', write chunks until - * 'output_len' is less than 'min_size'. - */ - list = *listp; - - /* First, if this is the very first IDAT (PNG_HAVE_IDAT not set) - * optimize the CINFO field: - */ -# ifdef PNG_WRITE_OPTIMIZE_CMF_SUPPORTED - if ((png_ptr->mode & PNG_HAVE_IDAT) == 0U) - { - affirm(start == 0U); - optimize_cmf(list->output, png_image_size(png_ptr)); - } -# endif /* WRITE_OPTIMIZE_CMF */ - - do /* write chunks */ - { - /* 'size' is the size of the chunk to write, limited to IDAT_size: - */ - if (size > output_len) /* Z_FINISH */ - size = output_len; - - debug(size >= min_size); - png_write_chunk_header(png_ptr, png_IDAT, size); - - do /* write the data of one chunk */ - { - /* The chunk data may be split across multiple compression - * buffers. This loop moves 'list' down the available - * compression buffers. - */ - png_uint_32 avail = PNG_ROW_BUFFER_SIZE - start; /* in *list */ - - if (avail > output_len) /* valid bytes */ - avail = output_len; - - if (avail > size) /* bytes needed for chunk */ - avail = size; - - affirm(list != NULL && avail > 0U && - start+avail <= PNG_ROW_BUFFER_SIZE); - png_write_chunk_data(png_ptr, list->output+start, avail); - output_len -= avail; - size -= avail; - start += avail; - - if (start == PNG_ROW_BUFFER_SIZE) - { - /* End of the buffer. If all the compressed data has been - * consumed (output_len == 0) this will set list to NULL - * because of the png_free_buffer_list call above. At this - * point 'size' should be 0 too and the loop will terminate. - */ - start = 0U; - listp = &list->next; - list = *listp; /* May be NULL at the end */ - } - } while (size > 0); - - png_write_chunk_end(png_ptr); - size = png_ptr->IDAT_size; /* For the next chunk */ - } while (output_len >= min_size); - - png_ptr->mode |= PNG_HAVE_IDAT; - png_ptr->zbuffer_len = output_len; - - if (output_len > 0U) /* Still got stuff to write */ - { - affirm(flush != Z_FINISH && list != NULL); - - /* If any compression buffers have been completely written move them - * to the end of the list so that they can be re-used and move - * 'list' to the head: - */ - if (listp != &png_ptr->zbuffer_list) /* list not at start */ - { - debug(list != png_ptr->zbuffer_list /* obviously */ && - listp != png_ptr->zbuffer_end /* because *end == NULL */); - *png_ptr->zbuffer_end = png_ptr->zbuffer_list; - *listp = NULL; - png_ptr->zbuffer_list = list; - } - - /* 'list' is now at the start, so 'start' can be stored. */ - png_ptr->zbuffer_start = start; - png_ptr->zbuffer_len = output_len; - } - - else /* output_len == 0U; all compressed data has been written */ - { - if (flush == Z_FINISH) /* end of data */ - { - png_ptr->zowner = 0U; /* release z_stream */ - png_ptr->mode |= PNG_AFTER_IDAT; - } - - /* Else: this is unlikely but possible; the compression code managed - * to exactly fill an IDAT chunk with the data for this block of row - * bytes so nothing is left in the buffer list. Simply reset the - * output pointers to the start of the list. This code is executed - * on Z_FINISH as well just to make the state safe. - */ - png_ptr->zstream.next_out = NULL; - png_ptr->zstream.avail_out = 0U; - png_ptr->zbuffer_start = 0U; - png_ptr->zbuffer_len = 0U; - png_ptr->zbuffer_end = &png_ptr->zbuffer_list; - } /* output_len == 0 */ - } /* flush == FINISH || png_ptr->zbuffer_len >= png_ptr->IDAT_size */ + png_write_IDAT(png_ptr, flush == Z_FINISH); } else /* ret != Z_OK && ret != Z_STREAM_END */ diff --git a/scripts/options.awk b/scripts/options.awk index 81b82ff09..79d24bbdc 100755 --- a/scripts/options.awk +++ b/scripts/options.awk @@ -243,7 +243,7 @@ $1 == "file" && NF >= 2{ # option NAME ( (requires|enables|if) NAME* | on | off | disabled | # sets SETTING VALUE+ )* -# +# # Declares an option 'NAME' and describes its default setting (disabled) # and its relationship to other options. The option is disabled # unless *all* the options listed after 'requires' are set and at diff --git a/scripts/pnglibconf.dfa b/scripts/pnglibconf.dfa index 845a7da83..f01535457 100644 --- a/scripts/pnglibconf.dfa +++ b/scripts/pnglibconf.dfa @@ -125,7 +125,7 @@ file pnglibconf.h scripts/pnglibconf.dfa PNGLCONF_H # # To avoid confusion use -DPNG_foo_SUPPORTED= on the command line, which does # the same thing as the #define. -# +# # SUMMARY: # These lines/macro settings are equivalent: # @@ -460,7 +460,7 @@ option SET_USER_LIMITS requires USER_LIMITS # See the comments above about how to change options and settings. # READ/WRITE tranform support -# +# # The internal TRANSFORM_MECH options are used to turn on (or off) the required # support code for the read and write transforms. They are off by default, # switching them on is not a good idea. Switching them off will cause the build @@ -496,7 +496,7 @@ option READ_16BIT requires READ enables 16BIT option READ_TRANSFORMS requires READ = NO_READ_TRANSFORMS READ_TRANSFORMS_NOT_SUPPORTED -option READ_QUANTIZE requires READ_TRANSFORMS enables TRANFORM_MECH +option READ_QUANTIZE requires READ_TRANSFORMS enables TRANSFORM_MECH # Read gamma handling. Gamma processing is a core part of libpng and many of # the capabilities are dependent on libpng performing gamma correction. diff --git a/scripts/pnglibconf.h.prebuilt b/scripts/pnglibconf.h.prebuilt index 91e62e5a8..7abce4811 100644 --- a/scripts/pnglibconf.h.prebuilt +++ b/scripts/pnglibconf.h.prebuilt @@ -116,7 +116,6 @@ #define PNG_STORE_UNKNOWN_CHUNKS_SUPPORTED #define PNG_TEXT_SUPPORTED #define PNG_TIME_RFC1123_SUPPORTED -#define PNG_TRANFORM_MECH_SUPPORTED #define PNG_TRANSFORM_MECH_SUPPORTED #define PNG_UNKNOWN_CHUNKS_SUPPORTED #define PNG_USER_CHUNKS_SUPPORTED @@ -194,7 +193,7 @@ #define PNG_DEFAULT_GAMMA_ACCURACY 665 #define PNG_DEFAULT_READ_MACROS 1 #define PNG_GAMMA_THRESHOLD_FIXED 153 -#define PNG_IDAT_READ_SIZE PNG_ZBUF_SIZE +#define PNG_IDAT_READ_SIZE 4096 #define PNG_INFLATE_BUF_SIZE 1024 #define PNG_MAX_GAMMA_8 11 #define PNG_QUANTIZE_BLUE_BITS 5 @@ -206,7 +205,7 @@ #define PNG_USER_CHUNK_MALLOC_MAX 8000000 #define PNG_USER_HEIGHT_MAX 1000000 #define PNG_USER_WIDTH_MAX 1000000 -#define PNG_ZBUF_SIZE 8192 +#define PNG_ZBUF_SIZE 4096 #define PNG_ZLIB_HEADER #define PNG_ZLIB_VERNUM 0 #define PNG_Z_DEFAULT_COMPRESSION (-1) From 84a8bb8244cbba8343873b08bc6fa374d6a94d2d Mon Sep 17 00:00:00 2001 From: John Bowler Date: Tue, 1 Dec 2015 08:16:53 -0800 Subject: [PATCH 2/2] Make png_struct palette, trans_alpha private This removes the side-effect on the png_struct palette of calling png_set_PLTE or png_set_tRNS. NOTE: this is a quiet API change, it was possible before to alter the palette on a PNG image by using png_set_PLTE, but this was unintended and inconsistent with the other png_set APIs. Fix a bug in palette index checking; png_struct::num_palette could, in principle, get changed by the transformations (e.g. png_set_quantize) and this would invalidate the check. The palette checking init function now makes a copy of png_struct::num_palette. Fix a bug in pngvalid error handling. A png_error in png_write_info is not continuable (a valid image cannot necessarily be written afterward) because the png_error aborts the write of subsequent pre-IDAT chunks. In particular an abort as a result of a bogus colorspace information (gAMA, cHRM, sBIT etc) prevents the write of the PLTE chunk. Signed-off-by: John Bowler --- contrib/libtests/pngvalid.c | 91 ++++++++++++++++++++----------------- pngread.c | 6 +-- pngrtran.c | 38 ++++++++-------- pngrutil.c | 73 ++++++++++++++--------------- pngset.c | 22 +++------ pngstruct.h | 5 +- pngtrans.c | 33 ++++++++------ pngwutil.c | 8 ---- 8 files changed, 137 insertions(+), 139 deletions(-) diff --git a/contrib/libtests/pngvalid.c b/contrib/libtests/pngvalid.c index 50abd409b..c2f8bde39 100644 --- a/contrib/libtests/pngvalid.c +++ b/contrib/libtests/pngvalid.c @@ -4276,62 +4276,71 @@ make_error(png_store* const ps, png_byte const colour_type, #undef exception__env /* And clear these flags */ - ps->expect_error = 0; ps->expect_warning = 0; - /* Now write the whole image, just to make sure that the detected, or - * undetected, errro has not created problems inside libpng. - */ - if (png_get_rowbytes(pp, pi) != - transform_rowsize(pp, colour_type, bit_depth)) - png_error(pp, "row size incorrect"); + if (ps->expect_error) + ps->expect_error = 0; else { - int npasses = set_write_interlace_handling(pp, interlace_type); - int pass; + /* Now write the whole image, just to make sure that the detected, or + * undetected, errro has not created problems inside libpng. This + * doesn't work if there was a png_error in png_write_info because that + * can abort before PLTE was written. + */ + if (png_get_rowbytes(pp, pi) != + transform_rowsize(pp, colour_type, bit_depth)) + png_error(pp, "row size incorrect"); - if (npasses != npasses_from_interlace_type(pp, interlace_type)) - png_error(pp, "write: png_set_interlace_handling failed"); - - for (pass=0; pass 0) - interlace_row(buffer, buffer, - bit_size(pp, colour_type, bit_depth), w, pass, - 0/*data always bigendian*/); - else - continue; - } -# endif /* do_own_interlace */ + if (interlace_type == PNG_INTERLACE_ADAM7) + { + /* The row must not be written if it doesn't exist, + * notice that there are two conditions here, either the + * row isn't ever in the pass or the row would be but + * isn't wide enough to contribute any pixels. In fact + * the wPass test can be used to skip the whole y loop + * in this case. + */ + if (PNG_ROW_IN_INTERLACE_PASS(y, pass) && + PNG_PASS_COLS(w, pass) > 0) + interlace_row(buffer, buffer, + bit_size(pp, colour_type, bit_depth), w, pass, + 0/*data always bigendian*/); + else + continue; + } +# endif /* do_own_interlace */ - png_write_row(pp, buffer); + png_write_row(pp, buffer); + } } - } - } + } /* image writing */ - png_write_end(pp, pi); + png_write_end(pp, pi); + } /* The following deletes the file that was just written. */ store_write_reset(ps); diff --git a/pngread.c b/pngread.c index 79cbac1e4..f6b8ffabe 100644 --- a/pngread.c +++ b/pngread.c @@ -727,22 +727,20 @@ png_read_destroy(png_structrp png_ptr) png_free(png_ptr, png_ptr->read_buffer); png_ptr->read_buffer = NULL; - if ((png_ptr->free_me & PNG_FREE_PLTE) != 0) + if (png_ptr->palette != NULL) { png_free(png_ptr, png_ptr->palette); png_ptr->num_palette = 0; png_ptr->palette = NULL; } - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_PLTE); #ifdef PNG_READ_tRNS_SUPPORTED - if ((png_ptr->free_me & PNG_FREE_TRNS) != 0) + if (png_ptr->trans_alpha != NULL) { png_free(png_ptr, png_ptr->trans_alpha); png_ptr->num_trans = 0; png_ptr->trans_alpha = NULL; } - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_TRNS); #endif if (png_ptr->zstream.state != NULL) diff --git a/pngrtran.c b/pngrtran.c index 9c54978c0..6594a2850 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -5985,21 +5985,15 @@ setup_palette_cache(png_structp png_ptr, png_byte cache[8*256]) static void png_remove_PLTE_and_tRNS(png_structrp png_ptr) { - if ((png_ptr->free_me & PNG_FREE_PLTE) != 0) - { - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_PLTE); + if (png_ptr->palette != NULL) png_free(png_ptr, png_ptr->palette); - } png_ptr->palette = NULL; png_ptr->num_palette = 0; # ifdef PNG_READ_tRNS_SUPPORTED - if ((png_ptr->free_me & PNG_FREE_TRNS) != 0) - { - png_ptr->free_me &= PNG_BIC_MASK(PNG_FREE_TRNS); + if (png_ptr->trans_alpha != NULL) png_free(png_ptr, png_ptr->trans_alpha); - } png_ptr->trans_alpha = NULL; png_ptr->num_trans = 0; @@ -6102,13 +6096,10 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp, * entries were shifted or inverted. This could be fixed, but it would * complicate the libpng API to expose the information. */ - png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, - PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); - png_ptr->free_me |= PNG_FREE_PLTE; - /* Write the transformed palette: */ { - png_colorp palette = png_ptr->palette; + png_colorp palette = png_voidcast(png_colorp, png_calloc(png_ptr, + sizeof (png_color[PNG_MAX_PALETTE_LENGTH]))); png_const_bytep p; const int is_color = (cp->tend.format & PNG_FORMAT_FLAG_COLOR) != 0; unsigned int i; @@ -6118,6 +6109,10 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp, png_byte trans_alpha[PNG_MAX_PALETTE_LENGTH]; # endif /* READ_tRNS */ + memset(palette, 0xFFU, sizeof (png_color[PNG_MAX_PALETTE_LENGTH])); + png_free(png_ptr, png_ptr->palette); + png_ptr->palette = palette; + for (i=0, p=cache.b8; itend.width; ++i) { if (is_color) @@ -6148,12 +6143,17 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp, # ifdef PNG_READ_tRNS_SUPPORTED if (num_trans > 0) { - png_ptr->trans_alpha = png_voidcast(png_bytep, png_malloc(png_ptr, - PNG_MAX_PALETTE_LENGTH)); - png_ptr->free_me |= PNG_FREE_TRNS; - memcpy(png_ptr->trans_alpha, trans_alpha, num_trans); - memset(png_ptr->trans_alpha+num_trans, 0xFFU, - PNG_MAX_PALETTE_LENGTH-num_trans); + png_bytep tRNS = png_voidcast(png_bytep, png_malloc(png_ptr, + PNG_MAX_PALETTE_LENGTH)); + + memset(tRNS, 0xFFU, PNG_MAX_PALETTE_LENGTH); + + if (png_ptr->trans_alpha != NULL) + png_free(png_ptr, png_ptr->trans_alpha); + + png_ptr->trans_alpha = tRNS; + + memcpy(tRNS, trans_alpha, num_trans); png_ptr->num_trans = png_check_bits(png_ptr, num_trans, 9); } # endif /* READ_tRNS */ diff --git a/pngrutil.c b/pngrutil.c index 6336300de..7bb53bafa 100644 --- a/pngrutil.c +++ b/pngrutil.c @@ -849,8 +849,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) { png_color palette[PNG_MAX_PALETTE_LENGTH]; png_uint_32 length = png_ptr->chunk_length; - int max_palette_length, num, i; - png_colorp pal_ptr; + png_uint_32 max_palette_length, num, i; png_debug(1, "in png_handle_PLTE"); @@ -878,7 +877,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) } /* The cast is safe because 'length' is less than 3*PNG_MAX_PALETTE_LENGTH */ - num = (int)length / 3; + num = length / 3U; /* If the palette has 256 or fewer entries but is too large for the bit * depth, we don't issue an error, to preserve the behavior of previous @@ -886,21 +885,21 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) * here. */ if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) - max_palette_length = (1 << png_ptr->bit_depth); + max_palette_length = (1U << png_ptr->bit_depth); else max_palette_length = PNG_MAX_PALETTE_LENGTH; if (num > max_palette_length) num = max_palette_length; - for (i = 0, pal_ptr = palette; i < num; i++, pal_ptr++) + for (i = 0; i < num; ++i) { png_byte buf[3]; png_crc_read(png_ptr, buf, 3); - pal_ptr->red = buf[0]; - pal_ptr->green = buf[1]; - pal_ptr->blue = buf[2]; + palette[i].red = buf[0]; + palette[i].green = buf[1]; + palette[i].blue = buf[2]; } /* If we actually need the PLTE chunk (ie for a paletted image), we do @@ -911,7 +910,7 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) #ifndef PNG_READ_OPT_PLTE_SUPPORTED if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) #endif - png_crc_finish(png_ptr, (int) length - num * 3); + png_crc_finish(png_ptr, length - num * 3U); #ifndef PNG_READ_OPT_PLTE_SUPPORTED else if (png_crc_error(png_ptr)) /* Only if we have a CRC error */ @@ -942,16 +941,16 @@ png_handle_PLTE(png_structrp png_ptr, png_inforp info_ptr) } #endif /* READ_OPT_PLTE */ - /* TODO: png_set_PLTE has the side effect of setting png_ptr->palette to its - * own copy of the palette. This has the side effect that when png_start_row - * is called (this happens after any call to png_read_update_info) the - * info_ptr palette gets changed. This is extremely unexpected and - * confusing. - * - * Fix this by not sharing the palette in this way. - */ png_set_PLTE(png_ptr, info_ptr, palette, num); + /* Ok, make our own copy since the set succeeded: */ + debug(png_ptr->palette == NULL); /* should only get set once */ + png_ptr->palette = png_voidcast(png_colorp, png_malloc(png_ptr, + sizeof (png_color[PNG_MAX_PALETTE_LENGTH]))); + memset(png_ptr->palette, 0xFFU, sizeof (png_color[PNG_MAX_PALETTE_LENGTH])); + memcpy(png_ptr->palette, info_ptr->palette, 3*num); + png_ptr->num_palette = info_ptr->num_palette; + /* The three chunks, bKGD, hIST and tRNS *must* appear after PLTE and before * IDAT. Prior to 1.6.0 this was not checked; instead the code merely * checked the apparent validity of a tRNS chunk inserted before PLTE on a @@ -1606,10 +1605,13 @@ png_handle_sPLT(png_structrp png_ptr, png_inforp info_ptr) static void png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) { + png_uint_32 num_trans; png_byte readbuf[PNG_MAX_PALETTE_LENGTH]; png_debug(1, "in png_handle_tRNS"); + png_ptr->num_trans = 0U; /* safety */ + if (info_ptr != NULL && (info_ptr->valid & PNG_INFO_tRNS)) { /* libpng 1.7.0: this used to be a benign error, but it doesn't look very @@ -1633,7 +1635,7 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) } png_crc_read(png_ptr, buf, 2); - png_ptr->num_trans = 1; + num_trans = 1U; png_ptr->trans_color.gray = png_get_uint_16(buf); } @@ -1648,7 +1650,7 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) } png_crc_read(png_ptr, buf, 6); - png_ptr->num_trans = 1; + num_trans = 1U; png_ptr->trans_color.red = png_get_uint_16(buf); png_ptr->trans_color.green = png_get_uint_16(buf + 2); png_ptr->trans_color.blue = png_get_uint_16(buf + 4); @@ -1656,21 +1658,18 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) else if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) { - png_uint_32 length; - /* png_find_chunk_op checks this: */ debug(png_ptr->mode & PNG_HAVE_PLTE); - length = png_ptr->chunk_length; + num_trans = png_ptr->chunk_length; - if (length > png_ptr->num_palette || length == 0) + if (num_trans > png_ptr->num_palette || num_trans == 0) { png_handle_bad_length(png_ptr); return; } - png_crc_read(png_ptr, readbuf, length); - png_ptr->num_trans = length & 0x1FF; + png_crc_read(png_ptr, readbuf, num_trans); } else @@ -1680,20 +1679,22 @@ png_handle_tRNS(png_structrp png_ptr, png_inforp info_ptr) } if (png_crc_finish(png_ptr, 0)) - { - png_ptr->num_trans = 0; return; + + /* Set it into the info_struct: */ + png_set_tRNS(png_ptr, info_ptr, readbuf, num_trans, &png_ptr->trans_color); + + /* Now make a copy of the buffer if one is required (palette images). */ + debug(png_ptr->trans_alpha == NULL); + if (png_ptr->color_type == PNG_COLOR_TYPE_PALETTE) + { + png_ptr->trans_alpha = png_voidcast(png_bytep, + png_malloc(png_ptr, PNG_MAX_PALETTE_LENGTH)); + memset(png_ptr->trans_alpha, 0xFFU, PNG_MAX_PALETTE_LENGTH); + memcpy(png_ptr->trans_alpha, info_ptr->trans_alpha, num_trans); } - /* TODO: this is a horrible side effect in the palette case because the - * png_struct ends up with a pointer to the tRNS buffer owned by the - * png_info. Fix this. - */ - png_set_tRNS(png_ptr, info_ptr, readbuf, png_ptr->num_trans, - &(png_ptr->trans_color)); - - if (info_ptr != NULL) - png_ptr->trans_alpha = info_ptr->trans_alpha; + png_ptr->num_trans = png_check_bits(png_ptr, num_trans, 9); } #else # define png_handle_tRNS NULL diff --git a/pngset.c b/pngset.c index 83857f942..033ef1f56 100644 --- a/pngset.c +++ b/pngset.c @@ -533,30 +533,20 @@ png_set_PLTE(png_structrp png_ptr, png_inforp info_ptr, png_error(png_ptr, "Invalid palette"); #endif /* MNG_FEATURES */ - /* It may not actually be necessary to set png_ptr->palette here; - * we do it for backward compatibility with the way the png_handle_tRNS - * function used to do the allocation. - * - * 1.6.0: the above statement appears to be incorrect; something has to set - * the palette inside png_struct on read. - */ png_free_data(png_ptr, info_ptr, PNG_FREE_PLTE, 0); /* Changed in libpng-1.2.1 to allocate PNG_MAX_PALETTE_LENGTH instead * of num_palette entries, in case of an invalid PNG file or incorrect * call to png_set_PLTE() with too-large sample values. */ - png_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, + info_ptr->palette = png_voidcast(png_colorp, png_calloc(png_ptr, PNG_MAX_PALETTE_LENGTH * (sizeof (png_color)))); if (num_palette > 0) - memcpy(png_ptr->palette, palette, num_palette * (sizeof (png_color))); - info_ptr->palette = png_ptr->palette; - info_ptr->num_palette = png_ptr->num_palette = png_check_bits(png_ptr, - num_palette, 9); + memcpy(info_ptr->palette, palette, num_palette * (sizeof (png_color))); + info_ptr->num_palette = png_check_bits(png_ptr, num_palette, 9); info_ptr->free_me |= PNG_FREE_PLTE; - info_ptr->valid |= PNG_INFO_PLTE; } @@ -962,11 +952,11 @@ png_set_tRNS(png_structrp png_ptr, png_inforp info_ptr, /* Expect png_set_PLTE to happen before png_set_tRNS, so num_palette will * be set, but this is not a requirement of the API. */ - if (png_ptr->num_palette) - max_num = png_ptr->num_palette; + if (info_ptr->num_palette) + max_num = info_ptr->num_palette; else - max_num = 1 << png_ptr->bit_depth; + max_num = 1 << info_ptr->bit_depth; if (num_trans > max_num) { diff --git a/pngstruct.h b/pngstruct.h index 43b39fae5..eadd5b9e2 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -370,17 +370,18 @@ struct png_struct_def * the hope is that such processors will generate code that is both smaller * and faster. */ +#ifdef PNG_READ_SUPPORTED png_colorp palette; /* palette from the input file */ +#endif /* READ */ #ifdef PNG_READ_tRNS_SUPPORTED png_bytep trans_alpha; /* alpha values for paletted files */ -#endif +#endif /* READ_tRNS */ png_uint_32 width; /* width of image in pixels */ png_uint_32 height; /* height of image in pixels */ png_uint_32 chunk_name; /* PNG_CHUNK() id of current chunk */ png_uint_32 chunk_length; /* Length (possibly remaining) in said chunk. */ png_uint_32 crc; /* current chunk CRC value */ - png_uint_32 free_me; /* items libpng is responsible for freeing */ unsigned int flags; /* flags (should be bit fields) */ unsigned int mode :6; /* where we are in the PNG file */ diff --git a/pngtrans.c b/pngtrans.c index 5ee0ab923..df1bc7bdc 100644 --- a/pngtrans.c +++ b/pngtrans.c @@ -580,12 +580,13 @@ init_transform_mech(png_structrp png_ptr, png_transform_control *tc, int start) #ifdef PNG_PALETTE_MAX_SUPPORTED static int -set_palette_max(png_structrp png_ptr, png_transformp tr, unsigned int max) +set_palette_max(png_structrp png_ptr, png_transformp tr, unsigned int max, + unsigned int format_max) /* Called whenever a new maximum pixel value is found */ { /* One of these must be true: */ # ifdef PNG_CHECK_FOR_INVALID_INDEX_SUPPORTED - if (max >= png_ptr->num_palette && !png_ptr->palette_index_check_issued) + if (max >= tr->args && !png_ptr->palette_index_check_issued) { # ifdef PNG_READ_SUPPORTED # ifdef PNG_WRITE_SUPPORTED @@ -604,7 +605,7 @@ set_palette_max(png_structrp png_ptr, png_transformp tr, unsigned int max) png_ptr->palette_index_max = png_check_bits(png_ptr, max, 9); # endif - if (max == (1U << png_ptr->bit_depth)-1U) + if (max == format_max) { tr->fn = NULL; /* no point continuing once the max has been seen */ return 1; /* stop */ @@ -633,7 +634,7 @@ palette_max_1bpp(png_transformp *tr, png_transform_controlp tc) } /* If the code reaches this point there is a set pixel */ - (void)set_palette_max(tc->png_ptr, *tr, 1); + (void)set_palette_max(tc->png_ptr, *tr, 1U, 1U); } static void @@ -690,7 +691,7 @@ palette_max_2bpp(png_transformp *tr, png_transform_controlp tc) continue; /* new_max is greater than max: */ - if (set_palette_max(tc->png_ptr, *tr, new_max)) + if (set_palette_max(tc->png_ptr, *tr, new_max, 3U)) return; /* Record new_max: */ @@ -726,7 +727,7 @@ palette_max_4bpp(png_transformp *tr, png_transform_controlp tc) if (max > (*tr)->args) { - if (set_palette_max(tc->png_ptr, *tr, max)) + if (set_palette_max(tc->png_ptr, *tr, max, 15U)) return; (*tr)->args = max; @@ -752,7 +753,7 @@ palette_max_8bpp(png_transformp *tr, png_transform_controlp tc) if (max > (*tr)->args) { - if (set_palette_max(tc->png_ptr, *tr, max)) + if (set_palette_max(tc->png_ptr, *tr, max, 255U)) return; (*tr)->args = max; @@ -765,13 +766,19 @@ palette_max_init(png_transformp *tr, png_transform_controlp tc) # define png_ptr (tc->png_ptr) if ((tc->format & PNG_FORMAT_FLAG_COLORMAP) != 0) { - if (tc->init == PNG_TC_INIT_FINAL) switch (tc->bit_depth) + if (tc->init == PNG_TC_INIT_FINAL) { - case 1: (*tr)->fn = palette_max_1bpp; break; - case 2: (*tr)->fn = palette_max_2bpp; break; - case 4: (*tr)->fn = palette_max_4bpp; break; - case 8: (*tr)->fn = palette_max_8bpp; break; - default:impossible("palette bit depth"); + /* Record the palette depth to check here: */ + (*tr)->args = png_ptr->num_palette; + + switch (tc->bit_depth) + { + case 1: (*tr)->fn = palette_max_1bpp; break; + case 2: (*tr)->fn = palette_max_2bpp; break; + case 4: (*tr)->fn = palette_max_4bpp; break; + case 8: (*tr)->fn = palette_max_8bpp; break; + default:impossible("palette bit depth"); + } } } diff --git a/pngwutil.c b/pngwutil.c index 6d01aa1c8..d6998e32f 100644 --- a/pngwutil.c +++ b/pngwutil.c @@ -1319,14 +1319,6 @@ png_write_tRNS(png_structrp png_ptr, png_const_bytep trans_alpha, if (color_type == PNG_COLOR_TYPE_PALETTE) { affirm(num_trans > 0 && num_trans <= PNG_MAX_PALETTE_LENGTH); - if ((unsigned int)/*SAFE*/num_trans > png_ptr->num_palette) - { - /* This is an error which can only be reliably detected late. */ - png_app_error(png_ptr, - "Invalid number of transparent colors specified"); - return; - } - { # ifdef PNG_WRITE_INVERT_ALPHA_SUPPORTED union