From badc1eb30bae6ce7cc7083490d26bcaa12e31805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Fri, 29 Mar 2024 14:55:15 +0100 Subject: [PATCH 1/6] Do not unescape HTML in Slack messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This resulted in broken matrix formatting. Tests are now also updated, in line with existing and expected behaviour. Signed-off-by: Tadeusz „tadzik” Sośnierz --- src/substitutions.ts | 5 ---- tests/unit/substitutionsTest.ts | 48 +++++++++++++-------------------- 2 files changed, 18 insertions(+), 35 deletions(-) diff --git a/src/substitutions.ts b/src/substitutions.ts index 069826e1..dd2bdea2 100644 --- a/src/substitutions.ts +++ b/src/substitutions.ts @@ -14,14 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Logger } from "matrix-appservice-bridge"; import * as emoji from "node-emoji"; import { Main } from "./Main"; import { ISlackFile } from "./BaseSlackHandler"; import escapeStringRegexp from "escape-string-regexp"; -const log = new Logger("substitutions"); - const ATTACHMENT_TYPES = ["m.audio", "m.video", "m.file", "m.image"]; const PILL_REGEX = /([^<]+)<\/a>/g; @@ -59,8 +56,6 @@ class Substitutions { * @param file options slack file object */ public slackToMatrix(body: string, file?: ISlackFile): string { - log.debug("running substitutions on ", body); - body = this.htmlUnescape(body); body = body.replace("", "@room"); body = body.replace("", "@room"); body = body.replace("", "@room"); diff --git a/tests/unit/substitutionsTest.ts b/tests/unit/substitutionsTest.ts index d52688fe..efefe053 100644 --- a/tests/unit/substitutionsTest.ts +++ b/tests/unit/substitutionsTest.ts @@ -297,34 +297,22 @@ describe("Substitutions", () => { }); }); - // describe("slackTextToMatrixHTML", () => { - // it("should repeat a plain string", async () => { - // const res = await substitutions.slackTextToMatrixHTML("Hello World!"); - // expect(res).to.equal("Hello World!"); - // }); - // it("should convert < and >", async () => { - // const res = await substitutions.slackTextToMatrixHTML("Hello"); - // expect(res).to.equal("<html>Hello</html>"); - // }); - // it("should convert a single new line to a
", async () => { - // const res = substitutions.slackTextToMatrixHTML("line 1\nline 2"); - // expect(res).to.equal("line 1
line 2"); - // }); - // it("should convert two new lines to paragraphs", async () => { - // const res = substitutions.slackTextToMatrixHTML("line 1\n\nline 3"); - // expect(res).to.equal("

line 1

line 3

"); - // }); - // it("should convert bold formatting", async () => { - // const res = substitutions.slackTextToMatrixHTML("This is *bold*!"); - // expect(res).to.equal("This is bold!"); - // }); - // it("should convert italic formatting", async () => { - // const res = substitutions.slackTextToMatrixHTML("This is /italics/!"); - // expect(res).to.equal("This is italics!"); - // }); - // it("should convert strikethrough formatting", async () => { - // const res = substitutions.slackTextToMatrixHTML("This is ~strikethrough~!"); - // expect(res).to.equal("This is strikethrough"); - // }); - // }); + describe("slackTextToMatrixHTML", () => { + it("should repeat a plain string", () => { + const res = substitutions.slackToMatrix("Hello World!"); + expect(res).to.equal("Hello World!"); + }); + it("should leave newlines intact", () => { + const res = substitutions.slackToMatrix("Hello\nWorld!"); + expect(res).to.equal("Hello\nWorld!"); + }); + it("should leave html encoding intact", () => { + const res = substitutions.slackToMatrix(""); + expect(res).to.equal(""); + }); + it("should convert !here", () => { + const res = substitutions.slackToMatrix(" hello"); + expect(res).to.equal("@room hello"); + }); + }); }); From 1921b50303bac6945ce2bbc04a2e1144055ab1ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Fri, 29 Mar 2024 15:17:09 +0100 Subject: [PATCH 2/6] Ensure we're not accidentally removing line breaks when stripping reply fallbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes GH-737 Signed-off-by: Tadeusz „tadzik” Sośnierz --- src/BridgedRoom.ts | 48 +++++---------------------------- src/substitutions.ts | 37 +++++++++++++++++++++++++ tests/unit/substitutionsTest.ts | 8 ++++++ 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/src/BridgedRoom.ts b/src/BridgedRoom.ts index 126d6815..eeaffad5 100644 --- a/src/BridgedRoom.ts +++ b/src/BridgedRoom.ts @@ -403,7 +403,7 @@ export class BridgedRoom { // re-write the message so the matrixToSlack converter works as expected. let newMessage = JSON.parse(JSON.stringify(message)); newMessage.content = message.content["m.new_content"]; - newMessage = await this.stripMatrixReplyFallback(newMessage); + newMessage = substitutions.stripMatrixReplyFallback(newMessage); const body = await substitutions.matrixToSlack(newMessage, this.main, this.SlackTeamId!); @@ -443,8 +443,11 @@ export class BridgedRoom { if (!this.slackWebhookUri && !this.botClient && !puppetedClient) { return false; } const slackClient = puppetedClient || this.botClient; const user = this.main.getOrCreateMatrixUser(message.sender); - message = await this.stripMatrixReplyFallback(message); + + message = substitutions.stripMatrixReplyFallback(message); + const matrixToSlackResult = await substitutions.matrixToSlack(message, this.main, this.SlackTeamId!); + if (!matrixToSlackResult) { // Could not handle content, dropped. log.warn(`Dropped ${message.event_id}, message content could not be identified`); @@ -994,7 +997,7 @@ export class BridgedRoom { if (message.thread_ts !== undefined && message.text) { let replyMEvent = await this.getReplyEvent(this.MatrixRoomId, message, this.SlackChannelId!); if (replyMEvent) { - replyMEvent = await this.stripMatrixReplyFallback(replyMEvent); + replyMEvent = substitutions.stripMatrixReplyFallback(replyMEvent); return await ghost.sendInThread( this.MatrixRoomId, message.text, this.SlackChannelId!, eventTS, replyMEvent, ); @@ -1048,7 +1051,7 @@ export class BridgedRoom { let replyEvent = await this.getReplyEvent( this.MatrixRoomId, message.message as unknown as ISlackMessageEvent, this.slackChannelId!, ); - replyEvent = await this.stripMatrixReplyFallback(replyEvent); + replyEvent = substitutions.stripMatrixReplyFallback(replyEvent); if (replyEvent) { const bodyFallback = ghost.getFallbackText(replyEvent); const formattedFallback = ghost.getFallbackHtml(this.MatrixRoomId, replyEvent); @@ -1152,43 +1155,6 @@ export class BridgedRoom { return intent.getEvent(roomID, replyToEvent.eventId); } - /* - Strip out reply fallbacks. Borrowed from - https://github.com/turt2live/matrix-js-bot-sdk/blob/master/src/preprocessors/RichRepliesPreprocessor.ts - */ - private async stripMatrixReplyFallback(event: any): Promise { - if (!event.content?.body) { - return event; - } - - let realHtml = event.content.formatted_body; - let realText = event.content.body || ""; - - if (event.content.format === "org.matrix.custom.html" && realHtml) { - const formattedBody = realHtml; - if (formattedBody.startsWith("") && formattedBody.indexOf("") !== -1) { - const parts = formattedBody.split(""); - realHtml = parts[1]; - event.content.formatted_body = realHtml.trim(); - } - } - - let processedFallback = false; - for (const line of realText.split("\n")) { - if (line.startsWith("> ") && !processedFallback) { - continue; - } else if (!processedFallback) { - realText = line; - processedFallback = true; - } else { - realText += line + "\n"; - } - } - - event.content.body = realText.trim(); - return event; - } - /* Given an event which is in reply to something else return the event ID of the top most event in the reply chain, i.e. the one without a relates to. diff --git a/src/substitutions.ts b/src/substitutions.ts index dd2bdea2..7222c1c5 100644 --- a/src/substitutions.ts +++ b/src/substitutions.ts @@ -312,6 +312,43 @@ class Substitutions { return `${file.url_private}?pub_secret=${pubSecret[1]}`; } } + + /* + Strip out reply fallbacks. Borrowed from + https://github.com/turt2live/matrix-js-bot-sdk/blob/master/src/preprocessors/RichRepliesPreprocessor.ts + */ + public stripMatrixReplyFallback(event: T & any): T { + if (!event.content?.body) { + return event; + } + + let realHtml = event.content.formatted_body; + let realText = event.content.body || ""; + + if (event.content.format === "org.matrix.custom.html" && realHtml) { + const formattedBody = realHtml; + if (formattedBody.startsWith("") && formattedBody.indexOf("") !== -1) { + const parts = formattedBody.split(""); + realHtml = parts[1]; + event.content.formatted_body = realHtml.trim(); + } + } + + let processedFallback = false; + for (const line of realText.split("\n")) { + if (line.startsWith("> ") && !processedFallback) { + continue; + } else if (!processedFallback) { + realText = line + "\n"; + processedFallback = true; + } else { + realText += line + "\n"; + } + } + + event.content.body = realText.trim(); + return event; + } } const substitutions = new Substitutions(); diff --git a/tests/unit/substitutionsTest.ts b/tests/unit/substitutionsTest.ts index efefe053..b1719513 100644 --- a/tests/unit/substitutionsTest.ts +++ b/tests/unit/substitutionsTest.ts @@ -315,4 +315,12 @@ describe("Substitutions", () => { expect(res).to.equal("@room hello"); }); }); + + describe("stripMatrixReplyFallback", () => { + it("should leave newlines intact in messages", () => { + let message = { content: { body: "foo\nbar" } }; + message = substitutions.stripMatrixReplyFallback(message); + expect(message.content.body).to.equal("foo\nbar"); + }); + }); }); From 6db082fa0bcd4b6aa784e2d1687bf89e71ba3559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Thu, 4 Apr 2024 16:01:44 +0200 Subject: [PATCH 3/6] Ensure we're not breaking code blocks M->S --- tests/unit/substitutionsTest.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/unit/substitutionsTest.ts b/tests/unit/substitutionsTest.ts index b1719513..da0fd79a 100644 --- a/tests/unit/substitutionsTest.ts +++ b/tests/unit/substitutionsTest.ts @@ -322,5 +322,10 @@ describe("Substitutions", () => { message = substitutions.stripMatrixReplyFallback(message); expect(message.content.body).to.equal("foo\nbar"); }); + it("should not break code block contents", () => { + let message = { content: { body: "```\nfoo\n```" } }; + message = substitutions.stripMatrixReplyFallback(message); + expect(message.content.body).to.equal("```\nfoo\n```"); + }); }); }); From 5ce9842d19df196269d79358e04bad031cdfd850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Thu, 4 Apr 2024 16:19:00 +0200 Subject: [PATCH 4/6] Correctly convert newlines when handling rich Slack messages Fixes GH-547 --- src/SlackGhost.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SlackGhost.ts b/src/SlackGhost.ts index 9fd574e2..98f28f11 100644 --- a/src/SlackGhost.ts +++ b/src/SlackGhost.ts @@ -318,7 +318,7 @@ export class SlackGhost { } public prepareFormattedBody(body: string): string { - return Slackdown.parse(body); + return Slackdown.parse(body).replace("\n", "
"); } public async sendInThread(roomId: string, text: string, slackRoomId: string, @@ -361,7 +361,7 @@ export class SlackGhost { const content = { body, format: "org.matrix.custom.html", - formatted_body: Slackdown.parse(text), + formatted_body: this.prepareFormattedBody(text), msgtype: "m.text", ...extra, }; From c3c6bf5737acab602d9b751106774a5d831807df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= Date: Fri, 5 Apr 2024 11:51:52 +0200 Subject: [PATCH 5/6] Add changelog --- changelog.d/781.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/781.bugfix diff --git a/changelog.d/781.bugfix b/changelog.d/781.bugfix new file mode 100644 index 00000000..c88c2069 --- /dev/null +++ b/changelog.d/781.bugfix @@ -0,0 +1 @@ +Fix various message formatting issues. From 4c80e0cd532f2fddac22bdbe97b84696270117c6 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 16 Apr 2024 12:33:18 +0100 Subject: [PATCH 6/6] Update tests/unit/substitutionsTest.ts Co-authored-by: David Vo --- tests/unit/substitutionsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/substitutionsTest.ts b/tests/unit/substitutionsTest.ts index da0fd79a..d348792e 100644 --- a/tests/unit/substitutionsTest.ts +++ b/tests/unit/substitutionsTest.ts @@ -297,7 +297,7 @@ describe("Substitutions", () => { }); }); - describe("slackTextToMatrixHTML", () => { + describe("slackToMatrix", () => { it("should repeat a plain string", () => { const res = substitutions.slackToMatrix("Hello World!"); expect(res).to.equal("Hello World!");