<p> Attention is currently required from: Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18127">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/res_geolocation.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/+/18127/comment/3f6b4d92_e88383cd">Patch Set #3, Line 50:</a> <code style="font-family:monospace,monospace">         AST_STRING_FIELD(method);</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Are you expecting lots of additional fields here?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">No but "method" isn't really defined in the spec so I don't know how long it will be.  That's why I made it a string field.  Well, that and I couldn't remember if sorcery would take care of allocating and freeing a simple char *.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_geolocation/geoloc_eprofile.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/+/18127/comment/2d0bc03f_b6c71e73">Patch Set #3, Line 50:</a> <code style="font-family:monospace,monospace"></code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Add a comment that this is safe because stringfields handles NULL</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">wilco.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18127/comment/ccf24539_e3d323ec">Patch Set #3, Line 101:</a> <code style="font-family:monospace,monospace"> eprofile->location_refinement = profile->location_refinement ? ast_variables_dup(profile->location_refinement) : NULL;</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">If any of this duplication fails, won't the effective profile be left in a bit of an undefined/not correct state?</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think we should not allow things to get in that state.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Yeah true.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18127/comment/6e8ed657_2e8c45b8">Patch Set #3, Line 122:</a> <code style="font-family:monospace,monospace">    if (ast_strlen_zero(uri)) {</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Are we confident in this simple URI handling here?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I had thought about applying the same rules we're doing for stir/shaken but we're not ever actually doing anything with that uri. The uri scheme is also not just http or https.  It could be "sip", "sips", "pres" or any other scheme for that matter.   It's just an opaque string to us and god help us if we accidentally misidentified it as malicious and dropped it.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18127">change 18127</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/+/18127"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: development/16/geolocation </div>
<div style="display:none"> Gerrit-Change-Id: I50b66bd041b2a62ab329406f20dbaeef1fa68fc1 </div>
<div style="display:none"> Gerrit-Change-Number: 18127 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 09 Mar 2022 15:49:40 +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: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>