Should be fine. Regards
Rüdiger > -----Ursprüngliche Nachricht----- > Von: Yann Ylavic [mailto:[email protected]] > Gesendet: Donnerstag, 21. August 2014 16:37 > An: httpd > Betreff: Re: svn commit: r1619383 - > /httpd/httpd/trunk/modules/filters/mod_deflate.c > > The (missing) patch... > > On Thu, Aug 21, 2014 at 4:34 PM, Yann Ylavic <[email protected]> > wrote: > > On Thu, Aug 21, 2014 at 3:11 PM, <[email protected]> wrote: > >> Author: covener > >> Date: Thu Aug 21 13:11:15 2014 > >> New Revision: 1619383 > >> > >> URL: http://svn.apache.org/r1619383 > >> Log: > >> A misplaced check for inflation limits prevented limiting relatively > >> small inputs. PR56872 > >> > >> Submitted By: Edward Lu > >> Committed By: covener > >> > > [...] > >> Modified: httpd/httpd/trunk/modules/filters/mod_deflate.c > >> URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_defla > te.c?rev=1619383&r1=1619382&r2=1619383&view=diff > >> > ======================================================================== > ====== > >> --- httpd/httpd/trunk/modules/filters/mod_deflate.c (original) > >> +++ httpd/httpd/trunk/modules/filters/mod_deflate.c Thu Aug 21 > 13:11:15 2014 > > [...] > >> @@ -1398,6 +1378,27 @@ static apr_status_t deflate_in_filter(ap > >> } > >> > >> zRC = inflate(&ctx->stream, Z_NO_FLUSH); > >> + len = c->bufferSize - ctx->stream.avail_out; > >> + > >> + ctx->inflate_total += len; > > > > Does the inflate() call garantee to fill the output buffer entirely > > whenever it has enough inputs (ie. after the call, either avail_in or > > avail_out is zero)? > > I can't find any clue in the docs nor in the (huge-state-machine- > )code. > > > > Otherwise, I think we'd better compute the number of bytes really > > produced by the inflate(), for each loop, that is : > > > > len = ctx->stream.avail_out; > > zRC = inflate(&ctx->stream, Z_NO_FLUSH); > > len -= ctx->stream.avail_out; > > > > or we way count the same bytes multiple times. > > > >> + if (inflate_limit && ctx->inflate_total > > inflate_limit) { > >> + inflateEnd(&ctx->stream); > >> + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, > r, APLOGNO(02648) > >> + "Inflated content length of %" > APR_OFF_T_FMT > >> + " is larger than the configured > limit" > >> + " of %" APR_OFF_T_FMT, > >> + ctx->inflate_total, inflate_limit); > >> + return APR_ENOSPC; > >> + } > >> + > >> + if (!check_ratio(r, ctx, dc)) { > >> + inflateEnd(&ctx->stream); > >> + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, > r, APLOGNO(02649) > >> + "Inflated content ratio is larger > than the " > >> + "configured limit %i by %i time(s)", > >> + dc->ratio_limit, dc->ratio_burst); > >> + return APR_EINVAL; > >> + } > >> > >> if (zRC == Z_STREAM_END) { > >> ctx->validation_buffer = apr_pcalloc(r- > >pool, > >> > >> > > > > I think the following test (from the sources, not included in the > diff) : > > > > if (zRC != Z_OK) { > > inflateEnd(&ctx->stream); > > ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, > > APLOGNO(01392) > > "Zlib error %d inflating data > (%s)", zRC, > > ctx->stream.msg); > > return APR_EGENERAL; > > } > > > > should probably be moved just after the inflate() call, so to check > > errors before limits. > > > > It would then become : > > > > zRC = inflate(&ctx->stream, Z_NO_FLUSH); > > > > if (zRC != Z_OK && zRC != Z_STREAM_END) { > > ... > > return APR_EGENERAL; > > } > > > > if (inflate_limit && ctx->inflate_total > > inflate_limit) { > > ... > > } > > > > if (!check_ratio(r, ctx, dc)) { > > ... > > } > > > > if (zRC == Z_STREAM_END) { > > ... > > break; > > } > > > > > > All this changes are in the attached patch (which is often more > clear). > > The patch also fixes the same issue regarding the check_ratio() call > > in inflate_out_filter(), and adds a missing check_ratio() in > > deflate_in_filter() when a FLUSH bucket is encountered (still > > following the existing inflate_limit check). > > > > Should I apply it?
