You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ryan Bloom <rb...@covalent.net> on 2001/08/07 01:09:01 UTC
Showstopper!
One of our QA guys at Covalent has just let me know that SSI's are broken in 2.0. Basically,
if, else, elif, and that family of SSI's don't seem to do what they are supposed to do.
BTW, he caught these using the perl test-framework in httpd-test. The test cases he was using
can be found in httpd-test/perl-framework/t/htdocs/modules/include/if*.shtml.
All of these tests work correctly under Apache 1.3.
For example:
<!--#if expr="\"1\" = \"1\""-->
pass
<!--#endif -->
<!--#if expr="\"1\" = \"2\""-->
fail
<!--#endif -->
Doesn't work correctly.
Ryan
_____________________________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
-----------------------------------------------------------------------------
Re: Showstopper!
Posted by Cliff Woolley <cl...@yahoo.com>.
On Mon, 6 Aug 2001, Ryan Bloom wrote:
> One of our QA guys at Covalent has just let me know that SSI's are
> broken in 2.0. Basically, if, else, elif, and that family of SSI's
> don't seem to do what they are supposed to do.
>
> BTW, he caught these using the perl test-framework in httpd-test.
> The test cases he was using can be found in
> httpd-test/perl-framework/t/htdocs/modules/include/if*.shtml.
>
> All of these tests work correctly under Apache 1.3.
>
> For example:
>
> <!--#if expr="\"1\" = \"1\""-->
> pass
> <!--#endif -->
> <!--#if expr="\"1\" = \"2\""-->
> fail
> <!--#endif -->
>
> Doesn't work correctly.
I've found the problem. ap_ssi_get_tag_and_value() is not correctly
re-null-terminating the string after it gets rid of the backslashes in the
expr. So where it should be this:
"1" = "1"
it thinks it's this:
"1" = "1""1\"
Since these two are clearly not equal, the #if, #elif, #else tag
handlers themselves are actually doing the right thing.
If you change the files to look like this:
<!--#if expr="1 = 1"-->
pass
<!--#endif -->
<!--#if expr="1 = 2"-->
fail
<!--#endif -->
Then all the tests pass.
Patch on the way.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: mod_include problems (was Re: Showstopper!)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Cliff Woolley" <cl...@yahoo.com>
Sent: Monday, August 06, 2001 9:46 PM
> On Mon, 6 Aug 2001, William A. Rowe, Jr. wrote:
>
> > Sure looks like that's the solution ... except ...
> >
> > it points out another possible (?) bug, that the <file> will later
> > disallow symlinks. The only reason I suspect this will work, is that
> > we redo all directory_walks, file_walks, and location_walks just a bit
> > later if the per_dir_config changes.
>
> Hmmm... will have to ponder that some more. But it does fix the segfault
> for now, at least.
>
> > I'd suggest this is safe (in both the _file AND _dirent flavors) so go
> > ahead and patch :)
>
> I just went back and checked _dirent, and it turns out that it did not
> exhibit this problem... rnew->per_dir_config = r->per_dir_config was
> already in the proposed spot.
Grump... my bad, I'm sure :(
Re: mod_include problems (was Re: Showstopper!)
Posted by Cliff Woolley <cl...@yahoo.com>.
On Mon, 6 Aug 2001, William A. Rowe, Jr. wrote:
> Sure looks like that's the solution ... except ...
>
> it points out another possible (?) bug, that the <file> will later
> disallow symlinks. The only reason I suspect this will work, is that
> we redo all directory_walks, file_walks, and location_walks just a bit
> later if the per_dir_config changes.
Hmmm... will have to ponder that some more. But it does fix the segfault
for now, at least.
> I'd suggest this is safe (in both the _file AND _dirent flavors) so go
> ahead and patch :)
I just went back and checked _dirent, and it turns out that it did not
exhibit this problem... rnew->per_dir_config = r->per_dir_config was
already in the proposed spot.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: mod_include problems (was Re: Showstopper!)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Sure looks like that's the solution ... except ...
it points out another possible (?) bug, that the <file> will later disallow symlinks.
The only reason I suspect this will work, is that we redo all directory_walks,
file_walks, and location_walks just a bit later if the per_dir_config changes.
I'd suggest this is safe (in both the _file AND _dirent flavors) so go ahead and
patch :)
Another observation, total apropo of nothing. The syntax to enable includes is
now suggested as;
<FilesMatch "\.shtml">
SetOutputFilter Includes
</FilesMatch>
or whatever, correct?
This will always cause per_dir_config to change. Ergo, we will always redo the
directory walk when we modify filters on a <Files> or <FilesMatch> basis.
Just thought you would like to ponder that :)
Bill
----- Original Message -----
From: "Cliff Woolley" <cl...@yahoo.com>
To: <ne...@apache.org>
Cc: "Ryan Bloom" <rb...@covalent.net>; "William Rowe" <wr...@rowe-clan.net>
Sent: Monday, August 06, 2001 9:27 PM
Subject: Re: mod_include problems (was Re: Showstopper!)
> On Mon, 6 Aug 2001, Cliff Woolley wrote:
>
>
> > It's not just that... include1.shtml causes a segfault, which I'm still
> > investigating.
>
> With optimizations turned off, my backtrace looks more like this:
>
> #0 0x080abf73 in ap_get_module_config (cv=0x0, m=0x8121d60)
> at util_debug.c:101
> #1 0x080b089c in ap_allow_options (r=0x8196044) at core.c:500
> #2 0x080b71b6 in ap_sub_req_lookup_file (new_file=0xbfffaeb0
> "inc-two.shtml",
> r=0x8194034, next_filter=0x8194f9c) at request.c:1517
> #3 0x08062828 in handle_include (ctx=0x81c618c, bb=0xbfffd384,
> r=0x8194034,
> f=0x8194f84, head_ptr=0x81cb1d8, inserted_head=0xbfffd318)
> at mod_include.c:784
> ...
>
> ap_sub_req_lookup_file() is calling ap_allow_options() before
> rnew->per_dir_config is set; ap_allow_options() is therefore calling
> ap_get_module_config with a null cv pointer. Here's the relevant snippet
> from ap_sub_req_lookup_file in request.c at line 1513, which appears to
> have been broken by revision 1.17 about five days ago:
>
>
> if (ap_allow_options(rnew) & OPT_SYM_LINKS) {
> if (((rv = apr_stat(&rnew->finfo, rnew->filename,
> APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
> && (rv != APR_INCOMPLETE))
> rnew->finfo.filetype = 0;
> }
> else
> if (((rv = apr_lstat(&rnew->finfo, rnew->filename,
> APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
> && (rv != APR_INCOMPLETE))
> rnew->finfo.filetype = 0;
>
> if ((res = check_safe_file(rnew))) {
> rnew->status = res;
> return rnew;
> }
>
> rnew->per_dir_config = r->per_dir_config;
>
>
> Is it as easy as moving that last line up above the call to
> ap_allow_options()?
>
> --Cliff
>
> --------------------------------------------------------------
> Cliff Woolley
> cliffwoolley@yahoo.com
> Charlottesville, VA
>
>
>
Re: mod_include problems (was Re: Showstopper!)
Posted by Cliff Woolley <cl...@yahoo.com>.
On Mon, 6 Aug 2001, Cliff Woolley wrote:
> It's not just that... include1.shtml causes a segfault, which I'm still
> investigating.
With optimizations turned off, my backtrace looks more like this:
#0 0x080abf73 in ap_get_module_config (cv=0x0, m=0x8121d60)
at util_debug.c:101
#1 0x080b089c in ap_allow_options (r=0x8196044) at core.c:500
#2 0x080b71b6 in ap_sub_req_lookup_file (new_file=0xbfffaeb0
"inc-two.shtml",
r=0x8194034, next_filter=0x8194f9c) at request.c:1517
#3 0x08062828 in handle_include (ctx=0x81c618c, bb=0xbfffd384,
r=0x8194034,
f=0x8194f84, head_ptr=0x81cb1d8, inserted_head=0xbfffd318)
at mod_include.c:784
...
ap_sub_req_lookup_file() is calling ap_allow_options() before
rnew->per_dir_config is set; ap_allow_options() is therefore calling
ap_get_module_config with a null cv pointer. Here's the relevant snippet
from ap_sub_req_lookup_file in request.c at line 1513, which appears to
have been broken by revision 1.17 about five days ago:
if (ap_allow_options(rnew) & OPT_SYM_LINKS) {
if (((rv = apr_stat(&rnew->finfo, rnew->filename,
APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
&& (rv != APR_INCOMPLETE))
rnew->finfo.filetype = 0;
}
else
if (((rv = apr_lstat(&rnew->finfo, rnew->filename,
APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
&& (rv != APR_INCOMPLETE))
rnew->finfo.filetype = 0;
if ((res = check_safe_file(rnew))) {
rnew->status = res;
return rnew;
}
rnew->per_dir_config = r->per_dir_config;
Is it as easy as moving that last line up above the call to
ap_allow_options()?
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: mod_include problems (was Re: Showstopper!)
Posted by Cliff Woolley <cl...@yahoo.com>.
On Mon, 6 Aug 2001, Ryan Bloom wrote:
> One of our QA guys at Covalent has just let me know that SSI's are
> broken in 2.0. Basically, if, else, elif, and that family of SSI's
> don't seem to do what they are supposed to do.
It's not just that... include1.shtml causes a segfault, which I'm still
investigating.
FYI, the file contains this:
jcw5q@paisley:/tmp/cliff/htdocs/testssi$ cat include1.shtml
<!--#include file="inc-two.shtml"-->
include.shtml body
jcw5q@paisley:/tmp/cliff/htdocs/testssi$ cat inc-two.shtml
inc-two.shtml body
Here's the gdb output:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 9187)]
ap_get_module_config (cv=0x8170624, m=0x8170bf4) at util_debug.c:101
101 return ((void **)cv)[m->module_index];
(gdb) bt
#0 ap_get_module_config (cv=0x8170624, m=0x8170bf4) at util_debug.c:101
#1 0x080a5bb2 in ap_sub_req_lookup_file (new_file=0xbfffaf20
"inc-two.shtml",
r=0x816e614, next_filter=0x816f5fc) at request.c:1517
#2 0x08061ef4 in handle_include (ctx=0x81a076c, bb=0xbfffd3d4,
r=0x816e614,
f=0x816f5e4, head_ptr=0x81a57b8, inserted_head=0xbfffcf74)
at mod_include.c:784
#3 0x080648d5 in send_parsed_content (bb=0xbfffd3d4, r=0x816e614,
f=0x816f5e4)
at mod_include.c:2494
#4 0x08064e87 in includes_filter (f=0x816f5e4, b=0x816f714)
at mod_include.c:2754
#5 0x0809d2ea in ap_pass_brigade (next=0x816f5e4, bb=0x816f714)
at util_filter.c:242
#6 0x080a35ea in default_handler (r=0x816e614) at core.c:3002
#7 0x080934d6 in ap_run_handler (r=0x816e614) at config.c:185
#8 0x0809395b in ap_invoke_handler (r=0x816e614) at config.c:344
#9 0x0806c71d in process_request_internal (r=0x816e614) at
http_request.c:378
#10 0x0806ca9c in ap_process_request (r=0x816e614) at http_request.c:444
#11 0x08068861 in ap_process_http_connection (c=0x816a6c4) at
http_core.c:287
#12 0x0809bdd6 in ap_run_process_connection (c=0x816a6c4) at
connection.c:82
#13 0x08091e5e in child_main (child_num_arg=0) at prefork.c:814
#14 0x08091fc0 in make_child (s=0x8107354, slot=0) at prefork.c:850
#15 0x080920d1 in startup_children (number_to_start=5) at prefork.c:924
#16 0x08092853 in ap_mpm_run (_pconf=0x8105fdc, plog=0x813c18c,
s=0x8107354)
at prefork.c:1139
#17 0x08097520 in main (argc=2, argv=0xbffff6b4) at main.c:427
#18 0x400c4177 in __libc_start_main (main=0x8097250 <main>, argc=2,
ubp_av=0xbffff6b4, init=0x805eaa4 <_init>, fini=0x80d8f20 <_fini>,
rtld_fini=0x4000e184 <_dl_fini>, stack_end=0xbffff6ac)
at ../sysdeps/generic/libc-start.c:129
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA