<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/2827/">https://reviewboard.asterisk.org/r/2827/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 6th, 2013, 8:07 a.m. CEST, <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This doesn't work well with PRACK or UPDATES. Asterisk could already have gotten an SDP in another message. (I have a branch with PRACK support that is in production). We need to check if we already have gotten an SDP before we decide to hang up the call. I also need to check the RFCs for this situation.</pre>
</blockquote>
<p>On September 6th, 2013, 10:26 a.m. CEST, <b>wdoekes</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">.. or re-INVITES.
I could imagine that some lazy clients skip the SDP in session refreshers.</pre>
</blockquote>
<p>On September 6th, 2013, 10:30 a.m. CEST, <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, but if this is a 200 OK it would mean that we would have sent an INVITE without SDP. Will that ever happen? Curious.</pre>
</blockquote>
<p>On September 6th, 2013, 10:31 a.m. CEST, <b>Olle E Johansson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As a summary, this is NOT ready for a SHIP IT nor a commit.</pre>
</blockquote>
<p>On September 6th, 2013, 5:27 p.m. CEST, <b>jrose</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not sure about PRACK or UPDATE for now, but in the case of a reinvite, it's not going to hang up the call, it's just going to issue a log message and execute ast_rtp_instance_activate() with whatever it's RTP was already doing, so that much should be fine. If it's a concern, we can always make this an effective NO-OP on reinvite as well.
I'll wait on this for the time being.</pre>
</blockquote>
<p>On September 6th, 2013, 9:34 p.m. CEST, <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">UPDATE and PRACK are irrelevant as far as chan_sip is concerned. chan_sip doesn't support those and it never will. As far as chan_sip is concerned, the only reliable response that may contain an SDP offer or answer is a 200 OK.
I'd be willing to compromise and say that if Asterisk receives a 200 OK with no SDP but the RTP remote address is non-NULL (meaning we've negotiated an SDP on this dialog, possibly due to an incoming 183), then we could just roll with what we have and hope for the best. Of course, the UAS that sent that 200 OK is totally broken in that case for relying on an unreliable response to provide an SDP answer.
Be aware, of course, that this can lead to its own set of issues as well. Our INVITE could be forked by a proxy to multiple UASs. If multiple destinations send 183 responses with SDP answers, then it becomes important not only that we negotiated an SDP but that we received an SDP on the particular branch that the 200 OK is received on. And as you know, chan_sip's transaction and forking awareness is pretty dismal.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What? Never will? I have all that code and hope to get it included at some point.
</pre>
<br />
<p>- Olle E</p>
<br />
<p>On September 5th, 2013, 9:31 p.m. CEST, jrose 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Joshua Colp, Matt Jordan, and Mark Michelson.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Sept. 5, 2013, 9:31 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-22424">ASTERISK-22424</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;">One of our SIP tests was previously pushing 200 OKs without SDP and Asterisk would accept these calls without question. According to Mark this should not be accepted because there will be no way to know where to send media to or receive media from in these circumstances. The approach this patch takes is to forcibly hang up the call at this point if there is no SDP on the response provided that it's not a response to a reinvite (in which case the behavior is the same as if there were an SDP that couldn't be parsed properly).</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 it against SIP_hold before and after
Tested it against a number of testsuite tests against SIP (any of the ones I could run before the patch)
Tested regular SIP phone calls (they didn't hit the modified code path though).</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/1.8/channels/chan_sip.c <span style="color: grey">(398378)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2827/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>