[asterisk-dev] [Code Review]: Fix natdetected flag being set when VIA doesn't include port in address

[Mark Michelson]

> On Nov. 26, 2012, 3:50 a.m., wdoekes wrote:
> > branches/11/channels/chan_sip.c, lines 17754-17764
> > <https://reviewboard.asterisk.org/r/2206/diff/1/?file=32226#file32226line17754>
> >
> >     For the sake of readable code, while you're in the neighourhood, I'd move the last bit into an else:
> >     
> >     if (ast_sockaddr_resolve_first(&tmp, c, 0)) {
> >        ..WARN..
> >        port = STANDARD_SIP_PORT;
> >     } else if (!(port = ast_sockaddr_port(&tmp))) {
> >        ...
> >     }
> >     
> >     It already works like that, but this ensures it stays working if someone changes code around somewhere else.

I agree with Walter here. It seems a bit odd to check tmp's port if an address could not be resolved.


- Mark


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


On Nov. 25, 2012, 10:37 p.m., elguero wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2206/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2012, 10:37 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> While troubleshooting another issue, came across this message in the debug log:
> [2012-11-25 19:07:25] DEBUG[13375][C-00000007]: chan_sip.c:17910 check_via: NAT detected for 216.115.69.144:0 / 216.115.69.144:5060
> Sending to 216.115.69.144:5060 (NAT)
> 
> Upon looking at the code in check_via, I discovered that if the port is not set on the address in the VIA header, the default SIP port is not being set on struct ast_sockaddr tmp which is used to check if the requesting UA might be behind NAT.
> 
> 
> This addresses bug ASTERISK-20724.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20724
> 
> 
> Diffs
> -----
> 
>   branches/11/channels/chan_sip.c 376416 
> 
> Diff: https://reviewboard.asterisk.org/r/2206/diff
> 
> 
> Testing
> -------
> 
> Local dev machine.
> 
> 
> Thanks,
> 
> elguero
> 
>

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