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

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

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.