<p> Attention is currently required from: George Joseph. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18830">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18830?tab=comments">Patch Set #1:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Go ahead and cherry-pick but really need a testsuite test for this.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p><p style="white-space: pre-wrap; word-wrap: break-word;">A major problem is the difference between a SIP URI in pjproject and the fromto_hdr struct.</p><p style="white-space: pre-wrap; word-wrap: break-word;">https://www.pjsip.org/pjsip/docs/html/structpjsip__fromto__hdr.htm<br>https://www.pjsip.org/pjsip/docs/html/structpjsip__sip__uri.htm</p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p><p style="white-space: pre-wrap; word-wrap: break-word;">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).</p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p><p style="white-space: pre-wrap; word-wrap: break-word;">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...</p><p style="white-space: pre-wrap; word-wrap: break-word;">(I was going to add header parameters in the above review as well, but that part of pjproject just flat out does not work).</p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">char buf[256];<br>pjsip_sip_uri *uri;<br>pj_str_t event_hdr_name = pj_str("From");<br>pjsip_generic_string_hdr *event_hdr = (pjsip_generic_string_hdr*)pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &event_hdr_name, NULL);<br>if (event_hdr) {<br> ast_log(LOG_NOTICE, "Full hdr: %s\n", event_hdr->hvalue.ptr);<br>}<br>ast_log(LOG_NOTICE, "No isup-oli in other_param\n");<br>ast_log(LOG_NOTICE, "raw other_param: %s\n", from->other_param.value.ptr);<br>ast_log(LOG_NOTICE, "Tag: %s\n", from->tag.ptr);<br>uri = (pjsip_sip_uri*) pjsip_uri_get_uri(rdata->msg_info.msg->line.req.uri);<br>if (uri) {<br> pjsip_param *param;<br> pj_str_t pname = pj_str("isup-oli");<br> param = pjsip_param_find(&uri->other_param, &pname);<br> if (param)<br> ast_log(LOG_NOTICE, "Param: %s\n", param->value.ptr);<br> else<br> ast_log(LOG_NOTICE, "No param\n");<br> ast_log(LOG_NOTICE, "raw other_param: %s\n", uri->other_param.value.ptr);<br> ast_log(LOG_NOTICE, "raw header_param: %s\n", uri->header_param.value.ptr);<br>} else {<br> ast_log(LOG_NOTICE, "No uri\n");<br> //from->uri->vptr->p_get_uri(&uri);<br> //ast_log(LOG_NOTICE, "OTHER: %s\n", uri->other_param.value.ptr);<br> //ast_log(LOG_NOTICE, "HDR: %s\n", uri->header_param.value.ptr);<br> from->vptr->print_on(from, buf, 256);<br> ast_log(LOG_NOTICE, "Full: %s\n", buf);<br>}</pre><p style="white-space: pre-wrap; word-wrap: break-word;"><br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: Full hdr: ��6�U^?<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: No isup-oli in other_param<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: raw other_param: (null)<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: Tag: 01267d1d-ea24-4297-84f2-9dfe64d22467<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: No param<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: raw other_param: (null)<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: raw header_param: (null)<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: Full: From: <sip:5552368@127.0.0.1;isup-oli=27>;tag=01267d1d-ea24-4297-84f2-9dfe64d22467.�U^?<br>[Jul 22 16:11:34] NOTICE[2103466] res_pjsip_caller_id.c: No ani2 isup-oli</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18830">change 18830</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18830"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ifb1bc3c512ad5f6faeaebd7817f004a2ecbd6428 </div>
<div style="display:none"> Gerrit-Change-Number: 18830 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 22 Jul 2022 20:20:32 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>