ldap/servers
by Mark Reynolds
ldap/servers/slapd/back-ldbm/monitor.c | 2
ldap/servers/slapd/dn.c | 93 ++++++++++++++++++++++++++-------
ldap/servers/slapd/main.c | 1
ldap/servers/slapd/slapi-private.h | 2
4 files changed, 78 insertions(+), 20 deletions(-)
New commits:
commit 03c90f04065059ee310e9fa7d98228e0aa39fa50
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Thu Jan 16 15:21:28 2014 -0500
Ticket 408 - Fix crash when disabling/enabling the setting
Bug Description: Enabling/disabling can lead to crash as the setting
was not designed to be dynamically updated.
Fix Description: Do not use the actual config setting to determine if the
cache is enabled. Instead we record when the cache is
initialized. The server still needs to be restarted for
the config change to take effect.
Also freed the cache at server shtudown.
https://fedorahosted.org/389/ticket/408
Reviewed by: rmeggins(Thanks!)
diff --git a/ldap/servers/slapd/back-ldbm/monitor.c b/ldap/servers/slapd/back-ldbm/monitor.c
index 3427809..409c771 100644
--- a/ldap/servers/slapd/back-ldbm/monitor.c
+++ b/ldap/servers/slapd/back-ldbm/monitor.c
@@ -146,7 +146,7 @@ int ldbm_back_monitor_instance_search(Slapi_PBlock *pb, Slapi_Entry *e,
MSET("maxDnCacheCount");
}
/* normalized dn cache stats */
- if(config_get_ndn_cache_enabled()){
+ if(ndn_cache_started()){
ndn_cache_get_stats(&hits, &tries, &size, &maxsize, &count);
sprintf(buf, "%" NSPRIu64, (long long unsigned int)tries);
MSET("normalizedDnCacheTries");
diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c
index b46a75b..e37f298 100644
--- a/ldap/servers/slapd/dn.c
+++ b/ldap/servers/slapd/dn.c
@@ -103,6 +103,7 @@ static void ndn_cache_update_lru(struct ndn_cache_lru **node);
static void ndn_cache_add(char *dn, size_t dn_len, char *ndn, size_t ndn_len);
static void ndn_cache_delete(char *dn);
static void ndn_cache_flush();
+static void ndn_cache_free();
static int ndn_started = 0;
static PRLock *lru_lock = NULL;
static Slapi_RWLock *ndn_cache_lock = NULL;
@@ -2776,7 +2777,7 @@ ndn_hash_string(const void *key)
void
ndn_cache_init()
{
- if(!config_get_ndn_cache_enabled()){
+ if(!config_get_ndn_cache_enabled() || ndn_started){
return;
}
ndn_cache_hashtable = PL_NewHashTable( NDN_CACHE_BUCKETS, ndn_hash_string, PL_CompareStrings, PL_CompareValues, 0, 0);
@@ -2789,24 +2790,49 @@ ndn_cache_init()
ndn_cache->cache_size = sizeof(struct ndn_cache_ctx) + sizeof(PLHashTable) + sizeof(PLHashTable);
ndn_cache->head = NULL;
ndn_cache->tail = NULL;
-
+ ndn_started = 1;
if ( NULL == ( lru_lock = PR_NewLock()) || NULL == ( ndn_cache_lock = slapi_new_rwlock())) {
- char *errorbuf = NULL;
- if(ndn_cache_hashtable){
- PL_HashTableDestroy(ndn_cache_hashtable);
- }
- ndn_cache_hashtable = NULL;
- config_set_ndn_cache_enabled(CONFIG_NDN_CACHE, "off", errorbuf, 1 );
- slapi_counter_destroy(&ndn_cache->cache_hits);
- slapi_counter_destroy(&ndn_cache->cache_tries);
- slapi_counter_destroy(&ndn_cache->cache_misses);
- slapi_ch_free((void **)&ndn_cache);
+ ndn_cache_destroy();
slapi_log_error( SLAPI_LOG_FATAL, "ndn_cache_init", "Failed to create locks. Disabling cache.\n" );
- } else {
- ndn_started = 1;
}
}
+void
+ndn_cache_destroy()
+{
+ char *errorbuf = NULL;
+
+ if(!ndn_started){
+ return;
+ }
+ if(lru_lock){
+ PR_DestroyLock(lru_lock);
+ lru_lock = NULL;
+ }
+ if(ndn_cache_lock){
+ slapi_destroy_rwlock(ndn_cache_lock);
+ ndn_cache_lock = NULL;
+ }
+ if(ndn_cache_hashtable){
+ ndn_cache_free();
+ PL_HashTableDestroy(ndn_cache_hashtable);
+ ndn_cache_hashtable = NULL;
+ }
+ config_set_ndn_cache_enabled(CONFIG_NDN_CACHE, "off", errorbuf, 1 );
+ slapi_counter_destroy(&ndn_cache->cache_hits);
+ slapi_counter_destroy(&ndn_cache->cache_tries);
+ slapi_counter_destroy(&ndn_cache->cache_misses);
+ slapi_ch_free((void **)&ndn_cache);
+
+ ndn_started = 0;
+}
+
+int
+ndn_cache_started()
+{
+ return ndn_started;
+}
+
/*
* Look up this dn in the ndn cache
*/
@@ -3019,19 +3045,48 @@ ndn_cache_flush()
slapi_log_error( SLAPI_LOG_CACHE, "ndn_cache_flush","Flushed cache.\n");
}
+static void
+ndn_cache_free()
+{
+ struct ndn_cache_lru *node, *next, *flush_node;
+
+ if(!ndn_cache){
+ return;
+ }
+
+ node = ndn_cache->tail;
+ while(ndn_cache->cache_count){
+ flush_node = node;
+ /* update the lru */
+ next = node->prev;
+ if(next){
+ next->next = NULL;
+ }
+ ndn_cache->tail = next;
+ node = next;
+ /* now update the hash */
+ ndn_cache->cache_count--;
+ ndn_cache_delete(flush_node->key);
+ slapi_ch_free_string(&flush_node->key);
+ slapi_ch_free((void **)&flush_node);
+ }
+}
+
/* this is already "write" locked from ndn_cache_add */
static void
ndn_cache_delete(char *dn)
{
- struct ndn_hash_val *ht_val;
+ struct ndn_hash_val *ht_entry;
- ht_val = (struct ndn_hash_val *)PL_HashTableLookupConst(ndn_cache_hashtable, dn);
- if(ht_val){
- ndn_cache->cache_size -= ht_val->size;
- slapi_ch_free_string(&ht_val->ndn);
+ ht_entry = (struct ndn_hash_val *)PL_HashTableLookupConst(ndn_cache_hashtable, dn);
+ if(ht_entry){
+ ndn_cache->cache_size -= ht_entry->size;
+ slapi_ch_free_string(&ht_entry->ndn);
+ slapi_ch_free((void **)&ht_entry);
PL_HashTableRemove(ndn_cache_hashtable, dn);
}
}
+
/* stats for monitor */
void
ndn_cache_get_stats(PRUint64 *hits, PRUint64 *tries, size_t *size, size_t *max_size, long *count)
diff --git a/ldap/servers/slapd/main.c b/ldap/servers/slapd/main.c
index f1812be..0c5b2ab 100644
--- a/ldap/servers/slapd/main.c
+++ b/ldap/servers/slapd/main.c
@@ -1280,6 +1280,7 @@ main( int argc, char **argv)
cleanup:
SSL_ShutdownServerSessionIDCache();
SSL_ClearSessionCache();
+ ndn_cache_destroy();
NSS_Shutdown();
PR_Cleanup();
#ifdef _WIN32
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
index f315f7e..fd5c54b 100644
--- a/ldap/servers/slapd/slapi-private.h
+++ b/ldap/servers/slapd/slapi-private.h
@@ -393,6 +393,8 @@ Slapi_DN *slapi_sdn_init_normdn_passin(Slapi_DN *sdn, const char *dn);
char *slapi_dn_normalize_original( char *dn );
char *slapi_dn_normalize_case_original( char *dn );
void ndn_cache_init();
+void ndn_cache_destroy();
+int ndn_cache_started();
void ndn_cache_get_stats(PRUint64 *hits, PRUint64 *tries, size_t *size, size_t *max_size, long *count);
#define NDN_DEFAULT_SIZE 20971520 /* 20mb - size of normalized dn cache */
10 years, 3 months
- 1
- 0
ldap/servers
by Mark Reynolds
ldap/servers/slapd/back-ldbm/dblayer.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
New commits:
commit 26aad754b1a559fbea02806cebf2ed7f6ab22848
Author: Mark Reynolds <mreynolds(a)redhat.com>
Date: Thu Jan 16 10:36:19 2014 -0500
Ticket 47654 - fix double free
Bug Description: Under certain circumstances the dbhome directory gets freed twice.
Fix Description: Copy the db home directory into the private db directory array, instead
of passing a reference to it.
https://fedorahosted.org/389/ticket/47654
Reviewed by: rmeggins(Thanks!)
diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c
index 2384471..9fae72b 100644
--- a/ldap/servers/slapd/back-ldbm/dblayer.c
+++ b/ldap/servers/slapd/back-ldbm/dblayer.c
@@ -1357,7 +1357,7 @@ dblayer_make_env(struct dblayer_private_env **env, struct ldbminfo *li)
if (home_dir && *home_dir &&
!charray_utf8_inlist(priv->dblayer_data_directories, home_dir))
{
- charray_add(&(priv->dblayer_data_directories), home_dir);
+ charray_add(&(priv->dblayer_data_directories), slapi_ch_strdup(home_dir));
}
/* user specified log dir */
@@ -2928,9 +2928,8 @@ int dblayer_post_close(struct ldbminfo *li, int dbmode)
charray_free(priv->dblayer_data_directories);
priv->dblayer_data_directories = NULL;
}
- if(dbmode == DBLAYER_NORMAL_MODE){
- slapi_ch_free_string(&priv->dblayer_home_directory);
- }
+ slapi_ch_free_string(&priv->dblayer_dbhome_directory);
+ slapi_ch_free_string(&priv->dblayer_home_directory);
return return_value;
}
@@ -2964,7 +2963,6 @@ int dblayer_close(struct ldbminfo *li, int dbmode)
return_value |= dblayer_instance_close(be);
}
}
- LDAPDebug1Arg(LDAP_DEBUG_ANY,"MARK dbmode %d\n",dbmode);
if (return_value != 0) {
/* force recovery next startup if any close failed */
10 years, 3 months
- 1
- 0
ldap/admin
by Richard Allen Megginson
ldap/admin/src/logconv.pl | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
New commits:
commit f83f7fe1d2d41809e9ab824e572558f881f12766
Author: Rich Megginson <rmeggins(a)redhat.com>
Date: Wed Jan 15 09:54:32 2014 -0700
Ticket #47675 logconv errors when search has invalid bind dn
https://fedorahosted.org/389/ticket/47675
Reviewed by: mreynolds (Thanks!)
Branch: master
Fix Description: Just skip processing the search request, and let the err=34
processing handle this case.
Platforms tested: RHEL6 x86_64
Flag Day: no
Doc impact: no
diff --git a/ldap/admin/src/logconv.pl b/ldap/admin/src/logconv.pl
index 7da46e0..0f82aee 100755
--- a/ldap/admin/src/logconv.pl
+++ b/ldap/admin/src/logconv.pl
@@ -2171,6 +2171,7 @@ sub parseLineNormal
}
if (($usage =~ /l/ || $verb eq "yes" || $usage =~ /u/ || $usage =~ /U/) and / SRCH /){
my ($filterConn, $filterOp);
+ $tmpp = "";
if (/ SRCH / && / attrs=/ && $_ =~ /filter=\"(.*)\" /i ){
$tmpp = $1;
$tmpp =~ tr/A-Z/a-z/;
@@ -2185,11 +2186,15 @@ sub parseLineNormal
$hashes->{filter}->{$tmpp}++;
if ($_ =~ /conn= *([0-9A-Z]+)/i) { $filterConn = $1; }
if ($_ =~ /op= *([0-9\-]+)/i) { $filterOp = $1; }
+ } elsif (/ SRCH / && /invalid dn/){
+ # this will be caught by the err=34 checking
}
- $filterCount++;
- if($usage =~ /u/ || $usage =~ /U/ || $verb eq "yes"){
- # we only need this for the unindexed search report
- $hashes->{filter_conn_op}->{"$serverRestartCount,$filterConn,$filterOp"} = $tmpp;
+ if ($tmpp) {
+ $filterCount++;
+ if($usage =~ /u/ || $usage =~ /U/ || $verb eq "yes"){
+ # we only need this for the unindexed search report
+ $hashes->{filter_conn_op}->{"$serverRestartCount,$filterConn,$filterOp"} = $tmpp;
+ }
}
}
if (($usage =~ /a/ || $verb eq "yes" || $usage =~ /u/ || $usage =~ /U/) and / SRCH /){
10 years, 3 months
- 1
- 0
dirsrvtests/suites
by Richard Allen Megginson
dirsrvtests/suites/schema/constants.py | 33 +++
dirsrvtests/suites/schema/finalizer.py | 51 ++++++
dirsrvtests/suites/schema/test_schema.py | 260 +++++++++++++++++++++++++++++++
3 files changed, 344 insertions(+)
New commits:
commit 3b049f6623288e4ff8f3eab240154206a4bc9820
Author: Rich Megginson <rmeggins(a)redhat.com>
Date: Mon Jan 6 14:20:59 2014 -0700
Ticket #47657 add schema test suite and tests for Ticket #47634
https://fedorahosted.org/389/ticket/47657
Reviewed by: tbordaz (Thanks!)
Branch: master
Fix Description: Add tests for schema ticket #47634. This also adds a suites
subdirectory to dirsrvtests and a schema subdirectory of suites.
Platforms tested: RHEL6 x86_64
Flag Day: no
Doc impact: no
diff --git a/dirsrvtests/suites/schema/constants.py b/dirsrvtests/suites/schema/constants.py
new file mode 100644
index 0000000..5751d97
--- /dev/null
+++ b/dirsrvtests/suites/schema/constants.py
@@ -0,0 +1,33 @@
+'''
+Created on Dec 18, 2013
+
+@author: rmeggins
+'''
+import os
+from lib389 import DN_DM
+from lib389._constants import *
+from lib389.properties import *
+
+SUFFIX = 'dc=example,dc=com'
+PASSWORD = 'password'
+
+# Used for standalone topology
+HOST_STANDALONE = LOCALHOST
+PORT_STANDALONE = 33389
+SERVERID_STANDALONE = 'schematest'
+
+# Each defined instance above must be added in that list
+ALL_INSTANCES = [ {SER_HOST: HOST_STANDALONE, SER_PORT: PORT_STANDALONE, SER_SERVERID_PROP: SERVERID_STANDALONE},
+ ]
+# This is a template
+args_instance = {
+ SER_DEPLOYED_DIR: os.environ.get('PREFIX', None),
+ SER_BACKUP_INST_DIR: os.environ.get('BACKUPDIR', DEFAULT_BACKUPDIR),
+ SER_ROOT_DN: DN_DM,
+ SER_ROOT_PW: PASSWORD,
+ SER_HOST: LOCALHOST,
+ SER_PORT: DEFAULT_PORT,
+ SER_SERVERID_PROP: "template",
+ SER_CREATION_SUFFIX: DEFAULT_SUFFIX}
+
+
diff --git a/dirsrvtests/suites/schema/finalizer.py b/dirsrvtests/suites/schema/finalizer.py
new file mode 100644
index 0000000..ff256f6
--- /dev/null
+++ b/dirsrvtests/suites/schema/finalizer.py
@@ -0,0 +1,51 @@
+'''
+Created on Nov 5, 2013
+
+@author: tbordaz
+'''
+import os
+import sys
+import time
+import ldap
+import logging
+import socket
+import time
+import logging
+import pytest
+from lib389 import DirSrv, Entry, tools
+from lib389.tools import DirSrvTools
+from lib389._constants import DN_DM
+from lib389.properties import *
+from constants import *
+
+log = logging.getLogger(__name__)
+
+global installation_prefix
+installation_prefix=os.getenv('PREFIX')
+
+def test_finalizer():
+ global installation_prefix
+
+ # for each defined instance, remove it
+ for args_instance in ALL_INSTANCES:
+ if installation_prefix:
+ # overwrite the environment setting
+ args_instance[SER_DEPLOYED_DIR] = installation_prefix
+
+ instance = DirSrv(verbose=True)
+ instance.allocate(args_instance)
+ if instance.exists():
+ instance.delete()
+
+def run_isolated():
+ '''
+ run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..)
+ To run isolated without py.test, you need to
+ - set the installation prefix
+ - run this program
+ '''
+ test_finalizer()
+
+if __name__ == '__main__':
+ run_isolated()
+
diff --git a/dirsrvtests/suites/schema/test_schema.py b/dirsrvtests/suites/schema/test_schema.py
new file mode 100644
index 0000000..cbf8579
--- /dev/null
+++ b/dirsrvtests/suites/schema/test_schema.py
@@ -0,0 +1,260 @@
+'''
+Created on Dec 18, 2013
+
+@author: rmeggins
+'''
+import os
+import sys
+import time
+import ldap
+from ldap.cidict import cidict
+from ldap.schema import SubSchema
+import logging
+import socket
+import time
+import logging
+import pytest
+import re
+from lib389 import DirSrv, Entry, tools
+from lib389.tools import DirSrvTools
+from lib389._constants import *
+from lib389.properties import *
+from constants import *
+
+logging.getLogger(__name__).setLevel(logging.DEBUG)
+log = logging.getLogger(__name__)
+
+installation_prefix = None
+
+class TopologyStandalone(object):
+ def __init__(self, standalone):
+ standalone.open()
+ self.standalone = standalone
+
+(a)pytest.fixture(scope="module")
+def topology(request):
+ '''
+ This fixture is used to create a DirSrv instance for the 'module'.
+ At the beginning, there may already be an instance.
+ There may also be a backup for the instance.
+
+ Principle:
+ If instance exists:
+ restart it
+ If backup exists:
+ create or rebind to instance
+ restore instance from backup
+ else:
+ Cleanup everything
+ remove instance
+ remove backup
+ Create instance
+ Create backup
+ '''
+ global installation_prefix
+
+ if installation_prefix:
+ args_instance[SER_DEPLOYED_DIR] = installation_prefix
+ schemainst = DirSrv(verbose=False)
+
+ # Args for the master instance
+ args_instance[SER_HOST] = HOST_STANDALONE
+ args_instance[SER_PORT] = PORT_STANDALONE
+ args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE
+ schemainst.allocate(args_instance)
+
+ # Get the status of the backups
+ backup = schemainst.checkBackupFS()
+
+ # Get the status of the instance and restart it if it exists
+ if schemainst.exists():
+ schemainst.stop(timeout=10)
+ schemainst.start(timeout=10)
+
+ if backup:
+ # The backup exists, assuming it is correct
+ # we just re-init the instance with it
+ if not schemainst.exists():
+ schemainst.create()
+ # Used to retrieve configuration information (dbdir, confdir...)
+ schemainst.open()
+
+ # restore from backup
+ schemainst.stop(timeout=10)
+ schemainst.restoreFS(backup)
+ schemainst.start(timeout=10)
+ else:
+ # We should be here only in two conditions
+ # - This is the first test
+ # so we need to create everything
+ # - Something weird happened (instance/backup destroyed)
+ # so we discard everything and recreate all
+
+ # Remove the backup. So even if we have a specific backup file
+ # (e.g backup) we clear all backups that an instance may have created
+ if backup:
+ schemainst.clearBackupFS()
+
+ # Remove all the instances
+ if schemainst.exists():
+ schemainst.delete()
+
+ # Create the instances
+ schemainst.create()
+ schemainst.open()
+
+ # Time to create the backup
+ schemainst.stop(timeout=10)
+ schemainst.backupfile = schemainst.backupFS()
+ schemainst.start(timeout=10)
+ #
+ return TopologyStandalone(schemainst)
+
+attrclass = ldap.schema.models.AttributeType
+occlass = ldap.schema.models.ObjectClass
+
+def ochasattr(subschema, oc, mustormay, attr, key):
+ """See if the oc and any of its parents and ancestors have the
+ given attr"""
+ rc = False
+ if not key in oc.__dict__:
+ dd = cidict()
+ for ii in oc.__dict__[mustormay]:
+ dd[ii] = ii
+ oc.__dict__[key] = dd
+ if attr in oc.__dict__[key]:
+ rc = True
+ else:
+ # look in parents
+ for noroid in oc.sup:
+ ocpar = subschema.get_obj(occlass, noroid)
+ assert(ocpar)
+ rc = ochasattr(subschema, ocpar, mustormay, attr, key)
+ if rc:
+ break
+ return rc
+
+def ochasattrs(subschema, oc, mustormay, attrs):
+ key = mustormay + "dict"
+ ret = []
+ for attr in attrs:
+ if not ochasattr(subschema, oc, mustormay, attr, key):
+ ret.append(attr)
+ return ret
+
+def mycmp(v1, v2):
+ v1ary, v2ary = [v1], [v2]
+ if isinstance(v1, list) or isinstance(v1, tuple):
+ v1ary, v2ary = list(set([x.lower() for x in v1])), list(set([x.lower() for x in v2]))
+ if not len(v1ary) == len(v2ary):
+ return False
+ for v1, v2 in zip(v1ary, v2ary):
+ if isinstance(v1, basestring):
+ if not len(v1) == len(v2):
+ return False
+ if not v1 == v2:
+ return False
+ return True
+
+def ocgetdiffs(ldschema, oc1, oc2):
+ fields = ['obsolete', 'names', 'desc', 'must', 'may', 'kind', 'sup']
+ ret = ''
+ for field in fields:
+ v1, v2 = oc1.__dict__[field], oc2.__dict__[field]
+ if field == 'may' or field == 'must':
+ missing = ochasattrs(ldschema, oc1, field, oc2.__dict__[field])
+ if missing:
+ ret = ret + '\t%s is missing %s\n' % (field, missing)
+ missing = ochasattrs(ldschema, oc2, field, oc1.__dict__[field])
+ if missing:
+ ret = ret + '\t%s is missing %s\n' % (field, missing)
+ elif not mycmp(v1, v2):
+ ret = ret + '\t%s differs: [%s] vs. [%s]\n' % (field, oc1.__dict__[field], oc2.__dict__[field])
+ return ret
+
+def atgetparfield(subschema, at, field):
+ v = None
+ for nameoroid in at.sup:
+ atpar = subschema.get_obj(attrclass, nameoroid)
+ assert(atpar)
+ v = atpar.__dict__.get(field, atgetparfield(subschema, atpar, field))
+ if v is not None:
+ break
+ return v
+
+syntax_len_supported = False
+
+def atgetdiffs(ldschema, at1, at2):
+ fields = ['names', 'desc', 'obsolete', 'sup', 'equality', 'ordering', 'substr', 'syntax',
+ 'single_value', 'collective', 'no_user_mod', 'usage']
+ if syntax_len_supported:
+ fields.append('syntax_len')
+ ret = ''
+ for field in fields:
+ v1 = at1.__dict__.get(field) or atgetparfield(ldschema, at1, field)
+ v2 = at2.__dict__.get(field) or atgetparfield(ldschema, at2, field)
+ if not mycmp(v1, v2):
+ ret = ret + '\t%s differs: [%s] vs. [%s]\n' % (field, at1.__dict__[field], at2.__dict__[field])
+ return ret
+
+def test_schema_comparewithfiles(topology):
+ '''Compare the schema from ldap cn=schema with the schema files'''
+ retval = True
+ schemainst = topology.standalone
+ ldschema = schemainst.schema.get_subschema()
+ assert ldschema
+ for fn in schemainst.schema.list_files():
+ fschema = schemainst.schema.file_to_subschema(fn)
+ if not fschema:
+ log.warn("Unable to parse %s as a schema file - skipping" % fn)
+ continue
+ assert fschema
+ for oid in fschema.listall(occlass):
+ se = fschema.get_obj(occlass, oid)
+ assert se
+ ldse = ldschema.get_obj(occlass, oid)
+ if not ldse:
+ log.error("objectclass in %s but not in %s: %s" % (fn, DN_SCHEMA, se))
+ retval = False
+ continue
+ ret = ocgetdiffs(ldschema, ldse, se)
+ if ret:
+ log.error("name %s oid %s\n%s" % (se.names[0], oid, ret))
+ retval = False
+ for oid in fschema.listall(attrclass):
+ se = fschema.get_obj(attrclass, oid)
+ assert se
+ ldse = ldschema.get_obj(attrclass, oid)
+ if not ldse:
+ log.error("attributetype in %s but not in %s: %s" % (fn, DN_SCHEMA, se))
+ retval = False
+ continue
+ ret = atgetdiffs(ldschema, ldse, se)
+ if ret:
+ log.error("name %s oid %s\n%s" % (se.names[0], oid, ret))
+ retval = False
+ assert retval
+
+def test_schema_final(topology):
+ topology.standalone.stop(timeout=10)
+
+def run_isolated():
+ '''
+ run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..)
+ To run isolated without py.test, you need to
+ - edit this file and comment '@pytest.fixture' line before 'topology' function.
+ - set the installation prefix
+ - run this program
+ '''
+ global installation_prefix
+ installation_prefix = os.environ.get('PREFIX')
+
+ topo = topology(True)
+
+ test_schema_comparewithfiles(topo)
+
+ test_schema_final(topo)
+
+if __name__ == '__main__':
+ run_isolated()
+
10 years, 3 months
- 1
- 0
Branch '389-ds-base-1.3.2' - ldap/servers
by Ludwig Krispenz
ldap/servers/plugins/sync/sync.h | 7 ++++---
ldap/servers/plugins/sync/sync_persist.c | 3 ++-
ldap/servers/plugins/sync/sync_refresh.c | 18 ++++++++++++------
3 files changed, 18 insertions(+), 10 deletions(-)
New commits:
commit d06de4e43234bbebbdb2ca4e57dbe17e26fcfe00
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
Date: Fri Dec 20 14:52:44 2013 +0100
Ticket 47629 - random crashes related to sync repl
Bug Description: if there is no cookie, the persist thread starts
before the initial refresh is complete, both threads use the
same operation structure and there is a race condition
on setting and freeing the search entry.
Fix Description: ensure that the persist thread only starts sending
updates once the refresh is complete
https://fedorahosted.org/389/ticket/47629
Reviewed by: Rich, thanks
diff --git a/ldap/servers/plugins/sync/sync.h b/ldap/servers/plugins/sync/sync.h
index cf73ce9..8cdc7d0 100644
--- a/ldap/servers/plugins/sync/sync.h
+++ b/ldap/servers/plugins/sync/sync.h
@@ -115,7 +115,7 @@ int sync_is_active (Slapi_Entry *e, Slapi_PBlock *pb);
int sync_is_active_scope (const Slapi_DN *dn, Slapi_PBlock *pb);
int sync_refresh_update_content(Slapi_PBlock *pb, Sync_Cookie *client_cookie, Sync_Cookie *session_cookie);
-int sync_refresh_initial_content(Slapi_PBlock *pb, int persist, Sync_Cookie *session_cookie);
+int sync_refresh_initial_content(Slapi_PBlock *pb, int persist, PRThread *tid, Sync_Cookie *session_cookie);
int sync_read_entry_from_changelog( Slapi_Entry *cl_entry, void *cb_data);
int sync_send_entry_from_changelog( Slapi_PBlock *pb, int chg_req, char *uniqueid);
void sync_send_deleted_entries (Slapi_PBlock *pb, Sync_UpdateNode *upd, int chg_count, Sync_Cookie *session_cookie);
@@ -190,7 +190,8 @@ typedef struct sync_request_list {
#define SYNC_FLAG_SEND_INTERMEDIATE 0x08
typedef struct sync_op_info {
- int send_flag; /* hint for preop plugins what to send */
- Sync_Cookie *cookie; /* cookie to add in control */
+ int send_flag; /* hint for preop plugins what to send */
+ Sync_Cookie *cookie;/* cookie to add in control */
+ PRThread *tid; /* thread for persistent phase */
} SyncOpInfo;
diff --git a/ldap/servers/plugins/sync/sync_persist.c b/ldap/servers/plugins/sync/sync_persist.c
index 7cd65c8..680bcf6 100644
--- a/ldap/servers/plugins/sync/sync_persist.c
+++ b/ldap/servers/plugins/sync/sync_persist.c
@@ -351,7 +351,8 @@ sync_persist_terminate (PRThread *tid)
cur = sync_request_list->sync_req_head;
while ( NULL != cur ) {
if ( cur->req_tid == tid ) {
- cur->req_active = PR_TRUE;
+ cur->req_active = PR_FALSE;
+ cur->req_complete = PR_TRUE;
rc = 0;
break;
}
diff --git a/ldap/servers/plugins/sync/sync_refresh.c b/ldap/servers/plugins/sync/sync_refresh.c
index 71a0edd..18f7884 100644
--- a/ldap/servers/plugins/sync/sync_refresh.c
+++ b/ldap/servers/plugins/sync/sync_refresh.c
@@ -37,7 +37,7 @@
#include "sync.h"
-static SyncOpInfo *new_SyncOpInfo(int flag, Sync_Cookie *cookie);
+static SyncOpInfo *new_SyncOpInfo(int flag, PRThread *tid, Sync_Cookie *cookie);
static int sync_extension_type;
static int sync_extension_handle;
@@ -157,7 +157,7 @@ int sync_srch_refresh_pre_search(Slapi_PBlock *pb)
sync_result_err(pb,rc, "Invalid session cookie");
}
} else {
- rc = sync_refresh_initial_content (pb, sync_persist, session_cookie);
+ rc = sync_refresh_initial_content (pb, sync_persist, tid, session_cookie);
if (rc == 0 && !sync_persist)
/* maintained in postop code */
session_cookie = NULL;
@@ -172,8 +172,9 @@ int sync_srch_refresh_pre_search(Slapi_PBlock *pb)
Slapi_Operation *operation;
slapi_pblock_get(pb, SLAPI_OPERATION, &operation);
- rc = sync_persist_startup(tid, session_cookie);
-
+ if (client_cookie) {
+ rc = sync_persist_startup(tid, session_cookie);
+ }
if (rc == 0) {
session_cookie = NULL; /* maintained in persist code */
slapi_operation_set_flag(operation, OP_FLAG_SYNC_PERSIST);
@@ -216,6 +217,8 @@ int sync_srch_refresh_post_search(Slapi_PBlock *pb)
* depending on the operation type, reset flag
*/
info->send_flag &= ~SYNC_FLAG_ADD_STATE_CTRL;
+ /* activate the persistent phase thread*/
+ sync_persist_startup(info->tid, info->cookie);
}
if (info->send_flag & SYNC_FLAG_ADD_DONE_CTRL) {
LDAPControl **ctrl = (LDAPControl **)slapi_ch_calloc(2, sizeof (LDAPControl *));
@@ -320,7 +323,7 @@ sync_refresh_update_content(Slapi_PBlock *pb, Sync_Cookie *client_cookie, Sync_C
}
int
-sync_refresh_initial_content(Slapi_PBlock *pb, int sync_persist, Sync_Cookie *sc)
+sync_refresh_initial_content(Slapi_PBlock *pb, int sync_persist, PRThread *tid, Sync_Cookie *sc)
{
/* the entries will be sent in the normal search process, but
* - a control has to be sent with each entry
@@ -341,11 +344,13 @@ sync_refresh_initial_content(Slapi_PBlock *pb, int sync_persist, Sync_Cookie *sc
(SYNC_FLAG_ADD_STATE_CTRL |
SYNC_FLAG_SEND_INTERMEDIATE |
SYNC_FLAG_NO_RESULT,
+ tid,
sc);
} else {
info = new_SyncOpInfo
(SYNC_FLAG_ADD_STATE_CTRL |
SYNC_FLAG_ADD_DONE_CTRL,
+ tid,
sc);
}
sync_set_operation_extension(pb, info);
@@ -672,10 +677,11 @@ sync_send_entry_from_changelog(Slapi_PBlock *pb,int chg_req, char *uniqueid)
}
static SyncOpInfo*
-new_SyncOpInfo(int flag, Sync_Cookie *cookie) {
+new_SyncOpInfo(int flag, PRThread *tid, Sync_Cookie *cookie) {
SyncOpInfo *spec = (SyncOpInfo *)slapi_ch_calloc(1, sizeof(SyncOpInfo));
spec->send_flag = flag;
spec->cookie = cookie;
+ spec->tid = tid;
return spec;
}
10 years, 3 months
- 1
- 0
ldap/servers
by Ludwig Krispenz
ldap/servers/plugins/sync/sync.h | 7 ++++---
ldap/servers/plugins/sync/sync_persist.c | 3 ++-
ldap/servers/plugins/sync/sync_refresh.c | 18 ++++++++++++------
3 files changed, 18 insertions(+), 10 deletions(-)
New commits:
commit 659b777ac2d2835bba5de0979a9a48ccc7ea0665
Author: Ludwig Krispenz <lkrispen(a)redhat.com>
Date: Fri Dec 20 14:52:44 2013 +0100
Ticket 47629 - random crashes related to sync repl
Bug Description: if there is no cookie, the persist thread starts
before the initial refresh is complete, both threads use the
same operation structure and there is a race condition
on setting and freeing the search entry.
Fix Description: ensure that the persist thread only starts sending
updates once the refresh is complete
https://fedorahosted.org/389/ticket/47629
Reviewed by: Rich, thanks
diff --git a/ldap/servers/plugins/sync/sync.h b/ldap/servers/plugins/sync/sync.h
index cf73ce9..8cdc7d0 100644
--- a/ldap/servers/plugins/sync/sync.h
+++ b/ldap/servers/plugins/sync/sync.h
@@ -115,7 +115,7 @@ int sync_is_active (Slapi_Entry *e, Slapi_PBlock *pb);
int sync_is_active_scope (const Slapi_DN *dn, Slapi_PBlock *pb);
int sync_refresh_update_content(Slapi_PBlock *pb, Sync_Cookie *client_cookie, Sync_Cookie *session_cookie);
-int sync_refresh_initial_content(Slapi_PBlock *pb, int persist, Sync_Cookie *session_cookie);
+int sync_refresh_initial_content(Slapi_PBlock *pb, int persist, PRThread *tid, Sync_Cookie *session_cookie);
int sync_read_entry_from_changelog( Slapi_Entry *cl_entry, void *cb_data);
int sync_send_entry_from_changelog( Slapi_PBlock *pb, int chg_req, char *uniqueid);
void sync_send_deleted_entries (Slapi_PBlock *pb, Sync_UpdateNode *upd, int chg_count, Sync_Cookie *session_cookie);
@@ -190,7 +190,8 @@ typedef struct sync_request_list {
#define SYNC_FLAG_SEND_INTERMEDIATE 0x08
typedef struct sync_op_info {
- int send_flag; /* hint for preop plugins what to send */
- Sync_Cookie *cookie; /* cookie to add in control */
+ int send_flag; /* hint for preop plugins what to send */
+ Sync_Cookie *cookie;/* cookie to add in control */
+ PRThread *tid; /* thread for persistent phase */
} SyncOpInfo;
diff --git a/ldap/servers/plugins/sync/sync_persist.c b/ldap/servers/plugins/sync/sync_persist.c
index 7cd65c8..680bcf6 100644
--- a/ldap/servers/plugins/sync/sync_persist.c
+++ b/ldap/servers/plugins/sync/sync_persist.c
@@ -351,7 +351,8 @@ sync_persist_terminate (PRThread *tid)
cur = sync_request_list->sync_req_head;
while ( NULL != cur ) {
if ( cur->req_tid == tid ) {
- cur->req_active = PR_TRUE;
+ cur->req_active = PR_FALSE;
+ cur->req_complete = PR_TRUE;
rc = 0;
break;
}
diff --git a/ldap/servers/plugins/sync/sync_refresh.c b/ldap/servers/plugins/sync/sync_refresh.c
index 71a0edd..18f7884 100644
--- a/ldap/servers/plugins/sync/sync_refresh.c
+++ b/ldap/servers/plugins/sync/sync_refresh.c
@@ -37,7 +37,7 @@
#include "sync.h"
-static SyncOpInfo *new_SyncOpInfo(int flag, Sync_Cookie *cookie);
+static SyncOpInfo *new_SyncOpInfo(int flag, PRThread *tid, Sync_Cookie *cookie);
static int sync_extension_type;
static int sync_extension_handle;
@@ -157,7 +157,7 @@ int sync_srch_refresh_pre_search(Slapi_PBlock *pb)
sync_result_err(pb,rc, "Invalid session cookie");
}
} else {
- rc = sync_refresh_initial_content (pb, sync_persist, session_cookie);
+ rc = sync_refresh_initial_content (pb, sync_persist, tid, session_cookie);
if (rc == 0 && !sync_persist)
/* maintained in postop code */
session_cookie = NULL;
@@ -172,8 +172,9 @@ int sync_srch_refresh_pre_search(Slapi_PBlock *pb)
Slapi_Operation *operation;
slapi_pblock_get(pb, SLAPI_OPERATION, &operation);
- rc = sync_persist_startup(tid, session_cookie);
-
+ if (client_cookie) {
+ rc = sync_persist_startup(tid, session_cookie);
+ }
if (rc == 0) {
session_cookie = NULL; /* maintained in persist code */
slapi_operation_set_flag(operation, OP_FLAG_SYNC_PERSIST);
@@ -216,6 +217,8 @@ int sync_srch_refresh_post_search(Slapi_PBlock *pb)
* depending on the operation type, reset flag
*/
info->send_flag &= ~SYNC_FLAG_ADD_STATE_CTRL;
+ /* activate the persistent phase thread*/
+ sync_persist_startup(info->tid, info->cookie);
}
if (info->send_flag & SYNC_FLAG_ADD_DONE_CTRL) {
LDAPControl **ctrl = (LDAPControl **)slapi_ch_calloc(2, sizeof (LDAPControl *));
@@ -320,7 +323,7 @@ sync_refresh_update_content(Slapi_PBlock *pb, Sync_Cookie *client_cookie, Sync_C
}
int
-sync_refresh_initial_content(Slapi_PBlock *pb, int sync_persist, Sync_Cookie *sc)
+sync_refresh_initial_content(Slapi_PBlock *pb, int sync_persist, PRThread *tid, Sync_Cookie *sc)
{
/* the entries will be sent in the normal search process, but
* - a control has to be sent with each entry
@@ -341,11 +344,13 @@ sync_refresh_initial_content(Slapi_PBlock *pb, int sync_persist, Sync_Cookie *sc
(SYNC_FLAG_ADD_STATE_CTRL |
SYNC_FLAG_SEND_INTERMEDIATE |
SYNC_FLAG_NO_RESULT,
+ tid,
sc);
} else {
info = new_SyncOpInfo
(SYNC_FLAG_ADD_STATE_CTRL |
SYNC_FLAG_ADD_DONE_CTRL,
+ tid,
sc);
}
sync_set_operation_extension(pb, info);
@@ -672,10 +677,11 @@ sync_send_entry_from_changelog(Slapi_PBlock *pb,int chg_req, char *uniqueid)
}
static SyncOpInfo*
-new_SyncOpInfo(int flag, Sync_Cookie *cookie) {
+new_SyncOpInfo(int flag, PRThread *tid, Sync_Cookie *cookie) {
SyncOpInfo *spec = (SyncOpInfo *)slapi_ch_calloc(1, sizeof(SyncOpInfo));
spec->send_flag = flag;
spec->cookie = cookie;
+ spec->tid = tid;
return spec;
}
10 years, 3 months
- 1
- 0
Branch '389-ds-base-1.3.2' - 2 commits - ldap/servers
by Noriko Hosoi
ldap/servers/plugins/acl/acl.c | 194 +++++++++++-----------
ldap/servers/plugins/acl/aclparse.c | 3
ldap/servers/plugins/acl/aclplugin.c | 6
ldap/servers/slapd/attr.c | 110 ++++++++++--
ldap/servers/slapd/result.c | 300 +++++++++++++++++------------------
ldap/servers/slapd/search.c | 23 +-
ldap/servers/slapd/slapi-plugin.h | 8
7 files changed, 363 insertions(+), 281 deletions(-)
New commits:
commit 270ad9a7ed2419a4cb19fafa4fa2109b5b99491c
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Mon Jan 13 14:43:46 2014 -0800
Ticket #47571 - targetattr ACIs ignore subtype
Description: commit 85a78741dfeb636a1cf7cced1576278e65f5bb58
introduced 2 coverity issues:
12423: Explicit null dereferenced
do_search (slapd/search.c)
If attribute list contains multiple "*"s and "aci"s in ldapsearch,
the previous code attempted to add "aci" once for "*" and replacing
"aci" with normalized aci (if any) once and eliminating the duplicated
"aci"s. The code contained a null dereferenced bug. Even if duplicated
attributes are included in the attribute list, they are removed later
in send_specific_attrs. Thus, this patch simplifies the logic to avoid
the null dereference.
12422: Logically dead code
comp_cmp (slapd/attr.c)
Eliminated the dead code (case s1 == NULL AND s2 == NULL).
https://fedorahosted.org/389/ticket/47571
Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!)
(cherry picked from commit 36c48b8395c6e4ca374c74f91dad6b022aa99ce1)
diff --git a/ldap/servers/slapd/attr.c b/ldap/servers/slapd/attr.c
index ef3fa10..39c6e99 100644
--- a/ldap/servers/slapd/attr.c
+++ b/ldap/servers/slapd/attr.c
@@ -111,7 +111,6 @@ comp_cmp( const char *s1p, const char *s2p )
if (s1) {
return 1;
}
- return 0;
}
while ( *s1 && (*s1 != ';') &&
*s2 && (*s2 != ';') &&
diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c
index 29729bb..d68ba7b 100644
--- a/ldap/servers/slapd/search.c
+++ b/ldap/servers/slapd/search.c
@@ -248,11 +248,10 @@ do_search( Slapi_PBlock *pb )
if ( attrs != NULL ) {
char *normaci = slapi_attr_syntax_normalize("aci");
int replace_aci = 0;
- if (0 == strcasecmp(normaci, "aci")) {
- /* normaci is identical to "aci" */
- slapi_ch_free_string(&normaci);
+ if (!normaci) {
normaci = slapi_ch_strdup("aci");
- } else {
+ } else if (strcasecmp(normaci, "aci")) {
+ /* normaci is not "aci" */
replace_aci = 1;
}
/*
@@ -284,20 +283,11 @@ do_search( Slapi_PBlock *pb )
*p = '\0';
} else if (strcmp(attrs[i], LDAP_ALL_USER_ATTRS /* '*' */) == 0) {
if (!charray_inlist(attrs, normaci)) {
- charray_add(&attrs, normaci); /* consumed */
- normaci = NULL;
+ charray_add(&attrs, slapi_ch_strdup(normaci));
}
} else if (replace_aci && (strcasecmp(attrs[i], "aci") == 0)) {
slapi_ch_free_string(&attrs[i]);
- if (charray_inlist(attrs, normaci)) { /* attrlist: "*" "aci" */
- int j = i;
- for (; attrs[j]; j++) { /* Shift the rest by 1 */
- attrs[j] = attrs[j+1];
- }
- } else { /* attrlist: "aci" "*" */
- attrs[i] = normaci; /* consumed */
- normaci = NULL;
- }
+ attrs[i] = slapi_ch_strdup(normaci);
}
}
slapi_ch_free_string(&normaci);
commit 9d2d939126111976ad78eab0186c675d9bc1c9dd
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Fri Jan 10 11:06:02 2014 -0800
Ticket #47571 - targetattr ACIs ignore subtype
Description:
Subtypes in targetattr, userattr in aci as well as filter and attribute list
in the search are supported.
* If targetattr contains subtypes, the base type only as well as other subtypes
are not allowed to access (or denied to access).
* If userattr contains subtypes, the base type as well as other subtypes in
entries do not match the userattr value.
* If attribute list in search has a base type attribute, and a targetattr has
a type with subtypes, then only the subtyped value is returned. E.g.,
attribute list: sn
targetattr: sn;en
==>
sn;en: <sn-en-value> and
sn;en;phonetic: <sn-en-phonetic-value> are returned
but
sn or sn;fr is not.
If attribute list has a type with subtype, then if the targetattr allows the
subtype, the value is returned. E.g.,
attribute list: sn;en
targetattr: sn;en
==>
sn;en: <sn-en-value> and
sn;en;phonetic: <sn-en-phonetic-value> are returned
but
sn or sn;fr is not.
1) slapd/attr.c
* slapi_attr_type_cmp assumed the subtype order in 2 args are identical,
but it is not always guaranteed. Removed the assumption.
* Added another compare type SLAPI_TYPE_CMP_SUBTYPES to comp_cmp which is
called by slapi_attr_type_cmp to support full subtypes comparison.
2) plugin/acl.c:
* Changed to call slapi_attr_type_cmp with human readable macros, e.g.,
SLAPI_TYPE_CMP_BASE, SLAPI_TYPE_CMP_SUBTYPE, etc.
* Replaced strcasecmp with slapi_attr_type_cmp for attribute type comparison.
* Changed to call slapi_attr_type_cmp with SLAPI_TYPE_CMP_SUBTYPES (full
subtype comparison) in acl__get_attrEval, where the next attribute to
compare is determined.
3) slapd/search.c,result.c
send_all_attrs/send_specific_attrs use a dontsendattr array to control the
duplicate attribute types. Replaced the logic with a simpler one by creating
an charray with no duplicates.
https://fedorahosted.org/389/ticket/47571
Reviewed by tbordaz(a)redhat.com (Thank you, Thierry!)
(cherry picked from commit 85a78741dfeb636a1cf7cced1576278e65f5bb58)
diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c
index bd7547d..9da6d95 100644
--- a/ldap/servers/plugins/acl/acl.c
+++ b/ldap/servers/plugins/acl/acl.c
@@ -549,15 +549,14 @@ acl_access_allowed(
** Check if we can use any cached information to determine
** access to this resource
*/
- if ( (access & SLAPI_ACL_SEARCH) &&
- (ret_val = acl__match_handlesFromCache ( aclpb , attr, access)) != -1) {
+ if ((access & SLAPI_ACL_SEARCH) &&
+ (ret_val = acl__match_handlesFromCache(aclpb, attr, access)) != -1) {
/* means got a result: allowed or not*/
if (ret_val == LDAP_SUCCESS ) {
- decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
+ decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
} else if (ret_val == LDAP_INSUFFICIENT_ACCESS) {
- decision_reason.reason =
- ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
+ decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
}
goto cleanup_and_ret;
}
@@ -1058,7 +1057,8 @@ acl_read_access_allowed_on_entry (
slapi_ch_free ( (void **) &aclpb->aclpb_Evalattr);
aclpb->aclpb_Evalattr = slapi_ch_malloc(len+1);
}
- PL_strncpyz (aclpb->aclpb_Evalattr, attr_type, len);
+ /* length needs to have 1 for '\0' */
+ PL_strncpyz (aclpb->aclpb_Evalattr, attr_type, len+1);
#ifdef DETERMINE_ACCESS_BASED_ON_REQUESTED_ATTRIBUTES
if ( attr_index >= 0 ) {
/*
@@ -1191,38 +1191,36 @@ acl_read_access_allowed_on_attr (
}
/*
- * Am I a anonymous dude ? then we can use our anonympous profile
+ * Am I anonymous? then we can use our anonympous profile
* We don't require the aclpb to have been initialized for anom stuff
- *
- */
+ */
slapi_pblock_get (pb, SLAPI_REQUESTOR_DN ,&clientDn );
- if ( clientDn && *clientDn == '\0' ) {
- ret_val = aclanom_match_profile ( pb, aclpb, e, attr,
- SLAPI_ACL_READ );
+ if (clientDn && (*clientDn == '\0')) {
+ ret_val = aclanom_match_profile (pb, aclpb, e, attr, SLAPI_ACL_READ);
TNF_PROBE_1_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","",
- tnf_string,anon_decision,"");
- if (ret_val != -1 ) return ret_val;
+ tnf_string,anon_decision,"");
+ if (ret_val != -1) {
+ return ret_val;
+ }
}
/* Then I must have a access to the entry. */
aclpb->aclpb_state |= ACLPB_ACCESS_ALLOWED_ON_ENTRY;
- if ( aclpb->aclpb_state & ACLPB_MATCHES_ALL_ACLS ) {
+ if (aclpb->aclpb_state & ACLPB_MATCHES_ALL_ACLS) {
ret_val = acl__attr_cached_result (aclpb, attr, SLAPI_ACL_READ);
- if (ret_val != -1 ) {
+ if (ret_val != -1) {
slapi_log_error(SLAPI_LOG_ACL, plugin_name,
- "MATCHED HANDLE:dn:%s attr: %s val:%d\n",
- n_edn, attr, ret_val );
- if ( ret_val == LDAP_SUCCESS) {
- decision_reason.reason =
- ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
+ "MATCHED HANDLE:dn:%s attr: %s val:%d\n",
+ n_edn, attr, ret_val );
+ if (ret_val == LDAP_SUCCESS) {
+ decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
} else {
- decision_reason.reason =
- ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
- }
+ decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
+ }
goto acl_access_allowed_on_attr_Exit;
- } else {
+ } else {
aclpb->aclpb_state |= ACLPB_COPY_EVALCONTEXT;
}
}
@@ -1258,50 +1256,54 @@ acl_read_access_allowed_on_attr (
** -- access is allowed on phone
** -- Don't know about the rest. Need to evaluate.
*/
-
- if ( slapi_attr_type_cmp (attr, aclpb->aclpb_Evalattr, 1) == 0) {
+ if (slapi_attr_type_cmp(aclpb->aclpb_Evalattr, attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
/* from now on we need to evaluate access on
** rest of the attrs.
*/
- aclpb->aclpb_state &= ~ACLPB_ACCESS_ALLOWED_ON_A_ATTR;
TNF_PROBE_1_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","",
- tnf_string,aclp_Evalattr1,"");
-
+ tnf_string,aclp_Evalattr1,"");
return LDAP_SUCCESS;
- } else {
- /*
- * Here, the attr that implied access to the entry (aclpb_Evalattr),
- * is not
- * the one we currently want evaluated--so
- * we need to evaluate access to attr--so fall through.
- */
}
- } else if (aclpb->aclpb_state & ACLPB_ACCESS_ALLOWED_USERATTR) {
+ /*
+ * Here, the attr that implied access to the entry (aclpb_Evalattr),
+ * is not
+ * the one we currently want evaluated--so
+ * we need to evaluate access to attr--so fall through.
+ */
+
+ } else if (aclpb->aclpb_state & ACLPB_ACCESS_ALLOWED_USERATTR) {
/* Only skip evaluation on the user attr on which we have
** evaluated before.
*/
- if ( slapi_attr_type_cmp (attr, aclpb->aclpb_Evalattr, 1) == 0) {
+ if (slapi_attr_type_cmp(aclpb->aclpb_Evalattr, attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
aclpb->aclpb_state &= ~ACLPB_ACCESS_ALLOWED_USERATTR;
TNF_PROBE_1_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","",
- tnf_string,aclp_Evalattr2,"");
+ tnf_string,aclp_Evalattr2,"");
return LDAP_SUCCESS;
}
}
/* we need to evaluate the access on this attr */
+ /*
+ * targetattr=sn;en
+ * search attribute list: cn sn
+ * ==>
+ * attr: cn sn;en
+ * aclpb_Evalattr: sn;en
+ */
return ( acl_access_allowed(pb, e, attr, val, access) );
/* This exit point prints a summary and returns ret_val */
acl_access_allowed_on_attr_Exit:
- /* print summary if loglevel set */
+ /* print summary if loglevel set */
if ( slapi_is_loglevel_set(loglevel) ) {
print_access_control_summary( "on attr",
ret_val, clientDn, aclpb,
acl_access2str(SLAPI_ACL_READ),
- attr, n_edn, &decision_reason);
+ attr, n_edn, &decision_reason);
}
TNF_PROBE_0_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","");
@@ -1697,8 +1699,9 @@ acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change)
mods = (LDAPMod **) change;
- for (j=0; mods[j] != NULL; j++) {
- if (strcasecmp(mods[j]->mod_type, aci_attr_type) == 0) {
+ for (j=0; mods && mods[j]; j++) {
+ if (slapi_attr_type_cmp(mods[j]->mod_type, aci_attr_type,
+ SLAPI_TYPE_CMP_SUBTYPE) == 0) {
/* Got an aci to mod in this list of mods, so
* take the acicache lock for the whole list of mods,
@@ -2165,7 +2168,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
} else {
dn_matched = ACL_TRUE;
}
-
+
if (aci->aci_type & ACI_TARGET_NOT) {
matches = (dn_matched ? ACL_FALSE : ACL_TRUE);
} else {
@@ -2347,7 +2350,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
* of the attribute in the entry--otherwise you
* could satisfy the filter and then put loads of other
* values in on the back of it.
- */
+ */
sval=NULL;
attrVal=NULL;
@@ -2356,18 +2359,16 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
while(k != -1 && !done) {
attrVal = slapi_value_get_berval(sval);
- if ( acl__make_filter_test_entry(
- &aclpb->aclpb_filter_test_entry,
- attrFilter->attr_str,
- (struct berval *)attrVal) == LDAP_SUCCESS ) {
-
- attr_matched= acl__test_filter(
- aclpb->aclpb_filter_test_entry,
- attrFilter->filter,
- 1 /* Do filter sense evaluation below */
- );
+ if (acl__make_filter_test_entry(&aclpb->aclpb_filter_test_entry,
+ attrFilter->attr_str,
+ (struct berval *)attrVal) == LDAP_SUCCESS ) {
+
+ attr_matched = acl__test_filter(aclpb->aclpb_filter_test_entry,
+ attrFilter->filter,
+ 1 /* Do filter sense evaluation below */
+ );
done = !attr_matched;
- slapi_entry_free( aclpb->aclpb_filter_test_entry );
+ slapi_entry_free( aclpb->aclpb_filter_test_entry );
}
k= slapi_attr_next_value(attr_ptr, k, &sval);
@@ -2379,8 +2380,8 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
* of the attribute in the entry satisfied the filter.
* Otherwise, attr_matched is ACL_FALSE and not every
* value satisfied the filter, so we will teminate the
- * scan of the filter list.
- */
+ * scan of the filter list.
+ */
}
@@ -2391,9 +2392,9 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
* Here, we've applied all the applicable filters to the entry.
* Each one must have been satisfied by all the values of the attribute.
* The result of this is stored in attr_matched.
- */
+ */
-#if 0
+#if 0
/*
* Don't support a notion of "add != " or "del != "
* at the moment.
@@ -2415,12 +2416,10 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
goto acl__resource_match_aci_EXIT;
}
- } else if ( ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&
- (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) ||
- ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) &&
- (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) ) {
-
-
+ } else if ( ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&
+ (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) ||
+ ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) &&
+ (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) ) {
/*
* Here, it's a modify add/del and we have attr filters.
* So, we need to scan the add/del filter list to find the filter
@@ -2436,15 +2435,11 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
int found = 0;
if ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&
- (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) {
-
+ (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) {
attrFilterArray = aci->targetAttrAddFilters;
-
} else if ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) &&
- (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) {
-
+ (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) {
attrFilterArray = aci->targetAttrDelFilters;
-
}
@@ -2456,12 +2451,12 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
num_attrs = 0;
while (attrFilterArray[num_attrs] && !found) {
- attrFilter = attrFilterArray[num_attrs];
+ attrFilter = attrFilterArray[num_attrs];
/* If this filter applies to the attribute, stop. */
if ((aclpb->aclpb_curr_attrEval) &&
- slapi_attr_type_cmp ( aclpb->aclpb_curr_attrEval->attrEval_name,
- attrFilter->attr_str, 1) == 0) {
+ slapi_attr_type_cmp(aclpb->aclpb_curr_attrEval->attrEval_name,
+ attrFilter->attr_str, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
found = 1;
}
num_attrs++;
@@ -2475,17 +2470,16 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
if (found) {
- if ( acl__make_filter_test_entry(
- &aclpb->aclpb_filter_test_entry,
- aclpb->aclpb_curr_attrEval->attrEval_name,
- aclpb->aclpb_curr_attrVal) == LDAP_SUCCESS ) {
+ if (acl__make_filter_test_entry(&aclpb->aclpb_filter_test_entry,
+ aclpb->aclpb_curr_attrEval->attrEval_name,
+ aclpb->aclpb_curr_attrVal) == LDAP_SUCCESS ) {
attr_matched= acl__test_filter(aclpb->aclpb_filter_test_entry,
attrFilter->filter,
1 /* Do filter sense evaluation below */
- );
- slapi_entry_free( aclpb->aclpb_filter_test_entry );
- }
+ );
+ slapi_entry_free( aclpb->aclpb_filter_test_entry );
+ }
/* No need to look further */
if (attr_matched == ACL_FALSE) {
@@ -2500,7 +2494,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
*/
attr_matched_in_targetattrfilters = 1;
- }
+ }
} /* targetvaluefilters */
@@ -2572,7 +2566,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
* bother to look at the attrlist.
*/
- if (!attr_matched_in_targetattrfilters) {
+ if (!attr_matched_in_targetattrfilters) {
/* match target attr */
if ((c_attrEval) &&
@@ -2587,10 +2581,13 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
num_attrs = 0;
while (attrArray[num_attrs] && !attr_matched) {
- attr = attrArray[num_attrs];
- if (attr->attr_type & ACL_ATTR_STRING) {
- if (slapi_attr_type_cmp ( res_attr,
- attr->u.attr_str, 1) == 0) {
+ attr = attrArray[num_attrs];
+ if (attr->attr_type & ACL_ATTR_STRING) {
+ /*
+ * res_attr: attr type to eval (e.g., filter "(sn;en=*)")
+ * attr->u.attr_str: targetattr value (e.g., sn;en)
+ */
+ if (slapi_attr_type_cmp(attr->u.attr_str, res_attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
attr_matched = ACL_TRUE;
*a_matched = ACL_TRUE;
}
@@ -3518,7 +3515,7 @@ acl__attr_cached_result (struct acl_pblock *aclpb, char *attr, int access )
if ( a_eval == NULL ) continue;
- if (strcasecmp ( attr, a_eval->attrEval_name ) == 0 ) {
+ if (slapi_attr_type_cmp(a_eval->attrEval_name, attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
if ( access & SLAPI_ACL_SEARCH ) {
if (a_eval->attrEval_s_status < ACL_ATTREVAL_DETERMINISTIC ) {
if ( a_eval->attrEval_s_status & ACL_ATTREVAL_SUCCESS)
@@ -3708,8 +3705,9 @@ acl_copyEval_context ( struct acl_pblock *aclpb, aclEvalContext *src,
continue;
for ( j = 0; j < dest->acle_numof_attrs; j++ ) {
- if ( strcasecmp ( src->acle_attrEval[i].attrEval_name,
- dest->acle_attrEval[j].attrEval_name ) == 0 ) {
+ if (slapi_attr_type_cmp(src->acle_attrEval[i].attrEval_name,
+ dest->acle_attrEval[j].attrEval_name,
+ SLAPI_TYPE_CMP_SUBTYPE) == 0) {
/* We have it. skip it. */
attr_exists = 1;
dd_slot = j;
@@ -3752,8 +3750,7 @@ acl_copyEval_context ( struct acl_pblock *aclpb, aclEvalContext *src,
(size_t)src->acle_numof_tmatched_handles, sizeof( int ), acl__cmp );
for (i=0; i < src->acle_numof_tmatched_handles; i++ ) {
- dest->acle_handles_matched_target[i] =
- src->acle_handles_matched_target[i];
+ dest->acle_handles_matched_target[i] = src->acle_handles_matched_target[i];
}
if ( src->acle_numof_tmatched_handles ) {
@@ -3918,9 +3915,12 @@ acl__get_attrEval ( struct acl_pblock *aclpb, char *attr )
/* Go thru and see if we have the attr already */
for (j=0; j < c_ContextEval->acle_numof_attrs; j++) {
c_attrEval = &c_ContextEval->acle_attrEval[j];
-
- if ( c_attrEval &&
- slapi_attr_type_cmp ( c_attrEval->attrEval_name, attr, 1) == 0 ) {
+
+ if (c_attrEval &&
+ /* attr: e.g., filter "(sn;en=*)" / attr list / attr in entry */
+ /* This compare must check all subtypes. "sn" vs. "sn;fr" should return 1 */
+ slapi_attr_type_cmp(c_attrEval->attrEval_name,
+ attr, SLAPI_TYPE_CMP_SUBTYPES) == 0) {
aclpb->aclpb_curr_attrEval = c_attrEval;
break;
}
diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c
index 5030b30..ccd387c 100644
--- a/ldap/servers/plugins/acl/aclparse.c
+++ b/ldap/servers/plugins/acl/aclparse.c
@@ -2224,8 +2224,7 @@ static int type_compare( Slapi_Filter *f, void *arg) {
if (slapi_filter_get_attribute_type( f, &filter_type) == 0) {
t = slapi_attr_syntax_normalize(t);
filter_type = slapi_attr_syntax_normalize(filter_type);
-
- if (slapi_attr_type_cmp(filter_type, t, 1) == 0) {
+ if (slapi_attr_type_cmp(filter_type, t, SLAPI_TYPE_CMP_BASE) == 0) {
rc = SLAPI_FILTER_SCAN_CONTINUE;
}
diff --git a/ldap/servers/plugins/acl/aclplugin.c b/ldap/servers/plugins/acl/aclplugin.c
index 0d35425..70a02d7 100644
--- a/ldap/servers/plugins/acl/aclplugin.c
+++ b/ldap/servers/plugins/acl/aclplugin.c
@@ -364,11 +364,11 @@ acl_access_allowed_main ( Slapi_PBlock *pb, Slapi_Entry *e, char **attrs,
if (attrs && *attrs) attr = attrs[0];
- if (ACLPLUGIN_ACCESS_READ_ON_ENTRY == flags)
+ if (ACLPLUGIN_ACCESS_READ_ON_ENTRY == flags) {
rc = acl_read_access_allowed_on_entry ( pb, e, attrs, access);
- else if ( ACLPLUGIN_ACCESS_READ_ON_ATTR == flags) {
+ } else if ( ACLPLUGIN_ACCESS_READ_ON_ATTR == flags) {
if (attr == NULL) {
- slapi_log_error( SLAPI_LOG_PLUGIN, plugin_name, "Missing attribute\n" );
+ slapi_log_error( SLAPI_LOG_PLUGIN, plugin_name, "Missing attribute\n" );
rc = LDAP_OPERATIONS_ERROR;
} else {
rc = acl_read_access_allowed_on_attr ( pb, e, attr, val, access);
diff --git a/ldap/servers/slapd/attr.c b/ldap/servers/slapd/attr.c
index b9cbe17..ef3fa10 100644
--- a/ldap/servers/slapd/attr.c
+++ b/ldap/servers/slapd/attr.c
@@ -76,8 +76,12 @@ typedef struct slapi_attr_value_index {
* find the next component of an attribute type.
*/
static const char *
-next_comp( const char *s )
+next_comp( const char *sp )
{
+ char *s = (char *)sp;
+ if (NULL == s) {
+ return( NULL );
+ }
while ( *s && *s != ';' ) {
s++;
}
@@ -93,14 +97,30 @@ next_comp( const char *s )
* compare two components of an attribute type.
*/
static int
-comp_cmp( const char *s1, const char *s2 )
+comp_cmp( const char *s1p, const char *s2p )
{
- while ( *s1 && *s1 != ';' && tolower( *s1 ) == tolower( *s2 ) ) {
+ char *s1 = (char *)s1p;
+ char *s2 = (char *)s2p;
+ if (NULL == s1) {
+ if (s2) {
+ return 1;
+ }
+ return 0;
+ }
+ if (NULL == s2) {
+ if (s1) {
+ return 1;
+ }
+ return 0;
+ }
+ while ( *s1 && (*s1 != ';') &&
+ *s2 && (*s2 != ';') &&
+ tolower( *s1 ) == tolower( *s2 ) ) {
s1++, s2++;
}
if ( *s1 != *s2 ) {
- if ( (*s1 == '\0' || *s1 == ';') &&
- (*s2 == '\0' || *s2 == ';') ) {
+ if ((*s1 == '\0' || *s1 == ';') &&
+ (*s2 == '\0' || *s2 == ';')) {
return( 0 );
} else {
return( 1 );
@@ -115,16 +135,18 @@ slapi_attr_type_cmp( const char *a1, const char *a2, int opt )
{
int rc= 0;
- switch ( opt ) {
- case SLAPI_TYPE_CMP_EXACT: /* compare base name + options as given */
- rc = strcasecmp( a1, a2 );
+ switch ( opt ) {
+ case SLAPI_TYPE_CMP_EXACT: /* compare base name + options as given */
+ rc = strcasecmp( a1, a2 );
break;
- case SLAPI_TYPE_CMP_BASE: /* ignore options on both names - compare base names only */
+ case SLAPI_TYPE_CMP_BASE: /* ignore options on both names - compare base names only */
rc = comp_cmp( a1, a2 );
- break;
+ break;
- case SLAPI_TYPE_CMP_SUBTYPE: /* ignore options on second name that are not in first name */
+ case SLAPI_TYPE_CMP_SUBTYPE: /* ignore options on second name that are not in first name */
+ {
+ const char *b2 = a2;
/*
* first, check that the base types match
*/
@@ -134,24 +156,73 @@ slapi_attr_type_cmp( const char *a1, const char *a2, int opt )
}
/*
* next, for each component in a1, make sure there is a
- * matching component in a2. the order must be the same,
- * so we can keep looking where we left off each time in a2
+ * matching component in a2.
*/
rc = 0;
- for ( a1 = next_comp( a1 ); a1 != NULL; a1 = next_comp( a1 ) ) {
- for ( a2 = next_comp( a2 ); a2 != NULL;
- a2 = next_comp( a2 ) ) {
- if ( comp_cmp( a1, a2 ) == 0 ) {
+ for ( a1 = next_comp( a1 ); a1; a1 = next_comp( a1 ) ) {
+ for ( b2 = next_comp( b2 ); b2; b2 = next_comp( b2 ) ) {
+ if ( comp_cmp( a1, b2 ) == 0 ) {
break;
}
}
- if ( a2 == NULL ) {
+ if ( b2 == NULL ) {
rc = 1;
break;
}
+ b2 = a2;
}
- break;
- }
+ break;
+ }
+ case SLAPI_TYPE_CMP_SUBTYPES: /* Both share the same subtypes */
+ {
+ const char *b1 = a1;
+ const char *b2 = a2;
+ /*
+ * first, check that the base types match
+ */
+ if (comp_cmp(a1, a2) != 0) {
+ rc = 1;
+ break;
+ }
+ /*
+ * next, for each component in a1, make sure there is a
+ * matching component in a2.
+ */
+ rc = 0;
+ for (b1 = next_comp(b1); b1; b1 = next_comp(b1)) {
+ for (b2 = next_comp(b2); b2; b2 = next_comp(b2)) {
+ if (comp_cmp(b1, b2) == 0) {
+ break;
+ }
+ }
+ if (b2 == NULL) {
+ rc = 1;
+ break;
+ }
+ b2 = a2;
+ }
+ if (0 == rc) {
+ b1 = a1;
+ b2 = a2;
+ /*
+ * next, for each component in a2, make sure there is a
+ * matching component in a1.
+ */
+ for (b2 = next_comp(b2); b2; b2 = next_comp(b2)) {
+ for (b1 = next_comp(b1); b1; b1 = next_comp(b1)) {
+ if (comp_cmp(b1, b2) == 0) {
+ break;
+ }
+ }
+ if (b1 == NULL) {
+ rc = 1;
+ break;
+ }
+ b1 = a1;
+ }
+ }
+ } /* SLAPI_TYPE_CMP_SUBTYPES */
+ } /* switch (opt) */
return( rc );
}
diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c
index 655717f..ef4179d 100644
--- a/ldap/servers/slapd/result.c
+++ b/ldap/servers/slapd/result.c
@@ -1002,17 +1002,17 @@ encode_attr_2(
attrs[0] = (char*)attribute_type;
#if !defined(DISABLE_ACL_CHECK)
- if ( plugin_call_acl_plugin (pb, e, attrs, NULL, SLAPI_ACL_READ,
- ACLPLUGIN_ACCESS_READ_ON_ATTR, NULL ) != LDAP_SUCCESS ) {
+ if (plugin_call_acl_plugin(pb, e, attrs, NULL, SLAPI_ACL_READ,
+ ACLPLUGIN_ACCESS_READ_ON_ATTR, NULL) != LDAP_SUCCESS) {
return( 0 );
}
#endif
- if ( ber_printf( ber, "{s[", returned_type ) == -1 ) {
+ if (ber_printf(ber, "{s[", returned_type?returned_type:attribute_type) == -1) {
LDAPDebug( LDAP_DEBUG_ANY, "ber_printf failed\n", 0, 0, 0 );
ber_free( ber, 1 );
- send_ldap_result( pb, LDAP_OPERATIONS_ERROR, NULL,
- "ber_printf type", 0, NULL );
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL,
+ "ber_printf type", 0, NULL);
return( -1 );
}
@@ -1146,7 +1146,7 @@ static const char *idds_map_attrt_v3(
/* Helper functions */
-static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_PBlock *pb,BerElement *ber,int attrsonly,int ldapversion,int *dontsendattr, int real_attrs_only, int some_named_attrs)
+static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_PBlock *pb,BerElement *ber,int attrsonly,int ldapversion, int real_attrs_only, int some_named_attrs)
{
int i = 0;
int rc = 0;
@@ -1259,10 +1259,11 @@ static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_
&attr_free_flags,
&item_count
);
- if (0 == rc && item_count > 0) {
+ if ((0 == rc) && (item_count > 0)) {
for(iter=0; iter<item_count; iter++)
{
+ int skipit;
if ( rc != 0 ) {
/* we hit an error - we need to free all of the stuff allocated by
slapi_vattr_namespace_values_get_sp */
@@ -1274,31 +1275,28 @@ static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_
name_to_return = actual_type_name[iter];
}
- /*
- * The dontsendattr array is used to track whether attributes
- * that were explicitly requested by the client have been
- * returned. Check here to see if the attribute we just
- * arranged to send back was explicitly requested, and if so,
- * set its dontsendattr flag so the send_specific_attrs()
- * function does not return it a second time.
- */
- for ( i = 0; attrs != NULL && attrs[i] != NULL; i++ ) {
- if ( !dontsendattr[i] && slapi_attr_type_cmp( current_type_name, attrs[i], SLAPI_TYPE_CMP_SUBTYPE ) == 0 ) {
- /* Client is also asking for an attr which is in '*', zap it. */
- dontsendattr[i]= 1;
+ /* If current_type_name is in attrs, we could rely on send_specific_attrs. */
+ skipit = 0;
+ for (i = 0; attrs && attrs[i]; i++) {
+ if (slapi_attr_type_cmp(current_type_name, attrs[i], SLAPI_TYPE_CMP_SUBTYPE) == 0) {
+ skipit = 1;
+ break;
}
}
- rc = encode_attr_2( pb, ber, e, values[iter], attrsonly, current_type_name, name_to_return );
-
- if (rewrite_rfc1274 != 0) {
- v2name = idds_map_attrt_v3(current_type_name);
- if (v2name != NULL) {
- /* also return values with RFC1274 attr name */
- rc = encode_attr_2(pb, ber, e, values[iter],
- attrsonly,
- current_type_name,
- v2name);
+ if (!skipit) {
+ rc = encode_attr_2(pb, ber, e, values[iter], attrsonly,
+ current_type_name, name_to_return );
+
+ if (rewrite_rfc1274 != 0) {
+ v2name = idds_map_attrt_v3(current_type_name);
+ if (v2name != NULL) {
+ /* also return values with RFC1274 attr name */
+ rc = encode_attr_2(pb, ber, e, values[iter],
+ attrsonly,
+ current_type_name,
+ v2name);
+ }
}
}
@@ -1329,137 +1327,143 @@ exit:
return rc;
}
-int send_specific_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_PBlock *pb,BerElement *ber,int attrsonly,int ldapversion,int *dontsendattr, int real_attrs_only)
+/*
+ * attrs need to expand including the subtypes found in the entry
+ * e.g., if "sn" is no the attrs and 'e' has sn, sn;en, and sn;fr,
+ * attrs should have sn, sn;en, and sn;fr, as well.
+ */
+int
+send_specific_attrs(Slapi_Entry *e, char **attrs, Slapi_Operation *op,
+ Slapi_PBlock *pb, BerElement *ber, int attrsonly,
+ int ldapversion, int real_attrs_only)
{
- int i,j = 0;
+ int i = 0;
int rc = 0;
int vattr_flags = 0;
vattr_context *ctx;
+ char **attrs_ext = NULL;
+ char **my_searchattrs = NULL;
- if(real_attrs_only == SLAPI_SEND_VATTR_FLAG_REALONLY)
+ if (real_attrs_only == SLAPI_SEND_VATTR_FLAG_REALONLY) {
vattr_flags = SLAPI_REALATTRS_ONLY;
- else
- {
+ } else {
vattr_flags = SLAPI_VIRTUALATTRS_REQUEST_POINTERS;
if(real_attrs_only == SLAPI_SEND_VATTR_FLAG_VIRTUALONLY)
vattr_flags |= SLAPI_VIRTUALATTRS_ONLY;
}
+
+ /* Create a copy of attrs with no duplicates */
+ if (attrs) {
+ for (i = 0; attrs[i]; i++) {
+ if (!charray_inlist(attrs_ext, attrs[i])) {
+ slapi_ch_array_add(&attrs_ext, slapi_ch_strdup(attrs[i]));
+ slapi_ch_array_add(&my_searchattrs, slapi_ch_strdup(op->o_searchattrs[i]));
+ }
+ }
+ }
+ if (attrs_ext) {
+ attrs = attrs_ext;
+ }
- for ( i = 0; attrs != NULL && attrs[i] != NULL; i++ )
- {
+ for ( i = 0; attrs && attrs[i] != NULL; i++ ) {
char *current_type_name = attrs[i];
- if(!dontsendattr[i]) {
- Slapi_ValueSet **values = NULL;
- int attr_free_flags = 0;
- char *name_to_return = NULL;
- char **actual_type_name= NULL;
- int *type_name_disposition = 0;
- int item_count = 0;
- int iter = 0;
- Slapi_DN *namespace_dn;
- Slapi_Backend *backend=0;
-
- /*
- * Here we call the computed attribute code to see whether
- * the requested attribute is to be computed.
- * The subroutine compute_attribute calls encode_attr on our behalf, in order
- * to avoid the inefficiency of returning a complex structure
- * which we'd have to free
- */
- rc = compute_attribute(attrs[i],pb,ber,e,attrsonly,op->o_searchattrs[i]);
- if (0 == rc) {
- continue; /* Means this was a computed attr and we prcessed it OK. */
- }
- if (-1 != rc) {
- /* Means that some error happened */
- return rc;
- }
- else {
- rc = 0; /* Means that we just didn't recognize this as a computed attr */
- }
-
- /* get the namespace dn */
- slapi_pblock_get( pb, SLAPI_BACKEND, (void *)&backend);
- namespace_dn = (Slapi_DN*)slapi_be_getsuffix(backend, 0);
+ Slapi_ValueSet **values = NULL;
+ int attr_free_flags = 0;
+ char *name_to_return = NULL;
+ char **actual_type_name= NULL;
+ int *type_name_disposition = 0;
+ int item_count = 0;
+ int iter = 0;
+ Slapi_DN *namespace_dn;
+ Slapi_Backend *backend=0;
- /* Get the attribute value from the vattr service */
- /* ctx will be freed by attr_context_ungrok() */
- ctx = vattr_context_new ( pb );
- rc = slapi_vattr_namespace_values_get_sp(
- ctx,
- e,
- namespace_dn,
- current_type_name,
- &values,
- &type_name_disposition,
- &actual_type_name,
- vattr_flags,
- &attr_free_flags,
- &item_count
- );
- if (0 == rc && item_count > 0) {
+ /*
+ * Here we call the computed attribute code to see whether
+ * the requested attribute is to be computed.
+ * The subroutine compute_attribute calls encode_attr on our behalf, in order
+ * to avoid the inefficiency of returning a complex structure
+ * which we'd have to free
+ */
+ rc = compute_attribute(attrs[i], pb, ber, e, attrsonly, my_searchattrs[i]);
+ if (0 == rc) {
+ continue; /* Means this was a computed attr and we prcessed it OK. */
+ }
+ if (-1 != rc) {
+ /* Means that some error happened */
+ return rc;
+ }
+ else {
+ rc = 0; /* Means that we just didn't recognize this as a computed attr */
+ }
- for(iter=0; iter<item_count; iter++)
- {
- if ( rc != 0 ) {
- /* we hit an error - we need to free all of the stuff allocated by
- slapi_vattr_namespace_values_get_sp */
- slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+ /* get the namespace dn */
+ slapi_pblock_get( pb, SLAPI_BACKEND, (void *)&backend);
+ namespace_dn = (Slapi_DN*)slapi_be_getsuffix(backend, 0);
+
+ /* Get the attribute value from the vattr service */
+ /* This call handles subtype, as well.
+ * e.g., current_type_name: cn
+ * ==>
+ * item_count: 5; actual_type_name: cn;sub0, ..., cn;sub4 */
+ /* ctx will be freed by attr_context_ungrok() */
+ ctx = vattr_context_new ( pb );
+ rc = slapi_vattr_namespace_values_get_sp(
+ ctx,
+ e,
+ namespace_dn,
+ current_type_name,
+ &values,
+ &type_name_disposition,
+ &actual_type_name,
+ vattr_flags,
+ &attr_free_flags,
+ &item_count
+ );
+ if ((0 == rc) && (item_count > 0)) {
+ for (iter = 0; iter < item_count; iter++) {
+ if ( rc != 0 ) {
+ /* we hit an error - we need to free all of the stuff allocated by
+ slapi_vattr_namespace_values_get_sp */
+ slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+ continue;
+ }
+ if (SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_SUBTYPE == type_name_disposition[iter]) {
+ name_to_return = actual_type_name[iter];
+ if ((iter > 0) && charray_inlist(attrs, name_to_return)) {
+ /* subtype retrieved by slapi_vattr_namespace_values_get_sp is
+ * included in the attr list. Skip the dup. */
continue;
}
- if (SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_SUBTYPE == type_name_disposition[iter]) {
- name_to_return = actual_type_name[iter];
- } else {
- name_to_return = op->o_searchattrs[i];
- }
-
- /*
- * The client may have specified a list of attributes
- * with duplicates, 'cn cn cn'.
- * We need to determine which of any duplicates take precedence
- * For subtypes, the attribute which is most generic should be
- * returned (since it will also trigger the return of the less
- * generic attribute subtypes.
- */
- for ( j = i+1; attrs[j] != NULL && dontsendattr[i]==0; j++ )
- {
- if ( !dontsendattr[j] && slapi_attr_type_cmp( attrs[j], actual_type_name[iter], SLAPI_TYPE_CMP_SUBTYPE ) == 0 )
- {
- /* discover which is the more generic attribute and cancel the other*/
- int attrbase = slapi_attr_type_cmp( attrs[j], current_type_name, SLAPI_TYPE_CMP_EXACT );
-
- if(attrbase >= 0)
- dontsendattr[j]= 1;
- else
- dontsendattr[i]= 1; /* the current value is superceeded later */
- }
- }
-
- /* we may have just cancelled ourselves so check */
- if(!dontsendattr[i])
- rc = encode_attr_2( pb, ber, e, values[iter], attrsonly, current_type_name, name_to_return );
-
- slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+ } else {
+ name_to_return = my_searchattrs[i];
}
- slapi_ch_free((void**)&actual_type_name);
- slapi_ch_free((void**)&type_name_disposition);
- slapi_ch_free((void**)&values);
- if ( rc != 0 ) {
- goto exit;
- }
+ /* need to pass actual_type_name (e.g., sn;en to evaluate the ACL */
+ rc = encode_attr_2(pb, ber, e, values[iter], attrsonly,
+ actual_type_name[iter], name_to_return);
+
+ slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+ }
- } else {
- /* if we got here, then either values is NULL or values contains no elements
- either way we can free it */
- slapi_ch_free((void**)&values);
- slapi_ch_free((void**)&actual_type_name);
- slapi_ch_free((void**)&type_name_disposition);
- rc = 0;
+ slapi_ch_free((void**)&actual_type_name);
+ slapi_ch_free((void**)&type_name_disposition);
+ slapi_ch_free((void**)&values);
+ if ( rc != 0 ) {
+ goto exit;
}
- }
+
+ } else {
+ /* if we got here, then either values is NULL or values contains no elements
+ either way we can free it */
+ slapi_ch_free((void**)&values);
+ slapi_ch_free((void**)&actual_type_name);
+ slapi_ch_free((void**)&type_name_disposition);
+ rc = 0;
+ }
}
exit:
+ slapi_ch_array_free(attrs_ext);
+ slapi_ch_array_free(my_searchattrs);
return rc;
}
@@ -1482,7 +1486,6 @@ send_ldap_search_entry_ext(
BerElement *ber = NULL;
int i, rc = 0, logit = 0;
int alluserattrs, noattrs, some_named_attrs;
- int *dontsendattr= NULL;
Slapi_Operation *operation;
int real_attrs_only = 0;
LDAPControl **ctrlp = 0;
@@ -1596,11 +1599,6 @@ send_ldap_search_entry_ext(
"Accepting illegal other attributes specified with "
"special \"1.1\" attribute\n", 0, 0, 0 );
}
- /*
- * We maintain a flag array so that we can remove requests
- * for duplicate attributes.
- */
- dontsendattr = (int*) slapi_ch_calloc( i+1, sizeof(int) );
}
@@ -1625,12 +1623,13 @@ send_ldap_search_entry_ext(
/* look through each attribute in the entry */
if ( alluserattrs ) {
- rc = send_all_attrs(e,attrs,op,pb,ber,attrsonly,conn->c_ldapversion,dontsendattr, real_attrs_only, some_named_attrs);
+ rc = send_all_attrs(e, attrs, op, pb, ber, attrsonly, conn->c_ldapversion,
+ real_attrs_only, some_named_attrs);
}
/* if the client explicitly specified a list of attributes look through each attribute requested */
if( (rc == 0) && (attrs!=NULL) && !noattrs) {
- rc = send_specific_attrs(e,attrs,op,pb,ber,attrsonly,conn->c_ldapversion,dontsendattr,real_attrs_only);
+ rc = send_specific_attrs(e,attrs,op,pb,ber,attrsonly,conn->c_ldapversion,real_attrs_only);
}
/* Append effective rights to the stream of attribute list */
@@ -1738,7 +1737,6 @@ cleanup:
ldap_controls_free(searchctrlp);
}
ber_free( ber, 1 );
- slapi_ch_free( (void **) &dontsendattr );
LDAPDebug( LDAP_DEBUG_TRACE, "<= send_ldap_search_entry\n", 0, 0, 0 );
return( rc );
@@ -2153,7 +2151,6 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
LDAPControl **ctrlp = NULL;
struct berval *bv = NULL;
BerElement *ber = NULL;
- int *dontsendattr = NULL;
int real_attrs_only = 0;
int rc = 0;
@@ -2191,13 +2188,12 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
* for duplicate attributes. We also need to set o_searchattrs
* for the attribute processing, as modify op's don't have search attrs.
*/
- dontsendattr = (int*) slapi_ch_calloc( attr_count+1, sizeof(int) );
op->o_searchattrs = attrs;
/* Send all the attributes */
if ( alluserattrs ) {
rc = send_all_attrs(e, attrs, op, pb, ber, 0, conn->c_ldapversion,
- dontsendattr, real_attrs_only, attr_count);
+ real_attrs_only, attr_count);
if(rc){
goto cleanup;
}
@@ -2205,8 +2201,7 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
/* Send a specified list of attributes */
if( attrs != NULL) {
- rc = send_specific_attrs(e, attrs, op, pb, ber, 0, conn->c_ldapversion,
- dontsendattr, real_attrs_only);
+ rc = send_specific_attrs(e, attrs, op, pb, ber, 0, conn->c_ldapversion, real_attrs_only);
if(rc){
goto cleanup;
}
@@ -2222,7 +2217,6 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
cleanup:
ber_free( ber, 1 );
- slapi_ch_free( (void **) &dontsendattr );
if(rc != 0){
return NULL;
} else {
diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c
index 59c4afb..29729bb 100644
--- a/ldap/servers/slapd/search.c
+++ b/ldap/servers/slapd/search.c
@@ -246,7 +246,15 @@ do_search( Slapi_PBlock *pb )
}
if ( attrs != NULL ) {
- int aciin = 0;
+ char *normaci = slapi_attr_syntax_normalize("aci");
+ int replace_aci = 0;
+ if (0 == strcasecmp(normaci, "aci")) {
+ /* normaci is identical to "aci" */
+ slapi_ch_free_string(&normaci);
+ normaci = slapi_ch_strdup("aci");
+ } else {
+ replace_aci = 1;
+ }
/*
* . store gerattrs if any
* . add "aci" once if "*" is given
@@ -274,13 +282,25 @@ do_search( Slapi_PBlock *pb )
charray_merge_nodup(&gerattrs, dummyary, 1);
/* null terminate the attribute name at the @ after it has been copied */
*p = '\0';
- }
- else if ( !aciin && strcasecmp(attrs[i], LDAP_ALL_USER_ATTRS) == 0 )
- {
- charray_add(&attrs, slapi_attr_syntax_normalize("aci"));
- aciin = 1;
+ } else if (strcmp(attrs[i], LDAP_ALL_USER_ATTRS /* '*' */) == 0) {
+ if (!charray_inlist(attrs, normaci)) {
+ charray_add(&attrs, normaci); /* consumed */
+ normaci = NULL;
+ }
+ } else if (replace_aci && (strcasecmp(attrs[i], "aci") == 0)) {
+ slapi_ch_free_string(&attrs[i]);
+ if (charray_inlist(attrs, normaci)) { /* attrlist: "*" "aci" */
+ int j = i;
+ for (; attrs[j]; j++) { /* Shift the rest by 1 */
+ attrs[j] = attrs[j+1];
+ }
+ } else { /* attrlist: "aci" "*" */
+ attrs[i] = normaci; /* consumed */
+ normaci = NULL;
+ }
}
}
+ slapi_ch_free_string(&normaci);
if (config_get_return_orig_type_switch()) {
/* return the original type, e.g., "sn (surname)" */
@@ -376,6 +396,7 @@ free_and_return:;
if ( !psearch || rc != 0 ) {
slapi_ch_free_string(&fstr);
slapi_filter_free( filter, 1 );
+ slapi_pblock_get( pb, SLAPI_SEARCH_ATTRS, &attrs );
charray_free( attrs ); /* passing NULL is fine */
charray_free( gerattrs ); /* passing NULL is fine */
/*
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index bb384f4..13a332c 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -4080,6 +4080,7 @@ int slapi_attr_value_find( const Slapi_Attr *a, const struct berval *v );
* \arg #SLAPI_TYPE_CMP_EXACT
* \arg #SLAPI_TYPE_CMP_BASE
* \arg #SLAPI_TYPE_CMP_SUBTYPE
+ * \arg #SLAPI_TYPE_CMP_SUBTYPES
* \return \c 0 if the type names are equal.
* \return A non-zero value if the type names are not equal.
* \see slapi_attr_type2plugin()
@@ -4112,6 +4113,13 @@ int slapi_attr_type_cmp( const char *t1, const char *t2, int opt );
#define SLAPI_TYPE_CMP_SUBTYPE 2
/**
+ * Compare types including subtypes in the both args.
+ *
+ * \see slapi_attr_type_cmp()
+ */
+#define SLAPI_TYPE_CMP_SUBTYPES 3
+
+/**
* Compare two attribute names to determine if they represent the same value.
*
* \param t1 Pointer to the first attribute you want to compare.
10 years, 3 months
- 1
- 0
ldap/servers
by Noriko Hosoi
ldap/servers/slapd/attr.c | 1 -
ldap/servers/slapd/search.c | 20 +++++---------------
2 files changed, 5 insertions(+), 16 deletions(-)
New commits:
commit 36c48b8395c6e4ca374c74f91dad6b022aa99ce1
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Mon Jan 13 14:43:46 2014 -0800
Ticket #47571 - targetattr ACIs ignore subtype
Description: commit 85a78741dfeb636a1cf7cced1576278e65f5bb58
introduced 2 coverity issues:
12423: Explicit null dereferenced
do_search (slapd/search.c)
If attribute list contains multiple "*"s and "aci"s in ldapsearch,
the previous code attempted to add "aci" once for "*" and replacing
"aci" with normalized aci (if any) once and eliminating the duplicated
"aci"s. The code contained a null dereferenced bug. Even if duplicated
attributes are included in the attribute list, they are removed later
in send_specific_attrs. Thus, this patch simplifies the logic to avoid
the null dereference.
12422: Logically dead code
comp_cmp (slapd/attr.c)
Eliminated the dead code (case s1 == NULL AND s2 == NULL).
https://fedorahosted.org/389/ticket/47571
Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!)
diff --git a/ldap/servers/slapd/attr.c b/ldap/servers/slapd/attr.c
index ef3fa10..39c6e99 100644
--- a/ldap/servers/slapd/attr.c
+++ b/ldap/servers/slapd/attr.c
@@ -111,7 +111,6 @@ comp_cmp( const char *s1p, const char *s2p )
if (s1) {
return 1;
}
- return 0;
}
while ( *s1 && (*s1 != ';') &&
*s2 && (*s2 != ';') &&
diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c
index 29729bb..d68ba7b 100644
--- a/ldap/servers/slapd/search.c
+++ b/ldap/servers/slapd/search.c
@@ -248,11 +248,10 @@ do_search( Slapi_PBlock *pb )
if ( attrs != NULL ) {
char *normaci = slapi_attr_syntax_normalize("aci");
int replace_aci = 0;
- if (0 == strcasecmp(normaci, "aci")) {
- /* normaci is identical to "aci" */
- slapi_ch_free_string(&normaci);
+ if (!normaci) {
normaci = slapi_ch_strdup("aci");
- } else {
+ } else if (strcasecmp(normaci, "aci")) {
+ /* normaci is not "aci" */
replace_aci = 1;
}
/*
@@ -284,20 +283,11 @@ do_search( Slapi_PBlock *pb )
*p = '\0';
} else if (strcmp(attrs[i], LDAP_ALL_USER_ATTRS /* '*' */) == 0) {
if (!charray_inlist(attrs, normaci)) {
- charray_add(&attrs, normaci); /* consumed */
- normaci = NULL;
+ charray_add(&attrs, slapi_ch_strdup(normaci));
}
} else if (replace_aci && (strcasecmp(attrs[i], "aci") == 0)) {
slapi_ch_free_string(&attrs[i]);
- if (charray_inlist(attrs, normaci)) { /* attrlist: "*" "aci" */
- int j = i;
- for (; attrs[j]; j++) { /* Shift the rest by 1 */
- attrs[j] = attrs[j+1];
- }
- } else { /* attrlist: "aci" "*" */
- attrs[i] = normaci; /* consumed */
- normaci = NULL;
- }
+ attrs[i] = slapi_ch_strdup(normaci);
}
}
slapi_ch_free_string(&normaci);
10 years, 3 months
- 1
- 0
Branch '389-ds-base-1.3.1' - ldap/servers
by Noriko Hosoi
ldap/servers/slapd/libglobs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
New commits:
commit 4aa849fa0a32d90e7d88574f35e1e17fbaf1034f
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Mon Jan 13 11:03:46 2014 -0800
Ticket #47660 - config_set_allowed_to_delete_attrs: Valgrind reports Invalid read
Description: There was a logic error in checking the availability of
a pointer. Before checking the contents of an address, the correctness
of the pointer needed to be checked.
Also, one memory leak was found in the error return case.
Note: these 2 issues were introduece by this commit:
commit 94b123780b21e503b78bceca9d60904206ef91fa
Trac Ticket #447 - Possible to add invalid attribute to nsslapd-allowed-to-delete-attrs
https://fedorahosted.org/389/ticket/47660
Reviewed by rmeggins(a)redhat.com (Thank you, Rich!)
(cherry picked from commit 1a788bf35a138d221f2bfb88d6da5fc5244d738c)
(cherry picked from commit 22c24f0d133cfcfc9f7457a84282d223ea3f6e25)
diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index 6df225d..bcf7db4 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/ldap/servers/slapd/libglobs.c
@@ -6704,7 +6704,7 @@ config_set_allowed_to_delete_attrs( const char *attrname, char *value,
int needcopy = 0;
allowed = slapi_str2charray_ext(vcopy, " ", 0);
for (s = allowed; s && *s; s++) ;
- for (--s; s && *s && (s >= allowed); s--) {
+ for (--s; s && (s >= allowed) && *s; s--) {
cgas = (struct config_get_and_set *)PL_HashTableLookup(confighash,
*s);
if (!cgas && PL_strcasecmp(*s, "aci") /* aci is an exception */) {
@@ -6725,6 +6725,7 @@ config_set_allowed_to_delete_attrs( const char *attrname, char *value,
slapi_log_error(SLAPI_LOG_FATAL, "config",
"%s: Given attributes are all invalid. No effects.\n",
CONFIG_ALLOWED_TO_DELETE_ATTRIBUTE);
+ slapi_ch_array_free(allowed);
return LDAP_NO_SUCH_ATTRIBUTE;
} else {
for (s = allowed, d = vcopy; s && *s; s++) {
10 years, 3 months
- 1
- 0
Branch '389-ds-base-1.3.2' - ldap/servers
by Noriko Hosoi
ldap/servers/slapd/libglobs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
New commits:
commit 22c24f0d133cfcfc9f7457a84282d223ea3f6e25
Author: Noriko Hosoi <nhosoi(a)redhat.com>
Date: Mon Jan 13 11:03:46 2014 -0800
Ticket #47660 - config_set_allowed_to_delete_attrs: Valgrind reports Invalid read
Description: There was a logic error in checking the availability of
a pointer. Before checking the contents of an address, the correctness
of the pointer needed to be checked.
Also, one memory leak was found in the error return case.
Note: these 2 issues were introduece by this commit:
commit 94b123780b21e503b78bceca9d60904206ef91fa
Trac Ticket #447 - Possible to add invalid attribute to nsslapd-allowed-to-delete-attrs
https://fedorahosted.org/389/ticket/47660
Reviewed by rmeggins(a)redhat.com (Thank you, Rich!)
(cherry picked from commit 1a788bf35a138d221f2bfb88d6da5fc5244d738c)
diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index a8ad3cc..940b60d 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/ldap/servers/slapd/libglobs.c
@@ -6759,7 +6759,7 @@ config_set_allowed_to_delete_attrs( const char *attrname, char *value,
int needcopy = 0;
allowed = slapi_str2charray_ext(vcopy, " ", 0);
for (s = allowed; s && *s; s++) ;
- for (--s; s && *s && (s >= allowed); s--) {
+ for (--s; s && (s >= allowed) && *s; s--) {
cgas = (struct config_get_and_set *)PL_HashTableLookup(confighash,
*s);
if (!cgas && PL_strcasecmp(*s, "aci") /* aci is an exception */) {
@@ -6780,6 +6780,7 @@ config_set_allowed_to_delete_attrs( const char *attrname, char *value,
slapi_log_error(SLAPI_LOG_FATAL, "config",
"%s: Given attributes are all invalid. No effects.\n",
CONFIG_ALLOWED_TO_DELETE_ATTRIBUTE);
+ slapi_ch_array_free(allowed);
return LDAP_NO_SUCH_ATTRIBUTE;
} else {
for (s = allowed, d = vcopy; s && *s; s++) {
10 years, 3 months
- 1
- 0