<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/2987/">https://reviewboard.asterisk.org/r/2987/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2987/diff/2/?file=48005#file48005line237" style="color: black; font-weight: bold; text-decoration: underline;">branches/12/include/asterisk/stasis.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">struct stasis_message_type;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">205</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">struct</span> <span class="n">ast_json</span> <span class="o">*</span><span class="p">(</span><span class="o">*</span><span class="n">to_json</span><span class="p">)(</span><span class="k">struct</span> <span class="n">stasis_message</span> <span class="o">*</span><span class="n">message</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">237</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">struct</span> <span class="n">ast_json</span> <span class="o">*</span><span class="p">(</span><span class="o">*</span><span class="n">to_json</span><span class="p">)(</span><span class="k">struct</span> <span class="n">stasis_message</span> <span class="o">*</span><span class="n">message</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="k"><span class="hl">struct</span></span><span class="hl"> </span><span class="n"><span class="hl">stasis_message_sanitizer</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n"><span class="hl">sanitize</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm a bit torn here.
In principle, I like the idea of passing in the sanitizer so that different consumers can sanitize JSON to their own needs.
But the fact that you have to pass around the sanitizer object everywhere, given that we only have one implementation, feels a bit weighty to the API.
I don't have a good suggestion for a solution. Just complaining out loud...</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2987/diff/2/?file=48011#file48011line1839" style="color: black; font-weight: bold; text-decoration: underline;">branches/12/main/rtp_engine.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static struct ast_json *rtcp_report_to_json(struct stasis_message *msg)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static struct ast_json *rtcp_report_to_json(struct stasis_message *msg,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1838</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"><span class="tb"> </span></span><span class="n"><span class="hl">json_payload</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span> <span class="n">ast_json_pack</span><span class="p">(</span><span class="s">"{s: O, s: O, s: O}"</span><span class="p">,</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1838</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"><span class="tb"> </span></span><span class="k"><span class="hl">return</span></span> <span class="n">ast_json_pack</span><span class="p">(</span><span class="s">"{s: O, s: O, s: O}"</span><span class="p">,</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1839</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="s">"channel"</span><span class="p">,</span> <span class="n">payload</span><span class="o">-></span><span class="n">snapshot</span> <span class="o">?</span> <span class="n">ast_channel_snapshot_to_json</span><span class="p">(</span><span class="n">payload</span><span class="o">-></span><span class="n">snapshot</span><span class="p">)</span> <span class="o">:</span> <span class="n">ast_json_null</span><span class="p">(),</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1839</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="s">"channel"</span><span class="p">,</span> <span class="n">payload</span><span class="o">-></span><span class="n">snapshot</span> <span class="o">?</span> <span class="n">ast_channel_snapshot_to_json</span><span class="p">(</span><span class="n">payload</span><span class="o">-></span><span class="n">snapshot</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">sanitize</span></span><span class="p">)</span> <span class="o">:</span> <span class="n">ast_json_null</span><span class="p">(),</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1840</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="s">"rtcp_report"</span><span class="p">,</span> <span class="n">json_rtcp_report</span><span class="p">,</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1840</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="s">"rtcp_report"</span><span class="p">,</span> <span class="n">json_rtcp_report</span><span class="p">,</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1841</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="s">"blob"</span><span class="p">,</span> <span class="n">payload</span><span class="o">-></span><span class="n">blob</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1841</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="s">"blob"</span><span class="p">,</span> <span class="n">payload</span><span class="o">-></span><span class="n">blob</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just a guess, but I think you'd want to drop the RTCP report completely if the associated channel was filtered out by the sanitizer.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/2987/diff/2/?file=48012#file48012line333" style="color: black; font-weight: bold; text-decoration: underline;">branches/12/main/stasis_bridges.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static struct ast_bridge_merge_message *bridge_merge_message_create(struct ast_bridge *to, struct ast_bridge *from)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">325</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">ast_json_pack</span><span class="p">(</span><span class="s">"{s: s, s: o, s: o, s: o}"</span><span class="p">,</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">333</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">ast_json_pack</span><span class="p">(</span><span class="s">"{s: s, s: o, s: o, s: o}"</span><span class="p">,</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">326</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"type"</span><span class="p">,</span> <span class="s">"BridgeMerged"</span><span class="p">,</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">334</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"type"</span><span class="p">,</span> <span class="s">"BridgeMerged"</span><span class="p">,</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">327</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"timestamp"</span><span class="p">,</span> <span class="n">ast_json_timeval</span><span class="p">(</span><span class="o">*</span><span class="n">stasis_message_timestamp</span><span class="p">(</span><span class="n">msg</span><span class="p">),</span> <span class="nb">NULL</span><span class="p">),</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">335</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"timestamp"</span><span class="p">,</span> <span class="n">ast_json_timeval</span><span class="p">(</span><span class="o">*</span><span class="n">stasis_message_timestamp</span><span class="p">(</span><span class="n">msg</span><span class="p">),</span> <span class="nb">NULL</span><span class="p">),</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">328</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"bridge"</span><span class="p">,</span> <span class="n">ast_bridge_snapshot_to_json</span><span class="p">(</span><span class="n">merge</span><span class="o">-></span><span class="n">to</span><span class="p">),</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">336</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"bridge"</span><span class="p">,</span> <span class="n">ast_bridge_snapshot_to_json</span><span class="p">(</span><span class="n">merge</span><span class="o">-></span><span class="n">to</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">sanitize</span></span><span class="p">),</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">329</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"bridge_from"</span><span class="p">,</span> <span class="n">ast_bridge_snapshot_to_json</span><span class="p">(</span><span class="n">merge</span><span class="o">-></span><span class="n">from</span><span class="p">));</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">337</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"bridge_from"</span><span class="p">,</span> <span class="n">ast_bridge_snapshot_to_json</span><span class="p">(</span><span class="n">merge</span><span class="o">-></span><span class="n">from</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">sanitize</span></span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Would I be right in assuming that if either bridge was filtered out, then ast_json_pack() returns NULL, effectively filtering out the message?
If so, I would rather be explicitly return NULL when either snapshot is NULL. The underlying Jansson json_pack() behavior is undocumented, and could easily change in a future release.
If not, then you'll end up with messages that effectively have missing fields, and it would be better to filter them out entirely.
Either way, this (and several other to_json methods below) need an explicit check to filter them out if one of their fields were null.</pre>
</div>
<br />
<p>- David Lee</p>
<br />
<p>On November 6th, 2013, 1:46 p.m. CST, opticron wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By opticron.</div>
<p style="color: grey;"><i>Updated Nov. 6, 2013, 1:46 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-22744">ASTERISK-22744</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This change prevents channels used as implementation details from leaking out to ARI. It does this by preventing creation of JSON blobs of channel snapshots created from those channels and sanitizing JSON blobs of bridge snapshots as they are created. This introduces a framework for excluding information from output targeted at Stasis applications on a consumer-by-consumer basis using channel sanitization callbacks which could be extended to bridges or endpoints if necessary.
This results in NULL inputs to ast_json_pack calls which generate unhelpful error messages, so that has been dealt with as well.
This also corrects a bug I noticed while investigating the issue where BridgeCreated events would not be created.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Manual testing with bridges and channels.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/12/res/stasis/app.c <span style="color: grey">(402347)</span></li>
<li>branches/12/res/res_stasis.c <span style="color: grey">(402347)</span></li>
<li>branches/12/res/ari/resource_endpoints.c <span style="color: grey">(402347)</span></li>
<li>branches/12/res/ari/resource_channels.c <span style="color: grey">(402347)</span></li>
<li>branches/12/res/ari/resource_bridges.c <span style="color: grey">(402347)</span></li>
<li>branches/12/main/stasis_message.c <span style="color: grey">(402347)</span></li>
<li>branches/12/main/stasis_endpoints.c <span style="color: grey">(402347)</span></li>
<li>branches/12/main/stasis_channels.c <span style="color: grey">(402347)</span></li>
<li>branches/12/main/stasis_bridges.c <span style="color: grey">(402347)</span></li>
<li>branches/12/main/rtp_engine.c <span style="color: grey">(402347)</span></li>
<li>branches/12/main/json.c <span style="color: grey">(402347)</span></li>
<li>branches/12/include/asterisk/stasis_endpoints.h <span style="color: grey">(402347)</span></li>
<li>branches/12/include/asterisk/stasis_channels.h <span style="color: grey">(402347)</span></li>
<li>branches/12/include/asterisk/stasis_bridges.h <span style="color: grey">(402347)</span></li>
<li>branches/12/include/asterisk/stasis_app.h <span style="color: grey">(402347)</span></li>
<li>branches/12/include/asterisk/stasis.h <span style="color: grey">(402347)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2987/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>