Description of problem: When setting an attribute nsSaslMapPriority on a mapping, the value is checked for invalid values. This is being done for values x==0 and x > 100 and it creates an error/warning message in the logs and resets the value to 100. If the value in modify operation is negative, it isn't reset in the entry.
This bug doesn't seem to have an effect on the evaluation order, thus setting low severity.
Version-Release number of selected component (if applicable): 389-ds-base-1.3.1.6-25
How reproducible: always
Steps to Reproduce: 1. Set nsSaslMapPriority on a mapping entry.
Actual results: The negative value is stored silently in the configuration.
Expected results: The value stored is 100 and the server logs an error/warning message.
Your fix looks good to me.
I'm curious bout this part aside from your fix... When resetting priority fails, it's just reported if TRACE level logging is set. I guess no error is returned, too. Is it okay? {{{ 395 desc.bv_val = LOW_PRIORITY_STR; 396 desc.bv_len = strlen(desc.bv_val); 397 newval[0] = &des;; 398 if (entry_replace_values(entry, "nsSaslMapPriority", newval) != 0){ 399 LDAPDebug( LDAP_DEBUG_TRACE, "sasl_map_config_parse_entry: failed to reset priority to (%d)\n", 400 LOW_PRIORITY,0,0); 401 } else { }}}
revision 0001-Ticket-47791-Negative-value-of-nsSaslMapPriority-is-.patch
Replying to [comment:5 nhosoi]:
Your fix looks good to me. I'm curious bout this part aside from your fix... When resetting priority fails, it's just reported if TRACE level logging is set. I guess no error is returned, too. Is it okay? {{{ 395 desc.bv_val = LOW_PRIORITY_STR; 396 desc.bv_len = strlen(desc.bv_val); 397 newval[0] = &des;; 398 if (entry_replace_values(entry, "nsSaslMapPriority", newval) != 0){ 399 LDAPDebug( LDAP_DEBUG_TRACE, "sasl_map_config_parse_entry: failed to reset priority to (%d)\n", 400 LOW_PRIORITY,0,0); 401 } else { }}}
Well the current design was to allow invalid values and just assume a default value. In either case, the lowest priority is set and used by the server. I'm not sure why the one condition was only using TRACE while the other was ANY.
It should be ANY, new patch attached...
Thanks, Mark!
git merge ticket47791 Updating 805386c..43cd7db Fast-forward ldap/servers/slapd/sasl_map.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
git push origin master 805386c..43cd7db master -> master
commit 43cd7db Author: Mark Reynolds mreynolds@redhat.com Date: Fri May 30 15:50:34 2014 -0400
Metadata Update from @mreynolds: - Issue assigned to mreynolds - Issue set to the milestone: 1.3.3 backlog
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/1122
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.