Opened 3 weeks ago

Closed 2 weeks ago

#28758 closed Bug (fixed)

Fix range min/max validators to prevent errors for infinite ranges

Reported by: myii Owned by: Tim Graham <timograham@…>
Component: contrib.postgres Version: master
Severity: Normal Keywords:
Cc: Shai Berger, myii Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

References

  1. https://www.postgresql.org/docs/current/static/rangetypes.html#rangetypes-infinite

8.17.4. Infinite (Unbounded) Ranges

The lower bound of a range can be omitted, meaning that all points less than the upper bound are included in the range. Likewise, if the upper bound of the range is omitted, then all points greater than the lower bound are included in the range.

  1. https://docs.djangoproject.com/en/dev/ref/contrib/postgres/fields/#range-fields

All of the range fields translate to psycopg2 Range objects in python, ...

  1. http://initd.org/psycopg/docs/extras.html#adapt-range

Parameters:

  • lower – lower bound for the range. None means unbound
  • upper – upper bound for the range. None means unbound
  1. https://docs.djangoproject.com/en/dev/ref/contrib/postgres/validators/#range-validators

Range validators

RangeMaxValueValidator

...

Validates that the upper bound of the range is not greater than limit_value.

RangeMinValueValidator

...

Validates that the lower bound of the range is not less than the limit_value.

  1. https://github.com/django/django/blob/617686e226231fe8ad3f2e49d3efabf6f5f434d3/tests/postgres_tests/test_ranges.py#L406,L422
class TestValidators(PostgreSQLTestCase):

    def test_max(self):
        validator = RangeMaxValueValidator(5)
        validator(NumericRange(0, 5))
        with self.assertRaises(exceptions.ValidationError) as cm:
            validator(NumericRange(0, 10))
        self.assertEqual(cm.exception.messages[0], 'Ensure that this range is completely less than or equal to 5.')
        self.assertEqual(cm.exception.code, 'max_value')

    def test_min(self):
        validator = RangeMinValueValidator(5)
        validator(NumericRange(10, 15))
        with self.assertRaises(exceptions.ValidationError) as cm:
            validator(NumericRange(0, 10))
        self.assertEqual(cm.exception.messages[0], 'Ensure that this range is completely greater than or equal to 5.')
        self.assertEqual(cm.exception.code, 'min_value')
  1. https://github.com/django/django/blob/617686e226231fe8ad3f2e49d3efabf6f5f434d3/django/contrib/postgres/validators.py#L70,L79
class RangeMaxValueValidator(MaxValueValidator):
    def compare(self, a, b):
        return a.upper > b
    message = _('Ensure that this range is completely less than or equal to %(limit_value)s.')


class RangeMinValueValidator(MinValueValidator):
    def compare(self, a, b):
        return a.lower < b
    message = _('Ensure that this range is completely greater than or equal to %(limit_value)s.')

Encountered Server Error (500) using Django 1.10 (Python 3.5)

Based on:

    pages = IntegerRangeField(
        validators=[
            RangeMaxValueValidator(750),
            RangeMinValueValidator(0),
        ],
    )

Input (via. django-admin):

  • lower: 0
  • upper: (blank)

Resulted in Server Error (500), with the corresponding traceback tail:

  File ".../django/contrib/postgres/validators.py", line 75, in compare
    return a.upper > b
TypeError: unorderable types: NoneType() > int()

Subsequently tested RangeMinValueValidator with the following input:

  • lower: (blank)
  • upper: 750

Resulted in Server Error (500), with the corresponding traceback tail:

  File ".../django/contrib/postgres/validators.py", line 82, in compare
    return a.lower < b
TypeError: unorderable types: NoneType() < int()

Devised test cases based on master

Neither of the two existing test cases(5) test for infinite (unbounded) ranges(1), i.e. using None(3). Furthermore, only NumericRange was being tested.

  1. Using the two existing test cases as a basis, produced another two test cases using None
  2. Then used these four test cases to devise similar test cases for DateRange as well

$ ./runtests.py --settings=test_postgres postgres_tests.test_ranges.TestValidators:

.E.E.E.E
======================================================================
ERROR: test_daterange_max_with_infinite_bound (postgres_tests.test_ranges.TestValidators)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/scratch/GitHub/django-repo/tests/postgres_tests/test_ranges.py", line 455, in test_daterange_max_with_infinite_bound
    validator(DateRange('2017-01-01', None))
  File "/home/scratch/GitHub/django-repo/django/core/validators.py", line 321, in __call__
    if self.compare(cleaned, self.limit_value):
  File "/home/scratch/GitHub/django-repo/django/contrib/postgres/validators.py", line 72, in compare
    return a.upper > b
TypeError: '>' not supported between instances of 'NoneType' and 'str'

======================================================================
ERROR: test_daterange_min_with_infinite_bound (postgres_tests.test_ranges.TestValidators)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/scratch/GitHub/django-repo/tests/postgres_tests/test_ranges.py", line 477, in test_daterange_min_with_infinite_bound
    validator(DateRange(None, '2019-01-01'))
  File "/home/scratch/GitHub/django-repo/django/core/validators.py", line 321, in __call__
    if self.compare(cleaned, self.limit_value):
  File "/home/scratch/GitHub/django-repo/django/contrib/postgres/validators.py", line 78, in compare
    return a.lower < b
TypeError: '<' not supported between instances of 'NoneType' and 'str'

======================================================================
ERROR: test_numericrange_max_with_infinite_bound (postgres_tests.test_ranges.TestValidators)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/scratch/GitHub/django-repo/tests/postgres_tests/test_ranges.py", line 420, in test_numericrange_max_with_infinite_bound
    validator(NumericRange(0, None))
  File "/home/scratch/GitHub/django-repo/django/core/validators.py", line 321, in __call__
    if self.compare(cleaned, self.limit_value):
  File "/home/scratch/GitHub/django-repo/django/contrib/postgres/validators.py", line 72, in compare
    return a.upper > b
TypeError: '>' not supported between instances of 'NoneType' and 'int'

======================================================================
ERROR: test_numericrange_min_with_infinite_bound (postgres_tests.test_ranges.TestValidators)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/scratch/GitHub/django-repo/tests/postgres_tests/test_ranges.py", line 436, in test_numericrange_min_with_infinite_bound
    validator(NumericRange(None, 10))
  File "/home/scratch/GitHub/django-repo/django/core/validators.py", line 321, in __call__
    if self.compare(cleaned, self.limit_value):
  File "/home/scratch/GitHub/django-repo/django/contrib/postgres/validators.py", line 78, in compare
    return a.lower < b
TypeError: '<' not supported between instances of 'NoneType' and 'int'

----------------------------------------------------------------------
Ran 8 tests in 0.038s

FAILED (errors=4)

Prepared patch

Proposed patch is to modify both of the range validators(6):

-        return a.upper > b
+        return True if a.upper is None else a.upper > b
-        return a.lower < b
+        return True if a.lower is None else a.lower < b

$ ./runtests.py --settings=test_postgres postgres_tests.test_ranges.TestValidators:

........
----------------------------------------------------------------------
Ran 8 tests in 0.006s

OK

Change History (4)

comment:1 Changed 3 weeks ago by myii

comment:2 Changed 3 weeks ago by Shai Berger

Cc: Shai Berger added
Triage Stage: UnreviewedAccepted

Accepting based on the very high quality of the report, haven't actually reproduced. Left some comments on the PR

comment:3 Changed 3 weeks ago by myii

Cc: myii added

GitHub patch rebased according to recommendations received from Shai Berger.

comment:4 Changed 2 weeks ago by Tim Graham <timograham@…>

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In 3e7497a0:

Fixed #28758 -- Fixed RangeMax/MinValueValidators crash with unbound ranges.

Note: See TracTickets for help on using tickets.
Back to Top