<p> Attention is currently required from: Sean Bright, Joshua Colp, George Joseph. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18830">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_session.c:</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/comment/d8888b11_753798e2">Patch Set #1, Line 1703:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">     /* Set URI (inside the <>, as opposed to header (outside the <>)) parameters, if requested.<br>        * This is often necessary for SS7 trunking over SIP. Most carriers require<br>    * that added parameters be URI parameters (as opposed to header parameters).<br>  *<br>     * other_param is how to add custom URI parameters to the header<br>       * XXX: header_param is for header parameters, but doesn't seem to work at the moment.<br>     *<br>     * Especially when dealing with SS7 or TDM elements over SIP,<br>  * injecting parameters into the user and/or method parameters<br>         * is required.<br>        *<br>     * For example, if wanted to set the isup-oli parameter to the channel's ANI2 value:<br>       * e.g. pjsip_param_add(dlg_pool, &dlg_info_uri->other_param, "isup-oli", ani2);<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I can trim this down as suggested.</p><p style="white-space: pre-wrap; word-wrap: break-word;">However, to address your comments:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Both of these would be "inside the <>". If they are not "inside the <>" then they are not URI parameters, so all of this just serves to confuse.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Some carriers require parameters inside the <> (URI parameters), and some require them outside the <> (header parameters).</p><p style="white-space: pre-wrap; word-wrap: break-word;">Ideally, both should be settable; pjproject doesn't seem to set header parameters at the moment properly so this just adds URI parameter setting support.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">The 'XXX: header_param is...' comment is irrelevant since you are setting URI parameters and not URI headers.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">That was a note about the above, that at this time only support for URI parameters is added, not header parameters. (header parameters, not headers, or URI headers)</p><p style="white-space: pre-wrap; word-wrap: break-word;"><br>XXXXX: I'm possibly getting method and header parameter terminology mixed up, I'll have to double check that but that might explain some of the confusion on my part. URI parameter is inside the <> and one of these other 2 is the part after that.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18830/comment/8481624b_5759f045">Patch Set #1, Line 1731:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">      while ((param_value = strsep(&params, ","))) {<br>                  param_name = strsep(&param_value, "=");<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">How do I add a parameter where the value contains a comma or equals sign? For example I want to add: […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Parameter names wouldn't contain equal signs, so I don't think the strsep on = is an issue.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If the value contained a comma, that could be an issue. I'll have to think about this - do you have a suggestion?</p><p style="white-space: pre-wrap; word-wrap: break-word;">One thought is perhaps require the value to be quoted in that case, as I think there is an ast_strsep function that would handle it properly in that case.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18830/comment/1d064c5e_a93bba8d">Patch Set #1, Line 1734:</a> <code style="font-family:monospace,monospace">                 pjsip_param_add(dlg_pool, &dlg_info_uri->other_param, param_name, S_OR(param_value, ""));</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">When setting the transport, user, method, ttl, maddr, or lr parameters wouldn't it be better to use  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Actually I think Asterisk already handles certain things such as the transport parameter and a couple others natively (transport for sure, since I saw that bit somewhere while working on this).</p><p style="white-space: pre-wrap; word-wrap: break-word;">The goal of this is to handle arbitrary custom parameters which neither Asterisk nor pjproject would be expected to be aware of. So the user shouldn't add transport in this manner, but use the native method instead.</p><p style="white-space: pre-wrap; word-wrap: break-word;">So I think we could either:<br>1) Detect those parameters and set the special fields instead.<br>2) Detect those parameters and reject setting them, to avoid conflict with the other mechanisms of setting them<br>3) Leave as is</p><p style="white-space: pre-wrap; word-wrap: break-word;">Generally speaking, this functionality is not for average users, it is more for advanced usage like those interfacing directly with carriers or SS7 networks.</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-Reviewer: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: 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: Mon, 25 Jul 2022 16:05:57 +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: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>