diff -Naur mod_log_sql-1.101/functions.h mod_log_sql-1.101-rs/functions.h --- mod_log_sql-1.101/functions.h Mon Sep 20 04:50:46 2004 +++ mod_log_sql-1.101-rs/functions.h Mon Feb 18 10:53:30 2008 @@ -36,11 +36,8 @@ #else char *rvalue = r->user; #endif - if (rvalue == NULL) { - rvalue = "-"; - } else if (strlen(rvalue) == 0) { - rvalue = "\"\""; - } + if (rvalue == NULL) return apr_pstrcat(r->pool,"-",NULL); + if (strlen(rvalue) == 0) rvalue=apr_pstrcat(r->pool,"\"\"",NULL); return rvalue; } @@ -85,16 +82,12 @@ { return (r->args) ? apr_pstrcat(r->pool, "?", r->args, NULL) - : ""; + : apr_pstrcat(r->pool,"",NULL); } static const char *extract_status(request_rec *r, char *a) { - if (r->status <= 0) { - return "-"; - } else { - return apr_psprintf(r->pool, "%d", r->status); - } + return (r->status <= 0) ? apr_pstrcat(r->pool,"-",NULL) : apr_psprintf(r->pool, "%d", r->status); } static const char *extract_virtual_host(request_rec *r, char *a) @@ -109,10 +102,7 @@ static const char *extract_machine_id(request_rec *r, char *a) { - if (!global_config.machid) - return "-"; - else - return global_config.machid; + return (global_config.machid) ? global_config.machid : apr_pstrcat(r->pool,"-",NULL); } static const char *extract_server_port(request_rec *r, char *a) @@ -135,7 +125,8 @@ if (*a == '\0' || !strcmp(a, "pid")) { return apr_psprintf(r->pool, "%" APR_PID_T_FMT, getpid()); } - else if (!strcmp(a, "tid")) { + + if (!strcmp(a, "tid")) { #if APR_HAS_THREADS apr_os_thread_t tid = apr_os_thread_current(); #else @@ -152,12 +143,7 @@ const char *tempref; tempref = apr_table_get(r->headers_in, "Referer"); - if (!tempref) - { - return "-"; - } else { - return tempref; - } + return (tempref) ? tempref : apr_pstrcat(r->pool,"-",NULL); } static const char *extract_agent(request_rec *r, char *a) @@ -165,12 +151,7 @@ const char *tempag; tempag = apr_table_get(r->headers_in, "User-Agent"); - if (!tempag) - { - return "-"; - } else { - return tempag; - } + return (tempag) ? tempag : apr_pstrcat(r->pool,"-",NULL); } static const char *extract_specific_cookie(request_rec *r, char *a) @@ -236,7 +217,7 @@ } } - return "-"; + return apr_pstrcat(r->pool,"-",NULL); } static const char *extract_cookie(request_rec *r, char *a) @@ -252,10 +233,7 @@ const char *tempid; tempid = apr_table_get(r->subprocess_env, "UNIQUE_ID"); - if (!tempid) - return "-"; - else - return tempid; + return (tempid) ? tempid : apr_pstrcat(r->pool,"-",NULL); } /* End declarations of various extract_ functions */ diff -Naur mod_log_sql-1.101/functions13.h mod_log_sql-1.101-rs/functions13.h --- mod_log_sql-1.101/functions13.h Thu Apr 29 20:05:24 2004 +++ mod_log_sql-1.101-rs/functions13.h Mon Feb 18 10:53:30 2008 @@ -3,7 +3,7 @@ static const char *extract_bytes_sent(request_rec *r, char *a) { if (!r->sent_bodyct) { - return "-"; + return ap_pstrcat(r->pool,"-",NULL); } else { long int bs; @@ -48,11 +48,11 @@ static const char *extract_connection_status(request_rec *r, char *a) { if (r->connection->aborted) - return "X"; + return ap_pstrcat(r->pool,"X",NULL); if ((r->connection->keepalive) && ((r->server->keep_alive_max - r->connection->keepalives) > 0)) { - return "+"; + return ap_pstrcat(r->pool,"+",NULL); } - return "-"; + return ap_pstrcat(r->pool,"-",NULL); } diff -Naur mod_log_sql-1.101/functions20.h mod_log_sql-1.101-rs/functions20.h --- mod_log_sql-1.101/functions20.h Tue Nov 7 03:56:37 2006 +++ mod_log_sql-1.101-rs/functions20.h Mon Feb 18 10:53:30 2008 @@ -3,7 +3,7 @@ static const char *extract_bytes_sent(request_rec *r, char *a) { if (!r->sent_bodyct || !r->bytes_sent) { - return "-"; + return apr_pstrcat(r->pool,"-",NULL); } else { return apr_psprintf(r->pool, "%" APR_OFF_T_FMT, r->bytes_sent); } @@ -105,12 +105,12 @@ static const char *extract_connection_status(request_rec *r, char *a) { if (r->connection->aborted) - return "X"; + return apr_pstrcat(r->pool,"X",NULL); if (r->connection->keepalive == AP_CONN_KEEPALIVE && (!r->server->keep_alive_max || (r->server->keep_alive_max - r->connection->keepalives) > 0)) { - return "+"; + return apr_pstrcat(r->pool,"+",NULL); } - return "-"; + return apr_pstrcat(r->pool,"-",NULL); } diff -Naur mod_log_sql-1.101/mod_log_sql.c mod_log_sql-1.101-rs/mod_log_sql.c --- mod_log_sql-1.101/mod_log_sql.c Tue Nov 7 03:43:23 2006 +++ mod_log_sql-1.101-rs/mod_log_sql.c Mon Feb 18 10:53:30 2008 @@ -8,6 +8,7 @@ # error Unsupported Apache version #endif + #ifdef HAVE_CONFIG_H /* Undefine these to prevent conflicts between Apache ap_config_auto.h and * my config.h. Only really needed for Apache < 2.0.48, but it can't hurt. @@ -152,7 +153,6 @@ static logsql_opendb_ret log_sql_opendb_link(server_rec* s) { - logsql_opendb_ret result; if (global_config.driver == NULL) { return LOGSQL_OPENDB_FAIL; } @@ -169,18 +169,14 @@ passwd */ if (global_config.db.parms) { - result = global_config.driver->connect(s, &global_config.db); - if (result==LOGSQL_OPENDB_FAIL) { - global_config.db.connected = 0; - } else { - global_config.db.connected = 1; - } + logsql_opendb_ret result = global_config.driver->connect(s, &global_config.db); + global_config.db.connected = (result==LOGSQL_OPENDB_FAIL) ? 0 : 1; return result; - } else { - log_error(APLOG_MARK, APLOG_ERR, 0, s, - "mod_log_sql: insufficient configuration info to establish database link"); - return LOGSQL_OPENDB_FAIL; - } + } + + log_error(APLOG_MARK, APLOG_ERR, 0, s, + "mod_log_sql: insufficient configuration info to establish database link"); + return LOGSQL_OPENDB_FAIL; } static void preserve_entry(request_rec *r, const char *query) @@ -434,19 +430,18 @@ logsql_opendb_ret retval; # if defined(WITH_APACHE20) /* Register cleanup hook to close DDB connection (apache 2 doesn't have child_exit) */ - apr_pool_cleanup_register(p, NULL, log_sql_close_link, log_sql_close_link); + /* RS: use apr_pool_cleanup_null as second hook */ + apr_pool_cleanup_register(p, NULL, log_sql_close_link, apr_pool_cleanup_null); # endif /* Open a link to the database */ retval = log_sql_opendb_link(s); switch (retval) { case LOGSQL_OPENDB_FAIL: - if (global_config.driver==NULL) { - log_error(APLOG_MARK, APLOG_ERR, 0, s, - "mod_log_sql: Driver module not loaded"); - } else { - log_error(APLOG_MARK, APLOG_ERR, 0, s, - "mod_log_sql: child spawned but unable to open database link"); - } + if (global_config.driver==NULL) { + log_error(APLOG_MARK, APLOG_ERR, 0, s, "mod_log_sql: Driver module not loaded"); + } else { + log_error(APLOG_MARK, APLOG_ERR, 0, s, "mod_log_sql: child spawned but unable to open database link"); + } break; case LOGSQL_OPENDB_SUCCESS: case LOGSQL_OPENDB_ALREADY: @@ -522,6 +517,11 @@ * Parms: request record, table type, table name, and the full SQL command */ +/* RS: + * if-statements having a return at the end don't need to be + continued with else -> have less indents +*/ + static logsql_query_ret safe_sql_insert(request_rec *r, logsql_tabletype table_type, const char *table_name, const char *query) { @@ -542,9 +542,12 @@ switch (result) { case LOGSQL_QUERY_SUCCESS: return LOGSQL_QUERY_SUCCESS; + case LOGSQL_QUERY_NOLINK: return LOGSQL_QUERY_FAIL; /* TODO: What do we do here */ + /* RS: I think we shall preserve the entry */ + case LOGSQL_QUERY_FAIL: global_config.driver->disconnect(&global_config.db); global_config.db.connected = 0; @@ -567,59 +570,59 @@ log_error(APLOG_MARK,APLOG_ERR, errno, r->server,"nanosleep unsuccessful"); } } -# endif /* win32 */ -# endif +# endif /* WIN32 */ +# endif /* WITH_APACHE13 */ + result = global_config.driver->insert(r,&global_config.db,query); if (result == LOGSQL_QUERY_SUCCESS) { return LOGSQL_QUERY_SUCCESS; - } else { - log_error(APLOG_MARK,APLOG_ERR,0,r->server,"second attempt failed"); - preserve_entry(r, query); - return LOGSQL_QUERY_PRESERVED; - } - } else { - log_error(APLOG_MARK,APLOG_ERR,0,r->server, - "reconnect failed, unable to reach database. SQL logging stopped until child regains a db connection."); - log_error(APLOG_MARK,APLOG_ERR,0,r->server, - "log entries are being preserved in %s",cls->preserve_file); + } + + log_error(APLOG_MARK,APLOG_ERR,0,r->server,"second attempt failed"); preserve_entry(r, query); return LOGSQL_QUERY_PRESERVED; } + + log_error(APLOG_MARK,APLOG_ERR,0,r->server, + "reconnect failed, unable to reach database. SQL logging stopped until child regains a db connection."); + log_error(APLOG_MARK,APLOG_ERR,0,r->server, + "log entries are being preserved in %s",cls->preserve_file); + preserve_entry(r, query); + return LOGSQL_QUERY_PRESERVED; break; + case LOGSQL_QUERY_NOTABLE: if (global_config.createtables) { - log_error(APLOG_MARK,APLOG_ERR,0,r->server, - "table doesn't exist...creating now"); + log_error(APLOG_MARK,APLOG_ERR,0,r->server,"table doesn't exist...creating now"); + if ((result = global_config.driver->create_table(r, &global_config.db, table_type, table_name))!=LOGSQL_TABLE_SUCCESS) { log_error(APLOG_MARK,APLOG_ERR,result,r->server, "child attempted but failed to create one or more tables for %s, preserving query", ap_get_server_name(r)); preserve_entry(r, query); return LOGSQL_QUERY_PRESERVED; - } else { - log_error(APLOG_MARK,APLOG_ERR,result, r->server, - "tables successfully created - retrying query"); - if ((result = global_config.driver->insert(r,&global_config.db,query))!=LOGSQL_QUERY_SUCCESS) { - log_error(APLOG_MARK,APLOG_ERR,result, r->server, - "giving up, preserving query"); - preserve_entry(r, query); - return LOGSQL_QUERY_PRESERVED; - } else { - log_error(APLOG_MARK,APLOG_NOTICE,0, r->server, - "query successful after table creation"); - return LOGSQL_QUERY_SUCCESS; - } + } + + log_error(APLOG_MARK,APLOG_ERR,result, r->server,"tables successfully created - retrying query"); + + if ((result = global_config.driver->insert(r,&global_config.db,query))!=LOGSQL_QUERY_SUCCESS) { + log_error(APLOG_MARK,APLOG_ERR,result, r->server, "giving up, preserving query"); + preserve_entry(r, query); + return LOGSQL_QUERY_PRESERVED; } - } else { - log_error(APLOG_MARK,APLOG_ERR,0,r->server, - "table doesn't exist, creation denied by configuration, preserving query"); - preserve_entry(r, query); - return LOGSQL_QUERY_PRESERVED; - } + + log_error(APLOG_MARK,APLOG_NOTICE,0, r->server,"query successful after table creation"); + return LOGSQL_QUERY_SUCCESS; + } + + log_error(APLOG_MARK,APLOG_ERR,0,r->server, + "table doesn't exist, creation denied by configuration, preserving query"); + preserve_entry(r, query); + return LOGSQL_QUERY_PRESERVED; break; + default: - log_error(APLOG_MARK,APLOG_ERR,0, r->server, - "Invalid return code from mog_log_query"); + log_error(APLOG_MARK,APLOG_ERR,0, r->server, "Invalid return code from mog_log_query"); return LOGSQL_QUERY_FAIL; break; } @@ -677,41 +680,46 @@ /* Parse through cookie lists and merge based on +/- prefixes */ /* TODO: rewrite as a function */ -#define DO_MERGE_ARRAY(parent,child,pool) \ -if (apr_is_empty_array(child)) { \ - apr_array_cat(child, parent); \ -} else { \ - apr_array_header_t *addlist, *dellist; \ - char **elem, **ptr = (char **)(child->elts); \ - int itr, overwrite = 0; \ - addlist = apr_array_make(pool,5,sizeof(char *)); \ - dellist = apr_array_make(subp,5,sizeof(char *)); \ -\ - for (itr=0; itrnelts; itr++) { \ - if (*ptr[itr] == '+') { \ - elem = (char **)apr_array_push(addlist); \ - *elem = (ptr[itr]+1); \ - } else if (*ptr[itr] == '-') { \ - elem = (char **)apr_array_push(dellist); \ - *elem = (ptr[itr]+1); \ - } else { \ - overwrite = 1; \ - elem = (char **)apr_array_push(addlist); \ - *elem = ptr[itr]; \ - } \ - } \ - child = apr_array_make(p,1,sizeof(char *)); \ - ptr = (char **)(parent->elts); \ - if (overwrite==0) { \ - /* if we are not overwriting the existing then prepare for merge */ \ - for (itr=0; itrnelts; itr++) { \ - if (!in_array(addlist, ptr[itr]) && !in_array(dellist,ptr[itr])) { \ - elem = apr_array_push(child); \ - *elem = apr_pstrdup(p, ptr[itr]); \ - } \ - } \ - } \ - apr_array_cat(child, addlist); \ +/* RS: rewritten as a function */ + +static void do_merge_array(apr_array_header_t *parent, apr_array_header_t *child, apr_pool_t *pool, apr_pool_t *mainpool) +{ +if (apr_is_empty_array(child)) { + apr_array_cat(child, parent); + } +else { + apr_array_header_t *addlist, *dellist; + char **elem, **ptr = (char **)(child->elts); + int itr, overwrite = 0; + addlist = apr_array_make(pool,5,sizeof(char *)); + dellist = apr_array_make(pool,5,sizeof(char *)); + + for (itr=0; itrnelts; itr++) { + if (*ptr[itr] == '+') { + elem = (char **)apr_array_push(addlist); + *elem = (ptr[itr]+1); + } else if (*ptr[itr] == '-') { + elem = (char **)apr_array_push(dellist); + *elem = (ptr[itr]+1); + } else { + overwrite = 1; + elem = (char **)apr_array_push(addlist); + *elem = ptr[itr]; + } + } + child = apr_array_make(mainpool,1,sizeof(char *)); + ptr = (char **)(parent->elts); + if (overwrite==0) { + /* if we are not overwriting the existing then prepare for merge */ + for (itr=0; itrnelts; itr++) { + if (!in_array(addlist, ptr[itr]) && !in_array(dellist,ptr[itr])) { + elem = apr_array_push(child); + *elem = apr_pstrdup(mainpool, ptr[itr]); + } + } + } + apr_array_cat(child, addlist); + } } static void *log_sql_merge_state(apr_pool_t *p, void *basev, void *addv) @@ -759,13 +767,13 @@ if (child->cookie_table_name == DEFAULT_COOKIE_TABLE_NAME) child->cookie_table_name = parent->cookie_table_name; - DO_MERGE_ARRAY(parent->transfer_ignore_list, child->transfer_ignore_list, subp); - DO_MERGE_ARRAY(parent->transfer_accept_list, child->transfer_accept_list, subp); - DO_MERGE_ARRAY(parent->remhost_ignore_list, child->remhost_ignore_list, subp); - DO_MERGE_ARRAY(parent->notes_list, child->notes_list, subp); - DO_MERGE_ARRAY(parent->hin_list, child->hin_list, subp); - DO_MERGE_ARRAY(parent->hout_list, child->hout_list, subp); - DO_MERGE_ARRAY(parent->cookie_list,child->cookie_list, subp); + do_merge_array(parent->transfer_ignore_list, child->transfer_ignore_list, subp, p); + do_merge_array(parent->transfer_accept_list, child->transfer_accept_list, subp, p); + do_merge_array(parent->remhost_ignore_list, child->remhost_ignore_list, subp, p); + do_merge_array(parent->notes_list, child->notes_list, subp, p); + do_merge_array(parent->hin_list, child->hin_list, subp, p); + do_merge_array(parent->hout_list, child->hout_list, subp, p); + do_merge_array(parent->cookie_list,child->cookie_list, subp, p); apr_pool_destroy(subp); @@ -779,9 +787,21 @@ /* Routine to perform the actual construction and execution of the relevant * INSERT statements. */ + +/* RS: for use of the iteration stuff at every loop in log_sql_transaction */ +/* sets ptrptr2 at the beginning to the end of the array_header */ + +#define for_each_apr_array_header(name) for ( ptrptr = (char **) name->elts, \ + ptrptr2 = (char **) (name->elts + (name->nelts * name->elt_size)); \ + ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + name->elt_size) \ + ) + static int log_sql_transaction(request_rec *orig) { - char **ptrptr, **ptrptr2; + /* RS: fail immediatly when no driver is available */ + if (global_config.driver == NULL) return OK; + + char **ptrptr, **ptrptr2; /* ptrptr2 used in for_each_apr_header_array! */ logsql_state *cls = ap_get_module_config(orig->server->module_config, &log_sql_module); const char *access_query; request_rec *r; @@ -790,9 +810,6 @@ const char *hout_tablename = cls->hout_table_name; const char *hin_tablename = cls->hin_table_name; const char *cookie_tablename = cls->cookie_table_name; - if (global_config.driver == NULL) { - return OK; - } /* We handle mass virtual hosting differently. Dynamically determine the name * of the table from the virtual server's name, and flag it for creation. */ @@ -833,285 +850,315 @@ /* Do we have enough info to log? */ if (!transfer_tablename) { return DECLINED; - } else { - const char *thehost; - const char *theitem; - char *fields = "", *values = ""; - char *itemsets = ""; - char *note_query = NULL; - char *hin_query = NULL; - char *hout_query = NULL; - char *cookie_query = NULL; - const char *unique_id; - const char *formatted_item; - int i,length; - int proceed; + } - for (r = orig; r->next; r = r->next) { - continue; + const char *thehost; + const char *theitem; + char *fields = "", *values = ""; + char *itemsets = ""; + char *note_query = NULL; + char *hin_query = NULL; + char *hout_query = NULL; + char *cookie_query = NULL; + const char *unique_id; + + int i,length; + int proceed; + + for (r = orig; r->next; r = r->next) { + continue; + } + + /* The following is a stolen upsetting mess of pointers, I'm sorry. + * Anyone with the motiviation and/or the time should feel free + * to make this cleaner. :) */ + + /* RS: used a for_each makro to iterate over apr_array_headers */ + + /* Go through each element of the accept list and compare it to the + * request_uri. If we don't get a match, return without logging */ + if ((r->uri) && (cls->transfer_accept_list->nelts)) { + + proceed = 0; + for_each_apr_array_header(cls->transfer_accept_list) { + if (ap_strstr(r->uri, *ptrptr)) { + proceed = 1; + break; + } } + if (!proceed) return OK; + } - /* The following is a stolen upsetting mess of pointers, I'm sorry. - * Anyone with the motiviation and/or the time should feel free - * to make this cleaner. :) */ - ptrptr2 = (char **) (cls->transfer_accept_list->elts + (cls->transfer_accept_list->nelts * cls->transfer_accept_list->elt_size)); - - /* Go through each element of the accept list and compare it to the - * request_uri. If we don't get a match, return without logging */ - if ((r->uri) && (cls->transfer_accept_list->nelts)) { - proceed = 0; - for (ptrptr = (char **) cls->transfer_accept_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->transfer_accept_list->elt_size)) - if (ap_strstr(r->uri, *ptrptr)) { - proceed = 1; - break; - } - if (!proceed) + /* Go through each element of the ignore list and compare it to the + * request_uri. If we get a match, return without logging */ + /* RS: check for cls->transfer_ignore_list->nelts, if we really must do the loop */ + if ((r->uri) && (cls->transfer_ignore_list->nelts)) { + + for_each_apr_array_header(cls->transfer_ignore_list) { + if (ap_strstr(r->uri, *ptrptr)) { return OK; + } } + } - /* Go through each element of the ignore list and compare it to the - * request_uri. If we get a match, return without logging */ - ptrptr2 = (char **) (cls->transfer_ignore_list->elts + (cls->transfer_ignore_list->nelts * cls->transfer_ignore_list->elt_size)); - if (r->uri) { - for (ptrptr = (char **) cls->transfer_ignore_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->transfer_ignore_list->elt_size)) - if (ap_strstr(r->uri, *ptrptr)) { - return OK; - } + /* Go through each element of the ignore list and compare it to the + * remote host. If we get a match, return without logging */ + /* RS: check for cls->remhost_ignore_list->nelts, if we really must do the loop */ + + thehost = ap_get_remote_host(r->connection, r->per_dir_config, REMOTE_NAME, NULL); + if ((thehost) && (cls->remhost_ignore_list->nelts)) { + + for_each_apr_array_header(cls->remhost_ignore_list) { + if (ap_strstr(thehost, *ptrptr)) { + return OK; + } } + } - /* Go through each element of the ignore list and compare it to the - * remote host. If we get a match, return without logging */ - ptrptr2 = (char **) (cls->remhost_ignore_list->elts + (cls->remhost_ignore_list->nelts * cls->remhost_ignore_list->elt_size)); - thehost = ap_get_remote_host(r->connection, r->per_dir_config, REMOTE_NAME, NULL); - if (thehost) { - for (ptrptr = (char **) cls->remhost_ignore_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->remhost_ignore_list->elt_size)) - if (ap_strstr(thehost, *ptrptr)) { - return OK; - } + /* Iterate through the format characters and set up the INSERT string according to + * what the user has configured. */ + length = strlen(cls->transfer_log_format); + for (i = 0; iparsed_log_format[i]; + if (item==NULL) { + log_error(APLOG_MARK, APLOG_ERR, 0, orig->server, + "Log Format '%c' unknown",cls->transfer_log_format[i]); + continue; } + /* Yes, this key is one of the configured keys. + * Call the key's function and put the returned value into 'formatted_item' */ + + formatted_item = (char *)item->func(item->want_orig_default ? orig : r, ""); + + + /* Massage 'formatted_item' for proper SQL eligibility... */ + if (!formatted_item) { + /* RS: no space allocated for formatted_item before */ + /* so use apr_palloc and form an empty string */ + formatted_item=apr_palloc(r->pool,1); + formatted_item[0]=0; + } else if (formatted_item[0] == '-' && formatted_item[1] == '\0' && !item->string_contents) { + /* If apache tried to log a '-' character for a numeric field, convert that to a zero + * because the database expects a numeral and will reject the '-' character. */ + + /* RS: no formatted_item="0" */ + formatted_item=apr_pstrcat(r->pool,"0",NULL); + } + + /* Append the fieldname and value-to-insert to the appropriate strings, quoting stringvals with ' as appropriate */ + fields = apr_pstrcat(r->pool, fields, (i ? "," : ""), item->sql_field_name, NULL); + values = apr_pstrcat(r->pool, values, (i ? "," : ""), global_config.driver->escape(formatted_item, r->pool,&global_config.db), NULL); + } - /* Iterate through the format characters and set up the INSERT string according to - * what the user has configured. */ - length = strlen(cls->transfer_log_format); - for (i = 0; iparsed_log_format[i]; - if (item==NULL) { - log_error(APLOG_MARK, APLOG_ERR, 0, orig->server, - "Log Format '%c' unknown",cls->transfer_log_format[i]); - continue; - } - /* Yes, this key is one of the configured keys. - * Call the key's function and put the returned value into 'formatted_item' */ - formatted_item = item->func(item->want_orig_default ? orig : r, ""); - - /* Massage 'formatted_item' for proper SQL eligibility... */ - if (!formatted_item) { - formatted_item = ""; - } else if (formatted_item[0] == '-' && formatted_item[1] == '\0' && !item->string_contents) { - /* If apache tried to log a '-' character for a numeric field, convert that to a zero - * because the database expects a numeral and will reject the '-' character. */ - formatted_item = "0"; - } + unique_id = extract_unique_id(r, ""); - /* Append the fieldname and value-to-insert to the appropriate strings, quoting stringvals with ' as appropriate */ - fields = apr_pstrcat(r->pool, fields, (i ? "," : ""), - item->sql_field_name, NULL); - values = apr_pstrcat(r->pool, values, (i ? "," : ""), - global_config.driver->escape(formatted_item, r->pool,&global_config.db), NULL); - } + /* Work through the list of notes defined by LogSQLWhichNotes */ + /* RS: check for cls->notes_list->nelts, if we really must do the loop */ + if(cls->notes_list->nelts) { - /* Work through the list of notes defined by LogSQLWhichNotes */ i = 0; - unique_id = extract_unique_id(r, ""); + /* RS: alloc an empty string (aka '\0') for itemsets */ + itemsets=apr_palloc(r->pool,1); + itemsets[0]=0; - ptrptr2 = (char **) (cls->notes_list->elts + (cls->notes_list->nelts * cls->notes_list->elt_size)); - for (ptrptr = (char **) cls->notes_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->notes_list->elt_size)) { + for_each_apr_array_header(cls->notes_list) { /* If the specified note (*ptrptr) exists for the current request... */ - if ((theitem = apr_table_get(r->notes, *ptrptr))) { + if ((theitem = apr_table_get(r->notes, *ptrptr))) { itemsets = apr_pstrcat(r->pool, itemsets, - (i > 0 ? "," : ""), - "(", - global_config.driver->escape(unique_id, r->pool, &global_config.db), - ",", - global_config.driver->escape(*ptrptr, r->pool,&global_config.db), - ",", - global_config.driver->escape(theitem, r->pool,&global_config.db), - ")", - NULL); + (i > 0 ? "," : ""), + "(", + global_config.driver->escape(unique_id, r->pool, &global_config.db), + ",", + global_config.driver->escape(*ptrptr, r->pool,&global_config.db), + ",", + global_config.driver->escape(theitem, r->pool,&global_config.db), + ")", + NULL); i++; } } + if ( *itemsets != '\0' ) { note_query = apr_psprintf(r->pool, "insert %s into %s (id, item, val) values %s", /*global_config.insertdelayed?"delayed":*/"", notes_tablename, itemsets); - - log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: note string: %s", note_query); + log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: note string: %s", note_query); } + } + + + /* Work through the list of notes defined by ??? */ + /* RS: check for cls->hout_list->nelts, if we really must do the loop */ + if(cls->hout_list->nelts) { - /* Work through the list of headers-out defined by LogSQLWhichHeadersOut*/ i = 0; - itemsets = ""; + /* RS: alloc an empty string (aka '\0') for itemsets */ + itemsets=apr_palloc(r->pool,1); + itemsets[0]=0; + + for_each_apr_array_header(cls->hout_list) { - ptrptr2 = (char **) (cls->hout_list->elts + (cls->hout_list->nelts * cls->hout_list->elt_size)); - for (ptrptr = (char **) cls->hout_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->hout_list->elt_size)) { /* If the specified header (*ptrptr) exists for the current request... */ - if ((theitem = apr_table_get(r->headers_out, *ptrptr))) { + if ((theitem = apr_table_get(r->headers_out, *ptrptr))) { itemsets = apr_pstrcat(r->pool, itemsets, - (i > 0 ? "," : ""), - "(", - global_config.driver->escape(unique_id, r->pool, &global_config.db), - ",", - global_config.driver->escape(*ptrptr, r->pool,&global_config.db), - ",", - global_config.driver->escape(theitem, r->pool,&global_config.db), - ")", - NULL); + (i > 0 ? "," : ""), + "(", + global_config.driver->escape(unique_id, r->pool, &global_config.db), + ",", + global_config.driver->escape(*ptrptr, r->pool,&global_config.db), + ",", + global_config.driver->escape(theitem, r->pool,&global_config.db), + ")", + NULL); i++; } } + if ( *itemsets != '\0' ) { hout_query = apr_psprintf(r->pool, "insert %s into %s (id, item, val) values %s", /*global_config.insertdelayed?"delayed":*/"", hout_tablename, itemsets); log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: header_out string: %s", hout_query); } + } - /* Work through the list of headers-in defined by LogSQLWhichHeadersIn */ + + /* Work through the list of notes defined by ??? */ + /* RS: check for cls->hin_list->nelts, if we really must do the loop */ + if(cls->hin_list->nelts) { + i = 0; - itemsets = ""; + /* RS: alloc an empty string (aka '\0') for itemsets */ + itemsets=apr_palloc(r->pool,1); + itemsets[0]=0; - ptrptr2 = (char **) (cls->hin_list->elts + (cls->hin_list->nelts * cls->hin_list->elt_size)); - for (ptrptr = (char **) cls->hin_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->hin_list->elt_size)) { + for_each_apr_array_header(cls->hin_list) { /* If the specified header (*ptrptr) exists for the current request... */ - if ((theitem = apr_table_get(r->headers_in, *ptrptr))) { + if ((theitem = apr_table_get(r->headers_in, *ptrptr))) { itemsets = apr_pstrcat(r->pool, itemsets, - (i > 0 ? "," : ""), - "(", - global_config.driver->escape(unique_id, r->pool, &global_config.db), - ",", - global_config.driver->escape(*ptrptr, r->pool,&global_config.db), - ",", - global_config.driver->escape(theitem, r->pool,&global_config.db), - ")", - NULL); + (i > 0 ? "," : ""), + "(", + global_config.driver->escape(unique_id, r->pool, &global_config.db), + ",", + global_config.driver->escape(*ptrptr, r->pool,&global_config.db), + ",", + global_config.driver->escape(theitem, r->pool,&global_config.db), + ")", + NULL); i++; } } + if ( *itemsets != '\0' ) { hin_query = apr_psprintf(r->pool, "insert %s into %s (id, item, val) values %s", - /*global_config.insertdelayed?"delayed":*/"", hin_tablename, itemsets); + /*global_config.insertdelayed?"delayed":*/"", hin_tablename, itemsets); log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: header_in string: %s", hin_query); } + } + + /* Work through the list of cookies defined by LogSQLWhichCookies */ + /* RS: check for cls->cookie_list->nelts, if we really must do the loop */ + if(cls->cookie_list->nelts) { - /* Work through the list of cookies defined by LogSQLWhichCookies */ i = 0; - itemsets = ""; + /* RS: alloc an empty string (aka '\0') for itemsets */ + itemsets=apr_palloc(r->pool,1); + itemsets[0]=0; - ptrptr2 = (char **) (cls->cookie_list->elts + (cls->cookie_list->nelts * cls->cookie_list->elt_size)); - for (ptrptr = (char **) cls->cookie_list->elts; ptrptr < ptrptr2; ptrptr = (char **) ((char *) ptrptr + cls->cookie_list->elt_size)) { + for_each_apr_array_header(cls->cookie_list) { /* If the specified cookie (*ptrptr) exists for the current request... */ - if ( strncmp((theitem = extract_specific_cookie(r, *ptrptr)), "-", 1) ) { + if (strncmp((theitem = extract_specific_cookie(r, *ptrptr)), "-", 1) ) { itemsets = apr_pstrcat(r->pool, itemsets, - (i > 0 ? "," : ""), - "(", - global_config.driver->escape(unique_id, r->pool, &global_config.db), - ",", - global_config.driver->escape(*ptrptr, r->pool,&global_config.db), - ",", - global_config.driver->escape(theitem, r->pool,&global_config.db), - ")", - NULL); + (i > 0 ? "," : ""), + "(", + global_config.driver->escape(unique_id, r->pool, &global_config.db), + ",", + global_config.driver->escape(*ptrptr, r->pool,&global_config.db), + ",", + global_config.driver->escape(theitem, r->pool,&global_config.db), + ")", + NULL); i++; } - } + if ( *itemsets != '\0' ) { cookie_query = apr_psprintf(r->pool, "insert %s into %s (id, item, val) values %s", /*global_config.insertdelayed?"delayed":*/"", cookie_tablename, itemsets); log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: cookie string: %s", cookie_query); } + } + - /* Set up the actual INSERT statement */ - access_query = apr_psprintf(r->pool, "insert %s into %s (%s) values (%s)", - /*global_config.insertdelayed?"delayed":*/"", transfer_tablename, fields, values); + /* Set up the actual INSERT statement */ + access_query = apr_psprintf(r->pool, "insert %s into %s (%s) values (%s)", + /*global_config.insertdelayed?"delayed":*/"", transfer_tablename, fields, values); log_error(APLOG_MARK,APLOG_DEBUG,0, r->server,"mod_log_sql: access string: %s", access_query); - /* If the person activated force-preserve, go ahead and push all the entries - * into the preserve file, then return. - */ - if (global_config.forcepreserve) { - log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: preservation forced"); - preserve_entry(orig, access_query); - if ( note_query != NULL ) - preserve_entry(orig, note_query); - if ( hin_query != NULL ) - preserve_entry(orig, hin_query); - if ( hout_query != NULL ) - preserve_entry(orig, hout_query); - if ( cookie_query != NULL ) - preserve_entry(orig, cookie_query); - return OK; - } - - /* How's our mysql link integrity? */ - if (!global_config.db.connected) { - if (!global_config.forcepreserve) { - /* Make a try to establish the link */ - log_sql_opendb_link(r->server); - } - if (!global_config.db.connected) { - /* Unable to re-establish a DB link, so assume that it's really - * gone and send the entry to the preserve file instead. - * This short-circuits safe_sql_query() during a db outage and therefore - * we don't keep logging the db error over and over. - */ - preserve_entry(orig, access_query); - if ( note_query != NULL ) - preserve_entry(orig, note_query); - if ( hin_query != NULL ) - preserve_entry(orig, hin_query); - if ( hout_query != NULL ) - preserve_entry(orig, hout_query); - if ( cookie_query != NULL ) - preserve_entry(orig, cookie_query); - - return OK; - } else { - /* Whew, we got the DB link back */ - log_error(APLOG_MARK,APLOG_NOTICE,0, orig->server,"mod_log_sql: child established database connection"); - } - } - + /* If the person activated force-preserve, go ahead and push all the entries + * into the preserve file, then return. + */ + if (global_config.forcepreserve) { + log_error(APLOG_MARK,APLOG_DEBUG,0, orig->server,"mod_log_sql: preservation forced"); + preserve_entry(orig, access_query); + if ( note_query != NULL ) + preserve_entry(orig, note_query); + if ( hin_query != NULL ) + preserve_entry(orig, hin_query); + if ( hout_query != NULL ) + preserve_entry(orig, hout_query); + if ( cookie_query != NULL ) + preserve_entry(orig, cookie_query); + return OK; + } - /* ---> So as of here we have a non-null value of mysql_log. <--- */ - /* ---> i.e. we have a good MySQL connection. <--- */ + /* How's our mysql link integrity? */ + if (!global_config.db.connected) { + if (!global_config.forcepreserve) { + /* Make a try to establish the link */ + log_sql_opendb_link(r->server); + } - /* Make the access-table insert */ - safe_sql_insert(orig,LOGSQL_TABLE_ACCESS,transfer_tablename,access_query); + if (!global_config.db.connected) { + /* Unable to re-establish a DB link, so assume that it's really + * gone and send the entry to the preserve file instead. + * This short-circuits safe_sql_query() during a db outage and therefore + * we don't keep logging the db error over and over. + */ + preserve_entry(orig, access_query); + if ( note_query != NULL ) preserve_entry(orig, note_query); + if ( hin_query != NULL ) preserve_entry(orig, hin_query); + if ( hout_query != NULL ) preserve_entry(orig, hout_query); + if ( cookie_query != NULL ) preserve_entry(orig, cookie_query); + return OK; + } - /* Log the optional notes, headers, etc. */ - if (note_query) - safe_sql_insert(orig, LOGSQL_TABLE_NOTES,notes_tablename,note_query); + /* Whew, we got the DB link back */ + log_error(APLOG_MARK,APLOG_NOTICE,0, orig->server,"mod_log_sql: child established database connection"); + } - if (hout_query) - safe_sql_insert(orig, LOGSQL_TABLE_HEADERSOUT,hout_tablename,hout_query); - if (hin_query) - safe_sql_insert(orig, LOGSQL_TABLE_HEADERSIN,hin_tablename,hin_query); + /* ---> So as of here we have a non-null value of mysql_log. <--- */ + /* ---> i.e. we have a good MySQL connection. <--- */ - if (cookie_query) - safe_sql_insert(orig, LOGSQL_TABLE_COOKIES,cookie_tablename,cookie_query); + /* Make the access-table insert */ + safe_sql_insert(orig,LOGSQL_TABLE_ACCESS,transfer_tablename,access_query); - return OK; - } + /* Log the optional notes, headers, etc. */ + if (note_query) safe_sql_insert(orig, LOGSQL_TABLE_NOTES,notes_tablename,note_query); + if (hout_query) safe_sql_insert(orig, LOGSQL_TABLE_HEADERSOUT,hout_tablename,hout_query); + if (hin_query) safe_sql_insert(orig, LOGSQL_TABLE_HEADERSIN,hin_tablename,hin_query); + if (cookie_query) safe_sql_insert(orig, LOGSQL_TABLE_COOKIES,cookie_tablename,cookie_query); + return OK; } diff -Naur mod_log_sql-1.101/mod_log_sql_mysql.c mod_log_sql-1.101-rs/mod_log_sql_mysql.c --- mod_log_sql-1.101/mod_log_sql_mysql.c Tue Nov 7 03:43:08 2006 +++ mod_log_sql-1.101-rs/mod_log_sql_mysql.c Mon Feb 18 10:57:40 2008 @@ -46,7 +46,8 @@ if (!socketfile) { - socketfile = "/var/lib/mysql/mysql.sock"; + /* RS: replace socketfile = "..." with apr_pstrcat, hope s->pool is the right one */ + socketfile = apr_pstrcat(s->process->pool,"/var/lib/mysql/mysql.sock",NULL); } if (mysql_real_connect(dblink, host, user, passwd, database, tcpport, @@ -54,13 +55,14 @@ log_error(APLOG_MARK,APLOG_DEBUG,0, s,"HOST: '%s' PORT: '%d' DB: '%s' USER: '%s' SOCKET: '%s'", host, tcpport, database, user, socketfile); return LOGSQL_OPENDB_SUCCESS; - } else { - log_error(APLOG_MARK,APLOG_ERR,0, s,"mod_log_sql_mysql: database connection error: mysql error: %s", - MYSQL_ERROR(dblink)); - log_error(APLOG_MARK,APLOG_DEBUG, 0, s,"HOST: '%s' PORT: '%d' DB: '%s' USER: '%s' SOCKET: '%s'", - host, tcpport, database, user, socketfile); - return LOGSQL_OPENDB_FAIL; - } + } + + log_error(APLOG_MARK,APLOG_ERR,0, s,"mod_log_sql_mysql: database connection error: mysql error: %s", + MYSQL_ERROR(dblink)); + log_error(APLOG_MARK,APLOG_DEBUG, 0, s,"HOST: '%s' PORT: '%d' DB: '%s' USER: '%s' SOCKET: '%s'", + host, tcpport, database, user, socketfile); + return LOGSQL_OPENDB_FAIL; + } /* Close the DB link */ @@ -78,40 +80,36 @@ logsql_dbconnection *db) { /* Return "NULL" for empty strings */ - if (!from_str || strlen(from_str) == 0) - return "NULL"; - else { - char *to_str; - unsigned long length = strlen(from_str); - unsigned long retval; + if (!from_str || strlen(from_str) == 0) { + /* RS: replaced 'return "NULL"' with an allocated string */ + return apr_pstrcat(p,"NULL",NULL); + } - /* Pre-allocate a new string that could hold twice the original, which would only - * happen if the whole original string was 'dangerous' characters. - */ - to_str = (char *) apr_palloc(p, length * 2 + 3); - if (!to_str) { - return from_str; - } + unsigned long length = strlen(from_str); + unsigned long retval; + + /* Pre-allocate a new string that could hold twice the original, which would only + * happen if the whole original string was 'dangerous' characters. + */ + char *to_str = (char *) apr_palloc(p, length * 2 + 3); + if (!to_str) { + return from_str; + } strcpy(to_str, "'"); - if (!db->connected) { - /* Well, I would have liked to use the current database charset. mysql is - * unavailable, however, so I fall back to the slightly less respectful - * mysql_escape_string() function that uses the default charset. - */ - retval = mysql_escape_string(to_str+1, from_str, length); - } else { - /* MySQL is available, so I'll go ahead and respect the current charset when - * I perform the escape. - */ - retval = mysql_real_escape_string((MYSQL *)db->handle, to_str+1, from_str, length); - } - strcat(to_str,"'"); - - if (retval) - return to_str; - else - return from_str; + if (!db->connected) { + /* Well, I would have liked to use the current database charset. mysql is + * unavailable, however, so I fall back to the slightly less respectful + * mysql_escape_string() function that uses the default charset. + */ + retval = mysql_escape_string(to_str+1, from_str, length); + } else { + /* MySQL is available, so I'll go ahead and respect the current charset when + * I perform the escape. + */ + retval = mysql_real_escape_string((MYSQL *)db->handle, to_str+1, from_str, length); } + strcat(to_str,"'"); + return (retval) ? to_str : from_str; } #if defined(WIN32) @@ -124,17 +122,20 @@ #define SIGNAL_RELEASE signal(SIGPIPE, handler); #endif /* Run a mysql insert query and return a categorized error or success */ +/* RS: + * SIGNAL_RELEASE directly after mysql_query + * dont safe mysql_errno +*/ + static logsql_query_ret log_sql_mysql_query(request_rec *r,logsql_dbconnection *db, const char *query) { int retval; - SIGNAL_VAR + SIGNAL_VAR - unsigned int real_error = 0; /*const char *real_error_str = NULL;*/ MYSQL *dblink = (MYSQL *)db->handle; - if (!dblink) { return LOGSQL_QUERY_NOLINK; } @@ -147,20 +148,17 @@ SIGNAL_RELEASE return LOGSQL_QUERY_SUCCESS; } + /* Restore SIGPIPE to its original handler function */ + SIGNAL_RELEASE + log_error(APLOG_MARK, APLOG_ERR, 0, r->server, "mysql_query returned (%d)", retval); - /* Check to see if the error is "nonexistent table" */ - real_error = mysql_errno(dblink); - if (real_error == ER_NO_SUCH_TABLE) { + /* Check to see if the error is "nonexistent table" */ + if (mysql_errno(dblink) == ER_NO_SUCH_TABLE) { log_error(APLOG_MARK,APLOG_ERR,0, r->server,"table does not exist, preserving query"); - /* Restore SIGPIPE to its original handler function */ - SIGNAL_RELEASE return LOGSQL_QUERY_NOTABLE; } - - /* Restore SIGPIPE to its original handler function */ - SIGNAL_RELEASE return LOGSQL_QUERY_FAIL; } @@ -175,7 +173,7 @@ char *create_prefix = "create table if not exists `"; char *create_suffix = NULL; - char *create_sql; + char *create_sql = NULL; MYSQL *dblink = (MYSQL *)db->handle; @@ -185,7 +183,12 @@ switch (table_type) { case LOGSQL_TABLE_ACCESS: - create_suffix = + /* + RS: no space allocated for create_suffix, so use apr_pstrcat here, + instead of = + */ + + create_suffix = apr_pstrcat(r->pool, "` (id char(19),\ agent varchar(255),\ bytes_sent int unsigned,\ @@ -212,16 +215,13 @@ time_stamp int unsigned,\ virtual_host varchar(255),\ bytes_in int unsigned,\ - bytes_out int unsigned)"; + bytes_out int unsigned)", NULL); break; case LOGSQL_TABLE_COOKIES: case LOGSQL_TABLE_HEADERSIN: case LOGSQL_TABLE_HEADERSOUT: case LOGSQL_TABLE_NOTES: - create_suffix = - "` (id char(19),\ - item varchar(80),\ - val varchar(80))"; + create_suffix = apr_pstrcat(r->pool,"` (id char(19), item varchar(80), val varchar(80))", NULL); break; } @@ -238,17 +238,17 @@ if (!dblink) { return LOGSQL_QUERY_NOLINK; } + /* A failed mysql_query() may send a SIGPIPE, so we ignore that signal momentarily. */ SIGNAL_GRAB + retval = mysql_query(dblink, create_sql); + SIGNAL_RELEASE - /* Run the create query */ - if ((retval = mysql_query(dblink, create_sql))) { + if (retval) { log_error(APLOG_MARK,APLOG_ERR,0, r->server,"failed to create table: %s", table_name); - SIGNAL_RELEASE return LOGSQL_TABLE_FAIL; } - SIGNAL_RELEASE return LOGSQL_TABLE_SUCCESS; }