#50512 Issue 50511 - lib389 PosixGroups type can not handle rdn properly
Merged 6 months ago by spichugi. Opened 6 months ago by aborah.
aborah/389-ds-base possix  into  master

@@ -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))

+ 

  

  

  

@@ -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))

Description: lib389 PosixGroups type can not handle rdn properly

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

Author: aborah

Reviewed by: ???

It is better to compare it with None. More safe

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.

@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

(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' ---- right

so we have to go with current version

rebased onto 2c0ab1e780df9c43283c4923785475b894b994e1

6 months ago

@spichugi changes are done , as per pour suggestion .

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

rebased onto 0b0d360adc8db5507024d45de9492f26640dd5c7

6 months 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

6 months ago

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

rebased onto 9ea5b9b

6 months ago

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

ensure_str was not imported there in PosixGroups before , now i have imported and applied same logic there also .

Pull-Request has been merged by spichugi

6 months ago