Fixed #31426 -- Added proper field validation to QuerySet.order_by().

Resolve the field reference instead of using fragile regex based string
reference validation.
This commit is contained in:
Simon Charette 2020-04-05 15:45:06 -04:00 committed by Mariusz Felisiak
parent 98ea4f0f46
commit 513948735b
3 changed files with 19 additions and 19 deletions

View File

@ -2,8 +2,6 @@
Constants specific to the SQL storage portion of the ORM. Constants specific to the SQL storage portion of the ORM.
""" """
from django.utils.regex_helper import _lazy_re_compile
# Size of each "chunk" for get_iterator calls. # Size of each "chunk" for get_iterator calls.
# Larger values are slightly faster at the expense of more storage space. # Larger values are slightly faster at the expense of more storage space.
GET_ITERATOR_CHUNK_SIZE = 100 GET_ITERATOR_CHUNK_SIZE = 100
@ -16,7 +14,6 @@ SINGLE = 'single'
CURSOR = 'cursor' CURSOR = 'cursor'
NO_RESULTS = 'no results' NO_RESULTS = 'no results'
ORDER_PATTERN = _lazy_re_compile(r'\?|[-+]?[.\w]+$')
ORDER_DIR = { ORDER_DIR = {
'ASC': ('ASC', 'DESC'), 'ASC': ('ASC', 'DESC'),
'DESC': ('DESC', 'ASC'), 'DESC': ('DESC', 'ASC'),

View File

@ -30,9 +30,7 @@ from django.db.models.lookups import Lookup
from django.db.models.query_utils import ( from django.db.models.query_utils import (
Q, check_rel_lookup_compatibility, refs_expression, Q, check_rel_lookup_compatibility, refs_expression,
) )
from django.db.models.sql.constants import ( from django.db.models.sql.constants import INNER, LOUTER, ORDER_DIR, SINGLE
INNER, LOUTER, ORDER_DIR, ORDER_PATTERN, SINGLE,
)
from django.db.models.sql.datastructures import ( from django.db.models.sql.datastructures import (
BaseTable, Empty, Join, MultiJoin, BaseTable, Empty, Join, MultiJoin,
) )
@ -1895,7 +1893,7 @@ class Query(BaseExpression):
""" """
errors = [] errors = []
for item in ordering: for item in ordering:
if isinstance(item, str) and ORDER_PATTERN.match(item): if isinstance(item, str):
if '.' in item: if '.' in item:
warnings.warn( warnings.warn(
'Passing column raw column aliases to order_by() is ' 'Passing column raw column aliases to order_by() is '
@ -1904,6 +1902,18 @@ class Query(BaseExpression):
category=RemovedInDjango40Warning, category=RemovedInDjango40Warning,
stacklevel=3, stacklevel=3,
) )
continue
if item == '?':
continue
if item.startswith('-'):
item = item[1:]
if item in self.annotations:
continue
if self.extra and item in self.extra:
continue
# names_to_path() validates the lookup. A descriptive
# FieldError will be raise if it's not.
self.names_to_path(item.split(LOOKUP_SEP), self.model._meta)
elif not hasattr(item, 'resolve_expression'): elif not hasattr(item, 'resolve_expression'):
errors.append(item) errors.append(item)
if getattr(item, 'contains_aggregate', False): if getattr(item, 'contains_aggregate', False):

View File

@ -3110,20 +3110,13 @@ class QuerySetExceptionTests(SimpleTestCase):
with self.assertRaisesMessage(AttributeError, msg): with self.assertRaisesMessage(AttributeError, msg):
list(qs) list(qs)
def test_invalid_qs_list(self):
# Test for #19895 - second iteration over invalid queryset
# raises errors.
qs = Article.objects.order_by('invalid_column')
msg = "Cannot resolve keyword 'invalid_column' into field."
with self.assertRaisesMessage(FieldError, msg):
list(qs)
with self.assertRaisesMessage(FieldError, msg):
list(qs)
def test_invalid_order_by(self): def test_invalid_order_by(self):
msg = "Invalid order_by arguments: ['*']" msg = (
"Cannot resolve keyword '*' into field. Choices are: created, id, "
"name"
)
with self.assertRaisesMessage(FieldError, msg): with self.assertRaisesMessage(FieldError, msg):
list(Article.objects.order_by('*')) Article.objects.order_by('*')
def test_invalid_queryset_model(self): def test_invalid_queryset_model(self):
msg = 'Cannot use QuerySet for "Article": Use a QuerySet for "ExtraInfo".' msg = 'Cannot use QuerySet for "Article": Use a QuerySet for "ExtraInfo".'