Changed the comments post view code to avoid raising an exception if handed invalid data for the object pk. Thanks to Leo for the test.

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

django/contrib/comments/views/comments.py

@@ -1,7 +1,7 @@
 from django import http
 from django.conf import settings
 from utils import next_redirect, confirmation_view
-from django.core.exceptions import ObjectDoesNotExist
+from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.db import models
 from django.shortcuts import render_to_response
 from django.template import RequestContext
@@ -62,6 +62,10 @@ def post_comment(request, next=None, using=None):
         return CommentPostBadRequest(
             "No object matching content-type %r and object PK %r exists." % \
                 (escape(ctype), escape(object_pk)))
+    except (ValueError, ValidationError), e:
+        return CommentPostBadRequest(
+            "Attempting go get content-type %r and object PK %r exists raised %s" % \
+                (escape(ctype), escape(object_pk), e.__class__.__name__))
 
     # Do we want to preview the comment?
     preview = "preview" in data

tests/regressiontests/comment_tests/fixtures/comment_tests.json

@@ -1,4 +1,11 @@
 [
+  {
+    "model" : "comment_tests.book",
+    "pk" : 1,
+    "fields" : {
+        "dewey_decimal" : "12.34"
+    }
+  },
   {
     "model" : "comment_tests.author",
     "pk" : 1,

tests/regressiontests/comment_tests/models.py

@@ -28,3 +28,7 @@ class Entry(models.Model):
 
     def __str__(self):
         return self.title
+
+class Book(models.Model):
+    dewey_decimal = models.DecimalField(primary_key = True, decimal_places=2, max_digits=5)
+    

tests/regressiontests/comment_tests/tests/comment_view_tests.py

@@ -3,7 +3,7 @@ from django.conf import settings
 from django.contrib.auth.models import User
 from django.contrib.comments import signals
 from django.contrib.comments.models import Comment
-from regressiontests.comment_tests.models import Article
+from regressiontests.comment_tests.models import Article, Book
 from regressiontests.comment_tests.tests import CommentTestCase
 
 post_redirect_re = re.compile(r'^http://testserver/posted/\?c=(?P<pk>\d+$)')
@@ -45,6 +45,22 @@ class CommentViewTests(CommentTestCase):
         response = self.client.post("/post/", data)
         self.assertEqual(response.status_code, 400)
 
+    def testPostInvalidIntegerPK(self):
+        a = Article.objects.get(pk=1)
+        data = self.getValidData(a)
+        data["comment"] = "This is another comment"
+        data["object_pk"] = u'\ufffd'
+        response = self.client.post("/post/", data)
+        self.assertEqual(response.status_code, 400)
+
+    def testPostInvalidDecimalPK(self):
+        b = Book.objects.get(pk='12.34')
+        data = self.getValidData(b)
+        data["comment"] = "This is another comment"
+        data["object_pk"] = 'cookies'
+        response = self.client.post("/post/", data)
+        self.assertEqual(response.status_code, 400)
+
     def testCommentPreview(self):
         a = Article.objects.get(pk=1)
         data = self.getValidData(a)
@@ -187,11 +203,11 @@ class CommentViewTests(CommentTestCase):
         location = response["Location"]
         match = post_redirect_re.match(location)
         self.failUnless(match != None, "Unexpected redirect location: %s" % location)
-        
+
         data["next"] = "/somewhere/else/"
         data["comment"] = "This is another comment"
         response = self.client.post("/post/", data)
-        location = response["Location"]        
+        location = response["Location"]
         match = re.search(r"^http://testserver/somewhere/else/\?c=\d+$", location)
         self.failUnless(match != None, "Unexpected redirect location: %s" % location)
 
@@ -199,7 +215,7 @@ class CommentViewTests(CommentTestCase):
         a = Article.objects.get(pk=1)
         data = self.getValidData(a)
         response = self.client.post("/post/", data)
-        location = response["Location"]        
+        location = response["Location"]
         match = post_redirect_re.match(location)
         self.failUnless(match != None, "Unexpected redirect location: %s" % location)
         pk = int(match.group('pk'))
@@ -216,14 +232,14 @@ class CommentViewTests(CommentTestCase):
         data["next"] = "/somewhere/else/?foo=bar"
         data["comment"] = "This is another comment"
         response = self.client.post("/post/", data)
-        location = response["Location"]        
+        location = response["Location"]
         match = re.search(r"^http://testserver/somewhere/else/\?foo=bar&c=\d+$", location)
         self.failUnless(match != None, "Unexpected redirect location: %s" % location)
 
-    def testCommentDoneReSubmitWithInvalidParams(self):
+    def testCommentPostRedirectWithInvalidIntegerPK(self):
         """
-        Tests that attempting to retrieve the location specified in the 
+        Tests that attempting to retrieve the location specified in the
-        post redirect, after adding some invalid data to the expected 
+        post redirect, after adding some invalid data to the expected
         querystring it ends with, doesn't cause a server error.
         """
         a = Article.objects.get(pk=1)
@@ -234,3 +250,4 @@ class CommentViewTests(CommentTestCase):
         broken_location = location + u"\ufffd"
         response = self.client.get(broken_location)
         self.assertEqual(response.status_code, 200)
+