[1.1.X] Fixed #11191 - Admin throws 500 instead of 404 for PK of inco…

…rrect type

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

Backport of r12011 from trunk



git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.1.X@12012 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/admin/options.py

@@ -6,7 +6,7 @@
 from django.contrib.admin import widgets
 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.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
@@ -347,6 +347,20 @@ def get_form(self, request, obj=None, **kwargs):
         defaults.update(kwargs)
         return modelform_factory(self.model, **defaults)
 
+    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.
@@ -795,13 +809,7 @@ def change_view(self, request, object_id, extra_context=None):
         model = self.model
         opts = model._meta
 
-        try:
-            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
+        obj = self.get_object(request, unquote(object_id))
 
         if not self.has_change_permission(request, obj):
             raise PermissionDenied
@@ -996,13 +1004,7 @@ def delete_view(self, request, object_id, extra_context=None):
         opts = self.model._meta
         app_label = opts.app_label
 
-        try:
-            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
+        obj = self.get_object(request, unquote(object_id))
 
         if not self.has_delete_permission(request, obj):
             raise PermissionDenied

tests/regressiontests/admin_views/tests.py

@@ -65,11 +65,20 @@ def testAddWithGETArgs(self):
 
     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.