[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#6504) Corrupted control value when using pagedresults with subordinate *ldap* database



Le 02/04/2010 06:44, masarati@aero.polimi.it a écrit :
>> masarati@aero.polimi.it wrote:
>>>> masarati@aero.polimi.it wrote:
>>>>>> Also, I note that the glued paged response seems to work incorrectly.
>>>>>> I
>>>>>> made a simple test system, where the root database contains exactly
>>>>>> one
>>>>>> entry (the suffix) and a back-ldap is glued on top.  If I request
>>>>>> entries
>>>>>> with a page size of 2, searching the suffix return 3 entries;
>>>>>> subsequent
>>>>>> searches return 2 entries from the proxy.  I haven't figured out yet
>>>>>> where
>>>>>> the issue is.
>>>>>
>>>>> I figured out how to fix the issue you reported.  It is related to the
>>>>> fact that slapd internally assumes that ldctl_oid values are constant
>>>>> strings, so it doesn't duplicate nor free them.  However, control OIDs
>>>>> returned by back-ldap (and friends) have been decoded from the wire by
>>>>> the
>>>>> client library, so they are actually malloc'ed.  That's why backglue
>>>>> ends
>>>>> up with a dangling pointer.
>>>>>
>>>>> I'm fixing it by having backglue duplicate (and free) control OIDs
>>>>> consistently.  Another option would be to replace those values with
>>>>> constant strings for known control values.  This would prevent gluing
>>>>> for
>>>>> unknown proxied controls.
>>>>
>>>> Or just tmpalloc them and don't bother to clean them up.
>>>
>>> Yes, I've reworked all the temporary controls allocation using tmpalloc.
>>> Too bad OIDs are char* and not bervals.
>>>
>>> With respect to the other issue, to honor the requested pagesize we'd
>>> need
>>> to intercept pagedresult requests, modify the page size in the request
>>> when a page crosses two databases, requesting the original page size
>>> minus
>>> entries already returned.  I wonder whether it's worth the effort,
>>> though.
>>
>> IMO pagedresults should be a (global) overlay, and backends shouldn't do
>> anything about it. Note that the sssvlv overlay already intercepts
>> pagedresults, it's impossible to handle things correctly otherwise.
>
> Well, I've just fixed it.  When changing database, I modified the paged
> results control value by setting the page size to pagesize -
> rs->sr_nentries, with an empty cookie.  I'll commit tomorrow, as now I
> only have http access.

Speedy patch - thank you! Just tried it out, and it works great for the 
issue I reported.

Testing further shows a limitation in crossing databases though. A 
search that spans over two glued ldap databases, returns n < pagesize 
results from the first, and continues over to the second, returning 
pagesize-n results from it, making pagesize results all together. So 
far, so good.

However, the subsequent search with the returned pagedresults cookie 
just starts a new search over, returning the same set of results. So it 
works fine for one page max.

But it doesn't look simple to solve this issue, unless backglue starts 
recording pagedresults cookies, etc...

Anyway, great improvement on not sending back free'd cookie :-)
-- 
--------------------------------------------------------------
Jonathan Clarke - jonathan@phillipoux.net
--------------------------------------------------------------
Ldap Synchronization Connector (LSC) - http://lsc-project.org
--------------------------------------------------------------