Merge pull request 'fix: correct Discord webhook JSON for issue events' (#5492) from kidsan/forgejo:discord-api-conformance into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5492
Reviewed-by: Otto <otto@codeberg.org>

options/locale/locale_en-US.ini

@@ -2360,6 +2360,7 @@ settings.slack_icon_url = Icon URL
 settings.slack_color = Color
 settings.discord_username = Username
 settings.discord_icon_url = Icon URL
+settings.discord_icon_url.exceeds_max_length = Icon URL must be less than or equal to 2048 characters
 settings.event_desc = Trigger on:
 settings.event_push_only = Push events
 settings.event_send_everything = All events

services/webhook/discord.go

@@ -22,8 +22,11 @@ import (
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
+	gitea_context "code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
 	"code.gitea.io/gitea/services/webhook/shared"
+
+	"gitea.com/go-chi/binding"
 )
 
 type discordHandler struct{}
@@ -31,13 +34,29 @@ type discordHandler struct{}
 func (discordHandler) Type() webhook_module.HookType { return webhook_module.DISCORD }
 func (discordHandler) Icon(size int) template.HTML   { return shared.ImgIcon("discord.png", size) }
 
-func (discordHandler) UnmarshalForm(bind func(any)) forms.WebhookForm {
+type discordForm struct {
-	var form struct {
+	forms.WebhookCoreForm
-		forms.WebhookCoreForm
+	PayloadURL string `binding:"Required;ValidUrl"`
-		PayloadURL string `binding:"Required;ValidUrl"`
+	Username   string `binding:"Required;MaxSize(80)"`
-		Username   string
+	IconURL    string `binding:"ValidUrl"`
-		IconURL    string
+}
+
+var _ binding.Validator = &discordForm{}
+
+// Validate implements binding.Validator.
+func (d *discordForm) Validate(req *http.Request, errs binding.Errors) binding.Errors {
+	ctx := gitea_context.GetWebContext(req)
+	if len([]rune(d.IconURL)) > 2048 {
+		errs = append(errs, binding.Error{
+			FieldNames: []string{"IconURL"},
+			Message:    ctx.Locale.TrString("repo.settings.discord_icon_url.exceeds_max_length"),
+		})
 	}
+	return errs
+}
+
+func (discordHandler) UnmarshalForm(bind func(any)) forms.WebhookForm {
+	var form discordForm
 	bind(&form)
 
 	return forms.WebhookForm{
@@ -56,7 +75,7 @@ func (discordHandler) UnmarshalForm(bind func(any)) forms.WebhookForm {
 type (
 	// DiscordEmbedFooter for Embed Footer Structure.
 	DiscordEmbedFooter struct {
-		Text string `json:"text"`
+		Text string `json:"text,omitempty"`
 	}
 
 	// DiscordEmbedAuthor for Embed Author Structure
@@ -80,16 +99,16 @@ type (
 		Color       int                 `json:"color"`
 		Footer      DiscordEmbedFooter  `json:"footer"`
 		Author      DiscordEmbedAuthor  `json:"author"`
-		Fields      []DiscordEmbedField `json:"fields"`
+		Fields      []DiscordEmbedField `json:"fields,omitempty"`
 	}
 
 	// DiscordPayload represents
 	DiscordPayload struct {
-		Wait      bool           `json:"wait"`
+		Wait      bool           `json:"-"`
-		Content   string         `json:"content"`
+		Content   string         `json:"-"`
 		Username  string         `json:"username"`
 		AvatarURL string         `json:"avatar_url,omitempty"`
-		TTS       bool           `json:"tts"`
+		TTS       bool           `json:"-"`
 		Embeds    []DiscordEmbed `json:"embeds"`
 	}
 
@@ -322,6 +341,12 @@ func parseHookPullRequestEventType(event webhook_module.HookEventType) (string,
 }
 
 func (d discordConvertor) createPayload(s *api.User, title, text, url string, color int) DiscordPayload {
+	if len([]rune(title)) > 256 {
+		title = fmt.Sprintf("%.253s...", title)
+	}
+	if len([]rune(text)) > 4096 {
+		text = fmt.Sprintf("%.4093s...", text)
+	}
 	return DiscordPayload{
 		Username:  d.Username,
 		AvatarURL: d.AvatarURL,

services/webhook/discord_test.go

@@ -120,6 +120,49 @@ func TestDiscordPayload(t *testing.T) {
 		assert.Equal(t, p.Sender.UserName, pl.Embeds[0].Author.Name)
 		assert.Equal(t, setting.AppURL+p.Sender.UserName, pl.Embeds[0].Author.URL)
 		assert.Equal(t, p.Sender.AvatarURL, pl.Embeds[0].Author.IconURL)
+
+		j, err := json.Marshal(pl)
+		require.NoError(t, err)
+
+		unsetFields := struct {
+			Content *string `json:"content"`
+			TTS     *bool   `json:"tts"`
+			Wait    *bool   `json:"wait"`
+			Fields  []any   `json:"fields"`
+			Footer  struct {
+				Text *string `json:"text"`
+			} `json:"footer"`
+		}{}
+
+		err = json.Unmarshal(j, &unsetFields)
+		require.NoError(t, err)
+		assert.Nil(t, unsetFields.Content)
+		assert.Nil(t, unsetFields.TTS)
+		assert.Nil(t, unsetFields.Wait)
+		assert.Nil(t, unsetFields.Fields)
+		assert.Nil(t, unsetFields.Footer.Text)
+	})
+
+	t.Run("Issue with long title", func(t *testing.T) {
+		p := issueTestPayloadWithLongTitle()
+
+		p.Action = api.HookIssueOpened
+		pl, err := dc.Issue(p)
+		require.NoError(t, err)
+
+		assert.Len(t, pl.Embeds, 1)
+		assert.Len(t, pl.Embeds[0].Title, 256)
+	})
+
+	t.Run("Issue with long body", func(t *testing.T) {
+		p := issueTestPayloadWithLongBody()
+
+		p.Action = api.HookIssueOpened
+		pl, err := dc.Issue(p)
+		require.NoError(t, err)
+
+		assert.Len(t, pl.Embeds, 1)
+		assert.Len(t, pl.Embeds[0].Description, 4096)
 	})
 
 	t.Run("IssueComment", func(t *testing.T) {

services/webhook/general_test.go

@@ -4,6 +4,7 @@
 package webhook
 
 import (
+	"strings"
 	"testing"
 
 	api "code.gitea.io/gitea/modules/structs"
@@ -113,6 +114,18 @@ func pushTestPayloadWithCommitMessage(message string) *api.PushPayload {
 }
 
 func issueTestPayload() *api.IssuePayload {
+	return issuePayloadWithTitleAndBody("crash", "issue body")
+}
+
+func issueTestPayloadWithLongBody() *api.IssuePayload {
+	return issuePayloadWithTitleAndBody("crash", strings.Repeat("issue body", 4097))
+}
+
+func issueTestPayloadWithLongTitle() *api.IssuePayload {
+	return issuePayloadWithTitleAndBody(strings.Repeat("a", 257), "issue body")
+}
+
+func issuePayloadWithTitleAndBody(title, body string) *api.IssuePayload {
 	return &api.IssuePayload{
 		Index: 2,
 		Sender: &api.User{
@@ -129,8 +142,8 @@ func issueTestPayload() *api.IssuePayload {
 			Index:   2,
 			URL:     "http://localhost:3000/api/v1/repos/test/repo/issues/2",
 			HTMLURL: "http://localhost:3000/test/repo/issues/2",
-			Title:   "crash",
+			Title:   title,
-			Body:    "issue body",
+			Body:    body,
 			Poster: &api.User{
 				UserName:  "user1",
 				AvatarURL: "http://localhost:3000/user1/avatar",

templates/webhook/new/discord.tmpl

@@ -5,9 +5,9 @@
 		<label for="payload_url">{{ctx.Locale.Tr "repo.settings.payload_url"}}</label>
 		<input id="payload_url" name="payload_url" type="url" value="{{.Webhook.URL}}" autofocus required>
 	</div>
-	<div class="field">
+	<div class="required field {{if .Err_PayloadURL}}error{{end}}">
 		<label for="username">{{ctx.Locale.Tr "repo.settings.discord_username"}}</label>
-		<input id="username" name="username" value="{{.HookMetadata.Username}}" placeholder="Forgejo">
+		<input id="username" name="username" value="{{.HookMetadata.Username}}" autofocus required placeholder="Forgejo">
 	</div>
 	<div class="field">
 		<label for="icon_url">{{ctx.Locale.Tr "repo.settings.discord_icon_url"}}</label>

tests/integration/repo_webhook_test.go

@@ -172,6 +172,7 @@ func TestWebhookForms(t *testing.T) {
 	}))
 
 	t.Run("discord/required", testWebhookForms("discord", session, map[string]string{
+		"username":    "john",
 		"payload_url": "https://discord.example.com",
 	}))
 	t.Run("discord/optional", testWebhookForms("discord", session, map[string]string{