[asterisk-dev] [Code Review] 2827: chan_sip: Reject call on 200 OK response to invite that lacks SDP

[jrose]

> On Sept. 5, 2013, 8:03 p.m., Mark Michelson wrote:
> > Looks like a sane change to me.
> > 
> > The only questionable part to me is activating the RTP instance on failure. I'm curious if that's really necessary considering we're about to destroy the dialog soon. I don't think having it there hurts necessarily, but it may also be fine to remove it.

I have no idea on that one. I'm only doing it that way because that's how it is handled when the SDP is present but can't be parsed. I figure it's best to do things the same way for now.


- jrose


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2827/#review9609
-----------------------------------------------------------


On Sept. 5, 2013, 7:31 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2827/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2013, 7:31 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp, Matt Jordan, and Mark Michelson.
> 
> 
> Bugs: ASTERISK-22424
>     https://issues.asterisk.org/jira/browse/ASTERISK-22424
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> 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).
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 398378 
> 
> Diff: https://reviewboard.asterisk.org/r/2827/diff/
> 
> 
> Testing
> -------
> 
> 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).
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130905/e3173f1d/attachment.htm>