From 1babbb71696e106c6602e06d2c7c7eb6b06cd8ee Mon Sep 17 00:00:00 2001
From: Slavi Pantaleev <slavi@devture.com>
Date: Thu, 25 Aug 2022 10:49:52 +0300
Subject: [PATCH] Make Notice() not do string formatting anymore

In various places, we build messages using `Sprintf` before passing them
to `Notice()`.

If we let `Notice()` do string formatting, we run the chance of having
it try to format our already-preformatted text. If our text includes
format references (e.g. `%s`), it would cause a problem (`%s(MISSING)`).

One way to trigger it is to change the bot prefix from `!pm` to `%pm`.
Doing so, `sendHelp()` would create some help message which contains
`%pm` references. `Notice()` would then try to process them as well,
leading to a bunch of `%!p(MISSING)m` in the final text.
---
 bot/bot.go     | 4 ++--
 bot/command.go | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/bot/bot.go b/bot/bot.go
index 49121be..946d1f3 100644
--- a/bot/bot.go
+++ b/bot/bot.go
@@ -61,8 +61,8 @@ func (b *Bot) Error(ctx context.Context, roomID id.RoomID, message string, args
 }
 
 // Notice sends a notice message to the matrix room
-func (b *Bot) Notice(ctx context.Context, roomID id.RoomID, message string, args ...interface{}) {
-	content := format.RenderMarkdown(fmt.Sprintf(message, args...), true, true)
+func (b *Bot) Notice(ctx context.Context, roomID id.RoomID, message string) {
+	content := format.RenderMarkdown(message, true, true)
 	content.MsgType = event.MsgNotice
 	_, err := b.lp.Send(roomID, &content)
 	if err != nil {
diff --git a/bot/command.go b/bot/command.go
index e3b4be5..99f9fac 100644
--- a/bot/command.go
+++ b/bot/command.go
@@ -177,7 +177,7 @@ func (b *Bot) getOption(ctx context.Context, evt *event.Event, name string) {
 
 	value := cfg.Get(name)
 	if value == "" {
-		b.Notice(span.Context(), evt.RoomID, "`%s` is not set", name)
+		b.Notice(span.Context(), evt.RoomID, fmt.Sprintf("`%s` is not set", name))
 		return
 	}
 
@@ -185,7 +185,7 @@ func (b *Bot) getOption(ctx context.Context, evt *event.Event, name string) {
 		msg = msg + "@" + b.domain
 	}
 
-	b.Notice(span.Context(), evt.RoomID, msg, name, value)
+	b.Notice(span.Context(), evt.RoomID, fmt.Sprintf(msg, name, value))
 }
 
 func (b *Bot) setOption(ctx context.Context, evt *event.Event, name, value string) {
@@ -201,7 +201,7 @@ func (b *Bot) setOption(ctx context.Context, evt *event.Event, name, value strin
 	if name == optionMailbox {
 		existingID, ok := b.GetMapping(ctx, value)
 		if ok && existingID != "" && existingID != evt.RoomID {
-			b.Notice(span.Context(), evt.RoomID, "Mailbox %s@%s already taken", value, b.domain)
+			b.Notice(span.Context(), evt.RoomID, fmt.Sprintf("Mailbox `%s@%s` already taken", value, b.domain))
 			return
 		}
 	}
@@ -232,5 +232,5 @@ func (b *Bot) setOption(ctx context.Context, evt *event.Event, name, value strin
 		return
 	}
 
-	b.Notice(span.Context(), evt.RoomID, msg, name, value)
+	b.Notice(span.Context(), evt.RoomID, fmt.Sprintf(msg, name, value))
 }