Bug 63628 - Support specifying the http status codes to be considered by ProxyErrorOverride
Summary: Support specifying the http status codes to be considered by ProxyErrorOverride
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_proxy (show other bugs)
Version: 2.5-HEAD
Hardware: PC Linux
: P2 enhancement with 3 votes (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: FixedInTrunk, PatchAvailable
Depends on:
Blocks:
 
Reported: 2019-08-02 13:07 UTC by Martin Drößler
Modified: 2020-04-11 21:20 UTC (History)
1 user (show)



Attachments
Patch to support specifying the http status codes to be considered by ProxyErrorOverride (12.11 KB, patch)
2019-08-02 13:09 UTC, Martin Drößler
Details | Diff
Patch ProxyErrorOverride to allow specifying custom codes (12.83 KB, patch)
2020-01-24 16:46 UTC, Martin Drößler
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Drößler 2019-08-02 13:07:54 UTC
The issue:
----------
When using the directive "ProxyErrorOverride", it will intercept all responses with status codes between 400 (including) and 600 (excluding).
But we need to "pass-through" certain responses like 401 or 404, and in some cases we only want to handle 50x responses all together.

There was a similar feature-request once: https://bz.apache.org/bugzilla/show_bug.cgi?id=50732


The (proposed) solution:
------------------------
Changing the usage of "ProxyErrorOverride" appeared not like an ideal solution to me, so I introduced a new directive "ProxyErrorIntercept" in addition to it.
When not specified in the config, the behaviour of "ProxyErrorOverride" is unchanged.
If specified, "ProxyErrorOverride" will only consider responses with the configured status codes.


Disclaimer:
-----------
My skills in C are a bit rusty. So I appreciate any constructive feedback for the deficiencies that might be in my code.
Comment 1 Martin Drößler 2019-08-02 13:09:28 UTC
Created attachment 36701 [details]
Patch to support specifying the http status codes to be considered by ProxyErrorOverride
Comment 2 Martin Drößler 2019-10-16 07:54:36 UTC
We're still blocked with our migration-project, due to this feature being missing.
Any idea on when we can expect a response?
Comment 3 Martin Drößler 2020-01-24 16:46:13 UTC
Created attachment 36979 [details]
Patch ProxyErrorOverride to allow specifying custom codes

I finally managed to implemented the changes proposed on the mailing-list:

> 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.
Done (but used PROXY_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.
Good point. I also changed that in the implementation as well as documentation.

> 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.
I merged the directives to allow a list of status codes after the ON in the ProxyErrorOverride directive.

> Another personal nit -- the name of the two added functions is not so
> clear to me.
I changed the name of at least one of the methods to make it less confusing. If you feel, that the naming still isn't optimal, I'm of course open for suggestions.

> 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.
You're right. Changed that as well.
Comment 4 Eric Covener 2020-04-11 21:20:56 UTC
Thanks for the enhancement, I've committed it in 1876404 with a handful of small changes for style/readability during review/test