[PATCH][PYTHON] Fix replPropertyMetaData sort order for schema extension attributes

Stefan Metzmacher metze at samba.org
Wed Aug 5 07:23:27 UTC 2015


Hi Andrew,

>>>> - Do we have a test that checks the objectClass sorting against the 
>>>> LDAP
>>>> server
>>>>   (and passed against windows)?
>>>
>>> No, not currently.  We should have it check that the correct
>>> attributeID is added, that it is sorted, and that when entries are
>>> modified, replaced or deleted, that the version is incremented. 
>>
>> That an answer to the wrong question...
> 
> Ah. :-)
> 
>> My question was about the sorting of objectClass values:
>> https://git.samba.org/?p=abartlet/samba.git/.git;a=commitdiff;h=c74751d5e89e9b36a0987589f4dd0e481137561d
> 
> I have no intention of matching Windows behaviour there.  The docs (and
> I should reference those) say the order is undefined for AUX classes.

I'm sorry but we really need to match Windows, everything else will
cause problems
with poorly written admin scripts.

And we need tests to prove it our behavior matches a Windows server.

> What windows actually does is store them in the inverse of the order you
> set them in :-)

We need tests to prove that, it's also possible that they are sorted by
governsID.

> (See the code in our DRS to LDB handler for objectClass that inverts the
> order). 
> 
> What this code does is make the input order deterministic, so that the
> rest of the calculation is deterministic. 

I think Windows is also deterministic, given specific input values.

>> Can you initialize alpha_sorted to NULL in the declaration we had a
>> number of security
>> bugs related to unitialized pointers already...
>> The code would also look much clearer if we would have:
>>
>> struct class_list *unsorted = NULL;
>> struct class_list *sorted = NULL;
>> ...
>> struct class_list *alpha_sorted = NULL;
>>
>> Instead of the spaghetti:
>>
>> struct class_list *unsorted = NULL, *sorted = NULL, *current = NULL,
>>         *poss_parent = NULL, *new_parent = NULL,
>>         *current_lowest = NULL, *current_lowest_struct = NULL,
>>         *alpha_sorted;
> 
> Certainly!
> 
>>>> - Please add BUG: https:... tags to the commit messages
>>>>   We want to backport this at least to v4-3-test and v4-2-test, but
>>>> maybe also to v4-1-test...
>>>
>>> The key is to backport 61b978872fe86906611f64430b2608f5e7ea7ad8, then
>>> these to fix up broken databases. 
>>>
>>> The big remaining issue is that our msDS-IntID handling is totally
>>> broken.  We use that only when pushing the values over the network with
>>> GetNCChanges, but not during storage in replPropertyMetaData.  This
>>> means for the an object with the same attribute modified on two servers
>>> we can end up with metadata for an entry twice, once for each style of
>>> attributeID.
>>
>> Ok, do we have bug reports for all this problems?
> 
> Not yet.

Please add them and cc me.

Thanks!

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150805/5a807b9b/signature.sig>


More information about the samba-technical mailing list