From bf27d1a35452d1833cabf40f151e1a04eb583226 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Tue, 2 Jan 2007 18:26:00 +0000 Subject: [PATCH] Handle error conditions in dbd_construct() properly. Simplify ap_dbd_open() and use correct arguments to apr_dbd_error() when non-threaded. Register correct cleanup data in non-threaded ap_dbd_acquire() and ap_dbd_cacquire(). Clean up configuration data and merge function. Use ap_log_error() wherever possible. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@491884 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 7 ++ modules/database/mod_dbd.c | 224 ++++++++++++++++++------------------- 2 files changed, 118 insertions(+), 113 deletions(-) diff --git a/CHANGES b/CHANGES index c561ace1a0..5fa4739b31 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,13 @@ Changes with Apache 2.3.0 [Remove entries to the current 2.0 and 2.2 section below, when backported] + *) mod_dbd: Handle error conditions in dbd_construct() properly. + Simplify ap_dbd_open() and use correct arguments to apr_dbd_error() + when non-threaded. Register correct cleanup data in non-threaded + ap_dbd_acquire() and ap_dbd_cacquire(). Clean up configuration data + and merge function. Use ap_log_error() wherever possible. + [Chris Darroch, Nick Kew] + *) mod_proxy_http: Handle request bodies larger than 2 GB by converting the Content-Length header of the request correctly. PR 40883. [Ruediger Pluem, toadie ] diff --git a/modules/database/mod_dbd.c b/modules/database/mod_dbd.c index bbe5437f43..56270f9153 100644 --- a/modules/database/mod_dbd.c +++ b/modules/database/mod_dbd.c @@ -20,15 +20,20 @@ * http://apache.webthing.com/database/ */ -#include +#include "apr_reslist.h" +#include "apr_strings.h" +#include "apr_hash.h" +#include "apr_lib.h" +#include "apr_dbd.h" + +#define APR_WANT_MEMFUNC +#define APR_WANT_STRFUNC +#include "apr_want.h" #include "http_protocol.h" #include "http_config.h" #include "http_log.h" #include "http_request.h" -#include "apr_reslist.h" -#include "apr_strings.h" -#include "apr_dbd.h" #include "mod_dbd.h" extern module AP_MODULE_DECLARE_DATA dbd_module; @@ -49,6 +54,7 @@ struct dbd_prepared { }; typedef struct { + server_rec *server; const char *name; const char *params; int persist; @@ -61,10 +67,10 @@ typedef struct { int nkeep; int nmax; int exptime; + int set; #else ap_dbd_t *rec; #endif - unsigned int set; } svr_cfg; typedef enum { cmd_name, cmd_params, cmd_persist, @@ -88,6 +94,7 @@ static void *create_dbd_config(apr_pool_t *pool, server_rec *s) { svr_cfg *svr = apr_pcalloc(pool, sizeof(svr_cfg)); + svr->server = s; svr->name = no_dbdriver; /* to generate meaningful error messages */ svr->params = ""; /* don't risk segfault on misconfiguration */ svr->persist = -1; @@ -107,24 +114,23 @@ static void *merge_dbd_config(apr_pool_t *pool, void *basev, void *addv) svr_cfg *add = (svr_cfg*) addv; svr_cfg *svr = apr_pcalloc(pool, sizeof(svr_cfg)); + svr->server = add->server; svr->name = (add->name != no_dbdriver) ? add->name : base->name; svr->params = strcmp(add->params, "") ? add->params : base->params; - svr->persist = (add->persist == -1) ? base->persist : add->persist; + svr->persist = (add->persist != -1) ? add->persist : base->persist; #if APR_HAS_THREADS svr->nmin = (add->set&NMIN_SET) ? add->nmin : base->nmin; svr->nkeep = (add->set&NKEEP_SET) ? add->nkeep : base->nkeep; svr->nmax = (add->set&NMAX_SET) ? add->nmax : base->nmax; svr->exptime = (add->set&EXPTIME_SET) ? add->exptime : base->exptime; #endif - svr->set = add->set | base->set; - svr->prepared = (add->prepared != NULL) ? add->prepared : base->prepared; return svr; } #define ISINT(val) \ for (p = val; *p; ++p) \ - if (!isdigit(*p)) \ + if (!apr_isdigit(*p)) \ return "Argument must be numeric!" static const char *dbd_param(cmd_parms *cmd, void *dconf, const char *val) @@ -258,7 +264,7 @@ static int dbd_post_config(apr_pool_t *pconf, apr_pool_t *plog, for (sp = s; sp; sp = sp->next) { svr_cfg *svr = ap_get_module_config(sp->module_config, &dbd_module); - const char *key = apr_psprintf(s->process->pool, "%pp", s); + const char *key = apr_psprintf(ptemp, "%pp", s); svr->prepared = apr_hash_get(dbd_prepared_defns, key, APR_HASH_KEY_STRING); @@ -321,8 +327,9 @@ static apr_status_t dbd_construct(void **data_ptr, /* this pool is mostly so dbd_close can destroy the prepared stmts */ rv = apr_pool_create(&rec->pool, pool); if (rv != APR_SUCCESS) { - ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool, - "DBD: Failed to create memory pool"); + ap_log_error(APLOG_MARK, APLOG_CRIT, rv, svr->server, + "DBD: Failed to create memory pool"); + return rv; } /* The driver is loaded at config time now, so this just checks a hash. @@ -334,24 +341,25 @@ static apr_status_t dbd_construct(void **data_ptr, if (rv != APR_SUCCESS) { switch (rv) { case APR_ENOTIMPL: - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: driver for %s not available", svr->name); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: driver for %s not available", svr->name); break; case APR_EDSOOPEN: - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: can't find driver for %s", svr->name); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: can't find driver for %s", svr->name); break; case APR_ESYMNOTFOUND: - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: driver for %s is invalid or corrupted", - svr->name); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: driver for %s is invalid or corrupted", + svr->name); break; default: - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: mod_dbd not compatible with APR in get_driver"); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: mod_dbd not compatible with APR in get_driver"); break; } + apr_pool_destroy(rec->pool); return rv; } @@ -359,33 +367,39 @@ static apr_status_t dbd_construct(void **data_ptr, if (rv != APR_SUCCESS) { switch (rv) { case APR_EGENERAL: - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: Can't connect to %s", svr->name); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: Can't connect to %s", svr->name); break; default: - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: mod_dbd not compatible with APR in open"); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: mod_dbd not compatible with APR in open"); break; } + apr_pool_destroy(rec->pool); return rv; } - *data_ptr = rec; - rv = dbd_prepared_init(rec->pool, svr, rec); if (rv != APR_SUCCESS) { const char *errmsg = apr_dbd_error(rec->driver, rec->handle, rv); - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, rec->pool, - "DBD: failed to prepare SQL statements: %s", - (errmsg ? errmsg : "[???]")); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, svr->server, + "DBD: failed to prepare SQL statements: %s", + (errmsg ? errmsg : "[???]")); + + apr_dbd_close(rec->driver, rec->handle); + apr_pool_destroy(rec->pool); + return rv; } - return rv; + *data_ptr = rec; + + return APR_SUCCESS; } #if APR_HAS_THREADS -static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr) +static apr_status_t dbd_setup(apr_pool_t *pool, server_rec *s, + svr_cfg *svr) { apr_status_t rv; @@ -397,8 +411,8 @@ static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr) */ rv = apr_pool_create(&svr->pool, pool); if (rv != APR_SUCCESS) { - ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool, - "DBD: Failed to create reslist memory pool"); + ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s, + "DBD: Failed to create reslist memory pool"); return rv; } @@ -407,19 +421,14 @@ static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr) apr_time_from_sec(svr->exptime), dbd_construct, dbd_destruct, svr, svr->pool); - if (rv == APR_SUCCESS) { - apr_pool_cleanup_register(svr->pool, svr->reslist, - (apr_status_t (*)(void*)) apr_reslist_destroy, - apr_pool_cleanup_null); - } - else { - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, svr->pool, - "DBD: failed to initialise"); + if (rv != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, + "DBD: failed to initialise"); apr_pool_destroy(svr->pool); - svr->pool = NULL; + return rv; } - return rv; + return APR_SUCCESS; } static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s) @@ -440,7 +449,7 @@ static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s) return APR_SUCCESS; } - rv = dbd_setup(pool, svr); + rv = dbd_setup(pool, s, svr); if (rv == APR_SUCCESS) { return rv; } @@ -451,16 +460,16 @@ static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s) rv = apr_thread_mutex_create(&svr->mutex, APR_THREAD_MUTEX_DEFAULT, pool); if (rv != APR_SUCCESS) { - ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool, - "DBD: Failed to create thread mutex"); + ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s, + "DBD: Failed to create thread mutex"); } return rv; } -static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s) +static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s, + svr_cfg *svr) { - svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module); apr_status_t rv = APR_SUCCESS, rv2; /* several threads could be here at the same time, all trying to @@ -473,19 +482,19 @@ static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s) rv2 = apr_thread_mutex_lock(svr->mutex); if (rv2 != APR_SUCCESS) { - ap_log_perror(APLOG_MARK, APLOG_ERR, rv2, pool, - "DBD: Failed to acquire thread mutex"); + ap_log_error(APLOG_MARK, APLOG_ERR, rv2, s, + "DBD: Failed to acquire thread mutex"); return rv2; } if (!svr->reslist) { - rv = dbd_setup(s->process->pool, svr); + rv = dbd_setup(s->process->pool, s, svr); } rv2 = apr_thread_mutex_unlock(svr->mutex); if (rv2 != APR_SUCCESS) { - ap_log_perror(APLOG_MARK, APLOG_ERR, rv2, pool, - "DBD: Failed to release thread mutex"); + ap_log_error(APLOG_MARK, APLOG_ERR, rv2, s, + "DBD: Failed to release thread mutex"); if (rv == APR_SUCCESS) { rv = rv2; } @@ -513,102 +522,91 @@ DBD_DECLARE_NONSTD(void) ap_dbd_close(server_rec *s, ap_dbd_t *rec) #endif } -#define arec ((ap_dbd_t*)rec) +static apr_status_t dbd_check(apr_pool_t *pool, server_rec *s, ap_dbd_t *rec) +{ + svr_cfg *svr; + apr_status_t rv = apr_dbd_check_conn(rec->driver, pool, rec->handle); + const char *errmsg; + + if ((rv == APR_SUCCESS) || (rv == APR_ENOTIMPL)) { + return APR_SUCCESS; + } + + errmsg = apr_dbd_error(rec->driver, rec->handle, rv); + if (!errmsg) { + errmsg = "(unknown)"; + } + + svr = ap_get_module_config(s->module_config, &dbd_module); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, + "DBD [%s] Error: %s", svr->name, errmsg); + return rv; +} -#if APR_HAS_THREADS DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open(apr_pool_t *pool, server_rec *s) { svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module); - void *rec = NULL; - apr_status_t rv = APR_SUCCESS; - const char *errmsg; + ap_dbd_t *rec = NULL; +#if APR_HAS_THREADS + apr_status_t rv; +#endif /* If nothing is configured, we shouldn't be here */ if (svr->name == no_dbdriver) { - ap_log_perror(APLOG_MARK, APLOG_ERR, 0, pool, "DBD: not configured"); + ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "DBD: not configured"); return NULL; } if (!svr->persist) { /* Return a once-only connection */ - rv = dbd_construct(&rec, svr, s->process->pool); - return (rv == APR_SUCCESS) ? arec : NULL; + dbd_construct((void**) &rec, svr, s->process->pool); + return rec; } +#if APR_HAS_THREADS if (!svr->reslist) { - if (dbd_setup_lock(pool, s) != APR_SUCCESS) { + if (dbd_setup_lock(pool, s, svr) != APR_SUCCESS) { return NULL; } } - rv = apr_reslist_acquire(svr->reslist, &rec); + rv = apr_reslist_acquire(svr->reslist, (void**) &rec); if (rv != APR_SUCCESS) { - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool, - "Failed to acquire DBD connection from pool!"); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, + "Failed to acquire DBD connection from pool!"); return NULL; } - rv = apr_dbd_check_conn(arec->driver, pool, arec->handle); - if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) { - errmsg = apr_dbd_error(arec->driver, arec->handle, rv); - if (!errmsg) { - errmsg = "(unknown)"; - } - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool, - "DBD [%s] Error: %s", svr->name, errmsg ); + if (dbd_check(pool, s, rec) != APR_SUCCESS) { apr_reslist_invalidate(svr->reslist, rec); return NULL; } - return arec; -} #else -DBD_DECLARE_NONSTD(ap_dbd_t*) ap_dbd_open(apr_pool_t *pool, server_rec *s) -{ - svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module); - void *rec = NULL; - apr_status_t rv = APR_SUCCESS; - const char *errmsg; - - /* If nothing is configured, we shouldn't be here */ - if (svr->name == no_dbdriver) { - ap_log_perror(APLOG_MARK, APLOG_ERR, 0, pool, "DBD: not configured"); - return NULL; - } - - if (!svr->persist) { - /* Return a once-only connection */ - rv = dbd_construct(&rec, svr, s->process->pool); - return (rv == APR_SUCCESS) ? arec : NULL; - } - /* If we have a persistent connection and it's good, we'll use it; * since this is non-threaded, we can update without a mutex */ - if (svr->rec) { - rv = apr_dbd_check_conn(svr->rec->driver, pool, svr->rec->handle); - if ((rv != APR_SUCCESS) && (rv != APR_ENOTIMPL)) { - errmsg = apr_dbd_error(arec->driver, arec->handle, rv); - if (!errmsg) { - errmsg = "(unknown)"; - } - ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool, - "DBD [%s] Error: %s", svr->name, errmsg); - svr->rec = NULL; + rec = svr->rec; + if (rec) { + if (dbd_check(pool, s, rec) != APR_SUCCESS) { + apr_pool_cleanup_run(s->process->pool, rec, dbd_close); + rec = NULL; } } /* We don't have a connection right now, so we'll open one */ - if (!svr->rec) { - if (dbd_construct(&rec, svr, s->process->pool) == APR_SUCCESS) { - svr->rec = arec ; - apr_pool_cleanup_register(s->process->pool, svr->rec, + if (!rec) { + dbd_construct((void**) &rec, svr, s->process->pool); + svr->rec = rec; + + if (rec) { + apr_pool_cleanup_register(s->process->pool, rec, dbd_close, apr_pool_cleanup_null); } } +#endif - return svr->rec; + return rec; } -#endif #if APR_HAS_THREADS typedef struct { @@ -710,7 +708,7 @@ DBD_DECLARE_NONSTD(ap_dbd_t *) ap_dbd_acquire(request_rec *r) ap_set_module_config(r->request_config, &dbd_module, rec); /* if persist then ap_dbd_open registered cleanup on proc pool */ if (!svr->persist) { - apr_pool_cleanup_register(r->pool, svr->rec, dbd_close, + apr_pool_cleanup_register(r->pool, rec, dbd_close, apr_pool_cleanup_null); } } @@ -732,7 +730,7 @@ DBD_DECLARE_NONSTD(ap_dbd_t *) ap_dbd_cacquire(conn_rec *c) ap_set_module_config(c->conn_config, &dbd_module, rec); /* if persist then ap_dbd_open registered cleanup on proc pool */ if (!svr->persist) { - apr_pool_cleanup_register(c->pool, svr->rec, dbd_close, + apr_pool_cleanup_register(c->pool, rec, dbd_close, apr_pool_cleanup_null); } } -- 2.40.0