httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From chr...@apache.org
Subject svn commit: r1362533 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID modules/fcgid/fcgid_bridge.c
Date Tue, 17 Jul 2012 15:06:41 GMT
Author: chrisd
Date: Tue Jul 17 15:06:41 2012
New Revision: 1362533

URL: http://svn.apache.org/viewvc?rev=1362533&view=rev
Log:
Conform script response parsing with mod_cgid and ensure no response body
is sent when ap_meets_conditions() determines that request conditions
are met.

The design of fcgid_bridge.c follows mod_cgid.c in its use of the
ap_scan_script_header_err_*() functions.  A patch submitted to the original
maintainer and committed in r753526 supports conditional requests simply by
not returning early when ap_meets_conditions() returns < 400.  However, the
response body is still sent, unlike mod_cgid's handling of this case.

In r541990 mod_cgid's handling of the 304 return code was altered and
key comments added.  This patch realigns mod_fcgid with mod_cgid and
adds further comments regarding mod_fcgid's output filter.

Modified:
    httpd/mod_fcgid/trunk/CHANGES-FCGID
    httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c

Modified: httpd/mod_fcgid/trunk/CHANGES-FCGID
URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/CHANGES-FCGID?rev=1362533&r1=1362532&r2=1362533&view=diff
==============================================================================
--- httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] (original)
+++ httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] Tue Jul 17 15:06:41 2012
@@ -10,6 +10,10 @@ Changes with mod_fcgid 2.3.8
      improved comments and logging, to reduce code duplication and
      simplify future maintenance.  [Chris Darroch]
 
+  *) Conform script response parsing with mod_cgid and ensure no response
+     body is sent when ap_meets_conditions() determines that request
+     conditions are met.  [Chris Darroch]
+
 Changes with mod_fcgid 2.3.7
 
   *) Introduce FcgidWin32PreventOrphans directive on Windows to use OS

Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c
URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c?rev=1362533&r1=1362532&r2=1362533&view=diff
==============================================================================
--- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c (original)
+++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c Tue Jul 17 15:06:41 2012
@@ -316,7 +316,42 @@ handle_request_ipc(request_rec *r, int r
     /* Check the script header first; return immediately on error. */
     if ((cond_status =
          ap_scan_script_header_err_core(r, sbuf, getsfunc_fcgid_BRIGADE,
-                                        brigade_stdout)) >= 400) {
+                                        brigade_stdout))) {
+        /*
+         * cond_status could be HTTP_NOT_MODIFIED in the case that the FCGI
+         * 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
+         * FCGI script set an explicit status code (whatever it is) and
+         * the only possible values for cond_status here are:
+         *
+         * HTTP_NOT_MODIFIED          (set by ap_meets_conditions)
+         * HTTP_PRECONDITION_FAILED   (set by ap_meets_conditions)
+         * HTTP_GATEWAY_TIME_OUT      (script timed out, returned no headers)
+         * HTTP_INTERNAL_SERVER_ERROR (if something went wrong during the
+         * processing of the response of the FCGI script, e.g broken headers
+         * or a crashed FCGI process).
+         */
+        if (cond_status == HTTP_NOT_MODIFIED) {
+            /* We need to remove our fcgid_filter before returning this
+             * status and code; otherwise, when ap_process_async_request()
+             * invokes ap_finalize_request_protocol() and that calls
+             * ap_pass_brigade(), fcgid_filter notices it has an empty
+             * brigade and returns without calling ap_pass_brigade() itself,
+             * which incorrectly circumvents the standard output filters.
+             */
+            ap_remove_output_filter(r->output_filters);
+
+            r->status = cond_status;
+            return OK;
+        }
+
         return cond_status;
     }
 
@@ -340,17 +375,32 @@ handle_request_ipc(request_rec *r, int r
          */
         apr_table_unset(r->headers_in, "Content-Length");
 
+        /* Setting this Location header value causes handle_request() to
+         * invoke ap_internal_redirect_handler(); that calls
+         * internal_internal_redirect() which sets the new sub-request's
+         * r->output_filters back to r->proto_output_filters before
+         * running the sub-request's handler.  Because we return here
+         * without invoking ap_pass_brigade(), our fcgid_filter is ignored.
+         */
         *location_ptr = location;
-        return HTTP_OK;
+        return OK;
     }
     else if (location && r->status == 200) {
         /* XX Note that if a script wants to produce its own Redirect
          * body, it now has to explicitly *say* "Status: 302"
          */
+
+        /* This return code causes ap_process_async_request() to invoke
+         * ap_die(); that calls ap_send_error_response(), which resets
+         * r->output_filters back to r->proto_output_filters, thus removing
+         * our fcgid_filter from the output chain before making a final call
+         * to ap_finalize_request_protocol(), which passes the brigade to
+         * the standard output filters.
+         */
         return HTTP_MOVED_TEMPORARILY;
     }
 
-    /* Now pass to output filter */
+    /* Now pass any remaining response body data to output filters */
     if ((rv = ap_pass_brigade(r->output_filters,
                               brigade_stdout)) != APR_SUCCESS) {
         if (!APR_STATUS_IS_ECONNABORTED(rv)) {
@@ -469,7 +519,7 @@ handle_request(request_rec * r, int role
         ap_internal_redirect_handler(location, r);
     }
 
-    /* Retrun condition status */
+    /* Return condition status */
     return cond_status;
 }
 



Mime
View raw message