You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2019/10/04 09:20:34 UTC
svn commit: r1867968 - in /httpd/httpd/trunk/modules/generators:
cgi_common.h mod_cgi.c mod_cgid.c
Author: jorton
Date: Fri Oct 4 09:20:33 2019
New Revision: 1867968
URL: http://svn.apache.org/viewvc?rev=1867968&view=rev
Log:
Move common (and near-identical) code for CGI response output handling
to cgi_common.h; the diff between the modules for this code was as
follows:
https://people.apache.org/~jorton/mod_cgi-to-cgid-handler.diff
Change from previous: mod_cgi will now explicitly discard output when
returning HTTP_MOVED_TEMPORARILY for relative redirects (should not be
functionally different), TRACE1 logging of ap_pass_brigade failures
for mod_cgid is dropped.
* modules/generators/cgi_common.h (cgi_handle_response): New function,
factored out from mod_cgid.
(discard_script_output): Copied function from mod_cgi/d unchanged.
* modules/generator/mod_cgid.c (cgid_handler),
modules/generator/mod_cgi.c (cgi_handler): Use cgi_handle_response.
Modified:
httpd/httpd/trunk/modules/generators/cgi_common.h
httpd/httpd/trunk/modules/generators/mod_cgi.c
httpd/httpd/trunk/modules/generators/mod_cgid.c
Modified: httpd/httpd/trunk/modules/generators/cgi_common.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/cgi_common.h?rev=1867968&r1=1867967&r2=1867968&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/cgi_common.h (original)
+++ httpd/httpd/trunk/modules/generators/cgi_common.h Fri Oct 4 09:20:33 2019
@@ -27,6 +27,27 @@
#include "httpd.h"
#include "util_filter.h"
+static void discard_script_output(apr_bucket_brigade *bb)
+{
+ apr_bucket *e;
+ const char *buf;
+ apr_size_t len;
+ apr_status_t rv;
+
+ for (e = APR_BRIGADE_FIRST(bb);
+ e != APR_BRIGADE_SENTINEL(bb);
+ e = APR_BUCKET_NEXT(e))
+ {
+ if (APR_BUCKET_IS_EOS(e)) {
+ break;
+ }
+ rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ);
+ if (rv != APR_SUCCESS) {
+ break;
+ }
+ }
+}
+
/* A CGI bucket type is needed to catch any output to stderr from the
* script; see PR 22030. */
static const apr_bucket_type_t bucket_type_cgi;
@@ -218,3 +239,122 @@ static const apr_bucket_type_t bucket_ty
apr_bucket_copy_notimpl
};
+/* Handle the CGI response output, having set up the brigade with the
+ * CGI or PIPE bucket as appropriate. */
+static int cgi_handle_response(request_rec *r, int nph, apr_bucket_brigade *bb,
+ apr_interval_time_t timeout, cgi_server_conf *conf,
+ char *logdata, apr_file_t *script_err)
+{
+ apr_status_t rv;
+
+ /* Handle script return... */
+ if (!nph) {
+ const char *location;
+ char sbuf[MAX_STRING_LEN];
+ int ret;
+
+ if ((ret = ap_scan_script_header_err_brigade_ex(r, bb, sbuf,
+ APLOG_MODULE_INDEX)))
+ {
+ ret = log_script(r, conf, ret, logdata, sbuf, bb, script_err);
+
+ /*
+ * 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");
+
+ if (location && r->status == 200) {
+ /* For a redirect whether internal or not, discard any
+ * remaining stdout from the script, and log any remaining
+ * stderr output, as normal. */
+ discard_script_output(bb);
+ apr_brigade_destroy(bb);
+
+ if (script_err) {
+ apr_file_pipe_timeout_set(script_err, timeout);
+ log_script_err(r, script_err);
+ }
+ }
+
+ if (location && location[0] == '/' && r->status == 200) {
+ /* This redirect needs to be a GET no matter what the original
+ * method was.
+ */
+ r->method = "GET";
+ r->method_number = M_GET;
+
+ /* We already read the message body (if any), so don't allow
+ * the redirected request to think it has one. We can ignore
+ * Transfer-Encoding, since we used REQUEST_CHUNKED_ERROR.
+ */
+ apr_table_unset(r->headers_in, "Content-Length");
+
+ ap_internal_redirect_handler(location, r);
+ return OK;
+ }
+ else if (location && r->status == 200) {
+ /* XXX: Note that if a script wants to produce its own Redirect
+ * body, it now has to explicitly *say* "Status: 302"
+ */
+ discard_script_output(bb);
+ apr_brigade_destroy(bb);
+ return HTTP_MOVED_TEMPORARILY;
+ }
+
+ rv = ap_pass_brigade(r->output_filters, bb);
+ }
+ else /* nph */ {
+ struct ap_filter_t *cur;
+
+ /* get rid of all filters up through protocol... since we
+ * haven't parsed off the headers, there is no way they can
+ * work
+ */
+
+ cur = r->proto_output_filters;
+ while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) {
+ cur = cur->next;
+ }
+ r->output_filters = r->proto_output_filters = cur;
+
+ rv = ap_pass_brigade(r->output_filters, bb);
+ }
+
+ /* don't soak up script output if errors occurred writing it
+ * out... otherwise, we prolong the life of the script when the
+ * connection drops or we stopped sending output for some other
+ * reason */
+ if (script_err && rv == APR_SUCCESS && !r->connection->aborted) {
+ apr_file_pipe_timeout_set(script_err, timeout);
+ log_script_err(r, script_err);
+ }
+
+ if (script_err) apr_file_close(script_err);
+
+ return OK; /* NOT r->status, even if it has changed. */
+}
Modified: httpd/httpd/trunk/modules/generators/mod_cgi.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgi.c?rev=1867968&r1=1867967&r2=1867968&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_cgi.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgi.c Fri Oct 4 09:20:33 2019
@@ -568,27 +568,6 @@ static apr_status_t default_build_comman
return APR_SUCCESS;
}
-static void discard_script_output(apr_bucket_brigade *bb)
-{
- apr_bucket *e;
- const char *buf;
- apr_size_t len;
- apr_status_t rv;
-
- for (e = APR_BRIGADE_FIRST(bb);
- e != APR_BRIGADE_SENTINEL(bb);
- e = APR_BUCKET_NEXT(e))
- {
- if (APR_BUCKET_IS_EOS(e)) {
- break;
- }
- rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ);
- if (rv != APR_SUCCESS) {
- break;
- }
- }
-}
-
#if APR_FILES_AS_SOCKETS
#include "cgi_common.h"
#endif
@@ -782,111 +761,7 @@ static int cgi_handler(request_rec *r)
b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, b);
- /* Handle script return... */
- if (!nph) {
- const char *location;
- char sbuf[MAX_STRING_LEN];
- int ret;
-
- if ((ret = ap_scan_script_header_err_brigade_ex(r, bb, sbuf,
- APLOG_MODULE_INDEX)))
- {
- ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
-
- /*
- * 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");
-
- if (location && r->status == 200) {
- /* For a redirect whether internal or not, discard any
- * remaining stdout from the script, and log any remaining
- * stderr output, as normal. */
- discard_script_output(bb);
- apr_brigade_destroy(bb);
- apr_file_pipe_timeout_set(script_err, timeout);
- log_script_err(r, script_err);
- }
-
- if (location && location[0] == '/' && r->status == 200) {
- /* This redirect needs to be a GET no matter what the original
- * method was.
- */
- r->method = "GET";
- r->method_number = M_GET;
-
- /* We already read the message body (if any), so don't allow
- * the redirected request to think it has one. We can ignore
- * Transfer-Encoding, since we used REQUEST_CHUNKED_ERROR.
- */
- apr_table_unset(r->headers_in, "Content-Length");
-
- ap_internal_redirect_handler(location, r);
- return OK;
- }
- else if (location && r->status == 200) {
- /* XXX: Note that if a script wants to produce its own Redirect
- * body, it now has to explicitly *say* "Status: 302"
- */
- return HTTP_MOVED_TEMPORARILY;
- }
-
- rv = ap_pass_brigade(r->output_filters, bb);
- }
- else /* nph */ {
- struct ap_filter_t *cur;
-
- /* get rid of all filters up through protocol... since we
- * haven't parsed off the headers, there is no way they can
- * work
- */
-
- cur = r->proto_output_filters;
- while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) {
- cur = cur->next;
- }
- r->output_filters = r->proto_output_filters = cur;
-
- rv = ap_pass_brigade(r->output_filters, bb);
- }
-
- /* don't soak up script output if errors occurred writing it
- * out... otherwise, we prolong the life of the script when the
- * connection drops or we stopped sending output for some other
- * reason */
- if (rv == APR_SUCCESS && !r->connection->aborted) {
- apr_file_pipe_timeout_set(script_err, timeout);
- log_script_err(r, script_err);
- }
-
- apr_file_close(script_err);
-
- return OK; /* NOT r->status, even if it has changed. */
+ return cgi_handle_response(r, nph, bb, timeout, conf, dbuf, script_err);
}
/*============================================================================
Modified: httpd/httpd/trunk/modules/generators/mod_cgid.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgid.c?rev=1867968&r1=1867967&r2=1867968&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_cgid.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgid.c Fri Oct 4 09:20:33 2019
@@ -1351,6 +1351,7 @@ static int log_script(request_rec *r, cg
#ifdef HAVE_CGID_FDPASSING
/* Pull in CGI bucket implementation. */
+#define cgi_server_conf cgid_server_conf
#include "cgi_common.h"
#endif
@@ -1420,27 +1421,6 @@ static int connect_to_daemon(int *sdptr,
return OK;
}
-static void discard_script_output(apr_bucket_brigade *bb)
-{
- apr_bucket *e;
- const char *buf;
- apr_size_t len;
- apr_status_t rv;
-
- for (e = APR_BRIGADE_FIRST(bb);
- e != APR_BRIGADE_SENTINEL(bb);
- e = APR_BUCKET_NEXT(e))
- {
- if (APR_BUCKET_IS_EOS(e)) {
- break;
- }
- rv = apr_bucket_read(e, &buf, &len, APR_BLOCK_READ);
- if (rv != APR_SUCCESS) {
- break;
- }
- }
-}
-
/****************************************************************
*
* Actual cgid handling...
@@ -1779,117 +1759,7 @@ static int cgid_handler(request_rec *r)
b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, b);
- /* Handle script return... */
- if (!nph) {
- const char *location;
- char sbuf[MAX_STRING_LEN];
- int ret;
-
- if ((ret = ap_scan_script_header_err_brigade_ex(r, bb, sbuf,
- APLOG_MODULE_INDEX)))
- {
- ret = log_script(r, conf, ret, dbuf, sbuf, bb, script_err);
-
- /*
- * 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");
-
- if (location && location[0] == '/' && r->status == 200) {
-
- /* Soak up all the script output */
- discard_script_output(bb);
- apr_brigade_destroy(bb);
- if (script_err) {
- apr_file_pipe_timeout_set(script_err, timeout);
- log_script_err(r, script_err);
- }
-
- /* This redirect needs to be a GET no matter what the original
- * method was.
- */
- r->method = "GET";
- r->method_number = M_GET;
-
- /* We already read the message body (if any), so don't allow
- * the redirected request to think it has one. We can ignore
- * Transfer-Encoding, since we used REQUEST_CHUNKED_ERROR.
- */
- apr_table_unset(r->headers_in, "Content-Length");
-
- ap_internal_redirect_handler(location, r);
- return OK;
- }
- else if (location && r->status == 200) {
- /* XXX: Note that if a script wants to produce its own Redirect
- * body, it now has to explicitly *say* "Status: 302"
- */
- discard_script_output(bb);
- apr_brigade_destroy(bb);
- return HTTP_MOVED_TEMPORARILY;
- }
-
- rv = ap_pass_brigade(r->output_filters, bb);
- if (rv != APR_SUCCESS) {
- ap_log_rerror(APLOG_MARK, APLOG_TRACE1, rv, r,
- "Failed to flush CGI output to client");
- }
- }
-
- if (nph) {
- struct ap_filter_t *cur;
-
- /* get rid of all filters up through protocol... since we
- * haven't parsed off the headers, there is no way they can
- * work
- */
-
- cur = r->proto_output_filters;
- while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) {
- cur = cur->next;
- }
- r->output_filters = r->proto_output_filters = cur;
-
- rv = ap_pass_brigade(r->output_filters, bb);
- }
-
- /* don't soak up script output if errors occurred writing it
- * out... otherwise, we prolong the life of the script when the
- * connection drops or we stopped sending output for some other
- * reason */
- if (script_err && rv == APR_SUCCESS && !r->connection->aborted) {
- apr_file_pipe_timeout_set(script_err, timeout);
- log_script_err(r, script_err);
- }
-
- if (script_err) apr_file_close(script_err);
-
- return OK; /* NOT r->status, even if it has changed. */
+ return cgi_handle_response(r, nph, bb, timeout, conf, dbuf, script_err);
}