Fix a problem with quoting in sqlite3 cdr module..
authorJason Parker <jparker@digium.com>
Thu, 6 Dec 2007 21:57:28 +0000 (21:57 +0000)
committerJason Parker <jparker@digium.com>
Thu, 6 Dec 2007 21:57:28 +0000 (21:57 +0000)
Closes issue #11070, patch by seanbright.

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@91598 65c4cc65-6c06-0410-ace0-fbb531ad65f3

cdr/cdr_sqlite3_custom.c

index 7eff53d..87718db 100644 (file)
@@ -61,8 +61,95 @@ static char *name = "cdr_sqlite3_custom";
 static sqlite3 *db = NULL;
 
 static char table[80];
-static char columns[1024];
-static char values[1024];
+static char *columns;
+
+struct values {
+       char *expression;
+       AST_LIST_ENTRY(values) list;
+};
+
+static AST_LIST_HEAD_STATIC(sql_values, values);
+
+static int free_config(void);
+
+static int load_column_config(const char *tmp)
+{
+       char *col = NULL;
+       char *cols = NULL;
+       char *escaped = NULL;
+       struct ast_str *column_string = NULL;
+
+       if (ast_strlen_zero(tmp)) {
+               ast_log(LOG_WARNING, "Column names not specified. Module not loaded.\n");
+               return -1;
+       }
+       if (!(column_string = ast_str_create(1024))) {
+               ast_log(LOG_ERROR, "Out of memory creating temporary buffer for column list for table '%s.'\n", table);
+               return -1;
+       }
+       if (!(cols = ast_strdup(tmp))) {
+               ast_log(LOG_ERROR, "Out of memory creating temporary buffer for column list for table '%s.'\n", table);
+               ast_free(column_string);
+               return -1;
+       }
+       while ((col = strsep(&cols, ","))) {
+               col = ast_strip(col);
+               escaped = sqlite3_mprintf("%q", col);
+               if (!escaped) {
+                       ast_log(LOG_ERROR, "Out of memory creating entry for column '%s' in table '%s.'\n", col, table);
+                       ast_free(column_string);
+                       ast_free(cols);
+                       return -1;
+               }
+               if (!column_string->used)
+                       ast_str_set(&column_string, 0, escaped);
+               else
+                       ast_str_append(&column_string, 0, ",%s", escaped);
+               sqlite3_free(escaped);
+       }
+       if (!(columns = ast_strdup(column_string->str))) {
+               ast_log(LOG_ERROR, "Out of memory copying columns string for table '%s.'\n", table);
+               ast_free(column_string);
+               ast_free(cols);
+               return -1;
+       }
+       ast_free(column_string);
+       ast_free(cols);
+
+       return 0;
+}
+
+static int load_values_config(const char *tmp)
+{
+       char *val = NULL;
+       char *vals = NULL;
+       struct values *value = NULL;
+
+       if (ast_strlen_zero(tmp)) {
+               ast_log(LOG_WARNING, "Values not specified. Module not loaded.\n");
+               return -1;
+       }
+       if (!(vals = ast_strdup(tmp))) {
+               ast_log(LOG_ERROR, "Out of memory creating temporary buffer for value '%s'\n", tmp);
+               return -1;
+       }
+       while ((val = strsep(&vals, ","))) {
+               /* Strip the single quotes off if they are there */
+               val = ast_strip_quoted(val, "'", "'");
+               value = ast_calloc(sizeof(char), sizeof(*value) + strlen(val) + 1);
+               if (!value) {
+                       ast_log(LOG_ERROR, "Out of memory creating entry for value '%s'\n", val);
+                       ast_free(vals);
+                       return -1;
+               }
+               value->expression = (char *) value + sizeof(*value);
+               ast_copy_string(value->expression, val, strlen(val) + 1);
+               AST_LIST_INSERT_TAIL(&sql_values, value, list);
+       }
+       ast_free(vals);
+
+       return 0;
+}
 
 static int load_config(int reload)
 {
@@ -73,20 +160,20 @@ static int load_config(int reload)
 
        if (!(cfg = ast_config_load(config_file, config_flags))) {
                if (reload)
-                       ast_log(LOG_WARNING, "%s: Failed to reload configuration file.\n", name);
-               else {
-                       ast_log(LOG_WARNING,
-                                       "%s: Failed to load configuration file. Module not activated.\n",
-                                       name);
-               }
+                       ast_log(LOG_WARNING, "Failed to reload configuration file.\n");
+               else
+                       ast_log(LOG_WARNING, "Failed to load configuration file. Module not activated.\n");
                return -1;
        } else if (cfg == CONFIG_STATUS_FILEUNCHANGED)
                return 0;
 
+       if (reload)
+               free_config();
+
        ast_mutex_lock(&lock);
 
        if (!(mappingvar = ast_variable_browse(cfg, "master"))) {
-               /* nothing configured */
+               /* Nothing configured */
                ast_mutex_unlock(&lock);
                ast_config_destroy(cfg);
                return 0;
@@ -97,49 +184,56 @@ static int load_config(int reload)
        if (!ast_strlen_zero(tmp))
                ast_copy_string(table, tmp, sizeof(table));
        else {
-               ast_log(LOG_WARNING, "%s: Table name not specified.  Assuming cdr.\n", name);
+               ast_log(LOG_WARNING, "Table name not specified.  Assuming cdr.\n");
                strcpy(table, "cdr");
        }
 
+       /* Columns */
        tmp = ast_variable_retrieve(cfg, "master", "columns");
-       if (!ast_strlen_zero(tmp))
-               ast_copy_string(columns, tmp, sizeof(columns));
-       else {
-               ast_log(LOG_WARNING, "%s: Column names not specified. Module not loaded.\n",
-                               name);
+       if (load_column_config(tmp)) {
                ast_mutex_unlock(&lock);
                ast_config_destroy(cfg);
+               free_config();
                return -1;
        }
 
+       /* Values */
        tmp = ast_variable_retrieve(cfg, "master", "values");
-       if (!ast_strlen_zero(tmp))
-               ast_copy_string(values, tmp, sizeof(values));
-       else {
-               ast_log(LOG_WARNING, "%s: Values not specified. Module not loaded.\n", name);
+       if (load_values_config(tmp)) {
                ast_mutex_unlock(&lock);
                ast_config_destroy(cfg);
+               free_config();
                return -1;
        }
 
-       ast_mutex_unlock(&lock);
+       ast_verb(3, "cdr_sqlite3_custom: Logging CDR records to table '%s' in 'master.db'\n", table);
 
+       ast_mutex_unlock(&lock);
        ast_config_destroy(cfg);
 
        return 0;
 }
 
-/* assumues 'to' buffer is at least strlen(from) * 2 + 1 bytes */
-static int do_escape(char *to, const char *from)
+static int free_config(void)
 {
-       char *out = to;
+       struct values *value;
+
+       ast_mutex_lock(&lock);
+
+       if (db) {
+               sqlite3_close(db);
+               db = NULL;
+       }
 
-       for (; *from; from++) {
-               if (*from == '\'' || *from == '\\')
-                       *out++ = *from;
-               *out++ = *from;
+       if (columns) {
+               ast_free(columns);
+               columns = NULL;
        }
-       *out = '\0';
+
+       while ((value = AST_LIST_REMOVE_HEAD(&sql_values, list)))
+               ast_free(value);
+
+       ast_mutex_unlock(&lock);
 
        return 0;
 }
@@ -147,36 +241,50 @@ static int do_escape(char *to, const char *from)
 static int sqlite3_log(struct ast_cdr *cdr)
 {
        int res = 0;
-       char *zErr = 0;
-       char *sql_cmd;
+       char *error = NULL;
+       char *sql = NULL;
        struct ast_channel dummy = { 0, };
-       int count;
+       int count = 0;
 
-       { /* Make it obvious that only sql_cmd should be used outside of this block */
-               char *sql_tmp_cmd;
-               char sql_insert_cmd[2048];
-               sql_tmp_cmd = sqlite3_mprintf("INSERT INTO %q (%q) VALUES (%q)", table, columns, values);
+       { /* Make it obvious that only sql should be used outside of this block */
+               char *escaped;
+               char subst_buf[2048];
+               struct values *value;
+               struct ast_str *value_string = ast_str_create(1024);
                dummy.cdr = cdr;
-               pbx_substitute_variables_helper(&dummy, sql_tmp_cmd, sql_insert_cmd, sizeof(sql_insert_cmd) - 1);
-               sqlite3_free(sql_tmp_cmd);
-               sql_cmd = alloca(strlen(sql_insert_cmd) * 2 + 1);
-               do_escape(sql_cmd, sql_insert_cmd);
+               AST_LIST_TRAVERSE(&sql_values, value, list) {
+                       memset(subst_buf, 0, sizeof(subst_buf));
+                       pbx_substitute_variables_helper(&dummy, value->expression, subst_buf, sizeof(subst_buf) - 1);
+                       escaped = sqlite3_mprintf("%q", subst_buf);
+                       if (!value_string->used)
+                               ast_str_append(&value_string, 0, "'%s'", escaped);
+                       else
+                               ast_str_append(&value_string, 0, ",'%s'", escaped);
+                       sqlite3_free(escaped);
+               }
+               sql = sqlite3_mprintf("INSERT INTO %q (%s) VALUES (%s)", table, columns, value_string->str);
+               ast_debug(1, "About to log: %s\n", sql);
+               ast_free(value_string);
        }
 
        ast_mutex_lock(&lock);
 
+       /* XXX This seems awful arbitrary... */
        for (count = 0; count < 5; count++) {
-               res = sqlite3_exec(db, sql_cmd, NULL, NULL, &zErr);
+               res = sqlite3_exec(db, sql, NULL, NULL, &error);
                if (res != SQLITE_BUSY && res != SQLITE_LOCKED)
                        break;
                usleep(200);
        }
 
-       if (zErr) {
-               ast_log(LOG_ERROR, "%s: %s. sentence: %s.\n", name, zErr, sql_cmd);
-               sqlite3_free(zErr);
+       if (error) {
+               ast_log(LOG_ERROR, "%s. SQL: %s.\n", error, sql);
+               sqlite3_free(error);
        }
 
+       if (sql)
+               sqlite3_free(sql);
+
        ast_mutex_unlock(&lock);
 
        return res;
@@ -184,8 +292,7 @@ static int sqlite3_log(struct ast_cdr *cdr)
 
 static int unload_module(void)
 {
-       if (db)
-               sqlite3_close(db);
+       free_config();
 
        ast_cdr_unregister(name);
 
@@ -194,48 +301,43 @@ static int unload_module(void)
 
 static int load_module(void)
 {
-       char *zErr;
-       char fn[PATH_MAX];
+       char *error;
+       char filename[PATH_MAX];
        int res;
-       char *sql_cmd;
+       char *sql;
 
        if (!load_config(0)) {
                res = ast_cdr_register(name, desc, sqlite3_log);
                if (res) {
-                       ast_log(LOG_ERROR, "%s: Unable to register custom SQLite3 CDR handling\n", name);
+                       ast_log(LOG_ERROR, "Unable to register custom SQLite3 CDR handling\n");
+                       free_config();
                        return AST_MODULE_LOAD_DECLINE;
                }
        } else
                return AST_MODULE_LOAD_DECLINE;
 
        /* is the database there? */
-       snprintf(fn, sizeof(fn), "%s/master.db", ast_config_AST_LOG_DIR);
-       res = sqlite3_open(fn, &db);
-       if (!db) {
-               ast_log(LOG_ERROR, "%s: Could not open database %s.\n", name, fn);
-               sqlite3_free(zErr);
+       snprintf(filename, sizeof(filename), "%s/master.db", ast_config_AST_LOG_DIR);
+       res = sqlite3_open(filename, &db);
+       if (res != SQLITE_OK) {
+               ast_log(LOG_ERROR, "Could not open database %s.\n", filename);
+               free_config();
                return AST_MODULE_LOAD_DECLINE;
        }
 
        /* is the table there? */
-       sql_cmd = sqlite3_mprintf("SELECT COUNT(AcctId) FROM %q;", table);
-       res = sqlite3_exec(db, sql_cmd, NULL, NULL, NULL);
-       sqlite3_free(sql_cmd);
-       if (res) {
-               sql_cmd = sqlite3_mprintf("CREATE TABLE %q (AcctId INTEGER PRIMARY KEY,%q)", table, columns);
-               res = sqlite3_exec(db, sql_cmd, NULL, NULL, &zErr);
-               sqlite3_free(sql_cmd);
-               if (zErr) {
-                       ast_log(LOG_WARNING, "%s: %s.\n", name, zErr);
-                       sqlite3_free(zErr);
-                       return 0;
-               }
-
-               if (res) {
-                       ast_log(LOG_ERROR, "%s: Unable to create table '%s': %s.\n", name, table, zErr);
-                       sqlite3_free(zErr);
-                       if (db)
-                               sqlite3_close(db);
+       sql = sqlite3_mprintf("SELECT COUNT(AcctId) FROM %q;", table);
+       res = sqlite3_exec(db, sql, NULL, NULL, NULL);
+       sqlite3_free(sql);
+       if (res != SQLITE_OK) {
+               /* We don't use %q for the column list here since we already escaped when building it */
+               sql = sqlite3_mprintf("CREATE TABLE %q (AcctId INTEGER PRIMARY KEY, %s)", table, columns);
+               res = sqlite3_exec(db, sql, NULL, NULL, &error);
+               sqlite3_free(sql);
+               if (res != SQLITE_OK) {
+                       ast_log(LOG_WARNING, "Unable to create table '%s': %s.\n", table, error);
+                       sqlite3_free(error);
+                       free_config();
                        return AST_MODULE_LOAD_DECLINE;
                }
        }