You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Martin Kraemer <ma...@mch.sni.de> on 1999/01/19 22:30:05 UTC
[PATCH] (Take 3) Core dump with Language Negotiation?!?
This is Dean's patch, plus the changes at the calling location he
mentioned (where ap_overlay_tables() used to be called), which I appended.
With this change, the dumps are gone, too (as with Roy's initial patch).
Because of what Dean said about his solution...
> Another option is to add AP_OVERLAP_TABLES_COPY and tweak the overlap
> tables code to strdup when necessary. Otherwise there's a potential for
> O(n^2) stuff.
... I'm slightly in favor of this patch. Votes?
Martin
--
<Ma...@MchP.Siemens.De> | Siemens Information and
Phone: +49-89-636-46021 | Communication Products
FAX: +49-89-636-47816 | 81730 Munich, Germany
Re: [PATCH] (Take 3) Core dump with Language Negotiation?!?
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 19 Jan 1999, Dean Gaudet wrote:
> But I'm questioning what the hell mod_dir is doing this for anyhow. It
> seems totally bogus. As equally bogus as the "fast redirect" in
> mod_negotiation -- see lines 2669 and on.
>
> Both cases are extremely similar -- we've got a sub_req that we want to
> use to finish the request. In mod_dir you'll see it uses
> internal_redirect which causes the URI lookup crud to be redone.
>
> There's got to be a nice simplification of this. The "fast redirect"
> code in mod_negotiation was a source of lots of bugs during 1.2.
>
> I would say the minimum patch we should do now to fix the core dumps is an
> ap_join_pools and remove the ap_destroy_sub_req... similar to what the
> fast redirect does.
Anyhow, here's the minimum patch I'm suggesting for this for now... unless
someone wants to wrap their head around this subrequest stuff. I think
the code should probably be the same as the code in mod_negotiation, and
it should all be copied out and put somewhere common.
But there is a corruption possibility, so we should at least patch that
for now.
Dean
Index: modules/standard/mod_dir.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_dir.c,v
retrieving revision 1.53
diff -u -r1.53 mod_dir.c
--- mod_dir.c 1999/01/01 19:05:08 1.53
+++ mod_dir.c 1999/01/30 02:08:35
@@ -179,13 +179,13 @@
if (ap_is_HTTP_REDIRECT(rr->status) ||
(rr->status == HTTP_NOT_ACCEPTABLE && num_names == 1)) {
+ ap_pool_join(r->pool, rr->pool);
error_notfound = rr->status;
r->notes = ap_overlay_tables(r->pool, r->notes, rr->notes);
r->headers_out = ap_overlay_tables(r->pool, r->headers_out,
rr->headers_out);
r->err_headers_out = ap_overlay_tables(r->pool, r->err_headers_out,
rr->err_headers_out);
- ap_destroy_sub_req(rr);
return error_notfound;
}
Re: [PATCH] (Take 3) Core dump with Language Negotiation?!?
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 19 Jan 1999, Martin Kraemer wrote:
> This is Dean's patch, plus the changes at the calling location he
> mentioned (where ap_overlay_tables() used to be called), which I appended.
>
> With this change, the dumps are gone, too (as with Roy's initial patch).
> Because of what Dean said about his solution...
Ya know, I suspect with either my version or Roy's version there's
problems with "set-cookie". If we just want ap_overlay_table with copy
semantics it'd be:
API_EXPORT(void) ap_copyover_table(table *dest, const table *source,
unsigned flags)
{
array_header *sarr = ap_table_elts(source);
table_entry *selt = (table_entry *)sarr->elts;
int i;
for (i = 0; i < sarr->nelts; ++i) {
ap_table_add(dest, selt[i].key, selt[i].val);
}
}
which doesn't have O(n^2) problems...
But I'm questioning what the hell mod_dir is doing this for anyhow. It
seems totally bogus. As equally bogus as the "fast redirect" in
mod_negotiation -- see lines 2669 and on.
Both cases are extremely similar -- we've got a sub_req that we want to
use to finish the request. In mod_dir you'll see it uses
internal_redirect which causes the URI lookup crud to be redone.
There's got to be a nice simplification of this. The "fast redirect"
code in mod_negotiation was a source of lots of bugs during 1.2.
I would say the minimum patch we should do now to fix the core dumps is an
ap_join_pools and remove the ap_destroy_sub_req... similar to what the
fast redirect does.
Dean