You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by RCHAPACH Rochester <rc...@us.ibm.com> on 2001/04/17 23:13:55 UTC
[PATCH] mod_include incorrect function table offset
>From a co-worker of mine...
Rob Simonson
simo@us.ibm.com
-------------------------------------------------
Rob,
I have described here the error I have encountered, and the fix I have done
in order to get mod_include to work correctly. The problem I encountered
was in the send_parsed_content subroutine in mod_include. The code as is
currently obtained from the Apache Software Foundation does not determine
the correct offset into the function table that is set up at post_config
time by mod_include. The include handler functions are placed into a hash
table, using the function name(i.e. 'echo', 'config', ' include', ...) and
the length of the function, plus the null terminator. The input to the
hash routine must be the same when the functions are retrieved in order for
function to be handled. The code calls get_combined_directive to get the
directive into the ctx control block. This routine will set the directive,
which is the entire field, from the start sequence to the end sequence,
into the ctx control block, and set this directive length in the ctx.
These fields are passed into the hash routine to find the include function
name. Since the function name and length are not the same as the directive
and directive length in the ctx, the hash table entry for the include
function is not found.
I have changed the following code in order to get the directive and
directive length passed to the hash routine to find the proper include
function.
Karen L. Richner
--------------------------------------------------
***************
*** 2352,2357 ****
--- 2352,2359 ----
apr_bucket *dptr = APR_BRIGADE_FIRST(*bb);
apr_bucket *tmp_dptr;
apr_bucket_brigade *tag_and_after;
+ char *c;
+ int dirlen;
int ret;
if (r->args) { /* add QUERY stuff to env cause it ain't
yet */
***************
*** 2508,2523 ****
* it NULL terminated (and include the NULL in the length)
for proper
* hash matching.
*/
! for (tmp_i = 0; tmp_i < ctx->directive_length; tmp_i++) {
ctx->combined_tag[tmp_i] =
apr_tolower(ctx->combined_tag[tmp_i]);
}
! ctx->combined_tag[ctx->directive_length] = '\0';
! ctx->curr_tag_pos =
&ctx->combined_tag[ctx->directive_length+1];
handle_func =
(int (*)(include_ctx_t *, apr_bucket_brigade **,
request_rec *,
ap_filter_t *, apr_bucket *, apr_bucket **))
! apr_hash_get(include_hash, ctx->combined_tag,
ctx->directive_length+1);
if (handle_func != NULL) {
ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
}
--- 2510,2533 ----
* it NULL terminated (and include the NULL in the length)
for proper
* hash matching.
*/
!
! c = ctx->combined_tag;
! while ((*c != '\0') && (*c != ' ')) {
! c++;
! }
! dirlen = c - ctx->combined_tag;
!
! for (tmp_i = 0; tmp_i < dirlen; tmp_i++) {
ctx->combined_tag[tmp_i] =
apr_tolower(ctx->combined_tag[tmp_i]);
}
! ctx->combined_tag[dirlen] = '\0';
! ctx->curr_tag_pos = &ctx->combined_tag[dirlen+1];
+
handle_func =
(int (*)(include_ctx_t *, apr_bucket_brigade **,
request_rec *,
ap_filter_t *, apr_bucket *, apr_bucket **))
! apr_hash_get(include_hash, ctx->combined_tag, dirlen+1);
if (handle_func != NULL) {
ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
}
Re: [PATCH] mod_include incorrect function table offset
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 07:57 PM 04/17/2001, you wrote:
>1. Unified diffs are easier than context diffs to review (diff -u
>blah rather than diff -C3 blah). We document for folks to use
>context diffs because unified diff is not available on all OSes but
>most of us prefer reviewing unified diffs. So post unified diffs if
>possible.
This is what we currently document:
http://dev.apache.org/patches.html
We prefer that patches be submitted in unified diff format:
diff -u file-old.c file.c
but that isn't available on all platforms. If your platform doesn't
support unified diffs, please use a context diff instead:
diff -C3 file-old.c file.c
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"
Re: [PATCH] mod_include incorrect function table offset
Posted by Bill Stoddard <bi...@wstoddard.com>.
Hey Rob,
Just a couple of comments on patch posting protocol (but not the code)...
1. Unified diffs are easier than context diffs to review (diff -u blah rather than diff -C3 blah).
We document for folks to use context diffs because unified diff is not available on all OSes but
most of us prefer reviewing unified diffs. So post unified diffs if possible.
2. Your mail program folded some of the longer lines which will prevent this patch from applying
cleanly w/o tweaking.
Keeping these two points in mind will help getting your patch reviewed and committed.
Thanks for the fix.
Bill
----- Original Message -----
From: "RCHAPACH Rochester" <rc...@us.ibm.com>
To: <ne...@apache.org>
Sent: Tuesday, April 17, 2001 5:13 PM
Subject: [PATCH] mod_include incorrect function table offset
> >From a co-worker of mine...
>
> Rob Simonson
> simo@us.ibm.com
>
> -------------------------------------------------
>
>
> Rob,
>
> I have described here the error I have encountered, and the fix I have done
> in order to get mod_include to work correctly. The problem I encountered
> was in the send_parsed_content subroutine in mod_include. The code as is
> currently obtained from the Apache Software Foundation does not determine
> the correct offset into the function table that is set up at post_config
> time by mod_include. The include handler functions are placed into a hash
> table, using the function name(i.e. 'echo', 'config', ' include', ...) and
> the length of the function, plus the null terminator. The input to the
> hash routine must be the same when the functions are retrieved in order for
> function to be handled. The code calls get_combined_directive to get the
> directive into the ctx control block. This routine will set the directive,
> which is the entire field, from the start sequence to the end sequence,
> into the ctx control block, and set this directive length in the ctx.
> These fields are passed into the hash routine to find the include function
> name. Since the function name and length are not the same as the directive
> and directive length in the ctx, the hash table entry for the include
> function is not found.
>
> I have changed the following code in order to get the directive and
> directive length passed to the hash routine to find the proper include
> function.
>
>
> Karen L. Richner
>
> --------------------------------------------------
>
> ***************
> *** 2352,2357 ****
> --- 2352,2359 ----
> apr_bucket *dptr = APR_BRIGADE_FIRST(*bb);
> apr_bucket *tmp_dptr;
> apr_bucket_brigade *tag_and_after;
> + char *c;
> + int dirlen;
> int ret;
>
> if (r->args) { /* add QUERY stuff to env cause it ain't
> yet */
> ***************
> *** 2508,2523 ****
> * it NULL terminated (and include the NULL in the length)
> for proper
> * hash matching.
> */
> ! for (tmp_i = 0; tmp_i < ctx->directive_length; tmp_i++) {
> ctx->combined_tag[tmp_i] =
> apr_tolower(ctx->combined_tag[tmp_i]);
> }
> ! ctx->combined_tag[ctx->directive_length] = '\0';
> ! ctx->curr_tag_pos =
> &ctx->combined_tag[ctx->directive_length+1];
>
> handle_func =
> (int (*)(include_ctx_t *, apr_bucket_brigade **,
> request_rec *,
> ap_filter_t *, apr_bucket *, apr_bucket **))
> ! apr_hash_get(include_hash, ctx->combined_tag,
> ctx->directive_length+1);
> if (handle_func != NULL) {
> ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
> }
> --- 2510,2533 ----
> * it NULL terminated (and include the NULL in the length)
> for proper
> * hash matching.
> */
> !
> ! c = ctx->combined_tag;
> ! while ((*c != '\0') && (*c != ' ')) {
> ! c++;
> ! }
> ! dirlen = c - ctx->combined_tag;
> !
> ! for (tmp_i = 0; tmp_i < dirlen; tmp_i++) {
> ctx->combined_tag[tmp_i] =
> apr_tolower(ctx->combined_tag[tmp_i]);
> }
> ! ctx->combined_tag[dirlen] = '\0';
> ! ctx->curr_tag_pos = &ctx->combined_tag[dirlen+1];
>
> +
> handle_func =
> (int (*)(include_ctx_t *, apr_bucket_brigade **,
> request_rec *,
> ap_filter_t *, apr_bucket *, apr_bucket **))
> ! apr_hash_get(include_hash, ctx->combined_tag, dirlen+1);
> if (handle_func != NULL) {
> ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
> }
>
>
>
Re: [PATCH] mod_include incorrect function table offset
Posted by "Paul J. Reder" <re...@raleigh.ibm.com>.
Rob and Karen,
I am a strong -1 on this patch. The code in find_end_sequence computes the
ctx->directive_length. That is the length of the SSI directive only, not the
whole SSI tag (which is stored in ctx->tag_length. This allows the full tag
to be parsed once, marking important indexes as it goes so that I don't need
to do things like recount the size of the directive. If there is any problem
in the code, I would prefer to fix it in the find_end_sequence function.
This is working fine on all of the platforms that I have been testing on. What
system are you developing/testing on? My guess is that either you have a
c library problem on your system, or the SSI tag that you are testing with has
some parse error in it.
Could you tell me what system you are testing this on and show me the SSI tag
that you are testing with? I would also like to see the error log associated
with trying to process it.
Thanks,
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [PATCH] mod_include incorrect function table offset
Posted by "Paul J. Reder" <re...@raleigh.ibm.com>.
Bill Stoddard wrote:
>
> Spent a bit of time looking at this and I think the fix is in the wrong place but I'm not enough of
> a mod_include expert to say for sure.
>
> First, mod_include generally works so I presume this is a case where a tag spans buckets. In that
> case, ctx->directive_length is wrong. Wouldn;t the best solution be to fix the content of
> ctx->directive_length (perhaps in get_combined_directive)
>
> Paul, opinions?
>
> Bill
Sorry for the extra travel delay. I will take a look at this today.
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein
Re: [PATCH] mod_include incorrect function table offset
Posted by Bill Stoddard <bi...@wstoddard.com>.
Spent a bit of time looking at this and I think the fix is in the wrong place but I'm not enough of
a mod_include expert to say for sure.
First, mod_include generally works so I presume this is a case where a tag spans buckets. In that
case, ctx->directive_length is wrong. Wouldn;t the best solution be to fix the content of
ctx->directive_length (perhaps in get_combined_directive)
Paul, opinions?
Bill
> >From a co-worker of mine...
>
> Rob Simonson
> simo@us.ibm.com
>
> -------------------------------------------------
>
>
> Rob,
>
> I have described here the error I have encountered, and the fix I have done
> in order to get mod_include to work correctly. The problem I encountered
> was in the send_parsed_content subroutine in mod_include. The code as is
> currently obtained from the Apache Software Foundation does not determine
> the correct offset into the function table that is set up at post_config
> time by mod_include. The include handler functions are placed into a hash
> table, using the function name(i.e. 'echo', 'config', ' include', ...) and
> the length of the function, plus the null terminator. The input to the
> hash routine must be the same when the functions are retrieved in order for
> function to be handled. The code calls get_combined_directive to get the
> directive into the ctx control block. This routine will set the directive,
> which is the entire field, from the start sequence to the end sequence,
> into the ctx control block, and set this directive length in the ctx.
> These fields are passed into the hash routine to find the include function
> name. Since the function name and length are not the same as the directive
> and directive length in the ctx, the hash table entry for the include
> function is not found.
>
> I have changed the following code in order to get the directive and
> directive length passed to the hash routine to find the proper include
> function.
>
>
> Karen L. Richner
>
> --------------------------------------------------
>
> ***************
> *** 2352,2357 ****
> --- 2352,2359 ----
> apr_bucket *dptr = APR_BRIGADE_FIRST(*bb);
> apr_bucket *tmp_dptr;
> apr_bucket_brigade *tag_and_after;
> + char *c;
> + int dirlen;
> int ret;
>
> if (r->args) { /* add QUERY stuff to env cause it ain't
> yet */
> ***************
> *** 2508,2523 ****
> * it NULL terminated (and include the NULL in the length)
> for proper
> * hash matching.
> */
> ! for (tmp_i = 0; tmp_i < ctx->directive_length; tmp_i++) {
> ctx->combined_tag[tmp_i] =
> apr_tolower(ctx->combined_tag[tmp_i]);
> }
> ! ctx->combined_tag[ctx->directive_length] = '\0';
> ! ctx->curr_tag_pos =
> &ctx->combined_tag[ctx->directive_length+1];
>
> handle_func =
> (int (*)(include_ctx_t *, apr_bucket_brigade **,
> request_rec *,
> ap_filter_t *, apr_bucket *, apr_bucket **))
> ! apr_hash_get(include_hash, ctx->combined_tag,
> ctx->directive_length+1);
> if (handle_func != NULL) {
> ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
> }
> --- 2510,2533 ----
> * it NULL terminated (and include the NULL in the length)
> for proper
> * hash matching.
> */
> !
> ! c = ctx->combined_tag;
> ! while ((*c != '\0') && (*c != ' ')) {
> ! c++;
> ! }
> ! dirlen = c - ctx->combined_tag;
> !
> ! for (tmp_i = 0; tmp_i < dirlen; tmp_i++) {
> ctx->combined_tag[tmp_i] =
> apr_tolower(ctx->combined_tag[tmp_i]);
> }
> ! ctx->combined_tag[dirlen] = '\0';
> ! ctx->curr_tag_pos = &ctx->combined_tag[dirlen+1];
>
> +
> handle_func =
> (int (*)(include_ctx_t *, apr_bucket_brigade **,
> request_rec *,
> ap_filter_t *, apr_bucket *, apr_bucket **))
> ! apr_hash_get(include_hash, ctx->combined_tag, dirlen+1);
> if (handle_func != NULL) {
> ret = (*handle_func)(ctx, bb, r, f, dptr, &content_head);
> }
>
>
>