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

[Martin Kosek]

On 09/06/2012 02:51 PM, Petr Viktorin wrote:
> On 09/05/2012 01:31 PM, Martin Kosek wrote:
>> 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.
>>
> 
> Both are good, ACK.
> 

Pushed to master, ipa-3-0.

Martin