PR#50512: Issue 50511 - lib389 PosixGroups type can not handle rdn properly - 389-ds-base

389-ds-base

#50512 Issue 50511 - lib389 PosixGroups type can not handle rdn properly

Closed 14 days ago by spichugi. Opened a year ago by aborah.

aborah/389-ds-base possix into master

Issue 50511 - lib389 PosixGroups type can not handle rdn properly

Anuj Borah • a year ago

9ea5b9b

src/lib389/lib389/idm/group.py

file modified

+7 -6

		`@@ -95,10 +95,10 @@`
		`]`
		`self._filterattrs = [RDN]`
		`self._childobject = Group`
		`- if rdn:`
		`- self._basedn = '{},{}'.format(ensure_str(rdn), ensure_str(basedn))`
		`- else:`
		`+ if rdn is None:`
		`self._basedn = ensure_str(basedn)`
		`+ else:`
		`+ self._basedn = '{},{}'.format(ensure_str(rdn), ensure_str(basedn))`


		`class UniqueGroup(DSLdapObject):`
		`@@ -137,10 +137,11 @@`
		`]`
		`self._filterattrs = [RDN]`
		`self._childobject = UniqueGroup`
		`- if rdn:`
		`- self._basedn = '{},{}'.format(ensure_str(rdn), ensure_str(basedn))`
		`- else:`
		`+ if rdn is None:`
		`self._basedn = ensure_str(basedn)`
		`+ else:`
		`+ self._basedn = '{},{}'.format(ensure_str(rdn), ensure_str(basedn))`
		`+`

src/lib389/lib389/idm/posixgroup.py

file modified

+5 -4

		`@@ -7,7 +7,7 @@`
		`# --- END COPYRIGHT BLOCK ---`

		`from lib389._mapped_object import DSLdapObject, DSLdapObjects`
		`- from lib389.utils import ds_is_older`
		`+ from lib389.utils import ds_is_older, ensure_str`

		`MUST_ATTRIBUTES = [`
		`'cn',`
		`@@ -76,6 +76,7 @@`
		`]`
		`self._filterattrs = [RDN]`
		`self._childobject = PosixGroup`
		`- self._basedn = '{},{}'.format(rdn, basedn)`
		`-`
		`-`
		`+ if rdn is None:`
		`+ self._basedn = ensure_str(basedn)`
		`+ else:`
		`+ self._basedn = '{},{}'.format(ensure_str(rdn), ensure_str(basedn))`

aborah commented a year ago

Description: lib389 PosixGroups type can not handle rdn properly

Fixes: https://pagure.io/389-ds-base/issue/50511

Author: aborah

Reviewed by: ???

spichugi commented on line 8 of src/lib389/lib389/idm/posixgroup.py a year ago

It is better to compare it with None. More safe

mhonek commented a year ago

It is better to compare it with None. More safe

The original Issue considers e.g. an empty string to be the same case as None. Therefore, the suggestion would be invalid. However, a question would be if the original request is itself valid; IDK.

aborah commented a year ago

@spichugi if we compare with None . it will make it

    if rdn is None:
        self._basedn = basedn
    else:
        self._basedn = '{},{}'.format(rdn, basedn)

(Pdb) PosixGroups(topo_m4.all_insts.get('master1'), SUBSUFFIX, rdn=None)._basedn
'dc=SubSuffix,dc=autoMembers,dc=com' ---- right
(Pdb) PosixGroups(topo_m4.all_insts.get('master1'), SUBSUFFIX, rdn='')._basedn
',dc=SubSuffix,dc=autoMembers,dc=com' ---- wrong

with the current version:

    if rdn:
        self._basedn = '{},{}'.format(rdn, basedn)
    else:
        self._basedn = basedn

so we have to go with current version

rebased onto 2c0ab1e780df9c43283c4923785475b894b994e1

a year ago

aborah commented a year ago

@spichugi changes are done , as per pour suggestion .

spichugi commented a year ago

Please, fix other groups.py functionality too as stated in the issue.

rebased onto 0b0d360adc8db5507024d45de9492f26640dd5c7

a year ago

aborah commented a year ago

@spichugi changes are done

spichugi commented a year ago

I think it is better to preserve ensure_str(rdn), ensure_str(basedn) part because it is safer.
Sorry if I created confusion by me first comment here.

rebased onto c64030836b87014146a4617ed0e97dd2bc397abb

a year ago

aborah commented a year ago

@spichugi changes are done

spichugi commented a year ago

You haven't applied the same logic for PosixGroups... Was it intentional?

rebased onto 9ea5b9b

a year ago

aborah commented a year ago