diff --git a/django/contrib/admin/models.py b/django/contrib/admin/models.py index 23c8661336..259884faba 100644 --- a/django/contrib/admin/models.py +++ b/django/contrib/admin/models.py @@ -1,6 +1,7 @@ from django.db import models from django.contrib.contenttypes.models import ContentType 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.encoding import smart_unicode from django.utils.safestring import mark_safe @@ -50,4 +51,4 @@ class LogEntry(models.Model): Returns the admin URL to edit the object represented by this log entry. 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))) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 082d3b3847..3b26f7b262 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -5,7 +5,7 @@ from django.newforms.models import modelform_factory, inlineformset_factory from django.newforms.models import BaseInlineFormset from django.contrib.contenttypes.models import ContentType 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.db import models, transaction from django.http import Http404, HttpResponse, HttpResponseRedirect @@ -24,26 +24,6 @@ get_ul_class = lambda x: 'radiolist%s' % ((x == HORIZONTAL) and ' inline' or '') class IncorrectLookupParameters(Exception): 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): """Returns a list of field names from an admin fieldsets structure.""" field_names = [] @@ -656,7 +636,7 @@ class ModelAdmin(BaseModelAdmin): # Populate deleted_objects, a data structure of all related objects that # will also be deleted. - deleted_objects = [mark_safe(u'%s: %s' % (escape(force_unicode(capfirst(opts.verbose_name))), force_unicode(object_id), escape(obj))), []] + deleted_objects = [mark_safe(u'%s: %s' % (escape(force_unicode(capfirst(opts.verbose_name))), quote(object_id), escape(obj))), []] perms_needed = sets.Set() get_deleted_objects(deleted_objects, perms_needed, request.user, obj, opts, 1, self.admin_site) diff --git a/django/contrib/admin/util.py b/django/contrib/admin/util.py index ecb10e55d2..6a585b2919 100644 --- a/django/contrib/admin/util.py +++ b/django/contrib/admin/util.py @@ -6,6 +6,43 @@ from django.utils.text import capfirst from django.utils.encoding import force_unicode 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): current = obj for i in range(depth): diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 4b76fcce7b..926270cc68 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -1,5 +1,6 @@ from django.contrib.admin.filterspecs import FilterSpec from django.contrib.admin.options import IncorrectLookupParameters +from django.contrib.admin.util import quote from django.core.paginator import Paginator, InvalidPage from django.db import models from django.db.models.query import QuerySet @@ -30,22 +31,6 @@ ERROR_FLAG = 'e' # Text to display within change-list table cells if the value is blank. 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): 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 diff --git a/django/db/models/base.py b/django/db/models/base.py index 92519f7003..62b5f805c6 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -137,6 +137,10 @@ class ModelBase(type): return get_model(new_class._meta.app_label, name, False) 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'): value.contribute_to_class(cls, name) else: diff --git a/tests/regressiontests/admin_views/fixtures/string-primary-key.xml b/tests/regressiontests/admin_views/fixtures/string-primary-key.xml new file mode 100644 index 0000000000..8e1dbf047f --- /dev/null +++ b/tests/regressiontests/admin_views/fixtures/string-primary-key.xml @@ -0,0 +1,6 @@ + + + + #%" {}|\^[]`]]> + + \ No newline at end of file diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index 311989bd55..2062107397 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/tests/regressiontests/admin_views/models.py @@ -48,7 +48,14 @@ class CustomArticleAdmin(admin.ModelAdmin): '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(CustomArticle, CustomArticleAdmin) admin.site.register(Section) +admin.site.register(ModelWithStringPrimaryKey) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 1e60101d33..ad50928aa9 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -2,10 +2,13 @@ from django.test import TestCase from django.contrib.auth.models import User, Permission 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.util import quote +from django.utils.html import escape # local test models -from models import Article, CustomArticle, Section +from models import Article, CustomArticle, Section, ModelWithStringPrimaryKey def get_perm(Model, perm): """Return the permission object, for the Model""" @@ -318,3 +321,42 @@ class AdminViewPermissionsTest(TestCase): self.assertRedirects(post, '/test_admin/admin/') self.failUnlessEqual(Article.objects.all().count(), 0) 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 = """%s""" % (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 = """%s""" % (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 = """%s""" % (quote(self.pk), escape(self.pk)) + self.assertContains(response, should_contain)