[Python-bugs-list] [ python-Bugs-640230 ] Discovered typo in zlib test.

[SourceForge.net]

Bugs item #640230, was opened at 2002-11-18 13:36
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=640230&group_id=5470

Category: None
Group: None
Status: Open
Resolution: None
>Priority: 7
Submitted By: Scott David Daniels (scott_daniels)
Assigned to: Nobody/Anonymous (nobody)
Summary: Discovered typo in zlib test.

Initial Comment:
In test_zlib.py (while chasing what appears to be a
documentation problem), I found a flaw in the code
that tests max_length limitted output from a 
decompression object test.

Starting at line 100, the existing code is:
...
bufs.append(deco.flush())
decomp2 = ''.join(buf)
if decomp2 != buf:
    print "max_length decompressobj failed"
else:
    print "max_length decompressobj succeeded"
...

This test will always succeed (''.join(str) == str).

What seems obviously meant is:
...
bufs.append(deco.flush())
decomp2 = ''.join(bufs)
if decomp2 != buf:
    print "max_length decompressobj failed"
else:
    print "max_length decompressobj succeeded"
...



----------------------------------------------------------------------

>Comment By: Tim Peters (tim_one)
Date: 2003-02-02 22:22

Message:
Logged In: YES 
user_id=31435

It looks like the problem with getting this reviewed is that 
no current developer understands the zlib code or its test.  
I bumped the priority in a sneaky attempt to attract more 
interest <wink>.  *If* nobody else bites soon, and I can 
make time, I'll assign it to myself.

Your efforts are appreciated!  Even if it doesn't seem like it.

----------------------------------------------------------------------

Comment By: Scott David Daniels (scott_daniels)
Date: 2003-01-31 22:33

Message:
Logged In: YES 
user_id=493818

I've attached a PyUnit-style replacement for test_zlib.py that 
tests this condition to Patch #678531,  which provides a
context 
diff updating zlibmodule.c to fix this problem.  Hope this
is enough 
to get the fix reviewed.


----------------------------------------------------------------------

Comment By: Scott David Daniels (scott_daniels)
Date: 2002-12-12 13:32

Message:
Logged In: YES 
user_id=493818

The following replacement for PyZlib_unflush addresses some 
of the problem, but leaves unaddressed the issues of what 
unused_data and unconsumed_tail should be at the end of the 
routine.

static PyObject *
PyZlib_unflush(compobject *self, PyObject *args)
{
    int err, length = DEFAULTALLOC;
    PyObject * retval = NULL;
    unsigned long start_total_out;

    if (!PyArg_ParseTuple(args, "|i:flush", &length))
	return NULL;
    if (!(retval = PyString_FromStringAndSize(NULL, length)))
	return NULL;


    ENTER_ZLIB

    start_total_out = self->zst.total_out;
    self->zst.avail_out = length;
    self->zst.next_out = (Byte *)PyString_AS_STRING(retval);

    Py_BEGIN_ALLOW_THREADS
    err = inflate(&(self->zst), Z_FINISH);
    Py_END_ALLOW_THREADS

    /* while Z_OK and the output buffer is full, there might
be more output,
       so extend the output buffer and try again */
    while ((err == Z_OK || err == Z_BUF_ERROR) &&
self->zst.avail_out == 0) {
	if (_PyString_Resize(&retval, length << 1) < 0)
	    goto error;
	self->zst.next_out = (Byte *)PyString_AS_STRING(retval) +
length;
	self->zst.avail_out = length;
	length = length << 1;

	Py_BEGIN_ALLOW_THREADS
	err = inflate(&(self->zst), Z_FINISH);
	Py_END_ALLOW_THREADS
    }

    /* Maybe _always_ call inflateEnd if clearing
is_initialized */
    if (err == Z_STREAM_END) {
	err = inflateEnd(&(self->zst));
	if (err != Z_OK) {
	    zlib_error(self->zst, err, "from inflateEnd()");
	    Py_DECREF(retval);
	    retval = NULL;
	    goto error;
	}
    }
    self->is_initialised = 0;
    _PyString_Resize(&retval, self->zst.total_out -
start_total_out);
    /* ??? Here fix unused_data and unconsumed_tail */

error:

    LEAVE_ZLIB

    return retval;
}



----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2002-11-19 17:08

Message:
Logged In: YES 
user_id=31435

I'd say it's a bug in flush(), or in the docs, then.  They say "all 
pending input is processed, and a string containing the 
remaining uncompressed output is returned", and that's not 
happening here.  That's pretty clearly what the author of this 
test *expected* to happen, too (the original bug you 
discovered looked much more like a typo than a failure to 
understand the module interface).

----------------------------------------------------------------------

Comment By: Scott David Daniels (scott_daniels)
Date: 2002-11-19 16:40

Message:
Logged In: YES 
user_id=493818

OK, The reason the code fails after the fix is as follows:
deco.flush() does not return any output.
The loop control says, "while we have more unexamined 
source."

However, the decompressor can consume all of the input 
before it has provided all of its output.  So, there are two 
possible fixes:

1) Minimal change:  Change the loop control to say, in 
effect, "While we have more input or are producing output.":
Around line 92:
Change:
    bufs = []
    while cb:
        max_length = 1 + len(cb)/10
To:
    bufs = []
    while cb or chunk:
        max_length = 1 + len(cb)/10


2) More reasonable(?) change:  After the insertion loop, (just 
before the flush) extract all remaining output:
Around line 99:
        cb = deco.unconsumed_tail
    bufs.append(deco.flush())
    decomp2 = ''.join(bufs)
Becomes:
        cb = deco.unconsumed_tail
    while bufs[-1]:
        bufs.append(deco.decompress('', max_length))
    bufs.append(deco.flush())
    decomp2 = ''.join(bufs)


---
Note, in any case, the
    bufs.append(deco.flush())
originally on line 100 _always_ appends a zero-length string.
I would suggest changing it to simply:
    deco.flush()

    

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2002-11-19 04:50

Message:
Logged In: YES 
user_id=6656

Assuming Tim meant what he said, not what he did: raising
priority.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2002-11-18 15:56

Message:
Logged In: YES 
user_id=31435

Good eye!  Unfortunately, when I repair that, the test fails 
here -- decomp2 is a proper prefix of buf then.  It's "too 
short".  Raised priority but left unassigned (someone who 
understands zlib better will have to jump in).

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=640230&group_id=5470