#47790 Integer config attributes accept invalid values at server startup
Closed: Fixed None Opened 5 years ago by nhosoi.

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

  1. systemctl restart dirsrv@dhcp201-149

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...

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)

2 years ago

Login to comment on this ticket.

Metadata