diff options
| -rw-r--r-- | ChangeLog | 19 | ||||
| -rw-r--r-- | generic/tkImgPNG.c | 93 | ||||
| -rw-r--r-- | tests/imgPNG.test | 20 |
3 files changed, 119 insertions, 13 deletions
@@ -1,3 +1,14 @@ +2010-04-12 Donal K. Fellows <dkf@users.sf.net> + + * 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. + 2010-04-09 Jan Nijtmans <nijtmans@users.sf.net> * doc/photo.n: Follow-up to [Bug 2983824]: update doc. @@ -15,10 +26,10 @@ 2010-04-06 Jan Nijtmans <nijtmans@users.sf.net> - * win/tcl.m4 Sync with Tcl version - * unix/tcl.m4 - * win/configure (regenerate with autoconf-2.59) - * unix/configure [Bug 2982540]: configure and install* script + * win/tcl.m4: Sync with Tcl version + * unix/tcl.m4: + * win/configure: (regenerate with autoconf-2.59) + * unix/configure: [Bug 2982540]: configure and install* script files should always have LF 2010-03-29 Jan Nijtmans <nijtmans@users.sf.net> 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. */ diff --git a/tests/imgPNG.test b/tests/imgPNG.test index 7fb2622..1cfaad0 100644 --- a/tests/imgPNG.test +++ b/tests/imgPNG.test @@ -8,7 +8,7 @@ # Copyright (c) 2008 Donal K. Fellows # All rights reserved. # -# RCS: @(#) $Id: imgPNG.test,v 1.2 2009/01/13 01:46:06 patthoyts Exp $ +# RCS: @(#) $Id: imgPNG.test,v 1.3 2010/04/12 08:40:17 dkf Exp $ package require tcltest 2.2 namespace import ::tcltest::* @@ -19,16 +19,20 @@ imageInit namespace eval png { variable encoded # Key names are from the names of the source images, which come from - # http://www.schaik.com/pngsuite/pngsuite.html + # http://www.schaik.com/pngsuite/pngsuite.html + # The exception is "BadX", which is used to test handling badly compressed + # images. array set encoded { basn0g08 "iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAAAAABWESUoAAAABGdBTUEAAYagMeiWXwAAAEFJREFUeJxjZGAkABQIyLMMBQWMDwgp+PcfP2B5MBwUMMoRkGdkonlcDAYFjI/wyv7/z/iH5nExGBQwyuCVZWQEAFDl/nE14thZAAAAAElFTkSuQmCC" basn2c08 "iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAIAAAD8GO2jAAAABGdBTUEAAYagMeiWXwAAAEhJREFUeJzt1cEJADAMAkCF7JH9t3ITO0Qr9KH4zuErtA0EO4AKFPgcoO3kfUx4QIECD0qHH8KEBxQo8KB0OCOpQIG7cHejwAGCsfleD0DPSwAAAABJRU5ErkJggg==" basn3p08 "iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAMAAABEpIrGAAAABGdBTUEAAYagMeiWXwAAAwBQTFRFIkQA9f/td/93y///EQoAOncAIiL//xH/EQAAIiIA/6xVZv9m/2Zm/wH/IhIA3P//zP+ZRET/AFVVIgAAy8v/REQAVf9Vy8sAMxoA/+zc7f//5P/L/9zcRP9EZmb/MwAARCIA7e3/ZmYA/6RE//+q7e0AAMvL/v///f/+//8BM/8zVSoAAQH/iIj/AKqqAQEARAAAiIgA/+TLulsAIv8iZjIA//+Zqqr/VQAAqqoAy2MAEf8R1P+qdzoA/0RE3GsAZgAAAf8BiEIA7P/ca9wA/9y6ADMzAO0A7XMA//+ImUoAEf//dwAA/4MB/7q6/nsA//7/AMsA/5mZIv//iAAA//93AIiI/9z/GjMAAACqM///AJkAmQAAAAABMmYA/7r/RP///6r/AHcAAP7+qgAASpkA//9m/yIiAACZi/8RVf///wEB/4j/AFUAABER///+//3+pP9EZv///2b/ADMA//9V/3d3AACI/0T/ABEAd///AGZm///tAAEA//XtERH///9E/yL//+3tEREAiP//AAB3k/8iANzcMzP//gD+urr/mf//MzMAY8sAuroArP9V///c//8ze/4A7QDtVVX/qv//3Nz/VVUAAABm3NwA3ADcg/8Bd3f//v7////L/1VVd3cA/v4AywDLAAD+AQIAAQAAEiIA//8iAEREm/8z/9SqAABVmZn/mZkAugC6KlUA/8vLtP9m/5sz//+6qgCqQogAU6oA/6qqAADtALq6//8RAP4AAABEAJmZmQCZ/8yZugAAiACIANwA/5MiAADc/v/+qlMAdwB3AgEAywAAAAAz/+3/ALoA/zMz7f/t/8SIvP93AKoAZgBmACIi3AAA/8v/3P/c/4sRAADLAAEBVQBVAIgAAAAiAf//y//L7QAA/4iIRABEW7oA/7x3/5n/AGYAuv+6AHd3c+0A/gAAMwAzAAC6/3f/AEQAqv+q//7+AAARIgAixP+IAO3tmf+Z/1X/ACIA/7RmEQARChEA/xER3P+6uv//iP+IAQAB/zP/uY7TYgAAAbFJREFUeJwNwQcACAQQAMBHqIxIZCs7Mwlla1hlZ+8VitCw9yoqNGiYDatsyt6jjIadlVkysve+u5jC9xTmV/qyl6bcJR7kAQZzg568xXmuE2lIyUNM5So7OMAFIhvp+YgGvEtFNnOKeJonSEvwP9NZzhHiOfLzBXPoxKP8yD6iPMXITjP+oTdfsp14lTJMJjGtOMFQfiFe4wWK8BP7qUd31hBNqMos2tKYFbRnJdGGjTzPz2yjEA1ZSKymKCM5ylaWcJrZxCZK8jgfU4vc/MW3xE7K8RUvsZb3Wc/XxCEqk4v/qMQlFvMZcZIafMOnLKM13zGceJNqPMU4KnCQAqQgbrKHpXSgFK/Qn6REO9YxjWE8Sx2SMJD4jfl8wgzy0YgPuEeUJQcD6EoWWpCaHsQkHuY9RpGON/icK0RyrvE680jG22TlHaIbx6jLnySkF+M5QxzmD6pwkTsMoSAdidqsojipuMyHzOQ4sYgfyElpzjKGErQkqvMyC7jFv9xmBM2JuTzDRDLxN4l4jF1EZjIwmhfZzSOMpT4xiH70IQG/k5En2UKcowudycsG8jCBmtwHgRv+EIeWyOAAAAAASUVORK5CYII=" basn6a08 "iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAABGdBTUEAAYagMeiWXwAAAG9JREFUeJzt1jEKgDAMRuEnZGhPofc/VQSPIcTdxUV4HVLoUCj8H00o2YoBMF57fpz/ujODHXUFRwPKBqj5DVigB041HiJ9gFyCVOMbsEIPXNwuAHkgiJL/4qABNqB7QAeUPBAE2QAZUDZAfwEb8ABSIBqcFg+4TAAAAABJRU5ErkJggg==" + + BadX "iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAABHNCSVQICAgIfAhkiAAAABN0RVh0U29mdHdhcmUAVGsgOC42YjEuMcrtT1oAAAAcSURBVHicYmBgYPjPgAr+ozP+o0uj68BUiWEmAAAA//8SozfjAAAAAElFTkSuQmCC" } # $encoded(basn0g08), $encoded(basn2c08), $encoded(basn3p08), $encoded(basn6a08) -test png-1.1 {reading basic images; grayscale} -setup { +test imgPNG-1.1 {reading basic images; grayscale} -setup { catch {rename foo ""} } -body { image create photo foo -data $encoded(basn0g08) @@ -36,7 +40,7 @@ test png-1.1 {reading basic images; grayscale} -setup { } -cleanup { rename foo "" } -result {32 32} -test png-1.2 {reading basic images; color} -setup { +test imgPNG-1.2 {reading basic images; color} -setup { catch {rename foo ""} } -body { image create photo foo -data $encoded(basn2c08) @@ -44,7 +48,7 @@ test png-1.2 {reading basic images; color} -setup { } -cleanup { rename foo "" } -result {32 32} -test png-1.3 {reading basic images; color with palette} -setup { +test imgPNG-1.3 {reading basic images; color with palette} -setup { catch {rename foo ""} } -body { image create photo foo -data $encoded(basn3p08) @@ -52,7 +56,7 @@ test png-1.3 {reading basic images; color with palette} -setup { } -cleanup { rename foo "" } -result {32 32} -test png-1.4 {reading basic images; alpha} -setup { +test imgPNG-1.4 {reading basic images; alpha} -setup { catch {rename foo ""} } -body { image create photo foo -data $encoded(basn6a08) @@ -60,6 +64,10 @@ test png-1.4 {reading basic images; alpha} -setup { } -cleanup { rename foo "" } -result {32 32} + +test imgPNG-2.1 {reading a bad image} -body { + image create photo -data $encoded(BadX) +} -returnCodes error -result {unfinalized data stream in PNG data} } namespace delete png |