nathelper: fix Contact concatenation when fix_nated_contact() runs after topology_hiding() (GH #2172)

When topology_hiding() is called before fix_nated_contact() and the
Contact header lacks angle-bracket enclosure (<>), both functions
create LUMP_DEL entries at the same buffer offset.  The message
builder's last_del exception prevents the second lump from being
skipped, causing both replacement values to be concatenated into
a single malformed Contact header.

With angle brackets the offsets differ by 1 byte (body.s vs uri.s)
so the message builder correctly suppresses the second lump.  Without
angle brackets body.s == uri.s, producing identical offsets and
triggering the bug.

Add a helper function contact_already_rewritten() that scans the
lump list for a prior LUMP_DEL covering the Contact URI.  The new
"nated_contact_override" module parameter controls behavior:

  0 (default) - skip: let the prior rewrite stand
  1           - override: neutralize the prior lump and let
                fix_nated_contact create its own replacement

Both modes prevent the concatenation bug.  The check is per-Contact
using continue, so multi-Contact messages still process uncovered
contacts normally.  The lump scan exits early since the list is
sorted by offset.

Closes: #2172

Author: NormB

modules/nathelper/doc/nathelper_admin.xml

@@ -584,6 +584,80 @@ modparam("nathelper", "cluster_id", 9)
 modparam("nathelper", "cluster_id", 9)
 modparam("nathelper", "cluster_sharing_tag", "vip")
 ...
+</programlisting>
+		</example>
+	</section>
+
+	<section id="param_nated_contact_override" xreflabel="nated_contact_override">
+		<title><varname>nated_contact_override</varname> (integer)</title>
+		<para>
+		Controls the behavior of
+		<xref linkend="func_fix_nated_contact"/> when the Contact header
+		has already been rewritten by a prior function in the same route,
+		such as <function>topology_hiding()</function>.
+		</para>
+		<para>
+		When both <function>topology_hiding()</function> and
+		<function>fix_nated_contact()</function> are used in the same
+		routing script, both attempt to rewrite the Contact header. If
+		<function>topology_hiding()</function> runs first, the Contact
+		is already scheduled for replacement by the time
+		<function>fix_nated_contact()</function> executes. Without this
+		parameter, calling them in this order can produce a malformed
+		Contact header containing two concatenated SIP URIs (see
+		<ulink url="https://github.com/OpenSIPS/opensips/issues/2172">
+		GH #2172</ulink>).
+		</para>
+		<para>
+		This parameter lets you choose which rewrite wins:
+		</para>
+		<itemizedlist>
+			<listitem><para>
+			<emphasis>0</emphasis> (default) - keep the existing
+			Contact rewrite as-is. For example, if
+			<function>topology_hiding()</function> already rewrote
+			the Contact, its topology-hidden Contact will be
+			preserved and <function>fix_nated_contact()</function>
+			will be silently skipped for that Contact.
+			</para></listitem>
+			<listitem><para>
+			<emphasis>1</emphasis> - override the existing rewrite
+			with the NAT-fixed Contact from
+			<function>fix_nated_contact()</function>. The Contact
+			will contain the source IP and port of the request
+			instead of the topology-hidden address.
+			</para></listitem>
+		</itemizedlist>
+		<note><para>
+		This parameter only takes effect when a Contact conflict is
+		detected (i.e., another function has already scheduled the
+		Contact for rewriting). When
+		<function>fix_nated_contact()</function> is called on its own
+		or before other Contact-rewriting functions, this parameter
+		has no effect and <function>fix_nated_contact()</function>
+		behaves normally.
+		</para></note>
+		<para>
+		If the message contains multiple Contact URIs, the check is
+		done per-Contact: only Contacts that conflict with a prior
+		rewrite are affected by this setting. Other Contacts are
+		processed by <function>fix_nated_contact()</function> normally.
+		</para>
+		<para>
+		<emphasis>
+			Default value is 0 (keep existing rewrite).
+		</emphasis>
+		</para>
+		<example>
+		<title>Set <varname>nated_contact_override</varname> parameter</title>
+		<programlisting format="linespecific">
+...
+# Keep topology_hiding()'s Contact (default behavior)
+modparam("nathelper", "nated_contact_override", 0)
+...
+# Override with fix_nated_contact()'s NAT-fixed Contact
+modparam("nathelper", "nated_contact_override", 1)
+...
 </programlisting>
 		</example>
 	</section>

modules/nathelper/nathelper.c

@@ -217,6 +217,14 @@ static unsigned int raw_ip = 0;
 static unsigned short raw_port = 0;
 int skip_oldip=0;
 
+/*
+ * Controls fix_nated_contact() when the Contact has already been rewritten
+ * by a prior function (e.g. topology_hiding()):
+ *   0 (default) = keep the existing Contact rewrite as-is
+ *   1           = override it with fix_nated_contact()'s NAT replacement
+ */
+static int nated_contact_override = 0;
+
 /*0-> disabled, 1 ->enabled*/
 unsigned int *natping_state=0;
 
@@ -261,6 +269,7 @@ static const param_export_t params[] = {
 	{"max_pings_lost",		     INT_PARAM, &max_pings_lost        },
 	{"cluster_id",               INT_PARAM, &nh_cluster_id         },
 	{"cluster_sharing_tag",      STR_PARAM, &nh_cluster_shtag      },
+	{"nated_contact_override",  INT_PARAM, &nated_contact_override},
 	{0, 0, 0}
 };
 
@@ -625,6 +634,49 @@ isnulladdr(str *sx, int pf)
 	return (sx->len == 7 && memcmp("0.0.0.0", sx->s, 7) == 0);
 }
 
+/*
+ * Check if a prior lump already covers this Contact URI (GH #2172).
+ * Returns 1 if fix_nated_contact should skip this contact, 0 to proceed.
+ */
+static int contact_already_rewritten(struct sip_msg *msg, contact_t *c)
+{
+	struct lump *crt;
+	unsigned int uri_off = c->uri.s - msg->buf;
+	unsigned int uri_end = uri_off + c->uri.len;
+
+	for (crt = msg->add_rm; crt; crt = crt->next) {
+		if (crt->type != HDR_CONTACT_T || crt->op != LUMP_DEL)
+			continue;
+		/* lumps are sorted by offset -- no further match possible */
+		if (crt->u.offset > uri_off)
+			break;
+		if (crt->u.offset + crt->len < uri_end)
+			continue;
+
+		/* prior lump fully covers this Contact URI */
+		if (nated_contact_override == 0) {
+			LM_DBG("Contact already rewritten at off=%u len=%u, "
+				"keeping existing (nated_contact_override=0)\n",
+				crt->u.offset, crt->len);
+			return 1;
+		}
+
+		/* override: neutralize the prior lump (same technique as
+		 * delete_existing_contact in topo_hiding_logic.c) */
+		LM_DBG("Contact already rewritten at off=%u len=%u, "
+			"overriding (nated_contact_override=1)\n",
+			crt->u.offset, crt->len);
+		crt->op = LUMP_NOP;
+		if (crt->after)
+			insert_cond_lump_after(crt, COND_FALSE, 0);
+		if (crt->before)
+			insert_cond_lump_before(crt, COND_FALSE, 0);
+		return 0;
+	}
+
+	return 0;
+}
+
 /*
  * Replaces ip:port pair in the Contact: field with the source address
  * of the packet.
@@ -653,6 +705,11 @@ fix_nated_contact_f(struct sip_msg* msg, str *params)
 			return -1;
 		}
 
+		/* GH #2172: skip if Contact already rewritten by e.g.
+		 * topology_hiding(); override instead if configured */
+		if (contact_already_rewritten(msg, c))
+			continue;
+
 		hostport = uri.host;
 		if (uri.port.len > 0)
 			hostport.len = uri.port.s + uri.port.len - uri.host.s;