Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
brosner committed Jul 16, 2008
1 parent 584c41a commit 075a2fd
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 40 deletions.
3 changes: 2 additions & 1 deletion django/contrib/admin/models.py
@@ -1,6 +1,7 @@
from django.db import models from django.db import models
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.contrib.admin.util import quote
from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_lazy as _
from django.utils.encoding import smart_unicode from django.utils.encoding import smart_unicode
from django.utils.safestring import mark_safe from django.utils.safestring import mark_safe
Expand Down Expand Up @@ -50,4 +51,4 @@ def get_admin_url(self):
Returns the admin URL to edit the object represented by this log entry. Returns the admin URL to edit the object represented by this log entry.
This is relative to the Django admin index page. This is relative to the Django admin index page.
""" """
return mark_safe(u"%s/%s/%s/" % (self.content_type.app_label, self.content_type.model, self.object_id)) return mark_safe(u"%s/%s/%s/" % (self.content_type.app_label, self.content_type.model, quote(self.object_id)))
24 changes: 2 additions & 22 deletions django/contrib/admin/options.py
Expand Up @@ -5,7 +5,7 @@
from django.newforms.models import BaseInlineFormset from django.newforms.models import BaseInlineFormset
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.admin import widgets from django.contrib.admin import widgets
from django.contrib.admin.util import get_deleted_objects from django.contrib.admin.util import quote, unquote, get_deleted_objects
from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.core.exceptions import ImproperlyConfigured, PermissionDenied
from django.db import models, transaction from django.db import models, transaction
from django.http import Http404, HttpResponse, HttpResponseRedirect from django.http import Http404, HttpResponse, HttpResponseRedirect
Expand All @@ -24,26 +24,6 @@
class IncorrectLookupParameters(Exception): class IncorrectLookupParameters(Exception):
pass pass


def unquote(s):
"""
Undo the effects of quote(). Based heavily on urllib.unquote().
"""
mychr = chr
myatoi = int
list = s.split('_')
res = [list[0]]
myappend = res.append
del list[0]
for item in list:
if item[1:2]:
try:
myappend(mychr(myatoi(item[:2], 16)) + item[2:])
except ValueError:
myappend('_' + item)
else:
myappend('_' + item)
return "".join(res)

def flatten_fieldsets(fieldsets): def flatten_fieldsets(fieldsets):
"""Returns a list of field names from an admin fieldsets structure.""" """Returns a list of field names from an admin fieldsets structure."""
field_names = [] field_names = []
Expand Down Expand Up @@ -656,7 +636,7 @@ def delete_view(self, request, object_id, extra_context=None):


# Populate deleted_objects, a data structure of all related objects that # Populate deleted_objects, a data structure of all related objects that
# will also be deleted. # will also be deleted.
deleted_objects = [mark_safe(u'%s: <a href="../../%s/">%s</a>' % (escape(force_unicode(capfirst(opts.verbose_name))), force_unicode(object_id), escape(obj))), []] deleted_objects = [mark_safe(u'%s: <a href="../../%s/">%s</a>' % (escape(force_unicode(capfirst(opts.verbose_name))), quote(object_id), escape(obj))), []]
perms_needed = sets.Set() perms_needed = sets.Set()
get_deleted_objects(deleted_objects, perms_needed, request.user, obj, opts, 1, self.admin_site) get_deleted_objects(deleted_objects, perms_needed, request.user, obj, opts, 1, self.admin_site)


Expand Down
37 changes: 37 additions & 0 deletions django/contrib/admin/util.py
Expand Up @@ -6,6 +6,43 @@
from django.utils.encoding import force_unicode from django.utils.encoding import force_unicode
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _



def quote(s):
"""
Ensure that primary key values do not confuse the admin URLs by escaping
any '/', '_' and ':' characters. Similar to urllib.quote, except that the
quoting is slightly different so that it doesn't get automatically
unquoted by the Web browser.
"""
if not isinstance(s, basestring):
return s
res = list(s)
for i in range(len(res)):
c = res[i]
if c in """:/_#?;@&=+$,"<>%\\""":
res[i] = '_%02X' % ord(c)
return ''.join(res)

def unquote(s):
"""
Undo the effects of quote(). Based heavily on urllib.unquote().
"""
mychr = chr
myatoi = int
list = s.split('_')
res = [list[0]]
myappend = res.append
del list[0]
for item in list:
if item[1:2]:
try:
myappend(mychr(myatoi(item[:2], 16)) + item[2:])
except ValueError:
myappend('_' + item)
else:
myappend('_' + item)
return "".join(res)

def _nest_help(obj, depth, val): def _nest_help(obj, depth, val):
current = obj current = obj
for i in range(depth): for i in range(depth):
Expand Down
17 changes: 1 addition & 16 deletions django/contrib/admin/views/main.py
@@ -1,5 +1,6 @@
from django.contrib.admin.filterspecs import FilterSpec from django.contrib.admin.filterspecs import FilterSpec
from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.options import IncorrectLookupParameters
from django.contrib.admin.util import quote
from django.core.paginator import Paginator, InvalidPage from django.core.paginator import Paginator, InvalidPage
from django.db import models from django.db import models
from django.db.models.query import QuerySet from django.db.models.query import QuerySet
Expand Down Expand Up @@ -30,22 +31,6 @@
# Text to display within change-list table cells if the value is blank. # Text to display within change-list table cells if the value is blank.
EMPTY_CHANGELIST_VALUE = '(None)' EMPTY_CHANGELIST_VALUE = '(None)'


def quote(s):
"""
Ensure that primary key values do not confuse the admin URLs by escaping
any '/', '_' and ':' characters. Similar to urllib.quote, except that the
quoting is slightly different so that it doesn't get automatically
unquoted by the Web browser.
"""
if type(s) != type(''):
return s
res = list(s)
for i in range(len(res)):
c = res[i]
if c in ':/_':
res[i] = '_%02X' % ord(c)
return ''.join(res)

class ChangeList(object): class ChangeList(object):
def __init__(self, request, model, list_display, list_display_links, list_filter, date_hierarchy, search_fields, list_select_related, list_per_page, model_admin): def __init__(self, request, model, list_display, list_display_links, list_filter, date_hierarchy, search_fields, list_select_related, list_per_page, model_admin):
self.model = model self.model = model
Expand Down
4 changes: 4 additions & 0 deletions django/db/models/base.py
Expand Up @@ -137,6 +137,10 @@ def __new__(cls, name, bases, attrs):
return get_model(new_class._meta.app_label, name, False) return get_model(new_class._meta.app_label, name, False)


def add_to_class(cls, name, value): def add_to_class(cls, name, value):
if name == 'Admin':
import warnings
warnings.warn("The inner Admin class for %s is no longer supported. "
"Please use a ModelAdmin instead." % cls.__name__)
if hasattr(value, 'contribute_to_class'): if hasattr(value, 'contribute_to_class'):
value.contribute_to_class(cls, name) value.contribute_to_class(cls, name)
else: else:
Expand Down
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<django-objects version="1.0">
<object pk="1" model="admin_views.modelwithstringprimarykey">
<field type="CharField" name="id"><![CDATA[abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 -_.!~*'() ;/?:@&=+$, <>#%" {}|\^[]`]]></field>
</object>
</django-objects>
7 changes: 7 additions & 0 deletions tests/regressiontests/admin_views/models.py
Expand Up @@ -48,7 +48,14 @@ def changelist_view(self, request):
'extra_var': 'Hello!' 'extra_var': 'Hello!'
} }
) )

class ModelWithStringPrimaryKey(models.Model):
id = models.CharField(max_length=255, primary_key=True)

def __unicode__(self):
return self.id


admin.site.register(Article, ArticleAdmin) admin.site.register(Article, ArticleAdmin)
admin.site.register(CustomArticle, CustomArticleAdmin) admin.site.register(CustomArticle, CustomArticleAdmin)
admin.site.register(Section) admin.site.register(Section)
admin.site.register(ModelWithStringPrimaryKey)
44 changes: 43 additions & 1 deletion tests/regressiontests/admin_views/tests.py
Expand Up @@ -2,10 +2,13 @@
from django.test import TestCase from django.test import TestCase
from django.contrib.auth.models import User, Permission from django.contrib.auth.models import User, Permission
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.contrib.admin.models import LogEntry
from django.contrib.admin.sites import LOGIN_FORM_KEY, _encode_post_data from django.contrib.admin.sites import LOGIN_FORM_KEY, _encode_post_data
from django.contrib.admin.util import quote
from django.utils.html import escape


# local test models # local test models
from models import Article, CustomArticle, Section from models import Article, CustomArticle, Section, ModelWithStringPrimaryKey


def get_perm(Model, perm): def get_perm(Model, perm):
"""Return the permission object, for the Model""" """Return the permission object, for the Model"""
Expand Down Expand Up @@ -318,3 +321,42 @@ def testDeleteView(self):
self.assertRedirects(post, '/test_admin/admin/') self.assertRedirects(post, '/test_admin/admin/')
self.failUnlessEqual(Article.objects.all().count(), 0) self.failUnlessEqual(Article.objects.all().count(), 0)
self.client.get('/test_admin/admin/logout/') self.client.get('/test_admin/admin/logout/')

class AdminViewStringPrimaryKeyTest(TestCase):
fixtures = ['admin-views-users.xml', 'string-primary-key.xml']

def __init__(self, *args):
super(AdminViewStringPrimaryKeyTest, self).__init__(*args)
self.pk = """abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 -_.!~*'() ;/?:@&=+$, <>#%" {}|\^[]`"""

def setUp(self):
self.client.login(username='super', password='secret')
content_type_pk = ContentType.objects.get_for_model(ModelWithStringPrimaryKey).pk
LogEntry.objects.log_action(100, content_type_pk, self.pk, self.pk, 2, change_message='')

def tearDown(self):
self.client.logout()

def test_get_change_view(self):
"Retrieving the object using urlencoded form of primary key should work"
response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/' % quote(self.pk))
self.assertContains(response, escape(self.pk))
self.failUnlessEqual(response.status_code, 200)

def test_changelist_to_changeform_link(self):
"The link from the changelist referring to the changeform of the object should be quoted"
response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/')
should_contain = """<tr class="row1"><th><a href="%s/">%s</a></th></tr>""" % (quote(self.pk), escape(self.pk))
self.assertContains(response, should_contain)

def test_recentactions_link(self):
"The link from the recent actions list referring to the changeform of the object should be quoted"
response = self.client.get('/test_admin/admin/')
should_contain = """<a href="admin_views/modelwithstringprimarykey/%s/">%s</a>""" % (quote(self.pk), escape(self.pk))
self.assertContains(response, should_contain)

def test_deleteconfirmation_link(self):
"The link from the delete confirmation page referring back to the changeform of the object should be quoted"
response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/' % quote(self.pk))
should_contain = """<a href="../../%s/">%s</a>""" % (quote(self.pk), escape(self.pk))
self.assertContains(response, should_contain)

0 comments on commit 075a2fd

Please sign in to comment.