[asterisk-commits] dlee: trunk r378935 - in /trunk: ./ channels/ include/asterisk/ main/ tests/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Sat Jan 12 00:43:42 CST 2013 

Author: dlee
Date: Sat Jan 12 00:43:37 2013
New Revision: 378935

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=378935
Log:
Fix XML encoding of 'identity display' in NOTIFY messages.

XML encoding in chan_sip is accomplished by naively building the XML
directly from strings. While this usually works, it fails to take into
account escaping the reserved characters in XML.

This patch adds an 'ast_xml_escape' function, which works similarly to
'ast_uri_encode'. This is used to properly escape the local_display
attribute in XML formatted NOTIFY messages.

Several things to note:
 * The Right Thing(TM) to do would probably be to replace the
   ast_build_string stuff with building an ast_xml_doc. That's a much
   bigger change, and out of scope for the original ticket, so I
   refrained myself.
 * It is with great sadness that I wrote my own ast_xml_escape
   function. There's one in libxml2, but it's knee-deep in
   libxml2-ness, and not easily used to one-off escape a
   string.
 * I only escaped the string we know is causing problems
   (local_display). At least some of the other strings are
   URI-encoded, which should be XML safe. Rather than figuring out
   what's safe and escaping what's not, it would be much cleaner to
   simply build an ast_xml_doc for the messages and let the XML
   library do the XML escaping. Like I said, that's out of scope.

(closes issue ABE-2902)
Reported by: Guenther Kelleter
Tested by: Guenther Kelleter
Review: http://reviewboard.digium.internal/r/365/

........

Merged revision 378919 from https://origsvn.digium.com/svn/asterisk/be/branches/C.3-bier
........

Merged revisions 378933 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 378934 from http://svn.asterisk.org/svn/asterisk/branches/11

Added:
    trunk/tests/test_xml_escape.c
      - copied unchanged from r378934, branches/11/tests/test_xml_escape.c
Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c
    trunk/include/asterisk/utils.h
    trunk/main/utils.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-11-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=378935&r1=378934&r2=378935
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Sat Jan 12 00:43:37 2013
@@ -14543,7 +14543,8 @@
 		ast_str_append(tmp, 0, "<?xml version=\"1.0\"?>\n");
 		ast_str_append(tmp, 0, "<dialog-info xmlns=\"urn:ietf:params:xml:ns:dialog-info\" version=\"%u\" state=\"%s\" entity=\"%s\">\n", p->dialogver, full ? "full" : "partial", mto);
 		if (data->state > 0 && (data->state & AST_EXTENSION_RINGING) && sip_cfg.notifyringing) {
-			const char *local_display = exten;
+			/* Twice the extension length should be enough for XML encoding */
+			char local_display[AST_MAX_EXTENSION * 2];
 			char *local_target = ast_strdupa(mto);
 			const char *remote_display = exten;
 			/* It may seem odd to base the remote_target on the To header here,
@@ -14556,6 +14557,8 @@
 			 */
 			char *remote_target = ast_strdupa(mto);
 
+			ast_xml_escape(exten, local_display, sizeof(local_display));
+
 			/* There are some limitations to how this works.  The primary one is that the
 			   callee must be dialing the same extension that is being monitored.  Simply dialing
 			   the hint'd device is not sufficient. */
@@ -14575,8 +14578,9 @@
 					local_target = ast_alloca(need);
 					snprintf(local_target, need, "sip:%s@%s", cid_num, p->fromdomain);
 
-					local_display = ast_strdupa(S_COR(ast_channel_caller(callee)->id.name.valid,
-						ast_channel_caller(callee)->id.name.str, ""));
+					ast_xml_escape(S_COR(ast_channel_caller(callee)->id.name.valid,
+							     ast_channel_caller(callee)->id.name.str, ""),
+						       local_display, sizeof(local_display));
 
 					connected_num = S_COR(ast_channel_connected(callee)->id.number.valid,
 						ast_channel_connected(callee)->id.number.str, "");

Modified: trunk/include/asterisk/utils.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/utils.h?view=diff&rev=378935&r1=378934&r2=378935
==============================================================================
--- trunk/include/asterisk/utils.h (original)
+++ trunk/include/asterisk/utils.h Sat Jan 12 00:43:37 2013
@@ -289,6 +289,20 @@
  * \param spec flags describing how the decoding should be performed
  */
 void ast_uri_decode(char *s, struct ast_flags spec);
+
+/*! ast_xml_escape
+	\brief Escape reserved characters for use in XML.
+
+	If \a outbuf is too short, the output string will be truncated.
+	Regardless, the output will always be null terminated.
+
+	\param string String to be converted
+	\param outbuf Resulting encoded string
+	\param buflen Size of output buffer
+	\return 0 for success
+	\return -1 if buflen is too short.
+ */
+int ast_xml_escape(const char *string, char *outbuf, size_t buflen);
 
 /*!
  * \brief Escape characters found in a quoted string.

Modified: trunk/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/utils.c?view=diff&rev=378935&r1=378934&r2=378935
==============================================================================
--- trunk/main/utils.c (original)
+++ trunk/main/utils.c Sat Jan 12 00:43:37 2013
@@ -483,6 +483,69 @@
 
 	return outbuf;
 }
+int ast_xml_escape(const char *string, char * const outbuf, const size_t buflen)
+{
+	char *dst = outbuf;
+	char *end = outbuf + buflen - 1; /* save one for the null terminator */
+
+	/* Handle the case for the empty output buffer */
+	if (buflen == 0) {
+		return -1;
+	}
+
+	/* Escaping rules from http://www.w3.org/TR/REC-xml/#syntax */
+	/* This also prevents partial entities at the end of a string */
+	while (*string && dst < end) {
+		const char *entity = NULL;
+		int len = 0;
+
+		switch (*string) {
+		case '<':
+			entity = "&lt;";
+			len = 4;
+			break;
+		case '&':
+			entity = "&amp;";
+			len = 5;
+			break;
+		case '>':
+			/* necessary if ]]> is in the string; easier to escape them all */
+			entity = "&gt;";
+			len = 4;
+			break;
+		case '\'':
+			/* necessary in single-quoted strings; easier to escape them all */
+			entity = "&apos;";
+			len = 6;
+			break;
+		case '"':
+			/* necessary in double-quoted strings; easier to escape them all */
+			entity = "&quot;";
+			len = 6;
+			break;
+		default:
+			*dst++ = *string++;
+			break;
+		}
+
+		if (entity) {
+			ast_assert(len == strlen(entity));
+			if (end - dst < len) {
+				/* no room for the entity; stop */
+				break;
+			}
+			/* just checked for length; strcpy is fine */
+			strcpy(dst, entity);
+			dst += len;
+			++string;
+		}
+	}
+	/* Write null terminator */
+	*dst = '\0';
+	/* If any chars are left in string, return failure */
+	return *string == '\0' ? 0 : -1;
+}
+
 /*! \brief  ast_inet_ntoa: Recursive thread safe replacement of inet_ntoa */
 const char *ast_inet_ntoa(struct in_addr ia)
 {

More information about the asterisk-commits mailing list