[Asterisk-code-review] ARI: Detect duplicate channel IDs (asterisk[master])

Mark Michelson asteriskteam at digium.com
Tue Oct 18 17:17:57 CDT 2016


Mark Michelson has uploaded a new change for review. ( https://gerrit.asterisk.org/4155 )

Change subject: ARI: Detect duplicate channel IDs
......................................................................

ARI: Detect duplicate channel IDs

ARI and AMI allow for an explicit channel ID to be specified
when originating channels. Unfortunately, there is nothing in
place to prevent someone from using the same ID for multiple
channels. Further complicating things, adding ID validation to channel
allocation makes it impossible for ARI to discern why channel allocation
failed, resulting in a vague error code being returned.

The fix for this is to institute a new method for channel errors to be
discerned. The method mirrors errno, in that when an error occurs, the
caller can consult the channel errno value to determine what the error
was. This initial iteration of the feature only introduces "unknown" and
"channel ID exists" errors. However, it's possible to add more errors as
needed.

ARI uses this feature to determine why channel allocation failed and can
return a 409 error during origination to show that a channel with the
given ID already exists.

ASTERISK-26421

Change-Id: Ibba7ae68842dab6df0c2e9c45559208bc89d3d06
---
M include/asterisk/channel.h
M include/asterisk/channel_internal.h
M main/channel.c
M main/channel_internal_api.c
M res/ari/resource_channels.c
M res/res_ari_channels.c
M rest-api/api-docs/channels.json
7 files changed, 82 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/55/4155/1

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index df752c9..ff92cc8 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4659,4 +4659,16 @@
  */
 int ast_channel_feature_hooks_replace(struct ast_channel *chan, struct ast_bridge_features *features);
 
+enum ast_channel_error {
+	/* Unable to determine what error occurred. */
+	AST_CHANNEL_ERROR_UNKNOWN,
+	/* Channel with this ID already exists */
+	AST_CHANNEL_ERROR_ID_EXISTS,
+};
+
+/*!
+ * \brief Get error code for latest channel operation.
+ */
+enum ast_channel_error ast_channel_errno(void);
+
 #endif /* _ASTERISK_CHANNEL_H */
diff --git a/include/asterisk/channel_internal.h b/include/asterisk/channel_internal.h
index d1231b4..2316e2f 100644
--- a/include/asterisk/channel_internal.h
+++ b/include/asterisk/channel_internal.h
@@ -25,3 +25,5 @@
 void ast_channel_internal_cleanup(struct ast_channel *chan);
 int ast_channel_internal_setup_topics(struct ast_channel *chan);
 
+void ast_channel_internal_errno_set(enum ast_channel_error error);
+enum ast_channel_error ast_channel_internal_errno(void);
diff --git a/main/channel.c b/main/channel.c
index 6804496..c36d219 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -10866,3 +10866,8 @@
 {
 	return channel_feature_hooks_set_full(chan, features, 1);
 }
+
+enum ast_channel_error ast_channel_errno(void)
+{
+	return ast_channel_internal_errno();
+}
diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c
index d94b267..79a4821 100644
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -1473,9 +1473,34 @@
 
 #define DIALED_CAUSES_BUCKETS 37
 
+static int does_id_conflict(const char *uniqueid)
+{
+	struct ast_channel *conflict;
+
+	if (ast_strlen_zero(uniqueid)) {
+		return 0;
+	}
+
+	conflict = ast_channel_get_by_name(uniqueid);
+	if (conflict) {
+		ast_log(LOG_ERROR, "Channel Unique ID '%s' already in use by channel %s(%p)\n",
+			uniqueid, ast_channel_name(conflict), conflict);
+		ast_channel_unref(conflict);
+		return 1;
+	}
+
+	return 0;
+}
+
 struct ast_channel *__ast_channel_internal_alloc(void (*destructor)(void *obj), const struct ast_assigned_ids *assignedids, const struct ast_channel *requestor, const char *file, int line, const char *function)
 {
 	struct ast_channel *tmp;
+
+	/* Take care of id conflicts early so we don't do unnecessary allocations */
+	if (assignedids && (does_id_conflict(assignedids->uniqueid) || does_id_conflict(assignedids->uniqueid2))) {
+		ast_channel_internal_errno_set(AST_CHANNEL_ERROR_ID_EXISTS);
+		return NULL;
+	}
 
 	tmp = __ao2_alloc(sizeof(*tmp), destructor,
 		AO2_ALLOC_OPT_LOCK_MUTEX, "", file, line, function);
@@ -1661,3 +1686,25 @@
 
 	return 0;
 }
+
+AST_THREADSTORAGE(channel_errno);
+
+void ast_channel_internal_errno_set(enum ast_channel_error error)
+{
+	enum ast_channel_error *error_code = ast_threadstorage_get(&channel_errno, sizeof(*error_code));
+	if (!error_code) {
+		return;
+	}
+
+	*error_code = error;
+}
+
+enum ast_channel_error ast_channel_internal_errno(void)
+{
+	enum ast_channel_error *error_code = ast_threadstorage_get(&channel_errno, sizeof(*error_code));
+	if (!error_code) {
+		return AST_CHANNEL_ERROR_UNKNOWN;
+	}
+
+	return *error_code;
+}
diff --git a/res/ari/resource_channels.c b/res/ari/resource_channels.c
index 8d32921..b00b237 100644
--- a/res/ari/resource_channels.c
+++ b/res/ari/resource_channels.c
@@ -1230,7 +1230,12 @@
 	}
 
 	if (ast_dial_prerun(dial, other, format_cap)) {
-		ast_ari_response_alloc_failed(response);
+		if (ast_channel_errno() == AST_CHANNEL_ERROR_ID_EXISTS) {
+			ast_ari_response_error(response, 409, "Conflict",
+				"Channel with given unique ID already exists");
+		} else {
+			ast_ari_response_alloc_failed(response);
+		}
 		ast_dial_destroy(dial);
 		ast_free(origination);
 		ast_channel_cleanup(other);
diff --git a/res/res_ari_channels.c b/res/res_ari_channels.c
index 25da17d..b7c088c 100644
--- a/res/res_ari_channels.c
+++ b/res/res_ari_channels.c
@@ -253,6 +253,7 @@
 	case 500: /* Internal Server Error */
 	case 501: /* Not Implemented */
 	case 400: /* Invalid parameters for originating a channel. */
+	case 409: /* Channel with given unique ID already exists. */
 		is_valid = 1;
 		break;
 	default:
@@ -615,6 +616,7 @@
 	case 500: /* Internal Server Error */
 	case 501: /* Not Implemented */
 	case 400: /* Invalid parameters for originating a channel. */
+	case 409: /* Channel with given unique ID already exists. */
 		is_valid = 1;
 		break;
 	default:
diff --git a/rest-api/api-docs/channels.json b/rest-api/api-docs/channels.json
index ee18bfe..60ace07 100644
--- a/rest-api/api-docs/channels.json
+++ b/rest-api/api-docs/channels.json
@@ -142,6 +142,10 @@
 						{
 							"code": 400,
 							"reason": "Invalid parameters for originating a channel."
+						},
+						{
+							"code": 409,
+							"reason": "Channel with given unique ID already exists."
 						}
 					]
 				}
@@ -368,6 +372,10 @@
 						{
 							"code": 400,
 							"reason": "Invalid parameters for originating a channel."
+						},
+						{
+							"code": 409,
+							"reason": "Channel with given unique ID already exists."
 						}
 					]
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibba7ae68842dab6df0c2e9c45559208bc89d3d06
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list