[svn-commits] dvossel: trunk r243200 - in /trunk: channels/ include/asterisk/ main/ tests/

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Jan 26 10:30:18 CST 2010


Author: dvossel
Date: Tue Jan 26 10:30:08 2010
New Revision: 243200

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=243200
Log:
RFC compliant uri and display-name encode/decode

1.  URI Encoding
This patch changes ast_uri_encode()'s behavior when doreserved is enabled.
Previously when doreserved was enabled only a small set of reserved
characters were encoded.  This set was comprised primarily of the reserved
characters defined in RFC3261 section 25.1, but contained other characters as
well.  Rather than only escaping the reserved set, doreserved now escapes
all characters not within the unreserved set as defined by RFC 3261 and
RFC 2396.  Also, the 'doreserved' variable has been renamed to 'do_special_char'
in attempts to avoid confusion.

When doreserve is not enabled, the previous logic of only encoding the
characters <= 0X1F and > 0X7f remains, except for the '%' character, which
must always be encoded as it signifies a HEX escaped character during the decode
process.

2. URI Decoding: Break up URI before decode.
In chan_sip.c ast_uri_decode is called on the entire URI instead of it's
individual parts after it is parsed.  This is not good as ast_uri_decode
can introduce special characters back into the URI which can mess up parsing.
This patch resolves this by not decoding a URI until parsing is completely
done.  There are many instances where we check to see if pedantic checking
is enabled before we decode a URI.  In these cases a new macro,
SIP_PEDANTIC_DECODE, is used on the individual parsed segments of the URI
rather than constantly putting if (pedantic) { decode() } checks everywhere
in the code.  In the areas where ast_uri_decode is not dependent upon
pedantic checking this macro is not used, but decoding is still moved to
each individual part of the URI.  The only behavior that should change from
this patch is the time at which decoding occurs.

Since I had to look over every place URI parsing occurs to create this
patch, I found several places where we use duplicate code for parsing.
To consolidate the code, those areas have updated to use the parse_uri()
function where possible.

3. SIP display-name decoding according to RFC3261 section 25.
To properly decode the display-name portion of a FROM header, chan_sip's
get_calleridname() function required a complete re-write.  More information
about this change can be found in the comments at the beginning of this function.

4. Unit Tests.
Unit tests for ast_uri_encode, ast_uri_decode, and get_calleridname() have been
written.  This involved the addition of the test_utils.c file for testing the
utils api.

(closes issue #16299)
Reported by: wdoekes
Patches:
      astsvn-16299-get_calleridname.diff uploaded by wdoekes (license 717)
      get_calleridname_rewrite.diff uploaded by dvossel (license 671)
Tested by: wdoekes, dvossel, Nick_Lewis

Review: https://reviewboard.asterisk.org/r/469/


Added:
    trunk/tests/test_utils.c   (with props)
Modified:
    trunk/channels/chan_sip.c
    trunk/include/asterisk/utils.h
    trunk/main/test.c
    trunk/main/utils.c

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=243200&r1=243199&r2=243200
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Tue Jan 26 10:30:08 2010
@@ -240,6 +240,7 @@
 #include "asterisk/utils.h"
 #include "asterisk/file.h"
 #include "asterisk/astobj.h"
+#include "asterisk/test.h"
 /*
    Uncomment the define below,  if you are having refcount related memory leaks.
    With this uncommented, this module will generate a file, /tmp/refs, which contains
@@ -1222,6 +1223,12 @@
 static struct sip_settings sip_cfg;		/*!< SIP configuration data.
 					\note in the future we could have multiple of these (per domain, per device group etc) */
 
+/*!< use this macro when ast_uri_decode is dependent on pedantic checking to be on. */
+#define SIP_PEDANTIC_DECODE(str)	\
+	if (sip_cfg.pedanticsipchecking && !ast_strlen_zero(str)) {	\
+		ast_uri_decode(str);	\
+	}	\
+
 static int global_match_auth_username;		/*!< Match auth username if available instead of From: Default off. */
 
 static int global_relaxdtmf;		/*!< Relax DTMF */
@@ -2677,7 +2684,7 @@
 static int parse_ok_contact(struct sip_pvt *pvt, struct sip_request *req);
 static int set_address_from_contact(struct sip_pvt *pvt);
 static void check_via(struct sip_pvt *p, struct sip_request *req);
-static char *get_calleridname(const char *input, char *output, size_t outputsize);
+static const char *get_calleridname(const char *input, char *output, size_t outputsize);
 static int get_rpid(struct sip_pvt *p, struct sip_request *oreq);
 static int get_rdnis(struct sip_pvt *p, struct sip_request *oreq, char **name, char **number, int *reason);
 static int get_destination(struct sip_pvt *p, struct sip_request *oreq);
@@ -13499,47 +13506,33 @@
 	enum check_auth_result res = AUTH_NOT_FOUND;
 	struct sip_peer *peer;
 	char tmp[256];
-	char *name, *c;
-	char *domain;
+	char *name = NULL, *c, *domain = NULL;
 	char *uri2 = ast_strdupa(uri);
 
 	terminate_uri(uri2);
 
 	ast_copy_string(tmp, get_header(req, "To"), sizeof(tmp));
-	if (sip_cfg.pedanticsipchecking)
-		ast_uri_decode(tmp);
 
 	c = get_in_brackets(tmp);
 	c = remove_uri_parameters(c);
 
-	if (!strncasecmp(c, "sip:", 4)) {
-		name = c + 4;
-	} else if (!strncasecmp(c, "sips:", 5)) {
-		name = c + 5;
-	} else {
-		name = c;
+	if (parse_uri(c, "sip:,sips:", &name, NULL, &domain, NULL, NULL, NULL)) {
 		ast_log(LOG_NOTICE, "Invalid to address: '%s' from %s (missing sip:) trying to use anyway...\n", c, ast_inet_ntoa(sin->sin_addr));
-	}
+		return -1;
+	}
+
+	SIP_PEDANTIC_DECODE(name);
+	SIP_PEDANTIC_DECODE(domain);
 
 	/*! \todo XXX here too we interpret a missing @domain as a name-only
 	 * URI, whereas the RFC says this is a domain-only uri.
 	 */
-	/* Strip off the domain name */
-	if ((c = strchr(name, '@'))) {
-		*c++ = '\0';
-		domain = c;
-		if ((c = strchr(domain, ':')))	/* Remove :port */
-			*c = '\0';
-		if (!AST_LIST_EMPTY(&domain_list)) {
-			if (!check_sip_domain(domain, NULL, 0)) {
-				transmit_response(p, "404 Not found (unknown domain)", &p->initreq);
-				return AUTH_UNKNOWN_DOMAIN;
-			}
-		}
-	}
-	c = strchr(name, ';');	/* Remove any Username parameters */
-	if (c)
-		*c = '\0';
+	if (!ast_strlen_zero(domain) && !AST_LIST_EMPTY(&domain_list)) {
+		if (!check_sip_domain(domain, NULL, 0)) {
+			transmit_response(p, "404 Not found (unknown domain)", &p->initreq);
+			return AUTH_UNKNOWN_DOMAIN;
+		}
+	}
 
 	ast_string_field_set(p, exten, name);
 	build_contact(p);
@@ -14016,10 +14009,9 @@
 */
 static int get_destination(struct sip_pvt *p, struct sip_request *oreq)
 {
-	char tmp[256] = "", *uri, *a;
+	char tmp[256] = "", *uri, *domain;
 	char tmpf[256] = "", *from = NULL;
 	struct sip_request *req;
-	char *colon;
 	char *decoded_uri;
 	
 	req = oreq;
@@ -14030,19 +14022,17 @@
 	if (req->rlPart2)
 		ast_copy_string(tmp, REQ_OFFSET_TO_STR(req, rlPart2), sizeof(tmp));
 	
-	if (sip_cfg.pedanticsipchecking)
-		ast_uri_decode(tmp);
-
 	uri = get_in_brackets(tmp);
-	
-	if (!strncasecmp(uri, "sip:", 4)) {
-		uri += 4;
-	} else if (!strncasecmp(uri, "sips:", 5)) {
-		uri += 5;
-	} else {
-		ast_log(LOG_WARNING, "Huh?  Not a SIP header (%s)?\n", uri);
+
+	if (parse_uri(uri, "sip:,sips:", &uri, NULL, &domain, NULL, NULL, NULL)) {
+		ast_log(LOG_WARNING, "Not a SIP header (%s)?\n", uri);
 		return -1;
 	}
+
+	SIP_PEDANTIC_DECODE(domain);
+	SIP_PEDANTIC_DECODE(uri);
+
+	ast_string_field_set(p, domain, domain);
 
 	/* Now find the From: caller ID and name */
 	/* XXX Why is this done in get_destination? Isn't it already done?
@@ -14050,46 +14040,17 @@
         */
 	ast_copy_string(tmpf, get_header(req, "From"), sizeof(tmpf));
 	if (!ast_strlen_zero(tmpf)) {
-		if (sip_cfg.pedanticsipchecking)
-			ast_uri_decode(tmpf);
 		from = get_in_brackets(tmpf);
-	}
-	
-	if (!ast_strlen_zero(from)) {
-		if (!strncasecmp(from, "sip:", 4)) {
-			from += 4;
-		} else if (!strncasecmp(from, "sips:", 5)) {
-			from += 5;
-		} else {
-			ast_log(LOG_WARNING, "Huh?  Not a SIP header (%s)?\n", from);
+		if (parse_uri(from, "sip:,sips:", &from, NULL, &domain, NULL, NULL, NULL)) {
+			ast_log(LOG_WARNING, "Not a SIP header (%s)?\n", from);
 			return -1;
 		}
-		if ((a = strchr(from, '@')))
-			*a++ = '\0';
-		else
-			a = from;	/* just a domain */
-		from = strsep(&from, ";");	/* Remove userinfo options */
-		a = strsep(&a, ";");		/* Remove URI options */
-		ast_string_field_set(p, fromdomain, a);
-	}
-
-	/* Skip any options and find the domain */
-
-	/* Get the target domain */
-	if ((a = strchr(uri, '@'))) {
-		*a++ = '\0';
-	} else {	/* No username part */
-		a = uri;
-		uri = "s";	/* Set extension to "s" */
-	}
-	colon = strchr(a, ':'); /* Remove :port */
-	if (colon)
-		*colon = '\0';
-
-	uri = strsep(&uri, ";");	/* Remove userinfo options */
-	a = strsep(&a, ";");		/* Remove URI options */
-
-	ast_string_field_set(p, domain, a);
+
+		SIP_PEDANTIC_DECODE(from);
+		SIP_PEDANTIC_DECODE(domain);
+
+		ast_string_field_set(p, fromdomain, domain);
+	}
 
 	if (!AST_LIST_EMPTY(&domain_list)) {
 		char domain_context[AST_MAX_EXTENSION];
@@ -14262,9 +14223,6 @@
 	}
 	h_refer_to = ast_strdupa(p_refer_to);
 	refer_to = get_in_brackets(h_refer_to);
-	if (sip_cfg.pedanticsipchecking)
-		ast_uri_decode(refer_to);
-
 	if (!strncasecmp(refer_to, "sip:", 4)) {
 		refer_to += 4;			/* Skip sip: */
 	} else if (!strncasecmp(refer_to, "sips:", 5)) {
@@ -14289,8 +14247,6 @@
 	if (!ast_strlen_zero(p_referred_by)) {
 		char *lessthan;
 		h_referred_by = ast_strdupa(p_referred_by);
-		if (sip_cfg.pedanticsipchecking)
-			ast_uri_decode(h_referred_by);
 
 		/* Store referrer's caller ID name */
 		ast_copy_string(referdata->referred_by_name, h_referred_by, sizeof(referdata->referred_by_name));
@@ -14299,6 +14255,7 @@
 		}
 
 		referred_by_uri = get_in_brackets(h_referred_by);
+
 		if (!strncasecmp(referred_by_uri, "sip:", 4)) {
 			referred_by_uri += 4;		/* Skip sip: */
 		} else if (!strncasecmp(referred_by_uri, "sips:", 5)) {
@@ -14316,7 +14273,6 @@
 		/* This is an attended transfer */
 		referdata->attendedtransfer = 1;
 		ast_copy_string(referdata->replaces_callid, ptr+9, sizeof(referdata->replaces_callid));
-		ast_uri_decode(referdata->replaces_callid);
 		if ((ptr = strchr(referdata->replaces_callid, ';'))) 	/* Find options */ {
 			*ptr++ = '\0';
 		}
@@ -14334,6 +14290,7 @@
 				*to = '\0';
 			if ((to = strchr(ptr, ';')))
 				*to = '\0';
+			ast_uri_decode(ptr);
 			ast_copy_string(referdata->replaces_callid_totag, ptr, sizeof(referdata->replaces_callid_totag));
 		}
 		
@@ -14343,6 +14300,7 @@
 				*to = '\0';
 			if ((to = strchr(ptr, ';')))
 				*to = '\0';
+			ast_uri_decode(ptr);
 			ast_copy_string(referdata->replaces_callid_fromtag, ptr, sizeof(referdata->replaces_callid_fromtag));
 		}
 		
@@ -14363,19 +14321,26 @@
 		if ((ptr = strchr(domain, ':')))	/* Remove :port */
 			*ptr = '\0';
 		
+		SIP_PEDANTIC_DECODE(domain);
+		SIP_PEDANTIC_DECODE(urioption);
+
 		/* Save the domain for the dial plan */
 		ast_copy_string(referdata->refer_to_domain, domain, sizeof(referdata->refer_to_domain));
-		if (urioption)
+		if (urioption) {
 			ast_copy_string(referdata->refer_to_urioption, urioption, sizeof(referdata->refer_to_urioption));
+		}
 	}
 
 	if ((ptr = strchr(refer_to, ';'))) 	/* Remove options */
 		*ptr = '\0';
+
+	SIP_PEDANTIC_DECODE(refer_to);
 	ast_copy_string(referdata->refer_to, refer_to, sizeof(referdata->refer_to));
 	
 	if (referred_by_uri) {
 		if ((ptr = strchr(referred_by_uri, ';'))) 	/* Remove options */
 			*ptr = '\0';
+		SIP_PEDANTIC_DECODE(referred_by_uri);
 		ast_copy_string(referdata->referred_by, referred_by_uri, sizeof(referdata->referred_by));
 	} else {
 		referdata->referred_by[0] = '\0';
@@ -14427,26 +14392,18 @@
 	ast_copy_string(tmp, get_header(req, "Also"), sizeof(tmp));
 	c = get_in_brackets(tmp);
 
-	if (sip_cfg.pedanticsipchecking)
-		ast_uri_decode(c);
-
-	if (!strncasecmp(c, "sip:", 4)) {
-		c += 4;
-	} else if (!strncasecmp(c, "sips:", 5)) {
-		c += 5;
-	} else {
+	if (parse_uri(c, "sip:,sips:", &c, NULL, &a, NULL, NULL, NULL)) {
 		ast_log(LOG_WARNING, "Huh?  Not a SIP header in Also: transfer (%s)?\n", c);
 		return -1;
 	}
-
-	if ((a = strchr(c, ';'))) 	/* Remove arguments */
-		*a = '\0';
 	
-	if ((a = strchr(c, '@'))) {	/* Separate Domain */
-		*a++ = '\0';
+	SIP_PEDANTIC_DECODE(c);
+	SIP_PEDANTIC_DECODE(a);
+
+	if (!ast_strlen_zero(a)) {
 		ast_copy_string(referdata->refer_to_domain, a, sizeof(referdata->refer_to_domain));
 	}
-	
+
 	if (sip_debug_test_pvt(p))
 		ast_verbose("Looking for %s in %s\n", c, p->context);
 
@@ -14573,48 +14530,185 @@
 	}
 }
 
-/*! \brief  Get caller id name from SIP headers */
-static char *get_calleridname(const char *input, char *output, size_t outputsize)
-{
-	const char *end = strchr(input, '<');	/* first_bracket */
-	const char *tmp = strchr(input, '"');	/* first quote */
-	int bytes = 0;
-	int maxbytes = outputsize - 1;
-
-	if (!end || end == input)	/* we require a part in brackets */
-		return NULL;
-
-	end--; /* move just before "<" */
-
-	if (tmp && tmp <= end) {
-		/* The quote (tmp) precedes the bracket (end+1).
-		 * Find the matching quote and return the content.
-		 */
-		end = strchr(tmp+1, '"');
-		if (!end)
-			return NULL;
-		bytes = (int) (end - tmp);
-		/* protect the output buffer */
-		if (bytes > maxbytes)
-			bytes = maxbytes;
-		ast_copy_string(output, tmp + 1, bytes);
-	} else {
-		/* No quoted string, or it is inside brackets. */
-		/* clear the empty characters in the begining*/
-		input = ast_skip_blanks(input);
-		/* clear the empty characters in the end */
-		while(*end && *end < 33 && end > input)
-			end--;
-		if (end >= input) {
-			bytes = (int) (end - input) + 2;
-			/* protect the output buffer */
-			if (bytes > maxbytes)
-				bytes = maxbytes;
-			ast_copy_string(output, input, bytes);
-		} else
-			return NULL;
-	}
-	return output;
+/*! \brief  Get caller id name from SIP headers, copy into output buffer
+ *
+ *  \retval input string pointer placed after display-name field if possible
+ */
+static const char *get_calleridname(const char *input, char *output, size_t outputsize)
+{
+	/* From RFC3261:
+	 * 
+	 * From           =  ( "From" / "f" ) HCOLON from-spec
+	 * from-spec      =  ( name-addr / addr-spec ) *( SEMI from-param )
+	 * name-addr      =  [ display-name ] LAQUOT addr-spec RAQUOT
+	 * display-name   =  *(token LWS)/ quoted-string
+	 * token          =  1*(alphanum / "-" / "." / "!" / "%" / "*"
+	 *                     / "_" / "+" / "`" / "'" / "~" )
+	 * quoted-string  =  SWS DQUOTE *(qdtext / quoted-pair ) DQUOTE
+	 * qdtext         =  LWS / %x21 / %x23-5B / %x5D-7E
+	 *                     / UTF8-NONASCII
+	 * quoted-pair    =  "\" (%x00-09 / %x0B-0C / %x0E-7F)
+	 *
+	 * HCOLON         = *WSP ":" SWS
+	 * SWS            = [LWS]
+	 * LWS            = *[*WSP CRLF] 1*WSP
+	 * WSP            = (SP / HTAB)
+	 *
+	 * Deviations from it:
+	 * - following CRLF's in LWS is not done (here at least)
+	 * - ascii NUL is never legal as it terminates the C-string
+	 * - utf8-nonascii is not checked for validity
+	 */
+	char *orig_output = output;
+	const char *orig_input = 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--;
+
+	/* quoted-string rules */
+	if (input[0] == '"') {
+		input++; /* skip the first " */
+
+		for (;((outputsize > 0) && *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) {
+					continue;  /* not a valid quoted-pair, so skip it */
+				}
+			} else if (((*input != 0x9) && ((unsigned char) *input < 0x20)) ||
+			            (*input == 0x7f)) {
+				continue; /* skip this invalid character. */
+			}
+
+			*output++ = *input;
+			outputsize--;
+		}
+
+		/* if this is successful, input should be at the ending quote */
+		if (!input || *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 */
+		*output = '\0';
+	} else {  /* either an addr-spec or tokenLWS-combo */
+		for (;((outputsize > 0) && *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;
+			} else if (*input == '<') {   /* end of tokenLWS-combo */
+				/* we could assert that the previous char is LWS, but we don't care */
+				break;
+			} else if (*input == ':') {
+				/* This invalid character which indicates this is addr-spec rather than display-name. */
+				*orig_output = '\0';
+				return orig_input;
+			} else {         /* else, invalid character we can skip. */
+				continue;    /* skip this character */
+			}
+		}
+
+		/* set NULL while trimming trailing whitespace */
+		do {
+			*output-- = '\0';
+		} while (*output == 0x9 || *output == ' '); /* we won't go past orig_output as first was a non-space */
+	}
+
+	return input;
+}
+
+AST_TEST_DEFINE(get_calleridname_test)
+{
+	int res = AST_TEST_PASS;
+	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 *noendquote = " \"quoted-text no end <stuff>";
+	const char *addrspec = " \"sip:blah at blah <stuff>";
+	const char *after_dname;
+	char dname[40];
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "sip_get_calleridname_test";
+		info->category = "channels/chan_sip/";
+		info->summary = "decodes callerid name from sip header";
+		info->description = "Decodes display-name field of sip header.  Checks for valid output and expected failure cases.";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	/* quoted-text with backslash escaped quote */
+	after_dname = get_calleridname(in1, dname, sizeof(dname));
+	ast_test_status_update(&args->status_update, "display-name1: %s\nafter: %s\n", dname, after_dname);
+	if (strcmp(dname, " quoted-text internal \" quote ")) {
+		ast_test_status_update(&args->status_update, "display-name1 test failed\n");
+		ast_str_append(&args->ast_test_error_str, 0, "quoted-text with internal backslash decode failed. \n");
+		res = AST_TEST_FAIL;
+	}
+
+	/* token text */
+	after_dname = get_calleridname(in2, dname, sizeof(dname));
+	ast_test_status_update(&args->status_update, "display-name2: %s\nafter: %s\n", dname, after_dname);
+	if (strcmp(dname, "token text with no quotes")) {
+		ast_test_status_update(&args->status_update, "display-name2 test failed\n");
+		ast_str_append(&args->ast_test_error_str, 0, "token text with decode failed. \n");
+		res = AST_TEST_FAIL;
+	}
+
+	/* quoted-text buffer overflow */
+	after_dname = get_calleridname(overflow1, dname, sizeof(dname));
+	ast_test_status_update(&args->status_update, "overflow display-name1: %s\nafter: %s\n", dname, after_dname);
+	if (*dname != '\0' && after_dname != overflow1) {
+		ast_test_status_update(&args->status_update, "overflow display-name1 test failed\n");
+		ast_str_append(&args->ast_test_error_str, 0, "quoted-text buffer overflow check failed. \n");
+		res = AST_TEST_FAIL;
+	}
+
+	/* quoted-text buffer with no terminating end quote */
+	after_dname = get_calleridname(noendquote, dname, sizeof(dname));
+	ast_test_status_update(&args->status_update, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname);
+	if (*dname != '\0' && after_dname != noendquote) {
+		ast_test_status_update(&args->status_update, "no end quote for quoted-text display-name failed\n");
+		ast_str_append(&args->ast_test_error_str, 0, "quoted-text buffer check no terminating end quote failed. \n");
+		res = AST_TEST_FAIL;
+	}
+
+	/* addr-spec rather than display-name. */
+	after_dname = get_calleridname(addrspec, dname, sizeof(dname));
+	ast_test_status_update(&args->status_update, "noendquote display-name1: %s\nafter: %s\n", dname, after_dname);
+	if (*dname != '\0' && after_dname != addrspec) {
+		ast_test_status_update(&args->status_update, "detection of addr-spec failed\n");
+		ast_str_append(&args->ast_test_error_str, 0, "detection of addr-spec failed. \n");
+		res = AST_TEST_FAIL;
+	}
+
+
+	return res;
 }
 
 
@@ -14790,26 +14884,29 @@
 					      int sipmethod, const char *uri, enum xmittype reliable,
 					      struct sockaddr_in *sin, struct sip_peer **authpeer)
 {
-	char from[256];
+	char from[256] = { 0, };
 	char *dummy;	/* dummy return value for parse_uri */
 	char *domain;	/* dummy return value for parse_uri */
 	char *of;
-	enum check_auth_result res;
+	enum check_auth_result res = AUTH_DONT_KNOW;
 	char calleridname[50];
 	char *uri2 = ast_strdupa(uri);
 
 	terminate_uri(uri2);	/* trim extra stuff */
 
 	ast_copy_string(from, get_header(req, "From"), sizeof(from));
-	if (sip_cfg.pedanticsipchecking)
-		ast_uri_decode(from);
 	/* XXX here tries to map the username for invite things */
 	memset(calleridname, 0, sizeof(calleridname));
-	get_calleridname(from, calleridname, sizeof(calleridname));
+
+	/* strip the display-name portion off the beginning of the FROM header. */
+	if (!(of = (char *) get_calleridname(from, calleridname, sizeof(calleridname)))) {
+		ast_log(LOG_ERROR, "FROM header can not be parsed \n");
+		return res;
+	}
+
 	if (calleridname[0])
 		ast_string_field_set(p, cid_name, calleridname);
 
-	of = get_in_brackets(from);
 	if (ast_strlen_zero(p->exten)) {
 		char *t = uri2;
 		if (!strncasecmp(t, "sip:", 4))
@@ -14820,9 +14917,13 @@
 		t = strchr(p->exten, '@');
 		if (t)
 			*t = '\0';
+
 		if (ast_strlen_zero(p->our_contact))
 			build_contact(p);
 	}
+
+	of = get_in_brackets(of);
+
 	/* save the URI part of the From header */
 	ast_string_field_set(p, from, of);
 
@@ -14830,6 +14931,9 @@
 	if (parse_uri(of, "sip:,sips:", &of, &dummy, &domain, &dummy, &dummy, NULL)) {
 		ast_log(LOG_NOTICE, "From address missing 'sip:', using it anyway\n");
 	}
+
+	SIP_PEDANTIC_DECODE(of);
+	SIP_PEDANTIC_DECODE(domain);
 
 	if (ast_strlen_zero(of)) {
 		/* XXX note: the original code considered a missing @host
@@ -20785,7 +20889,6 @@
 			ast_debug(3, "INVITE part of call transfer. Replaces [%s]\n", p_replaces);
 		/* Create a buffer we can manipulate */
 		replace_id = ast_strdupa(p_replaces);
-		ast_uri_decode(replace_id);
 
 		if (!p->refer && !sip_refer_allocate(p)) {
 			transmit_response_reliable(p, "500 Server Internal Error", req);
@@ -20816,6 +20919,10 @@
 				fromtag = strsep(&fromtag, "&"); /* trim what ? */
 			}
 		}
+
+		ast_uri_decode(fromtag);
+		ast_uri_decode(totag);
+		ast_uri_decode(replace_id);
 
 		if (sipdebug)
 			ast_debug(4, "Invite/replaces: Will use Replace-Call-ID : %s Fromtag: %s Totag: %s\n",
@@ -26861,6 +26968,18 @@
 	AST_CLI_DEFINE(sip_show_tcp, "List TCP Connections")
 };
 
+/*! \brief SIP test registration */
+static void sip_register_tests(void)
+{
+	AST_TEST_REGISTER(get_calleridname_test);
+}
+
+/*! \brief SIP test registration */
+static void sip_unregister_tests(void)
+{
+	AST_TEST_UNREGISTER(get_calleridname_test);
+}
+
 /*! \brief PBX load module - initialization */
 static int load_module(void)
 {
@@ -26950,6 +27069,9 @@
 		"useragent", RQ_CHAR, 20,
 		"lastms", RQ_INTEGER4, 11,
 		SENTINEL);
+
+
+	sip_register_tests();
 
 	return AST_MODULE_LOAD_SUCCESS;
 }
@@ -27074,6 +27196,8 @@
 	ast_unload_realtime("sipregs");
 	ast_unload_realtime("sippeers");
 
+	sip_unregister_tests();
+
 	return 0;
 }
 

Modified: trunk/include/asterisk/utils.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/utils.h?view=diff&rev=243200&r1=243199&r2=243200
==============================================================================
--- trunk/include/asterisk/utils.h (original)
+++ trunk/include/asterisk/utils.h Tue Jan 26 10:30:08 2010
@@ -248,22 +248,25 @@
  */
 int ast_base64decode(unsigned char *dst, const char *src, int max);
 
-/*!  \brief Turn text string to URI-encoded %XX version 
-
-\note 	At this point, we're converting from ISO-8859-x (8-bit), not UTF8
-	as in the SIP protocol spec 
-	If doreserved == 1 we will convert reserved characters also.
-	RFC 2396, section 2.4
-	outbuf needs to have more memory allocated than the instring
-	to have room for the expansion. Every char that is converted
-	is replaced by three ASCII characters.
-	\param string	String to be converted
-	\param outbuf	Resulting encoded string
-	\param buflen	Size of output buffer
-	\param doreserved	Convert reserved characters
-*/
-
-char *ast_uri_encode(const char *string, char *outbuf, int buflen, int doreserved);
+/*! \brief Turn text string to URI-encoded %XX version 
+ *
+ * \note 
+ *  At this point, this function is encoding agnostic; it does not
+ *  check whether it is fed legal UTF-8. We escape control
+ *  characters (\x00-\x1F\x7F), '%', and all characters above 0x7F.
+ *  If do_special_char == 1 we will convert all characters except alnum
+ *  and the mark set.
+ *  Outbuf needs to have more memory allocated than the instring
+ *  to have room for the expansion. Every char that is converted
+ *  is replaced by three ASCII characters.
+ *
+ *  \param string	String to be converted
+ *  \param outbuf	Resulting encoded string
+ *  \param buflen	Size of output buffer
+ *  \param do_special_char	Convert all non alphanum characters execept
+ *         those in the mark set as defined by rfc 3261 section 25.1
+ */
+char *ast_uri_encode(const char *string, char *outbuf, int buflen, int do_special_char);
 
 /*!	\brief Decode URI, URN, URL (overwrite string)
 	\param s	String to be decoded 
@@ -755,4 +758,8 @@
  */
 int ast_eid_cmp(const struct ast_eid *eid1, const struct ast_eid *eid2);
 
+/*!
+ * \brief Registers util api unit tests
+ */
+void ast_utils_register_tests(void);
 #endif /* _ASTERISK_UTILS_H */

Modified: trunk/main/test.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/test.c?view=diff&rev=243200&r1=243199&r2=243200
==============================================================================
--- trunk/main/test.c (original)
+++ trunk/main/test.c Tue Jan 26 10:30:08 2010
@@ -833,9 +833,6 @@
 #ifdef TEST_FRAMEWORK
 	/* Register cli commands */
 	ast_cli_register_multiple(test_cli, ARRAY_LEN(test_cli));
-
-	/* in the future this function could be used to register functions not
-	 * defined within a module */
 #endif
 
 	return 0;

Modified: trunk/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/utils.c?view=diff&rev=243200&r1=243199&r2=243200
==============================================================================
--- trunk/main/utils.c (original)
+++ trunk/main/utils.c Tue Jan 26 10:30:08 2010
@@ -368,41 +368,44 @@
 	b2a[(int)'/'] = 63;
 }
 
-/*! \brief  ast_uri_encode: Turn text string to URI-encoded %XX version
-\note 	At this point, we're converting from ISO-8859-x (8-bit), not UTF8
-	as in the SIP protocol spec 
-	If doreserved == 1 we will convert reserved characters also.
-	RFC 2396, section 2.4
-	outbuf needs to have more memory allocated than the instring
-	to have room for the expansion. Every char that is converted
-	is replaced by three ASCII characters.
-
-	Note: The doreserved option is needed for replaces header in
-	SIP transfers.
-*/
-char *ast_uri_encode(const char *string, char *outbuf, int buflen, int doreserved) 
-{
-	char *reserved = ";/?:@&=+$,# ";	/* Reserved chars */
-
- 	const char *ptr  = string;	/* Start with the string */
+/*! \brief Turn text string to URI-encoded %XX version 
+ *
+ * \note 
+ *  At this point, this function is encoding agnostic; it does not
+ *  check whether it is fed legal UTF-8. We escape control
+ *  characters (\x00-\x1F\x7F), '%', and all characters above 0x7F.
+ *  If do_special_char == 1 we will convert all characters except alnum
+ *  and mark.
+ *  Outbuf needs to have more memory allocated than the instring
+ *  to have room for the expansion. Every char that is converted
+ *  is replaced by three ASCII characters.
+ */
+char *ast_uri_encode(const char *string, char *outbuf, int buflen, int do_special_char)
+{
+	const char *ptr  = string;	/* Start with the string */
 	char *out = NULL;
 	char *buf = NULL;
-
+	const char *mark = "-_.!~*'()"; /* no encode set, RFC 2396 section 2.3, RFC 3261 sec 25 */
 	ast_copy_string(outbuf, string, buflen);
 
-	/* If there's no characters to convert, just go through and don't do anything */
 	while (*ptr) {
-		if ((*ptr < 32) || (doreserved && strchr(reserved, *ptr))) {
+		if ((const signed char) *ptr < 32 || *ptr == 0x7f || *ptr == '%' ||
+				(do_special_char &&
+				!(*ptr >= '0' && *ptr <= '9') &&      /* num */
+				!(*ptr >= 'A' && *ptr <= 'Z') &&      /* ALPHA */
+				!(*ptr >= 'a' && *ptr <= 'z') &&      /* alpha */
+				!strchr(mark, *ptr))) {               /* mark set */
+
 			/* Oops, we need to start working here */
 			if (!buf) {
 				buf = outbuf;
 				out = buf + (ptr - string) ;	/* Set output ptr */
 			}
-			out += sprintf(out, "%%%02x", (unsigned char) *ptr);
+			out += sprintf(out, "%%%02X", (unsigned char) *ptr);
 		} else if (buf) {
 			*out = *ptr;	/* Continue copying the string */
 			out++;
-		} 
+		}
 		ptr++;
 	}
 	if (buf)

Added: trunk/tests/test_utils.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_utils.c?view=auto&rev=243200
==============================================================================
--- trunk/tests/test_utils.c (added)
+++ trunk/tests/test_utils.c Tue Jan 26 10:30:08 2010
@@ -1,0 +1,114 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2010, Digium, Inc.
+ *
+ * David Vossel <dvossel 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 Unit Tests for utils API
+ *
+ * \author David Vossel <dvossel at digium.com>
+ */
+
+/*** MODULEINFO
+	<depend>TEST_FRAMEWORK</depend>
+ ***/
+
+#include "asterisk.h"
+
+ASTERISK_FILE_VERSION(__FILE__, "$Revision$");
+
+#include "asterisk/utils.h"
+#include "asterisk/test.h"
+#include "asterisk/module.h"
+
+AST_TEST_DEFINE(uri_encode_decode_test)
+{
+	int res = AST_TEST_PASS;
+	const char *in = "abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%^&*()_-+={[}]|\\:;\"'<,>.?/";
+	const char *expected1 = "abcdefghijklmnopurstuvwxyz%20ABCDEFGHIJKLMNOPQRSTUVWXYZ%201234567890%20~%60!%40%23%24%25%5E%26*()_-%2B%3D%7B%5B%7D%5D%7C%5C%3A%3B%22'%3C%2C%3E.%3F%2F";
+	const char *expected2 = "abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%25^&*()_-+={[}]|\\:;\"'<,>.?/";
+	char out[256] = { 0 };
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "uri_encode_decode_test";
+		info->category = "main/utils/";
+		info->summary = "encode and decode a hex escaped string";
+		info->description = "encode a string, verify encoded string matches what we expect.  Decode the encoded string, verify decoded string matches the original string.";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	ast_test_status_update(&args->status_update, "Input before executing ast_uri_encode:\n%s\n", in);
+	ast_test_status_update(&args->status_update, "Output expected for ast_uri_encode with enabling do_special_char:\n%s\n", expected1);
+	ast_test_status_update(&args->status_update, "Output expected for ast_uri_encode with out enabling do_special_char:\n%s\n\n", expected2);
+
+	/* Test with do_special_char enabled */
+	ast_uri_encode(in, out, sizeof(out), 1);
+	ast_test_status_update(&args->status_update, "Output after enabling do_special_char:\n%s\n", out);
+	if (strcmp(expected1, out)) {
+		ast_test_status_update(&args->status_update, "ENCODE DOES NOT MATCH EXPECTED, FAIL\n");
+		ast_str_append(&args->ast_test_error_str, 0, "enable do_special_char test encode failed: \n");
+		res = AST_TEST_FAIL;
+	}
+
+	/* Verify uri decode matches original */
+	ast_uri_decode(out);
+	if (strcmp(in, out)) {
+		ast_test_status_update(&args->status_update, "Decoded string did not match original input\n\n");
+		ast_str_append(&args->ast_test_error_str, 0, "enable do_special_char test decode failed: \n");
+		res = AST_TEST_FAIL;
+	} else {
+		ast_test_status_update(&args->status_update, "Decoded string matched original input\n\n");
+	}
+
+	/* Test with do_special_char disabled */
+	out[0] = '\0';
+	ast_uri_encode(in, out, sizeof(out), 0);
+	ast_test_status_update(&args->status_update, "Output after disabling do_special_char:\n%s\n", out);
+	if (strcmp(expected2, out)) {
+		ast_test_status_update(&args->status_update, "ENCODE DOES NOT MATCH EXPECTED, FAIL\n");
+		ast_str_append(&args->ast_test_error_str, 0, "no do_special_char test encode failed: \n");
+		res = AST_TEST_FAIL;
+	}
+
+	/* Verify uri decode matches original */
+	ast_uri_decode(out);
+	if (strcmp(in, out)) {
+		ast_test_status_update(&args->status_update, "Decoded string did not match original input\n\n");
+		ast_str_append(&args->ast_test_error_str, 0, "no do_special_char test decode failed\n");
+		res = AST_TEST_FAIL;
+	} else {
+		ast_test_status_update(&args->status_update, "Decoded string matched original input\n\n");
+	}
+	return res;
+}
+
+static int unload_module(void)
+{
+	AST_TEST_REGISTER(uri_encode_decode_test);
+	return 0;
+}
+
+static int load_module(void)
+{
+	AST_TEST_REGISTER(uri_encode_decode_test);
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Utils test module");

Propchange: trunk/tests/test_utils.c
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: trunk/tests/test_utils.c
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

Propchange: trunk/tests/test_utils.c
------------------------------------------------------------------------------
    svn:mime-type = text/plain




More information about the svn-commits mailing list