[asterisk-dev] [Code Review]: Fixing the regression from wrong set route information on provisional sip responses

[wdoekes]

> On Feb. 16, 2012, 10:33 a.m., Mark Michelson wrote:
> > This is definitely the correct approach to take. Don't make the route persistent when building a route set from a provisional response. I have an alternative implementation in my head though. Instead of telling the build_route() function whether to make the route set persistent, just have build_route() automatically do it if the passed-in request is a SIP provisional response. That way, people can't make the mistake of using the wrong value for the persistent parameter. The correct behavior will happen every time.
> 
> kwemheuer wrote:
>     This is my first visit in the reviewboard, so apologies if this is the wrong procedure. The patch fixes one of the problems of ASTERISK-19358 (sending ACK to wrong phone). But the issues original core is "Failed to CANCEL a call in ringing state". The fix for this was the patch from Mark Mitchelson from 14/Feb/12 2:07 PM. I think this should be integrated.

Mark wrote:
> Okay, I'm going to go ahead and commit the contents of the patch. I'll leave this issue open until you tell me I should close it though.

But I see he hasn't done that yet.

Nevertheless, IMO these are two different bugs, even if they may have been caused by the same changeset (which I don't know if they are).

The CANCEL issue is about using routing information when it shouldn't.
The ACK issue is about using the wrong routing information.

They are related, but not identical. Different bugs call for different patches. (And ideally, different issues on the issue tracker, but we won't bother you with that.)


- wdoekes


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


On Feb. 16, 2012, 3:58 p.m., schmidts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1749/
> -----------------------------------------------------------
> 
> (Updated Feb. 16, 2012, 3:58 p.m.)
> 
> 
> Review request for Asterisk Developers and wdoekes.
> 
> 
> Summary
> -------
> 
> by adding the parse_ok_header and build_route for provisional responses to invite. I had caused a regression that asterisk stores a wrong route set from this provisional packets. With this change a route set from a provisionial response will not be stored persistant and will be replaced with the route information from a 200 ok packet.
> see bug ASTERISK-19358 for more information and sip traces.
> 
> 
> This addresses bug ASTERISK-19358.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19358
> 
> 
> Diffs
> -----
> 
>   branches/1.8/channels/chan_sip.c 355573 
> 
> Diff: https://reviewboard.asterisk.org/r/1749/diff
> 
> 
> Testing
> -------
> 
> tested with the sipp scenario from wdoekes (thank you for this).
> 
> 
> Thanks,
> 
> schmidts
> 
>

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