#42117 [Asn->Csd]: [PATCH] bzip2.compress loses data in internal buffer

ID: 42117 Updated by: iliaa@php.net Reported By: phofstetter at sensational dot ch -Status: Assigned +Status: Closed Bug Type: Bzip2 Related Operating System: * PHP Version: 5.2CVS-2007-08-05 Assigned To: iliaa New Comment: This bug has been fixed in CVS. Snapshots of the sources are packaged every three hours; this change will be in the next snapshot. You can grab the snapshot at http://snaps.php.net/. Thank you for the report, and for helping us make PHP better. Previous Comments: ------------------------------------------------------------------------ [2007-08-08 08:21:12] jani@php.net Ilia, can you check this out please? See also bug #29521 which is also assigned to you. :) ------------------------------------------------------------------------ [2007-08-05 17:13:02] jani@php.net See also bug #29521 ------------------------------------------------------------------------ [2007-08-05 14:26:35] phofstetter at sensational dot ch Hello, even with the latest snapshot, the bug is still there. Data in bzip's internal buffer is lost on stream close due to the problem I discovered in bz2_filter.c The patch I proposed blow seems to do the right thing and in fact is now creating more than 1000 correct bzip2-streams per day, so I think it's save to say that it really does its job :-) It's illegal to compare the return code of BZ2_bzCompress(&(data->strm), BZ_FINISH); with BZ_OUTBUFF_FULL as BZ2_bzCompress *never* returns BZ_OUTBUFF_FULL (which is a return value of one of the higher level convenience functions in bzlib. Philip ------------------------------------------------------------------------ [2007-07-27 10:06:28] phofstetter at sensational dot ch looking at the documentation wasn't enough. When I looked at the source of bzlib, I found out this: BZ2_bzCompress called with BZ_FINISH keeps returning BZ_FINISH_OK (instead of BZ_RUN_OK which I assumed after reading the docs) until it's really done. Then it will return BZ_STREAM_END So the following patch fixes this bug: --- bz2_filter.c.orig 2007-07-27 11:24:44.000000000 +0200 +++ bz2_filter.c 2007-07-27 11:54:35.000000000 +0200 @@ -228,8 +228,8 @@ if (flags & PSFS_FLAG_FLUSH_CLOSE) { /* Spit it out! */ - status = BZ_OUTBUFF_FULL; - while (status == BZ_OUTBUFF_FULL) { + status = BZ_FINISH_OK; + while (status == BZ_FINISH_OK) { status = BZ2_bzCompress(&(data->strm), BZ_FINISH); if (data->strm.avail_out < data->outbuf_len) { size_t bucketlen = data->outbuf_len - data->strm.avail_out; With this modification, the complete data gets written out to the stream. Please consider applying this patch as without it, the bzip2.compress filter will sometimes (often - if the data is large enough to be bigger than the internal buffer) create corrupted data. Philip PS: The patch is against 5.2.2 as I'm unable to compile 5.2.3 on OSX with GD enabled due to gcc being called with an empty -L tag somewhere in configure. ------------------------------------------------------------------------ [2007-07-27 09:21:38] phofstetter at sensational dot ch after finally getting some sleep, today I looked at the problem in bz2_filter.c and I may have found something. On line 229+ we have if (flags & PSFS_FLAG_FLUSH_CLOSE) { /* Spit it out! */ status = BZ_OUTBUFF_FULL; while (status == BZ_OUTBUFF_FULL) { status = BZ2_bzCompress(&(data->strm), BZ_FINISH); if (data->strm.avail_out < data->outbuf_len) { size_t bucketlen = data->outbuf_len - data->strm.avail_out; bucket = php_stream_bucket_new(stream, estrndup(data->outbuf, bucketlen), bucketlen, 1, 0 TSRMLS_CC); php_stream_bucket_append(buckets_out, bucket TSRMLS_CC); data->strm.avail_out = data->outbuf_len; data->strm.next_out = data->outbuf; exit_status = PSFS_PASS_ON; } } } now the problem is IMHO that BZ2_bzCompress with BZ_FINISH will never return BZ_OUTBUFF_FULL. Looking at the documentation, it will return BZ_RUN_OK until all data has been processed when it will return BZ_FINISH_OK. So with the code as it is currently in PHP, it will only do one run ob BZ2_bzCompress and then stop working even though more calls could be needed. This is consistent with how the bug manifests itself. I will try to correct the return code handling, but keep in mind that my C-skills are subpar, so the patch I'm going to post afterwards is probably not as good as it could be, so please have a look at the thing. Philip ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/42117 -- Edit this bug report at http://bugs.php.net/?id=42117&edit=1