osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

AW: Bug in mod_ratelimit?



> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic <ylavic.dev@xxxxxxxxx>
> Gesendet: Montag, 23. Juli 2018 10:52
> An: httpd-dev <dev@xxxxxxxxxxxxxxxx>
> Betreff: Re: Bug in mod_ratelimit?
> 
> On Mon, Jul 23, 2018 at 7:45 AM, Plüm, Rüdiger, Vodafone Group
> <ruediger.pluem@xxxxxxxxxxxx> wrote:
> >
> >
> >> -----Ursprüngliche Nachricht-----
> >> Von: Eric Covener <covener@xxxxxxxxx>
> >> Gesendet: Sonntag, 22. Juli 2018 21:58
> >> An: Apache HTTP Server Development List <dev@xxxxxxxxxxxxxxxx>
> >> Betreff: Re: Bug in mod_ratelimit?
> >>
> >> > > You probably didn't test with chunked encoding (neither did I!),
> see
> >> > > how ap_http_header_filter() adds the "CHUNK" filter after the
> >> headers
> >> > > have been sent, so if anything retains the headers they will be
> >> > > considered as the body by the (late) ap_http_chunk_filter()..
> >>
> >> would it be reasonable to send a flush down, and/or track whether the
> >> headers are flushed somewhere out of the http filter ctx?
> >
> > Flush in case of mod_ratelimit or in general?
> 
> mod_ratelimit FLUSHes data already, and the first patch proposed here
> allowed the headers to pass with a FLUSH.
> Yet if headers are large enough to be rate limited, the first ones
> will pass before the "CHUNK" filter is added, but not the next ones.
> 
> > In general wouldn't
> > that cause "smaller" TCP packets to be sent if content is ready and
> > the headers are "short" and hence cause some performance
> > degradation?
> 
> In the general case I agree that we shouldn't always flush the
> headers, moreover mod_ratelimit isn't supposed to (and doesn't) honor
> FLUSHes. The rate is applied regardless of FLUSH buckets from previous
> filters.
> 
> One way to possibly address this would be to handle a new EOH (End Of
> Header) meta-bucket, à la EOS/EOR/..
> ap_http_header_filter() would add the "CHUNK" filter first and then
> pass the headers brigade ended by EOH.
> The "CHUNK" filter would do nothing until EOH.
> 
> I think my patch "rate_limit_filter+header_only.patch" is needed
> anyway w.r.t. header_only handling in ap_http_header_filter() and EOS.

I think above patches are the best way to get ahead right now.

Regards

Rüdiger