ARI: Correct error codes for bridge operations

This patch adds error checking to ARI bridge operations, when
adding/removing channels to/from bridges.

In general, the error codes fall out as follows:
 * Bridge not found - 404 Not Found
 * Bridge not in Stasis - 409 Conflict
 * Channel not found - 400 Bad Request
 * Channel not in Stasis - 422 Unprocessable Entity
 * Channel not in this bridge (on remove) - 422 Unprocessable Entity

(closes issue ASTERISK-22036)
Review: https://reviewboard.asterisk.org/r/2769/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@397565 65c4cc65-6c06-0410-ace0-fbb531ad65f3

3 files changed, 51 insertions, 21 deletions

diff --git a/res/ari/resource_bridges.c b/res/ari/resource_bridges.c
index 670792fc1..ef7767f71 100644
--- a/res/ari/resource_bridges.c
+++ b/res/ari/resource_bridges.c
@@ -99,6 +99,18 @@ static struct stasis_app_control *find_channel_control(
 
 	control = stasis_app_control_find_by_channel_id(channel_id);
 	if (control == NULL) {
+		/* Distinguish between 400 and 422 errors */
+		RAII_VAR(struct ast_channel_snapshot *, snapshot, NULL,
+			ao2_cleanup);
+		snapshot = ast_channel_snapshot_get_latest(channel_id);
+		if (snapshot == NULL) {
+			ast_log(LOG_DEBUG, "Couldn't find '%s'\n", channel_id);
+			ast_ari_response_error(response, 400, "Bad Request",
+				"Channel not found");
+			return NULL;
+		}
+
+		ast_log(LOG_DEBUG, "Found non-stasis '%s'\n", channel_id);
 		ast_ari_response_error(response, 422, "Unprocessable Entity",
 			"Channel not in Stasis application");
 		return NULL;
@@ -145,6 +157,7 @@ static struct control_list *control_list_create(struct ast_ari_response *respons
 		list->controls[list->count] =
 			find_channel_control(response, channels[i]);
 		if (!list->controls[list->count]) {
+			/* response filled in by find_channel_control() */
 			return NULL;
 		}
 		++list->count;
@@ -166,7 +179,7 @@ void ast_ari_add_channel_to_bridge(struct ast_variable *headers, struct ast_add_
 	size_t i;
 
 	if (!bridge) {
-		/* Response filled in by find_bridge */
+		/* Response filled in by find_bridge() */
 		return;
 	}
 
@@ -200,7 +213,7 @@ void ast_ari_remove_channel_from_bridge(struct ast_variable *headers, struct ast
 	size_t i;
 
 	if (!bridge) {
-		/* Response filled in by find_bridge */
+		/* Response filled in by find_bridge() */
 		return;
 	}
 
@@ -210,17 +223,22 @@ void ast_ari_remove_channel_from_bridge(struct ast_variable *headers, struct ast
 		return;
 	}
 
-	/* BUGBUG this should make sure the bridge requested for removal is actually
-	 * the bridge the channel is in. This will be possible once the bridge uniqueid
-	 * is added to the channel snapshot. A 409 response should be issued if the bridge
-	 * uniqueids don't match */
+	/* Make sure all of the channels are in this bridge */
 	for (i = 0; i < list->count; ++i) {
-		stasis_app_control_remove_channel_from_bridge(list->controls[i],
-			bridge);
+		if (stasis_app_get_bridge(list->controls[i]) != bridge) {
+			ast_log(LOG_WARNING, "Channel %s not in bridge %s\n",
+				args->channel[i], args->bridge_id);
+			ast_ari_response_error(response, 422,
+				"Unprocessable Entity",
+				"Channel not in this bridge");
+			return;
+		}
 	}
 
-	if (response->response_code) {
-		return;
+	/* Now actually remove it */
+	for (i = 0; i < list->count; ++i) {
+		stasis_app_control_remove_channel_from_bridge(list->controls[i],
+			bridge);
 	}
 
 	ast_ari_response_no_content(response);
diff --git a/res/res_ari_bridges.c b/res/res_ari_bridges.c
index fe72df714..bc8e20041 100644
--- a/res/res_ari_bridges.c
+++ b/res/res_ari_bridges.c
@@ -345,9 +345,10 @@ static void ast_ari_add_channel_to_bridge_cb(
 		break;
 	case 500: /* Internal Server Error */
 	case 501: /* Not Implemented */
+	case 400: /* Channel not found */
 	case 404: /* Bridge not found */
 	case 409: /* Bridge not in Stasis application */
-	case 422: /* Channel not found, or not in Stasis application */
+	case 422: /* Channel not in Stasis application */
 		is_valid = 1;
 		break;
 	default:
@@ -444,6 +445,10 @@ static void ast_ari_remove_channel_from_bridge_cb(
 		break;
 	case 500: /* Internal Server Error */
 	case 501: /* Not Implemented */
+	case 400: /* Channel not found */
+	case 404: /* Bridge not found */
+	case 409: /* Bridge not in Stasis application */
+	case 422: /* Channel not in this bridge */
 		is_valid = 1;
 		break;
 	default:
diff --git a/res/stasis/control.c b/res/stasis/control.c
index df279165f..8530abd9f 100644
--- a/res/stasis/control.c
+++ b/res/stasis/control.c
@@ -510,6 +510,9 @@ static void bridge_after_cb_failed(enum ast_bridge_after_cb_reason reason,
 		ast_bridge_after_cb_reason_string(reason));
 }
 
+static int OK = 0;
+static int FAIL = -1;
+
 static void *app_control_add_channel_to_bridge(
 	struct stasis_app_control *control,
 	struct ast_channel *chan, void *data)
@@ -542,7 +545,7 @@ static void *app_control_add_channel_to_bridge(
 		bridge_after_cb_failed, control);
 	if (res != 0) {
 		ast_log(LOG_ERROR, "Error setting after-bridge callback\n");
-		return NULL;
+		return &FAIL;
 	}
 
 	{
@@ -569,22 +572,24 @@ static void *app_control_add_channel_to_bridge(
 			ast_log(LOG_ERROR, "Error adding channel to bridge\n");
 			ast_channel_pbx_set(chan, control->pbx);
 			control->pbx = NULL;
-			return NULL;
+			return &FAIL;
 		}
 
 		ast_assert(stasis_app_get_bridge(control) == NULL);
 		control->bridge = bridge;
 	}
-	return NULL;
+	return &OK;
 }
 
-void stasis_app_control_add_channel_to_bridge(
+int stasis_app_control_add_channel_to_bridge(
 	struct stasis_app_control *control, struct ast_bridge *bridge)
 {
+	int *res;
 	ast_debug(3, "%s: Sending channel add_to_bridge command\n",
 			stasis_app_control_get_channel_id(control));
-	stasis_app_send_command_async(control,
+	res = stasis_app_send_command(control,
 		app_control_add_channel_to_bridge, bridge);
+	return *res;
 }
 
 static void *app_control_remove_channel_from_bridge(
@@ -594,7 +599,7 @@ static void *app_control_remove_channel_from_bridge(
 	struct ast_bridge *bridge = data;
 
 	if (!control) {
-		return NULL;
+		return &FAIL;
 	}
 
 	/* We should only depart from our own bridge */
@@ -606,20 +611,22 @@ static void *app_control_remove_channel_from_bridge(
 		ast_log(LOG_WARNING, "%s: Not in bridge %s; not removing\n",
 			stasis_app_control_get_channel_id(control),
 			bridge->uniqueid);
-		return NULL;
+		return &FAIL;
 	}
 
 	ast_bridge_depart(chan);
-	return NULL;
+	return &OK;
 }
 
-void stasis_app_control_remove_channel_from_bridge(
+int stasis_app_control_remove_channel_from_bridge(
 	struct stasis_app_control *control, struct ast_bridge *bridge)
 {
+	int *res;
 	ast_debug(3, "%s: Sending channel remove_from_bridge command\n",
 			stasis_app_control_get_channel_id(control));
-	stasis_app_send_command_async(control,
+	res = stasis_app_send_command(control,
 		app_control_remove_channel_from_bridge, bridge);
+	return *res;
 }
 
 const char *stasis_app_control_get_channel_id(