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 <kmo...@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 <kmo...@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; } -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
