git.net

[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
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.


Regards,
Yann.



( ! ) Warning: include(msgfooter.php): failed to open stream: No such file or directory in /var/www/git/apache2-developers/msg04356.html on line 117
Call Stack
#TimeMemoryFunctionLocation
10.0007364424{main}( ).../msg04356.html:0

( ! ) Warning: include(): Failed opening 'msgfooter.php' for inclusion (include_path='.:/var/www/git') in /var/www/git/apache2-developers/msg04356.html on line 117
Call Stack
#TimeMemoryFunctionLocation
10.0007364424{main}( ).../msg04356.html:0