<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/1590/">https://reviewboard.asterisk.org/r/1590/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">few minor comments</pre>
<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/1590/diff/1/?file=21797#file21797line1" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/configs/http.conf</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">[general]</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">enabled=yes</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">3</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bindaddr=127.0.0.1</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bindport=8088</pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">5</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">prefix=</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;">This is going to affect all tests, we should do like manager.conf. Disable it first and use include statements to enable.</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/1590/diff/1/?file=21798#file21798line3" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/configs/manager.conf</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">3</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">webenabled = yes</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;">same comment, do we want this enabled for all tests?</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/1590/diff/1/?file=21799#file21799line1" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/lib/python/asterisk/syncami.py</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kn">from</span> <span class="nn">urllib</span> <span class="kn">import</span> <span class="n">urlencode</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kn">from</span> <span class="nn">email.parser</span> <span class="kn">import</span> <span class="n">HeaderParser</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;">Random though, we could add this functionality into StarPy.</pre>
</div>
<br />
<p>- Paul</p>
<br />
<p>On November 17th, 2011, 1:20 p.m., Terry Wilson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Paul Belanger and mjordan.</div>
<div>By Terry Wilson.</div>
<p style="color: grey;"><i>Updated Nov. 17, 2011, 1:20 p.m.</i></p>
<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 test loops through all combinations of nat=route/no (force_rport/no in 1.8+) for [general] and [peer] in sip.conf, whether or not the incoming request specified ;rport in the Via, and whether request was sent from the same port as the one listed in the Via, for both a valid and an invalid user. The test records when the behavior differs and matches against when we expect that behavior to differ for each version of Asterisk (1.4, 1.6.2, 1.8, 10, trunk). It also uses a Quine-McClusky minimization solver to reduce the list of failures to a function describing the failures. For example:
On 1.4:
(((NOT port_matches_via) AND peer_nat AND (NOT general_nat)) OR ((NOT port_matches_via) AND (NOT peer_nat) AND general_nat))
and 1.8:
(((NOT port_matches_via) AND (NOT rport_specified) AND (NOT peer_nat) AND general_nat) OR ((NOT port_matches_via) AND (NOT rport_specified) AND peer_nat AND (NOT general_nat)))
As you see, there is an extra term in 1.8 because in 1.8 we honor the ;rport in the Via no matter what, so differing behavior only occurs when a request comes in *not* specifying ;rport. There is no equivalent for nat=route or nat=never in 1.8+ (these values were originally added to work around a bug in old Uniden phone firmware).
This patch also adds a very small SyncAMI class for synchronously sending AMI requests (over HTTP because it is much simpler to do via HTTP's built-in request/response architecture) and getting back a response. It also modifies the default manager.conf and http.conf files to support the new API. SyncAMI makes it much easier to do things like update a config and reload it over AMI as you don't have to split up what you are doing into a lot of callback functions and you really do want to block execution until they are complete. Normally, I would have done a separate review for SyncAMI, but as this patch requires its use I've left them together for the review.</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;">Tested against 1.4 and 1.8 and have gotten the values I expect back. The nat options for 1.4 and 1.6.2; and 1.8, 10, and trunk are the same, so I expect similar behavior there.</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>/asterisk/trunk/configs/branch-1.4/manager.conf <span style="color: grey">(2750)</span></li>
<li>/asterisk/trunk/configs/http.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/configs/manager.conf <span style="color: grey">(2750)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/syncami.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/lib/python/qm.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/configs/ast1/sip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/run-test <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/sipp/register.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/tests.yaml <span style="color: grey">(2750)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1590/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>