[server] db: comments: Constrain parent of new comments

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)
 -------------------

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 > ?;'

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):