[asterisk-commits] kmoore: branch 1.8 r422112 - in /branches/1.8: channels/ include/asterisk/ ma...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Aug 27 09:25:43 CDT 2014
Author: kmoore
Date: Wed Aug 27 09:25:34 2014
New Revision: 422112
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=422112
Log:
CallerID: Fix parsing of malformed callerid
This allows the callerid parsing function to handle malformed input
strings and strings containing escaped and unescaped double quotes.
This also adds a unittest to cover many of the cases where the parsing
algorithm previously failed.
Review: https://reviewboard.asterisk.org/r/3923/
Added:
branches/1.8/tests/test_callerid.c (with props)
Modified:
branches/1.8/channels/chan_sip.c
branches/1.8/include/asterisk/utils.h
branches/1.8/main/callerid.c
branches/1.8/main/utils.c
branches/1.8/tests/test_utils.c
Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=422112&r1=422111&r2=422112
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Wed Aug 27 09:25:34 2014
@@ -11251,6 +11251,7 @@
{
struct ast_str *tmp = ast_str_alloca(256);
char tmp2[256];
+ char lid_name_buf[128];
char *lid_num;
char *lid_name;
int lid_pres;
@@ -11278,6 +11279,7 @@
if (!lid_name) {
lid_name = lid_num;
}
+ ast_escape_quoted(lid_name, lid_name_buf, sizeof(lid_name_buf));
lid_pres = ast_party_id_presentation(&p->owner->connected.id);
if (((lid_pres & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED) &&
@@ -11301,7 +11303,7 @@
if (ast_test_flag(&p->flags[1], SIP_PAGE2_TRUST_ID_OUTBOUND) != SIP_PAGE2_TRUST_ID_OUTBOUND_LEGACY) {
/* trust_id_outbound = yes - Always give full information even if it's private, but append a privacy header
* When private data is included */
- ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name, lid_num, fromdomain);
+ ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name_buf, lid_num, fromdomain);
if ((lid_pres & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED) {
add_header(req, "Privacy", "id");
}
@@ -11309,14 +11311,14 @@
/* trust_id_outbound = legacy - behave in a non RFC-3325 compliant manner and send anonymized data when
* when handling private data. */
if ((lid_pres & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED) {
- ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name, lid_num, fromdomain);
+ ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>", lid_name_buf, lid_num, fromdomain);
} else {
ast_str_set(&tmp, -1, "%s", anonymous_string);
}
}
add_header(req, "P-Asserted-Identity", ast_str_buffer(tmp));
} else {
- ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>;party=%s", lid_name, lid_num, fromdomain, p->outgoing_call ? "calling" : "called");
+ ast_str_set(&tmp, -1, "\"%s\" <sip:%s@%s>;party=%s", lid_name_buf, lid_num, fromdomain, p->outgoing_call ? "calling" : "called");
switch (lid_pres) {
case AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED:
@@ -12550,8 +12552,11 @@
snprintf(header_text, sizeof(header_text), "<sip:%s@%s>;reason=%s", diverting_number,
ast_sockaddr_stringify_host_remote(&pvt->ourip), reason);
} else {
+ char diverting_name_buf[128];
+
+ ast_escape_quoted(diverting_name, diverting_name_buf, sizeof(diverting_name_buf));
snprintf(header_text, sizeof(header_text), "\"%s\" <sip:%s@%s>;reason=%s",
- diverting_name, diverting_number,
+ diverting_name_buf, diverting_number,
ast_sockaddr_stringify_host_remote(&pvt->ourip), reason);
}
Modified: branches/1.8/include/asterisk/utils.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/utils.h?view=diff&rev=422112&r1=422111&r2=422112
==============================================================================
--- branches/1.8/include/asterisk/utils.h (original)
+++ branches/1.8/include/asterisk/utils.h Wed Aug 27 09:25:34 2014
@@ -299,6 +299,15 @@
* \return a pointer to the escaped string
*/
char *ast_escape_quoted(const char *string, char *outbuf, int buflen);
+
+/*!
+ * \brief Unescape quotes in a string
+ *
+ * \param quote_str The string with quotes to be unescaped
+ *
+ * \note This function mutates the passed-in string.
+ */
+void ast_unescape_quoted(char *quote_str);
static force_inline void ast_slinear_saturated_add(short *input, short *value)
{
Modified: branches/1.8/main/callerid.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/callerid.c?view=diff&rev=422112&r1=422111&r2=422112
==============================================================================
--- branches/1.8/main/callerid.c (original)
+++ branches/1.8/main/callerid.c Wed Aug 27 09:25:34 2014
@@ -1008,50 +1008,39 @@
int ast_callerid_parse(char *instr, char **name, char **location)
{
- char *ns, *ne, *ls, *le;
-
- /* Try "name" <location> format or name <location> format */
- if ((ls = strrchr(instr, '<')) && (le = strrchr(ls, '>'))) {
- *ls = *le = '\0'; /* location found, trim off the brackets */
+ char *ls, *le, *name_start;
+
+ /* Handle surrounding quotes */
+ instr = ast_strip_quoted(instr, "\"", "\"");
+
+ /* Try "name" <location> format or name <location> format or with a missing > */
+ if ((ls = strrchr(instr, '<'))) {
+ if ((le = strrchr(ls, '>'))) {
+ *le = '\0'; /* location found, trim off the brackets */
+ }
+ *ls = '\0';
*location = ls + 1; /* and this is the result */
- if ((ns = strchr(instr, '"')) && (ne = strchr(ns + 1, '"'))) {
- *ns = *ne = '\0'; /* trim off the quotes */
- *name = ns + 1; /* and this is the name */
- } else if (ns) {
- /* An opening quote was found but no closing quote was. The closing
- * quote may actually be after the end of the bracketed number
- */
- if (strchr(le + 1, '\"')) {
- *ns = '\0';
- *name = ns + 1;
- ast_trim_blanks(*name);
- } else {
- *name = NULL;
- }
- } else { /* no quotes, trim off leading and trailing spaces */
- *name = ast_skip_blanks(instr);
- ast_trim_blanks(*name);
- }
+
+ name_start = ast_strip_quoted(instr, "\"", "\"");
} else { /* no valid brackets */
char tmp[256];
ast_copy_string(tmp, instr, sizeof(tmp));
ast_shrink_phone_number(tmp);
if (ast_isphonenumber(tmp)) { /* Assume it's just a location */
- *name = NULL;
+ name_start = NULL;
strcpy(instr, tmp); /* safe, because tmp will always be the same size or smaller than instr */
*location = instr;
} else { /* Assume it's just a name. */
*location = NULL;
- if ((ns = strchr(instr, '"')) && (ne = strchr(ns + 1, '"'))) {
- *ns = *ne = '\0'; /* trim off the quotes */
- *name = ns + 1; /* and this is the name */
- } else { /* no quotes, trim off leading and trailing spaces */
- *name = ast_skip_blanks(instr);
- ast_trim_blanks(*name);
- }
- }
- }
+ name_start = ast_strip_quoted(instr, "\"", "\"");
+ }
+ }
+
+ if (name_start) {
+ ast_unescape_quoted(name_start);
+ }
+ *name = name_start;
return 0;
}
@@ -1078,14 +1067,18 @@
{
if (!unknown)
unknown = "<unknown>";
- if (name && num)
- snprintf(buf, bufsiz, "\"%s\" <%s>", name, num);
- else if (name)
+ if (name && num) {
+ char name_buf[128];
+
+ ast_escape_quoted(name, name_buf, sizeof(name_buf));
+ snprintf(buf, bufsiz, "\"%s\" <%s>", name_buf, num);
+ } else if (name) {
ast_copy_string(buf, name, bufsiz);
- else if (num)
+ } else if (num) {
ast_copy_string(buf, num, bufsiz);
- else
+ } else {
ast_copy_string(buf, unknown, bufsiz);
+ }
return buf;
}
Modified: branches/1.8/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/utils.c?view=diff&rev=422112&r1=422111&r2=422112
==============================================================================
--- branches/1.8/main/utils.c (original)
+++ branches/1.8/main/utils.c Wed Aug 27 09:25:34 2014
@@ -456,6 +456,28 @@
}
return outbuf;
+}
+
+void ast_unescape_quoted(char *quote_str)
+{
+ int esc_pos;
+ int unesc_pos;
+ int quote_str_len = strlen(quote_str);
+
+ for (esc_pos = 0, unesc_pos = 0;
+ esc_pos < quote_str_len;
+ esc_pos++, unesc_pos++) {
+ if (quote_str[esc_pos] == '\\') {
+ /* at least one more char and current is \\ */
+ esc_pos++;
+ if (esc_pos >= quote_str_len) {
+ break;
+ }
+ }
+
+ quote_str[unesc_pos] = quote_str[esc_pos];
+ }
+ quote_str[unesc_pos] = '\0';
}
/*! \brief ast_uri_decode: Decode SIP URI, URN, URL (overwrite the string) */
Added: branches/1.8/tests/test_callerid.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/tests/test_callerid.c?view=auto&rev=422112
==============================================================================
--- branches/1.8/tests/test_callerid.c (added)
+++ branches/1.8/tests/test_callerid.c Wed Aug 27 09:25:34 2014
@@ -1,0 +1,165 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2014, Kinsey Moore
+ *
+ * Kinsey Moore <kmoore at digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*!
+ * \file
+ * \brief Callerid Tests
+ *
+ * \author\verbatim Kinsey Moore <kmoore at digium.com> \endverbatim
+ *
+ * This is an Asterisk test module for callerid functionality
+ * \ingroup tests
+ */
+
+/*** MODULEINFO
+ <depend>TEST_FRAMEWORK</depend>
+ <support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+#include "asterisk/callerid.h"
+
+ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
+
+#include "asterisk/utils.h"
+#include "asterisk/module.h"
+#include "asterisk/test.h"
+
+struct cid_set {
+ char *cid;
+ char *name;
+ char *number;
+};
+
+AST_TEST_DEFINE(parse_nominal)
+{
+ static const struct cid_set cid_sets[] = {
+ {"\"name\" <number>", "name", "number"},
+ {"\" name \" <number>", " name ", "number"},
+ {"name <number>", "name", "number"},
+ {" name <number>", "name", "number"},
+ {"\"\" <number>", NULL, "number"},
+ {"<number>", NULL, "number"},
+ {"name", "name", NULL},
+ {"\"name\"", "name", NULL},
+ {"\"name\" <>", "name", NULL},
+ {"name <>", "name", NULL},
+ {"1234", NULL, "1234"},
+ {"\"na\\\"me\" <number>", "na\"me", "number"},
+ };
+ char *name;
+ char *number;
+ int i;
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = "parse_nominal";
+ info->category = "/main/callerid/";
+ info->summary = "Callerid nominal parse unit test";
+ info->description =
+ "This tests parsing of nominal callerid strings.";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ for (i = 0; i < ARRAY_LEN(cid_sets); i++) {
+ RAII_VAR(char *, callerid, ast_strdup(cid_sets[i].cid), ast_free);
+
+ ast_callerid_parse(callerid, &name, &number);
+ if (!cid_sets[i].name == !ast_strlen_zero(name) || (cid_sets[i].name && strcmp(name, cid_sets[i].name))) {
+ ast_test_status_update(test,
+ "Expected callerid name '%s' instead of '%s'\n",
+ cid_sets[i].name, name);
+ return AST_TEST_FAIL;
+ }
+
+ if (!cid_sets[i].number == !ast_strlen_zero(number) || (cid_sets[i].number && strcmp(number, cid_sets[i].number))) {
+ ast_test_status_update(test,
+ "Expected callerid number '%s' instead of '%s'\n",
+ cid_sets[i].number, number);
+ return AST_TEST_FAIL;
+ }
+ }
+
+ return AST_TEST_PASS;
+}
+
+AST_TEST_DEFINE(parse_off_nominal)
+{
+ static const struct cid_set cid_sets[] = {
+ {"na\\\"me <number>", "na\"me", "number"},
+ {"\"na\"me\" <number>", "na\"me", "number"},
+ {"na\"me <number>", "na\"me", "number"},
+ {"\"name <number>", "\"name", "number"},
+ {"name <number", "name", "number"},
+ {"\"name <number>\"", "name", "number"},
+ };
+ char *name;
+ char *number;
+ int i;
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = "parse_off_nominal";
+ info->category = "/main/callerid/";
+ info->summary = "Callerid off-nominal parse unit test";
+ info->description =
+ "This tests parsing of off-nominal callerid strings.";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ for (i = 0; i < ARRAY_LEN(cid_sets); i++) {
+ RAII_VAR(char *, callerid, ast_strdup(cid_sets[i].cid), ast_free);
+
+ ast_callerid_parse(callerid, &name, &number);
+ if (!cid_sets[i].name == !ast_strlen_zero(name) || (cid_sets[i].name && strcmp(name, cid_sets[i].name))) {
+ ast_test_status_update(test,
+ "Expected callerid name '%s' instead of '%s'\n",
+ cid_sets[i].name, name);
+ return AST_TEST_FAIL;
+ }
+
+ if (!cid_sets[i].number == !ast_strlen_zero(number) || (cid_sets[i].number && strcmp(number, cid_sets[i].number))) {
+ ast_test_status_update(test,
+ "Expected callerid number '%s' instead of '%s'\n",
+ cid_sets[i].number, number);
+ return AST_TEST_FAIL;
+ }
+ }
+
+ return AST_TEST_PASS;
+}
+
+static int unload_module(void)
+{
+ AST_TEST_UNREGISTER(parse_nominal);
+ AST_TEST_UNREGISTER(parse_off_nominal);
+ return 0;
+}
+
+static int load_module(void)
+{
+ AST_TEST_REGISTER(parse_nominal);
+ AST_TEST_REGISTER(parse_off_nominal);
+ return AST_MODULE_LOAD_SUCCESS;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Callerid Parse Tests");
Propchange: branches/1.8/tests/test_callerid.c
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: branches/1.8/tests/test_callerid.c
------------------------------------------------------------------------------
svn:keywords = Author Date Id Revision
Propchange: branches/1.8/tests/test_callerid.c
------------------------------------------------------------------------------
svn:mime-type = text/plain
Modified: branches/1.8/tests/test_utils.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/tests/test_utils.c?view=diff&rev=422112&r1=422111&r2=422112
==============================================================================
--- branches/1.8/tests/test_utils.c (original)
+++ branches/1.8/tests/test_utils.c Wed Aug 27 09:25:34 2014
@@ -394,6 +394,100 @@
return res;
}
+struct quote_set {
+ char *input;
+ char *output;
+};
+
+AST_TEST_DEFINE(quote_mutation)
+{
+ char escaped[64];
+ static const struct quote_set escape_sets[] = {
+ {"\"string\"", "\\\"string\\\""},
+ {"\"string", "\\\"string"},
+ {"string\"", "string\\\""},
+ {"string", "string"},
+ {"str\"ing", "str\\\"ing"},
+ {"\"", "\\\""},
+ {"\\\"", "\\\\\\\""},
+ };
+ int i;
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = "quote_mutation";
+ info->category = "/main/utils/";
+ info->summary = "Test mutation of quotes in strings";
+ info->description =
+ "This tests escaping and unescaping of quotes in strings to "
+ "verify that the original string is recovered.";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ for (i = 0; i < ARRAY_LEN(escape_sets); i++) {
+ ast_escape_quoted(escape_sets[i].input, escaped, sizeof(escaped));
+
+ if (strcmp(escaped, escape_sets[i].output)) {
+ ast_test_status_update(test,
+ "Expected escaped string '%s' instead of '%s'\n",
+ escape_sets[i].output, escaped);
+ return AST_TEST_FAIL;
+ }
+
+ ast_unescape_quoted(escaped);
+ if (strcmp(escaped, escape_sets[i].input)) {
+ ast_test_status_update(test,
+ "Expected unescaped string '%s' instead of '%s'\n",
+ escape_sets[i].input, escaped);
+ return AST_TEST_FAIL;
+ }
+ }
+
+ return AST_TEST_PASS;
+}
+
+AST_TEST_DEFINE(quote_unescaping)
+{
+ static const struct quote_set escape_sets[] = {
+ {"\"string\"", "\"string\""},
+ {"\\\"string\"", "\"string\""},
+ {"\"string\\\"", "\"string\""},
+ {"str\\ing", "string"},
+ {"string\\", "string"},
+ {"\\string", "string"},
+ };
+ int i;
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = "quote_unescaping";
+ info->category = "/main/utils/";
+ info->summary = "Test unescaping of off-nominal strings";
+ info->description =
+ "This tests unescaping of strings which contain a mix of "
+ "escaped and unescaped sequences.";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ for (i = 0; i < ARRAY_LEN(escape_sets); i++) {
+ RAII_VAR(char *, escaped, ast_strdup(escape_sets[i].input), ast_free);
+
+ ast_unescape_quoted(escaped);
+ if (strcmp(escaped, escape_sets[i].output)) {
+ ast_test_status_update(test,
+ "Expected unescaped string '%s' instead of '%s'\n",
+ escape_sets[i].output, escaped);
+ return AST_TEST_FAIL;
+ }
+ }
+
+ return AST_TEST_PASS;
+}
+
static int unload_module(void)
{
AST_TEST_UNREGISTER(uri_encode_decode_test);
@@ -404,6 +498,8 @@
AST_TEST_UNREGISTER(crypto_loaded_test);
AST_TEST_UNREGISTER(adsi_loaded_test);
AST_TEST_UNREGISTER(agi_loaded_test);
+ AST_TEST_UNREGISTER(quote_mutation);
+ AST_TEST_UNREGISTER(quote_unescaping);
return 0;
}
@@ -417,6 +513,8 @@
AST_TEST_REGISTER(crypto_loaded_test);
AST_TEST_REGISTER(adsi_loaded_test);
AST_TEST_REGISTER(agi_loaded_test);
+ AST_TEST_REGISTER(quote_mutation);
+ AST_TEST_REGISTER(quote_unescaping);
return AST_MODULE_LOAD_SUCCESS;
}
More information about the asterisk-commits
mailing list