On 4/9/07, Nick Kew <[EMAIL PROTECTED]> wrote:
On Mon, 9 Apr 2007 11:08:55 -0400
"Jeff Trawick" <[EMAIL PROTECTED]> wrote:
> On 4/5/07, Nick Kew <[EMAIL PROTECTED]> wrote:
> > On Thu, 5 Apr 2007 10:04:19 +0100
> > Joe Orton <[EMAIL PROTECTED]> wrote:
> >
> >
> > > I agree that the intended behaviour of the original code was
> > > intuitively correct, only >= 400 errors should be overriden,
> >
> > A redirection page is likely to include a redirected URL.
> > In a reverse proxy situation, that may need to be rewritten.
> > Use of ProxyErrorOverride could be a valid alternative to
> > mod_proxy_html in that situation, couldn't it?
> >
> > Looks to me like a valid usage case for ProxyErrorOverride 3xx.
>
> We have a couple of people trying to make their actual problem go away
> permanently by keeping up with this thread and trying to match
> developer comments with patches.
Yep.
> IMO we should go ahead and fix what is broken now, and let
> requirements for the new feature (ProxyErrorOverride nxx) present
> themselves in the fullness of time. If somebody is relying on the
> current feature-by-defect, then we'll find out soon enough. If there
> is never a compelling requirement, then we're left with the simpler
> code (which is already hard enough to keep working ;) ).
Fine by me. I was ready to commit a give-the-users-the-choice patch,
then someone objected to it. If you want to commit the no-choice
patch instead, that's OK.
Either way, it'll want an accompanying documentation patch before it
goes into a release.
I wonder why "Error" in ProxyErrorOverride doesn't match the meaning
of ap_is_HTTP_ERROR(), as in the attached patch (with doc).
1xx isn't something the user should see/react to either.
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 527940)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -1448,7 +1448,7 @@
* if we are overriding the errors, we can't put the content
* of the page into the brigade
*/
- if (!conf->error_override || ap_is_HTTP_SUCCESS(r->status)) {
+ if (!conf->error_override || !ap_is_HTTP_ERROR(r->status)) {
/* read the body, pass it to the output filters */
apr_read_type_e mode = APR_NONBLOCK_READ;
int finish = FALSE;
@@ -1557,7 +1557,7 @@
if (conf->error_override) {
/* the code above this checks for 'OK' which is what the hook expects
*/
- if (ap_is_HTTP_SUCCESS(r->status))
+ if (!ap_is_HTTP_ERROR(r->status))
return OK;
else {
/* clear r->status for override error, otherwise ErrorDocument
Index: docs/manual/mod/mod_proxy.xml
===================================================================
--- docs/manual/mod/mod_proxy.xml (revision 527940)
+++ docs/manual/mod/mod_proxy.xml (working copy)
@@ -1196,6 +1196,9 @@
the error code and act accordingly (default behavior would display
the error page of the proxied server, turning this on shows the SSI
Error message).</p>
+
+ <p>This directive does not affect the processing of informational (1xx),
+ normal success (2xx), or redirect (3xx) responses.</p>
</usage>
</directivesynopsis>
