> -----Ursprüngliche Nachricht-----
> Von: Nick Kew
> Gesendet: Donnerstag, 16. November 2006 02:36
>
>
> On Wed, 15 Nov 2006 21:33:07 +0100
> Ruediger Pluem <[EMAIL PROTECTED]> wrote:
>
> >
> > 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:
> >
> > http://svn.apache.org/viewvc?view=rev&revision=475403
>
> Hang on. It's worse than that. Or else I'm suffering
> "shouldn't be working in the wee hours" syndrome.
>
> 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
===================================================================
--- mod_deflate.c (revision 475613)
+++ mod_deflate.c (working copy)
@@ -1144,20 +1144,24 @@
copy_size = VALIDATION_SIZE - ctx->validation_buffer_length;
if (copy_size > ctx->stream.avail_in)
copy_size = 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 -= 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 = 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 = 0;
}
zRC = Z_OK;
>
> > http://svn.apache.org/viewvc?view=rev&revision=475406
>
> 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.
>
> 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üdiger
