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