| [2761] | 1 | From a9cd2ffd227c19a458b27415dedaaf4a6b4778ec Mon Sep 17 00:00:00 2001 | 
|---|
 | 2 | From: Mark Reynolds <mreynolds@redhat.com> | 
|---|
 | 3 | Date: Thu, 11 Jun 2015 12:28:07 -0400 | 
|---|
 | 4 | Subject: [PATCH] Ticket 47921 - indirect cos does not reflect changes in the | 
|---|
 | 5 |  cos attribute | 
|---|
 | 6 |  | 
|---|
 | 7 | Bug Description:  Indirect cos results are incorrectly cached, so any changes | 
|---|
 | 8 |                   to entries that are indirect are not returned to the client. | 
|---|
 | 9 |  | 
|---|
 | 10 | Fix Description:  Do not cache the vattr result if it came from a indirect cos | 
|---|
 | 11 |                   definition. | 
|---|
 | 12 |  | 
|---|
 | 13 | https://fedorahosted.org/389/ticket/47921 | 
|---|
 | 14 |  | 
|---|
 | 15 | Reviewed by: ? | 
|---|
 | 16 | --- | 
|---|
 | 17 |  dirsrvtests/tickets/ticket47921_test.py | 155 ++++++++++++++++++++++++++++++++ | 
|---|
 | 18 |  ldap/servers/plugins/cos/cos_cache.c    |  26 ++++-- | 
|---|
 | 19 |  2 files changed, 174 insertions(+), 7 deletions(-) | 
|---|
 | 20 |  create mode 100644 dirsrvtests/tickets/ticket47921_test.py | 
|---|
 | 21 |  | 
|---|
 | 22 | diff --git a/dirsrvtests/tickets/ticket47921_test.py b/dirsrvtests/tickets/ticket47921_test.py | 
|---|
 | 23 | new file mode 100644 | 
|---|
 | 24 | index 0000000..951d33b | 
|---|
 | 25 | --- /dev/null | 
|---|
 | 26 | +++ b/dirsrvtests/tickets/ticket47921_test.py | 
|---|
 | 27 | @@ -0,0 +1,155 @@ | 
|---|
 | 28 | +import os | 
|---|
 | 29 | +import sys | 
|---|
 | 30 | +import time | 
|---|
 | 31 | +import ldap | 
|---|
 | 32 | +import logging | 
|---|
 | 33 | +import pytest | 
|---|
 | 34 | +from lib389 import DirSrv, Entry, tools, tasks | 
|---|
 | 35 | +from lib389.tools import DirSrvTools | 
|---|
 | 36 | +from lib389._constants import * | 
|---|
 | 37 | +from lib389.properties import * | 
|---|
 | 38 | +from lib389.tasks import * | 
|---|
 | 39 | +from lib389.utils import * | 
|---|
 | 40 | + | 
|---|
 | 41 | +logging.getLogger(__name__).setLevel(logging.DEBUG) | 
|---|
 | 42 | +log = logging.getLogger(__name__) | 
|---|
 | 43 | + | 
|---|
 | 44 | +installation1_prefix = None | 
|---|
 | 45 | + | 
|---|
 | 46 | + | 
|---|
 | 47 | +class TopologyStandalone(object): | 
|---|
 | 48 | +    def __init__(self, standalone): | 
|---|
 | 49 | +        standalone.open() | 
|---|
 | 50 | +        self.standalone = standalone | 
|---|
 | 51 | + | 
|---|
 | 52 | + | 
|---|
 | 53 | +@pytest.fixture(scope="module") | 
|---|
 | 54 | +def topology(request): | 
|---|
 | 55 | +    global installation1_prefix | 
|---|
 | 56 | +    if installation1_prefix: | 
|---|
 | 57 | +        args_instance[SER_DEPLOYED_DIR] = installation1_prefix | 
|---|
 | 58 | + | 
|---|
 | 59 | +    # Creating standalone instance ... | 
|---|
 | 60 | +    standalone = DirSrv(verbose=False) | 
|---|
 | 61 | +    args_instance[SER_HOST] = HOST_STANDALONE | 
|---|
 | 62 | +    args_instance[SER_PORT] = PORT_STANDALONE | 
|---|
 | 63 | +    args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE | 
|---|
 | 64 | +    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX | 
|---|
 | 65 | +    args_standalone = args_instance.copy() | 
|---|
 | 66 | +    standalone.allocate(args_standalone) | 
|---|
 | 67 | +    instance_standalone = standalone.exists() | 
|---|
 | 68 | +    if instance_standalone: | 
|---|
 | 69 | +        standalone.delete() | 
|---|
 | 70 | +    standalone.create() | 
|---|
 | 71 | +    standalone.open() | 
|---|
 | 72 | + | 
|---|
 | 73 | +    # Clear out the tmp dir | 
|---|
 | 74 | +    standalone.clearTmpDir(__file__) | 
|---|
 | 75 | + | 
|---|
 | 76 | +    return TopologyStandalone(standalone) | 
|---|
 | 77 | + | 
|---|
 | 78 | + | 
|---|
 | 79 | +def test_ticket47921(topology): | 
|---|
 | 80 | +    ''' | 
|---|
 | 81 | +    Test that indirect cos reflects the current value of the indirect entry | 
|---|
 | 82 | +    ''' | 
|---|
 | 83 | + | 
|---|
 | 84 | +    INDIRECT_COS_DN = 'cn=cos definition,' + DEFAULT_SUFFIX | 
|---|
 | 85 | +    MANAGER_DN = 'uid=my manager,ou=people,' + DEFAULT_SUFFIX | 
|---|
 | 86 | +    USER_DN = 'uid=user,ou=people,' + DEFAULT_SUFFIX | 
|---|
 | 87 | + | 
|---|
 | 88 | +    # Add COS definition | 
|---|
 | 89 | +    try: | 
|---|
 | 90 | +        topology.standalone.add_s(Entry((INDIRECT_COS_DN, | 
|---|
 | 91 | +            {'objectclass': 'top cosSuperDefinition cosIndirectDefinition ldapSubEntry'.split(), | 
|---|
 | 92 | +             'cosIndirectSpecifier': 'manager', | 
|---|
 | 93 | +             'cosAttribute': 'roomnumber' | 
|---|
 | 94 | +            }))) | 
|---|
 | 95 | +    except ldap.LDAPError, e: | 
|---|
 | 96 | +        log.fatal('Failed to add cos defintion, error: ' + e.message['desc']) | 
|---|
 | 97 | +        assert False | 
|---|
 | 98 | + | 
|---|
 | 99 | +    # Add manager entry | 
|---|
 | 100 | +    try: | 
|---|
 | 101 | +        topology.standalone.add_s(Entry((MANAGER_DN, | 
|---|
 | 102 | +            {'objectclass': 'top extensibleObject'.split(), | 
|---|
 | 103 | +             'uid': 'my manager', | 
|---|
 | 104 | +             'roomnumber': '1' | 
|---|
 | 105 | +            }))) | 
|---|
 | 106 | +    except ldap.LDAPError, e: | 
|---|
 | 107 | +        log.fatal('Failed to add manager entry, error: ' + e.message['desc']) | 
|---|
 | 108 | +        assert False | 
|---|
 | 109 | + | 
|---|
 | 110 | +    # Add user entry | 
|---|
 | 111 | +    try: | 
|---|
 | 112 | +        topology.standalone.add_s(Entry((USER_DN, | 
|---|
 | 113 | +            {'objectclass': 'top person organizationalPerson inetorgperson'.split(), | 
|---|
 | 114 | +             'sn': 'last', | 
|---|
 | 115 | +             'cn': 'full', | 
|---|
 | 116 | +             'givenname': 'mark', | 
|---|
 | 117 | +             'uid': 'user', | 
|---|
 | 118 | +             'manager': MANAGER_DN | 
|---|
 | 119 | +            }))) | 
|---|
 | 120 | +    except ldap.LDAPError, e: | 
|---|
 | 121 | +        log.fatal('Failed to add manager entry, error: ' + e.message['desc']) | 
|---|
 | 122 | +        assert False | 
|---|
 | 123 | + | 
|---|
 | 124 | +    # Test COS is working | 
|---|
 | 125 | +    try: | 
|---|
 | 126 | +        entry = topology.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, | 
|---|
 | 127 | +                                             "uid=user", | 
|---|
 | 128 | +                                             ['roomnumber']) | 
|---|
 | 129 | +        if entry: | 
|---|
 | 130 | +            if entry[0].getValue('roomnumber') != '1': | 
|---|
 | 131 | +                log.fatal('COS is not working.') | 
|---|
 | 132 | +                assert False | 
|---|
 | 133 | +        else: | 
|---|
 | 134 | +            log.fatal('Failed to find user entry') | 
|---|
 | 135 | +            assert False | 
|---|
 | 136 | +    except ldap.LDAPError, e: | 
|---|
 | 137 | +        log.error('Failed to search for user entry: ' + e.message['desc']) | 
|---|
 | 138 | +        assert False | 
|---|
 | 139 | + | 
|---|
 | 140 | +    # Modify manager entry | 
|---|
 | 141 | +    try: | 
|---|
 | 142 | +        topology.standalone.modify_s(MANAGER_DN, [(ldap.MOD_REPLACE, 'roomnumber', '2')]) | 
|---|
 | 143 | +    except ldap.LDAPError, e: | 
|---|
 | 144 | +        log.error('Failed to modify manager entry: ' + e.message['desc']) | 
|---|
 | 145 | +        assert False | 
|---|
 | 146 | + | 
|---|
 | 147 | +    # Confirm COS is returning the new value | 
|---|
 | 148 | +    try: | 
|---|
 | 149 | +        entry = topology.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, | 
|---|
 | 150 | +                                             "uid=user", | 
|---|
 | 151 | +                                             ['roomnumber']) | 
|---|
 | 152 | +        if entry: | 
|---|
 | 153 | +            if entry[0].getValue('roomnumber') != '2': | 
|---|
 | 154 | +                log.fatal('COS is not working after manager update.') | 
|---|
 | 155 | +                assert False | 
|---|
 | 156 | +        else: | 
|---|
 | 157 | +            log.fatal('Failed to find user entry') | 
|---|
 | 158 | +            assert False | 
|---|
 | 159 | +    except ldap.LDAPError, e: | 
|---|
 | 160 | +        log.error('Failed to search for user entry: ' + e.message['desc']) | 
|---|
 | 161 | +        assert False | 
|---|
 | 162 | + | 
|---|
 | 163 | +    log.info('Test complete') | 
|---|
 | 164 | + | 
|---|
 | 165 | + | 
|---|
 | 166 | +def test_ticket47921_final(topology): | 
|---|
 | 167 | +    topology.standalone.delete() | 
|---|
 | 168 | +    log.info('Testcase PASSED') | 
|---|
 | 169 | + | 
|---|
 | 170 | + | 
|---|
 | 171 | +def run_isolated(): | 
|---|
 | 172 | +    global installation1_prefix | 
|---|
 | 173 | +    installation1_prefix = None | 
|---|
 | 174 | + | 
|---|
 | 175 | +    topo = topology(True) | 
|---|
 | 176 | +    test_ticket47921(topo) | 
|---|
 | 177 | +    test_ticket47921_final(topo) | 
|---|
 | 178 | + | 
|---|
 | 179 | + | 
|---|
 | 180 | +if __name__ == '__main__': | 
|---|
 | 181 | +    run_isolated() | 
|---|
 | 182 | + | 
|---|
 | 183 | diff --git a/ldap/servers/plugins/cos/cos_cache.c b/ldap/servers/plugins/cos/cos_cache.c | 
|---|
 | 184 | index 7d8e877..fa2b6b5 100644 | 
|---|
 | 185 | --- a/ldap/servers/plugins/cos/cos_cache.c | 
|---|
 | 186 | +++ b/ldap/servers/plugins/cos/cos_cache.c | 
|---|
 | 187 | @@ -284,7 +284,7 @@ void cos_cache_backend_state_change(void *handle, char *be_name, | 
|---|
 | 188 |  static int cos_cache_vattr_get(vattr_sp_handle *handle, vattr_context *c, Slapi_Entry *e, char *type, Slapi_ValueSet** results,int *type_name_disposition, char** actual_type_name, int flags, int *free_flags, void *hint); | 
|---|
 | 189 |  static int cos_cache_vattr_compare(vattr_sp_handle *handle, vattr_context *c, Slapi_Entry *e, char *type, Slapi_Value *test_this, int* result, int flags, void *hint); | 
|---|
 | 190 |  static int cos_cache_vattr_types(vattr_sp_handle *handle,Slapi_Entry *e,vattr_type_list_context *type_context,int flags); | 
|---|
 | 191 | -static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, Slapi_Value *test_this, int *result, int *ops); | 
|---|
 | 192 | +static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, Slapi_Value *test_this, int *result, int *ops, int *indirect_cos); | 
|---|
 | 193 |   | 
|---|
 | 194 |  /*  | 
|---|
 | 195 |         compares s2 to s1 starting from end of string until the beginning of either | 
|---|
 | 196 | @@ -2096,8 +2096,9 @@ static int cos_cache_attrval_exists(cosAttrValue *pAttrs, const char *val) | 
|---|
 | 197 |   | 
|---|
 | 198 |  static int cos_cache_vattr_get(vattr_sp_handle *handle, vattr_context *c, Slapi_Entry *e, char *type, Slapi_ValueSet** results,int *type_name_disposition, char** actual_type_name, int flags, int *free_flags, void *hint) | 
|---|
 | 199 |  { | 
|---|
 | 200 | -       int ret = -1; | 
|---|
 | 201 |         cos_cache *pCache = 0; | 
|---|
 | 202 | +       int indirect_cos = 0; | 
|---|
 | 203 | +       int ret = -1; | 
|---|
 | 204 |   | 
|---|
 | 205 |         LDAPDebug( LDAP_DEBUG_TRACE, "--> cos_cache_vattr_get\n",0,0,0); | 
|---|
 | 206 |          | 
|---|
 | 207 | @@ -2108,10 +2109,15 @@ static int cos_cache_vattr_get(vattr_sp_handle *handle, vattr_context *c, Slapi_ | 
|---|
 | 208 |                 goto bail; | 
|---|
 | 209 |         } | 
|---|
 | 210 |   | 
|---|
 | 211 | -       ret = cos_cache_query_attr(pCache, c, e, type, results, NULL, NULL, NULL); | 
|---|
 | 212 | +       ret = cos_cache_query_attr(pCache, c, e, type, results, NULL, NULL, NULL, &indirect_cos); | 
|---|
 | 213 |         if(ret == 0) | 
|---|
 | 214 |         { | 
|---|
 | 215 | -        *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES | SLAPI_VIRTUALATTRS_VALUES_CACHEABLE; | 
|---|
 | 216 | +               if(indirect_cos){ | 
|---|
 | 217 | +                       /* we can't cache indirect cos */ | 
|---|
 | 218 | +                       *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES; | 
|---|
 | 219 | +               } else { | 
|---|
 | 220 | +                       *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES | SLAPI_VIRTUALATTRS_VALUES_CACHEABLE; | 
|---|
 | 221 | +               } | 
|---|
 | 222 |          *actual_type_name = slapi_ch_strdup(type); | 
|---|
 | 223 |                 *type_name_disposition = SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_EXACTLY_OR_ALIAS; | 
|---|
 | 224 |         } | 
|---|
 | 225 | @@ -2138,7 +2144,7 @@ static int cos_cache_vattr_compare(vattr_sp_handle *handle, vattr_context *c, Sl | 
|---|
 | 226 |                 goto bail; | 
|---|
 | 227 |         } | 
|---|
 | 228 |   | 
|---|
 | 229 | -       ret = cos_cache_query_attr(pCache, c, e, type, NULL, test_this, result, NULL); | 
|---|
 | 230 | +       ret = cos_cache_query_attr(pCache, c, e, type, NULL, test_this, result, NULL, NULL); | 
|---|
 | 231 |   | 
|---|
 | 232 |         cos_cache_release(pCache); | 
|---|
 | 233 |   | 
|---|
 | 234 | @@ -2179,7 +2185,7 @@ static int cos_cache_vattr_types(vattr_sp_handle *handle,Slapi_Entry *e, | 
|---|
 | 235 |                         lastattr = pCache->ppAttrIndex[index]->pAttrName; | 
|---|
 | 236 |   | 
|---|
 | 237 |                         if(1 == cos_cache_query_attr(pCache, NULL, e, lastattr, NULL, NULL, | 
|---|
 | 238 | -                                                                                        NULL, &props)) | 
|---|
 | 239 | +                                                                                        NULL, &props, NULL)) | 
|---|
 | 240 |                         { | 
|---|
 | 241 |                                 /* entry contains this attr */ | 
|---|
 | 242 |                                 vattr_type_thang thang = {0}; | 
|---|
 | 243 | @@ -2223,7 +2229,10 @@ bail: | 
|---|
 | 244 |         overriding and allow the DS logic to pick it up by denying knowledge | 
|---|
 | 245 |         of attribute | 
|---|
 | 246 |  */ | 
|---|
 | 247 | -static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, Slapi_Value *test_this, int *result, int *props) | 
|---|
 | 248 | +static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, | 
|---|
 | 249 | +                                Slapi_Entry *e, char *type, Slapi_ValueSet **out_attr, | 
|---|
 | 250 | +                                Slapi_Value *test_this, int *result, int *props, | 
|---|
 | 251 | +                                int *indirect_cos) | 
|---|
 | 252 |  { | 
|---|
 | 253 |         int ret = -1; | 
|---|
 | 254 |         cosCache *pCache = (cosCache*)ptheCache; | 
|---|
 | 255 | @@ -2420,6 +2429,9 @@ static int cos_cache_query_attr(cos_cache *ptheCache, vattr_context *context, Sl | 
|---|
 | 256 |                                                                 if (cos_cache_follow_pointer( context, (char*)slapi_value_get_string(indirectdn), | 
|---|
 | 257 |                                                                         type, &tmp_vals, test_this, result, pointer_flags) == 0) | 
|---|
 | 258 |                                                                 { | 
|---|
 | 259 | +                                                                       if(indirect_cos){ | 
|---|
 | 260 | +                                                                               *indirect_cos = 1; | 
|---|
 | 261 | +                                                                       } | 
|---|
 | 262 |                                                                         hit = 1; | 
|---|
 | 263 |                                                                         /* If the caller requested values, set them.  We need | 
|---|
 | 264 |                                                                          * to append values when we follow multiple pointers DNs. */ | 
|---|
 | 265 | --  | 
|---|
 | 266 | 1.9.3 | 
|---|
 | 267 |  | 
|---|