From bae301c18c25a16bad95e4f779e354b0285b18d2 Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Wed, 13 Dec 2017 16:43:18 -0500 Subject: cdr_mysql: Make sure connection charset is always set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the MYSQL_OPT_RECONNECT option is enabled, the MySQL client API will transparently reconnect when it needs to. Ideally this simplifies our code, but when this reconnection occurs all connection state is lost. Because we are not notified that this has happened, we don't know to set our character set again (with "SET NAMES 'xyz'"). Rather than calling SET NAMES, we instead set the MYSQL_SET_CHARSET_NAME option which will do it for us under the hood on each connect. This option has been present in the MySQL C API for at least 15 years, so it should be safe for most installations. I also snuck a few other changes into this patch: * Default the MySQL port to MYSQL_PORT (3306) instead of 0 if it's not defined. * Fix some erroneous and/or silly checks on the contents of the configuration ast_str values. ASTERISK-27366 #close Reported by: Halil İbrahim YILDIZ Change-Id: I36bf8dc5d5f83584e803b3b1a151dea9396ab8f5 --- addons/cdr_mysql.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/addons/cdr_mysql.c b/addons/cdr_mysql.c index c5cf10892..53ea92359 100644 --- a/addons/cdr_mysql.c +++ b/addons/cdr_mysql.c @@ -128,7 +128,7 @@ static char *handle_cli_cdr_mysql_status(struct ast_cli_entry *e, int cmd, struc else snprintf(status, 255, "Connected to %s@%s", ast_str_buffer(dbname), ast_str_buffer(hostname)); - if (!ast_strlen_zero(ast_str_buffer(dbuser))) + if (ast_str_strlen(dbuser)) snprintf(status2, 99, " with username %s", ast_str_buffer(dbuser)); if (ast_str_strlen(dbtable)) snprintf(status2, 99, " using table %s", ast_str_buffer(dbtable)); @@ -158,6 +158,16 @@ static struct ast_cli_entry cdr_mysql_status_cli[] = { AST_CLI_DEFINE(handle_cli_cdr_mysql_status, "Show connection status of cdr_mysql"), }; +static void configure_connection_charset(void) +{ + if (ast_str_strlen(dbcharset)) { + const char *charset = ast_str_buffer(dbcharset); + if (mysql_options(&mysql, MYSQL_SET_CHARSET_NAME, charset)) { + ast_log(LOG_WARNING, "Failed to set connection charset. Data inserted might be invalid.\n"); + } + } +} + static int mysql_log(struct ast_cdr *cdr) { struct ast_str *sql1 = ast_str_thread_get(&sql1_buf, 1024), *sql2 = ast_str_thread_get(&sql2_buf, 1024); @@ -190,15 +200,13 @@ db_reconnect: if (ssl_ca || ssl_cert || ssl_key) { mysql_ssl_set(&mysql, ssl_key ? ast_str_buffer(ssl_key) : NULL, ssl_cert ? ast_str_buffer(ssl_cert) : NULL, ssl_ca ? ast_str_buffer(ssl_ca) : NULL, NULL, NULL); } + + configure_connection_charset(); + if (mysql_real_connect(&mysql, ast_str_buffer(hostname), ast_str_buffer(dbuser), ast_str_buffer(password), ast_str_buffer(dbname), dbport, dbsock && ast_str_strlen(dbsock) ? ast_str_buffer(dbsock) : NULL, ssl_ca ? CLIENT_SSL : 0)) { connected = 1; connect_time = time(NULL); records = 0; - if (dbcharset) { - ast_str_set(&sql1, 0, "SET NAMES '%s'", ast_str_buffer(dbcharset)); - mysql_real_query(&mysql, ast_str_buffer(sql1), ast_str_strlen(sql1)); - ast_debug(1, "SQL command as follows: %s\n", ast_str_buffer(sql1)); - } } else { ast_log(LOG_ERROR, "Cannot connect to database server %s: (%d) %s\n", ast_str_buffer(hostname), mysql_errno(&mysql), mysql_error(&mysql)); connected = 0; @@ -500,7 +508,7 @@ static int my_load_module(int reload) res |= my_load_config_string(cfg, "global", "ssl_cert", &ssl_cert, ""); res |= my_load_config_string(cfg, "global", "ssl_key", &ssl_key, ""); - res |= my_load_config_number(cfg, "global", "port", &dbport, 0); + res |= my_load_config_number(cfg, "global", "port", &dbport, MYSQL_PORT); res |= my_load_config_number(cfg, "global", "timeout", &timeout, 0); res |= my_load_config_string(cfg, "global", "compat", &compat, "no"); res |= my_load_config_string(cfg, "global", "cdrzone", &cdrzone, ""); @@ -540,15 +548,16 @@ static int my_load_module(int reload) ast_debug(1, "Got hostname of %s\n", ast_str_buffer(hostname)); ast_debug(1, "Got port of %d\n", dbport); ast_debug(1, "Got a timeout of %d\n", timeout); - if (dbsock) + if (ast_str_strlen(dbsock)) { ast_debug(1, "Got sock file of %s\n", ast_str_buffer(dbsock)); + } ast_debug(1, "Got user of %s\n", ast_str_buffer(dbuser)); ast_debug(1, "Got dbname of %s\n", ast_str_buffer(dbname)); ast_debug(1, "Got password of %s\n", ast_str_buffer(password)); ast_debug(1, "%sunning in calldate compatibility mode\n", calldate_compat ? "R" : "Not r"); ast_debug(1, "Dates and times are localized to %s\n", S_OR(ast_str_buffer(cdrzone), "local timezone")); - if (dbcharset) { + if (ast_str_strlen(dbcharset)) { ast_debug(1, "Got DB charset of %s\n", ast_str_buffer(dbcharset)); } @@ -573,6 +582,9 @@ static int my_load_module(int reload) NULL, NULL); } temp = dbsock && ast_str_strlen(dbsock) ? ast_str_buffer(dbsock) : NULL; + + configure_connection_charset(); + if (!mysql_real_connect(&mysql, ast_str_buffer(hostname), ast_str_buffer(dbuser), ast_str_buffer(password), ast_str_buffer(dbname), dbport, temp, ssl_ca && ast_str_strlen(ssl_ca) ? CLIENT_SSL : 0)) { ast_log(LOG_ERROR, "Failed to connect to mysql database %s on %s.\n", ast_str_buffer(dbname), ast_str_buffer(hostname)); connected = 0; @@ -582,11 +594,6 @@ static int my_load_module(int reload) connected = 1; records = 0; connect_time = time(NULL); - if (dbcharset) { - snprintf(sqldesc, sizeof(sqldesc), "SET NAMES '%s'", ast_str_buffer(dbcharset)); - mysql_real_query(&mysql, sqldesc, strlen(sqldesc)); - ast_debug(1, "SQL command as follows: %s\n", sqldesc); - } /* Get table description */ snprintf(sqldesc, sizeof(sqldesc), "DESC %s", dbtable ? ast_str_buffer(dbtable) : "cdr"); -- cgit v1.2.3