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

Re: Bug in mod_ratelimit?

Hi Luca,

Sorry for the delay in response.. we did look into it further.. 

On of our devs had been looking into it and came up with the following:

While it will probably resolve the issues we saw, I’d be hesitant to move forward with that patch as it modifies how all output filters work with HEAD requests, 
this is too large a change, especially when the bug(s) being addressesed are in a single module.

I’d recommend making mod_ratelimit do the same “optimization” hack that other modules for HEAD requests instead, and keep the surface area for this bug fix isolated to mod_ratelimit only.

Something like what mod_brotli does:
         if (r->header_only && r->bytes_sent) {
             return ap_pass_brigade(f->next, bb);

If there are any further adjustments to this patch we’d be happy to take a look, just let us know.

Cory McIntire
Release Manager - EasyApache 
cPanel, Inc.

> On Jul 27, 2018, at 10:46 AM, Luca Toscano <toscano.luca@xxxxxxxxx> wrote:
> Hi Cory,
> 2018-07-20 13:47 GMT+02:00 Yann Ylavic <ylavic.dev@xxxxxxxxx>:
>> Hi Cory,
>> On Thu, Jul 19, 2018 at 11:23 PM, Cory McIntire <cory@xxxxxxxxxx> wrote:
>>> We’re going to revert to the 2.4.33 version of mod_ratelimit for now.
>>> HEAD requests with large amount of headers were still problematic in our testing with both versions of the patch applied.
>> Thanks for letting us know.
>> I think the right fix is the attached patch (tested with GET/HEAD with
>> large header and/or body, seems to work).
>> If by any chance you can give it a try...
> In the meantime, other people are testing Yann's last patch in
> https://bz.apache.org/bugzilla/show_bug.cgi?id=62568 (it is attached
> in there). If you could chime in whenever you have time and let us
> know your thoughts it would be really great.
> Thanks in advance!
> Luca

Attachment: smime.p7s
Description: S/MIME cryptographic signature