PR - Issue 50511 - lib389 PosixGroups type can not handle rdn properly

[389-ds-bot]

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50512

• Created at 2019-07-23 12:39:52 by aborah (@aborah-sudo)
• Merged at 2019-07-23 21:08:07

---

Description: lib389 PosixGroups type can not handle rdn properly

Fixes: Resolves: #3567

Author: aborah-sudo

Reviewed by: ???

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-07-23 13:17:05

It is better to compare it with None. More safe

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-07-23 14:15:12

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.

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 15:00:18

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

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 17:29:44

rebased onto 2c0ab1e780df9c43283c4923785475b894b994e1

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 17:31:17

@droideck changes are done , as per pour suggestion .

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-07-23 18:44:33

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

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 18:49:54

rebased onto 0b0d360adc8db5507024d45de9492f26640dd5c7

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 18:50:14

@droideck changes are done

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-07-23 18:52:33

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.

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 18:59:40

rebased onto c64030836b87014146a4617ed0e97dd2bc397abb

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 19:00:38

@droideck changes are done

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-07-23 19:23:32

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

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 19:26:32

rebased onto 9ea5b9b

[389-ds-bot]

Comment from aborah (@aborah-sudo) at 2019-07-23 19:28:08

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 .

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-07-23 19:38:26

Ack

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-07-23 21:08:07

Pull-Request has been merged by droideck

[389-ds-bot]

Patch
50512.patch