Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[1.2.X] Fixed #15103 - SuspiciousOperation with limit_choices_to and …

…raw_id_fields

Thanks to natrius for the report.

This patch also fixes some unicode bugs in affected code.

Backport of [15347] from trunk.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.2.X@15349 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 276ecc72097b45b5115a7d6657332d5a37ca5435 1 parent 8ab768f
Luke Plant authored January 28, 2011
12  django/contrib/admin/options.py
@@ -185,7 +185,16 @@ def _declared_fieldsets(self):
185 185
     def get_readonly_fields(self, request, obj=None):
186 186
         return self.readonly_fields
187 187
 
188  
-    def lookup_allowed(self, lookup):
  188
+    def lookup_allowed(self, lookup, value):
  189
+        model = self.model
  190
+        # Check FKey lookups that are allowed, so that popups produced by
  191
+        # ForeignKeyRawIdWidget, on the basis of ForeignKey.limit_choices_to,
  192
+        # are allowed to work.
  193
+        for l in model._meta.related_fkey_lookups:
  194
+            for k, v in widgets.url_params_from_lookup_dict(l).items():
  195
+                if k == lookup and v == value:
  196
+                    return True
  197
+
189 198
         parts = lookup.split(LOOKUP_SEP)
190 199
 
191 200
         # Last term in lookup is a query term (__exact, __startswith etc)
@@ -197,7 +206,6 @@ def lookup_allowed(self, lookup):
197 206
         # if foo has been specificially included in the lookup list; so
198 207
         # drop __id if it is the last part. However, first we need to find
199 208
         # the pk attribute name.
200  
-        model = self.model
201 209
         pk_attr_name = None
202 210
         for part in parts[:-1]:
203 211
             field, _, _, _ = model._meta.get_field_by_name(part)
10  django/contrib/admin/views/main.py
@@ -179,16 +179,18 @@ def get_query_set(self):
179 179
 
180 180
             # if key ends with __in, split parameter into separate values
181 181
             if key.endswith('__in'):
182  
-                lookup_params[key] = value.split(',')
  182
+                value = value.split(',')
  183
+                lookup_params[key] = value
183 184
 
184 185
             # if key ends with __isnull, special case '' and false
185 186
             if key.endswith('__isnull'):
186 187
                 if value.lower() in ('', 'false'):
187  
-                    lookup_params[key] = False
  188
+                    value = False
188 189
                 else:
189  
-                    lookup_params[key] = True
  190
+                    value = True
  191
+                lookup_params[key] = value
190 192
 
191  
-            if not self.model_admin.lookup_allowed(key):
  193
+            if not self.model_admin.lookup_allowed(key, value):
192 194
                 raise SuspiciousOperation(
193 195
                     "Filtering by %s not allowed" % key
194 196
                 )
37  django/contrib/admin/widgets.py
@@ -100,6 +100,23 @@ def render(self, name, value, attrs=None):
100 100
         output.append(super(AdminFileWidget, self).render(name, value, attrs))
101 101
         return mark_safe(u''.join(output))
102 102
 
  103
+def url_params_from_lookup_dict(lookups):
  104
+    """
  105
+    Converts the type of lookups specified in a ForeignKey limit_choices_to
  106
+    attribute to a dictionary of query parameters
  107
+    """
  108
+    params = {}
  109
+    if lookups and hasattr(lookups, 'items'):
  110
+        items = []
  111
+        for k, v in lookups.items():
  112
+            if isinstance(v, list):
  113
+                v = u','.join([str(x) for x in v])
  114
+            else:
  115
+                v = unicode(v)
  116
+            items.append((k, v))
  117
+        params.update(dict(items))
  118
+    return params
  119
+
103 120
 class ForeignKeyRawIdWidget(forms.TextInput):
104 121
     """
105 122
     A Widget for displaying ForeignKeys in the "raw_id" interface rather than
@@ -116,33 +133,23 @@ def render(self, name, value, attrs=None):
116 133
         related_url = '../../../%s/%s/' % (self.rel.to._meta.app_label, self.rel.to._meta.object_name.lower())
117 134
         params = self.url_parameters()
118 135
         if params:
119  
-            url = '?' + '&'.join(['%s=%s' % (k, v) for k, v in params.items()])
  136
+            url = u'?' + u'&'.join([u'%s=%s' % (k, v) for k, v in params.items()])
120 137
         else:
121  
-            url = ''
  138
+            url = u''
122 139
         if not attrs.has_key('class'):
123 140
             attrs['class'] = 'vForeignKeyRawIdAdminField' # The JavaScript looks for this hook.
124 141
         output = [super(ForeignKeyRawIdWidget, self).render(name, value, attrs)]
125 142
         # TODO: "id_" is hard-coded here. This should instead use the correct
126 143
         # API to determine the ID dynamically.
127  
-        output.append('<a href="%s%s" class="related-lookup" id="lookup_id_%s" onclick="return showRelatedObjectLookupPopup(this);"> ' % \
  144
+        output.append(u'<a href="%s%s" class="related-lookup" id="lookup_id_%s" onclick="return showRelatedObjectLookupPopup(this);"> ' % \
128 145
             (related_url, url, name))
129  
-        output.append('<img src="%simg/admin/selector-search.gif" width="16" height="16" alt="%s" /></a>' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup')))
  146
+        output.append(u'<img src="%simg/admin/selector-search.gif" width="16" height="16" alt="%s" /></a>' % (settings.ADMIN_MEDIA_PREFIX, _('Lookup')))
130 147
         if value:
131 148
             output.append(self.label_for_value(value))
132 149
         return mark_safe(u''.join(output))
133 150
 
134 151
     def base_url_parameters(self):
135  
-        params = {}
136  
-        if self.rel.limit_choices_to and hasattr(self.rel.limit_choices_to, 'items'):
137  
-            items = []
138  
-            for k, v in self.rel.limit_choices_to.items():
139  
-                if isinstance(v, list):
140  
-                    v = ','.join([str(x) for x in v])
141  
-                else:
142  
-                    v = str(v)
143  
-                items.append((k, v))
144  
-            params.update(dict(items))
145  
-        return params
  152
+        return url_params_from_lookup_dict(self.rel.limit_choices_to)
146 153
 
147 154
     def url_parameters(self):
148 155
         from django.contrib.admin.views.main import TO_FIELD_VAR
2  django/db/models/fields/related.py
@@ -895,6 +895,8 @@ def contribute_to_related_class(self, cls, related):
4  django/db/models/options.py
@@ -50,6 +50,10 @@ def __init__(self, meta, app_label=None):
50 50
         self.abstract_managers = []
51 51
         self.concrete_managers = []
52 52
 
  53
+        # List of all lookups defined in ForeignKey 'limit_choices_to' options
  54
+        # from *other* models. Needed for some admin checks. Internal use only.
  55
+        self.related_fkey_lookups = []
  56
+
53 57
     def contribute_to_class(self, cls, name):
54 58
         from django.db import connection
55 59
         from django.db.backends.util import truncate_name
23  tests/regressiontests/admin_views/models.py
@@ -151,6 +151,26 @@ def __unicode__(self):
151 151
 class ThingAdmin(admin.ModelAdmin):
152 152
     list_filter = ('color',)
153 153
 
  154
+class Actor(models.Model):
  155
+    name = models.CharField(max_length=50)
  156
+    age = models.IntegerField()
  157
+    def __unicode__(self):
  158
+        return self.name
  159
+
  160
+class Inquisition(models.Model):
  161
+    expected = models.BooleanField()
  162
+    leader = models.ForeignKey(Actor)
  163
+    def __unicode__(self):
  164
+        return self.expected
  165
+
  166
+class Sketch(models.Model):
  167
+    title = models.CharField(max_length=100)
  168
+    inquisition = models.ForeignKey(Inquisition, limit_choices_to={'leader__name': 'Palin',
  169
+                                                                   'leader__age': 27,
  170
+                                                                   })
  171
+    def __unicode__(self):
  172
+        return self.title
  173
+
154 174
 class Fabric(models.Model):
155 175
     NG_CHOICES = (
156 176
         ('Textured', (
@@ -605,6 +625,9 @@ class WorkHourAdmin(admin.ModelAdmin):
605 625
 admin.site.register(ModelWithStringPrimaryKey)
606 626
 admin.site.register(Color)
607 627
 admin.site.register(Thing, ThingAdmin)
  628
+admin.site.register(Actor)
  629
+admin.site.register(Inquisition)
  630
+admin.site.register(Sketch)
608 631
 admin.site.register(Person, PersonAdmin)
609 632
 admin.site.register(Persona, PersonaAdmin)
610 633
 admin.site.register(Subscriber, SubscriberAdmin)
11  tests/regressiontests/admin_views/tests.py
@@ -337,6 +337,17 @@ def test_disallowed_filtering(self):
337 337
         response = self.client.get("/test_admin/admin/admin_views/workhour/?employee__person_ptr__exact=%d" % e1.pk)
338 338
         self.assertEqual(response.status_code, 200)
339 339
 
  340
+    def test_allowed_filtering_15103(self):
  341
+        """
  342
+        Regressions test for ticket 15103 - filtering on fields defined in a
  343
+        ForeignKey 'limit_choices_to' should be allowed, otherwise raw_id_fields
  344
+        can break.
  345
+        """
  346
+        try:
  347
+            self.client.get("/test_admin/admin/admin_views/inquisition/?leader__name=Palin&leader__age=27")
  348
+        except SuspiciousOperation:
  349
+            self.fail("Filters should be allowed if they are defined on a ForeignKey pointing to this model")
  350
+
340 351
 class SaveAsTests(TestCase):
341 352
     fixtures = ['admin-views-users.xml','admin-views-person.xml']
342 353
 

0 notes on commit 276ecc7

Please sign in to comment.
Something went wrong with that request. Please try again.