[Asterisk-code-review] res pjsip caller id: Fix segfault when replacing rpid or pai... (asterisk[certified/13.1])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_pjsip_caller_id: Fix segfault when replacing rpid or pai header
......................................................................


res_pjsip_caller_id: Fix segfault when replacing rpid or pai header

If the PJSIP_HEADER dialplan function adds a PAI or RPID header and send_rpid
or send_pai is set, res_pjsip_caller_id attemps to retrieve, parse and modify
the header added by the dialplan function.  Since the header added by the
dialplan function is generic string, there are no virtual functions to parse
the uri and we get a segfault when we try.  Since the modify, was really only
an overwrite, we now just delete the old header if it was type PJSIP_H_OTHER
and recreate it.

This raises a question for another time though:  What should happen with
duplicate headers?  Right now res_pjsip_header_funcs doesn't check for dups
so if it's session supplement is loaded after res_pjsip_caller_id's (or any
other module that adds headers), there'll be dups in the message.

ASTERISK-25337 #close

Change-Id: I5e296b52d30f106b822c0eb27c4c2b0e0f71c7fa
(cherry picked from commit 34c64707d1aa346fb0e9c7f97e375d22dedf67d9)
---
M res/res_pjsip_caller_id.c
1 file changed, 32 insertions(+), 6 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_pjsip_caller_id.c b/res/res_pjsip_caller_id.c
index 3f366bf..edaf37f 100644
--- a/res/res_pjsip_caller_id.c
+++ b/res/res_pjsip_caller_id.c
@@ -516,9 +516,22 @@
 	 */
 	old_pai = pjsip_msg_find_hdr_by_name(tdata->msg, &pj_pai_name, NULL);
 	if (old_pai) {
-		modify_id_header(tdata->pool, old_pai, id);
-		add_privacy_header(tdata, id);
-		return;
+		/* If type is OTHER, then the existing header was most likely
+		 * added by the PJSIP_HEADER dial plan function as a simple
+		 * name/value pair.  We can't pass this to modify_id_header because
+		 * there are no virtual functions to get the uri.  We could parse
+		 * it into a pjsip_fromto_hdr but it isn't worth it since
+		 * modify_id_header is just going to overwrite the name and number
+		 * anyway.  We'll just remove it from the header list instead
+		 * and create a new one.
+		 */
+		if (old_pai->type == PJSIP_H_OTHER) {
+			pj_list_erase(old_pai);
+		} else {
+			modify_id_header(tdata->pool, old_pai, id);
+			add_privacy_header(tdata, id);
+			return;
+		}
 	}
 
 	pai_hdr = create_new_id_hdr(&pj_pai_name, tdata, id);
@@ -610,9 +623,22 @@
 	 */
 	old_rpid = pjsip_msg_find_hdr_by_name(tdata->msg, &pj_rpid_name, NULL);
 	if (old_rpid) {
-		modify_id_header(tdata->pool, old_rpid, id);
-		add_privacy_params(tdata, old_rpid, id);
-		return;
+		/* If type is OTHER, then the existing header was most likely
+		 * added by the PJSIP_HEADER dial plan function as a simple
+		 * name/value pair.  We can't pass this to modify_id_header because
+		 * there are no virtual functions to get the uri.  We could parse
+		 * it into a pjsip_fromto_hdr but it isn't worth it since
+		 * modify_id_header is just going to overwrite the name and number
+		 * anyway.  We'll just remove it from the header list instead
+		 * and create a new one.
+		 */
+		if (old_rpid->type == PJSIP_H_OTHER) {
+			pj_list_erase(old_rpid);
+		} else {
+			modify_id_header(tdata->pool, old_rpid, id);
+			add_privacy_params(tdata, old_rpid, id);
+			return;
+		}
 	}
 
 	rpid_hdr = create_new_id_hdr(&pj_rpid_name, tdata, id);

-- 
To view, visit https://gerrit.asterisk.org/2759
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5e296b52d30f106b822c0eb27c4c2b0e0f71c7fa
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>