* generic/tclZlib.c (ZlibTransformInput): [Bug 3525907]: Ensure that

decompressed input is flushed through the transform correctly when the
input stream gets to the end. Thanks to Alexandre Ferrieux and Andreas
Kupries for their work on this.

4 files changed, 294 insertions, 63 deletions

diff --git a/ChangeLog b/ChangeLog
index e848416..2166fb8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2012-05-23  Donal K. Fellows  <dkf@users.sf.net>
+
+	* generic/tclZlib.c (ZlibTransformInput): [Bug 3525907]: Ensure that
+	decompressed input is flushed through the transform correctly when the
+	input stream gets to the end. Thanks to Alexandre Ferrieux and Andreas
+	Kupries for their work on this.
+
 2012-05-21  Don Porter  <dgp@users.sourceforge.net>
 
 	* generic/tclFileName.c:	When using Tcl_SetObjLength() calls to grow
diff --git a/generic/tclIORTrans.c b/generic/tclIORTrans.c
index 6c9a41b..fd25f2d 100644
--- a/generic/tclIORTrans.c
+++ b/generic/tclIORTrans.c
@@ -439,13 +439,6 @@ static const char *msg_dstlost =
  */
 
 /*
- * Number of milliseconds to wait before firing an event to try to flush out
- * information waiting in buffers (fileevent support).
- */
-
-#define FLUSH_DELAY	(5)
-
-/*
  * Helper functions encapsulating some of the thread forwarding to make the
  * control flow in callers easier.
  */
@@ -1230,7 +1223,7 @@ ReflectInput(
  *
  * ReflectOutput --
  *
- *	This function is invoked when data is writen to the channel.
+ *	This function is invoked when data is written to the channel.
  *
  * Results:
  *	The number of bytes actually written.
@@ -2861,7 +2854,7 @@ TimerSetup(
 	return;
     }
 
-    rtPtr->timer = Tcl_CreateTimerHandler(FLUSH_DELAY, TimerRun, rtPtr);
+    rtPtr->timer = Tcl_CreateTimerHandler(0, TimerRun, rtPtr);
 }
 
 /*
diff --git a/generic/tclZlib.c b/generic/tclZlib.c
index 3673833..0c38602 100644
--- a/generic/tclZlib.c
+++ b/generic/tclZlib.c
@@ -17,6 +17,7 @@
 #include "tclInt.h"
 #ifdef HAVE_ZLIB
 #include <zlib.h>
+#include "tclIO.h"
 
 /*
  * Magic flags used with wbits fields to indicate that we're handling the gzip
@@ -90,6 +91,7 @@ typedef struct {
     GzipHeader outHeader;	/* Header to write to an output stream, when
 				 * compressing a gzip stream. */
     Tcl_TimerToken timer;	/* Timer used for keeping events fresh. */
+    Tcl_DString result;		/* Buffer for decompression results. */
 } ZlibChannelData;
 
 /*
@@ -112,11 +114,10 @@ typedef struct {
 #define DEFAULT_BUFFER_SIZE	4096
 
 /*
- * Time to wait (in milliseconds) before flushing the channel when reading
- * data through the transform.
+ * Convenience macro to make some casts easier to use.
  */
 
-#define TRANSFORM_FLUSH_DELAY	5
+#define UCHARP(x)	((unsigned char *) (x))
 
 /*
  * Prototypes for private procedures defined later in this file:
@@ -139,6 +140,10 @@ static void		ConvertError(Tcl_Interp *interp, int code);
 static void		ExtractHeader(gz_header *headerPtr, Tcl_Obj *dictObj);
 static int		GenerateHeader(Tcl_Interp *interp, Tcl_Obj *dictObj,
 			    GzipHeader *headerPtr, int *extraSizePtr);
+static int		ResultCopy(Tcl_DString *r, unsigned char *buf,
+			    int toRead);
+static int		ResultGenerate(ZlibChannelData *cd, int n, int flush,
+			    int *errorCodePtr);
 static Tcl_Channel	ZlibStackChannelTransform(Tcl_Interp *interp,
 			    int mode, int format, int level,
 			    Tcl_Channel channel, Tcl_Obj *gzipHeaderDictPtr);
@@ -2320,6 +2325,8 @@ ZlibTransformClose(
      * Release all memory.
      */
 
+    Tcl_DStringFree (&cd->result);
+
     if (cd->inBuffer) {
 	ckfree(cd->inBuffer);
 	cd->inBuffer = NULL;
@@ -2342,77 +2349,130 @@ ZlibTransformInput(
     ZlibChannelData *cd = instanceData;
     Tcl_DriverInputProc *inProc =
 	    Tcl_ChannelInputProc(Tcl_GetChannelType(cd->parent));
-    int e, readBytes, flush = Z_NO_FLUSH;
+    int readBytes, gotBytes, copied;
 
     if (cd->mode == TCL_ZLIB_STREAM_DEFLATE) {
 	return inProc(Tcl_GetChannelInstanceData(cd->parent), buf, toRead,
 		errorCodePtr);
     }
 
-    cd->inStream.next_out = (Bytef *) buf;
-    cd->inStream.avail_out = toRead;
-    if (cd->inStream.next_in == NULL) {
-	goto doReadFirst;
-    }
-    while (1) {
-	e = inflate(&cd->inStream, flush);
-	if ((e == Z_STREAM_END) || (e==Z_OK && cd->inStream.avail_out==0)) {
-	    return toRead - cd->inStream.avail_out;
-	}
-
+    gotBytes = 0;
+    while (toRead > 0) {
 	/*
-	 * Z_BUF_ERROR can be ignored as per http://www.zlib.net/zlib_how.html
-	 *
-	 * Just indicates that the zlib couldn't consume input/produce output,
-	 * and is fixed by supplying more input.
+	 * Loop until the request is satisfied (or no data available from
+	 * below, possibly EOF).
 	 */
 
-	if ((e != Z_OK) && (e != Z_BUF_ERROR)) {
-	    Tcl_Obj *errObj = Tcl_NewListObj(0, NULL);
+	copied = ResultCopy(&cd->result, UCHARP(buf), toRead);
+	toRead -= copied;
+	buf += copied;
+	gotBytes += copied;
 
-	    Tcl_ListObjAppendElement(NULL, errObj,
-		    Tcl_NewStringObj(cd->inStream.msg, -1));
-	    Tcl_SetChannelError(cd->parent, errObj);
-	    *errorCodePtr = EINVAL;
-	    return -1;
+	if (toRead == 0) {
+	    goto stop;
 	}
 
 	/*
-	 * Check if the inflate stopped early.
+	 * The buffer is exhausted, but the caller wants even more. We now
+	 * have to go to the underlying channel, get more bytes and then
+	 * transform them for delivery. We may not get what we want (full EOF
+	 * or temporarily out of data).
+	 *
+	 * Length (cd->result) == 0, toRead > 0 here.
+	 *
+	 * The zlib transform allows us to read at most one character from the
+	 * underlying channel to properly identify Z_STREAM_END without
+	 * reading over the border.
 	 */
 
-	if (cd->inStream.avail_in > 0) {
-	    continue;
+	readBytes = Tcl_ReadRaw(cd->parent, cd->inBuffer, 1);
+
+	if (readBytes < 0) {
+	    /*
+	     * Report errors to caller. The state of the seek system is
+	     * unchanged!
+	     */
+
+	    if ((Tcl_GetErrno() == EAGAIN) && (gotBytes > 0)) {
+		/*
+		 * EAGAIN is a special situation. If we had some data before
+		 * we report that instead of the request to re-try.
+		 */
+
+		goto stop;
+	    }
+
+	    *errorCodePtr = Tcl_GetErrno();
+	    goto error;
 	}
 
-	/*
-	 * Emptied the buffer of data from the underlying channel. Get some
-	 * more.
-	 */
+	if (readBytes == 0) {
+	    /*
+	     * Check wether we hit on EOF in 'parent' or not. If not
+	     * differentiate between blocking and non-blocking modes. In
+	     * non-blocking mode we ran temporarily out of data. Signal this
+	     * to the caller via EWOULDBLOCK and error return (-1). In the
+	     * other cases we simply return what we got and let the caller
+	     * wait for more. On the other hand, if we got an EOF we have to
+	     * convert and flush all waiting partial data.
+	     */
+
+	    if (!Tcl_Eof(cd->parent)) {
+		/*
+		 * The state of the seek system is unchanged!
+		 */
+
+		if ((gotBytes == 0) && (cd->flags & ASYNC)) {
+		    *errorCodePtr = EWOULDBLOCK;
+		    goto error;
+		}
+		goto stop;
+	    } else {
+		/*
+		 * (Semi-)Eof in parent.
+		 *
+		 * Now this is a bit different. The partial data waiting is
+		 * converted and returned.
+		 */
+
+		if (ResultGenerate(cd, 0, Z_SYNC_FLUSH, errorCodePtr) < 0) {
+		    goto error;
+		}
+
+		if (Tcl_DStringLength(&cd->result) == 0) {
+		    /*
+		     * The drain delivered nothing.
+		     */
+
+		    goto stop;
+		}
+
+		/*
+		 * Reset eof, force caller to drain result buffer.
+		 */
+
+		((Channel *) cd->parent)->state->flags &= ~CHANNEL_EOF;
+		continue; /* at: while (toRead > 0) */
+	    }
+	} /* readBytes == 0 */
 
-    doReadFirst:
 	/*
-	 * Hack for Bug 2762041. Disable pre-reading of lots of input, read
-	 * only one character. This way the Z_END_OF_STREAM can be read
-	 * without triggering an EOF in the base channel. The higher input
-	 * loops in DoReadChars() would react to that by stopping, despite the
-	 * transform still having data which could be read.
-	 *
-	 * This is only a hack because other transforms may not be able to
-	 * work around the general problem in this way.
+	 * Transform the read chunk, which was not empty. Anything we get back
+	 * is a transformation result to be put into our buffers, and the next
+	 * iteration will put it into the result.
 	 */
 
-	readBytes = Tcl_ReadRaw(cd->parent, cd->inBuffer, 1);
-	if (readBytes < 0) {
-	    *errorCodePtr = Tcl_GetErrno();
-	    return -1;
-	} else if (readBytes == 0) {
-	    flush = Z_SYNC_FLUSH;
+	if (ResultGenerate(cd, readBytes, Z_NO_FLUSH, errorCodePtr) < 0) {
+	    goto error;
 	}
+    } /* while toRead > 0 */
 
-	cd->inStream.next_in = (Bytef *) cd->inBuffer;
-	cd->inStream.avail_in = readBytes;
-    }
+ stop:
+    return gotBytes;
+
+ error:
+    gotBytes = -1;
+    goto stop;
 }
 
 static int
@@ -2521,6 +2581,11 @@ ZlibTransformSetOption(			/* not used */
 	return Tcl_BadChannelOption(interp, optionName, chanOptions);
     }
 
+    /*
+     * Pass all unknown options down, to deeper transforms and/or the base
+     * channel.
+     */
+
     return setOptionProc(Tcl_GetChannelInstanceData(cd->parent), interp,
 	    optionName, value);
 }
@@ -2615,8 +2680,9 @@ ZlibTransformWatch(
 
     watchProc = Tcl_ChannelWatchProc(Tcl_GetChannelType(cd->parent));
     watchProc(Tcl_GetChannelInstanceData(cd->parent), mask);
-    if (!(mask & TCL_READABLE)
-	    || (cd->inStream.avail_in == (uInt) cd->inAllocated)) {
+
+    if (!(mask & TCL_READABLE) ||
+	(Tcl_DStringLength(&cd->result) == 0)) {
 	ZlibTransformTimerKill(cd);
     } else {
 	ZlibTransformTimerSetup(cd);
@@ -2665,7 +2731,7 @@ ZlibTransformTimerSetup(
     ZlibChannelData *cd)
 {
     if (cd->timer == NULL) {
-	cd->timer = Tcl_CreateTimerHandler(TRANSFORM_FLUSH_DELAY,
+	cd->timer = Tcl_CreateTimerHandler(0,
 		ZlibTransformTimerRun, cd);
     }
 }
@@ -2804,6 +2870,8 @@ ZlibStackChannelTransform(
 	}
     }
 
+    Tcl_DStringInit(&cd->result);
+
     chan = Tcl_StackChannel(interp, &zlibChannelType, cd,
 	    Tcl_GetChannelMode(channel), channel);
     if (chan == NULL) {
@@ -2829,6 +2897,151 @@ ZlibStackChannelTransform(
 
 /*
  *----------------------------------------------------------------------
+ *
+ * ResultCopy --
+ *
+ *	Copies the requested number of bytes from the buffer into the
+ *	specified array and removes them from the buffer afterward. Copies
+ *	less if there is not enough data in the buffer.
+ *
+ * Side effects:
+ *	See above.
+ *
+ * Result:
+ *	The number of actually copied bytes, possibly less than 'toRead'.
+ *
+ *----------------------------------------------------------------------
+ */
+
+static int
+ResultCopy(
+    Tcl_DString *ds,		/* The buffer to read from */
+    unsigned char *buf,		/* The buffer to copy into */
+    int toRead)			/* Number of requested bytes */
+{
+    int have = Tcl_DStringLength(ds);
+
+    if (have == 0) {
+	/*
+	 * Nothing to copy in the case of an empty buffer.
+	 */
+
+	return 0;
+    }
+    if (have > toRead) {
+	/*
+	 * The internal buffer contains more than requested. Copy the
+	 * requested subset to the caller, shift the remaining bytes down, and
+	 * truncate.
+	 */
+
+	char *src = Tcl_DStringValue(ds);
+
+	memcpy(buf, src, toRead);
+	memmove(src, src + toRead, have - toRead);
+
+	Tcl_DStringSetLength(ds, have - toRead);
+	return toRead;
+    } else /* have <= toRead */ {
+	/*
+	 * There is just or not enough in the buffer to fully satisfy the
+	 * caller, so take everything as best effort.
+	 */
+
+	memcpy(buf, Tcl_DStringValue(ds), have);
+	Tcl_DStringSetLength(ds, 0);
+	return have;
+    }
+}
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * ResultGenerate --
+ *
+ *	Extract uncompressed bytes from the compression engine and store them
+ *	in our working buffer.
+ *
+ * Result:
+ *	Zero on success, -1 on error (with *errorCodePtr updated with reason).
+ *
+ * Side effects:
+ *	See above.
+ *
+ *----------------------------------------------------------------------
+ */
+
+static int
+ResultGenerate(
+    ZlibChannelData *cd,
+    int n,
+    int flush,
+    int *errorCodePtr)
+{
+#define MAXBUF	1024
+    unsigned char buf[MAXBUF];
+    int e, written;
+
+    cd->inStream.next_in = (Bytef *) cd->inBuffer;
+    cd->inStream.avail_in = n;
+
+    while (1) {
+	cd->inStream.next_out = (Bytef *) buf;
+	cd->inStream.avail_out = MAXBUF;
+
+	e = inflate(&cd->inStream, flush);
+
+	/*
+	 * avail_out is now the left over space in the output.  Therefore
+	 * "MAXBUF - avail_out" is the amount of bytes generated.
+	 */
+
+	written = MAXBUF - cd->inStream.avail_out;
+	if (written) {
+	    Tcl_DStringAppend(&cd->result, (char*) buf, written);
+	}
+
+	/*
+	 * The cases where we're definitely done.
+	 */
+
+	if (((flush == Z_SYNC_FLUSH) && (e == Z_BUF_ERROR))
+		|| (e == Z_STREAM_END)
+		|| (e == Z_OK && cd->inStream.avail_out == 0)) {
+	    return 0;
+	}
+
+	/*
+	 * Z_BUF_ERROR can be ignored as per http://www.zlib.net/zlib_how.html
+	 *
+	 * Just indicates that the zlib couldn't consume input/produce output,
+	 * and is fixed by supplying more input.
+	 *
+	 * Otherwise, we've got errors and need to report to higher-up.
+	 */
+
+	if ((e != Z_OK) && (e != Z_BUF_ERROR)) {
+	    Tcl_Obj *errObj = Tcl_NewListObj(0, NULL);
+
+	    Tcl_ListObjAppendElement(NULL, errObj,
+		    Tcl_NewStringObj(cd->inStream.msg, -1));
+	    Tcl_SetChannelError(cd->parent, errObj);
+	    *errorCodePtr = EINVAL;
+	    return -1;
+	}
+
+	/*
+	 * Check if the inflate stopped early.
+	 */
+
+	if (cd->inStream.avail_in <= 0 && flush != Z_SYNC_FLUSH) {
+	    return 0;
+	}
+    }
+}
+
+/*
+ *----------------------------------------------------------------------
  *	Finally, the TclZlibInit function. Used to install the zlib API.
  *----------------------------------------------------------------------
  */
diff --git a/tests/zlib.test b/tests/zlib.test
index d8d710a..4ed5b33 100644
--- a/tests/zlib.test
+++ b/tests/zlib.test
@@ -169,6 +169,24 @@ test zlib-8.4 {transformation and flushing: Bug 3517696} -setup {
     catch {close $fd}
     removeFile $file
 } -result {}
+test zlib-8.5 {transformation and flushing and fileevents: Bug 3525907} -setup {
+    foreach {r w} [chan pipe] break
+} -constraints zlib -body {
+    set ::res {}
+    fconfigure $w -buffering none
+    zlib push compress $w
+    puts -nonewline $w qwertyuiop
+    chan configure $w -flush sync
+    after 500 {puts -nonewline $w asdfghjkl;close $w}
+    fconfigure $r -blocking 0 -buffering none
+    zlib push decompress $r
+    fileevent $r readable {set msg [read $r];lappend ::res $msg;if {[eof $r]} {set ::done 1}}
+    after 250 {lappend ::res MIDDLE}
+    vwait ::done
+    set ::res
+} -cleanup {
+    catch {close $r}
+} -result {qwertyuiop MIDDLE asdfghjkl}
 
 test zlib-9.1 "check fcopy with push" -constraints zlib -setup {
     set sfile [makeFile {} testsrc.gz]