You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@hyperreal.org on 1999/05/13 20:26:00 UTC
cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
coar 99/05/13 11:25:59
Modified: htdocs/manual/mod mod_autoindex.html
src CHANGES
src/modules/standard mod_autoindex.c mod_include.c
Log:
Someone finally stood up and made the ReadmeName and HeaderName
features use subrequests. Not only that, but they can be
parsed for SSIs too!
PR: 1574, 3026, 3529, 3569, 4256
Submitted by: Raymond S Brand <rs...@rsbx.net>
Reviewed by: Ken Coar
Revision Changes Path
1.31 +57 -17 apache-1.3/htdocs/manual/mod/mod_autoindex.html
Index: mod_autoindex.html
===================================================================
RCS file: /home/cvs/apache-1.3/htdocs/manual/mod/mod_autoindex.html,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -r1.30 -r1.31
--- mod_autoindex.html 1999/01/04 14:35:53 1.30
+++ mod_autoindex.html 1999/05/13 18:25:45 1.31
@@ -426,20 +426,53 @@
<A
HREF="directive-dict.html#Module"
REL="Help"
-><STRONG>Module:</STRONG></A> mod_autoindex<P>
+><STRONG>Module:</STRONG></A> mod_autoindex
+ <BR>
+ <A
+ HREF="directive-dict.html#Compatibility"
+ REL="Help"
+ ><STRONG>Compatibility:</STRONG></A> some features only available after
+ 1.3.6; see text
+<P>
The HeaderName directive sets the name of the file that will be inserted
at the top of the index listing. <EM>Filename</EM> is the name of the file
-to include, and is taken to be relative to the directory being indexed.
-The server first attempts to include <EM>filename</EM><CODE>.html</CODE>
-as an HTML document, otherwise it will include <EM>filename</EM> as plain
-text. Example:
+to include.
+</P>
+<BLOCKQUOTE><STRONG>Apache 1.3.6 and earlier:</STRONG>
+The module first attempts to include <EM>filename</EM><CODE>.html</CODE>
+as an HTML document, otherwise it will try to include <EM>filename</EM> as
+plain text. <EM>Filename</EM> is treated as a filesystem path relative
+to the directory being indexed. In no case is SSI processing done.
+Example:
<BLOCKQUOTE><CODE>HeaderName HEADER</CODE></BLOCKQUOTE>
when indexing the directory <CODE>/web</CODE>, the server will first look for
the HTML file <CODE>/web/HEADER.html</CODE> and include it if found, otherwise
it will include the plain text file <CODE>/web/HEADER</CODE>, if it exists.
-
-<P>See also <A HREF="#readmename">ReadmeName</A>.<P><HR>
+</BLOCKQUOTE>
+<BLOCKQUOTE><STRONG>Apache versions after 1.3.6:</STRONG>
+<EM>Filename</EM> is treated as a URI path relative to the one used
+to access the directory being indexed, and must resolve to a document
+with a major content type of "<SAMP>text</SAMP>" (<EM>e.g.</EM>,
+<SAMP>text/html</SAMP>, <SAMP>text/plain</SAMP>, <EM>etc.</EM>).
+This means that <EM>filename</EM> may refer to a CGI script if the
+script's actual file type (as opposed to its output) is marked as
+<SAMP>text/html</SAMP> such as with a directive like:
+<PRE>
+ AddType text/html .cgi
+</PRE>
+<A HREF="../content-negotiation.html">Content negotiation</A>
+will be performed if the <SAMP>MultiViews</SAMP>
+<A HREF="core.html#options">option</A> is enabled.
+If <EM>filename</EM> resolves to a static <SAMP>text/html</SAMP> document
+(not a CGI script) and the
+<SAMP>Includes</SAMP> <A HREF="core.html#options">option</A> is enabled,
+the file will be processed for server-side includes (see the
+<A HREF="mod_include.html"><SAMP>mod_include</SAMP></A> documentation).
+</BLOCKQUOTE>
+<P>
+See also <A HREF="#readmename">ReadmeName</A>.
+<P><HR>
<H2><A NAME="indexignore">IndexIgnore</A></H2>
<!--%plaintext <?INDEX {\tt IndexIgnore} directive> -->
@@ -753,19 +786,26 @@
<A
HREF="directive-dict.html#Module"
REL="Help"
-><STRONG>Module:</STRONG></A> mod_autoindex<P>
+><STRONG>Module:</STRONG></A> mod_autoindex
+ <BR>
+ <A
+ HREF="directive-dict.html#Compatibility"
+ REL="Help"
+ ><STRONG>Compatibility:</STRONG></A> some features only available after
+ 1.3.6; see text
+<P>
The ReadmeName directive sets the name of the file that will be appended
to the end of the index listing. <EM>Filename</EM> is the name of the file
-to include, and is taken to be relative to the directory being indexed.
-The server first attempts to include <EM>filename</EM><CODE>.html</CODE>
-as an HTML document, otherwise it will include <EM>filename</EM> as plain
-text. Example:
-<BLOCKQUOTE><CODE>ReadmeName README</CODE></BLOCKQUOTE>
-when indexing the directory <CODE>/web</CODE>, the server will first look for
-the HTML file <CODE>/web/README.html</CODE> and include it if found, otherwise
-it will include the plain text file <CODE>/web/README</CODE>, if it exists.
-
+to include, and is taken to be relative to the location being indexed.
+</P>
+<BLOCKQUOTE>
+<STRONG>The <EM>filename</EM> argument is treated as a stub filename
+in Apache 1.3.6 and earlier, and as a relative URI in later versions.
+Details of how it is handled may be found under the description of
+the <A HREF="#headername">HeaderName</A> directive, which uses the
+same mechanism and changed at the same time as ReadmeName.</STRONG>
+</BLOCKQUOTE>
<P>See also <A HREF="#headername">HeaderName</A>.<P>
1.1354 +8 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1353
retrieving revision 1.1354
diff -u -r1.1353 -r1.1354
--- CHANGES 1999/05/12 16:50:40 1.1353
+++ CHANGES 1999/05/13 18:25:50 1.1354
@@ -1,5 +1,13 @@
Changes with Apache 1.3.7
+ *) Support for server-parsed and multiview-determined ReadmeName and
+ HeaderName files in mod_autoindex. Removed the restriction on
+ "/"s in ReadmeName and HeaderName directives since the *sub_req*
+ routines will deal with the access issues. (It's now possible to
+ have {site|group|project|customer|...} wide readmes and headers.)
+ [Raymond S Brand <rs...@rsbx.net>, Ken Coar] PR#1574, 3026, 3529,
+ 3569, 4256
+
*) When stat() fails, don't assume anything about the contents of
the struct stat. [Ed Korthof <ed...@bitmechanic.com>]
1.107 +208 -99 apache-1.3/src/modules/standard/mod_autoindex.c
Index: mod_autoindex.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_autoindex.c,v
retrieving revision 1.106
retrieving revision 1.107
diff -u -r1.106 -r1.107
--- mod_autoindex.c 1999/05/03 20:48:43 1.106
+++ mod_autoindex.c 1999/05/13 18:25:56 1.107
@@ -322,9 +322,6 @@
static const char *add_header(cmd_parms *cmd, void *d, char *name)
{
- if (strchr(name, '/')) {
- return "HeaderName cannot contain a /";
- }
push_item(((autoindex_config_rec *) d)->hdr_list, 0, NULL, cmd->path,
name);
return NULL;
@@ -332,9 +329,6 @@
static const char *add_readme(cmd_parms *cmd, void *d, char *name)
{
- if (strchr(name, '/')) {
- return "ReadmeName cannot contain a /";
- }
push_item(((autoindex_config_rec *) d)->rdme_list, 0, NULL, cmd->path,
name);
return NULL;
@@ -864,99 +858,223 @@
*/
/*
- * Look for the specified file, and pump it into the response stream if we
- * find it.
+ * Elements of the emitted document:
+ * Preamble
+ * Emitted unless SUPPRESS_PREAMBLE is set AND ap_run_sub_req
+ * succeeds for the (content_type == text/html) header file.
+ * Header file
+ * Emitted if found (and able).
+ * H1 tag line
+ * Emitted if a header file is NOT emitted.
+ * Directory stuff
+ * Always emitted.
+ * HR
+ * Emitted if FANCY_INDEXING is set.
+ * Readme file
+ * Emitted if found (and able).
+ * ServerSig
+ * Emitted if ServerSignature is not Off AND a readme file
+ * is NOT emitted.
+ * Postamble
+ * Emitted unless SUPPRESS_PREAMBLE is set AND ap_run_sub_req
+ * succeeds for the (content_type == text/html) readme file.
*/
-static int insert_readme(char *name, char *readme_fname, char *title,
- int hrule, int whichend, request_rec *r)
+
+
+/*
+ * emit a plain text file
+ */
+static void do_emit_plain(request_rec *r, FILE *f)
{
- char *fn;
- FILE *f;
- struct stat finfo;
- int plaintext = 0;
- request_rec *rr;
- autoindex_config_rec *cfg;
- int autoindex_opts;
+ char buf[IOBUFSIZE + 1];
+ int i, n, c, ch;
- cfg = (autoindex_config_rec *) ap_get_module_config(r->per_dir_config,
- &autoindex_module);
- autoindex_opts = cfg->opts;
- /* XXX: this is a load of crap, it needs to do a full sub_req_lookup_uri */
- fn = ap_make_full_path(r->pool, name, readme_fname);
- fn = ap_pstrcat(r->pool, fn, ".html", NULL);
- if (stat(fn, &finfo) == -1) {
- /* A brief fake multiviews search for README.html */
- fn[strlen(fn) - 5] = '\0';
- if (stat(fn, &finfo) == -1) {
- return 0;
- }
- plaintext = 1;
- if (hrule) {
- ap_rputs("<HR>\n", r);
+ ap_rputs("<PRE>\n", r);
+ while (!feof(f)) {
+ do {
+ n = fread(buf, sizeof(char), IOBUFSIZE, f);
+ }
+ while (n == -1 && ferror(f) && errno == EINTR);
+ if (n == -1 || n == 0) {
+ break;
+ }
+ buf[n] = '\0';
+ c = 0;
+ while (c < n) {
+ for (i = c; i < n; i++) {
+ if (buf[i] == '<' || buf[i] == '>' || buf[i] == '&') {
+ break;
+ }
+ }
+ ch = buf[i];
+ buf[i] = '\0';
+ ap_rputs(&buf[c], r);
+ if (ch == '<') {
+ ap_rputs("<", r);
+ }
+ else if (ch == '>') {
+ ap_rputs(">", r);
+ }
+ else if (ch == '&') {
+ ap_rputs("&", r);
+ }
+ c = i + 1;
}
- }
- else if (hrule) {
- ap_rputs("<HR>\n", r);
}
- /* XXX: when the above is rewritten properly, this necessary security
- * check will be redundant. -djg */
- rr = ap_sub_req_lookup_file(fn, r);
- if (rr->status != HTTP_OK) {
- ap_destroy_sub_req(rr);
- return 0;
- }
- ap_destroy_sub_req(rr);
- if (!(f = ap_pfopen(r->pool, fn, "r"))) {
- return 0;
+ ap_rputs("</PRE>\n", r);
+}
+
+/* See mod_include */
+#define SUB_REQ_STRING "Sub request to mod_include"
+#define PARENT_STRING "Parent request to mod_include"
+
+/*
+ * Handle the preamble through the H1 tag line, inclusive. Locate
+ * the file with a subrequests. Process text/html documents by actually
+ * running the subrequest; text/xxx documents get copied verbatim,
+ * and any other content type is ignored. This means that a non-text
+ * document (such as HEADER.gif) might get multiviewed as the result
+ * instead of a text document, meaning nothing will be displayed, but
+ * oh well.
+ */
+static void emit_head(request_rec *r, char *header_fname, int suppress_amble,
+ char *title)
+{
+ FILE *f;
+ request_rec *rr = NULL;
+ int emit_amble = 1;
+ int emit_H1 = 1;
+
+ /*
+ * If there's a header file, send a subrequest to look for it. If it's
+ * found and a text file, handle it -- otherwise fall through and
+ * pretend there's nothing there.
+ */
+ if ((header_fname != NULL)
+ && (rr = ap_sub_req_lookup_uri(header_fname, r))
+ && (rr->status == HTTP_OK)
+ && (rr->filename != NULL)
+ && S_ISREG(rr->finfo.st_mode)) {
+ /*
+ * Check for the two specific cases we allow: text/html and
+ * text/anything-else. The former is allowed to be processed for
+ * SSIs.
+ */
+ if (rr->content_type != NULL) {
+ if (!strcasecmp("text/html", rr->content_type)) {
+ /* Hope everything will work... */
+ emit_amble = 0;
+ emit_H1 = 0;
+
+ if (! suppress_amble) {
+ emit_preamble(r, title);
+ }
+ ap_table_add(r->notes, PARENT_STRING, "");
+ ap_table_add(rr->notes, SUB_REQ_STRING, "");
+ /*
+ * If there's a problem running the subrequest, display the
+ * preamble if we didn't do it before -- the header file
+ * didn't get displayed.
+ */
+ if (ap_run_sub_req(rr) != OK) {
+ /* It didn't work */
+ emit_amble = suppress_amble;
+ emit_H1 = 1;
+ }
+ }
+ else if (!strncasecmp("text/", rr->content_type, 5)) {
+ /*
+ * If we can open the file, prefix it with the preamble
+ * regardless; since we'll be sending a <PRE> block around
+ * the file's contents, any HTML header it had won't end up
+ * where it belongs.
+ */
+ if ((f = ap_pfopen(r->pool, rr->filename, "r")) != 0) {
+ emit_preamble(r, title);
+ emit_amble = 0;
+ do_emit_plain(r, f);
+ ap_pfclose(r->pool, f);
+ emit_H1 = 0;
+ }
+ }
+ }
}
- if ((whichend == FRONT_MATTER)
- && (!(autoindex_opts & SUPPRESS_PREAMBLE))) {
+
+ if (emit_amble) {
emit_preamble(r, title);
}
- if (!plaintext) {
- ap_send_fd(f, r);
+ if (emit_H1) {
+ ap_rvputs(r, "<H1>Index of ", title, "</H1>\n", NULL);
}
- else {
- char buf[IOBUFSIZE + 1];
- int i, n, c, ch;
- ap_rputs("<PRE>\n", r);
- while (!feof(f)) {
- do {
- n = fread(buf, sizeof(char), IOBUFSIZE, f);
- }
- while (n == -1 && ferror(f) && errno == EINTR);
- if (n == -1 || n == 0) {
- break;
- }
- buf[n] = '\0';
- c = 0;
- while (c < n) {
- for (i = c; i < n; i++) {
- if (buf[i] == '<' || buf[i] == '>' || buf[i] == '&') {
- break;
- }
- }
- ch = buf[i];
- buf[i] = '\0';
- ap_rputs(&buf[c], r);
- if (ch == '<') {
- ap_rputs("<", r);
- }
- else if (ch == '>') {
- ap_rputs(">", r);
+ if (rr != NULL) {
+ ap_destroy_sub_req(rr);
+ }
+}
+
+
+/*
+ * Handle the Readme file through the postamble, inclusive. Locate
+ * the file with a subrequests. Process text/html documents by actually
+ * running the subrequest; text/xxx documents get copied verbatim,
+ * and any other content type is ignored. This means that a non-text
+ * document (such as FOOTER.gif) might get multiviewed as the result
+ * instead of a text document, meaning nothing will be displayed, but
+ * oh well.
+ */
+static void emit_tail(request_rec *r, char *readme_fname, int suppress_amble)
+{
+ FILE *f;
+ request_rec *rr = NULL;
+ int suppress_post = 0;
+ int suppress_sig = 0;
+
+ /*
+ * If there's a readme file, send a subrequest to look for it. If it's
+ * found and a text file, handle it -- otherwise fall through and
+ * pretend there's nothing there.
+ */
+ if ((readme_fname != NULL)
+ && (rr = ap_sub_req_lookup_uri(readme_fname, r))
+ && (rr->status == HTTP_OK)
+ && (rr->filename != NULL)
+ && S_ISREG(rr->finfo.st_mode)) {
+ /*
+ * Check for the two specific cases we allow: text/html and
+ * text/anything-else. The former is allowed to be processed for
+ * SSIs.
+ */
+ if (rr->content_type != NULL) {
+ if (!strcasecmp("text/html", rr->content_type)) {
+ ap_table_add(r->notes, PARENT_STRING, "");
+ ap_table_add(rr->notes, SUB_REQ_STRING, "");
+ if (ap_run_sub_req(rr) == OK) {
+ /* worked... */
+ suppress_sig = 1;
+ suppress_post = suppress_amble;
}
- else if (ch == '&') {
- ap_rputs("&", r);
+ }
+ else if (!strncasecmp("text/", rr->content_type, 5)) {
+ /*
+ * If we can open the file, suppress the signature.
+ */
+ if ((f = ap_pfopen(r->pool, rr->filename, "r")) != 0) {
+ do_emit_plain(r, f);
+ ap_pfclose(r->pool, f);
+ suppress_sig = 1;
}
- c = i + 1;
}
}
+ }
+
+ if (!suppress_sig) {
+ ap_rputs(ap_psignature("", r), r);
}
- ap_pfclose(r->pool, f);
- if (plaintext) {
- ap_rputs("</PRE>\n", r);
+ if (!suppress_post) {
+ ap_rputs("</BODY></HTML>\n", r);
+ }
+ if (rr != NULL) {
+ ap_destroy_sub_req(rr);
}
- return 1;
}
@@ -1389,7 +1507,6 @@
int num_ent = 0, x;
struct ent *head, *p;
struct ent **ar = NULL;
- char *tmp;
const char *qstring;
int autoindex_opts = autoindex_conf->opts;
char keyid;
@@ -1419,12 +1536,8 @@
*title_endp-- = '\0';
}
- if ((!(tmp = find_header(autoindex_conf, r)))
- || (!(insert_readme(name, tmp, title_name, NO_HRULE, FRONT_MATTER, r)))
- ) {
- emit_preamble(r, title_name);
- ap_rvputs(r, "<H1>Index of ", title_name, "</H1>\n", NULL);
- }
+ emit_head(r, find_header(autoindex_conf, r),
+ autoindex_opts & SUPPRESS_PREAMBLE, title_name);
/*
* Figure out what sort of indexing (if any) we're supposed to use.
@@ -1489,15 +1602,11 @@
direction);
ap_pclosedir(r->pool, d);
- if ((tmp = find_readme(autoindex_conf, r))) {
- if (!insert_readme(name, tmp, "",
- ((autoindex_opts & FANCY_INDEXING) ? HRULE
- : NO_HRULE),
- END_MATTER, r)) {
- ap_rputs(ap_psignature("<HR>\n", r), r);
- }
+ if (autoindex_opts & FANCY_INDEXING) {
+ ap_rputs("<HR>\n", r);
}
- ap_rputs("</BODY></HTML>\n", r);
+ emit_tail(r, find_readme(autoindex_conf, r),
+ autoindex_opts & SUPPRESS_PREAMBLE);
ap_kill_timeout(r);
return 0;
1.115 +40 -9 apache-1.3/src/modules/standard/mod_include.c
Index: mod_include.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_include.c,v
retrieving revision 1.114
retrieving revision 1.115
diff -u -r1.114 -r1.115
--- mod_include.c 1999/05/05 17:46:07 1.114
+++ mod_include.c 1999/05/13 18:25:57 1.115
@@ -108,9 +108,6 @@
module MODULE_VAR_EXPORT includes_module;
-/* just need some arbitrary non-NULL pointer which can't also be a request_rec */
-#define NESTED_INCLUDE_MAGIC (&includes_module)
-
/* ------------------------ Environment function -------------------------- */
/* XXX: could use ap_table_overlap here */
@@ -746,9 +743,7 @@
}
/* destroy the sub request if it's not a nested include */
- if (rr != NULL
- && ap_get_module_config(rr->request_config, &includes_module)
- != NESTED_INCLUDE_MAGIC) {
+ if (rr != NULL) {
ap_destroy_sub_req(rr);
}
}
@@ -2376,6 +2371,41 @@
return OK;
}
+#define SUB_REQ_STRING "Sub request to mod_include"
+#define PARENT_STRING "Parent request to mod_include"
+
+ if (ap_table_get(r->notes, SUB_REQ_STRING) != NULL) {
+ request_rec *p = r->main;
+
+ /*
+ * The note is a flag to mod_include that this request is actually
+ * a subrequest from another module and that mod_include needs to
+ * treat it as if it's a subrequest from mod_include.
+ *
+ * HACK ALERT!
+ * There is no good way to pass the parent request_rec to mod_include.
+ * Tables only take string values and there is nowhere appropriate in
+ * in the request_rec that can safely be used.
+ *
+ * So we search up the chain of requests and redirects looking for
+ * the parent request.
+ */
+
+ while (p) {
+ if (ap_table_get(p->notes, PARENT_STRING) != NULL) {
+ /* Kludge --- See below */
+ ap_set_module_config(r->request_config, &includes_module, p);
+
+ ap_add_common_vars(p);
+ ap_add_cgi_vars(p);
+ add_include_vars(p, DEFAULT_TIME_FORMAT);
+ ap_table_unset(r->notes, SUB_REQ_STRING);
+ break;
+ }
+ p = (p->prev) ? p->prev : p->main;
+ }
+ }
+
if ((parent = ap_get_module_config(r->request_config, &includes_module))) {
/* Kludge --- for nested includes, we want to keep the subprocess
* environment of the base document (for compatibility); that means
@@ -2411,9 +2441,10 @@
send_parsed_content(f, r);
if (parent) {
- /* signify that the sub request should not be killed */
- ap_set_module_config(r->request_config, &includes_module,
- NESTED_INCLUDE_MAGIC);
+ /* Kludge --- Doing this allows the caller to safely destroy the
+ * sub_req
+ */
+ r->pool = ap_make_sub_pool(r->pool);
}
ap_kill_timeout(r);
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Posted by Dean Gaudet <dg...@arctic.org>.
On Fri, 14 May 1999, Raymond S Brand wrote:
> Dean Gaudet wrote:
> > /* Kludge --- Doing this allows the caller to safely destroy the
> > * sub_req
> > */
> > r->pool = ap_make_sub_pool(r->pool);
> >
> > Sorry, but that's just way off my bogosity meter.
>
> No more bogus than the current situation in mod_include.
Not really, the current situation is that it avoids destroying the
subrequest... which has a similar effect, as you point out with the patch
you just included. And further, it's local to mod_include.
> I fail to see how the above call sequence can happen. Can you flesh it out
> some? And are there any ap_*sub_req*() calls in the sequence?
It would require some crap in cleanups I'm sure, so I'll stop stretching.
But I'm certainly not going to ignore the 2.0 implications w.r.t.
context... you can ignore them if you want, but I can't.
> The sub_pool business is ONLY there so that a sub_req can be destroyed. I'm
> tired of this argument. Use the following;
I'm sorry you're tired of this argument, but I'm also of the opinion that
something is wrong with the subrequest mechanism and that there is an
abstraction we're missing... the stuff which is in mod_include already is
an example of something which isn't quite clean -- but is there to support
pre-existing functionality. What you're talking about is new
functionality.
Maybe the abstraction is as simple as a "root environment" or "global
environment". Or maybe we need to distinguish between a partial
subrequest and a full subrequest -- partial meaning "provides part of the
full response object". I don't know. I stopped hacking on this when I
got mod_include to work properly without memory corruption.
There's still something else I brought up -- I think this is backwards. I
don't think mod_autoindex should be bending over to behave like
mod_include. I think mod_autoindex should provide a way to be included by
mod_include.
Dean
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c
mod_include.c
Posted by Raymond S Brand <rs...@rsbx.net>.
Dean Gaudet wrote:
>
> On Thu, 13 May 1999, Raymond S Brand wrote:
>
> > Dean Gaudet wrote:
> > >
> > > ap_pool_join is an advisory debugging tool -- alloc.c does nothing to
> > > ensure that you preserve the lifetimes appropriately.
> >
> > I understand that. Please apply the patch to mod_include and look at
> > where the ap_make_sub_pool() I added is called.
>
> Yeah I saw that:
>
> /* Kludge --- Doing this allows the caller to safely destroy the
> * sub_req
> */
> r->pool = ap_make_sub_pool(r->pool);
>
> Sorry, but that's just way off my bogosity meter.
No more bogus than the current situation in mod_include. That code path
is only executed for sub_reqs created by mod_include (works fine, they're
about to be destroyed) or for sub_reqs created in another module and flagged
that they should be treated as a sub_req created in mod_inlcude (again
works fine, they're also about to be destroyed).
> Consider this sequence:
>
> in module foo: my_p = ap_make_sub_pool(r->pool);
> my_table = ap_make_table(my_p, 5);
> in your hacked mod_include: r->pool = ap_make_sub_pool(r->pool);
> in module foo: x = ap_pstrdup(r->pool, blahblahblah);
> ap_table_setn(my_table, "abc", x);
>
> That's broken. And module foo isn't what's broken, it's doing something
> that fits within the ancestor relationship... only the ancestry has been
> broken. Although I think the damage really only becomes apparent in
> a cleanup.
I fail to see how the above call sequence can happen. Can you flesh it out
some? And are there any ap_*sub_req*() calls in the sequence?
> Let's just say I'm going to take a lot more convincing that futzing with
> r->pool is fine. In general I consider that to be a read-only field.
> You'd certainly screw up if we had contexts in the pool like we're
> talking about for 2.0.
It may not survive with contexts in 2.0, but it's not clear that the existing
mod_include can coexist with contexts in 2.0 either.
The sub_pool business is ONLY there so that a sub_req can be destroyed. I'm
tired of this argument. Use the following;
In mod_include:
@@ -2376,6 +2365,38 @@
return OK;
}
+#define SUB_REQ_STRING "Sub request to mod_include"
+#define PARENT_STRING "Parent request to mod_include"
+
+ if (ap_table_get(r->notes, SUB_REQ_STRING)) {
+ request_rec *p = r->main;
+ /* The note is a flag to mod_include that this request is actually
+ * a subrequest from another module and that mod_include needs to
+ * treat it as if it's a subrequest from mod_include.
+ *
+ * HACK ALERT!
+ * There is no good way to pass the parent request_rec to mod_include.
+ * Tables only take string values and there is nowhere appropriate in
+ * in the request_rec that can safely be used.
+ *
+ * So we search up the chain of requests and redirects looking for
+ * the parent request.
+ */
+ while (p) {
+ if (ap_table_get(p->notes, PARENT_STRING)) {
+ /* Kludge --- See below */
+ ap_set_module_config(r->request_config, &includes_module, p);
+
+ ap_add_common_vars(p);
+ ap_add_cgi_vars(p);
+ add_include_vars(p, DEFAULT_TIME_FORMAT);
+ ap_table_unset(r->notes, SUB_REQ_STRING);
+ break;
+ }
+ p = (p->prev) ? p->prev : p->main;
+ }
+ }
+
if ((parent = ap_get_module_config(r->request_config, &includes_module))) {
/* Kludge --- for nested includes, we want to keep the subprocess
* environment of the base document (for compatibility); that means
In mod_autoindex:
@@ -924,6 +924,9 @@
ap_rputs("</PRE>\n", r);
}
+ /* See mod_include */
+ #define SUB_REQ_STRING "Sub request to mod_include"
+ #define PARENT_STRING "Parent request to mod_include"
/*
* Handle the preamble through the H1 tag line, inclusive. Locate
@@ -969,6 +972,8 @@
if (! suppress_amble) {
emit_preamble(r, title);
}
+ ap_table_add(r->notes, PARENT_STRING, "");
+ ap_table_add(rr->notes, SUB_REQ_STRING, "");
/*
* If there's a problem running the subrequest, display the
* preamble if we didn't do it before -- the header file
@@ -1006,9 +1011,6 @@
if (emit_H1) {
ap_rvputs(r, "<H1>Index of ", title, "</H1>\n", NULL);
}
- if (rr != NULL) {
- ap_destroy_sub_req(rr);
- }
}
@@ -1045,6 +1047,8
*/
if (rr->content_type != NULL) {
if (!strcasecmp("text/html", rr->content_type)) {
+ ap_table_add(r->notes, PARENT_STRING, "");
+ ap_table_add(rr->notes, SUB_REQ_STRING, "");
if (ap_run_sub_req(rr) == OK) {
/* worked... */
suppress_sig = 1;
@@ -1072,9 +1076,6 @@
if (!suppress_post) {
ap_rputs("</BODY></HTML>\n", r);
}
- if (rr != NULL) {
- ap_destroy_sub_req(rr);
}
}
Raymond S Brand
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Posted by Dean Gaudet <dg...@arctic.org>.
On Thu, 13 May 1999, Raymond S Brand wrote:
> Dean Gaudet wrote:
> >
> > ap_pool_join is an advisory debugging tool -- alloc.c does nothing to
> > ensure that you preserve the lifetimes appropriately.
>
> I understand that. Please apply the patch to mod_include and look at
> where the ap_make_sub_pool() I added is called.
Yeah I saw that:
/* Kludge --- Doing this allows the caller to safely destroy the
* sub_req
*/
r->pool = ap_make_sub_pool(r->pool);
Sorry, but that's just way off my bogosity meter.
Consider this sequence:
in module foo: my_p = ap_make_sub_pool(r->pool);
my_table = ap_make_table(my_p, 5);
in your hacked mod_include: r->pool = ap_make_sub_pool(r->pool);
in module foo: x = ap_pstrdup(r->pool, blahblahblah);
ap_table_setn(my_table, "abc", x);
That's broken. And module foo isn't what's broken, it's doing something
that fits within the ancestor relationship... only the ancestry has been
broken. Although I think the damage really only becomes apparent in
a cleanup.
Let's just say I'm going to take a lot more convincing that futzing with
r->pool is fine. In general I consider that to be a read-only field.
You'd certainly screw up if we had contexts in the pool like we're
talking about for 2.0.
Dean
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c
mod_include.c
Posted by Raymond S Brand <rs...@rsbx.net>.
Dean Gaudet wrote:
>
> ap_pool_join is an advisory debugging tool -- alloc.c does nothing to
> ensure that you preserve the lifetimes appropriately.
I understand that. Please apply the patch to mod_include and look at
where the ap_make_sub_pool() I added is called.
> Try two #includes within one .shtml -- set variable foo=bar in the first
> and set variable bleh=whatever in the second. Do #printenvs in between.
> Watch for garbage.
Just did; worked fine; I'd run into that before and fixed it with the
changes to mod_include.
Raymond S Brand
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Posted by Dean Gaudet <dg...@arctic.org>.
On Thu, 13 May 1999, Raymond S Brand wrote:
> As far as I can tell it's perfectly safe. The original pool that the sub_req
> is in has been ap_pool_join()ed to the parent_req pool; it is also a sub pool
> of the parent pool. The making of the (empty) sub pool here is, as the comment
> says, to allow the sub_req to safely destroyed. The code that destroys reqs
> does no other work than to call ap_destroy_pool() so this should be safe.
ap_pool_join is an advisory debugging tool -- alloc.c does nothing to
ensure that you preserve the lifetimes appropriately.
Try two #includes within one .shtml -- set variable foo=bar in the first
and set variable bleh=whatever in the second. Do #printenvs in between.
Watch for garbage.
Dean
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c
mod_include.c
Posted by Raymond S Brand <rs...@rsbx.net>.
Dean Gaudet wrote:
>
> On 13 May 1999 coar@hyperreal.org wrote:
>
> > Index: mod_include.c
>
> I never had a chance to look at Raymond's hacks to mod_include.
>
> > +#define SUB_REQ_STRING "Sub request to mod_include"
> > +#define PARENT_STRING "Parent request to mod_include"
>
> I don't like these #defines spread over two modules. I don't like modules
> which rely on each other.
They don't rely on each other, they build on each other. And they both
work if the other isn't loaded. Which header file do you suggest for
the defines?
>
> > @@ -2411,9 +2441,10 @@
> > send_parsed_content(f, r);
> >
> > if (parent) {
> > - /* signify that the sub request should not be killed */
> > - ap_set_module_config(r->request_config, &includes_module,
> > - NESTED_INCLUDE_MAGIC);
> > + /* Kludge --- Doing this allows the caller to safely destroy the
> > + * sub_req
> > + */
> > + r->pool = ap_make_sub_pool(r->pool);
>
> I haven't looked at this for long, but I believe this is totally wrong.
As far as I can tell it's perfectly safe. The original pool that the sub_req
is in has been ap_pool_join()ed to the parent_req pool; it is also a sub pool
of the parent pool. The making of the (empty) sub pool here is, as the comment
says, to allow the sub_req to safely destroyed. The code that destroys reqs
does no other work than to call ap_destroy_pool() so this should be safe.
>
> Try building with -DPOOL_DEBUG -DALLOC_DEBUG and see what happens.
-DPOOL_DEBUG=1 is what I used to come up with this scheme.
>
> I would much rather have Raymod's updates to mod_autoindex separate from
> the hacks to mod_include... when I asked for him to post his patches again
> "combined" I meant: post the patches with the style changes... I didn't
> mean combine the "evil hack" and the acceptable patch. I just didn't
> want to review a patch on top of a patch.
I can generate another patch that implements the rendezvous but the
smaller of the two patches attached to my 5/4/99 message is still good.
The handful lines added to mod_autoindex will have to be applied by hand;
though, due to the changes Ken and I made. The "Evil Hack" is long dead.
Raymond S Brand
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Posted by Dean Gaudet <dg...@arctic.org>.
No, those are fine. If they weren't, then mod_include would be at fault.
The only sub requests which aren't destroyed are the ones which
mod_include itself creates and shares its environment with.
Dean
On Thu, 13 May 1999, Raymond S Brand wrote:
> Since the changes to mod_include were backed out, the call to
> ap_destroy_sub_req() in emit_head() and in emit_tail() in
> mod_autoindex.c need to be commented out.
>
> Raymond S Brand
>
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c
mod_include.c
Posted by Raymond S Brand <rs...@rsbx.net>.
Since the changes to mod_include were backed out, the call to
ap_destroy_sub_req() in emit_head() and in emit_tail() in
mod_autoindex.c need to be commented out.
Raymond S Brand
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Dean Gaudet wrote:
>
> I don't like these #defines spread over two modules. I don't like
> modules which rely on each other.
I agree about the #defines, but I regarded the way mod_speling and
mod_negotiation use hard-coded (but not #defined :-) values in the
notes table to http_protocol.c as a precedent.
--
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Software Foundation <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Posted by Dean Gaudet <dg...@arctic.org>.
On 13 May 1999 coar@hyperreal.org wrote:
> Index: mod_include.c
I never had a chance to look at Raymond's hacks to mod_include.
> +#define SUB_REQ_STRING "Sub request to mod_include"
> +#define PARENT_STRING "Parent request to mod_include"
I don't like these #defines spread over two modules. I don't like modules
which rely on each other.
> @@ -2411,9 +2441,10 @@
> send_parsed_content(f, r);
>
> if (parent) {
> - /* signify that the sub request should not be killed */
> - ap_set_module_config(r->request_config, &includes_module,
> - NESTED_INCLUDE_MAGIC);
> + /* Kludge --- Doing this allows the caller to safely destroy the
> + * sub_req
> + */
> + r->pool = ap_make_sub_pool(r->pool);
I haven't looked at this for long, but I believe this is totally wrong.
Try building with -DPOOL_DEBUG -DALLOC_DEBUG and see what happens.
I would much rather have Raymod's updates to mod_autoindex separate from
the hacks to mod_include... when I asked for him to post his patches again
"combined" I meant: post the patches with the style changes... I didn't
mean combine the "evil hack" and the acceptable patch. I just didn't
want to review a patch on top of a patch.
Dean