You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by re...@apache.org on 2001/08/23 17:45:50 UTC
cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c
rederpj 01/08/23 08:45:50
Modified: . CHANGES
modules/filters mod_include.c mod_include.h
modules/generators mod_cgi.c mod_cgid.c
Log:
Fix for mod_include. Ryan's patch to check error
codes put a return in the wrong place. Also, the
include handler return code wasn't being checked.
I don't like macros with returns, so I converted
SPLIT_AND_PASS_PRETAG_BUCKETS into a function.
Revision Changes Path
1.324 +7 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.323
retrieving revision 1.324
diff -u -r1.323 -r1.324
--- CHANGES 2001/08/22 20:03:47 1.323
+++ CHANGES 2001/08/23 15:45:49 1.324
@@ -1,4 +1,11 @@
Changes with Apache 2.0.25-dev
+ *) Fix for mod_include. Ryan's patch to check error
+ codes put a return in the wrong place. Also, the
+ include handler return code wasn't being checked.
+ I don't like macros with returns, so I converted
+ SPLIT_AND_PASS_PRETAG_BUCKETS into a function.
+ [Paul J. Reder <re...@raleigh.ibm.com>]
+
*) fix segv in mod_mime if no AddTypes are configured
[John Sterling <st...@covalent.net>]
1.129 +35 -4 httpd-2.0/modules/filters/mod_include.c
Index: mod_include.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
retrieving revision 1.128
retrieving revision 1.129
diff -u -r1.128 -r1.129
--- mod_include.c 2001/08/23 15:35:50 1.128
+++ mod_include.c 2001/08/23 15:45:49 1.129
@@ -98,6 +98,29 @@
#define BYTE_COUNT_THRESHOLD AP_MIN_BYTES_TO_WRITE
+/* This function is used to split the brigade at the beginning of
+ * the tag and forward the pretag buckets before any substitution
+ * work is performed on the tag. This maintains proper ordering.
+ */
+static int split_and_pass_pretag_buckets(apr_bucket_brigade **brgd,
+ include_ctx_t *cntxt,
+ ap_filter_t *next)
+{
+ apr_bucket_brigade *tag_plus;
+ int rv;
+
+ if ((APR_BRIGADE_EMPTY(cntxt->ssi_tag_brigade)) &&
+ (cntxt->head_start_bucket != NULL)) {
+ tag_plus = apr_brigade_split(*brgd, cntxt->head_start_bucket);
+ rv = ap_pass_brigade(next, *brgd);
+ cntxt->bytes_parsed = 0;
+ *brgd = tag_plus;
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ }
+}
+
/* ------------------------ Environment function -------------------------- */
/* XXX: could use ap_table_overlap here */
@@ -857,7 +880,10 @@
if (!error_fmt) {
int rv;
- SPLIT_AND_PASS_PRETAG_BUCKETS(*bb, ctx, f->next);
+ rv = split_and_pass_pretag_buckets(bb, ctx, f->next);
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
if ((rv = ap_run_sub_req(rr))) {
if (APR_STATUS_IS_EPIPE(rv)) {
@@ -2341,7 +2367,6 @@
apr_bucket *dptr = APR_BRIGADE_FIRST(*bb);
apr_bucket *tmp_dptr;
apr_bucket_brigade *tag_and_after;
- int ret;
apr_status_t rv;
if (r->args) { /* add QUERY stuff to env cause it ain't yet */
@@ -2435,7 +2460,10 @@
*bb = tag_and_after;
}
else if (ctx->bytes_parsed >= BYTE_COUNT_THRESHOLD) {
- SPLIT_AND_PASS_PRETAG_BUCKETS(*bb, ctx, f->next);
+ rv = split_and_pass_pretag_buckets(bb, ctx, f->next);
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
}
}
else {
@@ -2506,7 +2534,10 @@
ctx->combined_tag,
ctx->directive_length+1);
if (handle_func != NULL) {
- ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
+ rv = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
+ if ((rv != 0) && (rv != 1)) {
+ return (rv);
+ }
}
else {
ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
1.20 +0 -16 httpd-2.0/modules/filters/mod_include.h
Index: mod_include.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.h,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -r1.19 -r1.20
--- mod_include.h 2001/08/03 03:41:51 1.19
+++ mod_include.h 2001/08/23 15:45:49 1.20
@@ -183,22 +183,6 @@
} \
}
-#define SPLIT_AND_PASS_PRETAG_BUCKETS(brgd, cntxt, next) \
-if ((APR_BRIGADE_EMPTY(cntxt->ssi_tag_brigade)) && \
- (cntxt->head_start_bucket != NULL)) { \
- apr_bucket_brigade *tag_plus; \
- int rv; \
- \
- tag_plus = apr_brigade_split(brgd, cntxt->head_start_bucket); \
- rv = ap_pass_brigade(next, brgd); \
- if (rv != APR_SUCCESS) { \
- return rv; \
- } \
- cntxt->bytes_parsed = 0; \
- brgd = tag_plus; \
-}
-
-
typedef int (include_handler_fn_t)(include_ctx_t *ctx, apr_bucket_brigade **bb,
request_rec *r, ap_filter_t *f, apr_bucket *head_ptr,
apr_bucket **inserted_head);
1.100 +33 -2 httpd-2.0/modules/generators/mod_cgi.c
Index: mod_cgi.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgi.c,v
retrieving revision 1.99
retrieving revision 1.100
diff -u -r1.99 -r1.100
--- mod_cgi.c 2001/08/11 04:04:12 1.99
+++ mod_cgi.c 2001/08/23 15:45:49 1.100
@@ -137,6 +137,29 @@
int bufbytes;
} cgi_server_conf;
+/* This function is used to split the brigade at the beginning of
+ * the tag and forward the pretag buckets before any substitution
+ * work is performed on the tag. This maintains proper ordering.
+ */
+static int split_and_pass_pretag_buckets(apr_bucket_brigade **brgd,
+ include_ctx_t *cntxt,
+ ap_filter_t *next)
+{
+ apr_bucket_brigade *tag_plus;
+ int rv;
+
+ if ((APR_BRIGADE_EMPTY(cntxt->ssi_tag_brigade)) &&
+ (cntxt->head_start_bucket != NULL)) {
+ tag_plus = apr_brigade_split(*brgd, cntxt->head_start_bucket);
+ rv = ap_pass_brigade(next, *brgd);
+ cntxt->bytes_parsed = 0;
+ *brgd = tag_plus;
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ }
+}
+
static void *create_cgi_config(apr_pool_t *p, server_rec *s)
{
cgi_server_conf *c =
@@ -440,7 +463,10 @@
else {
procnew = apr_pcalloc(p, sizeof(*procnew));
if (e_info->prog_type == RUN_AS_SSI) {
- SPLIT_AND_PASS_PRETAG_BUCKETS(*(e_info->bb), e_info->ctx, e_info->next);
+ rc = split_and_pass_pretag_buckets(e_info->bb, e_info->ctx, e_info->next);
+ if (rc != APR_SUCCESS) {
+ return rc;
+ }
}
rc = ap_os_create_privileged_process(r, procnew, command, argv, env, procattr, p);
@@ -896,6 +922,7 @@
char *tag = NULL;
char *tag_val = NULL;
char *file = r->filename;
+ int retval;
apr_bucket *tmp_buck;
char parsed_string[MAX_STRING_LEN];
@@ -928,7 +955,11 @@
}
else if (!strcmp(tag, "cgi")) {
cgi_pfn_ps(r, tag_val, parsed_string, sizeof(parsed_string), 0);
- SPLIT_AND_PASS_PRETAG_BUCKETS(*bb, ctx, f->next);
+ retval = split_and_pass_pretag_buckets(bb, ctx, f->next);
+ if (retval != APR_SUCCESS) {
+ return retval;
+ }
+
if (include_cgi(parsed_string, r, f->next, head_ptr, inserted_head) == -1) {
ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
"invalid CGI ref \"%s\" in %s", tag_val, file);
1.93 +33 -2 httpd-2.0/modules/generators/mod_cgid.c
Index: mod_cgid.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgid.c,v
retrieving revision 1.92
retrieving revision 1.93
diff -u -r1.92 -r1.93
--- mod_cgid.c 2001/08/11 04:04:12 1.92
+++ mod_cgid.c 2001/08/23 15:45:49 1.93
@@ -169,6 +169,29 @@
int bufbytes;
} cgid_server_conf;
+/* This function is used to split the brigade at the beginning of
+ * the tag and forward the pretag buckets before any substitution
+ * work is performed on the tag. This maintains proper ordering.
+ */
+static int split_and_pass_pretag_buckets(apr_bucket_brigade **brgd,
+ include_ctx_t *cntxt,
+ ap_filter_t *next)
+{
+ apr_bucket_brigade *tag_plus;
+ int rv;
+
+ if ((APR_BRIGADE_EMPTY(cntxt->ssi_tag_brigade)) &&
+ (cntxt->head_start_bucket != NULL)) {
+ tag_plus = apr_brigade_split(*brgd, cntxt->head_start_bucket);
+ rv = ap_pass_brigade(next, *brgd);
+ cntxt->bytes_parsed = 0;
+ *brgd = tag_plus;
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ }
+}
+
/* If a request includes query info in the URL (stuff after "?"), and
* the query info does not contain "=" (indicative of a FORM submission),
* then this routine is called to create the argument list to be passed
@@ -1174,7 +1197,10 @@
"unable to connect to cgi daemon");
}
- SPLIT_AND_PASS_PRETAG_BUCKETS(*bb, ctx, f->next);
+ retval = split_and_pass_pretag_buckets(bb, ctx, f->next);
+ if (retval != APR_SUCCESS) {
+ return retval;
+ }
send_req(sd, r, command, env, SSI_REQ);
@@ -1235,6 +1261,7 @@
char *file = r->filename;
apr_bucket *tmp_buck;
char parsed_string[MAX_STRING_LEN];
+ int retval;
*inserted_head = NULL;
if (ctx->flags & FLAG_PRINTING) {
@@ -1266,7 +1293,11 @@
}
else if (!strcmp(tag, "cgi")) {
cgid_pfn_ps(r, tag_val, parsed_string, sizeof(parsed_string), 0);
- SPLIT_AND_PASS_PRETAG_BUCKETS(*bb, ctx, f->next);
+ retval = split_and_pass_pretag_buckets(bb, ctx, f->next);
+ if (retval != APR_SUCCESS) {
+ return retval;
+ }
+
if (include_cgi(parsed_string, r, f->next, head_ptr, inserted_head) == -1) {
ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, r,
"invalid CGI ref \"%s\" in %s", tag_val, file);
Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c
Posted by Ryan Bloom <rb...@covalent.net>.
-1.
You just took a small macro, and turned it into a function, and copied that
function into three different places in the source tree. That's just asking
for future bugs.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c
Posted by Ryan Bloom <rb...@covalent.net>.
-1.
You just took a small macro, and turned it into a function, and copied that
function into three different places in the source tree. That's just asking
for future bugs.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c
Posted by Jeff Trawick <tr...@attglobal.net>.
rederpj@apache.org writes:
> Index: mod_include.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
> retrieving revision 1.128
> retrieving revision 1.129
> diff -u -r1.128 -r1.129
> --- mod_include.c 2001/08/23 15:35:50 1.128
> +++ mod_include.c 2001/08/23 15:45:49 1.129
> @@ -98,6 +98,29 @@
>
> #define BYTE_COUNT_THRESHOLD AP_MIN_BYTES_TO_WRITE
>
> +/* This function is used to split the brigade at the beginning of
> + * the tag and forward the pretag buckets before any substitution
> + * work is performed on the tag. This maintains proper ordering.
> + */
> +static int split_and_pass_pretag_buckets(apr_bucket_brigade **brgd,
should be "apr_status_t" instead of "int", right?
> + include_ctx_t *cntxt,
> + ap_filter_t *next)
> +{
> + apr_bucket_brigade *tag_plus;
> + int rv;
apr_status_t rv;
> +
> + if ((APR_BRIGADE_EMPTY(cntxt->ssi_tag_brigade)) &&
> + (cntxt->head_start_bucket != NULL)) {
> + tag_plus = apr_brigade_split(*brgd, cntxt->head_start_bucket);
> + rv = ap_pass_brigade(next, *brgd);
> + cntxt->bytes_parsed = 0;
> + *brgd = tag_plus;
> + if (rv != APR_SUCCESS) {
> + return rv;
> + }
> + }
besides not compiling cleanly this can't work dependably... you need
"return APR_SUCCESS;" here at the end of the function...
and then there is the issue Ryan raised...
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...