From 6bd31ad490a1be394770f4d655e5f34818bc1f55 Mon Sep 17 00:00:00 2001 From: Kees Hink Date: Wed, 14 Nov 2012 13:25:10 +0100 Subject: [PATCH] Make conversation view not break when comment-id cannot be converted to long, fixes #13327 --- CHANGES.txt | 4 ++ plone/app/discussion/conversation.py | 6 +- .../tests/functional_test_comment_url.txt | 70 +++++++++++++++++++ plone/app/discussion/tests/test_functional.py | 3 +- 4 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 plone/app/discussion/tests/functional_test_comment_url.txt diff --git a/CHANGES.txt b/CHANGES.txt index e77e101..c83a9d5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -4,6 +4,10 @@ Changelog 2.2.1 (unreleased) ------------------ +- Make conversation view not break when comment-id cannot be converted to long. + Fixes #13327 + [khink] + - fix insufficient privileges when trying to view the RSS feed of a comment collection [maartenkling] diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py index 2d870fe..ec2520c 100644 --- a/plone/app/discussion/conversation.py +++ b/plone/app/discussion/conversation.py @@ -210,7 +210,11 @@ class Conversation(Traversable, Persistent, Explicit): def __getitem__(self, key): """Get an item by its long key """ - return self._comments[long(key)].__of__(self) + try: + comment_id = long(key) + except ValueError: + return + return self._comments[comment_id].__of__(self) def __delitem__(self, key, suppress_container_modified=False): """Delete an item by its long key diff --git a/plone/app/discussion/tests/functional_test_comment_url.txt b/plone/app/discussion/tests/functional_test_comment_url.txt new file mode 100644 index 0000000..b2fde39 --- /dev/null +++ b/plone/app/discussion/tests/functional_test_comment_url.txt @@ -0,0 +1,70 @@ +=================================== + Dealing with faulty comment links +=================================== + +Make sure that calling specially crafted URLs doesn't break the conversation +view. + +See also https://dev.plone.org/ticket/13327 + + +Setting up and logging in +========================= + +First we have to set up some things and login. + + >>> app = layer['app'] + >>> from plone.testing.z2 import Browser + >>> browser = Browser(app) + >>> browser.addHeader('Authorization', 'Basic admin:secret') + >>> portal = layer['portal'] + >>> portal_url = 'http://nohost/plone' + +As we're expecting to see 404s, the test should not break on HTTP errors. + + >>> browser.raiseHttpErrors = False + +Enable commenting. + + >>> from zope.component import queryUtility + >>> from plone.registry.interfaces import IRegistry + >>> from plone.app.discussion.interfaces import IDiscussionSettings + >>> registry = queryUtility(IRegistry) + >>> settings = registry.forInterface(IDiscussionSettings) + >>> settings.globally_enabled = True + +Create a public page with comments allowed. + + >>> browser.open(portal_url) + >>> browser.getLink(id='document').click() + >>> browser.getControl(name='title').value = "Doc1" + >>> browser.getControl(name='allowDiscussion:boolean').value = True + >>> browser.getControl(name='form.button.save').click() + >>> urldoc1 = browser.url + +Check that the form has been properly submitted + + >>> browser.url + 'http://nohost/plone/doc1' + + +Checking invalid comment links +============================== + +URL has invalid comment id +-------------------------- + +Test a URL with a comment id that cannot be converted to long integer. + + >>> url_invalid_comment_id = "%s/++conversation++default/ThisCantBeRight" % urldoc1 + >>> browser.open(url_invalid_comment_id) + +We should not get an error, + + >>> "500 Internal Server Error" in str(browser.headers) + False + +but we should get a 404: + + >>> "404 Not Found" in str(browser.headers) + True diff --git a/plone/app/discussion/tests/test_functional.py b/plone/app/discussion/tests/test_functional.py index 25730b0..f269ec3 100644 --- a/plone/app/discussion/tests/test_functional.py +++ b/plone/app/discussion/tests/test_functional.py @@ -20,7 +20,8 @@ optionflags = ( doctest.REPORT_ONLY_FIRST_FAILURE) normal_testfiles = [ 'functional_test_comments.txt', - 'functional_test_comment_review_workflow.txt' + 'functional_test_comment_review_workflow.txt', + 'functional_test_comment_url.txt', ]