Summary:	ASTERISK-26580: [patch] Error during LDAP modify action when user unregisters
Reporter:	Nicholas John Koch (aviator)	Labels:
Date Opened:	2016-11-11 06:32:46.000-0600	Date Closed:	2017-02-21 15:55:02.000-0600
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Resources/res_config_ldap
Versions:	11.24.1	Frequency of Occurrence
Related Issues:
Environment:	3.16.0-4-amd64 #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03) x86_64 GNU/Linux slapd 2.4.40+dfsg-1+deb8u2	Attachments:	( 0) res_config_ldap.c-11.24.1.patch
Description:	When an account has been looked up from the LDAP directory and activated some values within the LDAP object (AsteriskSIPUser) are updated. In this special case we are looking at _AstAccountFullContact_. When this account now unregisters the LDAP object will be updated. During this process some attributes get deleted. The bug is that the first attribute in the list is trying to be modified without a value but it should be deleted instead. Wiresharking the issue showed that the LDAP modification command has an empty value which is in terms of the LDAP syntax a fault. Hence this modification command must be a deletion. The patch checks if the first new value ist empty (mostly _AstAccountFullContact_) and therefore inserts a deletion command rather than a modification command. A similar procedure is already done some lines below but has not been implemented to the first name/value pair.
Comments:	By: Asterisk Team (asteriskteam) 2016-11-11 06:32:47.146-0600 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Rusty Newton (rnewton) 2016-11-11 08:51:00.492-0600 Thanks. Let us know if you need help getting the patch on Gerrit. The contribution process is linked in the comment above. By: Nicholas John Koch (aviator) 2016-11-11 09:27:40.393-0600 Never done this before. I will read but a hint is appreciated ;-) By: Nicholas John Koch (aviator) 2016-11-12 02:39:31.476-0600 Ok, I did succeed. It is committed. Shall I make a fix for every branch now and master? What would be the best process on this? By: Rusty Newton (rnewton) 2016-11-13 08:58:03.624-0600 Thanks! I have two comments. One is that bug fixes are no longer accepted to 11. 11 now accepts only security fixes. You can see the life cycles for the Asterisk versions here: https://wiki.asterisk.org/wiki/display/AST/Asterisk+Versions You'll want to make sure your patch works on 13 and 14, then cherry-pick it to the correct versions and remove it for 11. Cherry-pick it to the branches, not specific versioned tags. https://wiki.asterisk.org/wiki/display/AST/Code+Review The second is that the commit message could use a little modification. It should stand on its own with a description. So you might use some of what you had for the description on this ticket. See the example at the bottom of this page: https://wiki.asterisk.org/wiki/display/AST/Commit+Messages By: Nicholas John Koch (aviator) 2016-11-15 02:11:21.414-0600 Thanks for the help. So I understand that I will revert my last review commit on 11.24 and then will test my fix on 13/14 and start over the process with a more precise commit message. Please apologise my rookie business. I like contributing to asterisk and I will stick on the process now a little better. By: Rusty Newton (rnewton) 2016-11-15 11:23:52.229-0600 [~aviator] , as you go through our documentation please let us know if there is any room for improvement. We tend to error on the side of more detail rather than less detail, so our contribution process documentation can seem a little overwhelming. If you ever have questions and want to ask them in realtime please feel free to drop by #asterisk-dev on irc.freenode.net , the developers hang out there and can help you out. By: Friendly Automation (friendly-automation) 2017-02-21 15:55:02.773-0600 Change 5004 merged by zuul: res_config_ldap: Fix erroneous LDAP_MOD_REPLACE in LDAP modify [https://gerrit.asterisk.org/5004|https://gerrit.asterisk.org/5004] By: Friendly Automation (friendly-automation) 2017-02-21 17:11:57.933-0600 Change 5009 merged by zuul: res_config_ldap: Fix erroneous LDAP_MOD_REPLACE in LDAP modify [https://gerrit.asterisk.org/5009|https://gerrit.asterisk.org/5009] By: Friendly Automation (friendly-automation) 2017-02-21 17:58:25.186-0600 Change 5010 merged by zuul: res_config_ldap: Fix erroneous LDAP_MOD_REPLACE in LDAP modify [https://gerrit.asterisk.org/5010|https://gerrit.asterisk.org/5010]