[asterisk-dev] [Code Review] 4279: chan_sip: Send CANCEL via proxy if CANCEL is to be sent after an UPDATE

[Matt Jordan]

> On Dec. 19, 2014, 9:59 a.m., Scott Griepentrog wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c, lines 11834-11842
> > <https://reviewboard.asterisk.org/r/4279/diff/1/?file=69816#file69816line11834>
> >
> >     I would recommend appending to the comment block something like this:
> >     
> >     ASTERISK-24628: Send UPDATE to the same destination as CANCEL
> 
> kwemheuer wrote:
>     OK. I think in the comment is a little typo ("unless this is a CANCEL _on_ an ACK.."). I think, it should have been "unless this is a CANCEL _or_ an ACK..". At least this is, what the code is doing. Should I fix this, when adding the new comment?

We don't typically put issue numbers in comments, as the commit message should have the issue number referenced in it. The output 'svn blame' should help explain the issue that caused the code change.


> On Dec. 19, 2014, 9:59 a.m., Scott Griepentrog wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c, lines 11840-11842
> > <https://reviewboard.asterisk.org/r/4279/diff/1/?file=69816#file69816line11840>
> >
> >     Purely for readability, the sipmethod == SIP_UPDATE condition should be indented to the same level as SIP_ACK, since it is similarly nested under the same negation started on the line containing SIP_CANCEL.
> 
> kwemheuer wrote:
>     OK. Shall I update my patch?

Yes! :-)


- Matt


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


On Dec. 17, 2014, 12:11 p.m., kwemheuer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4279/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2014, 12:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24628
>     https://issues.asterisk.org/jira/browse/ASTERISK-24628
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Given three SIP phones (A, B, C), all communicating via a proxy (in this case OpenSIPs) with asterisk. A call is established between A and B. B sets the call on hold (A is hearing MOH) and dials the extension of C. While phone C is ringing, B transfers the call (A is hearing ringing of phone C, B is idle). On SIP there is a REFER to transfer the call. In case "sendrpid=yes" asterisk sends an UPDATE to phone C. This update is sent directly (not through the proxy). If someone (e.g. B) is trying to get the call back (through a directed pickup), asterisk sends a CANCEL to C. But this CANCEL is sent directly to C not through the proxy. The phone ignores this CANCEL according to the RFC3261 (Section 9.1).
> 
> In other situations asterisk ensures, that a CANCEL is sent the same way the INVITE has been sent. In this case the previously sent UPDATE overwrites the address used later for the CANCEL. On the other hand the UPDATE has to be sent via the proxy too (IMHO). The call is in the ringing state and other endpoints maybe have to been updated too.
> 
> The patch solves the issue. In function reqprep the UPDATE is handled the same way the CANCEL was (in case the state is in ringing state).
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 429698 
> 
> Diff: https://reviewboard.asterisk.org/r/4279/diff/
> 
> 
> Testing
> -------
> 
> Patch is tested in the described scenario and solves the issue. Patched against 11.15.0.
> 
> 
> Thanks,
> 
> kwemheuer
> 
>

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