Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Stopped unconditionally reversing admin model add/change URLs.

Starting with [16857] this could cause HTTP 500 errors when
`ModelAdmin.get_urls()` has been customized to the point it doesn't
provide these standard URLs.

Fixes #17333. Refs #15294.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@17237 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 554f0601b59309df8c9da2f14c56f9ea97aabc91 1 parent 259ebcd
Ramiro Morales authored December 19, 2011
26  django/contrib/admin/sites.py
@@ -7,7 +7,7 @@
7 7
 from django.views.decorators.csrf import csrf_protect
8 8
 from django.db.models.base import ModelBase
9 9
 from django.core.exceptions import ImproperlyConfigured
10  
-from django.core.urlresolvers import reverse
  10
+from django.core.urlresolvers import reverse, NoReverseMatch
11 11
 from django.template.response import TemplateResponse
12 12
 from django.utils.safestring import mark_safe
13 13
 from django.utils.text import capfirst
@@ -342,10 +342,18 @@ def index(self, request, extra_context=None):
342 342
                     info = (app_label, model._meta.module_name)
343 343
                     model_dict = {
344 344
                         'name': capfirst(model._meta.verbose_name_plural),
345  
-                        'admin_url': reverse('admin:%s_%s_changelist' % info, current_app=self.name),
346  
-                        'add_url': reverse('admin:%s_%s_add' % info, current_app=self.name),
347 345
                         'perms': perms,
348 346
                     }
  347
+                    if perms.get('change', False):
  348
+                        try:
  349
+                            model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name)
  350
+                        except NoReverseMatch:
  351
+                            pass
  352
+                    if perms.get('add', False):
  353
+                        try:
  354
+                            model_dict['add_url'] = reverse('admin:%s_%s_add' % info, current_app=self.name)
  355
+                        except NoReverseMatch:
  356
+                            pass
349 357
                     if app_label in app_dict:
350 358
                         app_dict[app_label]['models'].append(model_dict)
351 359
                     else:
@@ -388,10 +396,18 @@ def app_index(self, request, app_label, extra_context=None):
388 396
                         info = (app_label, model._meta.module_name)
389 397
                         model_dict = {
390 398
                             'name': capfirst(model._meta.verbose_name_plural),
391  
-                            'admin_url': reverse('admin:%s_%s_changelist' % info, current_app=self.name),
392  
-                            'add_url': reverse('admin:%s_%s_add' % info, current_app=self.name),
393 399
                             'perms': perms,
394 400
                         }
  401
+                        if perms.get('change', False):
  402
+                            try:
  403
+                                model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name)
  404
+                            except NoReverseMatch:
  405
+                                pass
  406
+                        if perms.get('add', False):
  407
+                            try:
  408
+                                model_dict['add_url'] = reverse('admin:%s_%s_add' % info, current_app=self.name)
  409
+                            except NoReverseMatch:
  410
+                                pass
395 411
                         if app_dict:
396 412
                             app_dict['models'].append(model_dict),
397 413
                         else:
6  django/contrib/admin/templates/admin/index.html
@@ -19,19 +19,19 @@
19 19
         <caption><a href="{{ app.app_url }}" class="section">{% blocktrans with name=app.name %}{{ name }}{% endblocktrans %}</a></caption>
20 20
         {% for model in app.models %}
21 21
             <tr>
22  
-            {% if model.perms.change %}
  22
+            {% if model.admin_url %}
23 23
                 <th scope="row"><a href="{{ model.admin_url }}">{{ model.name }}</a></th>
24 24
             {% else %}
25 25
                 <th scope="row">{{ model.name }}</th>
26 26
             {% endif %}
27 27
 
28  
-            {% if model.perms.add %}
  28
+            {% if model.add_url %}
29 29
                 <td><a href="{{ model.add_url }}" class="addlink">{% trans 'Add' %}</a></td>
30 30
             {% else %}
31 31
                 <td>&nbsp;</td>
32 32
             {% endif %}
33 33
 
34  
-            {% if model.perms.change %}
  34
+            {% if model.admin_url %}
35 35
                 <td><a href="{{ model.admin_url }}" class="changelink">{% trans 'Change' %}</a></td>
36 36
             {% else %}
37 37
                 <td>&nbsp;</td>
17  tests/regressiontests/admin_views/admin.py
... ...
@@ -1,7 +1,6 @@
1 1
 # -*- coding: utf-8 -*-
2 2
 from __future__ import absolute_import
3 3
 
4  
-import datetime
5 4
 import tempfile
6 5
 import os
7 6
 
@@ -10,8 +9,10 @@
10 9
 from django.contrib.admin.views.main import ChangeList
11 10
 from django.core.files.storage import FileSystemStorage
12 11
 from django.core.mail import EmailMessage
  12
+from django.conf.urls import patterns, url
13 13
 from django.db import models
14 14
 from django.forms.models import BaseModelFormSet
  15
+from django.http import HttpResponse
15 16
 
16 17
 from .models import (Article, Chapter, Account, Media, Child, Parent, Picture,
17 18
     Widget, DooHickey, Grommet, Whatsit, FancyDoodad, Category, Link,
@@ -24,7 +25,7 @@
24 25
     CoverLetter, Story, OtherStory, Book, Promo, ChapterXtra1, Pizza, Topping,
25 26
     Album, Question, Answer, ComplexSortedPerson, PrePopulatedPostLargeSlug,
26 27
     AdminOrderedField, AdminOrderedModelMethod, AdminOrderedAdminMethod,
27  
-    AdminOrderedCallable)
  28
+    AdminOrderedCallable, Report)
28 29
 
29 30
 
30 31
 def callable_year(dt_value):
@@ -499,6 +500,17 @@ class AdminOrderedCallableAdmin(admin.ModelAdmin):
499 500
     ordering = ('order',)
500 501
     list_display = ('stuff', admin_ordered_callable)
501 502
 
  503
+class ReportAdmin(admin.ModelAdmin):
  504
+    def extra(self, request):
  505
+        return HttpResponse()
  506
+
  507
+    def get_urls(self):
  508
+        # Corner case: Don't call parent implementation
  509
+        return patterns('',
  510
+            url(r'^extra/$',
  511
+                self.extra,
  512
+                name='cable_extra'),
  513
+        )
502 514
 
503 515
 site = admin.AdminSite(name="admin")
504 516
 site.register(Article, ArticleAdmin)
@@ -543,6 +555,7 @@ class AdminOrderedCallableAdmin(admin.ModelAdmin):
543 555
 site.register(CoverLetter, CoverLetterAdmin)
544 556
 site.register(Story, StoryAdmin)
545 557
 site.register(OtherStory, OtherStoryAdmin)
  558
+site.register(Report, ReportAdmin)
546 559
 
547 560
 # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2.
548 561
 # That way we cover all four cases:
6  tests/regressiontests/admin_views/models.py
@@ -566,3 +566,9 @@ class AdminOrderedAdminMethod(models.Model):
566 566
 class AdminOrderedCallable(models.Model):
567 567
     order = models.IntegerField()
568 568
     stuff = models.CharField(max_length=200)
  569
+
  570
+class Report(models.Model):
  571
+    title = models.CharField(max_length=100)
  572
+
  573
+    def __unicode__(self):
  574
+        return self.title
34  tests/regressiontests/admin_views/tests.py
@@ -38,7 +38,8 @@
38 38
     Book, Promo, WorkHour, Employee, Question, Answer, Inquisition, Actor,
39 39
     FoodDelivery, RowLevelChangePermissionModel, Paper, CoverLetter, Story,
40 40
     OtherStory, ComplexSortedPerson, Parent, Child, AdminOrderedField,
41  
-    AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable)
  41
+    AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable,
  42
+    Report)
42 43
 
43 44
 
44 45
 ERROR_MESSAGE = "Please enter the correct username and password \
@@ -1090,6 +1091,37 @@ def testDisabledPermissionsWhenLoggedIn(self):
1090 1091
         self.assertContains(response, 'id="login-form"')
1091 1092
 
1092 1093
 
  1094
+class AdminViewsNoUrlTest(TestCase):
  1095
+    """Regression test for #17333"""
  1096
+
  1097
+    urls = "regressiontests.admin_views.urls"
  1098
+    fixtures = ['admin-views-users.xml']
  1099
+
  1100
+    def setUp(self):
  1101
+        opts = Report._meta
  1102
+        # User who can change Reports
  1103
+        change_user = User.objects.get(username='changeuser')
  1104
+        change_user.user_permissions.add(get_perm(Report,
  1105
+            opts.get_change_permission()))
  1106
+
  1107
+        # login POST dict
  1108
+        self.changeuser_login = {
  1109
+            REDIRECT_FIELD_NAME: '/test_admin/admin/',
  1110
+            LOGIN_FORM_KEY: 1,
  1111
+            'username': 'changeuser',
  1112
+            'password': 'secret',
  1113
+        }
  1114
+
  1115
+    def test_no_standard_modeladmin_urls(self):
  1116
+        """Admin index views don't break when user's ModelAdmin removes standard urls"""
  1117
+        self.client.get('/test_admin/admin/')
  1118
+        self.client.post('/test_admin/admin/', self.changeuser_login)
  1119
+        r = self.client.get('/test_admin/admin/')
  1120
+        # we shouldn' get an 500 error caused by a NoReverseMatch
  1121
+        self.assertEqual(r.status_code, 200)
  1122
+        self.client.get('/test_admin/admin/logout/')
  1123
+
  1124
+
1093 1125
 class AdminViewDeletedObjectsTest(TestCase):
1094 1126
     urls = "regressiontests.admin_views.urls"
1095 1127
     fixtures = ['admin-views-users.xml', 'deleted-objects.xml']

0 notes on commit 554f060

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