<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/4577/">https://reviewboard.asterisk.org/r/4577/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<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/4577/diff/4/?file=73633#file73633line479" style="color: black; font-weight: bold; text-decoration: underline;">/certified/branches/13.1/res/res_pjsip_t38.c</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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 void t38_masq(void *data, int framehook_id,</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">477</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">if</span> <span class="p">((</span><span class="n">datastore</span> <span class="o">=</span> <span class="n">ast_channel_datastore_find</span><span class="p">(</span><span class="n">session</span><span class="o">-></span><span class="n">channel</span><span class="p">,</span> <span class="o">&</span><span class="n">t38_framehook_datastore</span><span class="p">,</span> <span class="nb">NULL</span><span class="p">)))</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;">Bike shedding!
For readability purposes - and to match the pattern you use later when you allocate the datastore - I'd do the look up on its own line, then test to see if you found it:
datastore = ast_channel_datastore_find(...);
if (datastore) {
}</pre>
</div>
<br />
<p>- Matt Jordan</p>
<br />
<p>On April 6th, 2015, 1:16 p.m. CDT, Jonathan Rose 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 and Joshua Colp.</div>
<div>By Jonathan Rose.</div>
<p style="color: grey;"><i>Updated April 6, 2015, 1:16 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-24933">ASTERISK-24933</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;">Description:
Two Asterisk boxes each have the other as endpoints with authentication set.
First Asterisk box originates a call to the second using the PJSIP endpoint.
The first Asterisk box uses an extension with sendfax, the second uses
an extension with receivefax.
The session starts fairly normally, but resolution never appears in fax show
session output. After a while (~25 seconds) the call drops and the fax fails.
Error messages shown are as follows:
Sender: The call dropped prematurely
Receiver: Disconnected after permitted retries
Note that when not using authentication, the FAX will complete as expected.
When using chan_sip as the sender to a receiver of chan_pjsip, the FAX will
also complete as expected with authentication, but if chan_pjsip is the sender
it will fail regardless of whether the recipient is chan_sip or chan_pjsip.
The problem is caused by duplication of a framehook in res_pjsip_t38 which
occurs on the second invite sent out when responding to the auth challenge.
Fix:
In order to fix this, I added a simple flag to the pjsip session struct that would
be raised when the framehook is first attached to prevent duplication. I wouldn't
be surprised if there were a better way to do this.</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;">I've duplicated and modified the t38 PJSIP fax test in the testsuite to include authentication. It fails without the patch and passes with the patch. I also tested this locally with my two Asterisk machines in the above scenario. I'll be linking the test after putting it on Gerrit.</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>/certified/branches/13.1/res/res_pjsip_t38.c <span style="color: grey">(433316)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4577/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>