New subject: [Freeswitch-dev] [PATCH] fixing conference dial flexibly, fixing bad pointer op

22 Oct 2008

Greetings,

I noticed that the following currently fails in FreeSWITCH:

freeswitch@domain> conference 3001 dial {foo=bar}sofia/internal/2001%domain 3001 XML default Conference 3001 30

2008-10-22 04:01:14 [ERR] switch_core_session.c:249 switch_core_session_outgoing_channel() Could not locate channel type {foo=bar}sofia
2008-10-22 04:01:14 [ERR] switch_ivr_originate.c:1025 switch_ivr_originate() Cannot create outgoing channel of type [{foo=bar}sofia] cause: [CHAN_NOT_IMPLEMENTED]
API CALL [conference(3001 dial {foo=bar}sofia/internal/2001%domain 3001 XML default Conference 3001 30)] output:
Call Requested: result: [CHAN_NOT_IMPLEMENTED]

The example looks contrived, but this functionality is fairly critical
when you need to add, for example, P-Asserted-Identity or Diversion
headers for the call to go outbound.

Interestingly, I found that conference bgdial works correctly, which
got me looking at the source code.

Here is the offender, conference_outcall(...) in mod_conference.c:

		char *dialstr = switch_mprintf("{ignore_early_media=true}%s", bridgeto);
		status = switch_ivr_originate(NULL, &peer_session, cause, dialstr, 60, NULL, cid_name, cid_num, NULL, SOF_NONE);

This means that {ignore_early_media=true}{foo=bar}sofia/... is getting
passed to switch_ivr_originate.  Initially I looked for a better way
to pass channel variables to switch_ivr_originate, but I didn't see a
good way to do it without changing the interface around.

Then I thought, why shouldn't switch_ivr_originate simply accept
multiple sets of braces?  This feature could be useful in other
contexts: scripts, for example, that need to add channel variables and
then include a dialstring from another application layer, just like
conference_outcall is doing.

So I came up with the following patch, which also fixes a bug in the
current code that I'll note below the patch:

---
Author: Travis Cross <tc@traviscross.com>
Date:   Sat Oct 18 00:40:05 2008 +0000

    Allow multiple sets of channel variable brackets on originate.
    
    This change is intended in part to prevent channel variables that we
    add in the C code from interfering with the user's ability to use this
    feature.  This occurs, for example, in conference_outcall.

diff --git a/src/switch_ivr_originate.c b/src/switch_ivr_originate.c
index 02c1a37..57e002b 100644
--- a/src/switch_ivr_originate.c
+++ b/src/switch_ivr_originate.c
@@ -577,6 +577,8 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_originate(switch_core_session_t *sess
 	switch_call_cause_t reason = SWITCH_CAUSE_NONE;
 	uint8_t to = 0;
 	char *var_val, *vars = NULL;
+	int var_block_count = 0;
+	char *e = NULL;
 	const char *ringback_data = NULL;
 	switch_codec_t *read_codec = NULL;
 	uint8_t sent_ring = 0, early_ok = 1, return_ring_ready = 0, progress = 0;
@@ -611,18 +613,41 @@ SWITCH_DECLARE(switch_status_t) switch_ivr_originate(switch_core_session_t *sess
 		data++;
 	}
 
-	if (*data == '{') {
-		char *e = switch_find_end_paren(data, '{', '}');
-
-		if (e) {
+	/* extract channel variables, allowing multiple sets of braces */
+	while (*data == '{') {
+		if (!var_block_count) {
+			e = switch_find_end_paren(data, '{', '}');
+			if (!e || !*e) {
+				goto var_extract_error;
+			}
 			vars = data + 1;
-			*e++ = '\0';
-			data = e;
+			*e = '\0';
+			data = e + 1;
 		} else {
-			switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "Parse Error!\n");
-			status = SWITCH_STATUS_GENERR;
-			goto done;
+			if (e) {
+				*e = ',';
+			}
+			e = switch_find_end_paren(data, '{', '}');
+			if (!e || !*e) {
+				goto var_extract_error;
+			}
+			/* swallow the opening bracket */
+			int i = 0, j = 0;
+			while ((data + i) && *(data + i)) {
+				j = i; i++;
+				/* note that this affects vars[] */
+				data[j] = data[i];
+			}
+			*(--e) = '\0';
+			data = e + 1;
 		}
+		var_block_count++;
+		continue;
+    
+	var_extract_error:
+		switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "Parse Error!\n");
+		status = SWITCH_STATUS_GENERR;
+		goto done;
 	}
 
 	/* strip leading spaces (again) */
---

After applying the patch, conference dial works as expected.  It is
now supported and well defined to use multiple contiguous sets of
curly brackets in a dial string to pass channel variables.

The patch also corrects the following code from the original source:

		char *e = switch_find_end_paren(data, '{', '}');

		if (e) {
			vars = data + 1;
			*e++ = '\0';
			data = e;

If data does not contain a closing bracket, then switch_find_end_paren
will return the address of the end of the string.  The code above
would then set data to the first address in memory after the end of
the string, which would be very undefined.

Cheers,

-- tc

    

[Freeswitch-dev] [PATCH] fixing conference dial flexibly, fixing bad pointer op

Travis Cross

Anthony Minessale

Travis Cross

tags

participants (2)