Description of problem: nsslapd-ndn-cache-max-size accepts any invalid value.
Version-Release number of selected component (if applicable): 389-ds-base-1.3.1.6-25.el7.x86_64
How reproducible: Always
Steps to Reproduce: 1. ldapmodify -h localhost -p 389 -D "cn=directory manager" -w * <<EOF dn: cn=config changetype: modify replace: nsslapd-ndn-cache-max-size nsslapd-ndn-cache-max-size: ~ EOF
Actual results: DS does not give any error and operation is successful.
Expected results: Error is expected.
The input string "value" is converted to long integer by atol, which does not check the error, but returns 0 when the input is invalid. The set function then adjusts the size to the default size, and it returns SUCCESS. I think this is the right behaviour. Probably, we could log it in the error log, but I don't think we need to return an error there.
{{{ int config_set_ndn_cache_max_size(const char attrname, char value, char errorbuf, int apply ) { [...] size = atol(value); if(size < 0){ size = 0; / same as -1 */ } }}}
There was schema mistake that set nsslapd-ndn-cache-max-size as a Directory String, and not an Integer. However, manually editing the dse.ldif file allows invalid values to be set. The following patch addresses this scenario.
Not sure if we need this helper function: config_value_is_number(char *value)...
Is this not enough for checking the validity?
man strtol[l] ERRORS EINVAL (not in C99) The given base contains an unsupported value.
And I haven't checked it, but there's no param which could take a negative value?
Replying to [comment:6 nhosoi]:
Not sure if we need this helper function: config_value_is_number(char *value)... Is this not enough for checking the validity? man strtol[l] ERRORS EINVAL (not in C99) The given base contains an unsupported value. And I haven't checked it, but there's no param which could take a negative value?
Some parameters take "-1" as a value for unlimited. So strtol() complains about negative values, as does my current fix. So I have some more work to do...
Replying to [comment:7 mreynolds]:
Replying to [comment:6 nhosoi]: Not sure if we need this helper function: config_value_is_number(char *value)... Is this not enough for checking the validity? man strtol[l] ERRORS EINVAL (not in C99) The given base contains an unsupported value. And I haven't checked it, but there's no param which could take a negative value? Some parameters take "-1" as a value for unlimited. So strtol() complains about negative values, as does my current fix. So I have some more work to do...
Actually strtol() does accept negative numbers, reworking patch...
revision 0001-Ticket-47790-Integer-config-attributes-accept-invali.patch
New patch attached...
Thanks, Mark!!
git merge config_test Updating 8b305c1..d58a568 Fast-forward ldap/schema/01core389.ldif | 2 +- ldap/servers/slapd/libglobs.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------
git push origin master 8b305c1..d58a568 master -> master
commit d58a568 Author: Mark Reynolds mreynolds@redhat.com Date: Tue Jul 15 14:07:56 2014 -0400
Metadata Update from @nhosoi: - Issue assigned to mreynolds - Issue set to the milestone: 1.3.3 - 8/14 (August)
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/1121
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.