diff --git a/notify/util.go b/notify/util.go index fe4c9ea508..d18258fbda 100644 --- a/notify/util.go +++ b/notify/util.go @@ -135,8 +135,11 @@ func TruncateInBytes(s string, n int) (string, bool) { r := []rune(s) truncationTarget := n - 3 - // Next, let's truncate the runes to the lower possible number. - truncatedRunes := r[:truncationTarget] + // A string holding multi-byte characters has fewer runes than bytes, so + // truncationTarget can exceed the number of runes available. Start from + // the whole rune slice in that case; the loop below still trims it down to + // truncationTarget bytes. + truncatedRunes := r[:min(truncationTarget, len(r))] for len(string(truncatedRunes)) > truncationTarget { truncatedRunes = r[:len(truncatedRunes)-1] } diff --git a/notify/util_test.go b/notify/util_test.go index 2c2d4922e8..ad8b56a4a1 100644 --- a/notify/util_test.go +++ b/notify/util_test.go @@ -23,6 +23,7 @@ import ( "path" "reflect" "runtime" + "strings" "testing" "github.com/prometheus/common/model" @@ -99,6 +100,14 @@ func TestTruncate(t *testing.T) { runes: expect{out: "β€οΈβœ…πŸš€πŸ”₯βŒβ€οΈβœ…πŸš€πŸ”₯βŒβ€οΈβœ…πŸš€πŸ”₯βŒβ€¦", trunc: true}, bytes: expect{out: "β€οΈβœ…πŸš€β€¦", trunc: true}, }, + { + // Two runes, eight bytes: the byte budget left for the text is + // larger than the number of runes to pick it from. + in: "πŸ˜€πŸ˜€", + n: 7, + runes: expect{out: "πŸ˜€πŸ˜€", trunc: false}, + bytes: expect{out: "πŸ˜€β€¦", trunc: true}, + }, } type truncateFunc func(string, int) (string, bool) @@ -130,6 +139,42 @@ func TestTruncate(t *testing.T) { } } +// TestTruncateInBytesFewerRunesThanBytes covers strings that are longer than +// the byte limit while holding fewer runes than it, which is the case for any +// sufficiently long non-ASCII text. TruncateInBytes used to index the rune +// slice with the byte budget and panicked on those inputs. +func TestTruncateInBytesFewerRunesThanBytes(t *testing.T) { + // The message limit of the Webex notifier, the only caller of + // TruncateInBytes. + const n = 7439 + + for _, tc := range []struct { + name string + char string + count int + }{ + {name: "two-byte runes", char: "Π΄", count: 3720}, + {name: "three-byte runes", char: "δΈ–", count: 2600}, + {name: "four-byte runes", char: "πŸ˜€", count: 2000}, + } { + t.Run(tc.name, func(t *testing.T) { + in := strings.Repeat(tc.char, tc.count) + require.Greater(t, len(in), n, "the input must be over the byte limit") + require.Less(t, len([]rune(in)), n, "the input must hold fewer runes than the byte limit") + + out, truncated := TruncateInBytes(in, n) + require.True(t, truncated) + require.LessOrEqual(t, len(out), n) + require.True(t, strings.HasSuffix(out, truncationMarker)) + + kept := strings.TrimSuffix(out, truncationMarker) + require.True(t, strings.HasPrefix(in, kept), "the kept text must be a prefix of the input") + require.Len(t, []rune(kept), (n-len(truncationMarker))/len(tc.char), + "the kept text must hold as many whole runes as the byte budget allows") + }) + } +} + type brokenReader struct{} func (b brokenReader) Read([]byte) (int, error) { diff --git a/notify/webex/webex_test.go b/notify/webex/webex_test.go index eb12cfc2b6..fbb49f6e3a 100644 --- a/notify/webex/webex_test.go +++ b/notify/webex/webex_test.go @@ -15,10 +15,12 @@ package webex import ( "context" + "encoding/json" "io" "net/http" "net/http/httptest" "net/url" + "strings" "testing" "time" @@ -169,6 +171,54 @@ func TestWebexTemplating(t *testing.T) { } } +// TestWebexTruncatesMultiByteMessage sends a message that is over +// maxMessageSize in bytes while holding fewer runes than that, the shape any +// long non-ASCII alert takes. Notify used to panic on it. +func TestWebexTruncatesMultiByteMessage(t *testing.T) { + var out []byte + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + var err error + out, err = io.ReadAll(r.Body) + require.NoError(t, err) + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + notifier, err := New( + &config.WebexConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + APIURL: &amcommoncfg.URL{URL: u}, + Message: "{{ .CommonAnnotations.description }}", + }, + test.CreateTmpl(t), + promslog.NewNopLogger(), + ) + require.NoError(t, err) + + // 3720 Cyrillic characters are 7440 bytes, one byte over the limit. + description := strings.Repeat("Π΄", 3720) + require.Greater(t, len(description), maxMessageSize) + require.Less(t, len([]rune(description)), maxMessageSize) + + ctx := notify.WithGroupKey(context.Background(), "1") + retry, err := notifier.Notify(ctx, &types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"lbl1": "val1"}, + Annotations: model.LabelSet{"description": model.LabelValue(description)}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + }) + require.NoError(t, err) + require.False(t, retry) + + var w webhook + require.NoError(t, json.Unmarshal(out, &w)) + require.LessOrEqual(t, len(w.Markdown), maxMessageSize) + require.True(t, strings.HasPrefix(description, strings.TrimSuffix(w.Markdown, "…"))) +} + func TestWebexFailureReason(t *testing.T) { for _, tc := range []struct { name string