From 3c4bbbf2ff2c8bc4de818d74f0e9ac99d945d3c8 Mon Sep 17 00:00:00 2001
From: Bogdan-Andrei Iancu <bogdan@opensips.org>
Date: Thu, 27 Sep 2018 19:27:54 +0300
Subject: [PATCH] Fix forced dependency on RR module

* loading of the RR module is optional
* change the default matching mode to Dialog-ID Fallback

Fixes #1468
---
 modules/dialog/dialog.c             | 30 ++++++++++++++---------------
 modules/dialog/dlg_handlers.c       |  8 +++++---
 modules/dialog/doc/dialog_admin.xml | 15 ++++++++++++---
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/modules/dialog/dialog.c b/modules/dialog/dialog.c
index a9a9c14e573..0c038dab986 100644
--- a/modules/dialog/dialog.c
+++ b/modules/dialog/dialog.c
@@ -84,7 +84,7 @@ static char* profiles_wv_s = NULL;
 static char* profiles_nv_s = NULL;
 
 int dlg_bulk_del_no = 1; /* delete one by one */
-int seq_match_mode = SEQ_MATCH_STRICT_ID;
+int seq_match_mode = SEQ_MATCH_FALLBACK;
 int options_ping_interval = 30;      /* seconds */
 int reinvite_ping_interval = 300;    /* seconds */
 str dlg_extra_hdrs = {NULL,0};
@@ -381,7 +381,6 @@ static module_dependency_t *get_deps_db_mode(param_export_t *param)
 static dep_export_t deps = {
 	{ /* OpenSIPS module dependencies */
 		{ MOD_TYPE_DEFAULT, "tm", DEP_ABORT },
-		{ MOD_TYPE_DEFAULT, "rr", DEP_ABORT },
 		{ MOD_TYPE_NULL, NULL, 0 },
 	},
 	{ /* modparam dependencies */
@@ -903,19 +902,12 @@ static int mod_init(void)
 		return -1;
 	}
 
-
 	/* load the TM API */
 	if (load_tm_api(&d_tmb)!=0) {
 		LM_ERR("can't load TM API\n");
 		return -1;
 	}
 
-	/* load RR API also */
-	if (load_rr_api(&d_rrb)!=0) {
-		LM_ERR("can't load RR API\n");
-		return -1;
-	}
-
 	/* register callbacks*/
 	/* listen for all incoming requests  */
 	if ( d_tmb.register_tmcb( 0, 0, TMCB_REQUEST_IN, dlg_onreq, 0, 0 ) <=0 ) {
@@ -923,18 +915,26 @@ static int mod_init(void)
 		return -1;
 	}
 
-	/* listen for all routed requests  */
-	if ( d_rrb.register_rrcb( dlg_onroute, 0, 1 ) <0 ) {
-		LM_ERR("cannot register RR callback\n");
-		return -1;
+	/* load RR API */
+	if (load_rr_api(&d_rrb)!=0) {
+		/* make it null to use it as marker for "RR not loaded" */
+		memset( &d_rrb, 0, sizeof(d_rrb));
+	} else {
+		/* listen for all routed requests  */
+		if ( d_rrb.register_rrcb( dlg_onroute, 0, 1 ) <0 ) {
+			LM_ERR("cannot register RR callback\n");
+			return -1;
+		}
 	}
 
-	if (register_script_cb( dialog_cleanup, POST_SCRIPT_CB|REQ_TYPE_CB|RPL_TYPE_CB,0)<0) {
+	if (register_script_cb( dialog_cleanup,
+	POST_SCRIPT_CB|REQ_TYPE_CB|RPL_TYPE_CB,0)<0) {
 		LM_ERR("cannot register script callback");
 		return -1;
 	}
 
-	/* check params and register to clusterer for dialogs and profiles replication */
+	/* check params and register to clusterer for dialogs and
+	 * profiles replication */
 	if (dialog_repl_cluster < 0) {
 		LM_ERR("Invalid dialog_replication_cluster, must be 0 or "
 			"a positive cluster id\n");
diff --git a/modules/dialog/dlg_handlers.c b/modules/dialog/dlg_handlers.c
index 8bc0c7a0486..7a92d5aec78 100644
--- a/modules/dialog/dlg_handlers.c
+++ b/modules/dialog/dlg_handlers.c
@@ -77,6 +77,7 @@ static int       shutdown_done = 0;
 
 extern int       seq_match_mode;
 extern struct rr_binds d_rrb;
+#define has_rr() (d_rrb.add_rr_param!=NULL)
 extern int race_condition_timeout;
 
 /* statistic variables */
@@ -1362,7 +1363,7 @@ int dlg_create_dialog(struct cell* t, struct sip_msg *req,unsigned int flags)
 		extra_ref++; /* extra ref for the timer to delete the dialog */
 	link_dlg( dlg , extra_ref);
 
-	if ( seq_match_mode!=SEQ_MATCH_NO_ID &&
+	if ( seq_match_mode!=SEQ_MATCH_NO_ID && has_rr() &&
 		add_dlg_rr_param( req, dlg->h_entry, dlg->h_id)<0 ) {
 		LM_ERR("failed to add RR param\n");
 		goto error;
@@ -1553,7 +1554,8 @@ void dlg_onroute(struct sip_msg* req, str *route_params, void *param)
 		val = *((str *)param);
 
 	if ( seq_match_mode!=SEQ_MATCH_NO_ID ) {
-		if( val.s == NULL && d_rrb.get_route_param( req, &rr_param, &val)!=0) {
+		if( val.s == NULL &&
+		(!has_rr() || d_rrb.get_route_param( req, &rr_param, &val)!=0) ) {
 			LM_DBG("Route param '%.*s' not found\n", rr_param.len,rr_param.s);
 			if (seq_match_mode==SEQ_MATCH_STRICT_ID )
 				return;
@@ -2396,7 +2398,7 @@ int dlg_validate_dialog( struct sip_msg* req, struct dlg_cell *dlg)
 
 	/* because fix_routing was called on the request */
 
-	if(dlg->mod_flags & TOPOH_ONGOING)
+	if ((dlg->mod_flags & TOPOH_ONGOING) || !has_rr())
 		return 0;
 
 	if (dlg->state <= DLG_STATE_EARLY)
diff --git a/modules/dialog/doc/dialog_admin.xml b/modules/dialog/doc/dialog_admin.xml
index e9334787206..e2921214a86 100644
--- a/modules/dialog/doc/dialog_admin.xml
+++ b/modules/dialog/doc/dialog_admin.xml
@@ -171,7 +171,8 @@
 			</listitem>
 			<listitem>
 			<para>
-				<emphasis>RR</emphasis> - Record-Route module
+				<emphasis>RR</emphasis> - Record-Route module, optional, 
+				if Dialog ID matching is used in non Topo Hiding cases
 			</para>
 			</listitem>
 			<listitem>
@@ -371,14 +372,22 @@ modparam("dialog", "dlg_extra_hdrs", "Hint: credit expired\r\n")
 		</itemizedlist>
 		<para>
 		<emphasis>
-			Default value is <quote>0 (DID_ONLY)</quote>.
+			Default value is <quote>1 (DID_FALLBACK)</quote>.
 		</emphasis>
 		</para>
+		<para>
+		NOTE that if you have call looping on your OpenSIPS server (passing
+		more than once through the same OpenSIPS instance), it is strongly
+		suggested to use only DID_ONLY mode, as the SIP based matching will
+		have an undefined behavior - from SIP perspective, a sequential
+		dialog will match all the loops of the call, as the Call-ID, To and 
+		From TAGs are the same.
+		</para>
 		<example>
 		<title>Set <varname>dlg_match_mode</varname> parameter</title>
 		<programlisting format="linespecific">
 ...
-modparam("dialog", "dlg_match_mode", 1)
+modparam("dialog", "dlg_match_mode", 0)
 ...
 </programlisting>
 		</example>