On 19.7.2016 11:32, Alexander Bokovoy wrote:
On Tue, 19 Jul 2016, Jan Cholasta wrote:
On 18.7.2016 12:06, Martin Babinsky wrote:
On 07/16/2016 12:50 PM, Alexander Bokovoy wrote:
Hi,


I had some time and was blocked by these bugs to do my tickets so I
actually fixed these three problems that are assigned to Martin
Babinsky. Hopefully, Martin wouldn't be offended by that. :)

Note that this fix (patch 0211) has potential for a break but also
introduces a correct behavior in my view as we should not really have
non-lower cased keys in LDAP dictionaries in entry_to_dict() for both
normal and --raw modes.

----- 0211

Since commit a8dd7aa337f25abd938a582d0fcba51d3b356410 if IPA command
is called with --raw option, a retrieved LDAP entry's attribute
names aren't normalized to lower case when converting the entry
to a dictionary. This breaks overall assumption that dictionary
keys are in lower case.

Partially fixes 'ipa trust-add --raw' issues.

https://fedorahosted.org/freeipa/ticket/6059

----- 0212

Make sure we display raw values for 'trust-add --raw' case.

https://fedorahosted.org/freeipa/ticket/6059





Hi Alexander,

I am CC'ing Jan since I hope he knows why
a8dd7aa337f25abd938a582d0fcba51d3b356410 was implemented in this way. I
think there was a reason behind this decision and we should not revert
it without further discussion.

I think it was just because with --raw, the raw LDAP entry should be
returned, letter case and all. I don't really care if the names are
lowercased or not, but you should never assume anything about raw
results in your code anyway, so I think the revert would be pointless.
There were a few bugs with --raw unrelated to letter case in the past
and most of the offending code was fixed, the same should be done here
IMO.
This is not about LDAP entry, this is about Python dictionary from
entry_to_dict(). LDAP attribute name is case-insensitive.

We call entry_to_dict() in multiple places and we expect that
dictionary keys are lowcased in Python code. I don't think it is fair to
have this assumption broken when --raw is used -- after all, we are
dealing with Python dictionary and LDAP attributes are case insensitive.

We don't expect anything about raw results anywhere in our code base, except for buggy code, which needs be fixed anyway.

Martin's patch fixes the issue without any intrusive changes, so ACK. We can talk bigger changes later for 4.5.

Pushed to master: 2234a774416309a44aecb84f27e6cf4c6a1a990f

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to