On 06/27/2014 12:10 PM, Alexander Bokovoy wrote: > On Fri, 27 Jun 2014, Petr Spacek wrote: >> On 27.6.2014 11:21, Jan Cholasta wrote: >>> On 27.6.2014 10:58, Alexander Bokovoy wrote: >>>> On Fri, 27 Jun 2014, Jan Cholasta wrote: >>>>> On 27.6.2014 10:29, Alexander Bokovoy wrote: >>>>>> On Fri, 27 Jun 2014, Jan Cholasta wrote: >>>>>>> On 27.6.2014 10:15, Alexander Bokovoy wrote: >>>>>>>> On Fri, 20 Jun 2014, Martin Basti wrote: >>>>>>>>> On Fri, 2014-06-20 at 10:32 +0200, Jan Cholasta wrote: >>>>>>>>>> On 18.6.2014 16:49, Martin Basti wrote: >>>>>>>>>>> Due to compability with older versions, only IDNA domains should be >>>>>>>>>>> checked >>>>>>>>>>> Patch attached. >>>>>>>>>> >>>>>>>>>> I'm not particularly happy about the u'\xdf' special case. Isn't >>>>>>>>>> there a >>>>>>>>>> better way to do this check? >>>>>>>>> I cant find better way. u'\xdf' is mapped to ss, and ss is not IDN >>>>>>>>> string. >>>>>>>>> >>>>>>>>> Or just remove this validation. >>>>>>>>> >>>>>>>>>> (BTW I really think this should be a warning, not an error, but that >>>>>>>>>> would require larger amount of work, so I guess it's OK for now.) >>>>>>>>> (More pain than gain) >>>>>>>> Main thing in this patch is that the check should not be done against >>>>>>>> non-IDN strings. I want this version of the patch to go in for that >>>>>>>> reason as currently you cannot even complete ipa-adtrust-install >>>>>>>> run due >>>>>>>> to IDN normalisation check being applied to non-IDN domains. >>>>>>> >>>>>>> On non-IDN domains, the only effect of IDN normalization is that it >>>>>>> lower-cases the names (right?), so the check should compare >>>>>>> lower-cased original name with the normalized name, instead of >>>>>>> special-casing certain characters etc. >>>>>> .. what's the reason to do such comparison then? lower-cased non-IDN >>>>>> name will be equal to lower-cased normalized non-IDN name by definition, >>>>>> so the check is not needed in this case, at all. >>>>> >>>>> The point is that it works for both IDN and non-IDN, without >>>>> u'\xdf'-style hacks. >>>> No, your proposal of comparing low-cased value and normalized value is >>>> not going to work because low-cased value is in general not equal to >>>> normalized value for IDN names, only for non-IDN ones, due to the fact >>>> that lower case for non-ASCII Unicode character may map to a completely >>>> different character than in normalization situation. Take, for example, >>>> Turkish alphabet where there are six letters with different case rules >>>> (uppercase dotted i, dottless lowercase i, upper- and lowercase G with >>>> breve accent, and upper- and lowercase S with cedilla), which will break >>>> your generalized check. >>>> So you'll anyway will need to split these cases. >>>> >>> >>> I see. >>> >>> I'm still not comfortable with carrying the bit of knowledge about u'\xdf' >>> in >>> this particular spot. Can we check that a name is IDN some other way than >>> "domain_name.is_idn() or u'\xdf' in value"? >> >> Why can't we simply fix string constants in ipa-adtrust-install and avoid >> adding hacks for it? > Because they are correct, in the sense that they follow what is defined > for Active Directory. Yes, AD puts them in that case into DNS. There is > simply no reason to force lower case for non-IDN names. > > That said, a newer fix is attached, where error message is formatted > properly.
I would personally be OK with the change if the is_* are fixed as Honza proposed, current way is not so Python-ic/readable. I.e.: Instead of + is_idna = True in [encodings.idna.ToASCII(x) != x for x in labels] Use + is_idna = any(encodings.idna.ToASCII(x) != x for x in labels) Instead of + is_nonnorm = True in [encodings.idna.nameprep(x) != x for x in labels] use + is_nonnorm = any(encodings.idna.nameprep(x) != x for x in labels) However, we can wait till Monday for Martin2's feedback. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel