[Freeipa-devel] [PATCH] 300-301 Fix DNS SOA serial parameters boundaries

[Martin Kosek]

On 09/05/2012 12:26 PM, Petr Viktorin wrote:
> On 09/05/2012 12:14 PM, Petr Viktorin wrote:
>> This works well, but please see some comments below.
>>
>> On 09/04/2012 04:22 PM, Martin Kosek wrote:
>>> To test, simply run the following command:
>>>
>>>   ipa dnszone-mod example.com --serial=4294967295
>>>
>>> This should work well on patched server&client. Web UI should work too
>>> as it
>>> reads the max limit dynamically.
>>
>> Please put this in the test suite.

Done.

>>
>>> ---
>>> [PATCH 2/2] Fix DNS SOA serial parameters boundaries:
>>>
>>> Set correct boundaries for DNS SOA serial parameters (see RFC 1035,
>>> 2181).
>>>
>>>
>>> [PATCH 1/2] Transfer long numbers over XMLRPC
>>>
>>> Numeric parameters in ipalib were limited by XMLRPC boundaries for
>>> integer (2^31-1) which is too low for some LDAP attributes like DNS
>>> SOA serial field.
>>>
>>> Transfer numbers which are not in XMLRPC boundary as a string and not
>>> as a number to workaround this limitation. Int parameter had to be
>>> updated to also accept Python's long type as valid int type.
>>>
>>>
>>> freeipa-mkosek-300-transfer-long-numbers-over-xmlrpc.patch
>>>
>>>
>>>> From 8782015a17b130c5ebae8b014a7241810b10dedd Mon Sep 17 00:00:00 2001
>>> From: Martin Kosek<mkosek at redhat.com>
>>> Date: Tue, 4 Sep 2012 15:49:26 +0200
>>> Subject: [PATCH 1/2] Transfer long numbers over XMLRPC
>>>
>>> Numeric parameters in ipalib were limited by XMLRPC boundaries for
>>> integer (2^31-1) which is too low for some LDAP attributes like DNS
>>> SOA serial field.
>>>
>>> Transfer numbers which are not in XMLRPC boundary as a string and not
>>> as a number to workaround this limitation. Int parameter had to be
>>> updated to also accept Python's long type as valid int type.
>>>
>>> https://fedorahosted.org/freeipa/ticket/2568
>>> ---
>>>   ipalib/parameters.py | 12 ++++++------
>>>   ipalib/rpc.py        |  5 ++++-
>>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/ipalib/parameters.py b/ipalib/parameters.py
>>> index
>>> de0d14faf08d1ab79c99e65dab9cc08f406e3a1d..21e30356b2a351bf7a3be7d47d7fabf0130cf6d4
>>>
>>> 100644
>>> --- a/ipalib/parameters.py
>>> +++ b/ipalib/parameters.py
>>> @@ -1077,7 +1077,7 @@ class Number(Param):
>>>           """
>>>           if type(value) is self.type:
>>>               return value
>>> -        if type(value) in (unicode, int, float):
>>> +        if type(value) in (unicode, int, long, float):
>>
>>
>> PEP8 says that "Object type comparisons should always use isinstance()
>> instead of comparing types directly".
>> It would be nice to change the old code whenever it is touched. It's
>> also in a few more places in the patch.
>>

I considered doing this when I was developing the patch, but I did not want to
mix type/isinstance in the same code. Now, when I experimented and tried to
replace it in a larger scope, there were unexpected issues. Like bool type
suddenly passing an isinstance(value, (int, long)) test and causing issues later.

So I skipped this part (as we discussed off-list).

>>> diff --git a/ipalib/rpc.py b/ipalib/rpc.py
>>> index
>>> d1764e3e30492d5855450398e86689bfcad7aa39..85239ac65903acf447a4d971cce70f819979ce8d
>>>
>>> 100644
>>> --- a/ipalib/rpc.py
>>> +++ b/ipalib/rpc.py
>>> @@ -94,6 +95,8 @@ def xml_wrap(value):
>>>       if type(value) is Decimal:
>>>           # transfer Decimal as a string
>>>           return unicode(value)
>>> +    if isinstance(value, (int, long)) and (value < MININT or value >
>>> MAXINT):
>>> +        return unicode(value)
>>>       if isinstance(value, DN):
>>>           return str(value)
>>>       assert type(value) in (unicode, int, float, bool, NoneType)
>>
>> `long` should also be included here.
>>
>>  >>> api.Command.user_find(uidnumber=1511000000)
>> {'count': 1, 'truncated': False, 'result': ({...},), 'summary': u'1 user
>> matched'}
>>  >>> api.Command.user_find(uidnumber=1511000000 + 0L)
>> Traceback (most recent call last):
>>    File "<console>", line 1, in <module>
>> ...
>>    File "/usr/lib/python2.7/site-packages/ipalib/rpc.py", line 102, in
>> xml_wrap
>>      assert type(value) in (unicode, int, float, bool, NoneType)
>> AssertionError
>>
>>
>>
>>
> 
> One more thing: please update the VERSION file.
> 

I did not want to do this because I just messed with validation, but yeah, I
can do that.

Fixed patches attached.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-300-2-transfer-long-numbers-over-xmlrpc.patch
Type: text/x-patch
Size: 3688 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120905/feb374e4/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-301-2-fix-dns-soa-serial-parameters-boundaries.patch
Type: text/x-patch
Size: 12761 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120905/feb374e4/attachment-0001.bin>