From dev-return-55511-apmail-httpd-dev-archive=httpd.apache.org@httpd.apache.org Thu Nov 16 13:25:44 2006 Return-Path: Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: (qmail 82487 invoked from network); 16 Nov 2006 13:25:41 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 16 Nov 2006 13:25:41 -0000 Received: (qmail 93652 invoked by uid 500); 16 Nov 2006 13:25:46 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 93602 invoked by uid 500); 16 Nov 2006 13:25:46 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 93591 invoked by uid 99); 16 Nov 2006 13:25:46 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 16 Nov 2006 05:25:46 -0800 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests= X-Spam-Check-By: apache.org Received-SPF: neutral (herse.apache.org: local policy) Received: from [195.233.129.142] (HELO rat01037.dc-ratingen.de) (195.233.129.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 16 Nov 2006 05:25:31 -0800 Received: from rat01047.dc-ratingen.de (rat01047_e0 [195.233.128.119]) by rat01037.dc-ratingen.de (Switch-3.1.4/Switch-3.1.0) with ESMTP id kAGDP8Tj015756 for ; Thu, 16 Nov 2006 14:25:08 +0100 (MET) Received: from avoexf01.internal.vodafone.com ([145.230.4.132]) by rat01047.dc-ratingen.de (Switch-3.1.4/Switch-3.1.0) with ESMTP id kAGDP89e026236 for ; Thu, 16 Nov 2006 14:25:08 +0100 (MET) Received: from EITO-MBX03.internal.vodafone.com ([145.230.4.6]) by avoexf01.internal.vodafone.com with Microsoft SMTPSVC(6.0.3790.1830); Thu, 16 Nov 2006 14:25:11 +0100 X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: Re: http://svn.apache.org/viewvc?view=rev&revision=426799 Date: Thu, 16 Nov 2006 14:25:07 +0100 Message-ID: <3B21A253728EA247A10A692547A271530E7398@EITO-MBX03.internal.vodafone.com> In-Reply-To: <20061116013532.00ce3881@grimnir> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: Re: http://svn.apache.org/viewvc?view=rev&revision=426799 thread-index: AccJH9UgL5bs9+3GSly4pPIUgGietwAYHTjA From: =?iso-8859-1?Q?Pl=FCm=2C_R=FCdiger=2C_VF_EITO?= To: X-OriginalArrivalTime: 16 Nov 2006 13:25:11.0060 (UTC) FILETIME=[A4523D40:01C70982] X-Virus-Checked: Checked by ClamAV on apache.org > -----Urspr=FCngliche Nachricht----- > Von: Nick Kew=20 > Gesendet: Donnerstag, 16. November 2006 02:36 >=20 >=20 > On Wed, 15 Nov 2006 21:33:07 +0100 > Ruediger Pluem wrote: >=20 > >=20 > > Because of your question I had to rewalk the code path and I think > > I found two other bugs with my code. I fixed them on trunk: > >=20 > > http://svn.apache.org/viewvc?view=3Drev&revision=3D475403 >=20 > Hang on. It's worse than that. Or else I'm suffering=20 > "shouldn't be working in the wee hours" syndrome. >=20 > When you first set up the validation buffer, you copy available > data into it, and set validation_buffer_length. Now the memcpy > in this section of code is overwriting validation_buffer, > when it should be appending to it. Then you increment the > buffer_length, and decrement avail_in by the number of bytes > appended. At that point, if avail_in is nonzero we might want > to log a warning of extra junk. Argh. You are right. Good catch. I have currently no svn access but the patch below should fix this: Index: mod_deflate.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- mod_deflate.c (revision 475613) +++ mod_deflate.c (working copy) @@ -1144,20 +1144,24 @@ copy_size =3D VALIDATION_SIZE - = ctx->validation_buffer_length; if (copy_size > ctx->stream.avail_in) copy_size =3D ctx->stream.avail_in; - memcpy(ctx->validation_buffer, ctx->stream.next_in, = copy_size); - } else { + memcpy(ctx->validation_buffer + = ctx->validation_buffer_length, + ctx->stream.next_in, copy_size); + /* Saved copy_size bytes */ + ctx->stream.avail_in -=3D copy_size; + } + if (ctx->stream.avail_in) { ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "Zlib: %d bytes of garbage at the end of = " "compressed stream.", = ctx->stream.avail_in); + /* + * There is nothing worth consuming for zlib left, = because it is + * either garbage data or the data has been copied to = the + * validation buffer (processing validation data is no = business + * for zlib). So set ctx->stream.avail_in to zero to = indicate + * this to the following while loop. + */ + ctx->stream.avail_in =3D 0; } - /* - * There is nothing worth consuming for zlib left, because = it is - * either garbage data or the data has been copied to the - * validation buffer (processing validation data is no = business for - * zlib). So set ctx->stream.avail_in to zero to indicate = this to - * the following while loop. - */ - ctx->stream.avail_in =3D 0; } =20 zRC =3D Z_OK; >=20 > > http://svn.apache.org/viewvc?view=3Drev&revision=3D475406 >=20 > Why? I'd like to understand what makes that necessary. I think that I remember cases where two EOS buckets run down the chain. = IMHO this is a bug elsewhere, but I would like to prevent SEGFAULTing in = mod_deflate if this happens. So you might call it defensive programmming here. >=20 > Edge-cases can be notoriously hard to test. I wonder if there's > a compression/zlib test suite we could use? Regarding the compressed content I simply used files that I compressed = with gzip and stripped of the gz extension. I guess the hard case here is to = split them in a way to buckets and brigades such that all edge cases can be = tested. In this case a filter just after the default handler would be handy that = would allow us to split the brigade from the default handler at certain = boundaries and let us split the buckets inside these brigades at certain predefined = boundaries. Regards R=FCdiger