[Home]

Summary:ASTERISK-24063: [patch]Asterisk does not respect outbound proxy when sending qualify requests
Reporter:Damian Ivereigh (damianivereigh)Labels:
Date Opened:2014-07-20 06:19:37Date Closed:2014-10-17 08:09:35
Priority:MajorRegression?
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:11.4.0 11.6.0 Frequency of
Occurrence
Constant
Related
Issues:
Environment:CentOS 6Attachments:( 0) outboundproxy-dai.patch
Description:The outboundproxy setting is ignored when sending the qualify packets (OPTIONS). This means that if an asterisk server is unable to send the packet directly to a peer, it is unable to qualify any non inbound registered peer (e.g. a peer SIP Trunk). This problem is found on asterisk-11.6-cert4

I have a patch to fix it - I will attempt to attach it.
Comments:By: Damian Ivereigh (damianivereigh) 2014-07-20 06:24:46.212-0500

Fix outboundproxy setting being ignored for the qualification OPTIONS packets

By: Rusty Newton (rnewton) 2014-07-22 17:41:22.321-0500

[~damianivereigh]

Thanks! Though the patch looks small, it would still be great if you could move on to the next steps of the [Code Review process|https://wiki.asterisk.org/wiki/display/AST/Code+Review] and throw the patch on Reviewboard. Once you get the reviewboard URL you can edit this issue and link it in the appropriate Reviewboard field.

By: Damian Ivereigh (damianivereigh) 2014-07-23 21:31:31.368-0500

Well I tried to submit the patch to the review board, however it wants me to use rbt and submit it in a special form of unified diff. I really didn't want to spend hours learning how to use rbt and doing a whole checkout of the code. I am not a regular Asterisk developer and this is all just way over the top for what is just a one line patch.

I realise that for regular developers this is probably a very useful system, but as an open source project there really needs to a simple way to submit a suggested patch from the rabble, such as me. Otherwise you end up creating a "Catherdral" (c.f. "The Cathedral and the Bazaar" - Eric Raymond) type system - which is not good for open source projects.

Could you please submit it to the reviewboard for me given that you probably have all the tools and setup to do it.

Thanks mate,
Damian

By: Matt Jordan (mjordan) 2014-07-25 08:21:08.102-0500

Sorry, but no. You don't have to post it to review board, but that does mean that it is less likely that someone will participate in the review of your patch. Refusing to use the tools the project provides is your choice; it is also the choice of regular developers to pay less attention to your contribution if you refuse to use the same process as every one else.

The instructions for using review board are not onerous; in fact, you have an account automatically created for you that is tied to your JIRA account. The act of performing code review has had substantial benefits for the quality of the project; review board, as a tool, has been instrumental in that effort. There is no Cathedral here; we have an open system that anyone who has signed a license contributor agreement can participate in.

You're more than welcome to have your patch on this issue; if other contributors are interested in it, they may choose to post your patch for review. I would not expect that to happen quickly.

By: Walter Doekes (wdoekes) 2014-08-21 05:17:18.987-0500

Damian: I never use the rbt tool. You can upload on the website directly.

And yes, it uses the ubiquitous "unified" diff, which you get with any {{diff -u}} or {{svn diff}} or {{git diff}} or ...

By: Walter Doekes (wdoekes) 2014-08-21 05:33:58.131-0500

As for "one line of code": in chan_sip, one line of code can have a big ripple effect.

Just by browsing chan_sip, I cannot immediately tell whether the proxy gets unreffed at the end. AFAICS, we get to {{dialog_unref \-> __sip_destroy}} where {{p\->options->outboundproxy}} gets unreffed/freed, but {{p->outboundproxy}} itself does not.

By: Damian Ivereigh (damianivereigh) 2014-08-25 17:06:22.173-0500

Thanks for checking this out Walter. Finally figured out how to submit the patch using svn diff.

Code review: #3948

I was hoping that ref_proxy() would deal with the unreffing, but this does need to be verified.