Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Petr Spacek]

On 1.8.2016 18:31, Martin Basti wrote:
> 
> 
> On 28.07.2016 18:29, Ariel Barria wrote:
>> 2016-07-28 7:10 GMT-05:00 Petr Spacek :
>>> On 27.7.2016 18:26, Ariel Barria wrote:
 2016-07-26 9:39 GMT-05:00 Petr Spacek :
> On 26.7.2016 16:28, Jan Cholasta wrote:
>> Hi,
>>
>> On 26.7.2016 16:09, Martin Basti wrote:
>>>
>>> On 22.07.2016 00:14, Ariel Barria wrote:
 Hello everyone.

 I send patch for review.
>> NACK, six.u() is supposed to be used on string literals *only* [1].
>>
>> A proper fix would be something like:
>>
>>  value = self.to_text()
>>  if not isinstance(value, unicode):
>>  value = value.decode('ascii')
>>  return value
 thanks for the guidance and comments

> Most importantly, we should fix/provide this method in python-dns and
> inherit
> this method from there.
 Well, I made a pr, but apparently travis ci test failed with other
 versions of python, so it is possible that they do not approve, I will
 be performing other tests to see what the downside.

 https://github.com/rthalley/dnspython/pull/195
>>> Looking at the PR, there are functions
>>> dns.name.to_text()
>>> dns.name.to_unicode()
>>>
>>> What is missing in them?
>>>
>>> Petr^2 Spacek
>>>
>>>
>>> I will look on this, for some reason we received your e-mail just today
>>> (2016-07-26)
 welcome.
 it was my mistake, I sent the patch to the list before being signed to the
 list

>> Honza
>>
>> [1] 
>>> -- 
>>> Petr^2 Spacek
>> Hi.
>> I send the requested changes
>> thanks for review.
>>
>>
> 
> According the 
> https://github.com/rthalley/dnspython/blob/master/dns/name.py#L375
> 
> New dnspython always return binary strings, so we should always decode it,
> because IPA internals need punycoded domain in unicode format
> 
> IMO this is broken in current dnspython released in Fedora. There are plenty
> of py3 bugs, we have to provide newer dnspython package to fedora, so this
> should not be fixed on IPA side.

I'm fine with rebasing python-dns in Fedora when we find a version which works
better. Just tell me what version it is :-)

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Martin Basti]

On 28.07.2016 18:29, Ariel Barria wrote:

2016-07-28 7:10 GMT-05:00 Petr Spacek :

On 27.7.2016 18:26, Ariel Barria wrote:

2016-07-26 9:39 GMT-05:00 Petr Spacek :

On 26.7.2016 16:28, Jan Cholasta wrote:

Hi,

On 26.7.2016 16:09, Martin Basti wrote:


On 22.07.2016 00:14, Ariel Barria wrote:

Hello everyone.

I send patch for review.

NACK, six.u() is supposed to be used on string literals *only* [1].

A proper fix would be something like:

 value = self.to_text()
 if not isinstance(value, unicode):
 value = value.decode('ascii')
 return value

thanks for the guidance and comments


Most importantly, we should fix/provide this method in python-dns and inherit
this method from there.

Well, I made a pr, but apparently travis ci test failed with other
versions of python, so it is possible that they do not approve, I will
be performing other tests to see what the downside.

https://github.com/rthalley/dnspython/pull/195

Looking at the PR, there are functions
dns.name.to_text()
dns.name.to_unicode()

What is missing in them?

Petr^2 Spacek



I will look on this, for some reason we received your e-mail just today
(2016-07-26)

welcome.
it was my mistake, I sent the patch to the list before being signed to the list


Honza

[1] 

--
Petr^2 Spacek

Hi.
I send the requested changes
thanks for review.




According the 
https://github.com/rthalley/dnspython/blob/master/dns/name.py#L375


New dnspython always return binary strings, so we should always decode 
it, because IPA internals need punycoded domain in unicode format


IMO this is broken in current dnspython released in Fedora. There are 
plenty of py3 bugs, we have to provide newer dnspython package to 
fedora, so this should not be fixed on IPA side.


regards
Martin
-- 
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

Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Ariel Barria]

2016-07-28 7:10 GMT-05:00 Petr Spacek :
> On 27.7.2016 18:26, Ariel Barria wrote:
>> 2016-07-26 9:39 GMT-05:00 Petr Spacek :
>>> On 26.7.2016 16:28, Jan Cholasta wrote:
 Hi,

 On 26.7.2016 16:09, Martin Basti wrote:
>
>
> On 22.07.2016 00:14, Ariel Barria wrote:
>> Hello everyone.
>>
>> I send patch for review.

 NACK, six.u() is supposed to be used on string literals *only* [1].

 A proper fix would be something like:

 value = self.to_text()
 if not isinstance(value, unicode):
 value = value.decode('ascii')
 return value
>>
>> thanks for the guidance and comments
>>
>>>
>>> Most importantly, we should fix/provide this method in python-dns and 
>>> inherit
>>> this method from there.
>>
>> Well, I made a pr, but apparently travis ci test failed with other
>> versions of python, so it is possible that they do not approve, I will
>> be performing other tests to see what the downside.
>>
>> https://github.com/rthalley/dnspython/pull/195
>
> Looking at the PR, there are functions
> dns.name.to_text()
> dns.name.to_unicode()
>
> What is missing in them?
>
> Petr^2 Spacek
>
>
> I will look on this, for some reason we received your e-mail just today
> (2016-07-26)
>>
>> welcome.
>> it was my mistake, I sent the patch to the list before being signed to the 
>> list
>>

 Honza

 [1] 
> --
> Petr^2 Spacek

Hi.
I send the requested changes
thanks for review.
From 4da94aee34d56b7baf0888ca5ef83aaecbc9544b Mon Sep 17 00:00:00 2001
From: "Ariel O. Barria" 
Date: Thu, 21 Jul 2016 17:06:05 -0500
Subject: [PATCH] freeipa arielb 0001-0002 six.u function instead of the decode
 function

to avoid DNSName.ToASCII broken with python3
replace the decode () function to use the inherited
functions python-dns:
 - from_unicode()
 - to_unicode()
 - to_text()

https://fedorahosted.org/freeipa/ticket/5935
---
 ipapython/dnsutil.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py
index 16549c8f6a4a0782191526856e918d7ac6b94dcc..effdc0a8d198c6a30766fddb7a1865658913b176 100644
--- a/ipapython/dnsutil.py
+++ b/ipapython/dnsutil.py
@@ -71,7 +71,11 @@ class DNSName(dns.name.Name):
 
 def ToASCII(self):
 #method named by RFC 3490 and python standard library
-return self.to_text().decode('ascii')  # must be unicode string
+value = self.to_text()
+if not isinstance(value, unicode):
+n = dns.name.from_unicode(self.to_unicode())
+value = n.to_text(True)
+return value  # must be unicode string
 
 def canonicalize(self):
 return DNSName(super(DNSName, self).canonicalize())
-- 
2.7.4

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

Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Petr Spacek]

On 27.7.2016 18:26, Ariel Barria wrote:
> 2016-07-26 9:39 GMT-05:00 Petr Spacek :
>> On 26.7.2016 16:28, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 26.7.2016 16:09, Martin Basti wrote:


 On 22.07.2016 00:14, Ariel Barria wrote:
> Hello everyone.
>
> I send patch for review.
>>>
>>> NACK, six.u() is supposed to be used on string literals *only* [1].
>>>
>>> A proper fix would be something like:
>>>
>>> value = self.to_text()
>>> if not isinstance(value, unicode):
>>> value = value.decode('ascii')
>>> return value
> 
> thanks for the guidance and comments
> 
>>
>> Most importantly, we should fix/provide this method in python-dns and inherit
>> this method from there.
> 
> Well, I made a pr, but apparently travis ci test failed with other
> versions of python, so it is possible that they do not approve, I will
> be performing other tests to see what the downside.
> 
> https://github.com/rthalley/dnspython/pull/195

Looking at the PR, there are functions
dns.name.to_text()
dns.name.to_unicode()

What is missing in them?

Petr^2 Spacek


 I will look on this, for some reason we received your e-mail just today
 (2016-07-26)
> 
> welcome.
> it was my mistake, I sent the patch to the list before being signed to the 
> list
> 
>>>
>>> Honza
>>>
>>> [1] 
-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Petr Spacek]

On 26.7.2016 16:28, Jan Cholasta wrote:
> Hi,
> 
> On 26.7.2016 16:09, Martin Basti wrote:
>>
>>
>> On 22.07.2016 00:14, Ariel Barria wrote:
>>> Hello everyone.
>>>
>>> I send patch for review.
> 
> NACK, six.u() is supposed to be used on string literals *only* [1].
> 
> A proper fix would be something like:
> 
> value = self.to_text()
> if not isinstance(value, unicode):
> value = value.decode('ascii')
> return value

Most importantly, we should fix/provide this method in python-dns and inherit
this method from there.

Petr^2 Spacek

> 
>>>
>>> Regards,
>>>
>>>
>>
>> Thank you,
>>
>> I will look on this, for some reason we received your e-mail just today
>> (2016-07-26)
> 
> Honza
> 
> [1] 

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

Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Jan Cholasta]

Hi,

On 26.7.2016 16:09, Martin Basti wrote:



On 22.07.2016 00:14, Ariel Barria wrote:

Hello everyone.

I send patch for review.


NACK, six.u() is supposed to be used on string literals *only* [1].

A proper fix would be something like:

value = self.to_text()
if not isinstance(value, unicode):
value = value.decode('ascii')
return value



Regards,




Thank you,

I will look on this, for some reason we received your e-mail just today
(2016-07-26)


Honza

[1] 

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

Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

[Martin Basti]

On 22.07.2016 00:14, Ariel Barria wrote:

Hello everyone.

I send patch for review.

Regards,




Thank you,

I will look on this, for some reason we received your e-mail just today 
(2016-07-26)


Martin
-- 
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