On 27/07/2010 01:08, [email protected] wrote: >> Full_Name: Jonathan CLARKE >> Version: RE24 >> OS: >> URL: ftp://ftp.openldap.org/incoming/jonathan-clarke-pcache-100723.patch >> Submission from: (NULL) (80.13.86.63) >> >> >> While checking it's configuration, the pcache overlay verifies each >> configured >> attribute set (pcacheAttrset), to ensure that all attributes in the set >> are >> defined, via slap_str2ad. Given an attribute set with a non-existant >> attribute, >> an error is logged and slapd refuses to start (as expected): >> >> line 117 (pcacheAttrset 0 nonexistantAttr) >> /etc/ldap/slapd.conf: line 117: attribute type undefined. >> >> However, when a search request comes in, the requested attributes list is >> not >> checked by the pcache overlay to ensure that attributes are properly >> defined. In >> effect, slapd just ignores the non-existant attributes, and returns other >> attributes (or behaves as if 1.1 was requested if all requested attributes >> are >> invalid). >> >> This causes pcache's attribute set matching to fail for some requests, >> since it >> counts invalid attributes. If it were to ignore them, configured attribute >> sets >> might match, and successfully cache the search. The patch above implements >> this >> behaviour. Would you consider it for inclusion in OpenLDAP? >> >> I realize this may be considered "repairing bad requests", but sometimes >> one >> can't (easily) control what clients are requesting. Furthermore, it seems >> to >> make sense to have matching behavior all over (since slapd ignores invalid >> requested attributes, pcache should too, IMHO). > > Yours looks like a good catch; however, your patch looks a bit like an > overkill, since the an_desc field of the attribute list that is passed to > get_attr_set() should already be set if the attribute was recognized by > slapd, so you don't need to go through slap_bv2ad() once more.
Thanks for your feedback. Indeed, looking at it with your comments, definitely overkill. I meant to ask about this approach in the bug description but forgot, sorry. Here is a simpler patch which works the same for my tests: ftp://ftp.openldap.org/incoming/jonathan-clarke-pcache-100727.patch Jonathan -- -------------------------------------------------------------- Jonathan Clarke - [email protected] -------------------------------------------------------------- Ldap Synchronization Connector (LSC) - http://lsc-project.org --------------------------------------------------------------
