[patch] resolving segfaults when ap_run_pre_connection() fails.

[Stas Bekman]

why ap_process_connection() doesn't check the return value of
ap_run_pre_connection()? Currently if the pre_connection phase fails (without
setting c->aborted) ap_run_process_connection is executed
nevertheless.

if a custom pre_connection handler fails, before giving a chance for
core_pre_connection handler to run, it will cause a segfault later in
the net_time_filter filter (during the process_connection phase):

if (!ctx) {
f->ctx = ctx = apr_palloc(f->r->pool, sizeof(*ctx));
ctx->first_line = 1;
ctx->csd = ap_get_module_config(f->c->conn_config, &core_module);
}

ctx->csd is now NULL and it segfaults next in apr_socket_timeout_set():

if (mode != AP_MODE_INIT && mode != AP_MODE_EATCRLF) {
if (ctx->first_line) {
apr_socket_timeout_set(ctx->csd,
keptalive
? f->c->base_server->keep_alive_timeout
: f->c->base_server->timeout);

Now, since core_pre_connection returns DONE, it's impossible to insert
anything after it. So telling developers to insert their
pre_connection hooks after core_pre_connection is not an option.

Here is a proposed patch, may be logging an error message is a good idea.

Index: server/connection.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/connection.c,v
retrieving revision 1.106
diff -u -r1.106 connection.c
--- server/connection.c 15 Jul 2002 08:05:10 -0000 1.106
+++ server/connection.c 18 Feb 2003 01:11:38 -0000
@@ -199,11 +199,12 @@

AP_CORE_DECLARE(void) ap_process_connection(conn_rec *c, void *csd)
{
+ apr_status_t rc;
ap_update_vhost_given_ip(c);

- ap_run_pre_connection(c, csd);
-
- if (!c->aborted) {
+ rc = ap_run_pre_connection(c, csd);
+
+ if ((rc == OK || rc == DONE) && !c->aborted) {
ap_run_process_connection(c);
}
}

Finally, I see that there are other hooks in the public API whose return value is not checked (e.g. ap_run_child_init). I thought that developers are supposed to return a failure status if their callbacks fail and expect Apache to abort the processing.

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com