summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Michelson <mmichelson@digium.com>2016-10-17 14:18:57 -0500
committerMark Michelson <mmichelson@digium.com>2016-10-20 12:59:06 -0500
commite459b8dadf42a3a015b312dfa9aadf507b4c85d9 (patch)
tree283ece75c7142c563e26a85e616a7cb50a91d5ca
parente03364c40aa073b69ca8c6652fd17a91908b9aaf (diff)
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
-rw-r--r--include/asterisk/channel.h12
-rw-r--r--include/asterisk/channel_internal.h2
-rw-r--r--main/channel.c53
-rw-r--r--main/channel_internal_api.c22
-rw-r--r--res/ari/resource_channels.c7
-rw-r--r--res/res_ari_channels.c2
-rw-r--r--rest-api/api-docs/channels.json8
7 files changed, 100 insertions, 6 deletions
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index df752c902..ff92cc878 100644
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -4659,4 +4659,16 @@ int ast_channel_feature_hooks_append(struct ast_channel *chan, struct ast_bridge
*/
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 d1231b400..2316e2f24 100644
--- a/include/asterisk/channel_internal.h
+++ b/include/asterisk/channel_internal.h
@@ -25,3 +25,5 @@ int ast_channel_internal_is_finalized(struct ast_channel *chan);
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 6804496f4..4a9fe72a8 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -770,6 +770,27 @@ static const struct ast_channel_tech null_tech = {
static void ast_channel_destructor(void *obj);
static void ast_dummy_channel_destructor(void *obj);
+static int ast_channel_by_uniqueid_cb(void *obj, void *arg, void *data, int flags);
+
+static int does_id_conflict(const char *uniqueid)
+{
+ struct ast_channel *conflict;
+ int length = 0;
+
+ if (ast_strlen_zero(uniqueid)) {
+ return 0;
+ }
+
+ conflict = ast_channel_callback(ast_channel_by_uniqueid_cb, (char *) uniqueid, &length, OBJ_NOLOCK);
+ 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;
+}
/*! \brief Create a new channel structure */
static struct ast_channel * attribute_malloc __attribute__((format(printf, 15, 0)))
@@ -945,16 +966,33 @@ __ast_channel_alloc_ap(int needqueue, int state, const char *cid_num, const char
ast_channel_tech_set(tmp, &null_tech);
}
- ast_channel_internal_finalize(tmp);
-
- ast_atomic_fetchadd_int(&chancount, +1);
-
/* You might scream "locking inversion" at seeing this but it is actually perfectly fine.
* Since the channel was just created nothing can know about it yet or even acquire it.
*/
ast_channel_lock(tmp);
- ao2_link(channels, tmp);
+ ao2_lock(channels);
+
+ if (assignedids && (does_id_conflict(assignedids->uniqueid) || does_id_conflict(assignedids->uniqueid2))) {
+ ast_channel_internal_errno_set(AST_CHANNEL_ERROR_ID_EXISTS);
+ ao2_unlock(channels);
+ /* This is a bit unorthodox, but we can't just call ast_channel_stage_snapshot_done()
+ * because that will result in attempting to publish the channel snapshot. That causes
+ * badness in some places, such as CDRs. So we need to manually clear the flag on the
+ * channel that says that a snapshot is being cleared.
+ */
+ ast_clear_flag(ast_channel_flags(tmp), AST_FLAG_SNAPSHOT_STAGE);
+ ast_channel_unlock(tmp);
+ return ast_channel_unref(tmp);
+ }
+
+ ast_channel_internal_finalize(tmp);
+
+ ast_atomic_fetchadd_int(&chancount, +1);
+
+ ao2_link_flags(channels, tmp, OBJ_NOLOCK);
+
+ ao2_unlock(channels);
if (endpoint) {
ast_endpoint_add_channel(endpoint, tmp);
@@ -10866,3 +10904,8 @@ int ast_channel_feature_hooks_replace(struct ast_channel *chan, struct ast_bridg
{
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 d94b267e6..3c156d4fa 100644
--- a/main/channel_internal_api.c
+++ b/main/channel_internal_api.c
@@ -1661,3 +1661,25 @@ int ast_channel_internal_setup_topics(struct ast_channel *chan)
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 8d3292129..b00b2378d 100644
--- a/res/ari/resource_channels.c
+++ b/res/ari/resource_channels.c
@@ -1230,7 +1230,12 @@ static void ari_channels_handle_originate_with_id(const char *args_endpoint,
}
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 25da17d4d..b7c088c4d 100644
--- a/res/res_ari_channels.c
+++ b/res/res_ari_channels.c
@@ -253,6 +253,7 @@ static void ast_ari_channels_originate_cb(
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 @@ static void ast_ari_channels_originate_with_id_cb(
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 ee18bfe11..60ace075d 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."
}
]