[asterisk-dev] [Code Review] 3580: Handle 183 without SDP - don't always convert to ringing

[Matt Jordan]

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


This is one of those cases that really could use a test in the Asterisk Test Suite.
(1) It's dealing with odd behavior in a very old part of chan_sip
(2) The existing behavior works around bad behavior in an old PBX; the new patch properly handles a corner case of a modern system (multiple 183's aren't super common)
(3) The risk of someone coming along and making an inappropriate code change that breaks either the workaround for the Alcatel PBX or Lync is reasonably high

The purpose of a test here is less to verify the correctness of the patch - I believe you tested it with several thousand calls! - but rather to help "future proof" Asterisk against subsequent changes.

I'd be happy to help work some SIPp scenarios into the Asterisk Test Suite - do you think you could construct a SIPp scenario that sends the multiple 183's in the same fashion that Lync does?

- Matt Jordan


On June 3, 2014, 9:34 a.m., Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3580/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 9:34 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-15852
>     https://issues.asterisk.org/jira/browse/ASTERISK-15852
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Lync sends the first 183 with SDP using prack. Following 183s have no SDP. The current code treats follow-ups as RINGING, which is a bug.
> 
> Red blobs are a bonus in the review, but will not be offered in commit.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 414989 
> 
> Diff: https://reviewboard.asterisk.org/r/3580/diff/
> 
> 
> Testing
> -------
> 
> Just a few thousand calls. That's all.
> 
> 
> Thanks,
> 
> Olle E Johansson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140603/db9c00f0/attachment.html>