From 72b6bc8cafe1d64ff30481170cd68b85a710c67e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 10 Jul 2024 19:37:16 +0800 Subject: [PATCH] Refactor webhook (#31587) A more complete fix for #31588 1. Make "generic" code more readable 2. Clarify HTML or Markdown for the payload content --- models/activities/action_test.go | 4 +- models/repo/repo.go | 9 ++- routers/web/repo/repo.go | 2 +- services/convert/repository.go | 2 +- services/webhook/dingtalk.go | 19 +++--- services/webhook/discord.go | 24 +++---- services/webhook/feishu.go | 9 ++- services/webhook/general.go | 32 ++++----- services/webhook/general_test.go | 8 +-- services/webhook/matrix.go | 6 +- services/webhook/msteams.go | 17 +++-- services/webhook/notifier.go | 2 +- services/webhook/packagist.go | 12 ++-- services/webhook/payloader.go | 8 +-- services/webhook/slack.go | 22 +++--- services/webhook/telegram.go | 107 ++++++++++++++---------------- services/webhook/telegram_test.go | 55 +++++++++------ services/webhook/wechatwork.go | 17 +++-- 18 files changed, 173 insertions(+), 182 deletions(-) diff --git a/models/activities/action_test.go b/models/activities/action_test.go index 557415dcd..36a38c786 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" ) @@ -33,7 +34,8 @@ func TestAction_GetRepoLink(t *testing.T) { owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) comment := unittest.AssertExistsAndLoadBean(t, &issue_model.Comment{ID: 2}) action := &activities_model.Action{RepoID: repo.ID, CommentID: comment.ID} - setting.AppSubURL = "/suburl" + defer test.MockVariableValue(&setting.AppURL, "https://try.gitea.io/suburl/")() + defer test.MockVariableValue(&setting.AppSubURL, "/suburl")() expected := path.Join(setting.AppSubURL, owner.Name, repo.Name) assert.Equal(t, expected, action.GetRepoLink(db.DefaultContext)) assert.Equal(t, repo.HTMLURL(), action.GetRepoAbsoluteLink(db.DefaultContext)) diff --git a/models/repo/repo.go b/models/repo/repo.go index 189c4aba6..a5b36dd8a 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -18,6 +18,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/optional" @@ -321,8 +322,12 @@ func (repo *Repository) FullName() string { } // HTMLURL returns the repository HTML URL -func (repo *Repository) HTMLURL() string { - return setting.AppURL + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name) +func (repo *Repository) HTMLURL(ctxs ...context.Context) string { + ctx := context.TODO() + if len(ctxs) > 0 { + ctx = ctxs[0] + } + return httplib.MakeAbsoluteURL(ctx, repo.Link()) } // CommitLink make link to by commit full ID diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 70dcbae0e..2ef548b5b 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -668,7 +668,7 @@ func SearchRepo(ctx *context.Context) { Template: repo.IsTemplate, Mirror: repo.IsMirror, Stars: repo.NumStars, - HTMLURL: repo.HTMLURL(), + HTMLURL: repo.HTMLURL(ctx), Link: repo.Link(), Internal: !repo.IsPrivate && repo.Owner.Visibility == api.VisibleTypePrivate, }, diff --git a/services/convert/repository.go b/services/convert/repository.go index d7568e8d0..751260a45 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -191,7 +191,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR Fork: repo.IsFork, Parent: parent, Mirror: repo.IsMirror, - HTMLURL: repo.HTMLURL(), + HTMLURL: repo.HTMLURL(ctx), URL: repoAPIURL, SSHURL: cloneLink.SSH, CloneURL: cloneLink.HTTPS, diff --git a/services/webhook/dingtalk.go b/services/webhook/dingtalk.go index f6018f737..e382f5a9d 100644 --- a/services/webhook/dingtalk.go +++ b/services/webhook/dingtalk.go @@ -20,8 +20,8 @@ import ( ) type ( - // DingtalkPayload represents - DingtalkPayload dingtalk.Payload + DingtalkPayload dingtalk.Payload + dingtalkConvertor struct{} ) // Create implements PayloadConvertor Create method @@ -92,9 +92,9 @@ func (dc dingtalkConvertor) Push(p *api.PushPayload) (DingtalkPayload, error) { // Issue implements PayloadConvertor Issue method func (dc dingtalkConvertor) Issue(p *api.IssuePayload) (DingtalkPayload, error) { - text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true) + text, issueTitle, extraMarkdown, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true) - return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+attachmentText, "view issue", p.Issue.HTMLURL), nil + return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+extraMarkdown, "view issue", p.Issue.HTMLURL), nil } // Wiki implements PayloadConvertor Wiki method @@ -114,9 +114,9 @@ func (dc dingtalkConvertor) IssueComment(p *api.IssueCommentPayload) (DingtalkPa // PullRequest implements PayloadConvertor PullRequest method func (dc dingtalkConvertor) PullRequest(p *api.PullRequestPayload) (DingtalkPayload, error) { - text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true) + text, issueTitle, extraMarkdown, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true) - return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+attachmentText, "view pull request", p.PullRequest.HTMLURL), nil + return createDingtalkPayload(issueTitle, text+"\r\n\r\n"+extraMarkdown, "view pull request", p.PullRequest.HTMLURL), nil } // Review implements PayloadConvertor Review method @@ -186,10 +186,7 @@ func createDingtalkPayload(title, text, singleTitle, singleURL string) DingtalkP } } -type dingtalkConvertor struct{} - -var _ payloadConvertor[DingtalkPayload] = dingtalkConvertor{} - func newDingtalkRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { - return newJSONRequest(dingtalkConvertor{}, w, t, true) + var pc payloadConvertor[DingtalkPayload] = dingtalkConvertor{} + return newJSONRequest(pc, w, t, true) } diff --git a/services/webhook/discord.go b/services/webhook/discord.go index 31332396f..f27b39dac 100644 --- a/services/webhook/discord.go +++ b/services/webhook/discord.go @@ -100,6 +100,11 @@ var ( redColor = color("ff3232") ) +type discordConvertor struct { + Username string + AvatarURL string +} + // Create implements PayloadConvertor Create method func (d discordConvertor) Create(p *api.CreatePayload) (DiscordPayload, error) { // created tag/branch @@ -162,9 +167,9 @@ func (d discordConvertor) Push(p *api.PushPayload) (DiscordPayload, error) { // Issue implements PayloadConvertor Issue method func (d discordConvertor) Issue(p *api.IssuePayload) (DiscordPayload, error) { - title, _, text, color := getIssuesPayloadInfo(p, noneLinkFormatter, false) + title, _, extraMarkdown, color := getIssuesPayloadInfo(p, noneLinkFormatter, false) - return d.createPayload(p.Sender, title, text, p.Issue.HTMLURL, color), nil + return d.createPayload(p.Sender, title, extraMarkdown, p.Issue.HTMLURL, color), nil } // IssueComment implements PayloadConvertor IssueComment method @@ -176,9 +181,9 @@ func (d discordConvertor) IssueComment(p *api.IssueCommentPayload) (DiscordPaylo // PullRequest implements PayloadConvertor PullRequest method func (d discordConvertor) PullRequest(p *api.PullRequestPayload) (DiscordPayload, error) { - title, _, text, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false) + title, _, extraMarkdown, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false) - return d.createPayload(p.Sender, title, text, p.PullRequest.HTMLURL, color), nil + return d.createPayload(p.Sender, title, extraMarkdown, p.PullRequest.HTMLURL, color), nil } // Review implements PayloadConvertor Review method @@ -253,23 +258,16 @@ func (d discordConvertor) Package(p *api.PackagePayload) (DiscordPayload, error) return d.createPayload(p.Sender, text, "", p.Package.HTMLURL, color), nil } -type discordConvertor struct { - Username string - AvatarURL string -} - -var _ payloadConvertor[DiscordPayload] = discordConvertor{} - func newDiscordRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { meta := &DiscordMeta{} if err := json.Unmarshal([]byte(w.Meta), meta); err != nil { return nil, nil, fmt.Errorf("newDiscordRequest meta json: %w", err) } - sc := discordConvertor{ + var pc payloadConvertor[DiscordPayload] = discordConvertor{ Username: meta.Username, AvatarURL: meta.IconURL, } - return newJSONRequest(sc, w, t, true) + return newJSONRequest(pc, w, t, true) } func parseHookPullRequestEventType(event webhook_module.HookEventType) (string, error) { diff --git a/services/webhook/feishu.go b/services/webhook/feishu.go index 38f324aa7..7ca7d1cf5 100644 --- a/services/webhook/feishu.go +++ b/services/webhook/feishu.go @@ -36,6 +36,8 @@ func newFeishuTextPayload(text string) FeishuPayload { } } +type feishuConvertor struct{} + // Create implements PayloadConvertor Create method func (fc feishuConvertor) Create(p *api.CreatePayload) (FeishuPayload, error) { // created tag/branch @@ -164,10 +166,7 @@ func (fc feishuConvertor) Package(p *api.PackagePayload) (FeishuPayload, error) return newFeishuTextPayload(text), nil } -type feishuConvertor struct{} - -var _ payloadConvertor[FeishuPayload] = feishuConvertor{} - func newFeishuRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { - return newJSONRequest(feishuConvertor{}, w, t, true) + var pc payloadConvertor[FeishuPayload] = feishuConvertor{} + return newJSONRequest(pc, w, t, true) } diff --git a/services/webhook/general.go b/services/webhook/general.go index 69b944f4b..dde43bb34 100644 --- a/services/webhook/general.go +++ b/services/webhook/general.go @@ -91,12 +91,11 @@ func getIssuesCommentInfo(p *api.IssueCommentPayload) (title, link, by, operator return title, link, by, operator } -func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) { - repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) - issueTitle := fmt.Sprintf("#%d %s", p.Index, p.Issue.Title) +func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, withSender bool) (text, issueTitle, extraMarkdown string, color int) { + color = yellowColor + issueTitle = fmt.Sprintf("#%d %s", p.Index, p.Issue.Title) titleLink := linkFormatter(fmt.Sprintf("%s/issues/%d", p.Repository.HTMLURL, p.Index), issueTitle) - var text string - color := yellowColor + repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) switch p.Action { case api.HookIssueOpened: @@ -135,26 +134,23 @@ func getIssuesPayloadInfo(p *api.IssuePayload, linkFormatter linkFormatter, with text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+url.PathEscape(p.Sender.UserName), p.Sender.UserName)) } - var attachmentText string if p.Action == api.HookIssueOpened || p.Action == api.HookIssueEdited { - attachmentText = p.Issue.Body + extraMarkdown = p.Issue.Body } - return text, issueTitle, attachmentText, color + return text, issueTitle, extraMarkdown, color } -func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkFormatter, withSender bool) (string, string, string, int) { - repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) - issueTitle := fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title) +func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkFormatter, withSender bool) (text, issueTitle, extraMarkdown string, color int) { + color = yellowColor + issueTitle = fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title) titleLink := linkFormatter(p.PullRequest.URL, issueTitle) - var text string - var attachmentText string - color := yellowColor + repoLink := linkFormatter(p.Repository.HTMLURL, p.Repository.FullName) switch p.Action { case api.HookIssueOpened: text = fmt.Sprintf("[%s] Pull request opened: %s", repoLink, titleLink) - attachmentText = p.PullRequest.Body + extraMarkdown = p.PullRequest.Body color = greenColor case api.HookIssueClosed: if p.PullRequest.HasMerged { @@ -168,7 +164,7 @@ func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkForm text = fmt.Sprintf("[%s] Pull request re-opened: %s", repoLink, titleLink) case api.HookIssueEdited: text = fmt.Sprintf("[%s] Pull request edited: %s", repoLink, titleLink) - attachmentText = p.PullRequest.Body + extraMarkdown = p.PullRequest.Body case api.HookIssueAssigned: list := make([]string, len(p.PullRequest.Assignees)) for i, user := range p.PullRequest.Assignees { @@ -193,7 +189,7 @@ func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkForm text = fmt.Sprintf("[%s] Pull request milestone cleared: %s", repoLink, titleLink) case api.HookIssueReviewed: text = fmt.Sprintf("[%s] Pull request reviewed: %s", repoLink, titleLink) - attachmentText = p.Review.Content + extraMarkdown = p.Review.Content case api.HookIssueReviewRequested: text = fmt.Sprintf("[%s] Pull request review requested: %s", repoLink, titleLink) case api.HookIssueReviewRequestRemoved: @@ -203,7 +199,7 @@ func getPullRequestPayloadInfo(p *api.PullRequestPayload, linkFormatter linkForm text += fmt.Sprintf(" by %s", linkFormatter(setting.AppURL+p.Sender.UserName, p.Sender.UserName)) } - return text, issueTitle, attachmentText, color + return text, issueTitle, extraMarkdown, color } func getReleasePayloadInfo(p *api.ReleasePayload, linkFormatter linkFormatter, withSender bool) (text string, color int) { diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index 41bac3fd0..fc6e7140e 100644 --- a/services/webhook/general_test.go +++ b/services/webhook/general_test.go @@ -424,10 +424,10 @@ func TestGetIssuesPayloadInfo(t *testing.T) { for i, c := range cases { p.Action = c.action - text, issueTitle, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter, true) + text, issueTitle, extraMarkdown, color := getIssuesPayloadInfo(p, noneLinkFormatter, true) assert.Equal(t, c.text, text, "case %d", i) assert.Equal(t, c.issueTitle, issueTitle, "case %d", i) - assert.Equal(t, c.attachmentText, attachmentText, "case %d", i) + assert.Equal(t, c.attachmentText, extraMarkdown, "case %d", i) assert.Equal(t, c.color, color, "case %d", i) } } @@ -523,10 +523,10 @@ func TestGetPullRequestPayloadInfo(t *testing.T) { for i, c := range cases { p.Action = c.action - text, issueTitle, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter, true) + text, issueTitle, extraMarkdown, color := getPullRequestPayloadInfo(p, noneLinkFormatter, true) assert.Equal(t, c.text, text, "case %d", i) assert.Equal(t, c.issueTitle, issueTitle, "case %d", i) - assert.Equal(t, c.attachmentText, attachmentText, "case %d", i) + assert.Equal(t, c.attachmentText, extraMarkdown, "case %d", i) assert.Equal(t, c.color, color, "case %d", i) } } diff --git a/services/webhook/matrix.go b/services/webhook/matrix.go index e649a0760..5e9f808d8 100644 --- a/services/webhook/matrix.go +++ b/services/webhook/matrix.go @@ -29,10 +29,10 @@ func newMatrixRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_mo if err := json.Unmarshal([]byte(w.Meta), meta); err != nil { return nil, nil, fmt.Errorf("GetMatrixPayload meta json: %w", err) } - mc := matrixConvertor{ + var pc payloadConvertor[MatrixPayload] = matrixConvertor{ MsgType: messageTypeText[meta.MessageType], } - payload, err := newPayload(mc, []byte(t.PayloadContent), t.EventType) + payload, err := newPayload(pc, []byte(t.PayloadContent), t.EventType) if err != nil { return nil, nil, err } @@ -87,8 +87,6 @@ type MatrixPayload struct { Commits []*api.PayloadCommit `json:"io.gitea.commits,omitempty"` } -var _ payloadConvertor[MatrixPayload] = matrixConvertor{} - type matrixConvertor struct { MsgType string } diff --git a/services/webhook/msteams.go b/services/webhook/msteams.go index b052b9da1..7ef96ffa2 100644 --- a/services/webhook/msteams.go +++ b/services/webhook/msteams.go @@ -58,6 +58,8 @@ type ( } ) +type msteamsConvertor struct{} + // Create implements PayloadConvertor Create method func (m msteamsConvertor) Create(p *api.CreatePayload) (MSTeamsPayload, error) { // created tag/branch @@ -152,13 +154,13 @@ func (m msteamsConvertor) Push(p *api.PushPayload) (MSTeamsPayload, error) { // Issue implements PayloadConvertor Issue method func (m msteamsConvertor) Issue(p *api.IssuePayload) (MSTeamsPayload, error) { - title, _, attachmentText, color := getIssuesPayloadInfo(p, noneLinkFormatter, false) + title, _, extraMarkdown, color := getIssuesPayloadInfo(p, noneLinkFormatter, false) return createMSTeamsPayload( p.Repository, p.Sender, title, - attachmentText, + extraMarkdown, p.Issue.HTMLURL, color, &MSTeamsFact{"Issue #:", fmt.Sprintf("%d", p.Issue.ID)}, @@ -182,13 +184,13 @@ func (m msteamsConvertor) IssueComment(p *api.IssueCommentPayload) (MSTeamsPaylo // PullRequest implements PayloadConvertor PullRequest method func (m msteamsConvertor) PullRequest(p *api.PullRequestPayload) (MSTeamsPayload, error) { - title, _, attachmentText, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false) + title, _, extraMarkdown, color := getPullRequestPayloadInfo(p, noneLinkFormatter, false) return createMSTeamsPayload( p.Repository, p.Sender, title, - attachmentText, + extraMarkdown, p.PullRequest.HTMLURL, color, &MSTeamsFact{"Pull request #:", fmt.Sprintf("%d", p.PullRequest.ID)}, @@ -343,10 +345,7 @@ func createMSTeamsPayload(r *api.Repository, s *api.User, title, text, actionTar } } -type msteamsConvertor struct{} - -var _ payloadConvertor[MSTeamsPayload] = msteamsConvertor{} - func newMSTeamsRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { - return newJSONRequest(msteamsConvertor{}, w, t, true) + var pc payloadConvertor[MSTeamsPayload] = msteamsConvertor{} + return newJSONRequest(pc, w, t, true) } diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go index 587caf62f..53b1cc8c9 100644 --- a/services/webhook/notifier.go +++ b/services/webhook/notifier.go @@ -23,7 +23,7 @@ import ( ) func init() { - notify_service.RegisterNotifier(&webhookNotifier{}) + notify_service.RegisterNotifier(NewNotifier()) } type webhookNotifier struct { diff --git a/services/webhook/packagist.go b/services/webhook/packagist.go index 593b97a17..4d809ab3a 100644 --- a/services/webhook/packagist.go +++ b/services/webhook/packagist.go @@ -40,6 +40,10 @@ func GetPackagistHook(w *webhook_model.Webhook) *PackagistMeta { return s } +type packagistConvertor struct { + PackageURL string +} + // Create implements PayloadConvertor Create method func (pc packagistConvertor) Create(_ *api.CreatePayload) (PackagistPayload, error) { return PackagistPayload{}, nil @@ -106,18 +110,12 @@ func (pc packagistConvertor) Package(_ *api.PackagePayload) (PackagistPayload, e return PackagistPayload{}, nil } -type packagistConvertor struct { - PackageURL string -} - -var _ payloadConvertor[PackagistPayload] = packagistConvertor{} - func newPackagistRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { meta := &PackagistMeta{} if err := json.Unmarshal([]byte(w.Meta), meta); err != nil { return nil, nil, fmt.Errorf("newpackagistRequest meta json: %w", err) } - pc := packagistConvertor{ + var pc payloadConvertor[PackagistPayload] = packagistConvertor{ PackageURL: meta.PackageURL, } return newJSONRequest(pc, w, t, true) diff --git a/services/webhook/payloader.go b/services/webhook/payloader.go index 54a11a586..ab280a25b 100644 --- a/services/webhook/payloader.go +++ b/services/webhook/payloader.go @@ -30,16 +30,15 @@ type payloadConvertor[T any] interface { Package(*api.PackagePayload) (T, error) } -func convertUnmarshalledJSON[T, P any](convert func(P) (T, error), data []byte) (T, error) { +func convertUnmarshalledJSON[T, P any](convert func(P) (T, error), data []byte) (t T, err error) { var p P - if err := json.Unmarshal(data, &p); err != nil { - var t T + if err = json.Unmarshal(data, &p); err != nil { return t, fmt.Errorf("could not unmarshal payload: %w", err) } return convert(p) } -func newPayload[T any](rc payloadConvertor[T], data []byte, event webhook_module.HookEventType) (T, error) { +func newPayload[T any](rc payloadConvertor[T], data []byte, event webhook_module.HookEventType) (t T, err error) { switch event { case webhook_module.HookEventCreate: return convertUnmarshalledJSON(rc.Create, data) @@ -79,7 +78,6 @@ func newPayload[T any](rc payloadConvertor[T], data []byte, event webhook_module case webhook_module.HookEventPackage: return convertUnmarshalledJSON(rc.Package, data) } - var t T return t, fmt.Errorf("newPayload unsupported event: %s", event) } diff --git a/services/webhook/slack.go b/services/webhook/slack.go index ffa2936be..c905e7a89 100644 --- a/services/webhook/slack.go +++ b/services/webhook/slack.go @@ -118,17 +118,17 @@ func (s slackConvertor) Fork(p *api.ForkPayload) (SlackPayload, error) { // Issue implements payloadConvertor Issue method func (s slackConvertor) Issue(p *api.IssuePayload) (SlackPayload, error) { - text, issueTitle, attachmentText, color := getIssuesPayloadInfo(p, SlackLinkFormatter, true) + text, issueTitle, extraMarkdown, color := getIssuesPayloadInfo(p, SlackLinkFormatter, true) var attachments []SlackAttachment - if attachmentText != "" { - attachmentText = SlackTextFormatter(attachmentText) + if extraMarkdown != "" { + extraMarkdown = SlackTextFormatter(extraMarkdown) issueTitle = SlackTextFormatter(issueTitle) attachments = append(attachments, SlackAttachment{ Color: fmt.Sprintf("%x", color), Title: issueTitle, TitleLink: p.Issue.HTMLURL, - Text: attachmentText, + Text: extraMarkdown, }) } @@ -210,17 +210,17 @@ func (s slackConvertor) Push(p *api.PushPayload) (SlackPayload, error) { // PullRequest implements payloadConvertor PullRequest method func (s slackConvertor) PullRequest(p *api.PullRequestPayload) (SlackPayload, error) { - text, issueTitle, attachmentText, color := getPullRequestPayloadInfo(p, SlackLinkFormatter, true) + text, issueTitle, extraMarkdown, color := getPullRequestPayloadInfo(p, SlackLinkFormatter, true) var attachments []SlackAttachment - if attachmentText != "" { - attachmentText = SlackTextFormatter(p.PullRequest.Body) + if extraMarkdown != "" { + extraMarkdown = SlackTextFormatter(p.PullRequest.Body) issueTitle = SlackTextFormatter(issueTitle) attachments = append(attachments, SlackAttachment{ Color: fmt.Sprintf("%x", color), Title: issueTitle, TitleLink: p.PullRequest.HTMLURL, - Text: attachmentText, + Text: extraMarkdown, }) } @@ -281,20 +281,18 @@ type slackConvertor struct { Color string } -var _ payloadConvertor[SlackPayload] = slackConvertor{} - func newSlackRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { meta := &SlackMeta{} if err := json.Unmarshal([]byte(w.Meta), meta); err != nil { return nil, nil, fmt.Errorf("newSlackRequest meta json: %w", err) } - sc := slackConvertor{ + var pc payloadConvertor[SlackPayload] = slackConvertor{ Channel: meta.Channel, Username: meta.Username, IconURL: meta.IconURL, Color: meta.Color, } - return newJSONRequest(sc, w, t, true) + return newJSONRequest(pc, w, t, true) } var slackChannel = regexp.MustCompile(`^#?[a-z0-9_-]{1,80}$`) diff --git a/services/webhook/telegram.go b/services/webhook/telegram.go index de6c878da..e54d6f294 100644 --- a/services/webhook/telegram.go +++ b/services/webhook/telegram.go @@ -6,6 +6,7 @@ package webhook import ( "context" "fmt" + "html" "net/http" "strings" @@ -13,7 +14,9 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/markup" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" webhook_module "code.gitea.io/gitea/modules/webhook" ) @@ -42,41 +45,43 @@ func GetTelegramHook(w *webhook_model.Webhook) *TelegramMeta { return s } +type telegramConvertor struct{} + // Create implements PayloadConvertor Create method func (t telegramConvertor) Create(p *api.CreatePayload) (TelegramPayload, error) { // created tag/branch refName := git.RefName(p.Ref).ShortName() - title := fmt.Sprintf(`[%s] %s %s created`, p.Repo.HTMLURL, p.Repo.FullName, p.RefType, - p.Repo.HTMLURL+"/src/"+refName, refName) + title := fmt.Sprintf(`[%s] %s %s created`, + htmlLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName), + html.EscapeString(p.RefType), + htmlLinkFormatter(p.Repo.HTMLURL+"/src/"+util.PathEscapeSegments(refName), refName), + ) - return createTelegramPayload(title), nil + return createTelegramPayloadHTML(title), nil } // Delete implements PayloadConvertor Delete method func (t telegramConvertor) Delete(p *api.DeletePayload) (TelegramPayload, error) { // created tag/branch refName := git.RefName(p.Ref).ShortName() - title := fmt.Sprintf(`[%s] %s %s deleted`, p.Repo.HTMLURL, p.Repo.FullName, p.RefType, - p.Repo.HTMLURL+"/src/"+refName, refName) - - return createTelegramPayload(title), nil + title := fmt.Sprintf(`[%s] %s %s deleted`, + htmlLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName), + html.EscapeString(p.RefType), + htmlLinkFormatter(p.Repo.HTMLURL+"/src/"+util.PathEscapeSegments(refName), refName), + ) + return createTelegramPayloadHTML(title), nil } // Fork implements PayloadConvertor Fork method func (t telegramConvertor) Fork(p *api.ForkPayload) (TelegramPayload, error) { - title := fmt.Sprintf(`%s is forked to %s`, p.Forkee.FullName, p.Repo.HTMLURL, p.Repo.FullName) - - return createTelegramPayload(title), nil + title := fmt.Sprintf(`%s is forked to %s`, html.EscapeString(p.Forkee.FullName), htmlLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName)) + return createTelegramPayloadHTML(title), nil } // Push implements PayloadConvertor Push method func (t telegramConvertor) Push(p *api.PushPayload) (TelegramPayload, error) { - var ( - branchName = git.RefName(p.Ref).ShortName() - commitDesc string - ) - - var titleLink string + branchName := git.RefName(p.Ref).ShortName() + var titleLink, commitDesc string if p.TotalCommits == 1 { commitDesc = "1 new commit" titleLink = p.Commits[0].URL @@ -85,52 +90,42 @@ func (t telegramConvertor) Push(p *api.PushPayload) (TelegramPayload, error) { titleLink = p.CompareURL } if titleLink == "" { - titleLink = p.Repo.HTMLURL + "/src/" + branchName + titleLink = p.Repo.HTMLURL + "/src/" + util.PathEscapeSegments(branchName) } - title := fmt.Sprintf(`[%s:%s] %s`, p.Repo.HTMLURL, p.Repo.FullName, titleLink, branchName, commitDesc) + title := fmt.Sprintf(`[%s:%s] %s`, htmlLinkFormatter(p.Repo.HTMLURL, p.Repo.FullName), htmlLinkFormatter(titleLink, branchName), html.EscapeString(commitDesc)) - var text string - // for each commit, generate attachment text - for i, commit := range p.Commits { - var authorName string + var htmlCommits string + for _, commit := range p.Commits { + htmlCommits += fmt.Sprintf("\n[%s] %s", htmlLinkFormatter(commit.URL, commit.ID[:7]), html.EscapeString(strings.TrimRight(commit.Message, "\r\n"))) if commit.Author != nil { - authorName = " - " + commit.Author.Name - } - text += fmt.Sprintf(`[%s] %s`, commit.URL, commit.ID[:7], - strings.TrimRight(commit.Message, "\r\n")) + authorName - // add linebreak to each commit but the last - if i < len(p.Commits)-1 { - text += "\n" + htmlCommits += " - " + html.EscapeString(commit.Author.Name) } } - - return createTelegramPayload(title + "\n" + text), nil + return createTelegramPayloadHTML(title + htmlCommits), nil } // Issue implements PayloadConvertor Issue method func (t telegramConvertor) Issue(p *api.IssuePayload) (TelegramPayload, error) { - text, _, attachmentText, _ := getIssuesPayloadInfo(p, htmlLinkFormatter, true) - - return createTelegramPayload(text + "\n\n" + attachmentText), nil + text, _, extraMarkdown, _ := getIssuesPayloadInfo(p, htmlLinkFormatter, true) + // TODO: at the moment the markdown can't be rendered easily because it has context-aware links (eg: attachments) + return createTelegramPayloadHTML(text + "\n\n" + html.EscapeString(extraMarkdown)), nil } // IssueComment implements PayloadConvertor IssueComment method func (t telegramConvertor) IssueComment(p *api.IssueCommentPayload) (TelegramPayload, error) { text, _, _ := getIssueCommentPayloadInfo(p, htmlLinkFormatter, true) - - return createTelegramPayload(text + "\n" + p.Comment.Body), nil + return createTelegramPayloadHTML(text + "\n" + html.EscapeString(p.Comment.Body)), nil } // PullRequest implements PayloadConvertor PullRequest method func (t telegramConvertor) PullRequest(p *api.PullRequestPayload) (TelegramPayload, error) { - text, _, attachmentText, _ := getPullRequestPayloadInfo(p, htmlLinkFormatter, true) - - return createTelegramPayload(text + "\n" + attachmentText), nil + text, _, extraMarkdown, _ := getPullRequestPayloadInfo(p, htmlLinkFormatter, true) + return createTelegramPayloadHTML(text + "\n" + html.EscapeString(extraMarkdown)), nil } // Review implements PayloadConvertor Review method func (t telegramConvertor) Review(p *api.PullRequestPayload, event webhook_module.HookEventType) (TelegramPayload, error) { - var text, attachmentText string + var text, extraMarkdown string switch p.Action { case api.HookIssueReviewed: action, err := parseHookPullRequestEventType(event) @@ -138,11 +133,11 @@ func (t telegramConvertor) Review(p *api.PullRequestPayload, event webhook_modul return TelegramPayload{}, err } - text = fmt.Sprintf("[%s] Pull request review %s: #%d %s", p.Repository.FullName, action, p.Index, p.PullRequest.Title) - attachmentText = p.Review.Content + text = fmt.Sprintf("[%s] Pull request review %s: #%d %s", html.EscapeString(p.Repository.FullName), html.EscapeString(action), p.Index, html.EscapeString(p.PullRequest.Title)) + extraMarkdown = p.Review.Content } - return createTelegramPayload(text + "\n" + attachmentText), nil + return createTelegramPayloadHTML(text + "\n" + html.EscapeString(extraMarkdown)), nil } // Repository implements PayloadConvertor Repository method @@ -150,11 +145,11 @@ func (t telegramConvertor) Repository(p *api.RepositoryPayload) (TelegramPayload var title string switch p.Action { case api.HookRepoCreated: - title = fmt.Sprintf(`[%s] Repository created`, p.Repository.HTMLURL, p.Repository.FullName) - return createTelegramPayload(title), nil + title = fmt.Sprintf(`[%s] Repository created`, htmlLinkFormatter(p.Repository.HTMLURL, p.Repository.FullName)) + return createTelegramPayloadHTML(title), nil case api.HookRepoDeleted: - title = fmt.Sprintf("[%s] Repository deleted", p.Repository.FullName) - return createTelegramPayload(title), nil + title = fmt.Sprintf("[%s] Repository deleted", html.EscapeString(p.Repository.FullName)) + return createTelegramPayloadHTML(title), nil } return TelegramPayload{}, nil } @@ -163,34 +158,32 @@ func (t telegramConvertor) Repository(p *api.RepositoryPayload) (TelegramPayload func (t telegramConvertor) Wiki(p *api.WikiPayload) (TelegramPayload, error) { text, _, _ := getWikiPayloadInfo(p, htmlLinkFormatter, true) - return createTelegramPayload(text), nil + return createTelegramPayloadHTML(text), nil } // Release implements PayloadConvertor Release method func (t telegramConvertor) Release(p *api.ReleasePayload) (TelegramPayload, error) { text, _ := getReleasePayloadInfo(p, htmlLinkFormatter, true) - return createTelegramPayload(text), nil + return createTelegramPayloadHTML(text), nil } func (t telegramConvertor) Package(p *api.PackagePayload) (TelegramPayload, error) { text, _ := getPackagePayloadInfo(p, htmlLinkFormatter, true) - return createTelegramPayload(text), nil + return createTelegramPayloadHTML(text), nil } -func createTelegramPayload(message string) TelegramPayload { +func createTelegramPayloadHTML(msgHTML string) TelegramPayload { + // https://core.telegram.org/bots/api#formatting-options return TelegramPayload{ - Message: strings.TrimSpace(message), + Message: strings.TrimSpace(markup.Sanitize(msgHTML)), ParseMode: "HTML", DisableWebPreview: true, } } -type telegramConvertor struct{} - -var _ payloadConvertor[TelegramPayload] = telegramConvertor{} - func newTelegramRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { - return newJSONRequest(telegramConvertor{}, w, t, true) + var pc payloadConvertor[TelegramPayload] = telegramConvertor{} + return newJSONRequest(pc, w, t, true) } diff --git a/services/webhook/telegram_test.go b/services/webhook/telegram_test.go index 2fe5161b2..7ba81f156 100644 --- a/services/webhook/telegram_test.go +++ b/services/webhook/telegram_test.go @@ -20,11 +20,12 @@ func TestTelegramPayload(t *testing.T) { tc := telegramConvertor{} t.Run("Correct webhook params", func(t *testing.T) { - p := createTelegramPayload("testMsg ") - - assert.Equal(t, "HTML", p.ParseMode) - assert.Equal(t, true, p.DisableWebPreview) - assert.Equal(t, "testMsg", p.Message) + p := createTelegramPayloadHTML(`testMsg `) + assert.Equal(t, TelegramPayload{ + Message: `testMsg`, + ParseMode: "HTML", + DisableWebPreview: true, + }, p) }) t.Run("Create", func(t *testing.T) { @@ -33,7 +34,7 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Create(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] branch test created`, pl.Message) + assert.Equal(t, `[test/repo] branch test created`, pl.Message) }) t.Run("Delete", func(t *testing.T) { @@ -42,7 +43,7 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Delete(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] branch test deleted`, pl.Message) + assert.Equal(t, `[test/repo] branch test deleted`, pl.Message) }) t.Run("Fork", func(t *testing.T) { @@ -51,7 +52,7 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Fork(p) require.NoError(t, err) - assert.Equal(t, `test/repo2 is forked to test/repo`, pl.Message) + assert.Equal(t, `test/repo2 is forked to test/repo`, pl.Message) }) t.Run("Push", func(t *testing.T) { @@ -60,7 +61,9 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Push(p) require.NoError(t, err) - assert.Equal(t, "[test/repo:test] 2 new commits\n[2020558] commit message - user1\n[2020558] commit message - user1", pl.Message) + assert.Equal(t, `[test/repo:test] 2 new commits +[2020558] commit message - user1 +[2020558] commit message - user1`, pl.Message) }) t.Run("Issue", func(t *testing.T) { @@ -70,13 +73,15 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Issue(p) require.NoError(t, err) - assert.Equal(t, "[test/repo] Issue opened: #2 crash by user1\n\nissue body", pl.Message) + assert.Equal(t, `[test/repo] Issue opened: #2 crash by user1 + +issue body`, pl.Message) p.Action = api.HookIssueClosed pl, err = tc.Issue(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] Issue closed: #2 crash by user1`, pl.Message) + assert.Equal(t, `[test/repo] Issue closed: #2 crash by user1`, pl.Message) }) t.Run("IssueComment", func(t *testing.T) { @@ -85,7 +90,8 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.IssueComment(p) require.NoError(t, err) - assert.Equal(t, "[test/repo] New comment on issue #2 crash by user1\nmore info needed", pl.Message) + assert.Equal(t, `[test/repo] New comment on issue #2 crash by user1 +more info needed`, pl.Message) }) t.Run("PullRequest", func(t *testing.T) { @@ -94,7 +100,8 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.PullRequest(p) require.NoError(t, err) - assert.Equal(t, "[test/repo] Pull request opened: #12 Fix bug by user1\nfixes bug #2", pl.Message) + assert.Equal(t, `[test/repo] Pull request opened: #12 Fix bug by user1 +fixes bug #2`, pl.Message) }) t.Run("PullRequestComment", func(t *testing.T) { @@ -103,7 +110,8 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.IssueComment(p) require.NoError(t, err) - assert.Equal(t, "[test/repo] New comment on pull request #12 Fix bug by user1\nchanges requested", pl.Message) + assert.Equal(t, `[test/repo] New comment on pull request #12 Fix bug by user1 +changes requested`, pl.Message) }) t.Run("Review", func(t *testing.T) { @@ -113,7 +121,8 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Review(p, webhook_module.HookEventPullRequestReviewApproved) require.NoError(t, err) - assert.Equal(t, "[test/repo] Pull request review approved: #12 Fix bug\ngood job", pl.Message) + assert.Equal(t, `[test/repo] Pull request review approved: #12 Fix bug +good job`, pl.Message) }) t.Run("Repository", func(t *testing.T) { @@ -122,7 +131,7 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Repository(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] Repository created`, pl.Message) + assert.Equal(t, `[test/repo] Repository created`, pl.Message) }) t.Run("Package", func(t *testing.T) { @@ -131,7 +140,7 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Package(p) require.NoError(t, err) - assert.Equal(t, `Package created: GiteaContainer:latest by user1`, pl.Message) + assert.Equal(t, `Package created: GiteaContainer:latest by user1`, pl.Message) }) t.Run("Wiki", func(t *testing.T) { @@ -141,19 +150,19 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Wiki(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] New wiki page 'index' (Wiki change comment) by user1`, pl.Message) + assert.Equal(t, `[test/repo] New wiki page 'index' (Wiki change comment) by user1`, pl.Message) p.Action = api.HookWikiEdited pl, err = tc.Wiki(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] Wiki page 'index' edited (Wiki change comment) by user1`, pl.Message) + assert.Equal(t, `[test/repo] Wiki page 'index' edited (Wiki change comment) by user1`, pl.Message) p.Action = api.HookWikiDeleted pl, err = tc.Wiki(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] Wiki page 'index' deleted by user1`, pl.Message) + assert.Equal(t, `[test/repo] Wiki page 'index' deleted by user1`, pl.Message) }) t.Run("Release", func(t *testing.T) { @@ -162,7 +171,7 @@ func TestTelegramPayload(t *testing.T) { pl, err := tc.Release(p) require.NoError(t, err) - assert.Equal(t, `[test/repo] Release created: v1.0 by user1`, pl.Message) + assert.Equal(t, `[test/repo] Release created: v1.0 by user1`, pl.Message) }) } @@ -198,5 +207,7 @@ func TestTelegramJSONPayload(t *testing.T) { var body TelegramPayload err = json.NewDecoder(req.Body).Decode(&body) assert.NoError(t, err) - assert.Equal(t, "[test/repo:test] 2 new commits\n[2020558] commit message - user1\n[2020558] commit message - user1", body.Message) + assert.Equal(t, `[test/repo:test] 2 new commits +[2020558] commit message - user1 +[2020558] commit message - user1`, body.Message) } diff --git a/services/webhook/wechatwork.go b/services/webhook/wechatwork.go index 2e9d31cb7..1d8c1d7da 100644 --- a/services/webhook/wechatwork.go +++ b/services/webhook/wechatwork.go @@ -41,6 +41,8 @@ func newWechatworkMarkdownPayload(title string) WechatworkPayload { } } +type wechatworkConvertor struct{} + // Create implements PayloadConvertor Create method func (wc wechatworkConvertor) Create(p *api.CreatePayload) (WechatworkPayload, error) { // created tag/branch @@ -97,9 +99,9 @@ func (wc wechatworkConvertor) Push(p *api.PushPayload) (WechatworkPayload, error // Issue implements PayloadConvertor Issue method func (wc wechatworkConvertor) Issue(p *api.IssuePayload) (WechatworkPayload, error) { - text, issueTitle, attachmentText, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true) + text, issueTitle, extraMarkdown, _ := getIssuesPayloadInfo(p, noneLinkFormatter, true) var content string - content += fmt.Sprintf(" >%s\n >%s \n > %s \n [%s](%s)", text, attachmentText, issueTitle, p.Issue.HTMLURL, p.Issue.HTMLURL) + content += fmt.Sprintf(" >%s\n >%s \n > %s \n [%s](%s)", text, extraMarkdown, issueTitle, p.Issue.HTMLURL, p.Issue.HTMLURL) return newWechatworkMarkdownPayload(content), nil } @@ -115,9 +117,9 @@ func (wc wechatworkConvertor) IssueComment(p *api.IssueCommentPayload) (Wechatwo // PullRequest implements PayloadConvertor PullRequest method func (wc wechatworkConvertor) PullRequest(p *api.PullRequestPayload) (WechatworkPayload, error) { - text, issueTitle, attachmentText, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true) + text, issueTitle, extraMarkdown, _ := getPullRequestPayloadInfo(p, noneLinkFormatter, true) pr := fmt.Sprintf("> %s \r\n > %s \r\n > %s \r\n", - text, issueTitle, attachmentText) + text, issueTitle, extraMarkdown) return newWechatworkMarkdownPayload(pr), nil } @@ -173,10 +175,7 @@ func (wc wechatworkConvertor) Package(p *api.PackagePayload) (WechatworkPayload, return newWechatworkMarkdownPayload(text), nil } -type wechatworkConvertor struct{} - -var _ payloadConvertor[WechatworkPayload] = wechatworkConvertor{} - func newWechatworkRequest(_ context.Context, w *webhook_model.Webhook, t *webhook_model.HookTask) (*http.Request, []byte, error) { - return newJSONRequest(wechatworkConvertor{}, w, t, true) + var pc payloadConvertor[WechatworkPayload] = wechatworkConvertor{} + return newJSONRequest(pc, w, t, true) }