From cf3ebab4ba3f2d04d48a1fb6d29fc561a7aff4e1 Mon Sep 17 00:00:00 2001 From: Kidsan Date: Mon, 7 Oct 2024 22:47:45 +0200 Subject: [PATCH 1/2] fix: add length limit to discord webhook icon_url --- options/locale/locale_en-US.ini | 1 + services/webhook/discord.go | 31 +++++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a4e653b6ba..115fe589f8 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2358,6 +2358,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 diff --git a/services/webhook/discord.go b/services/webhook/discord.go index af1dd79927..d2fa7c7d1f 100644 --- a/services/webhook/discord.go +++ b/services/webhook/discord.go @@ -14,6 +14,8 @@ import ( "strings" "unicode/utf8" + "gitea.com/go-chi/binding" + webhook_model "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" @@ -22,6 +24,7 @@ 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" ) @@ -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 { - var form struct { - forms.WebhookCoreForm - PayloadURL string `binding:"Required;ValidUrl"` - Username string - IconURL string +type discordForm struct { + forms.WebhookCoreForm + PayloadURL string `binding:"Required;ValidUrl"` + Username string + 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{ From 6ea6f224b8fee02706cffa6e56e293d3413c709d Mon Sep 17 00:00:00 2001 From: Kidsan Date: Mon, 7 Oct 2024 22:47:52 +0200 Subject: [PATCH 2/2] fix: improve discord webhook api conformance This commit corrects some cases in the discord webhook payload that do not align with the discord documentation --- services/webhook/discord.go | 24 ++++++++------ services/webhook/discord_test.go | 43 ++++++++++++++++++++++++++ services/webhook/general_test.go | 17 ++++++++-- templates/webhook/new/discord.tmpl | 4 +-- tests/integration/repo_webhook_test.go | 1 + 5 files changed, 76 insertions(+), 13 deletions(-) diff --git a/services/webhook/discord.go b/services/webhook/discord.go index d2fa7c7d1f..b0142b8509 100644 --- a/services/webhook/discord.go +++ b/services/webhook/discord.go @@ -14,8 +14,6 @@ import ( "strings" "unicode/utf8" - "gitea.com/go-chi/binding" - webhook_model "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" @@ -27,6 +25,8 @@ import ( 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{} @@ -37,8 +37,8 @@ func (discordHandler) Icon(size int) template.HTML { return shared.ImgIcon("di type discordForm struct { forms.WebhookCoreForm PayloadURL string `binding:"Required;ValidUrl"` - Username string - IconURL string + Username string `binding:"Required;MaxSize(80)"` + IconURL string `binding:"ValidUrl"` } var _ binding.Validator = &discordForm{} @@ -75,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 @@ -99,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"` - Content string `json:"content"` + Wait bool `json:"-"` + Content string `json:"-"` Username string `json:"username"` AvatarURL string `json:"avatar_url,omitempty"` - TTS bool `json:"tts"` + TTS bool `json:"-"` Embeds []DiscordEmbed `json:"embeds"` } @@ -341,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, diff --git a/services/webhook/discord_test.go b/services/webhook/discord_test.go index cc5ec8a3e0..680f7806a9 100644 --- a/services/webhook/discord_test.go +++ b/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) { diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index 2d991f8300..6dcd787fab 100644 --- a/services/webhook/general_test.go +++ b/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", - Body: "issue body", + Title: title, + Body: body, Poster: &api.User{ UserName: "user1", AvatarURL: "http://localhost:3000/user1/avatar", diff --git a/templates/webhook/new/discord.tmpl b/templates/webhook/new/discord.tmpl index 48ac84fc6b..455a96c1f4 100644 --- a/templates/webhook/new/discord.tmpl +++ b/templates/webhook/new/discord.tmpl @@ -5,9 +5,9 @@ -
+
- +
diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index b98fce0f4a..8f65b5c34f 100644 --- a/tests/integration/repo_webhook_test.go +++ b/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{