[Asterisk-code-review] res_pjsip_session: Add custom parameter support. (asterisk[master])

[N A]

Attention is currently required from: George Joseph.
N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18830 )

Change subject: res_pjsip_session: Add custom parameter support.
......................................................................


Patch Set 1:

(1 comment)

Patchset:

PS1: 
> Go ahead and cherry-pick but really need a testsuite test for this.
I've spent several hours trying to make a test suite for this, and though I have the mechanics, I'm struggling trying to get a meaningful test with the way the pjproject library is written.

A major problem is the difference between a SIP URI in pjproject and the fromto_hdr struct.

https://www.pjsip.org/pjsip/docs/html/structpjsip__fromto__hdr.htm
https://www.pjsip.org/pjsip/docs/html/structpjsip__sip__uri.htm

The fromto_header has a other_param. The sip_uri has both other_param and header_param. It's unclear how you get the header_param from a fromto_header.

Furthermore, in trying to debug, pjproject always returns NULL for the parameters even though it's right there in the URI (printed out near the bottom).

There may be a bug in pjproject, but maybe somebody more seasoned with the library can see something I don't - this is like walking on toothpicks for me.

On the Asterisk side, this could probably be fixed by doing it the chan_sip way, screw pjproject and just get the entire header as a string and manually parse it and do it ourselves. That aside, I don't see a straightforward way forward to writing an end to end test for this at this time, and pjproject is anything but straightforward with parameters...

(I was going to add header parameters in the above review as well, but that part of pjproject just flat out does not work).

TL;DR - the added functionality in the review works, and I would like to add a test but this may be a larger issue that needs to be figured out in terms of how to work with the parameters on the pjproject side, and if something is broken there. That seems likely, given how there are issues on each side now.



char buf[256];
pjsip_sip_uri *uri;
pj_str_t event_hdr_name = pj_str("From");
pjsip_generic_string_hdr *event_hdr = (pjsip_generic_string_hdr*)pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &event_hdr_name, NULL);
if (event_hdr) {
	ast_log(LOG_NOTICE, "Full hdr: %s\n", event_hdr->hvalue.ptr);
}
ast_log(LOG_NOTICE, "No isup-oli in other_param\n");
ast_log(LOG_NOTICE, "raw other_param: %s\n", from->other_param.value.ptr);
ast_log(LOG_NOTICE, "Tag: %s\n", from->tag.ptr);
uri = (pjsip_sip_uri*) pjsip_uri_get_uri(rdata->msg_info.msg->line.req.uri);
if (uri) {
	pjsip_param *param;
	pj_str_t pname = pj_str("isup-oli");
	param = pjsip_param_find(&uri->other_param, &pname);
	if (param)
			ast_log(LOG_NOTICE, "Param: %s\n", param->value.ptr);
	else
			ast_log(LOG_NOTICE, "No param\n");
	ast_log(LOG_NOTICE, "raw other_param: %s\n", uri->other_param.value.ptr);
	ast_log(LOG_NOTICE, "raw header_param: %s\n", uri->header_param.value.ptr);
} else {
	ast_log(LOG_NOTICE, "No uri\n");
	//from->uri->vptr->p_get_uri(&uri);
	//ast_log(LOG_NOTICE, "OTHER: %s\n", uri->other_param.value.ptr);
	//ast_log(LOG_NOTICE, "HDR: %s\n", uri->header_param.value.ptr);
	from->vptr->print_on(from, buf, 256);
	ast_log(LOG_NOTICE, "Full: %s\n", buf);
}


[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: Full hdr: ��6�U^?
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: No isup-oli in other_param
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: raw other_param: (null)
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: Tag: 01267d1d-ea24-4297-84f2-9dfe64d22467
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: No param
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: raw other_param: (null)
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: raw header_param: (null)
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: Full: From: <sip:5552368 at 127.0.0.1;isup-oli=27>;tag=01267d1d-ea24-4297-84f2-9dfe64d22467.�U^?
[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: No ani2 isup-oli



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Ifb1bc3c512ad5f6faeaebd7817f004a2ecbd6428
Gerrit-Change-Number: 18830
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Fri, 22 Jul 2022 20:20:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220722/ae8971ea/attachment.html>