fix: improve discord webhook api conformance

This commit corrects some cases in the discord webhook payload that do
not align with the discord documentation

(cherry picked from commit 6ea6f224b8)
This commit is contained in:
Kidsan 2024-10-07 22:47:52 +02:00 committed by forgejo-backport-action
parent 2c0c6f408e
commit e2ffe12e50
5 changed files with 76 additions and 13 deletions

View file

@ -14,8 +14,6 @@ import (
"strings" "strings"
"unicode/utf8" "unicode/utf8"
"gitea.com/go-chi/binding"
webhook_model "code.gitea.io/gitea/models/webhook" webhook_model "code.gitea.io/gitea/models/webhook"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/json"
@ -27,6 +25,8 @@ import (
gitea_context "code.gitea.io/gitea/services/context" gitea_context "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/webhook/shared" "code.gitea.io/gitea/services/webhook/shared"
"gitea.com/go-chi/binding"
) )
type discordHandler struct{} type discordHandler struct{}
@ -37,8 +37,8 @@ func (discordHandler) Icon(size int) template.HTML { return shared.ImgIcon("di
type discordForm struct { type discordForm struct {
forms.WebhookCoreForm forms.WebhookCoreForm
PayloadURL string `binding:"Required;ValidUrl"` PayloadURL string `binding:"Required;ValidUrl"`
Username string Username string `binding:"Required;MaxSize(80)"`
IconURL string IconURL string `binding:"ValidUrl"`
} }
var _ binding.Validator = &discordForm{} var _ binding.Validator = &discordForm{}
@ -75,7 +75,7 @@ func (discordHandler) UnmarshalForm(bind func(any)) forms.WebhookForm {
type ( type (
// DiscordEmbedFooter for Embed Footer Structure. // DiscordEmbedFooter for Embed Footer Structure.
DiscordEmbedFooter struct { DiscordEmbedFooter struct {
Text string `json:"text"` Text string `json:"text,omitempty"`
} }
// DiscordEmbedAuthor for Embed Author Structure // DiscordEmbedAuthor for Embed Author Structure
@ -99,16 +99,16 @@ type (
Color int `json:"color"` Color int `json:"color"`
Footer DiscordEmbedFooter `json:"footer"` Footer DiscordEmbedFooter `json:"footer"`
Author DiscordEmbedAuthor `json:"author"` Author DiscordEmbedAuthor `json:"author"`
Fields []DiscordEmbedField `json:"fields"` Fields []DiscordEmbedField `json:"fields,omitempty"`
} }
// DiscordPayload represents // DiscordPayload represents
DiscordPayload struct { DiscordPayload struct {
Wait bool `json:"wait"` Wait bool `json:"-"`
Content string `json:"content"` Content string `json:"-"`
Username string `json:"username"` Username string `json:"username"`
AvatarURL string `json:"avatar_url,omitempty"` AvatarURL string `json:"avatar_url,omitempty"`
TTS bool `json:"tts"` TTS bool `json:"-"`
Embeds []DiscordEmbed `json:"embeds"` 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 { 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{ return DiscordPayload{
Username: d.Username, Username: d.Username,
AvatarURL: d.AvatarURL, AvatarURL: d.AvatarURL,

View file

@ -120,6 +120,49 @@ func TestDiscordPayload(t *testing.T) {
assert.Equal(t, p.Sender.UserName, pl.Embeds[0].Author.Name) 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, setting.AppURL+p.Sender.UserName, pl.Embeds[0].Author.URL)
assert.Equal(t, p.Sender.AvatarURL, pl.Embeds[0].Author.IconURL) 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) { t.Run("IssueComment", func(t *testing.T) {

View file

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

View file

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

View file

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