<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/7757">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">cdr_mysql: Make sure connection charset is always set<br><br>When the MYSQL_OPT_RECONNECT option is enabled, the MySQL client API<br>will transparently reconnect when it needs to. Ideally this simplifies<br>our code, but when this reconnection occurs all connection state is<br>lost. Because we are not notified that this has happened, we don't know<br>to set our character set again (with "SET NAMES 'xyz'").<br><br>Rather than calling SET NAMES, we instead set the MYSQL_SET_CHARSET_NAME<br>option which will do it for us under the hood on each connect. This<br>option has been present in the MySQL C API for at least 15 years, so it<br>should be safe for most installations.<br><br>I also snuck a few other changes into this patch:<br><br>* Default the MySQL port to MYSQL_PORT (3306) instead of 0 if it's not<br> defined.<br><br>* Fix some erroneous and/or silly checks on the contents of the<br> configuration ast_str values.<br><br>ASTERISK-27366 #close<br>Reported by: Halil İbrahim YILDIZ<br><br>Change-Id: I36bf8dc5d5f83584e803b3b1a151dea9396ab8f5<br>---<br>M addons/cdr_mysql.c<br>1 file changed, 21 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/addons/cdr_mysql.c b/addons/cdr_mysql.c<br>index b2e0a4c..00c75dd 100644<br>--- a/addons/cdr_mysql.c<br>+++ b/addons/cdr_mysql.c<br>@@ -128,7 +128,7 @@<br> else<br> snprintf(status, 255, "Connected to %s@%s", ast_str_buffer(dbname), ast_str_buffer(hostname));<br> <br>- if (!ast_strlen_zero(ast_str_buffer(dbuser)))<br>+ if (ast_str_strlen(dbuser))<br> snprintf(status2, 99, " with username %s", ast_str_buffer(dbuser));<br> if (ast_str_strlen(dbtable))<br> snprintf(status2, 99, " using table %s", ast_str_buffer(dbtable));<br>@@ -150,6 +150,16 @@<br> static struct ast_cli_entry cdr_mysql_status_cli[] = {<br> AST_CLI_DEFINE(handle_cli_cdr_mysql_status, "Show connection status of cdr_mysql"),<br> };<br>+<br>+static void configure_connection_charset(void)<br>+{<br>+ if (ast_str_strlen(dbcharset)) {<br>+ const char *charset = ast_str_buffer(dbcharset);<br>+ if (mysql_options(&mysql, MYSQL_SET_CHARSET_NAME, charset)) {<br>+ ast_log(LOG_WARNING, "Failed to set connection charset. Data inserted might be invalid.\n");<br>+ }<br>+ }<br>+}<br> <br> static int mysql_log(struct ast_cdr *cdr)<br> {<br>@@ -183,15 +193,13 @@<br> if (ssl_ca || ssl_cert || ssl_key) {<br> 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);<br> }<br>+<br>+ configure_connection_charset();<br>+<br> 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)) {<br> connected = 1;<br> connect_time = time(NULL);<br> records = 0;<br>- if (dbcharset) {<br>- ast_str_set(&sql1, 0, "SET NAMES '%s'", ast_str_buffer(dbcharset));<br>- mysql_real_query(&mysql, ast_str_buffer(sql1), ast_str_strlen(sql1));<br>- ast_debug(1, "SQL command as follows: %s\n", ast_str_buffer(sql1));<br>- }<br> } else {<br> ast_log(LOG_ERROR, "Cannot connect to database server %s: (%d) %s\n", ast_str_buffer(hostname), mysql_errno(&mysql), mysql_error(&mysql));<br> connected = 0;<br>@@ -493,7 +501,7 @@<br> res |= my_load_config_string(cfg, "global", "ssl_cert", &ssl_cert, "");<br> res |= my_load_config_string(cfg, "global", "ssl_key", &ssl_key, "");<br> <br>- res |= my_load_config_number(cfg, "global", "port", &dbport, 0);<br>+ res |= my_load_config_number(cfg, "global", "port", &dbport, MYSQL_PORT);<br> res |= my_load_config_number(cfg, "global", "timeout", &timeout, 0);<br> res |= my_load_config_string(cfg, "global", "compat", &compat, "no");<br> res |= my_load_config_string(cfg, "global", "cdrzone", &cdrzone, "");<br>@@ -533,15 +541,16 @@<br> ast_debug(1, "Got hostname of %s\n", ast_str_buffer(hostname));<br> ast_debug(1, "Got port of %d\n", dbport);<br> ast_debug(1, "Got a timeout of %d\n", timeout);<br>- if (dbsock)<br>+ if (ast_str_strlen(dbsock)) {<br> ast_debug(1, "Got sock file of %s\n", ast_str_buffer(dbsock));<br>+ }<br> ast_debug(1, "Got user of %s\n", ast_str_buffer(dbuser));<br> ast_debug(1, "Got dbname of %s\n", ast_str_buffer(dbname));<br> ast_debug(1, "Got password of %s\n", ast_str_buffer(password));<br> ast_debug(1, "%sunning in calldate compatibility mode\n", calldate_compat ? "R" : "Not r");<br> ast_debug(1, "Dates and times are localized to %s\n", S_OR(ast_str_buffer(cdrzone), "local timezone"));<br> <br>- if (dbcharset) {<br>+ if (ast_str_strlen(dbcharset)) {<br> ast_debug(1, "Got DB charset of %s\n", ast_str_buffer(dbcharset));<br> }<br> <br>@@ -566,6 +575,9 @@<br> NULL, NULL);<br> }<br> temp = dbsock && ast_str_strlen(dbsock) ? ast_str_buffer(dbsock) : NULL;<br>+<br>+ configure_connection_charset();<br>+<br> 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)) {<br> ast_log(LOG_ERROR, "Failed to connect to mysql database %s on %s.\n", ast_str_buffer(dbname), ast_str_buffer(hostname));<br> connected = 0;<br>@@ -575,11 +587,6 @@<br> connected = 1;<br> records = 0;<br> connect_time = time(NULL);<br>- if (dbcharset) {<br>- snprintf(sqldesc, sizeof(sqldesc), "SET NAMES '%s'", ast_str_buffer(dbcharset));<br>- mysql_real_query(&mysql, sqldesc, strlen(sqldesc));<br>- ast_debug(1, "SQL command as follows: %s\n", sqldesc);<br>- }<br> <br> /* Get table description */<br> snprintf(sqldesc, sizeof(sqldesc), "DESC %s", dbtable ? ast_str_buffer(dbtable) : "cdr");<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/7757">change 7757</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7757"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I36bf8dc5d5f83584e803b3b1a151dea9396ab8f5 </div>
<div style="display:none"> Gerrit-Change-Number: 7757 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>