<p> Attention is currently required from: Michael Bradeen, Joshua Colp, George Joseph. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18892">View Change</a></p><p>10 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File configs/samples/pjsip.conf.sample:</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/+/18892/comment/4b8fc2af_89cb5ae5">Patch Set #4, Line 1511:</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;">; You can match a TEL URI to an endpoint using a config object of type<br>; "identify". Set the endpoint you want it to be associated with, and match<br>; based on IP. For example:<br>;<br>; [tel-uri-identify]<br>; type=identify<br>; endpoint=my-endpoint<br>; match=127.0.0.1:5061<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This is a bit "strange" since match-by-ip applies to anything whether it has a tel uri or not. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/res_pjsip.h:</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/+/18892/comment/c7a21817_48890bb3">Patch Set #4, Line 3501:</a> <code style="font-family:monospace,monospace">User string or empty string if not present</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Do we really need to return an empty pj_str on error or can we just return NULL. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It could go either way. Having it return an empty string could be nice for situations where we don't care what is returned (empty or an actual string). Just removes the need to do a NULL check for that specific case.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip.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/+/18892/comment/dfc2ede9_3e93df62">Patch Set #4, Line 2372:</a> <code style="font-family:monospace,monospace">const pj_str_t *ast_sip_pjsip_uri_get_username(pjsip_uri *uri)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Do we really need to return an empty pj_str on error or can we just return NULL. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip/pjsip_message_filter.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/+/18892/comment/dafd2502_f4182056">Patch Set #4, Line 491:</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;">!ast_sip_is_uri_sip_sips(rdata->msg_info.msg->line.req.uri)<br> && <br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I don't think you need this any more do you? Just test for TEL?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">We still want to make sure it's SIP, SIPS, or TEL and nothing else. I think this is still needed, but I could be wrong.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_dialog_info_body_generator.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/+/18892/comment/687aaa5b_1c988cb7">Patch Set #4, Line 193:</a> <code style="font-family:monospace,monospace"> ast_copy_pj_str(dlg_remote_uri, ast_sip_pjsip_uri_get_hostname(dlg->local.info->uri), sizeof(dlg_remote_uri));</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So dlg_remote-uri is going to be empty if this is a tel-uri. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Maybe it would be better to bypass this conditional if it's a TEL URI?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_endpoint_identifier_anonymous.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/+/18892/comment/ed8775b4_83c12f11">Patch Set #4, Line 39:</a> <code style="font-family:monospace,monospace"> ast_copy_pj_str(domain, ast_sip_pjsip_uri_get_hostname(from), domain_size);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same as before. For a tel-uri the domain will be empty which will produce invalid uris below.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_endpoint_identifier_user.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/+/18892/comment/deb97101_9d791df8">Patch Set #4, Line 40:</a> <code style="font-family:monospace,monospace"> ast_copy_pj_str(username, ast_sip_pjsip_uri_get_username(from), username_size);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I thought we weren't going to allow an identify-by username for a tel uri?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ah yeah, can change the scheme check back.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18892/comment/6bf40ee5_cc696ae1">Patch Set #4, Line 41:</a> <code style="font-family:monospace,monospace"> ast_copy_pj_str(domain, ast_sip_pjsip_uri_get_hostname(from), domain_size);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same as before. An empty domain will produce invalid uris below.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_nat.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/+/18892/comment/1e87ee19_4247f64e">Patch Set #4, Line 115:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Just a whitespace change?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I originally had a comment there for myself and removed it when cleaning up the patch. Guess it cleaned up some whitespace without me realizing it :)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_pjsip_path.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/+/18892/comment/7d07ae00_3329400d">Patch Set #4, Line 53:</a> <code style="font-family:monospace,monospace"> domain_name = ast_alloca(uri_hostname->slen + 1);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same as before. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Maybe a URI scheme check is needed?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18892">change 18892</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/+/18892"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: If5729e6cd583be7acf666373bf9f1b9d653ec29a </div>
<div style="display:none"> Gerrit-Change-Number: 18892 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.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: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-CC: Alexei Gradinari <alex2grad@gmail.com> </div>
<div style="display:none"> Gerrit-Attention: Michael Bradeen <mbradeen@sangoma.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: Wed, 24 Aug 2022 15:29:09 +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>