diff --git a/CHANGES.rst b/CHANGES.rst index b8a5e4c39..7ec91758e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -21,6 +21,7 @@ Changelog for Isso - Move ``isso-dev.cfg`` to ``contrib/`` folder - Run automated screenshot comparisons for testing (`#889`_) - wsgi: Return HTTP errors as JSON if client prefers it (`#488`_, sundbry) +- Verify that parent of new comment is in same thread (`#885`_) - Add ``data-isso-page-author-hashes`` option to client which makes it possible to style comments and replies made by the page's author(s). - Add Ukrainian localisation (`#878`_, okawo80085) @@ -35,6 +36,7 @@ Changelog for Isso .. _879: https://github.com/posativ/isso/pull/879 .. _488: https://github.com/posativ/isso/pull/488 .. _#889: https://github.com/posativ/isso/pull/889 +.. _#885: https://github.com/posativ/isso/pull/885 0.12.6 (2022-03-06) ------------------- diff --git a/isso/db/comments.py b/isso/db/comments.py index f2d06c1e8..603799243 100644 --- a/isso/db/comments.py +++ b/isso/db/comments.py @@ -52,10 +52,27 @@ def add(self, uri, c): database values. """ + # Comment parent must be valid and be of same thread + # If parent itself has a valid parent, use that + def _find(uri, parent): + if parent in ["0", 0, None]: + return None + obj = self.get(parent) + if obj is None: # parent does not exist + return None + rv = self.db.execute([ + 'SELECT CASE WHEN EXISTS(', + ' SELECT comments.id FROM comments INNER JOIN threads', + ' ON comments.tid=threads.id WHERE threads.uri=?', + ' AND comments.id=?)', + ' THEN 1 ELSE 0 END;'], + (uri, parent)).fetchone() + if rv[0] == 0: # parent is not in current thread + return None + return _find(uri, obj.get("parent")) or parent + if c.get("parent") is not None: - ref = self.get(c["parent"]) - if ref.get("parent") is not None: - c["parent"] = ref["parent"] + c["parent"] = _find(uri, c["parent"]) self.db.execute([ 'INSERT INTO comments (', @@ -339,7 +356,7 @@ def count(self, *urls): def purge(self, delta): """ - Remove comments older than :param:`delta`. + Remove stale pending comments older than :param:`delta`. """ self.db.execute([ 'DELETE FROM comments WHERE mode = 2 AND ? - created > ?;' diff --git a/isso/tests/test_comments.py b/isso/tests/test_comments.py index 52e837e9a..c0c48d386 100644 --- a/isso/tests/test_comments.py +++ b/isso/tests/test_comments.py @@ -111,6 +111,34 @@ def testCreateInvalidParent(self): self.assertEqual(loads(invalid.data)["parent"], 1) + def testCreateInvalidThreadForParent(self): + + self.post('/new?uri=one', data=json.dumps({'text': '...'})) + # Parent which is not in same thread should be rejected, set to None + invalid = self.post( + '/new?uri=two', data=json.dumps({'text': '...', 'parent': 1})) + # Replies to commments in thread "two" are valid + valid = self.post( + '/new?uri=two', data=json.dumps({'text': '...', 'parent': 2})) + + self.assertEqual(loads(invalid.data)["parent"], None) + self.assertEqual(loads(valid.data)["parent"], 2) + + # Insert (invalid) comment into thread "two" with parent from thread 1 + self.app.db.execute([ + 'INSERT INTO COMMENTS (tid, parent, created, modified, mode,' + ' remote_addr, text, author, email, website, voters, notification)', + 'SELECT threads.id, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?', + 'FROM threads where threads.uri = ?;'], + (None, 0.0, 0.0, 1, None, 'Text', None, None, None, bytes(1), None, 'two') + ) + # For id=4, the parent has id=1, but is from thread "one". Because id=1 + # does not belong to the current thread "two", it is rejected and id=4 + # chosen instead. + impossible = self.post( + '/new?uri=two', data=json.dumps({'text': '...', 'parent': 4})) + self.assertEqual(loads(impossible.data)["parent"], 4) + def testVerifyFields(self): def verify(comment):