[Asterisk-code-review] mwi_subscribe_replaces_unsolicited: (...asterisk[16])

[Christian Savinovich]

Christian Savinovich has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/11627


Change subject: mwi_subscribe_replaces_unsolicited:
......................................................................

mwi_subscribe_replaces_unsolicited:

Fixes an bug whereby multiple NOTIFYs were replied back to endpoint when a
REGISTER was received.  This occurred when parameter
mwi_subscribe_replaces_unsolicited is set to yes in pjsip.conf.
The underlying problem here is that the
endpoint_receives_unsolicited_mwi_for_mailbox() function in res/res_pjsip_mwi.c
is sort of an overloaded function. When invoked from mwi_validate_for_aor() you
DO want to have the subscribe_replaces_unsolicited() logic occur whereby it
unsubscribes from stasis. When invoked from
create_mwi_subscriptions_for_endpoint you DO NOT want to have the
subscribe_replaces_unsolicited logic occur.The function should have a parameter
added to it which determines whether the logic is invoked or not.
The bug itself occurs because there are multiple mwi_subscription structures
for the mailbox, the logic which sends the NOTIFY when a REGISTER occurs goes
through all looking for mwi_subscription structures with the AOR. It finds
these multiple structures, calculates message count on them (despite having
no stasis subscriptions), and sends a NOTIFY.
Note: This fix does not restore back unsolicted NOTIFYs in the event
all subscriptions on a given AOR go away.  It was decided to create
a new issue for that.

Change-Id: I8f77c33b9b4d374d510aa5ecd4f700a77107d8d4
---
M res/res_pjsip_mwi.c
1 file changed, 31 insertions(+), 12 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/27/11627/1

diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index abd7ac0..c5e7963 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -119,6 +119,10 @@
 	char *aors;
 	/*! Is the MWI solicited (i.e. Initiated with an external SUBSCRIBE) ? */
 	unsigned int is_solicited;
+	/*! identifies if user wants to mute unsolicited NOTIFYs if subscription
+	*  exists.
+	*/
+	unsigned int is_subscribe_replaces_unsolicited;
 	/*! Identifier for the subscription.
 	 * The identifier is the same as the corresponding endpoint's stasis ID.
 	 * Used as a hash key
@@ -498,7 +502,6 @@
 		.body_type = AST_SIP_MESSAGE_ACCUMULATOR,
 		.body_data = mwi_data->counter,
 	};
-
 	if (ast_sip_create_request("NOTIFY", NULL, endpoint, NULL, contact, &tdata)) {
 		ast_log(LOG_WARNING, "Unable to create unsolicited NOTIFY request to endpoint %s URI %s\n", sub->id, contact->uri);
 		return 0;
@@ -677,7 +680,15 @@
 		return;
 	}
 
+	if (sub->is_subscribe_replaces_unsolicited) {
+		struct ast_sip_endpoint *endpoint = ast_sip_subscription_get_endpoint(sub->sip_sub);
+		if (endpoint->subscription.mwi.subscribe_replaces_unsolicited) {
+			return;
+		}
+	}
+
 	send_unsolicited_mwi_notify(sub, &counter);
+
 }
 
 static int unsubscribe_stasis(void *obj, void *arg, int flags)
@@ -710,7 +721,6 @@
 static void mwi_ds_destroy(void *data)
 {
 	struct mwi_subscription *sub = data;
-
 	ao2_ref(sub, -1);
 }
 
@@ -744,11 +754,12 @@
  *
  * \param endpoint The endpoint to check
  * \param mailbox The candidate mailbox
+ * \param invokedByValidate 1 or 0 if called from mwi_validate_for_aor
  * \retval 0 The endpoint does not receive unsolicited MWI for this mailbox
  * \retval 1 The endpoint receives unsolicited MWI for this mailbox
  */
-static int endpoint_receives_unsolicited_mwi_for_mailbox(struct ast_sip_endpoint *endpoint,
-		const char *mailbox)
+static int endpoint_receives_unsolicited_mwi_for_mailbox(struct ast_sip_endpoint *endpoint, 
+		const char *mailbox, int invokedByValidate)
 {
 	struct ao2_iterator *mwi_subs;
 	struct mwi_subscription *mwi_sub;
@@ -765,13 +776,19 @@
 
 		mwi_stasis = ao2_find(mwi_sub->stasis_subs, mailbox, OBJ_SEARCH_KEY);
 		if (mwi_stasis) {
-			if (endpoint->subscription.mwi.subscribe_replaces_unsolicited) {
-				unsubscribe_stasis(mwi_stasis, NULL, 0);
-				ao2_unlink(mwi_sub->stasis_subs, mwi_stasis);
-			} else {
+			if (invokedByValidate) {
+				if (endpoint->subscription.mwi.subscribe_replaces_unsolicited) {
+					mwi_sub->is_subscribe_replaces_unsolicited = 1;
+					unsubscribe_stasis(mwi_stasis, NULL, 0);
+					ao2_unlink(mwi_sub->stasis_subs, mwi_stasis);
+				} else {
+					ret = 1;
+				}
+				ao2_cleanup(mwi_stasis);
+			}
+			else {
 				ret = 1;
 			}
-			ao2_cleanup(mwi_stasis);
 		}
 	}
 
@@ -808,7 +825,8 @@
 			continue;
 		}
 
-		if (endpoint_receives_unsolicited_mwi_for_mailbox(endpoint, mailbox)) {
+		/* parameter 3 = 1 means "invoked-from-function-mwi-validate-for-aor" */
+		if (endpoint_receives_unsolicited_mwi_for_mailbox(endpoint, mailbox, 1)) {
 			ast_debug(1, "Endpoint '%s' already configured for unsolicited MWI for mailbox '%s'. "
 					"Denying MWI subscription to %s\n", ast_sorcery_object_get_id(endpoint), mailbox,
 					ast_sorcery_object_get_id(aor));
@@ -1021,7 +1039,7 @@
 }
 
 static void mwi_to_ami(struct ast_sip_subscription *sub,
-		       struct ast_str **buf)
+		struct ast_str **buf)
 {
 	struct mwi_subscription *mwi_sub;
 	struct ast_datastore *mwi_datastore;
@@ -1127,8 +1145,9 @@
 		struct mwi_stasis_subscription *mwi_stasis_sub;
 
 		/* check if subscription exists */
+		/* parameter 3 = 0 means "not-invoked-from-function-mwi-validate-for-aor" */
 		if (ast_strlen_zero(mailbox) ||
-			(!aggregate_sub && endpoint_receives_unsolicited_mwi_for_mailbox(endpoint, mailbox))) {
+			(!aggregate_sub && endpoint_receives_unsolicited_mwi_for_mailbox(endpoint, mailbox, 0))) {
 			continue;
 		}
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11627
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I8f77c33b9b4d374d510aa5ecd4f700a77107d8d4
Gerrit-Change-Number: 11627
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Savinovich <csavinovich at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190725/92a0e5e8/attachment-0001.html>