<p> Attention is currently required from: Sean Bright. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/19897">View Change</a></p><p>6 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</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/+/19897/comment/7cefa605_40f8e15f">Patch Set #2, Line 7:</a> <code style="font-family:monospace,monospace">res_pjsip_stir_shaken: Fix JSON field ordering and canonicalization.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">"Canonicalization" in the context of RFC 8224 § 8. […]</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">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/+/19897?tab=comments">Patch Set #6:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">Not sure yet what's up with the json test failure, I can't reproduce that on any of my machines currently (tests all pass). In the meantime:</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/json.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/+/19897/comment/f4255b0d_961ecb29">Patch Set #2, Line 816:</a> <code style="font-family:monospace,monospace">char *ast_json_dump_string_sorted(struct ast_json *root);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Rather than adding new API can we just add a new member to ast_json_encoding_format and update dump_ […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think this makes sense. I'm not a huge fan of the flags being mutually exclusive internally (since the enum doesn't specify the values), but it's probably fine for these 3 options.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File main/json.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/+/19897/comment/83545d7d_2f4f7697">Patch Set #2, Line 460:</a> <code style="font-family:monospace,monospace"> JSON_INDENT(2) | JSON_PRESERVE_ORDER : JSON_COMPACT;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You can remove `JSON_PRESERVE_ORDER` entirely as part of updating `dump_flags()` since it has been d […]</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 res/res_pjsip_stir_shaken.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/+/19897/comment/ff56fa94_b864f88b">Patch Set #2, Line 378:</a> <code style="font-family:monospace,monospace"> char *canonical_num;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You don't need this, you can just use `dest_tn` directly. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19897/comment/9d076cac_d7028ac5">Patch Set #2, Line 405:</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;">/* Canonicalize the telephone number according to RFC 8224 (required by RFC 8225) */<br> canonical_num = ast_malloc(strlen(dest_tn) + 1);<br> if (!canonical_num) {<br> return -1;<br> } else {<br> const char *s = dest_tn;<br> char *new = canonical_num;<br> /* We're only removing characters, if anything, so the buffer is guaranteed to be large enough */<br> while (*s) {<br> if (isdigit(*s) || *s == '#' || *s == '*') { /* Only characters allowed */<br> *new++ = *s;<br> }<br> s++;<br> }<br> *new = '\0';<br> ast_debug(4, "Canonicalized telephone number %s -> %s\n", dest_tn, canonical_num);<br> }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Do all of this with `dest_tn` above (at line 397) instead of doing another allocation. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19897">change 19897</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/+/19897"/><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: Iab76d39447c4b8cf133de85657dba02fda07f9a2 </div>
<div style="display:none"> Gerrit-Change-Number: 19897 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: N A <asterisk@phreaknet.org> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </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-Comment-Date: Tue, 28 Feb 2023 02:00: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: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>