queryset-refactor: Added some error checking for a potential crasher if model ordering is set up in a cycle somehow. The error reporting here isn't perfect (it doesn't give any hints about what the infinite loop might be), but it's better than nothing.

git-svn-id: http://code.djangoproject.com/svn/django/branches/queryset-refactor@7046 bcc190cf-cafb-0310-a4f2-bffc1f526a37

2 files changed, 32 insertions, 2 deletions

diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index cb8bbc36bf..af6ec2bfbc 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -507,7 +507,8 @@ class Query(object):
                     result.append('%s %s' % (elt, order))
         return result
 
-    def find_ordering_name(self, name, opts, alias=None, default_order='ASC'):
+    def find_ordering_name(self, name, opts, alias=None, default_order='ASC',
+            already_seen=None):
         """
         Returns the table alias (the name might be ambiguous, the alias will
         not be) and column name for ordering by the given 'name' parameter.
@@ -525,10 +526,18 @@ class Query(object):
         # If we get to this point and the field is a relation to another model,
         # append the default ordering for that model.
         if len(joins) > 1 and opts.ordering:
+            # Firstly, avoid infinite loops.
+            if not already_seen:
+                already_seen = {}
+            join_tuple = tuple([tuple(j) for j in joins])
+            if join_tuple in already_seen:
+                raise TypeError('Infinite loop caused by ordering.')
+            already_seen[join_tuple] = True
+
             results = []
             for item in opts.ordering:
                 results.extend(self.find_ordering_name(item, opts, alias,
-                        order))
+                        order, already_seen))
             return results
 
         if alias:
diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py
index 67f2cc098d..22fa4f7803 100644
--- a/tests/regressiontests/queries/models.py
+++ b/tests/regressiontests/queries/models.py
@@ -97,6 +97,20 @@ class X(models.Model):
 class Y(models.Model):
     x1 = models.ForeignKey(X, related_name='y1')
 
+# Some models with a cycle in the default ordering. This would be bad if we
+# didn't catch the infinite loop.
+class LoopX(models.Model):
+    y = models.ForeignKey('LoopY')
+
+    class Meta:
+        ordering = ['y']
+
+class LoopY(models.Model):
+    x = models.ForeignKey(LoopX)
+
+    class Meta:
+        ordering = ['x']
+
 __test__ = {'API_TESTS':"""
 >>> t1 = Tag(name='t1')
 >>> t1.save()
@@ -373,6 +387,13 @@ Bug #2076
 >>> Cover.objects.all()
 [<Cover: first>, <Cover: second>]
 
+# If you're not careful, it's possible to introduce infinite loops via default
+# ordering on foreign keys in a cycle. We detect that.
+>>> LoopX.objects.all()
+Traceback (most recent call last):
+...
+TypeError: Infinite loop caused by ordering.
+
 # If the remote model does not have a default ordering, we order by its 'id'
 # field.
 >>> Item.objects.order_by('creator', 'name')