Merge r722213,r722081, and r721679 from trunk:

* Avoid sending no answer at all if a custom error page causes an
  AP_FILTER_ERROR.

* allow ap_invoke_handler() to pass-through AP_FILTER_ERROR as if it were
  a reserved status code (OK/DECLINED/SUSPENDED). Prevents ap_die() from seeing a
  500 error when the http header filter has already taken care of the proper
  error response

* To be safe, consume the entire brigade after processing an error bucket in
  the HTTP output filter.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@722642 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 8c3af59b3851995e0c754969132d769f115a4ef8..052e37692305ab097e4ec4025ce7cb9df7b0ace5 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,16 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.11
 
+  *) core: When the ap_http_header_filter processes an error bucket, cleanup
+     the passed brigade before returning AP_FILTER_ERROR down the filter 
+     chain. This unambiguously ensures the same error bucket isn't revisited
+     [Ruediger Pluem]
+
+  *) core: Error responses set by filters were being coerced into 500 errors,
+     sometimes appended to the original error response. Log entry of:
+     'Handler for (null) returned invalid result code -3'
+     [Eric Covener]
+
   *) configure: Don't reject libtool 2.x
      PR 44817 [Arfrever Frehtes Taifersar Arahesis <Arfrever.FTA gmail.com>]
 

diff --git a/STATUS b/STATUS
index db3dd4c75f3914f288abf9cd71ffd3173d40b47f..73f98716379c8a4e8678be2b985cb36cd57b14e0 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -86,27 +86,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * core: Allow ap_invoke_handler to pass AP_FILTER_ERROR through as if it
-     were a reserved return code (OK/DECLINED) instead of converting it to 
-     internal server error. 
-     Additionally:
-       * Clear the brigade in ap_http_header_filter when handling an error bucket.
-       * Avoid sending no answer at all if a custom error page causes 
-         an AP_FILTER_ERROR.
-      Trunk version of patch:
-        http://svn.apache.org/viewvc?rev=721679&view=rev
-        http://svn.apache.org/viewvc?rev=722081&view=rev
-        http://svn.apache.org/viewvc?rev=722213&view=rev
-      Backport version for 2.2.x of updated patch:
-        http://people.apache.org/~covener/2.2.x-ap_filter_error+722081+722213.diff
-      +1: covener, rpluem, niq
-      rpluem says: 1. There is a bogus entry in your patch to CHANGES
-                   2. I guess we should also consider r722213, because
-                      with the current state of the patch we IMHO create
-                      a regression in the case that a custom error page
-                      results in AP_FILTER_ERROR before sending the page.
-      covener says: r722213 added and extra CHANGES entry removed (reset niq's +1)
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
 

diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c
index 9620fc9dfb0b10a8a026557e1d53334953592694..89263d4dcb67483b505add373325aec0fefa2eb1 100644 (file)
--- a/modules/http/http_filters.c
+++ b/modules/http/http_filters.c
@@ -1135,7 +1135,11 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
         }
     }
     if (eb) {
-        ap_die(eb->status, r);
+        int status;
+
+        status = eb->status;
+        apr_brigade_cleanup(b);
+        ap_die(status, r);
         return AP_FILTER_ERROR;
     }
 

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index 8287c9383651ef1c405e0f8bcc66aa373526ceac..fe10a196220be6823a32953686dc486028236878 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -80,7 +80,31 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
     request_rec *r_1st_err = r;
 
     if (type == AP_FILTER_ERROR) {
-        return;
+        ap_filter_t *next;
+
+        /*
+         * Check if we still have the ap_http_header_filter in place. If
+         * this is the case we should not ignore AP_FILTER_ERROR here because
+         * it means that we have not sent any response at all and never
+         * will. This is bad. Sent an internal server error instead.
+         */
+        next = r->output_filters;
+        while (next && (next->frec != ap_http_header_filter_handle)) {
+               next = next->next;
+        }
+
+        /*
+         * If next != NULL then we left the while above because of
+         * next->frec == ap_http_header_filter
+         */
+        if (next) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                          "Custom error page caused AP_FILTER_ERROR");
+            type = HTTP_INTERNAL_SERVER_ERROR;
+        }
+        else {
+            return;
+        }
     }
 
     if (type == DONE) {

diff --git a/server/config.c b/server/config.c
index d10f4afe26458ec5d985cab8261f6171a94e496f..9747c284a63a22f4d3b610bef80af6216d796124 100644 (file)
--- a/server/config.c
+++ b/server/config.c
@@ -378,6 +378,7 @@ AP_CORE_DECLARE(int) ap_invoke_handler(request_rec *r)
             "handler \"%s\" not found for: %s", r->handler, r->filename);
     }
     if ((result != OK) && (result != DONE) && (result != DECLINED)
+        && (result != AP_FILTER_ERROR)
         && !ap_is_HTTP_VALID_RESPONSE(result)) {
         /* If a module is deliberately returning something else
          * (request_rec in non-HTTP or proprietary extension?)