[Freeipa-devel] [PATCH] 275 Do not crash in Decimal parameter conversion

Martin Kosek mkosek at redhat.com
Fri Jun 15 10:54:02 UTC 2012 

On Thu, 2012-06-14 at 09:05 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Wed, 2012-06-13 at 18:05 -0400, Rob Crittenden wrote:
> >> Martin Kosek wrote:
> >>> On Thu, 2012-06-07 at 22:38 -0400, Rob Crittenden wrote:
> >>>> Martin Kosek wrote:
> >>>>> When invalid data is passed, an unhandled decimal exception could
> >>>>> be raised in Decimal number conversion. Handle the exception
> >>>>> more gracefully and report proper ipalib.errors.ConversionError.
> >>>>>
> >>>>> https://fedorahosted.org/freeipa/ticket/2705
> >>>>
> >>>> I'm being pedantic but I think the Decimal special values need to be
> >>>> handled better. Using Infinity returns a rather odd message:
> >>>>
> >>>> $ ipa dnsrecord-add example.com
> >>>> Record name: foo
> >>>> Please choose a type of DNS resource record to be added
> >>>> The most common types for this type of zone are: A, AAAA
> >>>>
> >>>> DNS resource record type: LOC
> >>>> LOC Degrees Latitude: 90
> >>>> [LOC Minutes Latitude]: 59
> >>>> [LOC Seconds Latitude]:
> >>>> 999999999999999999999999999999999999999999999999999999999999999999999
> >>>>    >>>   LOC Seconds Latitude: quantize result has too many digits for
> >>>> current context
> >>>> [LOC Seconds Latitude]: Infinity
> >>>>    >>>   LOC Seconds Latitude: quantize with one INF
> >>>>
> >>>> And using NaN raises an unhandled exception:
> >>>>
> >>>> [LOC Seconds Latitude]: NaN
> >>>> ipa: ERROR: InvalidOperation: comparison involving NaN
> >>>> Traceback (most recent call last):
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1263, in run
> >>>>        sys.exit(api.Backend.cli.run(argv))
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1049, in run
> >>>>        kw = self.argv_to_keyword_arguments(cmd, argv[1:])
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1035, in
> >>>> argv_to_keyword_arguments
> >>>>        self.prompt_interactively(cmd, kw)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1199, in
> >>>> prompt_interactively
> >>>>        callback(kw)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/plugins/dns.py", line 2147,
> >>>> in interactive_prompt_callback
> >>>>        user_options = param.prompt_parts(self.Backend)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/plugins/dns.py", line 768, in
> >>>> prompt_parts
> >>>>        self.__get_part_param(backend, part, user_options, default)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/plugins/dns.py", line 747, in
> >>>> __get_part_param
> >>>>        output_kw[name] = part(raw)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 556, in
> >>>> __call__
> >>>>        self.validate(value, supplied=self.name in kw)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 879, in
> >>>> validate
> >>>>        self._validate_scalar(value)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 893, in
> >>>> _validate_scalar
> >>>>        error = rule(ugettext, value)
> >>>>      File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 1244, in
> >>>> _rule_minvalue
> >>>>        if value<   self.minvalue:
> >>>>      File "/usr/lib64/python2.7/decimal.py", line 884, in __lt__
> >>>>        ans = self._compare_check_nans(other, context)
> >>>>      File "/usr/lib64/python2.7/decimal.py", line 786, in _compare_check_nans
> >>>>        self)
> >>>>      File "/usr/lib64/python2.7/decimal.py", line 3866, in _raise_error
> >>>>        raise error(explanation)
> >>>> InvalidOperation: comparison involving NaN
> >>>> ipa: ERROR: an internal error has occurred
> >>>>
> >>>> Otherwise it does what it should.
> >>>>
> >>>> rob
> >>>
> >>> Thanks for being pedantic, I found out that Decimal number validation
> >>> and normalization needs more care, dnsrecord-add would also fail with
> >>> values such as "1E4" or "-0".
> >>>
> >>> Attached patch improves Decimal number validation a lot and adds
> >>> optional exponent normalization. I also added missing tests for all
> >>> Decimal Parameter attributes.
> >>>
> >>> Martin
> >>
> >> Getting some lint errors. Ran out of time to investigate but its strange
> >> because AFAICT these are members.
> >>
> >> ipalib/parameters.py:1266: [E1101, Decimal._enforce_numberclass]
> >> Instance of 'Decimal' has no 'numberclass' member
> >> ipalib/parameters.py:1271: [E1101, Decimal._enforce_numberclass]
> >> Instance of 'Decimal' has no 'numberclass' member
> >> ipalib/parameters.py:1288: [E1101, Decimal._remove_exponent] Instance of
> >> 'Decimal' has no 'exponential' member
> >>
> >
> > I assume you run this lint:
> >
> > # ./make-lint ipalib/parameters.py
> >
> > This produces a lot of false positive lint errors... But if you run lint
> > for the entire project, my patches will pass:
> >
> > # git
> > apply /home/mkosek/freeipa-mkosek-275-2-decimal-parameter-conversion-and-normalization.patch
> > # ./make-lint
> > #
> >
> > That's why 275-2 includes a change for make-lint to not report these new
> > attributes:
> > diff --git a/make-lint b/make-lint
> > index 7ecd59d7e8c5a644f812d4b8987866e7d06236b5..30c5e00c1f0606c75ff1f7fec675ff673a6b87a0 100755
> > --- a/make-lint
> > +++ b/make-lint
> > @@ -61,7 +61,8 @@ class IPATypeChecker(TypeChecker):
> >               'csv', 'csv_separator', 'csv_skipspace'],
> >           'ipalib.parameters.Bool': ['truths', 'falsehoods'],
> >           'ipalib.parameters.Int': ['minvalue', 'maxvalue'],
> > -        'ipalib.parameters.Decimal': ['minvalue', 'maxvalue', 'precision'],
> > +        'ipalib.parameters.Decimal': ['minvalue', 'maxvalue', 'precision',
> > +                                      'numberclass', 'exponential'],
> >           'ipalib.parameters.Data': ['minlength', 'maxlength', 'length',
> >               'pattern', 'pattern_errmsg'],
> >           'ipalib.parameters.Enum': ['values'],
> >
> >
> > Bottom line - I do not think there is a lint problem with my patch :-)
> >
> > Martin
> >
> 
> This was output from './make-lint'. It blew up when I was trying to 
> build the rpms.
> 
> rob

Hm, I guess you have a different version of pylint, mine is error-less
(I use pylint-0.25.1-1.fc17.noarch).

Anyway, I have disabled the false positive pylint errors you reported,
it should not hurt and will keep your build clean. Updated patch is
attached.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-275-3-decimal-parameter-conversion-and-normalization.patch
Type: text/x-patch
Size: 9450 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120615/a3a46793/attachment.bin>

More information about the Freeipa-devel mailing list