Merge r541926, r541990 from trunk:

PR#39710 - badly broken errordocuments for CGI

We've just had another duplicate report of this on bugzilla.
We've got a simple patch, and people asking WTF is going on
with inaction.  Noone seems clear on why the patch shouldn't
be applied (http://marc.info/?l=apache-httpd-dev&m=117760311129386&w=2).

* Prevent running through the error stack by returning OK and setting r->status
  accordingly if ret is HTTP_NOT_MODIFIED as this breaks mod_cache validating a
  stale entity.

Reviewed by: jim

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

diff --git a/modules/generators/mod_cgi.c b/modules/generators/mod_cgi.c
index 133e18eb7550ff31e603f90d4fa8e53b2bc9cfd8..f9980f9d2a662077ba58eacc97aacd1238f8ef21 100644 (file)
--- a/modules/generators/mod_cgi.c
+++ b/modules/generators/mod_cgi.c
@@ -926,14 +926,30 @@ static int cgi_handler(request_rec *r)
         if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
             ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
 
-            /* Set our status. */
-            r->status = ret;
-
-            /* Pass EOS bucket down the filter chain. */
-            apr_brigade_cleanup(bb);
-            b = apr_bucket_eos_create(c->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(bb, b);
-            ap_pass_brigade(r->output_filters, bb);
+            /*
+             * ret could be HTTP_NOT_MODIFIED in the case that the CGI script
+             * does not set an explicit status and ap_meets_conditions, which
+             * is called by ap_scan_script_header_err_brigade, detects that
+             * the conditions of the requests are met and the response is
+             * not modified.
+             * In this case set r->status and return OK in order to prevent
+             * running through the error processing stack as this would
+             * break with mod_cache, if the conditions had been set by
+             * mod_cache itself to validate a stale entity.
+             * BTW: We circumvent the error processing stack anyway if the
+             * CGI script set an explicit status code (whatever it is) and
+             * the only possible values for ret here are:
+             *
+             * HTTP_NOT_MODIFIED          (set by ap_meets_conditions)
+             * HTTP_PRECONDITION_FAILED   (set by ap_meets_conditions)
+             * HTTP_INTERNAL_SERVER_ERROR (if something went wrong during the
+             * processing of the response of the CGI script, e.g broken headers
+             * or a crashed CGI process).
+             */
+            if (ret == HTTP_NOT_MODIFIED) {
+                r->status = ret;
+                return OK;
+            }
 
             return ret;
         }

diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c
index 2dc3eca12176ee90542d4e1c734608078bd72810..db5c504809d335009e2a5501828c2feb2a497fb6 100644 (file)
--- a/modules/generators/mod_cgid.c
+++ b/modules/generators/mod_cgid.c
@@ -1467,7 +1467,34 @@ static int cgid_handler(request_rec *r)
         APR_BRIGADE_INSERT_TAIL(bb, b);
 
         if ((ret = ap_scan_script_header_err_brigade(r, bb, sbuf))) {
-            return log_script(r, conf, ret, dbuf, sbuf, bb, NULL);
+            ret = log_script(r, conf, ret, dbuf, sbuf, bb, NULL);
+
+            /*
+             * ret could be HTTP_NOT_MODIFIED in the case that the CGI script
+             * does not set an explicit status and ap_meets_conditions, which
+             * is called by ap_scan_script_header_err_brigade, detects that
+             * the conditions of the requests are met and the response is
+             * not modified.
+             * In this case set r->status and return OK in order to prevent
+             * running through the error processing stack as this would
+             * break with mod_cache, if the conditions had been set by
+             * mod_cache itself to validate a stale entity.
+             * BTW: We circumvent the error processing stack anyway if the
+             * CGI script set an explicit status code (whatever it is) and
+             * the only possible values for ret here are:
+             *
+             * HTTP_NOT_MODIFIED          (set by ap_meets_conditions)
+             * HTTP_PRECONDITION_FAILED   (set by ap_meets_conditions)
+             * HTTP_INTERNAL_SERVER_ERROR (if something went wrong during the
+             * processing of the response of the CGI script, e.g broken headers
+             * or a crashed CGI process).
+             */
+            if (ret == HTTP_NOT_MODIFIED) {
+                r->status = ret;
+                return OK;
+            }
+
+            return ret;
         }
 
         location = apr_table_get(r->headers_out, "Location");