[Asterisk-code-review] Change in asterisk[13]: Detect potential forwarding loops based on count.

Matt Jordan (Code Review) asteriskteam at digium.com
Fri Apr 17 15:57:49 CDT 2015


Matt Jordan has submitted this change and it was merged.

Change subject: Detect potential forwarding loops based on count.
......................................................................


Detect potential forwarding loops based on count.

A potential problem that can arise is the following:

* Bob's phone is programmed to automatically forward to Carol.
* Carol's phone is programmed to automatically forward to Bob.
* Alice calls Bob.

If left unchecked, this results in an endless loops of call forwards
that would eventually result in some sort of fiery crash.

Asterisk's method of solving this issue was to track which interfaces
had been dialed. If a destination were dialed a second time, then
the attempt to call that destination would fail since a loop was
detected.

The problem with this method is that call forwarding has evolved. Some
SIP phones allow for a user to manually forward an incoming call to an
ad-hoc destination. This can mean that:

* There are legitimate use cases where a device may be dialed multiple
times, or
* There can be human error when forwarding calls.

This change removes the old method of detecting forwarding loops in
favor of keeping a count of the number of destinations a channel has
dialed on a particular branch of a call. If the number exceeds the
set number of max forwards, then the call fails. This approach has
the following advantages over the old:

* It is much simpler.
* It can detect loops involving local channels.
* It is user configurable.

The only disadvantage it has is that in the case where there is a
legitimate forwarding loop present, it takes longer to detect it.
However, the forwarding loop is still properly detected and the
call is cleaned up as it should be.

Address review feedback on gerrit.

* Correct "mfgium" to "Digium"
* Decrement max forwards by one in the case where allocation of the
  max forwards datastore is required.
* Remove irrelevant code change from pjsip_global_headers.c

ASTERISK-24958 #close

Change-Id: Ia7e4b7cd3bccfbd34d9a859838356931bba56c23
---
M apps/app_dial.c
M apps/app_followme.c
M apps/app_queue.c
M funcs/func_channel.c
M include/asterisk/global_datastores.h
A include/asterisk/max_forwards.h
M main/ccss.c
M main/channel.c
M main/dial.c
M main/features.c
M main/global_datastores.c
A main/max_forwards.c
M res/res_pjsip_diversion.c
13 files changed, 335 insertions(+), 244 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, approved; Verified



diff --git a/apps/app_dial.c b/apps/app_dial.c
index ff18f95..b6c0751 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -58,7 +58,6 @@
 #include "asterisk/manager.h"
 #include "asterisk/privacy.h"
 #include "asterisk/stringfields.h"
-#include "asterisk/global_datastores.h"
 #include "asterisk/dsp.h"
 #include "asterisk/aoc.h"
 #include "asterisk/ccss.h"
@@ -68,6 +67,7 @@
 #include "asterisk/stasis_channels.h"
 #include "asterisk/bridge_after.h"
 #include "asterisk/features_config.h"
+#include "asterisk/max_forwards.h"
 
 /*** DOCUMENTATION
 	<application name="Dial" language="en_US">
@@ -881,6 +881,7 @@
 			ast_channel_lock_both(in, o->chan);
 			ast_channel_inherit_variables(in, o->chan);
 			ast_channel_datastore_inherit(in, o->chan);
+			ast_max_forwards_decrement(o->chan);
 			ast_channel_unlock(in);
 			ast_channel_unlock(o->chan);
 			/* When a call is forwarded, we don't want to track new interfaces
@@ -2074,7 +2075,6 @@
 	);
 	struct ast_flags64 opts = { 0, };
 	char *opt_args[OPT_ARG_ARRAY_SIZE];
-	struct ast_datastore *datastore = NULL;
 	int fulldial = 0, num_dialed = 0;
 	int ignore_cc = 0;
 	char device_name[AST_CHANNEL_NAME];
@@ -2101,6 +2101,7 @@
 	 * \note This will not have any malloced strings so do not free it.
 	 */
 	struct ast_party_caller caller;
+	int max_forwards;
 
 	/* Reset all DIAL variables back to blank, to prevent confusion (in case we don't reset all of them). */
 	ast_channel_lock(chan);
@@ -2111,7 +2112,15 @@
 	pbx_builtin_setvar_helper(chan, "ANSWEREDTIME", "");
 	pbx_builtin_setvar_helper(chan, "DIALEDTIME", "");
 	ast_channel_stage_snapshot_done(chan);
+	max_forwards = ast_max_forwards_get(chan);
 	ast_channel_unlock(chan);
+
+	if (max_forwards <= 0) {
+		ast_log(LOG_WARNING, "Cannot place outbound call from channel '%s'. Max forwards exceeded\n",
+				ast_channel_name(chan));
+		pbx_builtin_setvar_helper(chan, "DIALSTATUS", "BUSY");
+		return -1;
+	}
 
 	if (ast_strlen_zero(data)) {
 		ast_log(LOG_WARNING, "Dial requires an argument (technology/resource)\n");
@@ -2314,9 +2323,6 @@
 		char *tech = strsep(&number, "/");
 		size_t tech_len;
 		size_t number_len;
-		/* find if we already dialed this interface */
-		struct ast_dialed_interface *di;
-		AST_LIST_HEAD(,ast_dialed_interface) *dialed_interfaces;
 
 		num_dialed++;
 		if (ast_strlen_zero(number)) {
@@ -2360,7 +2366,6 @@
 		/* Request the peer */
 
 		ast_channel_lock(chan);
-		datastore = ast_channel_datastore_find(chan, &dialed_interface_info, NULL);
 		/*
 		 * Seed the chanlist's connected line information with previously
 		 * acquired connected line info from the incoming channel.  The
@@ -2369,61 +2374,6 @@
 		 */
 		ast_party_connected_line_copy(&tmp->connected, ast_channel_connected(chan));
 		ast_channel_unlock(chan);
-
-		if (datastore)
-			dialed_interfaces = datastore->data;
-		else {
-			if (!(datastore = ast_datastore_alloc(&dialed_interface_info, NULL))) {
-				ast_log(LOG_WARNING, "Unable to create channel datastore for dialed interfaces. Aborting!\n");
-				chanlist_free(tmp);
-				goto out;
-			}
-			datastore->inheritance = DATASTORE_INHERIT_FOREVER;
-
-			if (!(dialed_interfaces = ast_calloc(1, sizeof(*dialed_interfaces)))) {
-				ast_datastore_free(datastore);
-				chanlist_free(tmp);
-				goto out;
-			}
-
-			datastore->data = dialed_interfaces;
-			AST_LIST_HEAD_INIT(dialed_interfaces);
-
-			ast_channel_lock(chan);
-			ast_channel_datastore_add(chan, datastore);
-			ast_channel_unlock(chan);
-		}
-
-		AST_LIST_LOCK(dialed_interfaces);
-		AST_LIST_TRAVERSE(dialed_interfaces, di, list) {
-			if (!strcasecmp(di->interface, tmp->interface)) {
-				ast_log(LOG_WARNING, "Skipping dialing interface '%s' again since it has already been dialed\n",
-					di->interface);
-				break;
-			}
-		}
-		AST_LIST_UNLOCK(dialed_interfaces);
-		if (di) {
-			fulldial++;
-			chanlist_free(tmp);
-			continue;
-		}
-
-		/* It is always ok to dial a Local interface.  We only keep track of
-		 * which "real" interfaces have been dialed.  The Local channel will
-		 * inherit this list so that if it ends up dialing a real interface,
-		 * it won't call one that has already been called. */
-		if (strcasecmp(tmp->tech, "Local")) {
-			if (!(di = ast_calloc(1, sizeof(*di) + strlen(tmp->interface)))) {
-				chanlist_free(tmp);
-				goto out;
-			}
-			strcpy(di->interface, tmp->interface);
-
-			AST_LIST_LOCK(dialed_interfaces);
-			AST_LIST_INSERT_TAIL(dialed_interfaces, di, list);
-			AST_LIST_UNLOCK(dialed_interfaces);
-		}
 
 		tc = ast_request(tmp->tech, ast_channel_nativeformats(chan), NULL, chan, tmp->number, &cause);
 		if (!tc) {
@@ -2465,6 +2415,7 @@
 		/* Inherit specially named variables from parent channel */
 		ast_channel_inherit_variables(chan, tc);
 		ast_channel_datastore_inherit(chan, tc);
+		ast_max_forwards_decrement(tc);
 
 		ast_channel_appl_set(tc, "AppDial");
 		ast_channel_data_set(tc, "(Outgoing Line)");
@@ -2680,18 +2631,6 @@
 	peer = wait_for_answer(chan, &out_chans, &to, peerflags, opt_args, &pa, &num, &result,
 		dtmf_progress, ignore_cc, &forced_clid, &stored_clid);
 
-	/* The ast_channel_datastore_remove() function could fail here if the
-	 * datastore was moved to another channel during a masquerade. If this is
-	 * the case, don't free the datastore here because later, when the channel
-	 * to which the datastore was moved hangs up, it will attempt to free this
-	 * datastore again, causing a crash
-	 */
-	ast_channel_lock(chan);
-	datastore = ast_channel_datastore_find(chan, &dialed_interface_info, NULL); /* make sure we weren't cleaned up already */
-	if (datastore && !ast_channel_datastore_remove(chan, datastore)) {
-		ast_datastore_free(datastore);
-	}
-	ast_channel_unlock(chan);
 	if (!peer) {
 		if (result) {
 			res = result;
diff --git a/apps/app_followme.c b/apps/app_followme.c
index 8531b17..e5a5ee3 100644
--- a/apps/app_followme.c
+++ b/apps/app_followme.c
@@ -64,6 +64,7 @@
 #include "asterisk/dsp.h"
 #include "asterisk/app.h"
 #include "asterisk/stasis_channels.h"
+#include "asterisk/max_forwards.h"
 
 /*** DOCUMENTATION
 	<application name="FollowMe" language="en_US">
@@ -1069,6 +1070,7 @@
 			ast_connected_line_copy_from_caller(ast_channel_connected(outbound), ast_channel_caller(caller));
 			ast_channel_inherit_variables(caller, outbound);
 			ast_channel_datastore_inherit(caller, outbound);
+			ast_max_forwards_decrement(outbound);
 			ast_channel_language_set(outbound, ast_channel_language(caller));
 			ast_channel_req_accountcodes(outbound, caller, AST_CHANNEL_REQUESTOR_BRIDGE_PEER);
 			ast_channel_musicclass_set(outbound, ast_channel_musicclass(caller));
@@ -1304,12 +1306,23 @@
 		AST_APP_ARG(options);
 	);
 	char *opt_args[FOLLOWMEFLAG_ARG_ARRAY_SIZE];
+	int max_forwards;
 
 	if (ast_strlen_zero(data)) {
 		ast_log(LOG_WARNING, "%s requires an argument (followmeid)\n", app);
 		return -1;
 	}
 
+	ast_channel_lock(chan);
+	max_forwards = ast_max_forwards_get(chan);
+	ast_channel_unlock(chan);
+
+	if (max_forwards <= 0) {
+		ast_log(LOG_WARNING, "Unable to execute FollowMe on channel %s. Max forwards exceeded\n",
+				ast_channel_name(chan));
+		return -1;
+	}
+
 	argstr = ast_strdupa((char *) data);
 
 	AST_STANDARD_APP_ARGS(args, argstr);
diff --git a/apps/app_queue.c b/apps/app_queue.c
index 6d45be3..df5ff30 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -98,7 +98,6 @@
 #include "asterisk/stringfields.h"
 #include "asterisk/astobj2.h"
 #include "asterisk/strings.h"
-#include "asterisk/global_datastores.h"
 #include "asterisk/taskprocessor.h"
 #include "asterisk/aoc.h"
 #include "asterisk/callerid.h"
@@ -113,6 +112,7 @@
 #include "asterisk/mixmonitor.h"
 #include "asterisk/core_unreal.h"
 #include "asterisk/bridge_basic.h"
+#include "asterisk/max_forwards.h"
 
 /*!
  * \par Please read before modifying this file.
@@ -4260,6 +4260,7 @@
 	/* Inherit specially named variables from parent channel */
 	ast_channel_inherit_variables(qe->chan, tmp->chan);
 	ast_channel_datastore_inherit(qe->chan, tmp->chan);
+	ast_max_forwards_decrement(tmp->chan);
 
 	/* Presense of ADSI CPE on outgoing channel follows ours */
 	ast_channel_adsicpe_set(tmp->chan, ast_channel_adsicpe(qe->chan));
@@ -4753,6 +4754,7 @@
 						ast_channel_lock_both(o->chan, in);
 						ast_channel_inherit_variables(in, o->chan);
 						ast_channel_datastore_inherit(in, o->chan);
+						ast_max_forwards_decrement(o->chan);
 
 						if (o->pending_connected_update) {
 							/*
@@ -6234,10 +6236,7 @@
  *
  * Here is the process of this function
  * 1. Process any options passed to the Queue() application. Options here mean the third argument to Queue()
- * 2. Iterate trough the members of the queue, creating a callattempt corresponding to each member. During this
- *    iteration, we also check the dialed_interfaces datastore to see if we have already attempted calling this
- *    member. If we have, we do not create a callattempt. This is in place to prevent call forwarding loops. Also
- *    during each iteration, we call calc_metric to determine which members should be rung when.
+ * 2. Iterate trough the members of the queue, creating a callattempt corresponding to each member.
  * 3. Call ring_one to place a call to the appropriate member(s)
  * 4. Call wait_for_answer to wait for an answer. If no one answers, return.
  * 5. Take care of any holdtime announcements, member delays, or other options which occur after a call has been answered.
@@ -6290,12 +6289,7 @@
 	int block_connected_line = 0;
 	int callcompletedinsl;
 	struct ao2_iterator memi;
-	struct ast_datastore *datastore;
 	struct queue_end_bridge *queue_end_bridge = NULL;
-
-	ast_channel_lock(qe->chan);
-	datastore = ast_channel_datastore_find(qe->chan, &dialed_interface_info, NULL);
-	ast_channel_unlock(qe->chan);
 
 	memset(&bridge_config, 0, sizeof(bridge_config));
 	tmpid[0] = 0;
@@ -6383,72 +6377,11 @@
 	memi = ao2_iterator_init(qe->parent->members, 0);
 	while ((cur = ao2_iterator_next(&memi))) {
 		struct callattempt *tmp = ast_calloc(1, sizeof(*tmp));
-		struct ast_dialed_interface *di;
-		AST_LIST_HEAD(,ast_dialed_interface) *dialed_interfaces;
 		if (!tmp) {
 			ao2_ref(cur, -1);
 			ao2_iterator_destroy(&memi);
 			ao2_unlock(qe->parent);
 			goto out;
-		}
-		if (!datastore) {
-			if (!(datastore = ast_datastore_alloc(&dialed_interface_info, NULL))) {
-				callattempt_free(tmp);
-				ao2_ref(cur, -1);
-				ao2_iterator_destroy(&memi);
-				ao2_unlock(qe->parent);
-				goto out;
-			}
-			datastore->inheritance = DATASTORE_INHERIT_FOREVER;
-			if (!(dialed_interfaces = ast_calloc(1, sizeof(*dialed_interfaces)))) {
-				callattempt_free(tmp);
-				ao2_ref(cur, -1);
-				ao2_iterator_destroy(&memi);
-				ao2_unlock(qe->parent);
-				goto out;
-			}
-			datastore->data = dialed_interfaces;
-			AST_LIST_HEAD_INIT(dialed_interfaces);
-
-			ast_channel_lock(qe->chan);
-			ast_channel_datastore_add(qe->chan, datastore);
-			ast_channel_unlock(qe->chan);
-		} else
-			dialed_interfaces = datastore->data;
-
-		AST_LIST_LOCK(dialed_interfaces);
-		AST_LIST_TRAVERSE(dialed_interfaces, di, list) {
-			if (!strcasecmp(cur->interface, di->interface)) {
-				ast_debug(1, "Skipping dialing interface '%s' since it has already been dialed\n",
-					di->interface);
-				break;
-			}
-		}
-		AST_LIST_UNLOCK(dialed_interfaces);
-
-		if (di) {
-			callattempt_free(tmp);
-			ao2_ref(cur, -1);
-			continue;
-		}
-
-		/* It is always ok to dial a Local interface.  We only keep track of
-		 * which "real" interfaces have been dialed.  The Local channel will
-		 * inherit this list so that if it ends up dialing a real interface,
-		 * it won't call one that has already been called. */
-		if (strncasecmp(cur->interface, "Local/", 6)) {
-			if (!(di = ast_calloc(1, sizeof(*di) + strlen(cur->interface)))) {
-				callattempt_free(tmp);
-				ao2_ref(cur, -1);
-				ao2_iterator_destroy(&memi);
-				ao2_unlock(qe->parent);
-				goto out;
-			}
-			strcpy(di->interface, cur->interface);
-
-			AST_LIST_LOCK(dialed_interfaces);
-			AST_LIST_INSERT_TAIL(dialed_interfaces, di, list);
-			AST_LIST_UNLOCK(dialed_interfaces);
 		}
 
 		/*
@@ -6508,16 +6441,7 @@
 	lpeer = wait_for_answer(qe, outgoing, &to, &digit, numbusies,
 		ast_test_flag(&(bridge_config.features_caller), AST_FEATURE_DISCONNECT),
 		forwardsallowed, ringing);
-	/* The ast_channel_datastore_remove() function could fail here if the
-	 * datastore was moved to another channel during a masquerade. If this is
-	 * the case, don't free the datastore here because later, when the channel
-	 * to which the datastore was moved hangs up, it will attempt to free this
-	 * datastore again, causing a crash
-	 */
-	ast_channel_lock(qe->chan);
-	if (datastore && !ast_channel_datastore_remove(qe->chan, datastore)) {
-		ast_datastore_free(datastore);
-	}
+
 	ast_channel_unlock(qe->chan);
 	ao2_lock(qe->parent);
 	if (qe->parent->strategy == QUEUE_STRATEGY_RRMEMORY || qe->parent->strategy == QUEUE_STRATEGY_RRORDERED) {
@@ -7709,12 +7633,22 @@
 	struct queue_ent qe = { 0 };
 	struct ast_flags opts = { 0, };
 	char *opt_args[OPT_ARG_ARRAY_SIZE];
+	int max_forwards;
 
 	if (ast_strlen_zero(data)) {
 		ast_log(LOG_WARNING, "Queue requires an argument: queuename[,options[,URL[,announceoverride[,timeout[,agi[,macro[,gosub[,rule[,position]]]]]]]]]\n");
 		return -1;
 	}
 
+	ast_channel_lock(chan);
+	max_forwards = ast_max_forwards_get(chan);
+	ast_channel_unlock(chan);
+
+	if (max_forwards <= 0) {
+		ast_log(LOG_WARNING, "Channel '%s' cannot enter queue. Max forwards exceeded\n", ast_channel_name(chan));
+		return -1;
+	}
+
 	parse = ast_strdupa(data);
 	AST_STANDARD_APP_ARGS(args, parse);
 
diff --git a/funcs/func_channel.c b/funcs/func_channel.c
index 4024cdf..7f87f45 100644
--- a/funcs/func_channel.c
+++ b/funcs/func_channel.c
@@ -46,6 +46,7 @@
 #include "asterisk/global_datastores.h"
 #include "asterisk/bridge_basic.h"
 #include "asterisk/bridge_after.h"
+#include "asterisk/max_forwards.h"
 
 /*** DOCUMENTATION
 	<function name="CHANNELS" language="en_US">
@@ -388,6 +389,16 @@
 					<enum name="caller_url">
 						<para>R/0 Returns caller URL</para>
 					</enum>
+					<enum name="max_forwards">
+						<para>R/W Get or set the maximum number of call forwards for this channel.
+
+						This number describes the number of times a call may be forwarded by this channel
+						before the call fails. "Forwards" in this case refers to redirects by phones as well
+						as calls to local channels.
+
+						Note that this has no relation to the SIP Max-Forwards header.
+						</para>
+					</enum>
 				</enumlist>
 			</parameter>
 		</syntax>
@@ -577,6 +588,10 @@
 			}
 		}
 		ast_channel_unlock(chan);
+	} else if (!strcasecmp(data, "max_forwards")) {
+		ast_channel_lock(chan);
+		snprintf(buf, len, "%d", ast_max_forwards_get(chan));
+		ast_channel_unlock(chan);
 	} else if (!ast_channel_tech(chan) || !ast_channel_tech(chan)->func_channel_read || ast_channel_tech(chan)->func_channel_read(chan, function, data, buf, len)) {
 		ast_log(LOG_WARNING, "Unknown or unavailable item requested: '%s'\n", data);
 		ret = -1;
@@ -737,6 +752,16 @@
 			store->media = ast_true(value) ? 1 : 0;
 		}
 		ast_channel_unlock(chan);
+	} else if (!strcasecmp(data, "max_forwards")) {
+		int max_forwards;
+		if (sscanf(value, "%d", &max_forwards) != 1) {
+			ast_log(LOG_WARNING, "Unable to set max forwards to '%s'\n", value);
+			ret = -1;
+		} else {
+			ast_channel_lock(chan);
+			ret = ast_max_forwards_set(chan, max_forwards);
+			ast_channel_unlock(chan);
+		}
 	} else if (!ast_channel_tech(chan)->func_channel_write
 		 || ast_channel_tech(chan)->func_channel_write(chan, function, data, value)) {
 		ast_log(LOG_WARNING, "Unknown or unavailable item requested: '%s'\n",
diff --git a/include/asterisk/global_datastores.h b/include/asterisk/global_datastores.h
index 16267a8..2946ede 100644
--- a/include/asterisk/global_datastores.h
+++ b/include/asterisk/global_datastores.h
@@ -26,13 +26,7 @@
 
 #include "asterisk/channel.h"
 
-extern const struct ast_datastore_info dialed_interface_info;
 extern const struct ast_datastore_info secure_call_info;
-
-struct ast_dialed_interface {
-	AST_LIST_ENTRY(ast_dialed_interface) list;
-	char interface[1];
-};
 
 struct ast_secure_call_store {
 	unsigned int signaling:1;
diff --git a/include/asterisk/max_forwards.h b/include/asterisk/max_forwards.h
new file mode 100644
index 0000000..3130b4b
--- /dev/null
+++ b/include/asterisk/max_forwards.h
@@ -0,0 +1,78 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2015, Digium, Inc.
+ *
+ * Mark Michelson <mmichelson 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.
+ */
+
+#ifndef MAX_FORWARDS_H
+
+struct ast_channel;
+
+/*!
+ * \brief Set the starting max forwards for a particular channel.
+ *
+ * \pre chan is locked
+ *
+ * \param starting_count The value to set the max forwards to.
+ * \param chan The channel on which to set the max forwards.
+ * \retval 0 Success
+ * \retval 1 Failure
+ */
+int ast_max_forwards_set(struct ast_channel *chan, int starting_count);
+
+/*!
+ * \brief Get the current max forwards for a particular channel.
+ *
+ * If the channel has not had max forwards set on it, then the channel
+ * will have the default max forwards set on it and that value will
+ * be returned.
+ *
+ * \pre chan is locked
+ *
+ * \param chan The channel to get the max forwards for.
+ * \return The current max forwards count on the channel
+ */
+int ast_max_forwards_get(struct ast_channel *chan);
+
+/*!
+ * \brief Decrement the max forwards count for a particular channel.
+ *
+ * If the channel has not had max forwards set on it, then the channel
+ * will have the default max forwards set on it and that value will
+ * not be decremented.
+ *
+ * \pre chan is locked
+ *
+ * \chan The channel for which the max forwards value should be decremented
+ * \retval 0 Success
+ * \retval -1 Failure
+ */
+int ast_max_forwards_decrement(struct ast_channel *chan);
+
+/*!
+ * \brief Reset the max forwards on a channel to its starting value.
+ *
+ * If the channel has not had max forwards set on it, then the channel
+ * will have the default max forwards set on it.
+ *
+ * \pre chan is locked.
+ *
+ * \param chan The channel on which to reset the max forwards count.
+ * \retval 0 Success
+ * \retval -1 Failure
+ */
+int ast_max_forwards_reset(struct ast_channel *chan);
+
+#endif /* MAX_FORWARDS_H */
diff --git a/main/ccss.c b/main/ccss.c
index ff5739a..0605810 100644
--- a/main/ccss.c
+++ b/main/ccss.c
@@ -2237,9 +2237,7 @@
  * Note that it is not necessarily erroneous to add the same
  * device to the tree twice. If the same device is called by
  * two different extension during the same call, then
- * that is a legitimate situation. Of course, I'm pretty sure
- * the dialed_interfaces global datastore will not allow that
- * to happen anyway.
+ * that is a legitimate situation.
  *
  * \param device_name The name of the device being added to the tree
  * \param dialstring The dialstring used to dial the device being added
diff --git a/main/channel.c b/main/channel.c
index a9fe7e3..c56f3ab 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -75,6 +75,7 @@
 #include "asterisk/bridge.h"
 #include "asterisk/test.h"
 #include "asterisk/stasis_channels.h"
+#include "asterisk/max_forwards.h"
 
 /*** DOCUMENTATION
  ***/
@@ -5621,6 +5622,7 @@
 	ast_channel_lock_both(parent, new_chan);
 	ast_channel_inherit_variables(parent, new_chan);
 	ast_channel_datastore_inherit(parent, new_chan);
+	ast_max_forwards_decrement(new_chan);
 	ast_channel_unlock(new_chan);
 	ast_channel_unlock(parent);
 }
@@ -5740,6 +5742,7 @@
 			ast_channel_lock_both(oh->parent_channel, chan);
 			ast_channel_inherit_variables(oh->parent_channel, chan);
 			ast_channel_datastore_inherit(oh->parent_channel, chan);
+			ast_max_forwards_decrement(chan);
 			ast_channel_unlock(oh->parent_channel);
 			ast_channel_unlock(chan);
 		}
diff --git a/main/dial.c b/main/dial.c
index 827b5ef..ca85cd3 100644
--- a/main/dial.c
+++ b/main/dial.c
@@ -44,6 +44,7 @@
 #include "asterisk/app.h"
 #include "asterisk/causes.h"
 #include "asterisk/stasis_channels.h"
+#include "asterisk/max_forwards.h"
 
 /*! \brief Main dialing structure. Contains global options, channels being dialed, and more! */
 struct ast_dial {
@@ -299,6 +300,19 @@
 		.uniqueid2 = channel->assignedid2,
 	};
 
+	if (chan) {
+		int max_forwards;
+
+		ast_channel_lock(chan);
+		max_forwards = ast_max_forwards_get(chan);
+		ast_channel_unlock(chan);
+
+		if (max_forwards <= 0) {
+			ast_log(LOG_WARNING, "Cannot dial from channel '%s'. Max forwards exceeded\n",
+					ast_channel_name(chan));
+		}
+	}
+
 	/* Copy device string over */
 	ast_copy_string(numsubst, channel->device, sizeof(numsubst));
 
@@ -337,6 +351,7 @@
 	if (chan) {
 		ast_channel_inherit_variables(chan, channel->owner);
 		ast_channel_datastore_inherit(chan, channel->owner);
+		ast_max_forwards_decrement(channel->owner);
 
 		/* Copy over callerid information */
 		ast_party_redirecting_copy(ast_channel_redirecting(channel->owner), ast_channel_redirecting(chan));
diff --git a/main/features.c b/main/features.c
index f3bf7a7..2f04386 100644
--- a/main/features.c
+++ b/main/features.c
@@ -78,6 +78,7 @@
 #include "asterisk/stasis.h"
 #include "asterisk/stasis_channels.h"
 #include "asterisk/features_config.h"
+#include "asterisk/max_forwards.h"
 
 /*** DOCUMENTATION
 	<application name="Bridge" language="en_US">
@@ -421,22 +422,6 @@
 	add_features_datastore(callee, &config->features_callee, &config->features_caller);
 }
 
-static void clear_dialed_interfaces(struct ast_channel *chan)
-{
-	struct ast_datastore *di_datastore;
-
-	ast_channel_lock(chan);
-	if ((di_datastore = ast_channel_datastore_find(chan, &dialed_interface_info, NULL))) {
-		if (option_debug) {
-			ast_log(LOG_DEBUG, "Removing dialed interfaces datastore on %s since we're bridging\n", ast_channel_name(chan));
-		}
-		if (!ast_channel_datastore_remove(chan, di_datastore)) {
-			ast_datastore_free(di_datastore);
-		}
-	}
-	ast_channel_unlock(chan);
-}
-
 static void bridge_config_set_limits_warning_values(struct ast_bridge_config *config, struct ast_bridge_features_limits *limits)
 {
 	if (config->end_sound) {
@@ -573,20 +558,13 @@
 	ast_channel_log("Pre-bridge PEER Channel info", peer);
 #endif
 
-	/*
-	 * If we are bridging a call, stop worrying about forwarding
-	 * loops.  We presume that if a call is being bridged, that the
-	 * humans in charge know what they're doing.  If they don't,
-	 * well, what can we do about that?
-	 */
-	clear_dialed_interfaces(chan);
-	clear_dialed_interfaces(peer);
-
 	res = 0;
 	ast_channel_lock(chan);
+	ast_max_forwards_reset(chan);
 	res |= ast_bridge_features_ds_append(chan, &config->features_caller);
 	ast_channel_unlock(chan);
 	ast_channel_lock(peer);
+	ast_max_forwards_reset(peer);
 	res |= ast_bridge_features_ds_append(peer, &config->features_callee);
 	ast_channel_unlock(peer);
 
diff --git a/main/global_datastores.c b/main/global_datastores.c
index 92c6bb4..b63e1df 100644
--- a/main/global_datastores.c
+++ b/main/global_datastores.c
@@ -32,62 +32,6 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/global_datastores.h"
-#include "asterisk/linkedlists.h"
-
-static void dialed_interface_destroy(void *data)
-{
-	struct ast_dialed_interface *di = NULL;
-	AST_LIST_HEAD(, ast_dialed_interface) *dialed_interface_list = data;
-	
-	if (!dialed_interface_list) {
-		return;
-	}
-
-	AST_LIST_LOCK(dialed_interface_list);
-	while ((di = AST_LIST_REMOVE_HEAD(dialed_interface_list, list)))
-		ast_free(di);
-	AST_LIST_UNLOCK(dialed_interface_list);
-
-	AST_LIST_HEAD_DESTROY(dialed_interface_list);
-	ast_free(dialed_interface_list);
-}
-
-static void *dialed_interface_duplicate(void *data)
-{
-	struct ast_dialed_interface *di = NULL;
-	AST_LIST_HEAD(, ast_dialed_interface) *old_list;
-	AST_LIST_HEAD(, ast_dialed_interface) *new_list = NULL;
-
-	if(!(old_list = data)) {
-		return NULL;
-	}
-
-	if(!(new_list = ast_calloc(1, sizeof(*new_list)))) {
-		return NULL;
-	}
-
-	AST_LIST_HEAD_INIT(new_list);
-	AST_LIST_LOCK(old_list);
-	AST_LIST_TRAVERSE(old_list, di, list) {
-		struct ast_dialed_interface *di2 = ast_calloc(1, sizeof(*di2) + strlen(di->interface));
-		if(!di2) {
-			AST_LIST_UNLOCK(old_list);
-			dialed_interface_destroy(new_list);
-			return NULL;
-		}
-		strcpy(di2->interface, di->interface);
-		AST_LIST_INSERT_TAIL(new_list, di2, list);
-	}
-	AST_LIST_UNLOCK(old_list);
-
-	return new_list;
-}
-
-const struct ast_datastore_info dialed_interface_info = {
-	.type = "dialed-interface",
-	.destroy = dialed_interface_destroy,
-	.duplicate = dialed_interface_duplicate,
-};
 
 static void secure_call_store_destroy(void *data)
 {
diff --git a/main/max_forwards.c b/main/max_forwards.c
new file mode 100644
index 0000000..8f1d4ee
--- /dev/null
+++ b/main/max_forwards.c
@@ -0,0 +1,165 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2015, Digium, Inc.
+ *
+ * Mark Michelson <mmichelson at digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not mfrectly 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, mfstributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+#include "asterisk.h"
+
+#include "asterisk/max_forwards.h"
+#include "asterisk/channel.h"
+
+#define DEFAULT_MAX_FORWARDS 20
+
+/*!
+ * \brief Channel datastore data for max forwards
+ */
+struct max_forwards {
+	/*! The starting count. Used to allow resetting to the original value */
+	int starting_count;
+	/*! The current count. When this reaches 0, you're outta luck */
+	int current_count;
+};
+
+static struct max_forwards *max_forwards_alloc(int starting_count, int current_count)
+{
+	struct max_forwards *mf;
+
+	mf = ast_malloc(sizeof(*mf));
+	if (!mf) {
+		return NULL;
+	}
+
+	mf->starting_count = starting_count;
+	mf->current_count = current_count;
+
+	return mf;
+}
+
+static void *max_forwards_duplicate(void *data)
+{
+	struct max_forwards *mf = data;
+
+	return max_forwards_alloc(mf->starting_count, mf->current_count);
+}
+
+static void max_forwards_destroy(void *data)
+{
+	ast_free(data);
+}
+
+const struct ast_datastore_info max_forwards_info = {
+	.type = "mfaled-interface",
+	.duplicate = max_forwards_duplicate,
+	.destroy = max_forwards_destroy,
+};
+
+static struct ast_datastore *max_forwards_datastore_alloc(struct ast_channel *chan,
+		int starting_count)
+{
+	struct ast_datastore *mf_datastore;
+	struct max_forwards *mf;
+
+	mf_datastore = ast_datastore_alloc(&max_forwards_info, NULL);
+	if (!mf_datastore) {
+		return NULL;
+	}
+	mf_datastore->inheritance = DATASTORE_INHERIT_FOREVER;
+
+	mf = max_forwards_alloc(starting_count, starting_count);
+	if (!mf) {
+		ast_datastore_free(mf_datastore);
+		return NULL;
+	}
+	mf_datastore->data = mf;
+
+	ast_channel_datastore_add(chan, mf_datastore);
+
+	return mf_datastore;
+}
+
+static struct ast_datastore *max_forwards_datastore_find_or_alloc(struct ast_channel *chan)
+{
+	struct ast_datastore *mf_datastore;
+
+	mf_datastore = ast_channel_datastore_find(chan, &max_forwards_info, NULL);
+	if (!mf_datastore) {
+		mf_datastore = max_forwards_datastore_alloc(chan, DEFAULT_MAX_FORWARDS);
+	}
+
+	return mf_datastore;
+}
+
+int ast_max_forwards_set(struct ast_channel *chan, int starting_count)
+{
+	struct ast_datastore *mf_datastore;
+	struct max_forwards *mf;
+
+	mf_datastore = max_forwards_datastore_find_or_alloc(chan);
+	if (!mf_datastore) {
+		return -1;
+	}
+
+	mf = mf_datastore->data;
+	mf->starting_count = mf->current_count = starting_count;
+
+	return 0;
+}
+
+int ast_max_forwards_get(struct ast_channel *chan)
+{
+	struct ast_datastore *mf_datastore;
+	struct max_forwards *mf;
+
+	mf_datastore = max_forwards_datastore_find_or_alloc(chan);
+	if (!mf_datastore) {
+		return -1;
+	}
+
+	mf = mf_datastore->data;
+	return mf->current_count;
+}
+
+int ast_max_forwards_decrement(struct ast_channel *chan)
+{
+	struct ast_datastore *mf_datastore;
+	struct max_forwards *mf;
+
+	mf_datastore = max_forwards_datastore_find_or_alloc(chan);
+	if (!mf_datastore) {
+		return -1;
+	}
+
+	mf = mf_datastore->data;
+	--mf->current_count;
+
+	return 0;
+}
+
+int ast_max_forwards_reset(struct ast_channel *chan)
+{
+	struct ast_datastore *mf_datastore;
+	struct max_forwards *mf;
+
+	mf_datastore = max_forwards_datastore_find_or_alloc(chan);
+	if (!mf_datastore) {
+		return -1;
+	}
+
+	mf = mf_datastore->data;
+	mf->current_count = mf->starting_count;
+
+	return 0;
+}
diff --git a/res/res_pjsip_diversion.c b/res/res_pjsip_diversion.c
index a4ac157..49f7892 100644
--- a/res/res_pjsip_diversion.c
+++ b/res/res_pjsip_diversion.c
@@ -248,6 +248,7 @@
 	pjsip_name_addr *name_addr;
 	pjsip_sip_uri *uri;
 	pjsip_param *param;
+	pjsip_fromto_hdr *old_hdr;
 
 	struct ast_party_id *id = &data->from;
 	pjsip_uri *base = PJSIP_MSG_FROM_HDR(tdata->msg)->uri;
@@ -273,6 +274,10 @@
 	pj_list_insert_before(&hdr->other_param, param);
 
 	hdr->uri = (pjsip_uri *) name_addr;
+	old_hdr = pjsip_msg_find_hdr_by_name(tdata->msg, &diversion_name, NULL);
+	if (old_hdr) {
+		pj_list_erase(old_hdr);
+	}
 	pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr *)hdr);
 }
 

-- 
To view, visit https://gerrit.asterisk.org/121
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7e4b7cd3bccfbd34d9a859838356931bba56c23
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>



More information about the asterisk-code-review mailing list