From e434573ef12934d8e8f77e2e794d2968dd734ded Mon Sep 17 00:00:00 2001 From: Karen Tracey Date: Tue, 16 Mar 2010 22:37:45 +0000 Subject: [PATCH] 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 | 6 +++- .../comment_tests/fixtures/comment_tests.json | 7 ++++ tests/regressiontests/comment_tests/models.py | 4 +++ .../comment_tests/tests/comment_view_tests.py | 33 ++++++++++++++----- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/django/contrib/comments/views/comments.py b/django/contrib/comments/views/comments.py index 6609103c60..c2b553fe0d 100644 --- a/django/contrib/comments/views/comments.py +++ b/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 diff --git a/tests/regressiontests/comment_tests/fixtures/comment_tests.json b/tests/regressiontests/comment_tests/fixtures/comment_tests.json index 6731876e22..55e2161a4c 100644 --- a/tests/regressiontests/comment_tests/fixtures/comment_tests.json +++ b/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, diff --git a/tests/regressiontests/comment_tests/models.py b/tests/regressiontests/comment_tests/models.py index 62f416882c..8877ea1155 100644 --- a/tests/regressiontests/comment_tests/models.py +++ b/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) + \ No newline at end of file diff --git a/tests/regressiontests/comment_tests/tests/comment_view_tests.py b/tests/regressiontests/comment_tests/tests/comment_view_tests.py index f33a752843..bb84ab788e 100644 --- a/tests/regressiontests/comment_tests/tests/comment_view_tests.py +++ b/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\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 - post redirect, after adding some invalid data to the expected + Tests that attempting to retrieve the location specified in the + 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) +