[asterisk-commits] rmudgett: branch 10 r351646 - in /branches/10: ./ channels/ channels/sip/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Jan 19 17:25:09 CST 2012
Author: rmudgett
Date: Thu Jan 19 17:25:05 2012
New Revision: 351646
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=351646
Log:
Misc minor fixes in reqresp_parser.c and chan_sip.c.
* Fix corner cases in get_calleridname() parsing and ensure that the
output buffer is nul terminated.
* Make get_calleridname() truncate the name it parses if the given buffer
is too small rather than abandoning the parse and not returning anything
for the name. Adjusted get_calleridname_test() unit test to handle the
truncation change.
* Fix get_in_brackets_test() unit test to check the results of
get_in_brackets() correctly.
* Fix parse_name_andor_addr() to not return the address of a local buffer.
This function is currently not used.
* Fix potential NULL pointer dereference in sip_sendtext().
* No need to memset(calleridname) in check_user_full() or tmp_name in
get_name_and_number() because get_calleridname() ensures that it is nul
terminated.
* Reply with an accurate response if get_msg_text() fails in
receive_message(). This is academic in v1.8 because get_msg_text() can
never fail.
........
Merged revisions 351618 from http://svn.asterisk.org/svn/asterisk/branches/1.8
Modified:
branches/10/ (props changed)
branches/10/channels/chan_sip.c
branches/10/channels/sip/reqresp_parser.c
Propchange: branches/10/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: branches/10/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/channels/chan_sip.c?view=diff&rev=351646&r1=351645&r2=351646
==============================================================================
--- branches/10/channels/chan_sip.c (original)
+++ branches/10/channels/chan_sip.c Thu Jan 19 17:25:05 2012
@@ -4475,20 +4475,26 @@
static int sip_sendtext(struct ast_channel *ast, const char *text)
{
struct sip_pvt *dialog = ast->tech_pvt;
- int debug = sip_debug_test_pvt(dialog);
-
- if (!dialog)
+ int debug;
+
+ if (!dialog) {
return -1;
+ }
/* NOT ast_strlen_zero, because a zero-length message is specifically
* allowed by RFC 3428 (See section 10, Examples) */
- if (!text)
+ if (!text) {
return 0;
+ }
if(!is_method_allowed(&dialog->allowed_methods, SIP_MESSAGE)) {
ast_debug(2, "Trying to send MESSAGE to device that does not support it.\n");
return(0);
}
- if (debug)
+
+ debug = sip_debug_test_pvt(dialog);
+ if (debug) {
ast_verbose("Sending text %s on %s\n", text, ast->name);
+ }
+
transmit_message_with_text(dialog, text, 0, 0);
return 0;
}
@@ -16289,7 +16295,6 @@
ast_copy_string(from, sip_get_header(req, "From"), sizeof(from));
/* XXX here tries to map the username for invite things */
- memset(calleridname, 0, sizeof(calleridname));
/* strip the display-name portion off the beginning of the FROM header. */
if (!(of = (char *) get_calleridname(from, calleridname, sizeof(calleridname)))) {
@@ -16509,9 +16514,10 @@
if (get_msg_text2(&buf, req)) {
ast_log(LOG_WARNING, "Unable to retrieve text from %s\n", p->callid);
- transmit_response(p, "202 Accepted", req);
- if (!p->owner)
+ transmit_response(p, "500 Internal Server Error", req);
+ if (!p->owner) {
sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
+ }
return;
}
Modified: branches/10/channels/sip/reqresp_parser.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/channels/sip/reqresp_parser.c?view=diff&rev=351646&r1=351645&r2=351646
==============================================================================
--- branches/10/channels/sip/reqresp_parser.c (original)
+++ branches/10/channels/sip/reqresp_parser.c Thu Jan 19 17:25:05 2012
@@ -685,63 +685,77 @@
char *orig_output = output;
const char *orig_input = input;
+ if (!output || !outputsize) {
+ /* Bad output parameters. Should never happen. */
+ return input;
+ }
+
/* clear any empty characters in the beginning */
input = ast_skip_blanks(input);
- /* no data at all or no storage room? */
- if (!input || *input == '<' || !outputsize || !output) {
- return orig_input;
- }
-
/* make sure the output buffer is initilized */
*orig_output = '\0';
/* make room for '\0' at the end of the output buffer */
- outputsize--;
+ --outputsize;
+
+ /* no data at all or no display name? */
+ if (!input || *input == '<') {
+ return input;
+ }
/* quoted-string rules */
if (input[0] == '"') {
input++; /* skip the first " */
- for (;((outputsize > 0) && *input); input++) {
+ for (; *input; ++input) {
if (*input == '"') { /* end of quoted-string */
break;
} else if (*input == 0x5c) { /* quoted-pair = "\" (%x00-09 / %x0B-0C / %x0E-7F) */
- input++;
- if (!*input || (unsigned char)*input > 0x7f || *input == 0xa || *input == 0xd) {
+ ++input;
+ if (!*input) {
+ break;
+ }
+ if ((unsigned char) *input > 0x7f || *input == 0xa || *input == 0xd) {
continue; /* not a valid quoted-pair, so skip it */
}
- } else if (((*input != 0x9) && ((unsigned char) *input < 0x20)) ||
- (*input == 0x7f)) {
+ } else if ((*input != 0x9 && (unsigned char) *input < 0x20)
+ || *input == 0x7f) {
continue; /* skip this invalid character. */
}
- *output++ = *input;
- outputsize--;
+ if (0 < outputsize) {
+ /* We still have room for the output display-name. */
+ *output++ = *input;
+ --outputsize;
+ }
}
/* if this is successful, input should be at the ending quote */
- if (!input || *input != '"') {
+ if (*input != '"') {
ast_log(LOG_WARNING, "No ending quote for display-name was found\n");
*orig_output = '\0';
return orig_input;
}
/* make sure input is past the last quote */
- input++;
-
- /* terminate outbuf */
+ ++input;
+
+ /* terminate output */
*output = '\0';
} else { /* either an addr-spec or tokenLWS-combo */
- for (;((outputsize > 0) && *input); input++) {
+ for (; *input; ++input) {
/* token or WSP (without LWS) */
if ((*input >= '0' && *input <= '9') || (*input >= 'A' && *input <= 'Z')
|| (*input >= 'a' && *input <= 'z') || *input == '-' || *input == '.'
|| *input == '!' || *input == '%' || *input == '*' || *input == '_'
|| *input == '+' || *input == '`' || *input == '\'' || *input == '~'
|| *input == 0x9 || *input == ' ') {
- *output++ = *input;
- outputsize -= 1;
+ if (0 < outputsize) {
+ /* We still have room for the output display-name. */
+ *output++ = *input;
+ --outputsize;
+ }
} else if (*input == '<') { /* end of tokenLWS-combo */
/* we could assert that the previous char is LWS, but we don't care */
break;
@@ -759,10 +773,10 @@
return orig_input;
}
- /* set NULL while trimming trailing whitespace */
+ /* terminate output while trimming any trailing whitespace */
do {
*output-- = '\0';
- } while (*output == 0x9 || *output == ' '); /* we won't go past orig_output as first was a non-space */
+ } while (orig_output <= output && (*output == 0x9 || *output == ' '));
}
return input;
@@ -771,11 +785,12 @@
AST_TEST_DEFINE(get_calleridname_test)
{
int res = AST_TEST_PASS;
- const char *in1 = "\" quoted-text internal \\\" quote \"<stuff>";
+ const char *in1 = " \" quoted-text internal \\\" quote \"<stuff>";
const char *in2 = " token text with no quotes <stuff>";
const char *overflow1 = " \"quoted-text overflow 1234567890123456789012345678901234567890\" <stuff>";
+ const char *overflow2 = " non-quoted text overflow 1234567890123456789012345678901234567890 <stuff>";
const char *noendquote = " \"quoted-text no end <stuff>";
- const char *addrspec = " \"sip:blah at blah <stuff>";
+ const char *addrspec = " sip:blah at blah";
const char *no_quotes_no_brackets = "blah at blah";
const char *after_dname;
char dname[40];
@@ -810,8 +825,16 @@
/* quoted-text buffer overflow */
after_dname = get_calleridname(overflow1, dname, sizeof(dname));
ast_test_status_update(test, "overflow display-name1: %s\nafter: %s\n", dname, after_dname);
- if (*dname != '\0' && after_dname != overflow1) {
+ if (strcmp(dname, "quoted-text overflow 123456789012345678")) {
ast_test_status_update(test, "overflow display-name1 test failed\n");
+ res = AST_TEST_FAIL;
+ }
+
+ /* non-quoted-text buffer overflow */
+ after_dname = get_calleridname(overflow2, dname, sizeof(dname));
+ ast_test_status_update(test, "overflow display-name2: %s\nafter: %s\n", dname, after_dname);
+ if (strcmp(dname, "non-quoted text overflow 12345678901234")) {
+ ast_test_status_update(test, "overflow display-name2 test failed\n");
res = AST_TEST_FAIL;
}
@@ -825,7 +848,7 @@
/* addr-spec rather than display-name. */
after_dname = get_calleridname(addrspec, dname, sizeof(dname));
- ast_test_status_update(test, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname);
+ ast_test_status_update(test, "addr-spec display-name1: %s\nafter: %s\n", dname, after_dname);
if (*dname != '\0' && after_dname != addrspec) {
ast_test_status_update(test, "detection of addr-spec failed\n");
res = AST_TEST_FAIL;
@@ -839,14 +862,13 @@
res = AST_TEST_FAIL;
}
-
return res;
}
int get_name_and_number(const char *hdr, char **name, char **number)
{
char header[256];
- char tmp_name[50] = { 0, };
+ char tmp_name[50];
char *tmp_number = NULL;
char *hostport = NULL;
char *dummy = NULL;
@@ -1069,14 +1091,14 @@
AST_TEST_DEFINE(get_in_brackets_test)
{
int res = AST_TEST_PASS;
- char *in_brackets = "<sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
+ char in_brackets[] = "sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah";
char no_name[] = "<sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
char quoted_string[] = "\"I'm a quote stri><ng\" <sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
char missing_end_quote[] = "\"I'm a quote string <sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
char name_no_quotes[] = "name not in quotes <sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
char no_end_bracket[] = "name not in quotes <sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah";
char no_name_no_brackets[] = "sip:name at host";
- char missing_start_bracket[] = "name not in quotes sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
+ char missing_start_bracket[] = "sip:name:secret at host:port;transport=tcp?headers=testblah&headers2=blahblah>";
char *uri = NULL;
switch (cmd) {
@@ -1093,57 +1115,49 @@
}
/* Test 1, simple get in brackets */
- if (!(uri = get_in_brackets(no_name)) || !(strcmp(uri, in_brackets))) {
-
- ast_test_status_update(test, "Test 1, simple get in brackets failed.\n");
+ if (!(uri = get_in_brackets(no_name)) || strcmp(uri, in_brackets)) {
+ ast_test_status_update(test, "Test 1, simple get in brackets failed. %s\n", uri);
res = AST_TEST_FAIL;
}
/* Test 2, starts with quoted string */
- if (!(uri = get_in_brackets(quoted_string)) || !(strcmp(uri, in_brackets))) {
-
- ast_test_status_update(test, "Test 2, get in brackets with quoted string in front failed.\n");
+ if (!(uri = get_in_brackets(quoted_string)) || strcmp(uri, in_brackets)) {
+ ast_test_status_update(test, "Test 2, get in brackets with quoted string in front failed. %s\n", uri);
res = AST_TEST_FAIL;
}
/* Test 3, missing end quote */
- if (!(uri = get_in_brackets(missing_end_quote)) || !(strcmp(uri, in_brackets))) {
-
- ast_test_status_update(test, "Test 3, missing end quote failed.\n");
+ if (!(uri = get_in_brackets(missing_end_quote)) || !strcmp(uri, in_brackets)) {
+ ast_test_status_update(test, "Test 3, missing end quote failed. %s\n", uri);
res = AST_TEST_FAIL;
}
/* Test 4, starts with a name not in quotes */
- if (!(uri = get_in_brackets(name_no_quotes)) || !(strcmp(uri, in_brackets))) {
-
- ast_test_status_update(test, "Test 4, passing name not in quotes failed.\n");
+ if (!(uri = get_in_brackets(name_no_quotes)) || strcmp(uri, in_brackets)) {
+ ast_test_status_update(test, "Test 4, passing name not in quotes failed. %s\n", uri);
res = AST_TEST_FAIL;
}
/* Test 5, no end bracket, should just return everything after the first '<' */
- if (!(uri = get_in_brackets(no_end_bracket)) || !(strcmp(uri, in_brackets))) {
-
- ast_test_status_update(test, "Test 5, no end bracket failed.\n");
+ if (!(uri = get_in_brackets(no_end_bracket)) || !strcmp(uri, in_brackets)) {
+ ast_test_status_update(test, "Test 5, no end bracket failed. %s\n", uri);
res = AST_TEST_FAIL;
}
/* Test 6, NULL input */
if ((uri = get_in_brackets(NULL))) {
-
ast_test_status_update(test, "Test 6, NULL input failed.\n");
res = AST_TEST_FAIL;
}
/* Test 7, no name, and no brackets. */
- if (!(uri = get_in_brackets(no_name_no_brackets)) || (strcmp(uri, "sip:name at host"))) {
-
+ if (!(uri = get_in_brackets(no_name_no_brackets)) || strcmp(uri, "sip:name at host")) {
ast_test_status_update(test, "Test 7 failed. %s\n", uri);
res = AST_TEST_FAIL;
}
/* Test 8, no start bracket, but with ending bracket. */
- if (!(uri = get_in_brackets(missing_start_bracket)) || !(strcmp(uri, in_brackets))) {
-
+ if (!(uri = get_in_brackets(missing_start_bracket)) || strcmp(uri, in_brackets)) {
ast_test_status_update(test, "Test 8 failed. %s\n", uri);
res = AST_TEST_FAIL;
}
@@ -1158,20 +1172,42 @@
char **residue)
{
char buf[1024];
- char **residue2=residue;
+ char **residue2 = residue;
+ char *orig_uri = uri;
int ret;
+
+ buf[0] = '\0';
if (name) {
- get_calleridname(uri,buf,sizeof(buf));
- *name = buf;
- }
- ret = get_in_brackets_full(uri,&uri,residue);
- if (ret == 0) { /* uri is in brackets so do not treat unknown trailing uri parameters as potential messageheader parameters */
- *residue = *residue + 1; /* step over the first semicolon so as per parse uri residue */
+ uri = (char *) get_calleridname(uri, buf, sizeof(buf));
+ }
+ ret = get_in_brackets_full(uri, &uri, residue);
+ if (ret == 0) {
+ /*
+ * The uri is in brackets so do not treat unknown trailing uri
+ * parameters as potential message header parameters.
+ */
+ if (residue && **residue) {
+ /* step over the first semicolon as per parse_uri_full residue */
+ *residue = *residue + 1;
+ }
residue2 = NULL;
}
- return parse_uri_full(uri, scheme, user, pass, hostport, params, headers,
- residue2);
+ if (name) {
+ if (buf[0]) {
+ /*
+ * There is always room at orig_uri for the display-name because
+ * at least one character has always been removed. A '"' or '<'
+ * has been removed.
+ */
+ strcpy(orig_uri, buf);
+ *name = orig_uri;
+ } else {
+ *name = "";
+ }
+ }
+
+ return parse_uri_full(uri, scheme, user, pass, hostport, params, headers, residue2);
}
AST_TEST_DEFINE(parse_name_andor_addr_test)
@@ -1313,7 +1349,7 @@
name = user = pass = hostport = headers = residue = NULL;
params.transport = params.user = params.method = params.ttl = params.maddr = NULL;
params.lr = 0;
- ast_copy_string(uri,testdataptr->uri,sizeof(uri));
+ ast_copy_string(uri,testdataptr->uri,sizeof(uri));
if (parse_name_andor_addr(uri, "sip:,sips:",
testdataptr->nameptr,
testdataptr->userptr,
@@ -1330,17 +1366,17 @@
((testdataptr->residueptr) && strcmp(testdataptr->residue, residue)) ||
((testdataptr->paramsptr) && strcmp(testdataptr->params.transport,params.transport)) ||
((testdataptr->paramsptr) && strcmp(testdataptr->params.user,params.user))
- ) {
- ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc);
- res = AST_TEST_FAIL;
- }
- }
-
+ ) {
+ ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc);
+ res = AST_TEST_FAIL;
+ }
+ }
return res;
}
-int get_comma(char *in, char **out) {
+int get_comma(char *in, char **out)
+{
char *c;
char *parse = in;
if (out) {
@@ -1376,35 +1412,35 @@
return 1;
}
-int parse_contact_header(char *contactheader, struct contactliststruct *contactlist) {
+int parse_contact_header(char *contactheader, struct contactliststruct *contactlist)
+{
int res;
int last;
char *comma;
char *residue;
char *param;
char *value;
- struct contact *contact=NULL;
+ struct contact *split_contact = NULL;
if (*contactheader == '*') {
return 1;
}
- contact = malloc(sizeof(*contact));
-
- AST_LIST_HEAD_SET_NOLOCK(contactlist, contact);
- while ((last = get_comma(contactheader,&comma)) != -1) {
-
+ split_contact = ast_calloc(1, sizeof(*split_contact));
+
+ AST_LIST_HEAD_SET_NOLOCK(contactlist, split_contact);
+ while ((last = get_comma(contactheader, &comma)) != -1) {
res = parse_name_andor_addr(contactheader, "sip:,sips:",
- &contact->name, &contact->user,
- &contact->pass, &contact->hostport,
- &contact->params, &contact->headers,
- &residue);
+ &split_contact->name, &split_contact->user,
+ &split_contact->pass, &split_contact->hostport,
+ &split_contact->params, &split_contact->headers,
+ &residue);
if (res == -1) {
return res;
}
/* parse contact params */
- contact->expires = contact->q = "";
+ split_contact->expires = split_contact->q = "";
while ((value = strchr(residue,'='))) {
*value++ = '\0';
@@ -1417,20 +1453,19 @@
}
if (!strcmp(param,"expires")) {
- contact->expires = value;
+ split_contact->expires = value;
} else if (!strcmp(param,"q")) {
- contact->q = value;
+ split_contact->q = value;
}
}
- if(last) {
+ if (last) {
return 0;
}
contactheader = comma;
- contact = malloc(sizeof(*contact));
- AST_LIST_INSERT_TAIL(contactlist, contact, list);
-
+ split_contact = ast_calloc(1, sizeof(*split_contact));
+ AST_LIST_INSERT_TAIL(contactlist, split_contact, list);
}
return last;
}
@@ -1563,16 +1598,15 @@
strcmp(tdcontactptr->params.transport, contactptr->params.transport) ||
strcmp(tdcontactptr->params.ttl, contactptr->params.ttl) ||
(tdcontactptr->params.lr != contactptr->params.lr)
- ) {
+ ) {
ast_test_status_update(test, "Sub-Test: %s,failed.\n", testdataptr->desc);
res = AST_TEST_FAIL;
break;
}
- contactptr = AST_LIST_NEXT(contactptr,list);
+ contactptr = AST_LIST_NEXT(contactptr,list);
}
}
-
}
return res;
More information about the asterisk-commits
mailing list