#47791 Negative value of nsSaslMapPriority is not reset to lowest priority
Closed: Fixed None Opened 5 years ago by nhosoi.

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] = &desc;
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 {
}}}

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] = &desc;
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...

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

2 years ago

Login to comment on this ticket.

Metadata