From e112f1cd40888d2470fbeaa528a66d6f53376ec0 Mon Sep 17 00:00:00 2001
From: Simon Backx <simon@ghost.org>
Date: Tue, 2 Aug 2022 17:44:00 +0200
Subject: [PATCH] Added empty line trimming to comment messages

fixes https://github.com/TryGhost/Team/issues/1737

- Empty lines at start
- Empty lines at end
- Duplicate empty lines inside the comment message (max one allowed)
---
 ghost/core/core/server/models/comment.js      | 48 +++++++++++++------
 .../core/server/services/comments/emails.js   |  2 +-
 ghost/core/core/shared/html-to-plaintext.js   | 18 +++++++
 .../__snapshots__/comments.test.js.snap       | 20 ++++----
 .../e2e-api/members-comments/comments.test.js |  6 +--
 5 files changed, 66 insertions(+), 28 deletions(-)

diff --git a/ghost/core/core/server/models/comment.js b/ghost/core/core/server/models/comment.js
index 995eb659cb..64a0583cd5 100644
--- a/ghost/core/core/server/models/comment.js
+++ b/ghost/core/core/server/models/comment.js
@@ -9,6 +9,24 @@ const messages = {
     notYourCommentToDestroy: 'You may only delete your own comments'
 };
 
+function escapeRegex(string) {
+    return string.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&');
+}
+
+/**
+ * Remove empty paragraps from the start and end
+ * + remove duplicate empty paragrapsh (only one empty line allowed)
+ */
+function trimParagraphs(str) {
+    const paragraph = '<p></p>';
+    const escapedParagraph = escapeRegex(paragraph);
+
+    const startReg = new RegExp('^(' + escapedParagraph + ')+');
+    const endReg = new RegExp('(' + escapedParagraph + ')+$');
+    const duplicates = new RegExp('(' + escapedParagraph + ')+');
+    return str.replace(startReg, '').replace(endReg, '').replace(duplicates, paragraph);
+}
+
 const Comment = ghostBookshelf.Model.extend({
     tableName: 'comments',
 
@@ -49,20 +67,22 @@ const Comment = ghostBookshelf.Model.extend({
         if (this.hasChanged('html')) {
             const sanitizeHtml = require('sanitize-html');
 
-            this.set('html', sanitizeHtml(this.get('html'), {
-                allowedTags: ['p', 'br', 'a', 'blockquote'],
-                allowedAttributes: {
-                    a: ['href', 'target', 'rel']
-                },
-                selfClosing: ['br'],
-                // Enforce _blank and safe URLs
-                transformTags: {
-                    a: sanitizeHtml.simpleTransform('a', {
-                        target: '_blank', 
-                        rel: 'ugc noopener noreferrer nofollow'
-                    })
-                }
-            }));
+            this.set('html', trimParagraphs(
+                sanitizeHtml(this.get('html'), {
+                    allowedTags: ['p', 'br', 'a', 'blockquote'],
+                    allowedAttributes: {
+                        a: ['href', 'target', 'rel']
+                    },
+                    selfClosing: ['br'],
+                    // Enforce _blank and safe URLs
+                    transformTags: {
+                        a: sanitizeHtml.simpleTransform('a', {
+                            target: '_blank', 
+                            rel: 'ugc noopener noreferrer nofollow'
+                        })
+                    }
+                })
+            ));
         }
     },
 
diff --git a/ghost/core/core/server/services/comments/emails.js b/ghost/core/core/server/services/comments/emails.js
index d71118934c..5520de78b8 100644
--- a/ghost/core/core/server/services/comments/emails.js
+++ b/ghost/core/core/server/services/comments/emails.js
@@ -129,7 +129,7 @@ class CommentsServiceEmails {
             postTitle: post.get('title'),
             postUrl: this.urlService.getUrlByResourceId(post.get('id'), {absolute: true}),
             commentHtml: comment.get('html'),
-            commentText: htmlToPlaintext.email(comment.get('html')),
+            commentText: htmlToPlaintext.comment(comment.get('html')),
             commentDate: moment(comment.get('created_at')).tz(this.settingsCache.get('timezone')).format('D MMM YYYY'),
             
             reporterName: reporter.name,
diff --git a/ghost/core/core/shared/html-to-plaintext.js b/ghost/core/core/shared/html-to-plaintext.js
index 45ff86622b..f7b22b2cc6 100644
--- a/ghost/core/core/shared/html-to-plaintext.js
+++ b/ghost/core/core/shared/html-to-plaintext.js
@@ -34,6 +34,7 @@ const baseSettings = {
 
 let excerptConverter;
 let emailConverter;
+let commentConverter;
 
 const loadConverters = () => {
     if (excerptConverter && emailConverter) {
@@ -63,8 +64,19 @@ const loadConverters = () => {
         ]
     });
 
+    const commentSettings = mergeSettings({
+        preserveNewlines: false,
+        selectors: [
+            // equiv hideLinkHrefIfSameAsText: true
+            {selector: 'a', options: {hideLinkHrefIfSameAsText: true}},
+            // No space between <p> tags. An empty <p> is needed
+            {selector: 'p', options: {leadingLineBreaks: 1, trailingLineBreaks: 1}}
+        ]
+    });
+
     excerptConverter = compile(excerptSettings);
     emailConverter = compile(emailSettings);
+    commentConverter = compile(commentSettings);
 };
 
 module.exports.excerpt = (html) => {
@@ -78,3 +90,9 @@ module.exports.email = (html) => {
 
     return emailConverter(html);
 };
+
+module.exports.comment = (html) => {
+    loadConverters();
+
+    return commentConverter(html);
+};
diff --git a/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap b/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap
index a9360a0030..9abca9d7f0 100644
--- a/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap
+++ b/ghost/core/test/e2e-api/members-comments/__snapshots__/comments.test.js.snap
@@ -40,7 +40,7 @@ Object {
     Object {
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "edited_at": null,
-      "html": "<p>This is a message</p><p>New line</p>",
+      "html": "<p>This is a message</p><p></p><p>New line</p>",
       "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
       "liked": Any<Boolean>,
       "likes_count": Any<Number>,
@@ -72,7 +72,7 @@ exports[`Comments API when authenticated Can browse all comments of a post 2: [h
 Object {
   "access-control-allow-origin": "*",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "1057",
+  "content-length": "1064",
   "content-type": "application/json; charset=utf-8",
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
   "vary": "Accept-Encoding",
@@ -86,7 +86,7 @@ Object {
     Object {
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "edited_at": null,
-      "html": "<p>This is a message</p><p>New line</p>",
+      "html": "<p>This is a message</p><p></p><p>New line</p>",
       "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
       "liked": false,
       "likes_count": 0,
@@ -101,7 +101,7 @@ exports[`Comments API when authenticated Can comment on a post 2: [headers] 1`]
 Object {
   "access-control-allow-origin": "*",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "219",
+  "content-length": "226",
   "content-type": "application/json; charset=utf-8",
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
   "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/comments\\\\/\\[a-f0-9\\]\\{24\\}\\\\//,
@@ -169,7 +169,7 @@ Object {
     Object {
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "edited_at": null,
-      "html": "<p>This is a message</p><p>New line</p>",
+      "html": "<p>This is a message</p><p></p><p>New line</p>",
       "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
       "liked": Any<Boolean>,
       "likes_count": Any<Number>,
@@ -239,7 +239,7 @@ exports[`Comments API when authenticated Can like a comment 2: [headers] 1`] = `
 Object {
   "access-control-allow-origin": "*",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "648",
+  "content-length": "655",
   "content-type": "application/json; charset=utf-8",
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
   "vary": "Accept-Encoding",
@@ -262,7 +262,7 @@ Object {
     Object {
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "edited_at": null,
-      "html": "<p>This is a message</p><p>New line</p>",
+      "html": "<p>This is a message</p><p></p><p>New line</p>",
       "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
       "liked": Any<Boolean>,
       "likes_count": Any<Number>,
@@ -301,7 +301,7 @@ exports[`Comments API when authenticated Can like a comment 5: [headers] 1`] = `
 Object {
   "access-control-allow-origin": "*",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "647",
+  "content-length": "654",
   "content-type": "application/json; charset=utf-8",
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
   "vary": "Accept-Encoding",
@@ -473,7 +473,7 @@ Object {
     Object {
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "edited_at": null,
-      "html": "<p>This is a message</p><p>New line</p>",
+      "html": "<p>This is a message</p><p></p><p>New line</p>",
       "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
       "liked": Any<Boolean>,
       "likes_count": Any<Number>,
@@ -512,7 +512,7 @@ exports[`Comments API when authenticated Can remove a like 3: [headers] 1`] = `
 Object {
   "access-control-allow-origin": "*",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "648",
+  "content-length": "655",
   "content-type": "application/json; charset=utf-8",
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
   "vary": "Accept-Encoding",
diff --git a/ghost/core/test/e2e-api/members-comments/comments.test.js b/ghost/core/test/e2e-api/members-comments/comments.test.js
index 159a16265e..9910576ba1 100644
--- a/ghost/core/test/e2e-api/members-comments/comments.test.js
+++ b/ghost/core/test/e2e-api/members-comments/comments.test.js
@@ -165,7 +165,7 @@ describe('Comments API', function () {
                 .post(`/api/comments/`)
                 .body({comments: [{
                     post_id: postId,
-                    html: '<p>This is a <strong>message</strong></p><p>New line</p>'
+                    html: '<div></div><p></p><p>This is a <strong>message</strong></p><p></p><p></p><p>New line</p><p></p>'
                 }]})
                 .expectStatus(201)
                 .matchHeaderSnapshot({
@@ -184,7 +184,7 @@ describe('Comments API', function () {
                 subject: '💬 New comment on your post: ' + postTitle,
                 to: fixtureManager.get('users', 0).email,
                 // Note that the <strong> tag is removed by the sanitizer
-                html: new RegExp(escapeRegExp('<p>This is a message</p><p>New line</p>'))
+                html: new RegExp(escapeRegExp('<p>This is a message</p><p></p><p>New line</p>'))
             });
 
             // Wait for the dispatched events (because this happens async)
@@ -391,7 +391,7 @@ describe('Comments API', function () {
             mockManager.assert.sentEmail({
                 subject: '🚩 A comment has been reported on your post',
                 to: fixtureManager.get('users', 0).email,
-                html: new RegExp(escapeRegExp('<p>This is a message</p><p>New line</p>')),
+                html: new RegExp(escapeRegExp('<p>This is a message</p><p></p><p>New line</p>')),
                 text: new RegExp(escapeRegExp('This is a message\n\nNew line'))
             });
         });