You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Martin Drößler <ma...@martindroessler.de> on 2020/02/03 17:33:16 UTC
Re: [PATCH 63628] Support specifying the http status codes to be
considered by ProxyErrorOverride
I attached a new/improved patch to the bug-ticket:
https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
Regards
Martin Drößler
Am 05.12.19 um 14:29 schrieb Eric Covener:
> On Thu, Dec 5, 2019 at 7:51 AM Martin Drößler <ma...@martindroessler.de> wrote:
>>
>> We're still in need of this feature.
>> Is there anyone who can review the patch?
>
> I think the proxy_util.c additions need an ap_ prefix and need to be
> declared like all of the other non-static functions with AP_DECLARE.
>
> The description and the manual seem to hide the use of this for
> non-error codes while the diff seems to go out of its way to allow
> non-error codes.
> I think it should either be constrained in the diff or have some
> notes/warnings/elaboration in the doc.
>
> I personally do not like the use of two directives and the intercept
> and override terminology mixing. I prefer that ProxyErrorOverride is
> extended to accept ON or a list of status codes.
> Another personal nit -- the name of the two added functions is not so
> clear to me.
>
>
> +int is_proxy_error_intercept_code(proxy_dir_conf *conf, int code)
> +{
> + if (apr_is_empty_array(conf->error_intercept_codes))
> + return 0;
> +
> + proxy_status_code *list = (proxy_status_code *)
> conf->error_intercept_codes->elts;
> +
> + int i;
> ^^ not c89
>
>
> Also, is_proxy_error_intercept_code could be static (and not in
> mod_proxy.h) or just part of the other method since it is only called
> from the other method.
>
>
>
>>
>> Regards
>> Martin Drößler
>>
>> Martin Drößler schrieb am 16.09.2019 17:40 (GMT +02:00):
>>
>>> Quick reminder.
>>>
>>> Martin Drößler schrieb am 22.08.2019 10:25:
>>>
>>>> From: https://httpd.apache.org/dev/patches.html
>>>>> Post to the developers list pointing out your patch and why you feel it is
>>>>> important. Feel free to do this about once a week and continue until you get
>>>>> a
>>>>> response.
>>>>
>>>> In this regard: the weekly friendly reminder.
>>>>
>>>>
>>>> Regards
>>>> Martin Drößler
>>>>
>>>> Martin Drößler schrieb am 13.08.2019 10:18:
>>>>
>>>>> Hi,
>>>>>
>>>>> one and a half week ago I submitted a patch/bugreport for this feature.
>>>>> See: https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
>>>>>
>>>>> And, as suggested by the how-to (http://httpd.apache.org/dev/patches.html), I
>>>>> wanted to ask about some feedback.
>>>>>
>>>>> It would definitely help me and my company to decide, if we can continue with
>>>>> our migration-project.
>>>>>
>>>>>
>>>>> thanks,
>>>>> Martin Drößler
>>>>>
>>>>
>>>
>>
>
>
Re: [PATCH 63628] Support specifying the http status codes to be
considered by ProxyErrorOverride
Posted by Martin Drößler <ma...@martindroessler.de>.
Friendly reminder.
Am 26.02.20 um 10:13 schrieb Martin Drößler:
> Hi,
> any remarks regarding the new patch?
> Are the changes acceptable, or are there still some improvements required?
>
> Regards
> MGD
>
> Martin Drößler schrieb am 03.02.2020 18:33 (GMT +01:00):
>
>> I attached a new/improved patch to the bug-ticket:
>> https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
>>
>>
>> Regards
>> Martin Drößler
>>
>> Am 05.12.19 um 14:29 schrieb Eric Covener:
>>> On Thu, Dec 5, 2019 at 7:51 AM Martin Drößler <ma...@martindroessler.de>
>>> wrote:
>>>>
>>>> We're still in need of this feature.
>>>> Is there anyone who can review the patch?
>>>
>>> I think the proxy_util.c additions need an ap_ prefix and need to be
>>> declared like all of the other non-static functions with AP_DECLARE.
>>>
>>> The description and the manual seem to hide the use of this for
>>> non-error codes while the diff seems to go out of its way to allow
>>> non-error codes.
>>> I think it should either be constrained in the diff or have some
>>> notes/warnings/elaboration in the doc.
>>>
>>> I personally do not like the use of two directives and the intercept
>>> and override terminology mixing. I prefer that ProxyErrorOverride is
>>> extended to accept ON or a list of status codes.
>>> Another personal nit -- the name of the two added functions is not so
>>> clear to me.
>>>
>>>
>>> +int is_proxy_error_intercept_code(proxy_dir_conf *conf, int code)
>>> +{
>>> + if (apr_is_empty_array(conf->error_intercept_codes))
>>> + return 0;
>>> +
>>> + proxy_status_code *list = (proxy_status_code *)
>>> conf->error_intercept_codes->elts;
>>> +
>>> + int i;
>>> ^^ not c89
>>>
>>>
>>> Also, is_proxy_error_intercept_code could be static (and not in
>>> mod_proxy.h) or just part of the other method since it is only called
>>> from the other method.
>>>
>>>
>>>
>>>>
>>>> Regards
>>>> Martin Drößler
>>>>
>>>> Martin Drößler schrieb am 16.09.2019 17:40 (GMT +02:00):
>>>>
>>>>> Quick reminder.
>>>>>
>>>>> Martin Drößler schrieb am 22.08.2019 10:25:
>>>>>
>>>>>> From: https://httpd.apache.org/dev/patches.html
>>>>>>> Post to the developers list pointing out your patch and why you feel it is
>>>>>>> important. Feel free to do this about once a week and continue until you
>>>>>>> get
>>>>>>> a
>>>>>>> response.
>>>>>>
>>>>>> In this regard: the weekly friendly reminder.
>>>>>>
>>>>>>
>>>>>> Regards
>>>>>> Martin Drößler
>>>>>>
>>>>>> Martin Drößler schrieb am 13.08.2019 10:18:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> one and a half week ago I submitted a patch/bugreport for this feature.
>>>>>>> See: https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
>>>>>>>
>>>>>>> And, as suggested by the how-to
>>>>>>> (http://httpd.apache.org/dev/patches.html), I
>>>>>>> wanted to ask about some feedback.
>>>>>>>
>>>>>>> It would definitely help me and my company to decide, if we can continue
>>>>>>> with
>>>>>>> our migration-project.
>>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>> Martin Drößler
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride
Posted by Martin Drößler <ma...@martindroessler.de>.
Hi,
any remarks regarding the new patch?
Are the changes acceptable, or are there still some improvements required?
Regards
MGD
Martin Drößler schrieb am 03.02.2020 18:33 (GMT +01:00):
> I attached a new/improved patch to the bug-ticket:
> https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
>
>
> Regards
> Martin Drößler
>
> Am 05.12.19 um 14:29 schrieb Eric Covener:
>> On Thu, Dec 5, 2019 at 7:51 AM Martin Drößler <ma...@martindroessler.de>
>> wrote:
>>>
>>> We're still in need of this feature.
>>> Is there anyone who can review the patch?
>>
>> I think the proxy_util.c additions need an ap_ prefix and need to be
>> declared like all of the other non-static functions with AP_DECLARE.
>>
>> The description and the manual seem to hide the use of this for
>> non-error codes while the diff seems to go out of its way to allow
>> non-error codes.
>> I think it should either be constrained in the diff or have some
>> notes/warnings/elaboration in the doc.
>>
>> I personally do not like the use of two directives and the intercept
>> and override terminology mixing. I prefer that ProxyErrorOverride is
>> extended to accept ON or a list of status codes.
>> Another personal nit -- the name of the two added functions is not so
>> clear to me.
>>
>>
>> +int is_proxy_error_intercept_code(proxy_dir_conf *conf, int code)
>> +{
>> + if (apr_is_empty_array(conf->error_intercept_codes))
>> + return 0;
>> +
>> + proxy_status_code *list = (proxy_status_code *)
>> conf->error_intercept_codes->elts;
>> +
>> + int i;
>> ^^ not c89
>>
>>
>> Also, is_proxy_error_intercept_code could be static (and not in
>> mod_proxy.h) or just part of the other method since it is only called
>> from the other method.
>>
>>
>>
>>>
>>> Regards
>>> Martin Drößler
>>>
>>> Martin Drößler schrieb am 16.09.2019 17:40 (GMT +02:00):
>>>
>>>> Quick reminder.
>>>>
>>>> Martin Drößler schrieb am 22.08.2019 10:25:
>>>>
>>>>> From: https://httpd.apache.org/dev/patches.html
>>>>>> Post to the developers list pointing out your patch and why you feel it is
>>>>>> important. Feel free to do this about once a week and continue until you
>>>>>> get
>>>>>> a
>>>>>> response.
>>>>>
>>>>> In this regard: the weekly friendly reminder.
>>>>>
>>>>>
>>>>> Regards
>>>>> Martin Drößler
>>>>>
>>>>> Martin Drößler schrieb am 13.08.2019 10:18:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> one and a half week ago I submitted a patch/bugreport for this feature.
>>>>>> See: https://bz.apache.org/bugzilla/show_bug.cgi?id=63628
>>>>>>
>>>>>> And, as suggested by the how-to
>>>>>> (http://httpd.apache.org/dev/patches.html), I
>>>>>> wanted to ask about some feedback.
>>>>>>
>>>>>> It would definitely help me and my company to decide, if we can continue
>>>>>> with
>>>>>> our migration-project.
>>>>>>
>>>>>>
>>>>>> thanks,
>>>>>> Martin Drößler
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>