From 12206e3e7018bcb8bb5987f3bdfec890ac24c155 Mon Sep 17 00:00:00 2001 From: Michael Stingl Date: Wed, 20 May 2026 18:45:24 +0200 Subject: [PATCH] fix(notifications): don't re-escape email vars for each recipient escapeStringMap mutated its input map. The recipient loop in eventsNotifier.render reuses that map across iterations, so each recipient past the first got values with one extra HTML escape layer. Return a new map instead. Fixes #2804 Signed-off-by: Michael Stingl --- .../fix-notification-email-double-escape.md | 9 +++++++ services/notifications/pkg/email/email.go | 7 +++--- .../notifications/pkg/email/email_test.go | 25 +++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 changelog/unreleased/fix-notification-email-double-escape.md create mode 100644 services/notifications/pkg/email/email_test.go diff --git a/changelog/unreleased/fix-notification-email-double-escape.md b/changelog/unreleased/fix-notification-email-double-escape.md new file mode 100644 index 0000000000..ff06a645e4 --- /dev/null +++ b/changelog/unreleased/fix-notification-email-double-escape.md @@ -0,0 +1,9 @@ +Bugfix: Don't re-escape notification email vars for each recipient + +When a notification went to several recipients in one call (e.g. a group +invite to a space), each recipient past the first got subjects and bodies +with one extra layer of HTML escaping. escapeStringMap mutated its input +map, and the recipient loop reused the same map across iterations. It now +returns a new map. + +https://github.com/opencloud-eu/opencloud/issues/2804 diff --git a/services/notifications/pkg/email/email.go b/services/notifications/pkg/email/email.go index c53511c30e..b511dd6b18 100644 --- a/services/notifications/pkg/email/email.go +++ b/services/notifications/pkg/email/email.go @@ -204,8 +204,9 @@ func validateMime(incipit []byte) bool { } func escapeStringMap(vars map[string]string) map[string]string { - for k := range vars { - vars[k] = html.EscapeString(vars[k]) + escaped := make(map[string]string, len(vars)) + for k, v := range vars { + escaped[k] = html.EscapeString(v) } - return vars + return escaped } diff --git a/services/notifications/pkg/email/email_test.go b/services/notifications/pkg/email/email_test.go new file mode 100644 index 0000000000..01df4094f8 --- /dev/null +++ b/services/notifications/pkg/email/email_test.go @@ -0,0 +1,25 @@ +package email + +import "testing" + +// Regression for #2804: escapeStringMap used to mutate its input map, and the +// recipient render loop reuses that map across iterations. +func TestEscapeStringMapDoesNotMutateInput(t *testing.T) { + const raw = "Test & Demo" + input := map[string]string{"SpaceName": raw} + + first := escapeStringMap(input) + second := escapeStringMap(input) + + if got, want := first["SpaceName"], "Test & Demo"; got != want { + t.Errorf("first call: got %q, want %q", got, want) + } + if first["SpaceName"] != second["SpaceName"] { + t.Errorf("escapeStringMap not idempotent on shared input: first=%q second=%q", + first["SpaceName"], second["SpaceName"]) + } + if input["SpaceName"] != raw { + t.Errorf("escapeStringMap mutated its input: got %q, want %q", + input["SpaceName"], raw) + } +}