[devel] IDAT compression failed if preceded by a compressed text chunk
This was because the attempt to reset the zlib stream in png_write_IDAT
happened after the first IDAT chunk had been deflated - much too late.
In this change internal functions are added to claim/release the z_stream
and, hopefully, make the code more robust. Also deflateEnd checking is
added - previously libpng would ignore an error at the end of the stream.
diff --git a/ANNOUNCE b/ANNOUNCE
index 814d178..0181bd0 100644
--- a/ANNOUNCE
+++ b/ANNOUNCE
@@ -89,6 +89,13 @@
PNG_NO_WARNINGS does actually work now.
Added PNG_WRITE_OPTIMIZE_CMF_SUPPORTED macro to make the zlib "CMF" byte
optimization configureable.
+ IDAT compression failed if preceded by a compressed text chunk
+ This was because the attempt to reset the zlib stream in png_write_IDAT
+ happened after the first IDAT chunk had been deflated - much too late.
+ In this change internal functions are added to claim/release the z_stream
+ and, hopefully, make the code more robust. Also deflateEnd checking is
+ added - previously libpng would ignore an error at the end of the stream.
+
Send comments/corrections/commendations to png-mng-implement at lists.sf.net:
(subscription required; visit
diff --git a/CHANGES b/CHANGES
index 1aaa5b1..46aa680 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3348,6 +3348,12 @@
PNG_NO_WARNINGS does actually work now.
Added PNG_WRITE_OPTIMIZE_CMF_SUPPORTED macro to make the zlib "CMF" byte
optimization configureable.
+ IDAT compression failed if preceded by a compressed text chunk
+ This was because the attempt to reset the zlib stream in png_write_IDAT
+ happened after the first IDAT chunk had been deflated - much too late.
+ In this change internal functions are added to claim/release the z_stream
+ and, hopefully, make the code more robust. Also deflateEnd checking is
+ added - previously libpng would ignore an error at the end of the stream.
Send comments/corrections/commendations to png-mng-implement at lists.sf.net
(subscription required; visit
diff --git a/png.h b/png.h
index 9b13185..4d1d53c 100644
--- a/png.h
+++ b/png.h
@@ -1394,6 +1394,7 @@
#define PNG_FILTER_HEURISTIC_WEIGHTED 2 /* Experimental feature */
#define PNG_FILTER_HEURISTIC_LAST 3 /* Not a valid value */
+#ifdef PNG_WRITE_SUPPORTED
/* Set the library compression level. Currently, valid values range from
* 0 - 9, corresponding directly to the zlib compression levels 0 - 9
* (0 - no compression, 9 - "maximal" compression). Note that tests have
@@ -1415,6 +1416,7 @@
PNG_EXPORT(73, void, png_set_compression_method, (png_structp png_ptr,
int method));
+#endif
#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION
/* Also set zlib parameters for compressing non-IDAT chunks */
diff --git a/pngerror.c b/pngerror.c
index 20160cf..eef4ec4 100644
--- a/pngerror.c
+++ b/pngerror.c
@@ -102,7 +102,6 @@
}
#endif /* PNG_ERROR_TEXT_SUPPORTED */
-#if defined(PNG_WARNINGS_SUPPORTED) || defined(PNG_TIME_RFC1123_SUPPORTED)
/* Utility to safely appends strings to a buffer. This never errors out so
* error checking is not required in the caller.
*/
@@ -112,8 +111,9 @@
{
if (buffer != NULL && pos < bufsize)
{
- if (string != NULL) while (*string != '\0' && pos < bufsize-1)
- buffer[pos++] = *string++;
+ if (string != NULL)
+ while (*string != '\0' && pos < bufsize-1)
+ buffer[pos++] = *string++;
buffer[pos] = '\0';
}
@@ -121,6 +121,7 @@
return pos;
}
+#if defined(PNG_WARNINGS_SUPPORTED) || defined(PNG_TIME_RFC1123_SUPPORTED)
/* Utility to dump an unsigned value into a buffer, given a start pointer and
* and end pointer (which should point just *beyond* the end of the buffer!)
* Returns the pointer to the start of the formatted string.
diff --git a/pngpriv.h b/pngpriv.h
index c18b26c..abf3a2e 100644
--- a/pngpriv.h
+++ b/pngpriv.h
@@ -276,7 +276,6 @@
#define PNG_BACKGROUND_IS_GRAY 0x800
#define PNG_HAVE_PNG_SIGNATURE 0x1000
#define PNG_HAVE_CHUNK_AFTER_IDAT 0x2000 /* Have another chunk after IDAT */
-#define PNG_ZLIB_READY_FOR_ZTXT 0x4000 /* 1: ready for ZTXT; 0: for IDAT */
/* Flags for the transformations the PNG library does on the image data */
#define PNG_BGR 0x0001
@@ -740,9 +739,6 @@
PNG_EXTERN void png_write_find_filter PNGARG((png_structp png_ptr,
png_row_infop row_info));
-/* Write out the filtered row. */
-PNG_EXTERN void png_write_filtered_row PNGARG((png_structp png_ptr,
- png_bytep filtered_row));
/* Finish a row while reading, dealing with interlacing passes, etc. */
PNG_EXTERN void png_read_finish_row PNGARG((png_structp png_ptr));
@@ -1089,10 +1085,6 @@
png_const_charp name),PNG_NORETURN);
#endif
-/* Various internal functions to handle formatted warning messages, currently
- * only implemented for warnings.
- */
-#if defined(PNG_WARNINGS_SUPPORTED) || defined(PNG_TIME_RFC1123_SUPPORTED)
/* Puts 'string' into 'buffer' at buffer[pos], taking care never to overwrite
* the end. Always leaves the buffer nul terminated. Never errors out (and
* there is no error code.)
@@ -1100,6 +1092,10 @@
PNG_EXTERN size_t png_safecat(png_charp buffer, size_t bufsize, size_t pos,
png_const_charp string);
+/* Various internal functions to handle formatted warning messages, currently
+ * only implemented for warnings.
+ */
+#if defined(PNG_WARNINGS_SUPPORTED) || defined(PNG_TIME_RFC1123_SUPPORTED)
/* Utility to dump an unsigned value into a buffer, given a start pointer and
* and end pointer (which should point just *beyond* the end of the buffer!)
* Returns the pointer to the start of the formatted string. This utility only
diff --git a/pngstruct.h b/pngstruct.h
index 7dc209c..e468312 100644
--- a/pngstruct.h
+++ b/pngstruct.h
@@ -66,11 +66,26 @@
z_stream zstream; /* pointer to decompression structure (below) */
png_bytep zbuf; /* buffer for zlib */
uInt zbuf_size; /* size of zbuf (typically 65536) */
+#ifdef PNG_WRITE_SUPPORTED
+
+/* Added in 1.5.3: state to keep track of whether the zstream has been
+ * initialized and if so whether it is for IDAT or some other chunk.
+ */
+#define PNG_ZLIB_UNINITIALIZED 0
+#define PNG_ZLIB_FOR_IDAT 1
+#define PNG_ZLIB_FOR_TEXT 2 /* anything other than IDAT */
+#define PNG_ZLIB_USE_MASK 3 /* bottom two bits */
+#define PNG_ZLIB_IN_USE 4 /* a flag value */
+
+ png_uint_32 zlib_state; /* State of zlib initialization */
+/* End of material added at libpng 1.5.3 */
+
int zlib_level; /* holds zlib compression level */
int zlib_method; /* holds zlib compression method */
int zlib_window_bits; /* holds zlib compression window bits */
int zlib_mem_level; /* holds zlib compression memory level */
int zlib_strategy; /* holds zlib compression strategy */
+#endif
/* Added at libpng 1.5.3 */
#if defined(PNG_WRITE_COMPRESSED_TEXT_SUPPORTED) || \
defined(PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION)
diff --git a/pngwrite.c b/pngwrite.c
index 1eb360b..1a9ecb7 100644
--- a/pngwrite.c
+++ b/pngwrite.c
@@ -844,8 +844,6 @@
{
/* Write the IDAT and reset the zlib output buffer */
png_write_IDAT(png_ptr, png_ptr->zbuf, png_ptr->zbuf_size);
- png_ptr->zstream.next_out = png_ptr->zbuf;
- png_ptr->zstream.avail_out = (uInt)png_ptr->zbuf_size;
wrote_IDAT = 1;
}
} while (wrote_IDAT == 1);
@@ -856,8 +854,6 @@
/* Write the IDAT and reset the zlib output buffer */
png_write_IDAT(png_ptr, png_ptr->zbuf,
png_ptr->zbuf_size - png_ptr->zstream.avail_out);
- png_ptr->zstream.next_out = png_ptr->zbuf;
- png_ptr->zstream.avail_out = (uInt)png_ptr->zbuf_size;
}
png_ptr->flush_rows = 0;
png_flush(png_ptr);
@@ -954,7 +950,8 @@
png_debug(1, "in png_write_destroy");
/* Free any memory zlib uses */
- deflateEnd(&png_ptr->zstream);
+ if (png_ptr->zlib_state != PNG_ZLIB_UNINITIALIZED)
+ deflateEnd(&png_ptr->zstream);
/* Free our memory. png_free checks NULL for us. */
png_free(png_ptr, png_ptr->zbuf);
diff --git a/pngwutil.c b/pngwutil.c
index 637f8b3..0b48215 100644
--- a/pngwutil.c
+++ b/pngwutil.c
@@ -192,6 +192,148 @@
png_write_data(png_ptr, buf, (png_size_t)4);
}
+/* Initialize the compressor for the appropriate type of compression. */
+static void
+png_zlib_claim(png_structp png_ptr, png_uint_32 state)
+{
+ if (!(png_ptr->zlib_state & PNG_ZLIB_IN_USE))
+ {
+ /* If already initialized for 'state' do not re-init. */
+ if (png_ptr->zlib_state != state)
+ {
+ int ret = Z_OK;
+ png_const_charp who = "-";
+
+ /* If actually initialized for another state do a deflateEnd. */
+ if (png_ptr->zlib_state != PNG_ZLIB_UNINITIALIZED)
+ {
+ ret = deflateEnd(&png_ptr->zstream);
+ who = "end";
+ png_ptr->zlib_state = PNG_ZLIB_UNINITIALIZED;
+ }
+
+ /* zlib itself detects an incomplete state on deflateEnd */
+ if (ret == Z_OK) switch (state)
+ {
+# ifdef PNG_WRITE_COMPRESSED_TEXT_SUPPORTED
+ case PNG_ZLIB_FOR_TEXT:
+ ret = deflateInit2(&png_ptr->zstream,
+ png_ptr->zlib_text_level, png_ptr->zlib_text_method,
+ png_ptr->zlib_text_window_bits,
+ png_ptr->zlib_text_mem_level, png_ptr->zlib_text_strategy);
+ who = "text";
+ break;
+# endif
+
+ case PNG_ZLIB_FOR_IDAT:
+ ret = deflateInit2(&png_ptr->zstream, png_ptr->zlib_level,
+ png_ptr->zlib_method, png_ptr->zlib_window_bits,
+ png_ptr->zlib_mem_level, png_ptr->zlib_strategy);
+ who = "IDAT";
+ break;
+
+ default:
+ png_error(png_ptr, "invalid zlib state");
+ }
+
+ if (ret == Z_OK)
+ png_ptr->zlib_state = state;
+
+ else /* an error in deflateEnd or deflateInit2 */
+ {
+ size_t pos = 0;
+ char msg[64];
+
+ pos = png_safecat(msg, sizeof msg, pos,
+ "zlib failed to initialize compressor (");
+ pos = png_safecat(msg, sizeof msg, pos, who);
+
+ switch (ret)
+ {
+ case Z_VERSION_ERROR:
+ pos = png_safecat(msg, sizeof msg, pos, ") version error");
+ break;
+
+ case Z_STREAM_ERROR:
+ pos = png_safecat(msg, sizeof msg, pos, ") stream error");
+ break;
+
+ case Z_MEM_ERROR:
+ pos = png_safecat(msg, sizeof msg, pos, ") memory error");
+ break;
+
+ default:
+ pos = png_safecat(msg, sizeof msg, pos, ") unknown error");
+ break;
+ }
+
+ png_error(png_ptr, msg);
+ }
+ }
+
+ /* Here on success, claim the zstream: */
+ png_ptr->zlib_state |= PNG_ZLIB_IN_USE;
+ }
+
+ else
+ png_error(png_ptr, "zstream already in use (internal error)");
+}
+
+/* The opposite: release the stream. It is also reset, this API will warn on
+ * error but will not fail.
+ */
+static void
+png_zlib_release(png_structp png_ptr)
+{
+ if (png_ptr->zlib_state & PNG_ZLIB_IN_USE)
+ {
+ int ret = deflateReset(&png_ptr->zstream);
+
+ png_ptr->zlib_state &= ~PNG_ZLIB_IN_USE;
+
+ if (ret != Z_OK)
+ {
+ png_const_charp err;
+ png_warning_parameters p;
+
+ switch (ret)
+ {
+ case Z_VERSION_ERROR:
+ err = "version";
+ break;
+
+ case Z_STREAM_ERROR:
+ err = "stream";
+ break;
+
+ case Z_MEM_ERROR:
+ err = "memory";
+ break;
+
+ default:
+ err = "unknown";
+ break;
+ }
+
+ png_warning_parameter_unsigned(p, 1, PNG_NUMBER_FORMAT_d, ret);
+ png_warning_parameter(p, 2, err);
+
+ if (png_ptr->zstream.msg)
+ err = png_ptr->zstream.msg;
+ else
+ err = "[no zlib message]";
+
+ png_warning_parameter(p, 3, err);
+
+ png_formatted_warning(png_ptr, p,
+ "zlib failed to reset compressor: @1(@2): @3");
+ }
+ }
+
+ else
+ png_warning(png_ptr, "zstream not in use (internal error)");
+}
+
#ifdef PNG_WRITE_COMPRESSED_TEXT_SUPPORTED
/* This pair of functions encapsulates the operation of (a) compressing a
* text string, and (b) issuing it later as a series of chunk data writes.
@@ -252,37 +394,7 @@
* data, or if the input string is incredibly large (although this
* wouldn't cause a failure, just a slowdown due to swapping).
*/
-
- if (!(png_ptr->mode & PNG_ZLIB_READY_FOR_ZTXT))
- {
-
- /* png_warning(png_ptr, "Initialize compressor for ztxt"); */
- /* Free memory from previously opened zstream */
- deflateEnd(&png_ptr->zstream);
-
- /* Initialize the compressor for zTXt compression. */
- ret = deflateInit2(&png_ptr->zstream, png_ptr->zlib_text_level,
- png_ptr->zlib_text_method, png_ptr->zlib_text_window_bits,
- png_ptr->zlib_text_mem_level, png_ptr->zlib_text_strategy);
-
- if (ret != Z_OK)
- {
- if (ret == Z_VERSION_ERROR)
- png_error(png_ptr,
- "zlib failed to initialize compressor for text-- version error");
-
- if (ret == Z_STREAM_ERROR)
- png_error(png_ptr,
- "zlib failed to initialize compressor for text-- stream error");
-
- if (ret == Z_MEM_ERROR)
- png_error(png_ptr,
- "zlib failed to initialize compressor for text-- mem error");
-
- png_error(png_ptr, "zlib failed to initialize compressor for text");
- }
- png_ptr->mode |= PNG_ZLIB_READY_FOR_ZTXT;
- }
+ png_zlib_claim(png_ptr, PNG_ZLIB_FOR_TEXT);
/* Set up the compression buffers */
/* TODO: the following cast hides a potential overflow problem. */
@@ -527,7 +639,7 @@
(png_size_t)(png_ptr->zbuf_size - png_ptr->zstream.avail_out));
/* Reset zlib for another zTXt/iTXt or image data */
- deflateReset(&png_ptr->zstream);
+ png_zlib_release(png_ptr);
}
#endif /* PNG_WRITE_COMPRESSED_TEXT_SUPPORTED */
@@ -541,7 +653,6 @@
int interlace_type)
{
PNG_IHDR;
- int ret;
png_byte buf[13]; /* Buffer to store the IHDR info */
@@ -747,10 +858,8 @@
#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION */
#endif /* PNG_WRITE_COMPRESSED_TEXT_SUPPORTED */
- /* Initialize the zlib compressor */
- ret = deflateInit2(&png_ptr->zstream, png_ptr->zlib_level,
- png_ptr->zlib_method, png_ptr->zlib_window_bits,
- png_ptr->zlib_mem_level, png_ptr->zlib_strategy);
+ /* Record that the compressor has not yet been initialized. */
+ png_ptr->zlib_state = PNG_ZLIB_UNINITIALIZED;
png_ptr->mode = PNG_HAVE_IHDR; /* not READY_FOR_ZTXT */
}
@@ -837,58 +946,14 @@
png_debug(1, "in png_write_IDAT");
+#ifdef PNG_WRITE_OPTIMIZE_CMF_SUPPORTED
if (!(png_ptr->mode & PNG_HAVE_IDAT) &&
png_ptr->compression_type == PNG_COMPRESSION_TYPE_BASE)
{
-#ifdef PNG_WRITE_OPTIMIZE_CMF_SUPPORTED
- unsigned int z_cmf; /* zlib compression method and flags */
-#endif /* PNG_WRITE_OPTIMIZE_CMF_SUPPORTED */
-
-#ifdef PNG_WRITE_COMPRESSED_TEXT_SUPPORTED
- int ret;
-
- /* Reinitialize the compressor if necessary */
- if (png_ptr->mode & PNG_ZLIB_READY_FOR_ZTXT)
- {
- /* Free memory from previously opened zstream */
- deflateEnd(&png_ptr->zstream);
-
- ret = deflateInit2(&png_ptr->zstream, png_ptr->zlib_level,
- png_ptr->zlib_method, png_ptr->zlib_window_bits,
- png_ptr->zlib_mem_level, png_ptr->zlib_strategy);
-
- if (ret != Z_OK)
- {
- if (ret == Z_VERSION_ERROR)
- png_error(png_ptr,
- "zlib failed to initialize compressor -- version error");
-
- if (ret == Z_STREAM_ERROR)
- png_error(png_ptr,
- "zlib failed to initialize compressor -- stream error");
-
- if (ret == Z_MEM_ERROR)
- png_error(png_ptr,
- "zlib failed to initialize compressor -- mem error");
-
- png_error(png_ptr, "zlib failed to initialize compressor");
- }
- png_ptr->mode &= ~PNG_ZLIB_READY_FOR_ZTXT; /* Ready for IDAT */
- }
-#endif /* PNG_WRITE_COMPRESSED_TEXT_SUPPORTED */
-
- png_ptr->zstream.next_out = png_ptr->zbuf;
- png_ptr->zstream.avail_out = (uInt)png_ptr->zbuf_size;
- /* libpng is not interested in zstream.data_type, so set it
- * to a predefined value, to avoid its evaluation inside zlib
- */
- png_ptr->zstream.data_type = Z_BINARY;
-
-#ifdef PNG_WRITE_OPTIMIZE_CMF_SUPPORTED
/* Optimize the CMF field in the zlib stream. This hack of the zlib
* stream is compliant to the stream specification.
*/
- z_cmf = data[0];
+ unsigned int z_cmf = data[0]; /* zlib compression method and flags */
if ((z_cmf & 0x0f) == 8 && (z_cmf & 0xf0) <= 0x70)
{
@@ -946,11 +1011,18 @@
else
png_error(png_ptr,
"Invalid zlib compression method or flags in IDAT");
-#endif /* PNG_WRITE_OPTIMIZE_CMF_SUPPORTED */
}
+#endif /* PNG_WRITE_OPTIMIZE_CMF_SUPPORTED */
png_write_chunk(png_ptr, png_IDAT, data, length);
png_ptr->mode |= PNG_HAVE_IDAT;
+
+ /* Prior to 1.5.3 this code was replicated in every caller (except at the
+ * end, where it isn't technically necessary). Since this function has
+ * flushed the data we can safely reset the zlib output buffer here.
+ */
+ png_ptr->zstream.next_out = png_ptr->zbuf;
+ png_ptr->zstream.avail_out = (uInt)png_ptr->zbuf_size;
}
/* Write an IEND chunk */
@@ -2029,6 +2101,7 @@
png_ptr->usr_width = png_ptr->width;
}
+ png_zlib_claim(png_ptr, PNG_ZLIB_FOR_IDAT);
png_ptr->zstream.avail_out = (uInt)png_ptr->zbuf_size;
png_ptr->zstream.next_out = png_ptr->zbuf;
}
@@ -2150,7 +2223,7 @@
png_ptr->zstream.avail_out);
}
- deflateReset(&png_ptr->zstream);
+ png_zlib_release(png_ptr);
png_ptr->zstream.data_type = Z_BINARY;
}
@@ -2339,6 +2412,8 @@
* been specified by the application, and then writes the row out with the
* chosen filter.
*/
+static void png_write_filtered_row(png_structp png_ptr, png_bytep filtered_row);
+
#define PNG_MAXSUM (((png_uint_32)(-1)) >> 1)
#define PNG_HISHIFT 10
#define PNG_LOMASK ((png_uint_32)0xffffL)
@@ -3014,7 +3089,7 @@
/* Do the actual writing of a previously filtered row. */
-void /* PRIVATE */
+static void
png_write_filtered_row(png_structp png_ptr, png_bytep filtered_row)
{
png_size_t avail;
@@ -3074,8 +3149,6 @@
{
/* Write the IDAT and reset the zlib output buffer */
png_write_IDAT(png_ptr, png_ptr->zbuf, png_ptr->zbuf_size);
- png_ptr->zstream.next_out = png_ptr->zbuf;
- png_ptr->zstream.avail_out = (uInt)png_ptr->zbuf_size;
}
/* Repeat until all data has been compressed */
} while (avail > 0 || png_ptr->zstream.avail_in > 0);
diff --git a/scripts/pnglibconf.dfa b/scripts/pnglibconf.dfa
index 2366f8b..8f44962 100644
--- a/scripts/pnglibconf.dfa
+++ b/scripts/pnglibconf.dfa
@@ -553,6 +553,7 @@
# added at libpng-1.5.3
option WRITE_OPTIMIZE_CMF requires WRITE
+option READ_COMPRESSED_TEXT disabled
option READ_iCCP enables READ_COMPRESSED_TEXT
option READ_iTXt enables READ_COMPRESSED_TEXT
option READ_zTXt enables READ_COMPRESSED_TEXT
@@ -561,6 +562,7 @@
option WRITE_oFFs enables SAVE_INT_32
option WRITE_pCAL enables SAVE_INT_32
+option WRITE_COMPRESSED_TEXT disabled
option WRITE_iCCP enables WRITE_COMPRESSED_TEXT
option WRITE_iTXt enables WRITE_COMPRESSED_TEXT
option WRITE_zTXt enables WRITE_COMPRESSED_TEXT
