[asterisk-dev] [Code Review] Properly route responses according to the Via headers in the request

Matthew Nicholson reviewboard at asterisk.org
Wed Dec 29 19:07:03 UTC 2010



> On 2010-12-22 18:29:52, David Vossel wrote:
> > /branches/1.4/channels/chan_sip.c, lines 5003-5011
> > <https://reviewboard.asterisk.org/r/1059/diff/6/?file=14800#file14800line5003>
> >
> >     Is there any reason we'd want to modify the behavior here based on the NAT flags similar to the way the set_address_from_contact() function does?

NAT handling is done in sip_real_dst() called from __sip_xmit().


> On 2010-12-22 18:29:52, David Vossel wrote:
> > /branches/1.4/channels/chan_sip.c, line 6672
> > <https://reviewboard.asterisk.org/r/1059/diff/6/?file=14800#file14800line6672>
> >
> >     Instead of decoupling the p->sa = p->recv from the process_via header when an error occurs.  You could just put p->sa = p->recv before all the return -1's in process_via().

Done.


> On 2010-12-22 18:29:52, David Vossel wrote:
> > /branches/1.4/channels/chan_sip.c, lines 4950-4953
> > <https://reviewboard.asterisk.org/r/1059/diff/6/?file=14800#file14800line4950>
> >
> >     Maybe indicate in the documentation that it sets the peer's address from the via or received address as well.

Done.


> On 2010-12-22 18:29:52, David Vossel wrote:
> > /branches/1.4/channels/chan_sip.c, line 4982
> > <https://reviewboard.asterisk.org/r/1059/diff/6/?file=14800#file14800line4982>
> >
> >     A host:port is likely never going to be this large, but a 64 byte limit seems like it could be hit.  Why not make it 128 bytes and declare it under the "maddr=" check since that's where it is used.

Done.


- Matthew


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


On 2010-12-29 13:06:27, Matthew Nicholson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1059/
> -----------------------------------------------------------
> 
> (Updated 2010-12-29 13:06:27)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch makes asterisk respect the Via headers in a request when responding to the request. Without this patch, the request is always routed back to the address the initial request was received from (unless nat=yes).  This can cause problems if the initial request comes through a proxy and additional requests (such as INFO dtmf tones) come from a different proxy.
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/channels/chan_sip.c 299447 
> 
> Diff: https://reviewboard.asterisk.org/r/1059/diff
> 
> 
> Testing
> -------
> 
> Briefly tested using openser as a proxy and another asterisk machine as the requester.  I sent and invite, then some INFO DTMF messages.  Without the patch, our asterisk machine sends all responses to the INFO requests to the proxy, with the patch they are properly routed to the requesting asterisk machine.
> 
> 
> Thanks,
> 
> Matthew
> 
>

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


More information about the asterisk-dev mailing list