Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

newforms-admin: Fixed #5490 -- Properly quote special characters in p…

…rimary keys in the admin. Added tests to ensure functionality. This also moves quote and unquote to django/contrib/admin/util.py. Thanks jdetaeye and shanx for all your help.

git-svn-id: http://code.djangoproject.com/svn/django/branches/newforms-admin@7935 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 075a2fd938a95a77a5a63567bfad596b303765bf 1 parent 584c41a
Brian Rosner authored July 16, 2008
3  django/contrib/admin/models.py
... ...
@@ -1,6 +1,7 @@
1 1
 from django.db import models
2 2
 from django.contrib.contenttypes.models import ContentType
3 3
 from django.contrib.auth.models import User
  4
+from django.contrib.admin.util import quote
4 5
 from django.utils.translation import ugettext_lazy as _
5 6
 from django.utils.encoding import smart_unicode
6 7
 from django.utils.safestring import mark_safe
@@ -50,4 +51,4 @@ def get_admin_url(self):
50 51
         Returns the admin URL to edit the object represented by this log entry.
51 52
         This is relative to the Django admin index page.
52 53
         """
53  
-        return mark_safe(u"%s/%s/%s/" % (self.content_type.app_label, self.content_type.model, self.object_id))
  54
+        return mark_safe(u"%s/%s/%s/" % (self.content_type.app_label, self.content_type.model, quote(self.object_id)))
24  django/contrib/admin/options.py
@@ -5,7 +5,7 @@
5 5
 from django.newforms.models import BaseInlineFormset
6 6
 from django.contrib.contenttypes.models import ContentType
7 7
 from django.contrib.admin import widgets
8  
-from django.contrib.admin.util import get_deleted_objects
  8
+from django.contrib.admin.util import quote, unquote, get_deleted_objects
9 9
 from django.core.exceptions import ImproperlyConfigured, PermissionDenied
10 10
 from django.db import models, transaction
11 11
 from django.http import Http404, HttpResponse, HttpResponseRedirect
@@ -24,26 +24,6 @@
24 24
 class IncorrectLookupParameters(Exception):
25 25
     pass
26 26
 
27  
-def unquote(s):
28  
-    """
29  
-    Undo the effects of quote(). Based heavily on urllib.unquote().
30  
-    """
31  
-    mychr = chr
32  
-    myatoi = int
33  
-    list = s.split('_')
34  
-    res = [list[0]]
35  
-    myappend = res.append
36  
-    del list[0]
37  
-    for item in list:
38  
-        if item[1:2]:
39  
-            try:
40  
-                myappend(mychr(myatoi(item[:2], 16)) + item[2:])
41  
-            except ValueError:
42  
-                myappend('_' + item)
43  
-        else:
44  
-            myappend('_' + item)
45  
-    return "".join(res)
46  
-
47 27
 def flatten_fieldsets(fieldsets):
48 28
     """Returns a list of field names from an admin fieldsets structure."""
49 29
     field_names = []
@@ -656,7 +636,7 @@ def delete_view(self, request, object_id, extra_context=None):
656 636
 
657 637
         # Populate deleted_objects, a data structure of all related objects that
658 638
         # will also be deleted.
659  
-        deleted_objects = [mark_safe(u'%s: <a href="../../%s/">%s</a>' % (escape(force_unicode(capfirst(opts.verbose_name))), force_unicode(object_id), escape(obj))), []]
  639
+        deleted_objects = [mark_safe(u'%s: <a href="../../%s/">%s</a>' % (escape(force_unicode(capfirst(opts.verbose_name))), quote(object_id), escape(obj))), []]
660 640
         perms_needed = sets.Set()
661 641
         get_deleted_objects(deleted_objects, perms_needed, request.user, obj, opts, 1, self.admin_site)
662 642
 
37  django/contrib/admin/util.py
@@ -6,6 +6,43 @@
6 6
 from django.utils.encoding import force_unicode
7 7
 from django.utils.translation import ugettext as _
8 8
 
  9
+
  10
+def quote(s):
  11
+    """
  12
+    Ensure that primary key values do not confuse the admin URLs by escaping
  13
+    any '/', '_' and ':' characters. Similar to urllib.quote, except that the
  14
+    quoting is slightly different so that it doesn't get automatically
  15
+    unquoted by the Web browser.
  16
+    """
  17
+    if not isinstance(s, basestring):
  18
+        return s
  19
+    res = list(s)
  20
+    for i in range(len(res)):
  21
+        c = res[i]
  22
+        if c in """:/_#?;@&=+$,"<>%\\""":
  23
+            res[i] = '_%02X' % ord(c)
  24
+    return ''.join(res)
  25
+
  26
+def unquote(s):
  27
+    """
  28
+    Undo the effects of quote(). Based heavily on urllib.unquote().
  29
+    """
  30
+    mychr = chr
  31
+    myatoi = int
  32
+    list = s.split('_')
  33
+    res = [list[0]]
  34
+    myappend = res.append
  35
+    del list[0]
  36
+    for item in list:
  37
+        if item[1:2]:
  38
+            try:
  39
+                myappend(mychr(myatoi(item[:2], 16)) + item[2:])
  40
+            except ValueError:
  41
+                myappend('_' + item)
  42
+        else:
  43
+            myappend('_' + item)
  44
+    return "".join(res)
  45
+
9 46
 def _nest_help(obj, depth, val):
10 47
     current = obj
11 48
     for i in range(depth):
17  django/contrib/admin/views/main.py
... ...
@@ -1,5 +1,6 @@
1 1
 from django.contrib.admin.filterspecs import FilterSpec
2 2
 from django.contrib.admin.options import IncorrectLookupParameters
  3
+from django.contrib.admin.util import quote
3 4
 from django.core.paginator import Paginator, InvalidPage
4 5
 from django.db import models
5 6
 from django.db.models.query import QuerySet
@@ -30,22 +31,6 @@
30 31
 # Text to display within change-list table cells if the value is blank.
31 32
 EMPTY_CHANGELIST_VALUE = '(None)'
32 33
 
33  
-def quote(s):
34  
-    """
35  
-    Ensure that primary key values do not confuse the admin URLs by escaping
36  
-    any '/', '_' and ':' characters. Similar to urllib.quote, except that the
37  
-    quoting is slightly different so that it doesn't get automatically
38  
-    unquoted by the Web browser.
39  
-    """
40  
-    if type(s) != type(''):
41  
-        return s
42  
-    res = list(s)
43  
-    for i in range(len(res)):
44  
-        c = res[i]
45  
-        if c in ':/_':
46  
-            res[i] = '_%02X' % ord(c)
47  
-    return ''.join(res)
48  
-
49 34
 class ChangeList(object):
50 35
     def __init__(self, request, model, list_display, list_display_links, list_filter, date_hierarchy, search_fields, list_select_related, list_per_page, model_admin):
51 36
         self.model = model
4  django/db/models/base.py
@@ -137,6 +137,10 @@ def __new__(cls, name, bases, attrs):
137 137
         return get_model(new_class._meta.app_label, name, False)
138 138
 
139 139
     def add_to_class(cls, name, value):
  140
+        if name == 'Admin':
  141
+            import warnings
  142
+            warnings.warn("The inner Admin class for %s is no longer supported. "
  143
+                "Please use a ModelAdmin instead." % cls.__name__)
140 144
         if hasattr(value, 'contribute_to_class'):
141 145
             value.contribute_to_class(cls, name)
142 146
         else:
6  tests/regressiontests/admin_views/fixtures/string-primary-key.xml
... ...
@@ -0,0 +1,6 @@
  1
+<?xml version="1.0" encoding="utf-8"?>
  2
+<django-objects version="1.0">
  3
+    <object pk="1" model="admin_views.modelwithstringprimarykey">
  4
+        <field type="CharField" name="id"><![CDATA[abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 -_.!~*'() ;/?:@&=+$, <>#%" {}|\^[]`]]></field>
  5
+    </object>    
  6
+</django-objects>
7  tests/regressiontests/admin_views/models.py
@@ -48,7 +48,14 @@ def changelist_view(self, request):
48 48
                 'extra_var': 'Hello!'
49 49
             }
50 50
         )
  51
+
  52
+class ModelWithStringPrimaryKey(models.Model):
  53
+    id = models.CharField(max_length=255, primary_key=True)
  54
+    
  55
+    def __unicode__(self):
  56
+        return self.id
51 57
         
52 58
 admin.site.register(Article, ArticleAdmin)
53 59
 admin.site.register(CustomArticle, CustomArticleAdmin)
54 60
 admin.site.register(Section)
  61
+admin.site.register(ModelWithStringPrimaryKey)
44  tests/regressiontests/admin_views/tests.py
@@ -2,10 +2,13 @@
2 2
 from django.test import TestCase
3 3
 from django.contrib.auth.models import User, Permission
4 4
 from django.contrib.contenttypes.models import ContentType
  5
+from django.contrib.admin.models import LogEntry
5 6
 from django.contrib.admin.sites import LOGIN_FORM_KEY, _encode_post_data
  7
+from django.contrib.admin.util import quote
  8
+from django.utils.html import escape
6 9
 
7 10
 # local test models
8  
-from models import Article, CustomArticle, Section
  11
+from models import Article, CustomArticle, Section, ModelWithStringPrimaryKey
9 12
 
10 13
 def get_perm(Model, perm):
11 14
     """Return the permission object, for the Model"""
@@ -318,3 +321,42 @@ def testDeleteView(self):
318 321
         self.assertRedirects(post, '/test_admin/admin/')
319 322
         self.failUnlessEqual(Article.objects.all().count(), 0)
320 323
         self.client.get('/test_admin/admin/logout/')
  324
+
  325
+class AdminViewStringPrimaryKeyTest(TestCase):
  326
+    fixtures = ['admin-views-users.xml', 'string-primary-key.xml']
  327
+    
  328
+    def __init__(self, *args):
  329
+        super(AdminViewStringPrimaryKeyTest, self).__init__(*args)
  330
+        self.pk = """abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 -_.!~*'() ;/?:@&=+$, <>#%" {}|\^[]`"""
  331
+    
  332
+    def setUp(self):
  333
+        self.client.login(username='super', password='secret')
  334
+        content_type_pk = ContentType.objects.get_for_model(ModelWithStringPrimaryKey).pk
  335
+        LogEntry.objects.log_action(100, content_type_pk, self.pk, self.pk, 2, change_message='')
  336
+    
  337
+    def tearDown(self):
  338
+        self.client.logout()
  339
+    
  340
+    def test_get_change_view(self):
  341
+        "Retrieving the object using urlencoded form of primary key should work"
  342
+        response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/' % quote(self.pk))
  343
+        self.assertContains(response, escape(self.pk))
  344
+        self.failUnlessEqual(response.status_code, 200)
  345
+    
  346
+    def test_changelist_to_changeform_link(self):
  347
+        "The link from the changelist referring to the changeform of the object should be quoted"
  348
+        response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/')
  349
+        should_contain = """<tr class="row1"><th><a href="%s/">%s</a></th></tr>""" % (quote(self.pk), escape(self.pk))
  350
+        self.assertContains(response, should_contain)
  351
+    
  352
+    def test_recentactions_link(self):
  353
+        "The link from the recent actions list referring to the changeform of the object should be quoted"
  354
+        response = self.client.get('/test_admin/admin/')
  355
+        should_contain = """<a href="admin_views/modelwithstringprimarykey/%s/">%s</a>""" % (quote(self.pk), escape(self.pk))
  356
+        self.assertContains(response, should_contain)
  357
+    
  358
+    def test_deleteconfirmation_link(self):
  359
+        "The link from the delete confirmation page referring back to the changeform of the object should be quoted"
  360
+        response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/' % quote(self.pk))
  361
+        should_contain = """<a href="../../%s/">%s</a>""" % (quote(self.pk), escape(self.pk))
  362
+        self.assertContains(response, should_contain)

0 notes on commit 075a2fd

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