[asterisk-dev] [Code Review] 2851: chan_sip: Remove requirement for resolving host when outbound proxy in use

wdoekes reviewboard at asterisk.org
Mon Nov 18 06:38:09 CST 2013



> On Nov. 7, 2013, 11 p.m., Mark Michelson wrote:
> > /branches/1.8/CHANGES, lines 17-18
> > <https://reviewboard.asterisk.org/r/2851/diff/3/?file=47949#file47949line17>
> >
> >     This first sentence could use some rewording. I initially read it to mean that if a hostname were provided as the outboundproxy option, then it would not be resolved. What you mean here is that a host option will not be resolved if an outboundproxy is provided, right?
> 
> wdoekes wrote:
>     That sentence can definitely use some fixage, indeed.
>     
>     The other parts of the patch look good (except for the double line-feed in create_addr_from_peer()).
>     
>     I tried it and it didn't break my common use case scenario. OTOH it didn't fix it either :)
>     
>     For future reference:
>     
>       I use the dialog-based outboundproxy (set as Dial-string argument).
>       The dialog doesn't find its way into build_peer(),
>       so obproxy_get() doesn't get a dialog to check if there is an outboundproxy
>       => the lookup gets done anyway.
> 
> Joshua Colp wrote:
>     What's the Dial string for that?

/*! \brief PBX interface function -build SIP pvt structure
 *      SIP calls initiated by the PBX arrive here.
 *
 * \verbatim
 *      SIP Dial string syntax:
 *              SIP/devicename
 *      or      SIP/username at domain (SIP uri)
 *      or      SIP/username[:password[:md5secret[:authname[:transport]]]]@host[:port]
 *      or      SIP/devicename/extension
 *      or      SIP/devicename/extension/IPorHost
 *      or      SIP/username at domain//IPorHost
 *      and there is an optional [!dnid] argument you can append to alter the
 *      To: header.
 * \endverbatim
 */

That would be the last two:

        if (!ast_strlen_zero(remote_address)) {
                p->options->outboundproxy = proxy_from_config(remote_address, 0, NULL);
                if (!p->options->outboundproxy) {
                        ast_log(LOG_WARNING, "Unable to parse outboundproxy %s. We will not use this remote IP address\n", remote_address);
                }
        }

IIRC, originally, the remote_address was placed in a very temporary structure (p->sa or something?), but that only worked for the first request -- i.e. fail when a 401/407 was encountered.


- wdoekes


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


On Nov. 16, 2013, 4:24 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2851/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2013, 4:24 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21231 and ASTERISK-21694
>     https://issues.asterisk.org/jira/browse/ASTERISK-21231
>     https://issues.asterisk.org/jira/browse/ASTERISK-21694
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> During the development of 1.8 chan_sip was moved to using dnsmgr for host resolution. These changes introduced a regression where all hosts were looked up in DNS, including when an outbound proxy is in use. This is incorrect. The attached change removes this requirement.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 402874 
>   /branches/1.8/CHANGES 402874 
> 
> Diff: https://reviewboard.asterisk.org/r/2851/diff/
> 
> 
> Testing
> -------
> 
> Configured an outbound proxy at global and peer level, confirmed that no resolution occurs for the host and also that traffic goes to where it should. Removed outbound proxy and confirmed things returned to normal.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

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


More information about the asterisk-dev mailing list