[Asterisk-code-review] res config mysql: Fix broken column type checking (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_config_mysql: Fix broken column type checking
......................................................................


res_config_mysql: Fix broken column type checking

MySQL configuration engine contains a bug in require_mysql(). This
function is used for column type checking in tables. This bug only
affects DATETIME, DATE and FLOAT types.

It came from mixing the first condition (switch-case-like
if/then/else), to check the expected column type, with the second
condition, to check the actual column type against the expected column
type. Both conditions must be checked separately in order to avoid the
execution of the wrong block.

ASTERISK-18252 #comment This patch might fix the issue
Reported by: Gareth Blades

ASTERISK-25041 #close
Reported by: Alexandre Fournier
Tested by: Alexandre Fournier

Change-Id: I0b8bf7e68ab938be8e6525a249260cb648cb0bfa
---
M addons/res_config_mysql.c
1 file changed, 36 insertions(+), 30 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, approved; Verified
  Jonathan Rose: Looks good to me, but someone else must approve



diff --git a/addons/res_config_mysql.c b/addons/res_config_mysql.c
index 299fe56..76bdeed 100644
--- a/addons/res_config_mysql.c
+++ b/addons/res_config_mysql.c
@@ -1202,38 +1202,44 @@
 							PICK_WHICH_ALTER_ACTION(bigint)
 						}
 					}
-				} else if (strncmp(column->type, "float", 5) == 0 && !ast_rq_is_int(type) && type != RQ_FLOAT) {
-					if (table->database->requirements == RQ_WARN) {
-						ast_log(LOG_WARNING, "Realtime table %s@%s: Column %s cannot be a %s\n", tablename, database, column->name, column->type);
-						res = -1;
-					} else if (table->database->requirements == RQ_CREATECLOSE && modify_mysql(database, tablename, column, type, size) == 0) {
-						table_altered = 1;
-					} else if (table->database->requirements == RQ_CREATECHAR && modify_mysql(database, tablename, column, RQ_CHAR, size) == 0) {
-						table_altered = 1;
-					} else {
-						res = -1;
+				} else if (strncmp(column->type, "float", 5) == 0) {
+					if (!ast_rq_is_int(type) && type != RQ_FLOAT) {
+						if (table->database->requirements == RQ_WARN) {
+							ast_log(LOG_WARNING, "Realtime table %s@%s: Column %s cannot be a %s\n", tablename, database, column->name, column->type);
+							res = -1;
+						} else if (table->database->requirements == RQ_CREATECLOSE && modify_mysql(database, tablename, column, type, size) == 0) {
+							table_altered = 1;
+						} else if (table->database->requirements == RQ_CREATECHAR && modify_mysql(database, tablename, column, RQ_CHAR, size) == 0) {
+							table_altered = 1;
+						} else {
+							res = -1;
+						}
 					}
-				} else if ((strncmp(column->type, "datetime", 8) == 0 || strncmp(column->type, "timestamp", 9) == 0) && type != RQ_DATETIME) {
-					if (table->database->requirements == RQ_WARN) {
-						ast_log(LOG_WARNING, "Realtime table %s@%s: Column %s cannot be a %s\n", tablename, database, column->name, column->type);
-						res = -1;
-					} else if (table->database->requirements == RQ_CREATECLOSE && modify_mysql(database, tablename, column, type, size) == 0) {
-						table_altered = 1;
-					} else if (table->database->requirements == RQ_CREATECHAR && modify_mysql(database, tablename, column, RQ_CHAR, size) == 0) {
-						table_altered = 1;
-					} else {
-						res = -1;
+				} else if (strncmp(column->type, "datetime", 8) == 0 || strncmp(column->type, "timestamp", 9) == 0) {
+					if (type != RQ_DATETIME) {
+						if (table->database->requirements == RQ_WARN) {
+							ast_log(LOG_WARNING, "Realtime table %s@%s: Column %s cannot be a %s\n", tablename, database, column->name, column->type);
+							res = -1;
+						} else if (table->database->requirements == RQ_CREATECLOSE && modify_mysql(database, tablename, column, type, size) == 0) {
+							table_altered = 1;
+						} else if (table->database->requirements == RQ_CREATECHAR && modify_mysql(database, tablename, column, RQ_CHAR, size) == 0) {
+							table_altered = 1;
+						} else {
+							res = -1;
+						}
 					}
-				} else if ((strncmp(column->type, "date", 4) == 0) && type != RQ_DATE) {
-					if (table->database->requirements == RQ_WARN) {
-						ast_log(LOG_WARNING, "Realtime table %s@%s: Column %s cannot be a %s\n", tablename, database, column->name, column->type);
-						res = -1;
-					} else if (table->database->requirements == RQ_CREATECLOSE && modify_mysql(database, tablename, column, type, size) == 0) {
-						table_altered = 1;
-					} else if (table->database->requirements == RQ_CREATECHAR && modify_mysql(database, tablename, column, RQ_CHAR, size) == 0) {
-						table_altered = 1;
-					} else {
-						res = -1;
+				} else if (strncmp(column->type, "date", 4) == 0) {
+					if (type != RQ_DATE) {
+						if (table->database->requirements == RQ_WARN) {
+							ast_log(LOG_WARNING, "Realtime table %s@%s: Column %s cannot be a %s\n", tablename, database, column->name, column->type);
+							res = -1;
+						} else if (table->database->requirements == RQ_CREATECLOSE && modify_mysql(database, tablename, column, type, size) == 0) {
+							table_altered = 1;
+						} else if (table->database->requirements == RQ_CREATECHAR && modify_mysql(database, tablename, column, RQ_CHAR, size) == 0) {
+							table_altered = 1;
+						} else {
+							res = -1;
+						}
 					}
 				} else { /* Other, possibly unsupported types? */
 					if (table->database->requirements == RQ_WARN) {

-- 
To view, visit https://gerrit.asterisk.org/420
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0b8bf7e68ab938be8e6525a249260cb648cb0bfa
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexandre Fournier <alexandre.fournier at kiplink.fr>
Gerrit-Reviewer: Jonathan Rose <jrose at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>