Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride

Martin Drößler
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
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride

Martin Drößler
We're still in need of this feature.
Is there anyone who can review the patch?

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

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride

Eric Covener
On Thu, Dec 5, 2019 at 7:51 AM Martin Drößler <[hidden email]> 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
> >>>
> >>
> >
>


--
Eric Covener
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 63628] Support specifying the http status codes to be considered by ProxyErrorOverride

Martin Drößler
Thanks for the feedback.
I'll try to find some time to implement the proposed changes.

Regards
Martin Drößler


Eric Covener schrieb am 05.12.2019 14:29 (GMT +01:00):

> On Thu, Dec 5, 2019 at 7:51 AM Martin Drößler <[hidden email]>
> 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
>> >>>
>> >>
>> >
>>
>
>
> --
> Eric Covener
> [hidden email]
>