Fixed #11191 - Admin throws 500 instead of 404 for PK of incorrect type

Thanks to mmachine for report and test, and Chris Beaven for the patch



git-svn-id: http://code.djangoproject.com/svn/django/trunk@12011 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/admin/options.py

@@ -8,7 +8,7 @@ from django.contrib.admin import helpers
 from django.contrib.admin.util import unquote, flatten_fieldsets, get_deleted_objects, model_ngettext, model_format_dict
 from django.contrib import messages
 from django.views.decorators.csrf import csrf_protect
-from django.core.exceptions import PermissionDenied
+from django.core.exceptions import PermissionDenied, ValidationError
 from django.db import models, transaction
 from django.db.models.fields import BLANK_CHOICE_DASH
 from django.http import Http404, HttpResponse, HttpResponseRedirect
@@ -368,6 +368,20 @@ class ModelAdmin(BaseModelAdmin):
         from django.contrib.admin.views.main import ChangeList
         return ChangeList
 
+    def get_object(self, request, object_id):
+        """
+        Returns an instance matching the primary key provided. ``None``  is
+        returned if no match is found (or the object_id failed validation
+        against the primary key field).
+        """
+        queryset = self.queryset(request)
+        model = queryset.model
+        try:
+            object_id = model._meta.pk.to_python(object_id)
+            return queryset.get(pk=object_id)
+        except (model.DoesNotExist, ValidationError):
+            return None
+
     def get_changelist_form(self, request, **kwargs):
         """
         Returns a Form class for use in the Formset on the changelist page.
@@ -825,13 +839,7 @@ class ModelAdmin(BaseModelAdmin):
         model = self.model
         opts = model._meta
 
-        try:
+        obj = self.get_object(request, unquote(object_id))
-            obj = self.queryset(request).get(pk=unquote(object_id))
-        except model.DoesNotExist:
-            # Don't raise Http404 just yet, because we haven't checked
-            # permissions yet. We don't want an unauthenticated user to be able
-            # to determine whether a given object exists.
-            obj = None
 
         if not self.has_change_permission(request, obj):
             raise PermissionDenied
@@ -1036,13 +1044,7 @@ class ModelAdmin(BaseModelAdmin):
         opts = self.model._meta
         app_label = opts.app_label
 
-        try:
+        obj = self.get_object(request, unquote(object_id))
-            obj = self.queryset(request).get(pk=unquote(object_id))
-        except self.model.DoesNotExist:
-            # Don't raise Http404 just yet, because we haven't checked
-            # permissions yet. We don't want an unauthenticated user to be able
-            # to determine whether a given object exists.
-            obj = None
 
         if not self.has_delete_permission(request, obj):
             raise PermissionDenied

tests/regressiontests/admin_views/tests.py

@@ -63,11 +63,20 @@ class AdminViewBasicTest(TestCase):
 
     def testBasicEditGet(self):
         """
-        A smoke test to ensureGET on the change_view works.
+        A smoke test to ensure GET on the change_view works.
         """
         response = self.client.get('/test_admin/%s/admin_views/section/1/' % self.urlbit)
         self.failUnlessEqual(response.status_code, 200)
 
+    def testBasicEditGetStringPK(self):
+        """
+        A smoke test to ensure GET on the change_view works (returns an HTTP
+        404 error, see #11191) when passing a string as the PK argument for a
+        model with an integer PK field.
+        """
+        response = self.client.get('/test_admin/%s/admin_views/section/abc/' % self.urlbit)
+        self.failUnlessEqual(response.status_code, 404)
+
     def testBasicAddPost(self):
         """
         A smoke test to ensure POST on add_view works.