fix: truncate long activity fields to prevent Discord rejection #18

Merged
deluan merged 3 commits from fix/truncate-long-fields into main 2026-03-04 10:48:56 -07:00
2 changed files with 148 additions and 0 deletions
+36
View File
@@ -55,6 +55,30 @@ const (
const heartbeatInterval = 41 // Heartbeat interval in seconds const heartbeatInterval = 41 // Heartbeat interval in seconds
// Discord API field length limits
const (
maxTextLength = 128 // Max characters for text fields (details, state, name, large_text)
maxURLLength = 256 // Max characters for URL fields (details_url, state_url, etc.)
)
// truncateText truncates s to maxTextLength runes, appending "…" if truncated.
func truncateText(s string) string {
runes := []rune(s)
if len(runes) <= maxTextLength {
return s
}
return string(runes[:maxTextLength-1]) + "…"
}
// truncateURL returns s unchanged if within maxURLLength, otherwise returns ""
// (a truncated URL would be broken, so we omit it entirely).
func truncateURL(s string) string {
if len(s) <= maxURLLength {
return s
}
return ""
}
// activity represents a Discord activity sent via Gateway opcode 3. // activity represents a Discord activity sent via Gateway opcode 3.
type activity struct { type activity struct {
Name string `json:"name"` Name string `json:"name"`
@@ -200,6 +224,18 @@ func (r *discordRPC) processImage(imageURL, clientID, token string, ttl int64) (
func (r *discordRPC) sendActivity(clientID, username, token string, data activity) error { func (r *discordRPC) sendActivity(clientID, username, token string, data activity) error {
pdk.Log(pdk.LogInfo, fmt.Sprintf("Sending activity for user %s: %s - %s", username, data.Details, data.State)) pdk.Log(pdk.LogInfo, fmt.Sprintf("Sending activity for user %s: %s - %s", username, data.Details, data.State))
// Truncate text fields to Discord's 128-character limit
data.Name = truncateText(data.Name)
data.Details = truncateText(data.Details)
data.State = truncateText(data.State)
data.Assets.LargeText = truncateText(data.Assets.LargeText)
// Omit URLs that exceed Discord's 256-character limit
data.DetailsURL = truncateURL(data.DetailsURL)
data.StateURL = truncateURL(data.StateURL)
data.Assets.LargeURL = truncateURL(data.Assets.LargeURL)
data.Assets.SmallURL = truncateURL(data.Assets.SmallURL)
// Try track artwork first, fall back to Navidrome logo // Try track artwork first, fall back to Navidrome logo
usingDefaultImage := false usingDefaultImage := false
processedImage, err := r.processImage(data.Assets.LargeImage, clientID, token, imageCacheTTL) processedImage, err := r.processImage(data.Assets.LargeImage, clientID, token, imageCacheTTL)
+112
View File
@@ -1,6 +1,7 @@
package main package main
import ( import (
"encoding/json"
"errors" "errors"
"strings" "strings"
@@ -435,6 +436,66 @@ var _ = Describe("discordRPC", func() {
}) })
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
}) })
It("truncates long text fields and omits long URLs", func() {
host.CacheMock.On("GetString", discordImageKey).Return("mp:cached/art", true, nil).Once()
host.CacheMock.On("GetString", discordImageKey).Return("mp:cached/logo", true, nil).Once()
longName := strings.Repeat("N", 200)
longTitle := strings.Repeat("T", 200)
longArtist := strings.Repeat("A", 200)
longAlbum := strings.Repeat("B", 200)
longURL := "https://example.com/" + strings.Repeat("x", 237)
truncatedName := strings.Repeat("N", 127) + "…"
truncatedTitle := strings.Repeat("T", 127) + "…"
truncatedArtist := strings.Repeat("A", 127) + "…"
truncatedAlbum := strings.Repeat("B", 127) + "…"
host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool {
var message struct {
D json.RawMessage `json:"d"`
}
if err := json.Unmarshal([]byte(msg), &message); err != nil {
return false
}
var presence presencePayload
if err := json.Unmarshal(message.D, &presence); err != nil {
return false
}
if len(presence.Activities) != 1 {
return false
}
act := presence.Activities[0]
return act.Name == truncatedName &&
act.Details == truncatedTitle &&
act.State == truncatedArtist &&
act.Assets.LargeText == truncatedAlbum &&
act.DetailsURL == "" &&
act.StateURL == "" &&
act.Assets.LargeURL == "" &&
act.Assets.SmallURL == ""
})).Return(nil)
err := r.sendActivity("client123", "testuser", "token123", activity{
Application: "client123",
Name: longName,
Type: 2,
Details: longTitle,
DetailsURL: longURL,
gemini-code-assist[bot] commented 2026-03-04 10:33:39 -07:00 (Migrated from github.com)
Review

medium

The test validation using strings.Contains is fragile. It could lead to false positives if the checked substring appears elsewhere in the JSON payload, or if JSON encoding adds escape characters. For example, it doesn't strictly check that the URL fields are omitted, only that the long URL string isn't present.

A more robust approach would be to unmarshal the JSON message and assert on the specific field values. This would ensure the correct fields are being truncated and that the URL fields are correctly omitted (by checking for an empty string value, or that the key is not present due to omitempty).

Consider refactoring the check like this:

// You would need to add: import "encoding/json"
host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool {
    var message struct {
        D json.RawMessage `json:"d"`
    }
    if err := json.Unmarshal([]byte(msg), &message); err != nil {
        return false
    }

    var presence presencePayload
    if err := json.Unmarshal(message.D, &presence); err != nil {
        return false
    }

    if len(presence.Activities) != 1 {
        return false
    }
    act := presence.Activities[0]

    truncatedName := strings.Repeat("N", 127) + "…"
    truncatedTitle := strings.Repeat("T", 127) + "…"
    truncatedArtist := strings.Repeat("A", 127) + "…"
    truncatedAlbum := strings.Repeat("B", 127) + "…"

    return act.Name == truncatedName &&
        act.Details == truncatedTitle &&
        act.State == truncatedArtist &&
        act.Assets.LargeText == truncatedAlbum &&
        act.DetailsURL == "" &&
        act.StateURL == "" &&
        act.Assets.LargeURL == "" &&
        act.Assets.SmallURL == ""
})).Return(nil)
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The test validation using `strings.Contains` is fragile. It could lead to false positives if the checked substring appears elsewhere in the JSON payload, or if JSON encoding adds escape characters. For example, it doesn't strictly check that the URL fields are omitted, only that the long URL string isn't present. A more robust approach would be to unmarshal the JSON message and assert on the specific field values. This would ensure the correct fields are being truncated and that the URL fields are correctly omitted (by checking for an empty string value, or that the key is not present due to `omitempty`). Consider refactoring the check like this: ```go // You would need to add: import "encoding/json" host.WebSocketMock.On("SendText", "testuser", mock.MatchedBy(func(msg string) bool { var message struct { D json.RawMessage `json:"d"` } if err := json.Unmarshal([]byte(msg), &message); err != nil { return false } var presence presencePayload if err := json.Unmarshal(message.D, &presence); err != nil { return false } if len(presence.Activities) != 1 { return false } act := presence.Activities[0] truncatedName := strings.Repeat("N", 127) + "…" truncatedTitle := strings.Repeat("T", 127) + "…" truncatedArtist := strings.Repeat("A", 127) + "…" truncatedAlbum := strings.Repeat("B", 127) + "…" return act.Name == truncatedName && act.Details == truncatedTitle && act.State == truncatedArtist && act.Assets.LargeText == truncatedAlbum && act.DetailsURL == "" && act.StateURL == "" && act.Assets.LargeURL == "" && act.Assets.SmallURL == "" })).Return(nil) ```
State: longArtist,
StateURL: longURL,
Assets: activityAssets{
LargeImage: "https://example.com/art.jpg",
LargeText: longAlbum,
LargeURL: longURL,
SmallImage: navidromeLogoURL,
SmallText: "Navidrome",
SmallURL: longURL,
},
})
Expect(err).ToNot(HaveOccurred())
})
}) })
Describe("clearActivity", func() { Describe("clearActivity", func() {
@@ -448,4 +509,55 @@ var _ = Describe("discordRPC", func() {
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())
}) })
}) })
Describe("truncateText", func() {
It("returns short strings unchanged", func() {
Expect(truncateText("hello")).To(Equal("hello"))
})
It("returns exactly 128-char strings unchanged", func() {
s := strings.Repeat("a", 128)
Expect(truncateText(s)).To(Equal(s))
})
It("truncates strings over 128 chars to 127 + ellipsis", func() {
s := strings.Repeat("a", 200)
result := truncateText(s)
Expect([]rune(result)).To(HaveLen(128))
Expect(result).To(HaveSuffix("…"))
})
It("handles multi-byte characters correctly", func() {
// 130 Japanese characters — each is one rune but 3 bytes
s := strings.Repeat("あ", 130)
result := truncateText(s)
runes := []rune(result)
Expect(runes).To(HaveLen(128))
Expect(string(runes[127])).To(Equal("…"))
})
It("returns empty string unchanged", func() {
Expect(truncateText("")).To(Equal(""))
})
})
Describe("truncateURL", func() {
It("returns short URLs unchanged", func() {
Expect(truncateURL("https://example.com")).To(Equal("https://example.com"))
})
It("returns exactly 256-char URLs unchanged", func() {
u := "https://example.com/" + strings.Repeat("a", 236)
Expect(truncateURL(u)).To(Equal(u))
})
It("returns empty string for URLs over 256 chars", func() {
u := "https://example.com/" + strings.Repeat("a", 237)
Expect(truncateURL(u)).To(Equal(""))
})
It("returns empty string unchanged", func() {
Expect(truncateURL("")).To(Equal(""))
})
})
}) })