Small bug in zlib module

[Greg Chapman]

On 6 Jun 2000 16:42:06 -0000, Peter Schneider-Kamp wrote:
> Greg Chapman wrote:
> >
> > Using Python 1.5.2 (under Windows 98), this script causes python to
> > enter a loop in which it progressively allocates larger and larger
> > blocks of memory:
> >
> >   import zlib
> >   s = zlib.decompress("")
> > I assume PyZlib_decompress should have a check for an empty input
> > string and (if found) should generate an appropriate exception.
> >
> On the current CVS version it immediately (as far as I can see without
> looping) returns with an error.
>

I just checked ver. 2.32 (which seems to be the latest in the SourceForge
web CVS repository).  If the check you're referring to is this:

  if (r_strlen <= 0)
      r_strlen = 1;

that doesn't catch the condition I described.  Instead, you need to check
the "length" variable, which has the length of the input string (and I don't
see such a check; perhaps I missed it?).  Anyway, as it turns out, simply
checking for an empty input string is not sufficient.  The following causes
Python to enter the same memory allocation loop:

  import zlib
  s = zlib.decompress("\010")

I received a reply from eito which I believe correctly diagnoses the problem
and solution (this reply apparently did not go to the list):

On Tue, 6 Jun 2000 13:07:58 -0700, eito wrote:
> I think because, in zlib module, when it gets Z_BUF_ERROR, it does not
> check for the cause of the error. I don't know if the author of this
> module intentionally left the checking. (the same is true in the
> latest CVS version) Because it will return an error (MemoryError)
> anyway.
>
>
>   [from zlib.h (version 1.13)]
>   "Inflate() returns ... Z_BUF_ERROR if no progress is possible or
>    if there was not enough room in the output buffer when Z_FINISH
>    is used."
>
>
> The zlib's description is not so clear to me, but I modified zlib
> module anyway... The following patch (in the end of this e-mail) for
> zlibmodule.c (of version 1.5.2) should fix your problem. This patch
> checks for the output buffer, if the inflate() gets Z_BUF_ERROR. And
> if there is a room in the output buffer, it return an error.
> Otherwise, it goes to case(Z_OK), which tries to allocate more output
> buffer.
>

Eito's patch (see below) looks OK to me, provided that the test avail_out >
0 is a reliable way to distinguish between the two kinds of Z_BUF_ERROR
(i.e., cases where no progress is possible and cases where progress is
possible, but a bigger output buffer is needed).

By the way, I apologize for posting this to the main list.  I didn't realize
until yesterday that www.python.org has a bug list to handle this sort of
report.

Greg Chapman


-----zlibmodule.patch-----
*** zlibmodule.c.orig   Tue Jun  6 09:42:12 2000
--- zlibmodule.c        Tue Jun  6 09:53:12 2000
***************
*** 222,227 ****
--- 222,240 ----
          case(Z_STREAM_END):
            break;
        case(Z_BUF_ERROR):
+             /*
+              * If there is at least 1 byte of room in zst.next_out
+              * and get this error, I assume that it means zlib cannot
+              * process inflate.
+              */
+             if (0 < zst.avail_out)
+             {
+               PyErr_Format(ZlibError, "Error %i while decompressing data",
+                            err);
+               inflateEnd(&zst);
+               Py_DECREF(result_str);
+               return NULL;
+             }
          case(Z_OK):
            /* need more memory */
            if (_PyString_Resize(&result_str, r_strlen << 1) == -1)
-----zlibmodule.patch-----