* generic/tkImgPNG.c (WriteIDAT): [Bug 2984787]: Use the correct

	flushing semantics when handling the last data from the image. Without
	this, many PNG readers (notably including Firefox) refuse to show the
	image and instead complain about errors.
	(ReadIDAT): Added sanity checks to ensure that when we've got bad data
	of the sorts of forms we were previously generating, we detect it and
	error out rather than silently failing.
	(WriteExtraChunks): New function to write in some basic metadata.

1 files changed, 90 insertions, 3 deletions

diff --git a/generic/tkImgPNG.c b/generic/tkImgPNG.c
index 0468ca9..c138b78 100644
--- a/generic/tkImgPNG.c
+++ b/generic/tkImgPNG.c
@@ -9,7 +9,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkImgPNG.c,v 1.5 2010/02/05 22:45:03 nijtmans Exp $
+ * RCS: @(#) $Id: tkImgPNG.c,v 1.6 2010/04/12 08:40:15 dkf Exp $
  */
 
 #include "tkInt.h"
@@ -256,6 +256,8 @@ static inline int	WriteChunk(Tcl_Interp *interp, PNGImage *pngPtr,
 static int		WriteData(Tcl_Interp *interp, PNGImage *pngPtr,
 			    const unsigned char *srcPtr, int srcSz,
 			    unsigned long *crcPtr);
+static int		WriteExtraChunks(Tcl_Interp *interp,
+			    PNGImage *pngPtr);
 static int		WriteIHDR(Tcl_Interp *interp, PNGImage *pngPtr,
 			    Tk_PhotoImageBlock *blockPtr);
 static int		WriteIDAT(Tcl_Interp *interp, PNGImage *pngPtr,
@@ -2127,6 +2129,22 @@ ReadIDAT(
 	 */
     }
 
+    /*
+     * Ensure that when we've got to the end of the compressed data, we've
+     * also got to the end of the compressed stream. This sanity check is
+     * enforced by most PNG readers.
+     */
+
+    if (!Tcl_ZlibStreamEof(pngPtr->stream)) {
+	Tcl_AppendResult(interp, "unfinalized data stream in PNG data", NULL);
+	return TCL_ERROR;
+    }
+    if (chunkSz != 0) {
+	Tcl_AppendResult(interp,
+		"compressed data after stream finalize in PNG data", NULL);
+	return TCL_ERROR;
+    }
+
     if (CheckCRC(interp, pngPtr, crc) == TCL_ERROR) {
 	return TCL_ERROR;
     }
@@ -3099,11 +3117,13 @@ WriteIDAT(
 
 	/*
 	 * Compress the line of pixels into the destination. If this is the
-	 * last line, flush at the same time.
+	 * last line, finalize the compressor at the same time. Note that this
+	 * can't be just a flush; that leads to a file that some PNG readers
+	 * choke on. [Bug 2984787]
 	 */
 
 	if (rowNum + 1 == blockPtr->height) {
-	    flush = TCL_ZLIB_FLUSH;
+	    flush = TCL_ZLIB_FINALIZE;
 	}
 	if (Tcl_ZlibStreamPut(pngPtr->stream, pngPtr->thisLineObj,
 		flush) != TCL_OK) {
@@ -3138,6 +3158,64 @@ WriteIDAT(
 /*
  *----------------------------------------------------------------------
  *
+ * WriteExtraChunks --
+ *
+ *	Writes an sBIT and a tEXt chunks to the PNG image, describing a bunch
+ *	of not very important metadata that many readers seem to need anyway.
+ *
+ * Results:
+ *	TCL_OK, or TCL_ERROR if the write fails.
+ *
+ * Side effects:
+ *	None
+ *
+ *----------------------------------------------------------------------
+ */
+
+static int
+WriteExtraChunks(
+    Tcl_Interp *interp,
+    PNGImage *pngPtr)
+{
+    static const unsigned char sBIT_contents[] = {
+	8, 8, 8, 8
+    };
+    Tcl_DString buf;
+
+    /*
+     * Each byte of each channel is always significant; we always write RGBA
+     * images with 8 bits per channel as that is what the photo image's basic
+     * data model is.
+     */
+
+    if (WriteChunk(interp, pngPtr, CHUNK_sBIT, sBIT_contents, 4) != TCL_OK) {
+	return TCL_ERROR;
+    }
+
+    /*
+     * Say that it is Tk that made the PNG. Note that we *need* the NUL at the
+     * end of "Software" to be transferred; do *not* change the length
+     * parameter to -1 there!
+     */
+
+    Tcl_DStringInit(&buf);
+    Tcl_DStringAppend(&buf, "Software", 9);
+    Tcl_DStringAppend(&buf, "Tk Toolkit v", -1);
+    Tcl_DStringAppend(&buf, TK_PATCH_LEVEL, -1);
+    if (WriteChunk(interp, pngPtr, CHUNK_tEXt,
+	    (unsigned char *) Tcl_DStringValue(&buf),
+	    Tcl_DStringLength(&buf)) != TCL_OK) {
+	Tcl_DStringFree(&buf);
+	return TCL_ERROR;
+    }
+    Tcl_DStringFree(&buf);
+
+    return TCL_OK;
+}
+
+/*
+ *----------------------------------------------------------------------
+ *
  * EncodePNG --
  *
  *	This function handles the entirety of writing a PNG file (or data)
@@ -3235,6 +3313,15 @@ EncodePNG(
     }
 
     /*
+     * Write out the extra chunks containing metadata that is of interest to
+     * other programs more than us.
+     */
+
+    if (WriteExtraChunks(interp, pngPtr) == TCL_ERROR) {
+	return TCL_ERROR;
+    }
+
+    /*
      * Write out the image pixels in the IDAT (data) chunk.
      */