Opened 7 years ago
Closed 7 years ago
#28758 closed Bug (fixed)
Fix range min/max validators to prevent errors for infinite ranges
Reported by: | myii | Owned by: | |
---|---|---|---|
Component: | contrib.postgres | Version: | dev |
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
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.
All of the range fields translate to psycopg2 Range objects in python, ...
Parameters:
- lower – lower bound for the range.
None
means unbound- upper – upper bound for the range.
None
means unbound
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
.
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')
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.
- Using the two existing test cases as a basis, produced another two test cases using
None
- 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 by , 7 years ago
comment:2 by , 7 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Accepting based on the very high quality of the report, haven't actually reproduced. Left some comments on the PR
comment:3 by , 7 years ago
Cc: | added |
---|
GitHub patch rebased according to recommendations received from Shai Berger.
Tests + proposed patch: https://github.com/django/django/pull/9315