You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Chris Darroch <ch...@pearsoncmg.com> on 2012/06/30 01:25:56 UTC
mod_fcgid support for ap_meets_conditions()
Hi --
After many years sailing far away from httpd shores, I happen to be
making a visit again and might have a trickle of mod_fcgid patches
to share. Because it's been a while I thought I'd post the first
one or two for RTC instead of just committing them directly.
This one is just adding comments, mostly, plus one change to
align the guts of fcgid_bridge.c with mod_cgid.c where they both use
ap_scan_script_header_err_*() functions.
It looks to me like fcgid_bridge.c drew its inspiration from
mod_cgid.c around 2004. A patch was then submitted to the original
maintainer and committed which supports If-Modified-Since/Last-Modified
by not returning early when the HTTP code < 400; see also the ChangeLog
for version 1.04:
http://svn.apache.org/viewvc?view=revision&revision=753526
Over in mod_cgid.c, though, the code went a different direction,
circa 2007:
http://svn.apache.org/viewvc?view=revision&revision=541990
This patch tries to bring mod_fcgid.c back as close as possible
to mod_cgid.c and adds a lot of comments (for my own sanity and
perhaps that of others trying to trace the sequence of events).
If this looks OK, I'd love to commit and move on to the next
patch ... assuming I can keep paddling near the shore and avoid those
dangerous undertows! Thanks and cheers,
Chris.
--
GPG Key ID: 088335A9
GPG Key Fingerprint: 86CD 3297 7493 75BC F820 6715 F54F E648 0883 35A9
==========================================================================
--- mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c (revision 1354646)
+++ mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c (working copy)
@@ -316,7 +316,42 @@
/* 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 HTTP_OK;
+ }
+
return cond_status;
}
@@ -336,6 +371,13 @@
*/
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;
}
@@ -343,10 +385,18 @@
/* 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 (role == FCGI_RESPONDER
&& (rv = ap_pass_brigade(r->output_filters,
brigade_stdout)) != APR_SUCCESS) {
Re: mod_fcgid support for ap_meets_conditions()
Posted by Chris Darroch <ch...@pearsoncmg.com>.
William A. Rowe Jr. wrote:
> On 6/29/2012 6:25 PM, Chris Darroch wrote:
>>
>> If this looks OK, I'd love to commit and move on to the next
>> patch ... assuming I can keep paddling near the shore and avoid those
>> dangerous undertows! Thanks and cheers,
>
> Looks sensible to me.
Thanks, Bill -- committed with minor edits in r1362533.
Chris.
--
GPG Key ID: 088335A9
GPG Key Fingerprint: 86CD 3297 7493 75BC F820 6715 F54F E648 0883 35A9
Re: mod_fcgid support for ap_meets_conditions()
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/29/2012 6:25 PM, Chris Darroch wrote:
>
> If this looks OK, I'd love to commit and move on to the next
> patch ... assuming I can keep paddling near the shore and avoid those
> dangerous undertows! Thanks and cheers,
Looks sensible to me.