Re: [asterisk-dev] [Code Review] 4038: Testsuite: Process REF_DEBUG logs, fail any test that leaks
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4038/ --- (Updated Oct. 29, 2014, 5:04 a.m.) Review request for Asterisk Developers. Changes --- Fix error handling in refleaks-summary script. Bugs: ASTERISK-24379 https://issues.asterisk.org/jira/browse/ASTERISK-24379 Repository: testsuite Description --- This causes any test that leaks references to fail if REF_DEBUG is enabled. Additionally run-local is modified to allow REF_DEBUG to be enabled through setup: MENUSELECT_OPTIONS='--enable REF_DEBUG' ./run-local setup Note if this option is used with Asterisk 1.8 all tests will fail due to manager.c leaking the sessions container. Diffs (updated) - /asterisk/trunk/runtests.py 5803 /asterisk/trunk/run-local 5803 /asterisk/trunk/contrib/scripts/refleaks-summary PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4038/diff/ Testing --- Ran against tests/channels/SIP/route on Asterisk 11 with and without r4037 applied. Test fails without, passes with. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4122/#review13604 --- /asterisk/trunk/tests/pbx/dialplan_reload/run-test https://reviewboard.asterisk.org/r/4122/#comment24117 I don't quite see what happens here: - the test does a 'core restart gracefully' 50 times? Does that fit in 30 seconds now? Or.. that is the reset_timeout() below? In that case a little comment somewhere would help, so I wouldn't be surprised when the test runs for longer than 30 seconds and succeeds. /asterisk/trunk/tests/pbx/dialplan_reload/run-test https://reviewboard.asterisk.org/r/4122/#comment24116 Is it needed to callLater() instead of just calling it? Applies to the other 0-second callLaters too. You'll defer handling soon enough. The only other example where I find a callLater(0, in the testsuite, is broken anyway: reactor.callLater(0, self.launch_test()) # tests/fastagi/record-file/run-test /asterisk/trunk/tests/pbx/dialplan_reload/run-test https://reviewboard.asterisk.org/r/4122/#comment24115 Restarted #%d % self.count /asterisk/trunk/tests/pbx/dialplan_reload/run-test https://reviewboard.asterisk.org/r/4122/#comment24113 Why do we wait (an arbitrary) 3 seconds for this? Can't we call the fullybooted_run immediately? /asterisk/trunk/tests/pbx/dialplan_reload/run-test https://reviewboard.asterisk.org/r/4122/#comment24114 Restarting #%d % self.count Lastly: PEP says space after a comma, please. - wdoekes On Oct. 28, 2014, 10:20 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4122/ --- (Updated Oct. 28, 2014, 10:20 p.m.) Review request for Asterisk Developers and Scott Griepentrog. Repository: testsuite Description --- In some configurations 3 seconds is not enough of a delay before Asterisk is fully booted, preventing core restart gracefully from succeeding. This causes many iterations to be skipped, and in some cases the test never ends. Make use of core waitfullybooted to delay restarts. Remove unused global variables workingdir and testdir, add global variable restart_iterations to specify 50 runs. Decrease reactor_timeout from 300 to 30, use reset_timeout instead. Diffs - /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 Diff: https://reviewboard.asterisk.org/r/4122/diff/ Testing --- Yes Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4038: Testsuite: Process REF_DEBUG logs, fail any test that leaks
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4038/#review13605 --- /asterisk/trunk/contrib/scripts/refleaks-summary https://reviewboard.asterisk.org/r/4038/#comment24118 How awfully correct of you :) But then -h should exit 0. - wdoekes On Oct. 29, 2014, 9:04 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4038/ --- (Updated Oct. 29, 2014, 9:04 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24379 https://issues.asterisk.org/jira/browse/ASTERISK-24379 Repository: testsuite Description --- This causes any test that leaks references to fail if REF_DEBUG is enabled. Additionally run-local is modified to allow REF_DEBUG to be enabled through setup: MENUSELECT_OPTIONS='--enable REF_DEBUG' ./run-local setup Note if this option is used with Asterisk 1.8 all tests will fail due to manager.c leaking the sessions container. Diffs - /asterisk/trunk/runtests.py 5803 /asterisk/trunk/run-local 5803 /asterisk/trunk/contrib/scripts/refleaks-summary PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4038/diff/ Testing --- Ran against tests/channels/SIP/route on Asterisk 11 with and without r4037 applied. Test fails without, passes with. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4038: Testsuite: Process REF_DEBUG logs, fail any test that leaks
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4038/ --- (Updated Oct. 29, 2014, 5:47 a.m.) Review request for Asterisk Developers. Changes --- exit 0 for -h Bugs: ASTERISK-24379 https://issues.asterisk.org/jira/browse/ASTERISK-24379 Repository: testsuite Description --- This causes any test that leaks references to fail if REF_DEBUG is enabled. Additionally run-local is modified to allow REF_DEBUG to be enabled through setup: MENUSELECT_OPTIONS='--enable REF_DEBUG' ./run-local setup Note if this option is used with Asterisk 1.8 all tests will fail due to manager.c leaking the sessions container. Diffs (updated) - /asterisk/trunk/runtests.py 5803 /asterisk/trunk/run-local 5803 /asterisk/trunk/contrib/scripts/refleaks-summary PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4038/diff/ Testing --- Ran against tests/channels/SIP/route on Asterisk 11 with and without r4037 applied. Test fails without, passes with. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24190 https://issues.asterisk.org/jira/browse/ASTERISK-24190 Repository: Asterisk Description --- In update_messages_by_imapuser(), messages were appended without checking bounds: vms-msgArray[vms-vmArrayIndex++] = number; This patch ensures that there is enough room. However, I did find quirky usage of thread local storage which I couldn't explain. Perhaps someone else can shed some light on the XXX's that I left in the code: - vms is thread-local, so it may not need to be freed. But on line 3033, it is overwritten if (strcmp(vms_p-imapuser, vmu-imapuser) || strcmp(vms_p-username, vmu-mailbox)) (or should it be freed in vmstate_delete?) - in vmstate_insert, an alternative mailbox overwrites the supplied one, but no msgArray copying is done. That can't be right. Diffs - /branches/1.8/apps/app_voicemail.c 426569 Diff: https://reviewboard.asterisk.org/r/4126/diff/ Testing --- The reporter -- Nick Adams -- has run this patch in production for a number of months now, without issues. Thanks, wdoekes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4038: Testsuite: Process REF_DEBUG logs, fail any test that leaks
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4038/#review13606 --- Ship it! LGTM. /asterisk/trunk/runtests.py https://reviewboard.asterisk.org/r/4038/#comment24120 lstrip usage is wrong. /asterisk/trunk/runtests.py https://reviewboard.asterisk.org/r/4038/#comment24119 lstrip() does not do what it looks like. I thought someone fixed this everywhere already? - wdoekes On Oct. 29, 2014, 9:47 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4038/ --- (Updated Oct. 29, 2014, 9:47 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24379 https://issues.asterisk.org/jira/browse/ASTERISK-24379 Repository: testsuite Description --- This causes any test that leaks references to fail if REF_DEBUG is enabled. Additionally run-local is modified to allow REF_DEBUG to be enabled through setup: MENUSELECT_OPTIONS='--enable REF_DEBUG' ./run-local setup Note if this option is used with Asterisk 1.8 all tests will fail due to manager.c leaking the sessions container. Diffs - /asterisk/trunk/runtests.py 5803 /asterisk/trunk/run-local 5803 /asterisk/trunk/contrib/scripts/refleaks-summary PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4038/diff/ Testing --- Ran against tests/channels/SIP/route on Asterisk 11 with and without r4037 applied. Test fails without, passes with. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4094: Install refcounter.py to /var/lib/asterisk/scripts
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4094/#review13607 --- Ship it! Ship It! - wdoekes On Oct. 17, 2014, 11:16 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4094/ --- (Updated Oct. 17, 2014, 11:16 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24432 https://issues.asterisk.org/jira/browse/ASTERISK-24432 Repository: Asterisk Description --- This change causes refcounter.py to be installed to /var/lib/asterisk/scripts when REF_DEBUG is enabled. Diffs - /branches/1.8/contrib/Makefile PRE-CREATION /branches/1.8/Makefile 425404 Diff: https://reviewboard.asterisk.org/r/4094/diff/ Testing --- make install and make uninstall with and without REF_DEBUG enabled. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4117: Fix building chan_phone on big endian systems
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4117/ --- (Updated Oct. 29, 2014, 5:33 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 426570 Bugs: https://issues.asterisk.org/jira/browse/ASTERISK-24458 https://issues.asterisk.org/jira/browse/https://issues.asterisk.org/jira/browse/ASTERISK-24458 Repository: Asterisk Description --- A left over from the formats conversion. Note: there seem to be a few other left-over AST_FORMAT_SLINEAR, mostly in comments. Fix those as well? Diffs - /branches/13/channels/chan_phone.c 426440 Diff: https://reviewboard.asterisk.org/r/4117/diff/ Testing --- Big endian platforms (sparc, powerpc, s390x) on buildd.debian.org now build. Thanks, Tzafrir Cohen -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload
On Oct. 29, 2014, 5:18 a.m., wdoekes wrote: /asterisk/trunk/tests/pbx/dialplan_reload/run-test, line 64 https://reviewboard.asterisk.org/r/4122/diff/1/?file=68588#file68588line64 Is it needed to callLater() instead of just calling it? Applies to the other 0-second callLaters too. You'll defer handling soon enough. The only other example where I find a callLater(0, in the testsuite, is broken anyway: reactor.callLater(0, self.launch_test()) # tests/fastagi/record-file/run-test Also removed other calls to reactor.callLater. On Oct. 29, 2014, 5:18 a.m., wdoekes wrote: /asterisk/trunk/tests/pbx/dialplan_reload/run-test, line 74 https://reviewboard.asterisk.org/r/4122/diff/1/?file=68588#file68588line74 Why do we wait (an arbitrary) 3 seconds for this? Can't we call the fullybooted_run immediately? I think it might be something to do with our running 'restart'. At a certain point the CLI is not available, so fullybooted_run fails without the delay (or with 1 second delay). Maybe it could be decreased to 2 seconds but that would make the test more sensitive. On Oct. 29, 2014, 5:18 a.m., Corey Farrell wrote: Lastly: PEP says space after a comma, please. All other results of pep8 in this file also fixed. - Corey --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4122/#review13604 --- On Oct. 28, 2014, 6:20 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4122/ --- (Updated Oct. 28, 2014, 6:20 p.m.) Review request for Asterisk Developers and Scott Griepentrog. Repository: testsuite Description --- In some configurations 3 seconds is not enough of a delay before Asterisk is fully booted, preventing core restart gracefully from succeeding. This causes many iterations to be skipped, and in some cases the test never ends. Make use of core waitfullybooted to delay restarts. Remove unused global variables workingdir and testdir, add global variable restart_iterations to specify 50 runs. Decrease reactor_timeout from 300 to 30, use reset_timeout instead. Diffs - /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 Diff: https://reviewboard.asterisk.org/r/4122/diff/ Testing --- Yes Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4122/ --- (Updated Oct. 29, 2014, 7:27 a.m.) Review request for Asterisk Developers and Scott Griepentrog. Repository: testsuite Description --- In some configurations 3 seconds is not enough of a delay before Asterisk is fully booted, preventing core restart gracefully from succeeding. This causes many iterations to be skipped, and in some cases the test never ends. Make use of core waitfullybooted to delay restarts. Remove unused global variables workingdir and testdir, add global variable restart_iterations to specify 50 runs. Decrease reactor_timeout from 300 to 30, use reset_timeout instead. Diffs (updated) - /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 Diff: https://reviewboard.asterisk.org/r/4122/diff/ Testing --- Yes Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/#review13611 --- I suggest reusing the 'media_encryption' pjsip.conf option with possible values of 'yes', 'no', 'attempt'/'try' instead of a new option. If not I suggest renaming the option to something like 'media_encryption_attempt' or 'media_encryption_try'. - jbigelow On Oct. 21, 2014, 8:36 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- (Updated Oct. 21, 2014, 8:36 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs - /trunk/res/res_pjsip_session.c 426078 /trunk/res/res_pjsip_sdp_rtp.c 426078 /trunk/res/res_pjsip/pjsip_configuration.c 426078 /trunk/res/res_pjsip.c 426078 /trunk/include/asterisk/res_pjsip_session.h 426078 /trunk/include/asterisk/res_pjsip.h 426078 /trunk/contrib/ast-db-manage/config/versions/1443687dda65_add_media_encryption_optimistic_to_pjsip.py PRE-CREATION /trunk/configs/samples/pjsip.conf.sample 426078 /trunk/CHANGES 426078 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
On Oct. 29, 2014, 1:49 p.m., jbigelow wrote: I suggest reusing the 'media_encryption' pjsip.conf option with possible values of 'yes', 'no', 'attempt'/'try' instead of a new option. If not I suggest renaming the option to something like 'media_encryption_attempt' or 'media_encryption_try'. media_encryption isn't a yes/no, it specifies which encryption method to use. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/#review13611 --- On Oct. 21, 2014, 1:36 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- (Updated Oct. 21, 2014, 1:36 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs - /trunk/res/res_pjsip_session.c 426078 /trunk/res/res_pjsip_sdp_rtp.c 426078 /trunk/res/res_pjsip/pjsip_configuration.c 426078 /trunk/res/res_pjsip.c 426078 /trunk/include/asterisk/res_pjsip_session.h 426078 /trunk/include/asterisk/res_pjsip.h 426078 /trunk/contrib/ast-db-manage/config/versions/1443687dda65_add_media_encryption_optimistic_to_pjsip.py PRE-CREATION /trunk/configs/samples/pjsip.conf.sample 426078 /trunk/CHANGES 426078 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] AstriDevCon 2014: Agenda item Deprecate AMI/AGI (Ben Klang)
Hi, I've been reading recent emails on the developers list and just wanted to add my 2 cents: While the AGI approach was never useful for my needs, I recently finished 2 years of almost straight development time to develop a full-fledged call answering solution and I would be VERY annoyed (to say the least) if AMI was deprecated! While many people do web programming due to it's simplicity, I find a C-based program is ALWAYS more elegant. As such, AMI was the ONLY interface to Asterisk that was useful to building the interface we needed (keep in mind that web socket support for C-based applications is VERY poor! We've researched it for another application found that we were not able to do what we needed it for!) we are just starting to enjoy the benefits of the work. I'm not saying web development doesn't have it's merits, as some applications demand it, but in my opinion a C-based program is better catered for someone using it 24-7. So please, if you want to deprecate something, don't do so to AMI! Note: While I'd have no problem myself with deprecating the dial-plan (actually, if I didn't have to deal with it at all the complexities of requiring a channel to be in async AGI mode before issuing an AMI command to it, then that would have very much simplified my development!), I can understand why a lot of people would be adverse to such a change. In summary, I think having different ways of controlling Asterisk are required, depending on the application: - AMI for those wishing to interface with a more elegant C-based application. - Something like the dial plan for those wishing to use Asterisk as is, without developing an external interface. - ARI or AGI for web-based solutions (hence why deprecating AGI probably wouldn't be negative, being that they are 2 solutions to the same ends..but DEFINITELY keep the AMI as it's purpose/use is different). Whatever you do, please keep the AMI interface! Thank You! On 10/28/2014 06:03 PM, Ben Langfeld wrote: On 28 October 2014 19:47, Derek Andrew derek.and...@usask.ca mailto:derek.and...@usask.ca wrote: What is the alternative to the dial plan? Is everyone talking about getting rid of the statements like: exten = s,1, what is the alternative? Remote applications based on APIs like ARI. This is the start of the discussion, and please remember that nothing has been decided or even presented as a robust plan yet. This is brain-storming. Additionally, note that the original proposal was to deprecate AMI/AGI in favour of ARI once it is feature complete with those protocols; an entirely lesser change than the removal of the dialplan in its entirety. -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
On Oct. 29, 2014, 8:49 a.m., jbigelow wrote: I suggest reusing the 'media_encryption' pjsip.conf option with possible values of 'yes', 'no', 'attempt'/'try' instead of a new option. If not I suggest renaming the option to something like 'media_encryption_attempt' or 'media_encryption_try'. Joshua Colp wrote: media_encryption isn't a yes/no, it specifies which encryption method to use. Ah, the sample file with 'media_encryption=no' threw me off. Could possibly have values such as 'attempt_sdes' / 'try_sdes' but then again I don't recall any other values of options being in a format like that. Just a thought. Otherwise I still suggest the option being named something like 'media_encryption_attempt' or 'media_encryption_try'. Or possibly reverse the name to 'media_encryption_force' with a default of 'yes'. - jbigelow --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/#review13611 --- On Oct. 21, 2014, 8:36 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- (Updated Oct. 21, 2014, 8:36 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs - /trunk/res/res_pjsip_session.c 426078 /trunk/res/res_pjsip_sdp_rtp.c 426078 /trunk/res/res_pjsip/pjsip_configuration.c 426078 /trunk/res/res_pjsip.c 426078 /trunk/include/asterisk/res_pjsip_session.h 426078 /trunk/include/asterisk/res_pjsip.h 426078 /trunk/contrib/ast-db-manage/config/versions/1443687dda65_add_media_encryption_optimistic_to_pjsip.py PRE-CREATION /trunk/configs/samples/pjsip.conf.sample 426078 /trunk/CHANGES 426078 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/#review13614 --- /branches/1.8/apps/app_voicemail.c https://reviewboard.asterisk.org/r/4126/#comment24127 The struct vm_state in vm_execmain is allocated on the stack. init_vm_state is called on it if IMAP_STORAGE is defined, regardless of the value of vmu. However, we only call vmstate_delete if vmu was allocated - so this may introduce a potential memory leak (on top of the ones you've already pointed out). - Matt Jordan On Oct. 29, 2014, 4:53 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/ --- (Updated Oct. 29, 2014, 4:53 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24190 https://issues.asterisk.org/jira/browse/ASTERISK-24190 Repository: Asterisk Description --- In update_messages_by_imapuser(), messages were appended without checking bounds: vms-msgArray[vms-vmArrayIndex++] = number; This patch ensures that there is enough room. However, I did find quirky usage of thread local storage which I couldn't explain. Perhaps someone else can shed some light on the XXX's that I left in the code: - vms is thread-local, so it may not need to be freed. But on line 3033, it is overwritten if (strcmp(vms_p-imapuser, vmu-imapuser) || strcmp(vms_p-username, vmu-mailbox)) (or should it be freed in vmstate_delete?) - in vmstate_insert, an alternative mailbox overwrites the supplied one, but no msgArray copying is done. That can't be right. Diffs - /branches/1.8/apps/app_voicemail.c 426569 Diff: https://reviewboard.asterisk.org/r/4126/diff/ Testing --- The reporter -- Nick Adams -- has run this patch in production for a number of months now, without issues. Thanks, wdoekes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/#review13610 --- /branches/1.8/apps/app_voicemail.c https://reviewboard.asterisk.org/r/4126/#comment24124 You shouldn't need the error message here - ast_realloc will log an error message already if the allocation fails. /branches/1.8/apps/app_voicemail.c https://reviewboard.asterisk.org/r/4126/#comment24125 Looking at the code, I'm not sure how it could be free'd on all possible code paths. Clearly if it gets associated with thread local storage, it will be free'd appropriately. That only happens however in vm_execmain (which, oddly enough, associates a structure on the stack with thread local storage, then clears it upon exiting to prevent an attempt to free memory on the stack). Other then that, I don't see where memory allocated here is free'd. I didn't find: (1) An instance where a caller of create_vm_state_from_user free'd memory. This is probably appropriate, however, since the caller of create_vm_state_from_user does not own the memory returned (it is either thread local storage or it is stored in the vmstates list) (2) When we remove an entry from vmstates in vmstate_delete, I did not see us actually destroy the vms instance. Given the somewhat vague ownership semantics surrounding this memory - it can be stack allocated, dynamically allocated and assigned to thread local storage/stored in a linked list - I'm hesitant to recommend anything here right now. It's probably worth opening a separate issue for this however. /branches/1.8/apps/app_voicemail.c https://reviewboard.asterisk.org/r/4126/#comment24126 This seems fishy as well. I'm not sure what the point of copying vmArrayIndex would be if you don't also have the msgArray. - Matt Jordan On Oct. 29, 2014, 4:53 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/ --- (Updated Oct. 29, 2014, 4:53 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24190 https://issues.asterisk.org/jira/browse/ASTERISK-24190 Repository: Asterisk Description --- In update_messages_by_imapuser(), messages were appended without checking bounds: vms-msgArray[vms-vmArrayIndex++] = number; This patch ensures that there is enough room. However, I did find quirky usage of thread local storage which I couldn't explain. Perhaps someone else can shed some light on the XXX's that I left in the code: - vms is thread-local, so it may not need to be freed. But on line 3033, it is overwritten if (strcmp(vms_p-imapuser, vmu-imapuser) || strcmp(vms_p-username, vmu-mailbox)) (or should it be freed in vmstate_delete?) - in vmstate_insert, an alternative mailbox overwrites the supplied one, but no msgArray copying is done. That can't be right. Diffs - /branches/1.8/apps/app_voicemail.c 426569 Diff: https://reviewboard.asterisk.org/r/4126/diff/ Testing --- The reporter -- Nick Adams -- has run this patch in production for a number of months now, without issues. Thanks, wdoekes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4124: audiohooks: Clean references to formats
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4124/#review13616 --- Ship it! Ship It! - Matt Jordan On Oct. 28, 2014, 11:22 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4124/ --- (Updated Oct. 28, 2014, 11:22 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24465 https://issues.asterisk.org/jira/browse/ASTERISK-24465 Repository: Asterisk Description --- Cleanup references to in_translate[x].format / out_translate[x].format in ast_audiohook_detach_list. Diffs - /branches/13/main/audiohook.c 426528 Diff: https://reviewboard.asterisk.org/r/4124/diff/ Testing --- Yes Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4125: app_queue: fix a couple leaks to struct call_queue in set_member_value
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4125/#review13615 --- Ship it! One more queue_unref(q) needed and this should be good to go. /branches/11/apps/app_queue.c https://reviewboard.asterisk.org/r/4125/#comment24128 Glancing at the nearby code I noticed another leak. - Kevin Harwell On Oct. 28, 2014, 11:52 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4125/ --- (Updated Oct. 28, 2014, 11:52 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24466 https://issues.asterisk.org/jira/browse/ASTERISK-24466 Repository: Asterisk Description --- set_member_value has a couple leaks to references in the variable q found through testsuite tests/queues/set_penalty. This change also removes the REF_DEBUG_ONLY_QUEUES compiler declaration, this is no longer possible with the updated REF_DEBUG code. Diffs - /branches/11/apps/app_queue.c 426569 Diff: https://reviewboard.asterisk.org/r/4125/diff/ Testing --- All tests/queues/set_penalty no longer leaks any references (verifies first added queue_unref). I'm unsure if the second added queue_unref has been tested, but seems like it is needed. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4121: testsuite: Close ARI websocket connections before stopping reactor
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4121/#review13617 --- Ship it! Ship It! - Matt Jordan On Oct. 28, 2014, 4:19 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4121/ --- (Updated Oct. 28, 2014, 4:19 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- All (or most) tests in tests/rest_api leak numerous referenced objects by not closing the ARI websocket connection. Diffs - /asterisk/trunk/lib/python/asterisk/ari.py 5796 Diff: https://reviewboard.asterisk.org/r/4121/diff/ Testing --- Using r4038 Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.
On Oct. 29, 2014, 2:41 p.m., Matt Jordan wrote:
/branches/1.8/apps/app_voicemail.c, lines 3233-3243
https://reviewboard.asterisk.org/r/4126/diff/1/?file=68594#file68594line3233
The struct vm_state in vm_execmain is allocated on the stack.
init_vm_state is called on it if IMAP_STORAGE is defined, regardless of the
value of vmu. However, we only call vmstate_delete if vmu was allocated -
so this may introduce a potential memory leak (on top of the ones you've
already pointed out).
As far as I can tell, we won't get there unless vmu is non-zero:
/* Let's hope that this means that if (valid) then (vmu) */
if (!valid) {
goto out;
}
Otherwise we would crash here anyway:
/* Set language from config to override channel language */
if (!ast_strlen_zero(vmu-language))
ast_string_field_set(chan, language, vmu-language);
And we would (have) leak(ed) the mutex too.
Looks like it, based on this too:
if (valid) {
int new = 0, old = 0, urgent = 0;
snprintf(ext_context, sizeof(ext_context), %s@%s,
vms.username, vmu-context);
- wdoekes
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4126/#review13614
---
On Oct. 29, 2014, 9:53 a.m., wdoekes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4126/
---
(Updated Oct. 29, 2014, 9:53 a.m.)
Review request for Asterisk Developers.
Bugs: ASTERISK-24190
https://issues.asterisk.org/jira/browse/ASTERISK-24190
Repository: Asterisk
Description
---
In update_messages_by_imapuser(), messages were appended without checking
bounds:
vms-msgArray[vms-vmArrayIndex++] = number;
This patch ensures that there is enough room.
However, I did find quirky usage of thread local storage which I couldn't
explain. Perhaps someone else can shed some light on the XXX's that I left in
the code:
- vms is thread-local, so it may not need to be freed. But on line 3033, it
is overwritten if (strcmp(vms_p-imapuser, vmu-imapuser) ||
strcmp(vms_p-username, vmu-mailbox))
(or should it be freed in vmstate_delete?)
- in vmstate_insert, an alternative mailbox overwrites the supplied one, but
no msgArray copying is done. That can't be right.
Diffs
-
/branches/1.8/apps/app_voicemail.c 426569
Diff: https://reviewboard.asterisk.org/r/4126/diff/
Testing
---
The reporter -- Nick Adams -- has run this patch in production for a number
of months now, without issues.
Thanks,
wdoekes
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/ --- (Updated Oct. 29, 2014, 3:40 p.m.) Review request for Asterisk Developers. Changes --- Removed LOG_ERROR about not enough mem. Bugs: ASTERISK-24190 https://issues.asterisk.org/jira/browse/ASTERISK-24190 Repository: Asterisk Description --- In update_messages_by_imapuser(), messages were appended without checking bounds: vms-msgArray[vms-vmArrayIndex++] = number; This patch ensures that there is enough room. However, I did find quirky usage of thread local storage which I couldn't explain. Perhaps someone else can shed some light on the XXX's that I left in the code: - vms is thread-local, so it may not need to be freed. But on line 3033, it is overwritten if (strcmp(vms_p-imapuser, vmu-imapuser) || strcmp(vms_p-username, vmu-mailbox)) (or should it be freed in vmstate_delete?) - in vmstate_insert, an alternative mailbox overwrites the supplied one, but no msgArray copying is done. That can't be right. Diffs (updated) - /branches/1.8/apps/app_voicemail.c 426593 Diff: https://reviewboard.asterisk.org/r/4126/diff/ Testing --- The reporter -- Nick Adams -- has run this patch in production for a number of months now, without issues. Thanks, wdoekes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4119: pjsip: handle outbound unregister correctly
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4119/#review13619 --- Ship it! /branches/12/res/res_pjsip_outbound_registration.c https://reviewboard.asterisk.org/r/4119/#comment24130 s/it's/its - Kevin Harwell On Oct. 28, 2014, 3:03 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4119/ --- (Updated Oct. 28, 2014, 3:03 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24411 https://issues.asterisk.org/jira/browse/ASTERISK-24411 Repository: Asterisk Description --- Patch from John Bigelow: This patch sets the status of the outbound registration to reflect when it has been unregistered. Since the registration is unregistered rather than stopped, the registration schedule remains active as before. The patch also updates the documentation of both the AMI and CLI commands. Diffs - /branches/12/res/res_pjsip_outbound_registration.c 426523 Diff: https://reviewboard.asterisk.org/r/4119/diff/ Testing --- Previously failing test channels/pjsip/registration/outbound/unregister/unauthed now passes. Other pjsip tests that were passing still pass. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 2964: res_pjsip_outbound_registration: Add virtual line support for automatic inbound matching
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2964/#review13620 --- Ship it! Ship It! - opticron On Oct. 10, 2014, 8:18 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2964/ --- (Updated Oct. 10, 2014, 8:18 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This patch adds virtual line support to the res_pjsip_outbound_registration module. This is an optional feature and simply adds a line URI parameter to the Contact we place in the outbound registration. If this line parameter is present on incoming requests we use it to establish a relationship to the outbound registration and match it to a user configured endpoint. This has the benefit that when registering to another server where it is supported you no longer have to do IP based matching for all of their potential servers. The downside (and why this is optional) is that if a third party got the line parameter they could send you calls as if they were the legit remote server. Diffs - /trunk/res/res_pjsip_outbound_registration.c 425156 /trunk/configs/samples/pjsip.conf.sample 425156 Diff: https://reviewboard.asterisk.org/r/2964/diff/ Testing --- Registered to an ITSP, placed an inbound call from them, confirmed matched using line parameter. Registered to a chan_sip instance, placed an inbound call from it, confirmed matched using line parameter. Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4099: Optimistic SRTP Tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4099/#review13621 --- /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_optimistic_offer/sipp/offer.xml https://reviewboard.asterisk.org/r/4099/#comment24131 The other three new tests should have this type of check in the 200 response as well. - opticron On Oct. 21, 2014, 8:49 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4099/ --- (Updated Oct. 21, 2014, 8:49 a.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This change removes 1 SIPP scenario from the old SRTP negotiation tests which would fail (because optimistic is now supported) and adds 4 new tests to cover the new optimistic support. These test do: 1. Asterisk is configured with mandatory encryption and receives an offer with optimistic, it accepts the offer. 2. Asterisk is configured with optimistic encryption and receives an offer with optimistic, it accepts the offer. 3. Asterisk is configured with optimistic encryption and receives an offer with mandatory, it accepts the offer. 4. Asterisk is configured with optimistic encryption and receives an offer without any crypto, it accepts the offer. The other SRTP negotiation tests cover the mandatory situations and other assorted crypto stuff. Diffs - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5766 /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/test-config.yaml 5766 /asterisk/trunk/tests/channels/pjsip/srtp_negotiation/sipp/decline_not_enabled.xml 5766 /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_optimistic_offer/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_optimistic_offer/sipp/offer.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_optimistic_offer/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_optimistic_offer/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_no_crypto/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_no_crypto/sipp/offer.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_no_crypto/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_no_crypto/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_mandatory_offer/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_mandatory_offer/sipp/offer.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_mandatory_offer/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/optimistic_with_mandatory_offer/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/mandatory_with_optimistic_offer/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/mandatory_with_optimistic_offer/sipp/offer.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/mandatory_with_optimistic_offer/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/optimistic_srtp/mandatory_with_optimistic_offer/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4099/diff/ Testing --- Ran tests, confirmed happy. Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/#review13622 --- Ship it! Ship It! - Matt Jordan On Oct. 29, 2014, 10:40 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4126/ --- (Updated Oct. 29, 2014, 10:40 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24190 https://issues.asterisk.org/jira/browse/ASTERISK-24190 Repository: Asterisk Description --- In update_messages_by_imapuser(), messages were appended without checking bounds: vms-msgArray[vms-vmArrayIndex++] = number; This patch ensures that there is enough room. However, I did find quirky usage of thread local storage which I couldn't explain. Perhaps someone else can shed some light on the XXX's that I left in the code: - vms is thread-local, so it may not need to be freed. But on line 3033, it is overwritten if (strcmp(vms_p-imapuser, vmu-imapuser) || strcmp(vms_p-username, vmu-mailbox)) (or should it be freed in vmstate_delete?) - in vmstate_insert, an alternative mailbox overwrites the supplied one, but no msgArray copying is done. That can't be right. Diffs - /branches/1.8/apps/app_voicemail.c 426593 Diff: https://reviewboard.asterisk.org/r/4126/diff/ Testing --- The reporter -- Nick Adams -- has run this patch in production for a number of months now, without issues. Thanks, wdoekes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] AstriDevCon 2014: Agenda item Deprecate AMI/AGI (Ben Klang)
On Oct 28, 2014, at 5:03 PM, Ben Langfeld b...@langfeld.me wrote: On 28 October 2014 19:47, Derek Andrew derek.and...@usask.ca wrote: What is the alternative to the dial plan? Is everyone talking about getting rid of the statements like: exten = s,1, what is the alternative? Remote applications based on APIs like ARI. This is the start of the discussion, and please remember that nothing has been decided or even presented as a robust plan yet. This is brain-storming. We’re not at the start of the “discussion” to deprecate the dial plan. The start of the “discussion” began when some developers decided to try standing Asterisk on its head by adding “asynchronous AGI.” Evidently, that was good so then they continued the “discussion” by adding ARI/Stasis. Now the “discussion” is in full career as ARI/Stasis has metastasized beyond its original scope to encompass all of Asterisk. None of said “discussion” ever happened on the lists nor was the broader Asterisk community involved as far as I can determine. A parallel “discussion” was started by a shill at AstiCon this year to begin to get the “vast unwashed” onboard with ARI/Stasis, that is, so that Matt could come back from AstiCon claiming that the broader Asterisk community is in agreement that ARI/Stasis is the future of Asterisk and that the dial plan can be deprecated. The inevitable result of these parallel paths is a completely predictable train wreck when the developers designing features that users don’t want crash into users who have been using Asterisk as originally designed. Additionally, note that the original proposal was to deprecate AMI/AGI in favour of ARI once it is feature complete with those protocols; an entirely lesser change than the removal of the dialplan in its entirety. So you're saying that deprecating the dial plan is not on the table? How then do you explain statements like this: Leif: we're in a transition, moving from dialplan model to external control model. Probably need external application to be built for us to move completely away from AMI/AGI.” or this Paul: take away apps, and whatever is in the core is what we should care about.” -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] AstriDevCon 2014: Agenda item Deprecate AMI/AGI (Ben Klang)
On 10/28/2014 06:03 PM, Ben Langfeld wrote: On 28 October 2014 19:47, Derek Andrew derek.and...@usask.ca mailto:derek.and...@usask.ca wrote: What is the alternative to the dial plan? Is everyone talking about getting rid of the statements like: exten = s,1, what is the alternative? Remote applications based on APIs like ARI. This is the start of the discussion, and please remember that nothing has been decided or even presented as a robust plan yet. This is brain-storming. Additionally, note that the original proposal was to deprecate AMI/AGI in favour of ARI once it is feature complete with those protocols; an entirely lesser change than the removal of the dialplan in its entirety. Since this thread has my name on it, I guess it’s past time that I explain my motivation for making the suggestion, and try to restore some of the context that was present in the discussion at AstriDevCon. Before I jump into the details of my proposal, I’d like to clarify terms: * Deprecating something means that the project decides to no longer recommend its use. It does NOT mean immediate removal. As others on the list have pointed out, Asterisk 13 is out and will be supported for 5 years. Anything that is *deprecated* in Asterisk 14 or even 15 is likely to still be *supported*, just *not recommended*. That means that anything we decide to deprecate today is likely to be available for at least 7 years (2 years to next LTS release + 5 years of support). Given the excellent history of the Asterisk project at being backward compatible, it may even be longer than that. I’d say exactly how long depends on the community and the interest in maintaining it. * Removing something means it is gone from Asterisk. I made no proposals to remove anything, only to deprecate. Deprecating things is an important function of software projects. It allows us to gradually cut ties on old functionality that has been superseded and to focus development efforts and available resources on the replacement features. Deprecating gives the community a chance to communicate that at some point in the future, a feature will stop working. Until that time, when the deprecation graduates to removal, the feature is still supported. If we never deprecated anything, the project would eventually grind to a halt because we would spend all our time making sure that each new feature was compatible all the way back to Asterisk 1.0. Clearly there’s a middle ground. I’d like to consider deprecating features that can have viable replacements so we can appropriately focus our limited community resources. Now, on to what I originally proposed. I don’t *think* anyone actually recommended deprecating extensions.conf. There was a suggestion (by Leif I think, and in any event, I agree with it) that it might be nice to have the ability to control calls in Asterisk that never touch the extensions.conf. Control would come via ARI from external applications. Not having to configure the dialplan basically saves a step and makes it ever so slightly easier for application developers who don’t care about extensions.conf. From my perspective, and that of many Adhearsion applications, our extensions.conf is essentially empty except for the line that routes the call to AGI. For anyone who doesn’t want to use ARI, extensions.conf would continue to be available. I also agree with other posters to this thread who argue that not everything needs to be handled by an external application. For those cases, extensions.conf is sufficient. I am not in favor of deprecating or removing extensions.conf at this time. However, I most certainly did propose to deprecate AMI and AGI. As protocols they have several significant drawbacks: * AMI has historically had many problems with internal consistency. While that has improved dramatically, it’s still a difficult protocol to write generic parsers for due to the amount of state you have to track and the edge cases you have to consider. * AGI’s has two fatal flaws: 1) it is blocking. Once you start a Dial or Playback, you can’t do anything else until it completes. 2) It depends on Dialplan applications - if there isn’t a dialplan application, you can’t do it. * AGI additionally suffers from the fact that its ability to return information is severely limited. Each AGI command results in a single code (which isn’t consistently used to indicate error or success) and many of the actually useful pieces of information you want returned from AGI actually have to come in the form of channel variables. Channel variables aren’t technically part of AGI at all, but rather the responsibility of the dialplan application that created them. This makes documentation difficult. * AGI (and dialplan) have hard-coded limits of 1024 bytes of input. This causes all kinds of breakage when you need to pass more data, such as a long text-to-speech string * AMI and AGI were not invented
Re: [asterisk-dev] [Code Review] 4125: app_queue: fix a couple leaks to struct call_queue in set_member_value
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4125/ --- (Updated Oct. 29, 2014, 4:33 p.m.) Review request for Asterisk Developers. Changes --- Add one more queue_unref Bugs: ASTERISK-24466 https://issues.asterisk.org/jira/browse/ASTERISK-24466 Repository: Asterisk Description --- set_member_value has a couple leaks to references in the variable q found through testsuite tests/queues/set_penalty. This change also removes the REF_DEBUG_ONLY_QUEUES compiler declaration, this is no longer possible with the updated REF_DEBUG code. Diffs (updated) - /branches/11/apps/app_queue.c 426569 Diff: https://reviewboard.asterisk.org/r/4125/diff/ Testing --- All tests/queues/set_penalty no longer leaks any references (verifies first added queue_unref). I'm unsure if the second added queue_unref has been tested, but seems like it is needed. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4101/#review13624 --- /trunk/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4101/#comment24137 This should be defined within the scope of the if block below. /trunk/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4101/#comment24138 This needs a ast_channel_unref after finding the label. /trunk/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4101/#comment24135 This debug message should be removed in the final version of this patch. /trunk/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4101/#comment24134 This should check for a return of -1 and emit the appropriate error message if the priority of the label can not be found. /trunk/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4101/#comment24136 Same comment about this debug message. /trunk/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4101/#comment24139 Same comment on the debug statements here. - opticron On Oct. 21, 2014, 12:50 p.m., greenfieldtech wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4101/ --- (Updated Oct. 21, 2014, 12:50 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- This patch changes the current behavior of ARI, to allow channel originate/continue requests to be performed with labels as the priority, not only integer values. Diffs - /trunk/rest-api/api-docs/channels.json 425359 /trunk/res/res_ari_channels.c 425359 /trunk/res/ari/resource_channels.c 425359 /trunk/res/ari/resource_channels.h 425359 Diff: https://reviewboard.asterisk.org/r/4101/diff/ Testing --- Testing was performed by testing the following scenarios: 1. Originating a call to a numeric priority - works 2. Originating a call to a null priority - works 3. Originating a call to a label - works 4. Continue a call to a label - not tested yet Thanks, greenfieldtech -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4116: res_pjsip: incorrect qualify statistics after disabling for contact
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4116/#review13625 --- Ship it! Ship It! - opticron On Oct. 27, 2014, 4:43 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4116/ --- (Updated Oct. 27, 2014, 4:43 p.m.) Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-24462 https://issues.asterisk.org/jira/browse/ASTERISK-24462 Repository: Asterisk Description --- When removing the qualify_frequency from an AoR or a contact the statistics shown when issuing pjsip show aors from the CLI are incorrect. This patch deletes the contact's status object from sorcery, disassociating it from the contact, if the qualify_freqency is removed from configuration. Diffs - branches/12/res/res_pjsip/pjsip_options.c 426251 Diff: https://reviewboard.asterisk.org/r/4116/diff/ Testing --- Using static and dynamic contacts and various combinations of adding, removing, and reloading the configuration for both AoR and contact level qualify_freqency options noted that the qualify statistics are now correctly reflected. Thanks, Kevin Harwell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4120: res_pjsip_acl: contact ACL permits are being interpreted incorrectly
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4120/#review13626 --- Ship it! Ship It! - opticron On Oct. 28, 2014, 2:45 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4120/ --- (Updated Oct. 28, 2014, 2:45 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- In the attempt to skip past the 'contact' part of the variable name before passing it into the acl handler, we have a momentary lapse of sanity and forget that '_allow' isn't 'allow' and ast_append_acl interprets it as another deny. Diffs - /branches/12/res/res_pjsip_acl.c 426232 Diff: https://reviewboard.asterisk.org/r/4120/diff/ Testing --- deny=0.0.0.0/24 allow=ip address of a device that tries to register Note that I have to reload pjsip after startup in order for the ACL to work... that seems like a bug surely. In any event, with the patch the device successfully registers. Without the patch, the registration is blocked by the ACL. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] AstriDevCon 2014: Agenda item Deprecate AMI/AGI (Ben Klang)
On Wed, Oct 29, 2014 at 12:31 PM, Paul Albrecht palbre...@glccom.com wrote: None of said “discussion” ever happened on the lists nor was the broader Asterisk community involved as far as I can determine. A parallel “discussion” was started by a shill at AstiCon this year to begin to get the “vast unwashed” onboard with ARI/Stasis, that is, so that Matt could come back from AstiCon claiming that the broader Asterisk community is in agreement that ARI/Stasis is the future of Asterisk and that the dial plan can be deprecated. I'm not going to take the time to comment on every trivial detail here, but I'd be remiss if I didn't highight a few things. First, Asterisk development (including ARI) happened in the open. It's been discussed at the Asterisk Developer Conferences, on the mailing lists, etc. While some people have suggested that they'd like to move completely to ARI, I personally don't think that we'll see a complete move away from the dialplan anytime in the next ten years. That being said, I'm glad that the people who *want* to move away from the dialplan are more easily able to do so now. Part of that stems from the fact that Asterisk has traditionally been a PBX, and is now moving to also be a media engine. For some of my own personal projects, I'll probably never move away from the dialplan. For others, the dialplan is something I can live without. While the majority of the people in the room at AstriDevCon were in agreement that ARI/Stasis *is* part of the future of Asterisk, I don't think there was anywhere close to a majority that thought the dial plan can be deprecated. Second, let me state that attacking the people in this channel doesn't help your case -- let's keep things civil here and debate *what* is right, not *who is right*. By using inflammatory language, you're just making other developers in this channel *less* likely to take you seriously, not more likely. I'd personally never want this channel to just become an echo chamber, but it does need to be safe space where people feel like they can share their opinions and ideas. If people feel threatened or belittled, they're much more likely to be hostile to your opinions or ideas. -- Jared Smith -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4120: res_pjsip_acl: contact ACL permits are being interpreted incorrectly
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4120/#review13627 --- As an addendum to opticron's Ship It: This needs a test. - Matt Jordan On Oct. 28, 2014, 2:45 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4120/ --- (Updated Oct. 28, 2014, 2:45 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- In the attempt to skip past the 'contact' part of the variable name before passing it into the acl handler, we have a momentary lapse of sanity and forget that '_allow' isn't 'allow' and ast_append_acl interprets it as another deny. Diffs - /branches/12/res/res_pjsip_acl.c 426232 Diff: https://reviewboard.asterisk.org/r/4120/diff/ Testing --- deny=0.0.0.0/24 allow=ip address of a device that tries to register Note that I have to reload pjsip after startup in order for the ACL to work... that seems like a bug surely. In any event, with the patch the device successfully registers. Without the patch, the registration is blocked by the ACL. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4107: Wiki: Some new PJSIP-related pages
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4107/#review13623 --- Configuring res_pjsip for Presence Subscriptions Under the Configuration section, 3rd paragraph, the context option description - (just to be sure) the subscribing endpoint's context must be set to that of the hint? - Resource List Subscriptions (RLS) Under Batching Notifications, 1st paragraph the following sentence fragment sounds a little strange: ...any further state changes of states in the list ... - Configuring Outbound Registrations Under the Authentication last sentence: Details about what options are available in auth sections can be found here. -- I'm guessing here was meant to be a link? Also, the Authentication section is under Dealing with Failure. I'd probably move the Dealing with Failure section below the Authentication part. Under Manually Unregistering I'd probably add a note that after manually unregistering the specified outbound registration will continue attempting re-registers based on it last registration expiration. - Asterisk PJSIP Troubleshooting Guide There is a gap between the Overview section and the next. Can this be shrunk or is it due to some kind of wiki formatting thing? Personally, I'd lean toward breaking the troubleshooting guide into sub-pages. Especially if you think more will be added later as things come up. Breaking things up would certainly make it easier to find a particular section when navigating via the content tree. - Kevin Harwell On Oct. 22, 2014, 4:37 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4107/ --- (Updated Oct. 22, 2014, 4:37 p.m.) Review request for Asterisk Developers. Description --- I have written the following wiki pages: https://wiki.asterisk.org/wiki/display/AST/Configuring+res_pjsip+for+Presence+Subscriptions https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=30278158 https://wiki.asterisk.org/wiki/display/AST/Configuring+Outbound+Registrations https://wiki.asterisk.org/wiki/display/AST/Asterisk+PJSIP+Troubleshooting+Guide The first three pages are tutorial pages for Asterisk PJSIP usage. The first focuses on setting up presence subscriptions, the second focuses on setting up resource list subscriptions, and the third focuses on configuring outbound registrations. The fourth page is a general PJSIP troubleshooting guide. The intent of this page is not to be exhaustive at the moment, since this is a page that likely will be updated frequently as specific issues are encountered. This page may need to be split into multiple pages, but I'll leave that judgment to the reviewers. Diffs - Diff: https://reviewboard.asterisk.org/r/4107/diff/ Testing --- Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4114: Prevent stringfields from accumulating unused memory
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4114/#review13628 --- 1) In stringfields.h:ast_string_field_ptr_set_by_fields(), the __p__ and ptr pointers are the same by initialization so the test for *__p__ != *ptr is always false and will not release the old string value when __ast_string_field_alloc_space() allocates space for the new string value. I think this is the primary leak. 2) In utils.c:__ast_string_field_ptr_grow(), the increase of pool-used doesn't seem right. It should be increased to keep alignment similar to utils.c:__ast_string_field_alloc_space(). 3) I think a check needs to be added to utils.c:__ast_string_field_ptr_build_va() for the case when the string created by vsnprintf() is empty so the pool string can be set to the constant __ast_string_field_empty pointer. (Like is done in stringfields.h:ast_string_field_ptr_set_by_fields()) 4) All of these fixes would apply to v1.8 as well. /branches/11/main/utils.c https://reviewboard.asterisk.org/r/4114/#comment24140 This should be reverted. ptr is the string being released from the pool and __ast_string_field_empty can never be in a pool buffer by definition. /branches/11/main/utils.c https://reviewboard.asterisk.org/r/4114/#comment24141 Doing this check for every pool is overkill when you are only releasing one string from one pool. Once the string is found in a pool you don't need to continue looking in any remaining pools. Setting pool-used = 0 is a good catch for the first pool as this fixes reclaiming the space of the first pool. - rmudgett On Oct. 27, 2014, 3:20 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4114/ --- (Updated Oct. 27, 2014, 3:20 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24307 https://issues.asterisk.org/jira/browse/ASTERISK-24307 Repository: Asterisk Description --- Any time a stringfield is blanked it currently prevents any currently allocated memory from being freed. If a stringfield is repeatedly set to blank then set to a non-blank value, it causes new pools to be continuously allocated and never freed. I'm unsure if the loop can be optimized, maybe the break can be re-added to the original location on the condition that ptr == __ast_string_field_empty? Diffs - /branches/11/main/utils.c 426232 Diff: https://reviewboard.asterisk.org/r/4114/diff/ Testing --- Manual test using https://github.com/elessard1/asterisk-lab/blob/master/examples/lab_stringfields_leak.c to verify that old pools are now freed. Full testsuite against Asterisk 13. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 2478: Support multiple Require: and Supported: headers in the same request
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2478/ --- (Updated Oct. 29, 2014, 8:39 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 426594 Bugs: ASTERISK-21721 https://issues.asterisk.org/jira/browse/ASTERISK-21721 Repository: Asterisk Description --- We have servers sending multiple Supported: and Require: options in separate headers in the same message. This patch fixes that support so that we parse more than the first header found. Diffs - /trunk/channels/sip/reqresp_parser.c 414046 /trunk/channels/chan_sip.c 414046 Diff: https://reviewboard.asterisk.org/r/2478/diff/ Testing --- Tested with the server that caused the issue. Problem solved and we did reach interoperability! Thanks, Olle E Johansson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3437: chan_sip: Add support for a few more 4xx error responses
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3437/ --- (Updated Oct. 29, 2014, 8:57 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 426599 Repository: Asterisk Description --- Adding improved support for 400, 414 and 493 response codes. I would like to add this as a bug fix to 1.8 and up. Diffs - /trunk/channels/chan_sip.c 414046 Diff: https://reviewboard.asterisk.org/r/3437/diff/ Testing --- A lot during interoperability tests. Thanks, Olle E Johansson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
