From b2db1a7fe00131bfb4b3b9ff413050ea15e73738 Mon Sep 17 00:00:00 2001 From: Felix Sommer Date: Sun, 16 Feb 2025 07:15:18 +0100 Subject: [PATCH 01/10] feat: Add support for embedding images in emails as base64 data URIs --- custom/conf/app.example.ini | 6 ++ modules/setting/mailer.go | 24 ++++--- services/mailer/mail.go | 121 ++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 10 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 899209874f7cc..57c66dddbba55 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1767,6 +1767,12 @@ LEVEL = Info ;; ;; convert \r\n to \n for Sendmail ;SENDMAIL_CONVERT_CRLF = true +;; +;; convert links of attached images to inline images. Only for images hosted in this gitea instance. +;BASE64_EMBED_IMAGES = false +;; +;; The maximum size of sum of all images in a single email. Default is 9.5MB +;BASE64_EMBED_IMAGES_MAX_SIZE_PER_EMAIL = 9961472 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index 4c3dff6850947..a850ee3c0c0c3 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -13,21 +13,23 @@ import ( "code.gitea.io/gitea/modules/log" - shellquote "github.com/kballard/go-shellquote" + "github.com/kballard/go-shellquote" ) // Mailer represents mail service. type Mailer struct { // Mailer - Name string `ini:"NAME"` - From string `ini:"FROM"` - EnvelopeFrom string `ini:"ENVELOPE_FROM"` - OverrideEnvelopeFrom bool `ini:"-"` - FromName string `ini:"-"` - FromEmail string `ini:"-"` - SendAsPlainText bool `ini:"SEND_AS_PLAIN_TEXT"` - SubjectPrefix string `ini:"SUBJECT_PREFIX"` - OverrideHeader map[string][]string `ini:"-"` + Name string `ini:"NAME"` + From string `ini:"FROM"` + EnvelopeFrom string `ini:"ENVELOPE_FROM"` + OverrideEnvelopeFrom bool `ini:"-"` + FromName string `ini:"-"` + FromEmail string `ini:"-"` + SendAsPlainText bool `ini:"SEND_AS_PLAIN_TEXT"` + SubjectPrefix string `ini:"SUBJECT_PREFIX"` + OverrideHeader map[string][]string `ini:"-"` + Base64EmbedImages bool `ini:"BASE64_EMBED_IMAGES"` + Base64EmbedImagesMaxSizePerEmail int64 `ini:"BASE64_EMBED_IMAGES_MAX_SIZE_PER_EMAIL"` // SMTP sender Protocol string `ini:"PROTOCOL"` @@ -150,6 +152,8 @@ func loadMailerFrom(rootCfg ConfigProvider) { sec.Key("SENDMAIL_TIMEOUT").MustDuration(5 * time.Minute) sec.Key("SENDMAIL_CONVERT_CRLF").MustBool(true) sec.Key("FROM").MustString(sec.Key("USER").String()) + sec.Key("BASE64_EMBED_IMAGES").MustBool(false) + sec.Key("BASE64_EMBED_IMAGES_MAX_SIZE_PER_EMAIL").MustInt64(9.5 * 1024 * 1024) // Now map the values on to the MailService MailService = &Mailer{} diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 52e19bde6f261..8bf991da13d87 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -7,9 +7,12 @@ package mailer import ( "bytes" "context" + "encoding/base64" "fmt" "html/template" + "io" "mime" + "net/http" "regexp" "strconv" "strings" @@ -18,19 +21,24 @@ import ( activities_model "code.gitea.io/gitea/models/activities" issues_model "code.gitea.io/gitea/models/issues" + access_model "code.gitea.io/gitea/models/perm/access" "code.gitea.io/gitea/models/renderhelper" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/emoji" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/translation" incoming_payload "code.gitea.io/gitea/services/mailer/incoming/payload" sender_service "code.gitea.io/gitea/services/mailer/sender" "code.gitea.io/gitea/services/mailer/token" + + "golang.org/x/net/html" ) const ( @@ -228,6 +236,15 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient return nil, err } + if setting.MailService.Base64EmbedImages { + bodyStr := string(body) + bodyStr, err = Base64InlineImages(bodyStr, ctx) + if err != nil { + return nil, err + } + body = template.HTML(bodyStr) + } + actType, actName, tplName := actionToTemplate(ctx.Issue, ctx.ActionType, commentType, reviewType) if actName != "new" { @@ -359,6 +376,110 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient return msgs, nil } +func Base64InlineImages(body string, ctx *mailCommentContext) (string, error) { + doc, err := html.Parse(strings.NewReader(body)) + if err != nil { + log.Error("Failed to parse HTML body: %v", err) + return "", err + } + + var totalEmbeddedImagesSize int64 + + var processNode func(*html.Node) + processNode = func(n *html.Node) { + if n.Type == html.ElementNode { + if n.Data == "img" { + for i, attr := range n.Attr { + if attr.Key == "src" { + attachmentPath := attr.Val + dataURI, err := AttachmentSrcToBase64DataURI(attachmentPath, ctx, &totalEmbeddedImagesSize) + if err != nil { + log.Trace("attachmentSrcToDataURI not possible: %v", err) // Not an error, just skip. This is probably an image from outside the gitea instance. + continue + } + log.Trace("Old value of src attribute: %s, new value (first 100 characters): %s", attr.Val, dataURI[:100]) + n.Attr[i].Val = dataURI + break + } + } + } + } + + for c := n.FirstChild; c != nil; c = c.NextSibling { + processNode(c) + } + } + + processNode(doc) + + var buf bytes.Buffer + err = html.Render(&buf, doc) + if err != nil { + log.Error("Failed to render modified HTML: %v", err) + return "", err + } + return buf.String(), nil +} + +func AttachmentSrcToBase64DataURI(attachmentPath string, ctx *mailCommentContext, totalEmbeddedImagesSize *int64) (string, error) { + if !strings.HasPrefix(attachmentPath, setting.AppURL) { // external image + return "", fmt.Errorf("external image") + } + parts := strings.Split(attachmentPath, "/attachments/") + if len(parts) <= 1 { + return "", fmt.Errorf("invalid attachment path: %s", attachmentPath) + } + + attachmentUUID := parts[len(parts)-1] + attachment, err := repo_model.GetAttachmentByUUID(ctx, attachmentUUID) + if err != nil { + return "", err + } + + // "Doer" is theoretically not the correct permission check (as Doer created the action on which to send), but as this is batch processed the receipants can't be accessed. + // Therefore we check the Doer, with which we counter leaking information as a Doer brute force attack on attachments would be possible. + perm, err := access_model.GetUserRepoPermission(ctx, ctx.Issue.Repo, ctx.Doer) + if err != nil { + return "", err + } + if !perm.CanRead(unit.TypeIssues) { + return "", fmt.Errorf("no permission") + } + + fr, err := storage.Attachments.Open(attachment.RelativePath()) + if err != nil { + return "", err + } + defer fr.Close() + + maxSize := setting.MailService.Base64EmbedImagesMaxSizePerEmail // at maximum read the whole available combined email size, to prevent maliciously large file reads + + lr := &io.LimitedReader{R: fr, N: maxSize + 1} + content, err := io.ReadAll(lr) + if err != nil { + return "", err + } + if len(content) > int(maxSize) { + return "", fmt.Errorf("file size exceeds the embedded image max limit \\(%d bytes\\)", maxSize) + } + + if *totalEmbeddedImagesSize+int64(len(content)) > setting.MailService.Base64EmbedImagesMaxSizePerEmail { + return "", fmt.Errorf("total embedded images exceed max limit: %d > %d", *totalEmbeddedImagesSize+int64(len(content)), setting.MailService.Base64EmbedImagesMaxSizePerEmail) + } + *totalEmbeddedImagesSize += int64(len(content)) + + mimeType := http.DetectContentType(content) + + if !strings.HasPrefix(mimeType, "image/") { + return "", fmt.Errorf("not an image") + } + + encoded := base64.StdEncoding.EncodeToString(content) + dataURI := fmt.Sprintf("data:%s;base64,%s", mimeType, encoded) + + return dataURI, nil +} + func generateMessageIDForIssue(issue *issues_model.Issue, comment *issues_model.Comment, actionType activities_model.ActionType) string { var path string if issue.IsPull { From 5bf5f388d13824059ee7c2ce4a7b749f1c25e6ad Mon Sep 17 00:00:00 2001 From: Felix Sommer Date: Sun, 16 Feb 2025 07:15:31 +0100 Subject: [PATCH 02/10] tests, including an attachment (gitea.png) in an issue --- models/fixtures/attachment.yml | 13 ++ models/fixtures/issue.yml | 17 +++ models/fixtures/issue_index.yml | 2 +- models/fixtures/repository.yml | 2 +- models/issues/issue_test.go | 10 +- models/issues/issue_user_test.go | 2 +- models/repo/attachment_test.go | 2 +- modules/indexer/issues/indexer_test.go | 26 ++-- services/issue/issue_test.go | 4 +- services/issue/suggestion_test.go | 4 +- services/mailer/mail_test.go | 132 ++++++++++++++++++ tests/integration/api_issue_test.go | 10 +- tests/integration/api_nodeinfo_test.go | 2 +- tests/integration/api_repo_test.go | 2 +- tests/integration/issue_test.go | 8 +- tests/integration/pull_merge_test.go | 4 +- .../1/b/1b267670-1793-4cd0-abc1-449269b7cff9 | Bin 0 -> 240 bytes 17 files changed, 201 insertions(+), 39 deletions(-) create mode 100644 tests/testdata/data/attachments/1/b/1b267670-1793-4cd0-abc1-449269b7cff9 diff --git a/models/fixtures/attachment.yml b/models/fixtures/attachment.yml index 7882d8bff2089..f6b5393b4dc65 100644 --- a/models/fixtures/attachment.yml +++ b/models/fixtures/attachment.yml @@ -153,3 +153,16 @@ download_count: 0 size: 0 created_unix: 946684800 + +- + id: 13 + uuid: 1b267670-1793-4cd0-abc1-449269b7cff9 + repo_id: 1 + issue_id: 23 + release_id: 0 + uploader_id: 0 + comment_id: 2 + name: gitea.png + download_count: 0 + size: 1458 + created_unix: 946684800 diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index ca5b1c6cd1df5..c829c755296ed 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -372,3 +372,20 @@ created_unix: 1707270422 updated_unix: 1707270422 is_locked: false + +- + id: 23 + repo_id: 1 + index: 6 + poster_id: 1 + original_author_id: 0 + name: issue23 + content: 'content including this image: gitea.png with some more content behind it' + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: false + num_comments: 0 + created_unix: 946684801 + updated_unix: 978307201 + is_locked: false diff --git a/models/fixtures/issue_index.yml b/models/fixtures/issue_index.yml index 5aabc08e388c5..c1e0b546a4013 100644 --- a/models/fixtures/issue_index.yml +++ b/models/fixtures/issue_index.yml @@ -1,6 +1,6 @@ - group_id: 1 - max_index: 5 + max_index: 6 - group_id: 2 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 552a78cbd2773..47f9cb8a5d012 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -9,7 +9,7 @@ num_watches: 4 num_stars: 0 num_forks: 0 - num_issues: 2 + num_issues: 3 num_closed_issues: 1 num_pulls: 3 num_closed_pulls: 0 diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index dbbb1e417901f..77c387d0e0d40 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -57,7 +57,7 @@ func Test_GetIssueIDsByRepoID(t *testing.T) { ids, err := issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) - assert.Len(t, ids, 5) + assert.Len(t, ids, 6) } func TestIssueAPIURL(t *testing.T) { @@ -170,7 +170,7 @@ func TestIssues(t *testing.T) { PageSize: 4, }, }, - []int64{1, 2, 3, 5}, + []int64{1, 23, 2, 3}, }, { issues_model.IssuesOptions{ @@ -249,11 +249,11 @@ func TestIssue_InsertIssue(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) // there are 5 issues and max index is 5 on repository 1, so this one should 6 - issue := testInsertIssue(t, "my issue1", "special issue's comments?", 6) + issue := testInsertIssue(t, "my issue1", "special issue's comments?", 7) _, err := db.DeleteByID[issues_model.Issue](db.DefaultContext, issue.ID) assert.NoError(t, err) - issue = testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?", 7) + issue = testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?", 8) _, err = db.DeleteByID[issues_model.Issue](db.DefaultContext, issue.ID) assert.NoError(t, err) } @@ -380,7 +380,7 @@ func TestCountIssues(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{}) assert.NoError(t, err) - assert.EqualValues(t, 22, count) + assert.EqualValues(t, 23, count) } func TestIssueLoadAttributes(t *testing.T) { diff --git a/models/issues/issue_user_test.go b/models/issues/issue_user_test.go index 7c21aa15eef6a..fef599a36641a 100644 --- a/models/issues/issue_user_test.go +++ b/models/issues/issue_user_test.go @@ -22,7 +22,7 @@ func Test_NewIssueUsers(t *testing.T) { newIssue := &issues_model.Issue{ RepoID: repo.ID, PosterID: 4, - Index: 6, + Index: 7, Title: "newTestIssueTitle", Content: "newTestIssueContent", } diff --git a/models/repo/attachment_test.go b/models/repo/attachment_test.go index c059ffd39a91e..48313aa382235 100644 --- a/models/repo/attachment_test.go +++ b/models/repo/attachment_test.go @@ -51,7 +51,7 @@ func TestDeleteAttachments(t *testing.T) { count, err = repo_model.DeleteAttachmentsByComment(db.DefaultContext, 2, false) assert.NoError(t, err) - assert.Equal(t, 2, count) + assert.Equal(t, 3, count) err = repo_model.DeleteAttachment(db.DefaultContext, &repo_model.Attachment{ID: 8}, false) assert.NoError(t, err) diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index 8043d33eebb88..736936012eb80 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -57,7 +57,7 @@ func searchIssueWithKeyword(t *testing.T) { Keyword: "issue2", RepoIDs: []int64{1}, }, - []int64{2}, + []int64{2, 23}, }, { SearchOptions{ @@ -106,7 +106,7 @@ func searchIssueByIndex(t *testing.T) { Keyword: "2", RepoIDs: []int64{1, 2, 3, 32}, }, - []int64{17, 12, 7, 2}, + []int64{17, 12, 7, 2, 23}, }, { SearchOptions{ @@ -133,7 +133,7 @@ func searchIssueInRepo(t *testing.T) { SearchOptions{ RepoIDs: []int64{1}, }, - []int64{11, 5, 3, 2, 1}, + []int64{11, 5, 3, 2, 23, 1}, }, { SearchOptions{ @@ -177,7 +177,7 @@ func searchIssueByID(t *testing.T) { opts: SearchOptions{ PosterID: optional.Some(int64(1)), }, - expectedIDs: []int64{11, 6, 3, 2, 1}, + expectedIDs: []int64{11, 6, 3, 2, 23, 1}, }, { opts: SearchOptions{ @@ -188,7 +188,7 @@ func searchIssueByID(t *testing.T) { { // NOTE: This tests no assignees filtering and also ToSearchOptions() to ensure it will set AssigneeID to 0 when it is passed as -1. opts: *ToSearchOptions("", &issues.IssuesOptions{AssigneeID: optional.Some(db.NoConditionID)}), - expectedIDs: []int64{22, 21, 16, 15, 14, 13, 12, 11, 20, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2}, + expectedIDs: []int64{22, 21, 16, 15, 14, 13, 12, 11, 20, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 23}, }, { opts: SearchOptions{ @@ -212,7 +212,7 @@ func searchIssueByID(t *testing.T) { opts: SearchOptions{ SubscriberID: optional.Some(int64(1)), }, - expectedIDs: []int64{11, 6, 5, 3, 2, 1}, + expectedIDs: []int64{11, 6, 5, 3, 2, 23, 1}, }, { // issue 20 request user 15 and team 5 which user 15 belongs to @@ -247,7 +247,7 @@ func searchIssueIsPull(t *testing.T) { SearchOptions{ IsPull: optional.Some(false), }, - []int64{17, 16, 15, 14, 13, 6, 5, 18, 10, 7, 4, 1}, + []int64{17, 16, 15, 14, 13, 6, 5, 18, 10, 7, 4, 23, 1}, }, { SearchOptions{ @@ -272,7 +272,7 @@ func searchIssueIsClosed(t *testing.T) { SearchOptions{ IsClosed: optional.Some(false), }, - []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 1}, + []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 23, 1}, }, { SearchOptions{ @@ -297,7 +297,7 @@ func searchIssueIsArchived(t *testing.T) { SearchOptions{ IsArchived: optional.Some(false), }, - []int64{22, 21, 17, 16, 15, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1}, + []int64{22, 21, 17, 16, 15, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 23, 1}, }, { SearchOptions{ @@ -359,7 +359,7 @@ func searchIssueByLabelID(t *testing.T) { SearchOptions{ ExcludedLabelIDs: []int64{1}, }, - []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3}, + []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 23}, }, } for _, test := range tests { @@ -378,7 +378,7 @@ func searchIssueByTime(t *testing.T) { SearchOptions{ UpdatedAfterUnix: optional.Some(int64(0)), }, - []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1}, + []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 23, 1}, }, } for _, test := range tests { @@ -397,7 +397,7 @@ func searchIssueWithOrder(t *testing.T) { SearchOptions{ SortBy: internal.SortByCreatedAsc, }, - []int64{1, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17, 21, 22}, + []int64{1, 23, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17, 21, 22}, }, } for _, test := range tests { @@ -451,7 +451,7 @@ func searchIssueWithPaginator(t *testing.T) { }, }, []int64{22, 21, 17, 16, 15}, - 22, + 23, }, } for _, test := range tests { diff --git a/services/issue/issue_test.go b/services/issue/issue_test.go index 8806cec0e7c5d..d7c850e6eb071 100644 --- a/services/issue/issue_test.go +++ b/services/issue/issue_test.go @@ -37,7 +37,7 @@ func TestIssue_DeleteIssue(t *testing.T) { issueIDs, err := issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) - assert.Len(t, issueIDs, 5) + assert.Len(t, issueIDs, 6) issue := &issues_model.Issue{ RepoID: 1, @@ -48,7 +48,7 @@ func TestIssue_DeleteIssue(t *testing.T) { assert.NoError(t, err) issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) - assert.Len(t, issueIDs, 4) + assert.Len(t, issueIDs, 5) // check attachment removal attachments, err := repo_model.GetAttachmentsByIssueID(db.DefaultContext, 4) diff --git a/services/issue/suggestion_test.go b/services/issue/suggestion_test.go index 84cfd520ac40a..84771694c4626 100644 --- a/services/issue/suggestion_test.go +++ b/services/issue/suggestion_test.go @@ -26,7 +26,7 @@ func Test_Suggestion(t *testing.T) { }{ { keyword: "", - expectedIndexes: []int64{5, 1, 4, 2, 3}, + expectedIndexes: []int64{5, 6, 1, 4, 2}, }, { keyword: "1", @@ -34,7 +34,7 @@ func Test_Suggestion(t *testing.T) { }, { keyword: "issue", - expectedIndexes: []int64{4, 1, 2, 3}, + expectedIndexes: []int64{6, 4, 1, 2, 3}, }, { keyword: "pull", diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 8298ac4a34281..2a2e57dbce3f4 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -10,6 +10,7 @@ import ( "html/template" "io" "mime/quotedprintable" + "path/filepath" "regexp" "strings" "testing" @@ -23,6 +24,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" sender_service "code.gitea.io/gitea/services/mailer/sender" "github.com/stretchr/testify/assert" @@ -59,6 +61,7 @@ func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Re setting.MailService = &mailService setting.Domain = "localhost" + setting.AppURL = "https://try.gitea.io/" doer = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, Owner: doer}) @@ -450,3 +453,132 @@ func TestFromDisplayName(t *testing.T) { assert.EqualValues(t, "Mister X (by Code IT on [code.it])", fromDisplayName(&user_model.User{FullName: "Mister X", Name: "tmp"})) }) } + +func PrepareAttachmentsStorage(t testing.TB) { // same as in test_utils.go + // prepare attachments directory and files + assert.NoError(t, storage.Clean(storage.Attachments)) + + s, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{ + Path: filepath.Join(filepath.Dir(setting.AppPath), "tests", "testdata", "data", "attachments"), + }) + assert.NoError(t, err) + assert.NoError(t, s.IterateObjects("", func(p string, obj storage.Object) error { + _, err = storage.Copy(storage.Attachments, p, s, p) + return err + })) +} + +func TestEmbedBase64ImagesInEmail(t *testing.T) { + // Fake context setup + doer, repo, _, _ := prepareMailerTest(t) + PrepareAttachmentsStorage(t) + setting.MailService.Base64EmbedImages = true + setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 * 1024 * 1024 + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 23, Repo: repo, Poster: doer}) + assert.NoError(t, issue.LoadRepo(db.DefaultContext)) + + subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) + bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) + + recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} + msgs, err := composeIssueCommentMessages(&mailCommentContext{ + Context: context.TODO(), // TODO: use a correct context + Issue: issue, Doer: doer, ActionType: activities_model.ActionCreateIssue, + Content: strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), + }, "en-US", recipients, false, "issue create") + + mailBody := msgs[0].Body + re := regexp.MustCompile(`(?s)(.*?)`) + matches := re.FindStringSubmatch(mailBody) + if len(matches) > 1 { + mailBody = matches[1] + } + // check if the mail body was correctly generated + assert.NoError(t, err) + assert.Contains(t, mailBody, "content including this image") + + // check if an image was embedded + assert.Contains(t, mailBody, "data:image/png;base64,") + + // check if the image was embedded only once + assert.Equal(t, 1, strings.Count(mailBody, "data:image/png;base64,")) + + img2InternalBase64 := "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAFAAAAAxAQMAAAB3d7wRAAAABlBMVEVgmyF6qkqITHmkAAAAAXRSTlMBN+Ho8AAAAJhJREFUKM+V0DsOwyAQBNCxXLjkCFwk0t7McDQfhS4tpQuEzWc/iaUU2eo1zC4DUMWYF3DxVKzGTXjBGb2RsjJEo6ZhN1Zj+cEgi/9hBQl3YflkkIsbo5IO5glKTuhPpavM3Hp4C7WdjEWYrL5GMkp/R+s4GPlh/CZn4MEwv9aHHiyD3ujm5X22eaMyDa5yAm+O0B1TPa1l3W2qZWMg+KgtAAAAAElFTkSuQmCC" + + // check if the image was embedded correctly + assert.Contains(t, mailBody, img2InternalBase64) +} + +func TestEmbedBase64Images(t *testing.T) { + user, repo, _, _ := prepareMailerTest(t) + PrepareAttachmentsStorage(t) + setting.MailService.Base64EmbedImages = true + setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 * 1024 * 1024 + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 23, Repo: repo, Poster: user}) + + attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 13, IssueID: issue.ID, RepoID: repo.ID}) + ctx0 := context.Background() + + ctx := &mailCommentContext{Context: ctx0 /* TODO: use a correct context */, Issue: issue, Doer: user} + + img1ExternalURL := "https://via.placeholder.com/10" + img1ExternalImg := "" + + img2InternalURL := setting.AppURL + repo.Owner.Name + "/" + repo.Name + "/attachments/" + attachment.UUID + img2InternalImg := "" + img2InternalBase64 := "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAFAAAAAxAQMAAAB3d7wRAAAABlBMVEVgmyF6qkqITHmkAAAAAXRSTlMBN+Ho8AAAAJhJREFUKM+V0DsOwyAQBNCxXLjkCFwk0t7McDQfhS4tpQuEzWc/iaUU2eo1zC4DUMWYF3DxVKzGTXjBGb2RsjJEo6ZhN1Zj+cEgi/9hBQl3YflkkIsbo5IO5glKTuhPpavM3Hp4C7WdjEWYrL5GMkp/R+s4GPlh/CZn4MEwv9aHHiyD3ujm5X22eaMyDa5yAm+O0B1TPa1l3W2qZWMg+KgtAAAAAElFTkSuQmCC" + img2InternalBase64Img := "" + + // 1st Test: convert internal image to base64 + t.Run("replaceSpecifiedBase64ImagesInternal", func(t *testing.T) { + totalEmbeddedImagesSize := int64(0) + + resultImg1Internal, err := AttachmentSrcToBase64DataURI(img2InternalURL, ctx, &totalEmbeddedImagesSize) + assert.NoError(t, err) + assert.Equal(t, img2InternalBase64, resultImg1Internal) // replace cause internal image + }) + + // 2nd Test: convert external image to base64 -> abort cause external image + t.Run("replaceSpecifiedBase64ImagesExternal", func(t *testing.T) { + totalEmbeddedImagesSize := int64(0) + + resultImg1External, err := AttachmentSrcToBase64DataURI(img1ExternalURL, ctx, &totalEmbeddedImagesSize) + assert.Error(t, err) + assert.Equal(t, "", resultImg1External) // don't replace cause external image + }) + + // 3rd Test: generate email body with 1 internal and 1 external image, expect the result to have the internal image replaced with base64 data and the external not replaced + t.Run("generateEmailBody", func(t *testing.T) { + mailBody := "

Test1

" + img1ExternalImg + "

Test2

" + img2InternalImg + "

Test3

" + expectedMailBody := "

Test1

" + img1ExternalImg + "

Test2

" + img2InternalBase64Img + "

Test3

" + resultMailBody, err := Base64InlineImages(mailBody, ctx) + + assert.NoError(t, err) + assert.Equal(t, expectedMailBody, resultMailBody) + }) + + // 4th Test, generate email body with 2 internal images, but set Mailer.Base64EmbedImagesMaxSizePerEmail to the size of the first image (+1), expect the first image to be replaced and the second not + t.Run("generateEmailBodyWithMaxSize", func(t *testing.T) { + setting.MailService.Base64EmbedImagesMaxSizePerEmail = int64(len(img2InternalBase64) + 1) + + mailBody := "

Test1

" + img2InternalImg + "

Test2

" + img2InternalImg + "

Test3

" + expectedMailBody := "

Test1

" + img2InternalBase64Img + "

Test2

" + img2InternalImg + "

Test3

" + resultMailBody, err := Base64InlineImages(mailBody, ctx) + + assert.NoError(t, err) + assert.Equal(t, expectedMailBody, resultMailBody) + }) + + // 5th Test, generate email body with 3 internal images, but set Mailer.Base64EmbedImagesMaxSizePerEmail to the size of all 3 images (+1), expect all images to be replaced + t.Run("generateEmailBodyWith3Images", func(t *testing.T) { + setting.MailService.Base64EmbedImagesMaxSizePerEmail = int64(len(img2InternalBase64)*3 + 1) + + mailBody := "

Test1

" + img2InternalImg + "

Test2

" + img2InternalImg + "

Test3

" + img2InternalImg + "" + expectedMailBody := "

Test1

" + img2InternalBase64Img + "

Test2

" + img2InternalBase64Img + "

Test3

" + img2InternalBase64Img + "" + resultMailBody, err := Base64InlineImages(mailBody, ctx) + + assert.NoError(t, err) + assert.Equal(t, expectedMailBody, resultMailBody) + }) +} diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index d8394a33d96f4..8b81b7196f43c 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -287,7 +287,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(publicOnlyToken) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 15) // 15 public issues + assert.Len(t, apiIssues, 16) // 16 public issues since := "2000-01-01T00:50:01+00:00" // 946687801 before := time.Unix(999307200, 0).Format(time.RFC3339) @@ -297,7 +297,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 11) + assert.Len(t, apiIssues, 12) query.Del("since") query.Del("before") @@ -313,7 +313,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 20) query.Add("limit", "10") @@ -321,7 +321,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 10) query = url.Values{"assigned": {"true"}, "state": {"all"}} @@ -350,7 +350,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 8) + assert.Len(t, apiIssues, 9) query = url.Values{"owner": {"org3"}} // organization link.RawQuery = query.Encode() diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go index 75f8dbb4ba3aa..c6b4663b4a4d9 100644 --- a/tests/integration/api_nodeinfo_test.go +++ b/tests/integration/api_nodeinfo_test.go @@ -33,7 +33,7 @@ func TestNodeinfo(t *testing.T) { assert.True(t, nodeinfo.OpenRegistrations) assert.Equal(t, "gitea", nodeinfo.Software.Name) assert.Equal(t, 29, nodeinfo.Usage.Users.Total) - assert.Equal(t, 22, nodeinfo.Usage.LocalPosts) + assert.Equal(t, 23, nodeinfo.Usage.LocalPosts) assert.Equal(t, 3, nodeinfo.Usage.LocalComments) }) } diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 22f26d87d4332..5180e3d17ee57 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -268,7 +268,7 @@ func TestAPIViewRepo(t *testing.T) { assert.EqualValues(t, 1, repo.ID) assert.EqualValues(t, "repo1", repo.Name) assert.EqualValues(t, 2, repo.Releases) - assert.EqualValues(t, 1, repo.OpenIssues) + assert.EqualValues(t, 2, repo.OpenIssues) assert.EqualValues(t, 3, repo.OpenPulls) req = NewRequest(t, "GET", "/api/v1/repos/user12/repo10") diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index bd0cedd300baf..d66e78c9ae4c6 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -495,7 +495,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 11) + assert.Len(t, apiIssues, 12) query.Del("since") query.Del("before") @@ -511,7 +511,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 20) query.Add("limit", "5") @@ -519,7 +519,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 5) query = url.Values{"assigned": {"true"}, "state": {"all"}} @@ -548,7 +548,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 8) + assert.Len(t, apiIssues, 9) query = url.Values{"owner": {"org3"}} // organization link.RawQuery = query.Encode() diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 169df8618e89d..6128b8d8a72d4 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -924,7 +924,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing. Run(&git.RunOpts{Dir: dstPath, Stderr: stderrBuf}) assert.NoError(t, err) - assert.Contains(t, stderrBuf.String(), setting.AppURL+"user2/repo1/pulls/6") + assert.Contains(t, stderrBuf.String(), setting.AppURL+"user2/repo1/pulls/7") baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ @@ -1044,7 +1044,7 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) { token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - mergeReq := NewRequestWithValues(t, "POST", "/api/v1/repos/user2/repo1/pulls/6/merge", map[string]string{ + mergeReq := NewRequestWithValues(t, "POST", "/api/v1/repos/user2/repo1/pulls/7/merge", map[string]string{ "_csrf": csrf, "head_commit_id": "", "merge_when_checks_succeed": "false", diff --git a/tests/testdata/data/attachments/1/b/1b267670-1793-4cd0-abc1-449269b7cff9 b/tests/testdata/data/attachments/1/b/1b267670-1793-4cd0-abc1-449269b7cff9 new file mode 100644 index 0000000000000000000000000000000000000000..8fc22b562f026901bbb803be54828eaedf7797c1 GIT binary patch literal 240 zcmeAS@N?(olHy`uVBq!ia0vp^0YGfX$P6UQ%l8NZDYgKg5Z8p+idCz;I(#aZ07V!} zg8YIR%^$w_0OZW@ba4#PI6w7*HQ!+c0hS9JV|F~@h*7z8?@WP-e5;=BQtpGqvV zMQ*+_J)_4QaCC-v!N-s_$9yXeO75Mw$;f5#vPAQ+y8kD#a+W9lOqtLvy?7Ge zGfpqR7ye6EpSe?2!M$~EkL!#z``nDY>fK*kNc>Fvqn7^QpuzrY?Q%NJ_g*}ETDz@s nu@UdOBBuPl3$nqsYg6y$u1ZZ-__0D4=tc%lS3j3^P6 Date: Sun, 2 Mar 2025 21:42:58 +0800 Subject: [PATCH 03/10] revert unnecessary tests --- models/fixtures/attachment.yml | 13 ------------- models/fixtures/issue.yml | 17 ----------------- models/fixtures/issue_index.yml | 2 +- models/fixtures/repository.yml | 2 +- models/issues/issue_test.go | 10 +++++----- models/issues/issue_user_test.go | 2 +- models/repo/attachment_test.go | 2 +- modules/indexer/issues/indexer_test.go | 26 +++++++++++++------------- services/issue/issue_test.go | 4 ++-- services/issue/suggestion_test.go | 4 ++-- tests/integration/api_issue_test.go | 10 +++++----- tests/integration/api_nodeinfo_test.go | 2 +- tests/integration/api_repo_test.go | 2 +- tests/integration/issue_test.go | 8 ++++---- tests/integration/pull_merge_test.go | 4 ++-- 15 files changed, 39 insertions(+), 69 deletions(-) diff --git a/models/fixtures/attachment.yml b/models/fixtures/attachment.yml index f6b5393b4dc65..7882d8bff2089 100644 --- a/models/fixtures/attachment.yml +++ b/models/fixtures/attachment.yml @@ -153,16 +153,3 @@ download_count: 0 size: 0 created_unix: 946684800 - -- - id: 13 - uuid: 1b267670-1793-4cd0-abc1-449269b7cff9 - repo_id: 1 - issue_id: 23 - release_id: 0 - uploader_id: 0 - comment_id: 2 - name: gitea.png - download_count: 0 - size: 1458 - created_unix: 946684800 diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index c829c755296ed..ca5b1c6cd1df5 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -372,20 +372,3 @@ created_unix: 1707270422 updated_unix: 1707270422 is_locked: false - -- - id: 23 - repo_id: 1 - index: 6 - poster_id: 1 - original_author_id: 0 - name: issue23 - content: 'content including this image: gitea.png with some more content behind it' - milestone_id: 0 - priority: 0 - is_closed: false - is_pull: false - num_comments: 0 - created_unix: 946684801 - updated_unix: 978307201 - is_locked: false diff --git a/models/fixtures/issue_index.yml b/models/fixtures/issue_index.yml index c1e0b546a4013..5aabc08e388c5 100644 --- a/models/fixtures/issue_index.yml +++ b/models/fixtures/issue_index.yml @@ -1,6 +1,6 @@ - group_id: 1 - max_index: 6 + max_index: 5 - group_id: 2 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 47f9cb8a5d012..552a78cbd2773 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -9,7 +9,7 @@ num_watches: 4 num_stars: 0 num_forks: 0 - num_issues: 3 + num_issues: 2 num_closed_issues: 1 num_pulls: 3 num_closed_pulls: 0 diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index c2ba161ee2de2..3f76a81bb65d4 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -56,7 +56,7 @@ func Test_GetIssueIDsByRepoID(t *testing.T) { ids, err := issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) - assert.Len(t, ids, 6) + assert.Len(t, ids, 5) } func TestIssueAPIURL(t *testing.T) { @@ -169,7 +169,7 @@ func TestIssues(t *testing.T) { PageSize: 4, }, }, - []int64{1, 23, 2, 3}, + []int64{1, 2, 3, 5}, }, { issues_model.IssuesOptions{ @@ -248,11 +248,11 @@ func TestIssue_InsertIssue(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) // there are 5 issues and max index is 5 on repository 1, so this one should 6 - issue := testInsertIssue(t, "my issue1", "special issue's comments?", 7) + issue := testInsertIssue(t, "my issue1", "special issue's comments?", 6) _, err := db.DeleteByID[issues_model.Issue](db.DefaultContext, issue.ID) assert.NoError(t, err) - issue = testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?", 8) + issue = testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?", 7) _, err = db.DeleteByID[issues_model.Issue](db.DefaultContext, issue.ID) assert.NoError(t, err) } @@ -379,7 +379,7 @@ func TestCountIssues(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{}) assert.NoError(t, err) - assert.EqualValues(t, 23, count) + assert.EqualValues(t, 22, count) } func TestIssueLoadAttributes(t *testing.T) { diff --git a/models/issues/issue_user_test.go b/models/issues/issue_user_test.go index fef599a36641a..7c21aa15eef6a 100644 --- a/models/issues/issue_user_test.go +++ b/models/issues/issue_user_test.go @@ -22,7 +22,7 @@ func Test_NewIssueUsers(t *testing.T) { newIssue := &issues_model.Issue{ RepoID: repo.ID, PosterID: 4, - Index: 7, + Index: 6, Title: "newTestIssueTitle", Content: "newTestIssueContent", } diff --git a/models/repo/attachment_test.go b/models/repo/attachment_test.go index 48313aa382235..c059ffd39a91e 100644 --- a/models/repo/attachment_test.go +++ b/models/repo/attachment_test.go @@ -51,7 +51,7 @@ func TestDeleteAttachments(t *testing.T) { count, err = repo_model.DeleteAttachmentsByComment(db.DefaultContext, 2, false) assert.NoError(t, err) - assert.Equal(t, 3, count) + assert.Equal(t, 2, count) err = repo_model.DeleteAttachment(db.DefaultContext, &repo_model.Attachment{ID: 8}, false) assert.NoError(t, err) diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index bbb171d4b71ec..7def2a2c6e5b9 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -56,7 +56,7 @@ func searchIssueWithKeyword(t *testing.T) { Keyword: "issue2", RepoIDs: []int64{1}, }, - []int64{2, 23}, + []int64{2}, }, { SearchOptions{ @@ -105,7 +105,7 @@ func searchIssueByIndex(t *testing.T) { Keyword: "2", RepoIDs: []int64{1, 2, 3, 32}, }, - []int64{17, 12, 7, 2, 23}, + []int64{17, 12, 7, 2}, }, { SearchOptions{ @@ -132,7 +132,7 @@ func searchIssueInRepo(t *testing.T) { SearchOptions{ RepoIDs: []int64{1}, }, - []int64{11, 5, 3, 2, 23, 1}, + []int64{11, 5, 3, 2, 1}, }, { SearchOptions{ @@ -176,7 +176,7 @@ func searchIssueByID(t *testing.T) { opts: SearchOptions{ PosterID: optional.Some(int64(1)), }, - expectedIDs: []int64{11, 6, 3, 2, 23, 1}, + expectedIDs: []int64{11, 6, 3, 2, 1}, }, { opts: SearchOptions{ @@ -187,7 +187,7 @@ func searchIssueByID(t *testing.T) { { // NOTE: This tests no assignees filtering and also ToSearchOptions() to ensure it will set AssigneeID to 0 when it is passed as -1. opts: *ToSearchOptions("", &issues.IssuesOptions{AssigneeID: optional.Some(db.NoConditionID)}), - expectedIDs: []int64{22, 21, 16, 15, 14, 13, 12, 11, 20, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 23}, + expectedIDs: []int64{22, 21, 16, 15, 14, 13, 12, 11, 20, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2}, }, { opts: SearchOptions{ @@ -211,7 +211,7 @@ func searchIssueByID(t *testing.T) { opts: SearchOptions{ SubscriberID: optional.Some(int64(1)), }, - expectedIDs: []int64{11, 6, 5, 3, 2, 23, 1}, + expectedIDs: []int64{11, 6, 5, 3, 2, 1}, }, { // issue 20 request user 15 and team 5 which user 15 belongs to @@ -246,7 +246,7 @@ func searchIssueIsPull(t *testing.T) { SearchOptions{ IsPull: optional.Some(false), }, - []int64{17, 16, 15, 14, 13, 6, 5, 18, 10, 7, 4, 23, 1}, + []int64{17, 16, 15, 14, 13, 6, 5, 18, 10, 7, 4, 1}, }, { SearchOptions{ @@ -271,7 +271,7 @@ func searchIssueIsClosed(t *testing.T) { SearchOptions{ IsClosed: optional.Some(false), }, - []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 23, 1}, + []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 1}, }, { SearchOptions{ @@ -296,7 +296,7 @@ func searchIssueIsArchived(t *testing.T) { SearchOptions{ IsArchived: optional.Some(false), }, - []int64{22, 21, 17, 16, 15, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 23, 1}, + []int64{22, 21, 17, 16, 15, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1}, }, { SearchOptions{ @@ -358,7 +358,7 @@ func searchIssueByLabelID(t *testing.T) { SearchOptions{ ExcludedLabelIDs: []int64{1}, }, - []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 23}, + []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3}, }, } for _, test := range tests { @@ -377,7 +377,7 @@ func searchIssueByTime(t *testing.T) { SearchOptions{ UpdatedAfterUnix: optional.Some(int64(0)), }, - []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 23, 1}, + []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1}, }, } for _, test := range tests { @@ -396,7 +396,7 @@ func searchIssueWithOrder(t *testing.T) { SearchOptions{ SortBy: internal.SortByCreatedAsc, }, - []int64{1, 23, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17, 21, 22}, + []int64{1, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17, 21, 22}, }, } for _, test := range tests { @@ -450,7 +450,7 @@ func searchIssueWithPaginator(t *testing.T) { }, }, []int64{22, 21, 17, 16, 15}, - 23, + 22, }, } for _, test := range tests { diff --git a/services/issue/issue_test.go b/services/issue/issue_test.go index d7c850e6eb071..8806cec0e7c5d 100644 --- a/services/issue/issue_test.go +++ b/services/issue/issue_test.go @@ -37,7 +37,7 @@ func TestIssue_DeleteIssue(t *testing.T) { issueIDs, err := issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) - assert.Len(t, issueIDs, 6) + assert.Len(t, issueIDs, 5) issue := &issues_model.Issue{ RepoID: 1, @@ -48,7 +48,7 @@ func TestIssue_DeleteIssue(t *testing.T) { assert.NoError(t, err) issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) - assert.Len(t, issueIDs, 5) + assert.Len(t, issueIDs, 4) // check attachment removal attachments, err := repo_model.GetAttachmentsByIssueID(db.DefaultContext, 4) diff --git a/services/issue/suggestion_test.go b/services/issue/suggestion_test.go index 84771694c4626..84cfd520ac40a 100644 --- a/services/issue/suggestion_test.go +++ b/services/issue/suggestion_test.go @@ -26,7 +26,7 @@ func Test_Suggestion(t *testing.T) { }{ { keyword: "", - expectedIndexes: []int64{5, 6, 1, 4, 2}, + expectedIndexes: []int64{5, 1, 4, 2, 3}, }, { keyword: "1", @@ -34,7 +34,7 @@ func Test_Suggestion(t *testing.T) { }, { keyword: "issue", - expectedIndexes: []int64{6, 4, 1, 2, 3}, + expectedIndexes: []int64{4, 1, 2, 3}, }, { keyword: "pull", diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 8b81b7196f43c..d8394a33d96f4 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -287,7 +287,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(publicOnlyToken) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 16) // 16 public issues + assert.Len(t, apiIssues, 15) // 15 public issues since := "2000-01-01T00:50:01+00:00" // 946687801 before := time.Unix(999307200, 0).Format(time.RFC3339) @@ -297,7 +297,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 12) + assert.Len(t, apiIssues, 11) query.Del("since") query.Del("before") @@ -313,7 +313,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 20) query.Add("limit", "10") @@ -321,7 +321,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 10) query = url.Values{"assigned": {"true"}, "state": {"all"}} @@ -350,7 +350,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()).AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 9) + assert.Len(t, apiIssues, 8) query = url.Values{"owner": {"org3"}} // organization link.RawQuery = query.Encode() diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go index c6b4663b4a4d9..75f8dbb4ba3aa 100644 --- a/tests/integration/api_nodeinfo_test.go +++ b/tests/integration/api_nodeinfo_test.go @@ -33,7 +33,7 @@ func TestNodeinfo(t *testing.T) { assert.True(t, nodeinfo.OpenRegistrations) assert.Equal(t, "gitea", nodeinfo.Software.Name) assert.Equal(t, 29, nodeinfo.Usage.Users.Total) - assert.Equal(t, 23, nodeinfo.Usage.LocalPosts) + assert.Equal(t, 22, nodeinfo.Usage.LocalPosts) assert.Equal(t, 3, nodeinfo.Usage.LocalComments) }) } diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 5180e3d17ee57..22f26d87d4332 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -268,7 +268,7 @@ func TestAPIViewRepo(t *testing.T) { assert.EqualValues(t, 1, repo.ID) assert.EqualValues(t, "repo1", repo.Name) assert.EqualValues(t, 2, repo.Releases) - assert.EqualValues(t, 2, repo.OpenIssues) + assert.EqualValues(t, 1, repo.OpenIssues) assert.EqualValues(t, 3, repo.OpenPulls) req = NewRequest(t, "GET", "/api/v1/repos/user12/repo10") diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index a327092aeed61..dc0c9b135017d 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -494,7 +494,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 12) + assert.Len(t, apiIssues, 11) query.Del("since") query.Del("before") @@ -510,7 +510,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 20) query.Add("limit", "5") @@ -518,7 +518,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.EqualValues(t, "23", resp.Header().Get("X-Total-Count")) + assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count")) assert.Len(t, apiIssues, 5) query = url.Values{"assigned": {"true"}, "state": {"all"}} @@ -547,7 +547,7 @@ func TestSearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 9) + assert.Len(t, apiIssues, 8) query = url.Values{"owner": {"org3"}} // organization link.RawQuery = query.Encode() diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 5ac915538143b..0b7ff52c47053 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -923,7 +923,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing. Run(&git.RunOpts{Dir: dstPath, Stderr: stderrBuf}) assert.NoError(t, err) - assert.Contains(t, stderrBuf.String(), setting.AppURL+"user2/repo1/pulls/7") + assert.Contains(t, stderrBuf.String(), setting.AppURL+"user2/repo1/pulls/6") baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ @@ -1043,7 +1043,7 @@ func TestPullNonMergeForAdminWithBranchProtection(t *testing.T) { token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - mergeReq := NewRequestWithValues(t, "POST", "/api/v1/repos/user2/repo1/pulls/7/merge", map[string]string{ + mergeReq := NewRequestWithValues(t, "POST", "/api/v1/repos/user2/repo1/pulls/6/merge", map[string]string{ "_csrf": csrf, "head_commit_id": "", "merge_when_checks_succeed": "false", From 96136bc473bd4ae3f2734ca4786e3780c7832f0b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 2 Mar 2025 23:43:55 +0800 Subject: [PATCH 04/10] prepare test --- services/mailer/mail_test.go | 66 +++++++++--------- .../1/b/1b267670-1793-4cd0-abc1-449269b7cff9 | Bin 240 -> 0 bytes 2 files changed, 32 insertions(+), 34 deletions(-) delete mode 100644 tests/testdata/data/attachments/1/b/1b267670-1793-4cd0-abc1-449269b7cff9 diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index f7ae46c9ef89a..2844cb48acf0a 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -10,7 +10,6 @@ import ( "html/template" "io" "mime/quotedprintable" - "path/filepath" "regexp" "strings" "testing" @@ -24,10 +23,11 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/test" sender_service "code.gitea.io/gitea/services/mailer/sender" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const subjectTpl = ` @@ -454,37 +454,31 @@ func TestFromDisplayName(t *testing.T) { }) } -func PrepareAttachmentsStorage(t testing.TB) { // same as in test_utils.go - // prepare attachments directory and files - assert.NoError(t, storage.Clean(storage.Attachments)) - - s, err := storage.NewStorage(setting.LocalStorageType, &setting.Storage{ - Path: filepath.Join(filepath.Dir(setting.AppPath), "tests", "testdata", "data", "attachments"), - }) - assert.NoError(t, err) - assert.NoError(t, s.IterateObjects("", func(p string, obj storage.Object) error { - _, err = storage.Copy(storage.Attachments, p, s, p) - return err - })) -} - func TestEmbedBase64ImagesInEmail(t *testing.T) { - // Fake context setup doer, repo, _, _ := prepareMailerTest(t) - PrepareAttachmentsStorage(t) - setting.MailService.Base64EmbedImages = true - setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 * 1024 * 1024 - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 23, Repo: repo, Poster: doer}) - assert.NoError(t, issue.LoadRepo(db.DefaultContext)) + defer test.MockVariableValue(&setting.MailService.Base64EmbedImages, true) + defer test.MockVariableValue(&setting.MailService.Base64EmbedImagesMaxSizePerEmail, 10*1024*1024) + + err := issues_model.NewIssue(t.Context(), repo, &issues_model.Issue{ + Poster: doer, + RepoID: repo.ID, + Title: "test issue attachment", + Content: `content including this image: gitea.png with some more content behind it`, + }, nil, nil) + require.NoError(t, err) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "test issue attachment"}) + require.NoError(t, issue.LoadRepo(t.Context())) subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} msgs, err := composeIssueCommentMessages(&mailCommentContext{ - Context: context.TODO(), // TODO: use a correct context - Issue: issue, Doer: doer, ActionType: activities_model.ActionCreateIssue, - Content: strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), + Context: t.Context(), + Issue: issue, + Doer: doer, + ActionType: activities_model.ActionCreateIssue, + Content: strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), }, "en-US", recipients, false, "issue create") mailBody := msgs[0].Body @@ -511,16 +505,20 @@ func TestEmbedBase64ImagesInEmail(t *testing.T) { func TestEmbedBase64Images(t *testing.T) { user, repo, _, _ := prepareMailerTest(t) - PrepareAttachmentsStorage(t) - setting.MailService.Base64EmbedImages = true - setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 * 1024 * 1024 - - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 23, Repo: repo, Poster: user}) - + defer test.MockVariableValue(&setting.MailService.Base64EmbedImages, true) + defer test.MockVariableValue(&setting.MailService.Base64EmbedImagesMaxSizePerEmail, 10*1024*1024) + + err := issues_model.NewIssue(t.Context(), repo, &issues_model.Issue{ + Poster: user, + RepoID: repo.ID, + Title: "test issue attachment", + Content: `content including this image: gitea.png with some more content behind it`, + }, nil, nil) + require.NoError(t, err) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "test issue attachment"}) + require.NoError(t, issue.LoadRepo(t.Context())) attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 13, IssueID: issue.ID, RepoID: repo.ID}) - ctx0 := context.Background() - - ctx := &mailCommentContext{Context: ctx0 /* TODO: use a correct context */, Issue: issue, Doer: user} + ctx := &mailCommentContext{Context: t.Context(), Issue: issue, Doer: user} img1ExternalURL := "https://via.placeholder.com/10" img1ExternalImg := "" diff --git a/tests/testdata/data/attachments/1/b/1b267670-1793-4cd0-abc1-449269b7cff9 b/tests/testdata/data/attachments/1/b/1b267670-1793-4cd0-abc1-449269b7cff9 deleted file mode 100644 index 8fc22b562f026901bbb803be54828eaedf7797c1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 240 zcmeAS@N?(olHy`uVBq!ia0vp^0YGfX$P6UQ%l8NZDYgKg5Z8p+idCz;I(#aZ07V!} zg8YIR%^$w_0OZW@ba4#PI6w7*HQ!+c0hS9JV|F~@h*7z8?@WP-e5;=BQtpGqvV zMQ*+_J)_4QaCC-v!N-s_$9yXeO75Mw$;f5#vPAQ+y8kD#a+W9lOqtLvy?7Ge zGfpqR7ye6EpSe?2!M$~EkL!#z``nDY>fK*kNc>Fvqn7^QpuzrY?Q%NJ_g*}ETDz@s nu@UdOBBuPl3$nqsYg6y$u1ZZ-__0D4=tc%lS3j3^P6 Date: Mon, 3 Mar 2025 12:12:50 +0800 Subject: [PATCH 05/10] temp fix --- services/mailer/mail.go | 3 +- services/mailer/mail_test.go | 99 ++++++++++++++---------------------- 2 files changed, 38 insertions(+), 64 deletions(-) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 9fd8eee81479d..e95eb2466830a 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -75,7 +75,6 @@ func Base64InlineImages(body string, ctx *mailCommentContext) (string, error) { log.Trace("attachmentSrcToDataURI not possible: %v", err) // Not an error, just skip. This is probably an image from outside the gitea instance. continue } - log.Trace("Old value of src attribute: %s, new value (first 100 characters): %s", attr.Val, dataURI[:100]) n.Attr[i].Val = dataURI break } @@ -115,7 +114,7 @@ func AttachmentSrcToBase64DataURI(attachmentPath string, ctx *mailCommentContext } // "Doer" is theoretically not the correct permission check (as Doer created the action on which to send), but as this is batch processed the receipants can't be accessed. - // Therefore we check the Doer, with which we counter leaking information as a Doer brute force attack on attachments would be possible. + // Therefore, we check the Doer, with which we counter leaking information as a Doer brute force attack on attachments would be possible. perm, err := access_model.GetUserRepoPermission(ctx, ctx.Issue.Repo, ctx.Doer) if err != nil { return "", err diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 2844cb48acf0a..70f2f75fcbe05 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -23,7 +23,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/services/attachment" sender_service "code.gitea.io/gitea/services/mailer/sender" "github.com/stretchr/testify/assert" @@ -55,23 +55,36 @@ const bodyTpl = ` func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, comment *issues_model.Comment) { assert.NoError(t, unittest.PrepareTestDatabase()) - mailService := setting.Mailer{ - From: "test@gitea.com", - } - - setting.MailService = &mailService + setting.MailService = &setting.Mailer{From: "test@gitea.com"} setting.Domain = "localhost" setting.AppURL = "https://try.gitea.io/" doer = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, Owner: doer}) issue = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1, Repo: repo, Poster: doer}) - assert.NoError(t, issue.LoadRepo(db.DefaultContext)) comment = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 2, Issue: issue}) + require.NoError(t, issue.LoadRepo(db.DefaultContext)) return doer, repo, issue, comment } -func TestComposeIssueCommentMessage(t *testing.T) { +func prepareMailerBase64Test(t *testing.T) (doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, att *repo_model.Attachment) { + user, repo, issue, comment := prepareMailerTest(t) + setting.MailService.Base64EmbedImages = true + setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 * 1024 * 1024 + att, err := attachment.NewAttachment(t.Context(), &repo_model.Attachment{ + RepoID: repo.ID, + IssueID: issue.ID, + UploaderID: user.ID, + CommentID: comment.ID, + Name: "test.png", + }, bytes.NewReader([]byte("\x89\x50\x4e\x47\x0d\x0a\x1a\x0a")), 8) + require.NoError(t, err) + issue.Content = fmt.Sprintf(`MSG-BEFORE MSG-AFTER`, att.UUID) + require.NoError(t, issues_model.UpdateIssueCols(t.Context(), issue, "content")) + return user, repo, issue, att +} + +func TestComposeIssueComment(t *testing.T) { doer, _, issue, comment := prepareMailerTest(t) markup.Init(&markup.RenderHelperFuncs{ @@ -112,7 +125,8 @@ func TestComposeIssueCommentMessage(t *testing.T) { assert.Len(t, gomailMsg.GetGenHeader("List-Unsubscribe"), 2) // url + mailto var buf bytes.Buffer - gomailMsg.WriteTo(&buf) + _, err = gomailMsg.WriteTo(&buf) + require.NoError(t, err) b, err := io.ReadAll(quotedprintable.NewReader(&buf)) assert.NoError(t, err) @@ -407,9 +421,9 @@ func TestGenerateMessageIDForRelease(t *testing.T) { } func TestFromDisplayName(t *testing.T) { - template, err := texttmpl.New("mailFrom").Parse("{{ .DisplayName }}") + tmpl, err := texttmpl.New("mailFrom").Parse("{{ .DisplayName }}") assert.NoError(t, err) - setting.MailService = &setting.Mailer{FromDisplayNameFormatTemplate: template} + setting.MailService = &setting.Mailer{FromDisplayNameFormatTemplate: tmpl} defer func() { setting.MailService = nil }() tests := []struct { @@ -438,9 +452,9 @@ func TestFromDisplayName(t *testing.T) { } t.Run("template with all available vars", func(t *testing.T) { - template, err = texttmpl.New("mailFrom").Parse("{{ .DisplayName }} (by {{ .AppName }} on [{{ .Domain }}])") + tmpl, err = texttmpl.New("mailFrom").Parse("{{ .DisplayName }} (by {{ .AppName }} on [{{ .Domain }}])") assert.NoError(t, err) - setting.MailService = &setting.Mailer{FromDisplayNameFormatTemplate: template} + setting.MailService = &setting.Mailer{FromDisplayNameFormatTemplate: tmpl} oldAppName := setting.AppName setting.AppName = "Code IT" oldDomain := setting.Domain @@ -455,20 +469,7 @@ func TestFromDisplayName(t *testing.T) { } func TestEmbedBase64ImagesInEmail(t *testing.T) { - doer, repo, _, _ := prepareMailerTest(t) - defer test.MockVariableValue(&setting.MailService.Base64EmbedImages, true) - defer test.MockVariableValue(&setting.MailService.Base64EmbedImagesMaxSizePerEmail, 10*1024*1024) - - err := issues_model.NewIssue(t.Context(), repo, &issues_model.Issue{ - Poster: doer, - RepoID: repo.ID, - Title: "test issue attachment", - Content: `content including this image: gitea.png with some more content behind it`, - }, nil, nil) - require.NoError(t, err) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "test issue attachment"}) - require.NoError(t, issue.LoadRepo(t.Context())) - + doer, _, issue, _ := prepareMailerBase64Test(t) subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) @@ -480,52 +481,26 @@ func TestEmbedBase64ImagesInEmail(t *testing.T) { ActionType: activities_model.ActionCreateIssue, Content: strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), }, "en-US", recipients, false, "issue create") + require.NoError(t, err) mailBody := msgs[0].Body - re := regexp.MustCompile(`(?s)(.*?)`) + re := regexp.MustCompile(`MSG-BEFORE.*MSG-AFTER`) matches := re.FindStringSubmatch(mailBody) - if len(matches) > 1 { - mailBody = matches[1] - } - // check if the mail body was correctly generated - assert.NoError(t, err) - assert.Contains(t, mailBody, "content including this image") - - // check if an image was embedded - assert.Contains(t, mailBody, "data:image/png;base64,") - - // check if the image was embedded only once - assert.Equal(t, 1, strings.Count(mailBody, "data:image/png;base64,")) - - img2InternalBase64 := "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAFAAAAAxAQMAAAB3d7wRAAAABlBMVEVgmyF6qkqITHmkAAAAAXRSTlMBN+Ho8AAAAJhJREFUKM+V0DsOwyAQBNCxXLjkCFwk0t7McDQfhS4tpQuEzWc/iaUU2eo1zC4DUMWYF3DxVKzGTXjBGb2RsjJEo6ZhN1Zj+cEgi/9hBQl3YflkkIsbo5IO5glKTuhPpavM3Hp4C7WdjEWYrL5GMkp/R+s4GPlh/CZn4MEwv9aHHiyD3ujm5X22eaMyDa5yAm+O0B1TPa1l3W2qZWMg+KgtAAAAAElFTkSuQmCC" - - // check if the image was embedded correctly - assert.Contains(t, mailBody, img2InternalBase64) + require.NotEmpty(t, matches) + mailBody = matches[0] + assert.Equal(t, `MSG-BEFORE MSG-AFTER`, mailBody) } func TestEmbedBase64Images(t *testing.T) { - user, repo, _, _ := prepareMailerTest(t) - defer test.MockVariableValue(&setting.MailService.Base64EmbedImages, true) - defer test.MockVariableValue(&setting.MailService.Base64EmbedImagesMaxSizePerEmail, 10*1024*1024) - - err := issues_model.NewIssue(t.Context(), repo, &issues_model.Issue{ - Poster: user, - RepoID: repo.ID, - Title: "test issue attachment", - Content: `content including this image: gitea.png with some more content behind it`, - }, nil, nil) - require.NoError(t, err) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "test issue attachment"}) - require.NoError(t, issue.LoadRepo(t.Context())) - attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 13, IssueID: issue.ID, RepoID: repo.ID}) + user, repo, issue, att := prepareMailerBase64Test(t) ctx := &mailCommentContext{Context: t.Context(), Issue: issue, Doer: user} img1ExternalURL := "https://via.placeholder.com/10" img1ExternalImg := "" - img2InternalURL := setting.AppURL + repo.Owner.Name + "/" + repo.Name + "/attachments/" + attachment.UUID + img2InternalURL := setting.AppURL + repo.Owner.Name + "/" + repo.Name + "/attachments/" + att.UUID img2InternalImg := "" - img2InternalBase64 := "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAFAAAAAxAQMAAAB3d7wRAAAABlBMVEVgmyF6qkqITHmkAAAAAXRSTlMBN+Ho8AAAAJhJREFUKM+V0DsOwyAQBNCxXLjkCFwk0t7McDQfhS4tpQuEzWc/iaUU2eo1zC4DUMWYF3DxVKzGTXjBGb2RsjJEo6ZhN1Zj+cEgi/9hBQl3YflkkIsbo5IO5glKTuhPpavM3Hp4C7WdjEWYrL5GMkp/R+s4GPlh/CZn4MEwv9aHHiyD3ujm5X22eaMyDa5yAm+O0B1TPa1l3W2qZWMg+KgtAAAAAElFTkSuQmCC" + img2InternalBase64 := "data:image/png;base64,iVBORw0KGgo=" img2InternalBase64Img := "" // 1st Test: convert internal image to base64 @@ -558,7 +533,7 @@ func TestEmbedBase64Images(t *testing.T) { // 4th Test, generate email body with 2 internal images, but set Mailer.Base64EmbedImagesMaxSizePerEmail to the size of the first image (+1), expect the first image to be replaced and the second not t.Run("generateEmailBodyWithMaxSize", func(t *testing.T) { - setting.MailService.Base64EmbedImagesMaxSizePerEmail = int64(len(img2InternalBase64) + 1) + setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 mailBody := "

Test1

" + img2InternalImg + "

Test2

" + img2InternalImg + "

Test3

" expectedMailBody := "

Test1

" + img2InternalBase64Img + "

Test2

" + img2InternalImg + "

Test3

" From 8c77179f0087523b7e6068128b4b7af78dea13db Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 4 Mar 2025 14:47:04 +0800 Subject: [PATCH 06/10] temp: fix permission check --- services/mailer/mail.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index e95eb2466830a..c93f7a1cfee92 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -6,6 +6,7 @@ package mailer import ( "bytes" + "context" "encoding/base64" "fmt" "html/template" @@ -16,9 +17,7 @@ import ( "strings" texttmpl "text/template" - access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -54,11 +53,20 @@ func sanitizeSubject(subject string) string { return mime.QEncoding.Encode("utf-8", string(runes)) } -func Base64InlineImages(body string, ctx *mailCommentContext) (string, error) { +type mailAttachmentBase64Embedder struct { + doer *user_model.User + repo *repo_model.Repository + maxSize int64 +} + +func newMailAttachmentBase64Embedder(doer *user_model.User, repo *repo_model.Repository, maxSize int64) *mailAttachmentBase64Embedder { + return &mailAttachmentBase64Embedder{doer: doer, repo: repo, maxSize: maxSize} +} + +func (b64embedder *mailAttachmentBase64Embedder) Base64InlineImages(ctx context.Context, body string) (string, error) { doc, err := html.Parse(strings.NewReader(body)) if err != nil { - log.Error("Failed to parse HTML body: %v", err) - return "", err + return "", fmt.Errorf("%w", err) } var totalEmbeddedImagesSize int64 @@ -70,7 +78,7 @@ func Base64InlineImages(body string, ctx *mailCommentContext) (string, error) { for i, attr := range n.Attr { if attr.Key == "src" { attachmentPath := attr.Val - dataURI, err := AttachmentSrcToBase64DataURI(attachmentPath, ctx, &totalEmbeddedImagesSize) + dataURI, err := b64embedder.AttachmentSrcToBase64DataURI(ctx, attachmentPath, &totalEmbeddedImagesSize) if err != nil { log.Trace("attachmentSrcToDataURI not possible: %v", err) // Not an error, just skip. This is probably an image from outside the gitea instance. continue @@ -98,7 +106,7 @@ func Base64InlineImages(body string, ctx *mailCommentContext) (string, error) { return buf.String(), nil } -func AttachmentSrcToBase64DataURI(attachmentPath string, ctx *mailCommentContext, totalEmbeddedImagesSize *int64) (string, error) { +func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ctx context.Context, attachmentPath string, totalEmbeddedImagesSize *int64) (string, error) { if !strings.HasPrefix(attachmentPath, setting.AppURL) { // external image return "", fmt.Errorf("external image") } @@ -113,14 +121,8 @@ func AttachmentSrcToBase64DataURI(attachmentPath string, ctx *mailCommentContext return "", err } - // "Doer" is theoretically not the correct permission check (as Doer created the action on which to send), but as this is batch processed the receipants can't be accessed. - // Therefore, we check the Doer, with which we counter leaking information as a Doer brute force attack on attachments would be possible. - perm, err := access_model.GetUserRepoPermission(ctx, ctx.Issue.Repo, ctx.Doer) - if err != nil { - return "", err - } - if !perm.CanRead(unit.TypeIssues) { - return "", fmt.Errorf("no permission") + if attachment.RepoID != b64embedder.repo.ID { + return "", fmt.Errorf("attachment does not belong to the repository") } fr, err := storage.Attachments.Open(attachment.RelativePath()) @@ -129,15 +131,13 @@ func AttachmentSrcToBase64DataURI(attachmentPath string, ctx *mailCommentContext } defer fr.Close() - maxSize := setting.MailService.Base64EmbedImagesMaxSizePerEmail // at maximum read the whole available combined email size, to prevent maliciously large file reads - - lr := &io.LimitedReader{R: fr, N: maxSize + 1} + lr := &io.LimitedReader{R: fr, N: b64embedder.maxSize + 1} content, err := io.ReadAll(lr) if err != nil { return "", err } - if len(content) > int(maxSize) { - return "", fmt.Errorf("file size exceeds the embedded image max limit \\(%d bytes\\)", maxSize) + if int64(len(content)) > b64embedder.maxSize { + return "", fmt.Errorf("file size exceeds the embedded image max limit \\(%d bytes\\)", b64embedder.maxSize) } if *totalEmbeddedImagesSize+int64(len(content)) > setting.MailService.Base64EmbedImagesMaxSizePerEmail { From 1a395b3340a6d0504d339ff6e635ae19be656488 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 5 Mar 2025 09:50:32 +0800 Subject: [PATCH 07/10] fix url detection --- custom/conf/app.example.ini | 5 +- modules/httplib/url.go | 78 +++++++++++++++++++++++----- modules/httplib/url_test.go | 23 ++++++++ modules/setting/mailer.go | 25 +++++---- services/mailer/mail.go | 76 +++++++++++++-------------- services/mailer/mail_issue_common.go | 19 ++++--- 6 files changed, 149 insertions(+), 77 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 57c66dddbba55..178d7a13631fe 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1769,10 +1769,7 @@ LEVEL = Info ;SENDMAIL_CONVERT_CRLF = true ;; ;; convert links of attached images to inline images. Only for images hosted in this gitea instance. -;BASE64_EMBED_IMAGES = false -;; -;; The maximum size of sum of all images in a single email. Default is 9.5MB -;BASE64_EMBED_IMAGES_MAX_SIZE_PER_EMAIL = 9961472 +;EMBED_ATTACHMENT_IMAGES = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/httplib/url.go b/modules/httplib/url.go index f543c09190673..5d5b64dc0cfbf 100644 --- a/modules/httplib/url.go +++ b/modules/httplib/url.go @@ -102,25 +102,77 @@ func MakeAbsoluteURL(ctx context.Context, link string) string { return GuessCurrentHostURL(ctx) + "/" + strings.TrimPrefix(link, "/") } -func IsCurrentGiteaSiteURL(ctx context.Context, s string) bool { +type urlType int + +const ( + urlTypeGiteaAbsolute urlType = iota + 1 // "http://gitea/subpath" + urlTypeGiteaPageRelative // "/subpath" + urlTypeGiteaSiteRelative // "?key=val" + urlTypeUnknown // "http://other" +) + +func detectURLRoutePath(ctx context.Context, s string) (routePath string, ut urlType) { u, err := url.Parse(s) if err != nil { - return false + return "", urlTypeUnknown } + cleanedPath := "" if u.Path != "" { - cleanedPath := util.PathJoinRelX(u.Path) - if cleanedPath == "" || cleanedPath == "." { - u.Path = "/" - } else { - u.Path = "/" + cleanedPath + "/" - } + cleanedPath = util.PathJoinRelX(u.Path) + cleanedPath = util.Iif(cleanedPath == ".", "", "/"+cleanedPath) } if urlIsRelative(s, u) { - return u.Path == "" || strings.HasPrefix(strings.ToLower(u.Path), strings.ToLower(setting.AppSubURL+"/")) - } - if u.Path == "" { - u.Path = "/" + if u.Path == "" { + return "", urlTypeGiteaPageRelative + } + if strings.HasPrefix(strings.ToLower(cleanedPath+"/"), strings.ToLower(setting.AppSubURL+"/")) { + return cleanedPath[len(setting.AppSubURL):], urlTypeGiteaSiteRelative + } + return "", urlTypeUnknown } + u.Path = cleanedPath + "/" urlLower := strings.ToLower(u.String()) - return strings.HasPrefix(urlLower, strings.ToLower(setting.AppURL)) || strings.HasPrefix(urlLower, strings.ToLower(GuessCurrentAppURL(ctx))) + if strings.HasPrefix(urlLower, strings.ToLower(setting.AppURL)) { + return cleanedPath[len(setting.AppSubURL):], urlTypeGiteaAbsolute + } + guessedCurURL := GuessCurrentAppURL(ctx) + if strings.HasPrefix(urlLower, strings.ToLower(guessedCurURL)) { + return cleanedPath[len(setting.AppSubURL):], urlTypeGiteaAbsolute + } + return "", urlTypeUnknown +} + +func IsCurrentGiteaSiteURL(ctx context.Context, s string) bool { + _, ut := detectURLRoutePath(ctx, s) + return ut != urlTypeUnknown +} + +type GiteaSiteURL struct { + RoutePath string + OwnerName string + RepoName string + RepoSubPath string +} + +func ParseGiteaSiteURL(ctx context.Context, s string) *GiteaSiteURL { + routePath, ut := detectURLRoutePath(ctx, s) + if ut == urlTypeUnknown || ut == urlTypeGiteaPageRelative { + return nil + } + ret := &GiteaSiteURL{RoutePath: routePath} + fields := strings.SplitN(strings.TrimPrefix(ret.RoutePath, "/"), "/", 3) + + // TODO: now it only does a quick check for some known reserved paths, should do more strict checks in the future + if fields[0] == "attachments" { + return ret + } + if len(fields) < 2 { + return ret + } + ret.OwnerName = fields[0] + ret.RepoName = fields[1] + if len(fields) == 3 { + ret.RepoSubPath = "/" + fields[2] + } + return ret } diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go index cb8fac0a2198c..d57653646b306 100644 --- a/modules/httplib/url_test.go +++ b/modules/httplib/url_test.go @@ -122,3 +122,26 @@ func TestIsCurrentGiteaSiteURL(t *testing.T) { assert.True(t, IsCurrentGiteaSiteURL(ctx, "https://user-host")) assert.False(t, IsCurrentGiteaSiteURL(ctx, "https://forwarded-host")) } + +func TestParseGiteaSiteURL(t *testing.T) { + defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")() + defer test.MockVariableValue(&setting.AppSubURL, "/sub")() + ctx := t.Context() + tests := []struct { + url string + exp *GiteaSiteURL + }{ + {"http://localhost:3000/sub?k=v", &GiteaSiteURL{RoutePath: ""}}, + {"http://localhost:3000/sub/", &GiteaSiteURL{RoutePath: ""}}, + {"http://localhost:3000/sub/foo", &GiteaSiteURL{RoutePath: "/foo"}}, + {"http://localhost:3000/sub/foo/bar", &GiteaSiteURL{RoutePath: "/foo/bar", OwnerName: "foo", RepoName: "bar"}}, + {"http://localhost:3000/sub/foo/bar/", &GiteaSiteURL{RoutePath: "/foo/bar", OwnerName: "foo", RepoName: "bar"}}, + {"http://localhost:3000/sub/attachments/bar", &GiteaSiteURL{RoutePath: "/attachments/bar"}}, + {"http://localhost:3000/other", nil}, + {"http://other/", nil}, + } + for _, test := range tests { + su := ParseGiteaSiteURL(ctx, test.url) + assert.Equal(t, test.exp, su, "URL = %s", test.url) + } +} diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index a850ee3c0c0c3..e79ff304474b8 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -19,17 +19,18 @@ import ( // Mailer represents mail service. type Mailer struct { // Mailer - Name string `ini:"NAME"` - From string `ini:"FROM"` - EnvelopeFrom string `ini:"ENVELOPE_FROM"` - OverrideEnvelopeFrom bool `ini:"-"` - FromName string `ini:"-"` - FromEmail string `ini:"-"` - SendAsPlainText bool `ini:"SEND_AS_PLAIN_TEXT"` - SubjectPrefix string `ini:"SUBJECT_PREFIX"` - OverrideHeader map[string][]string `ini:"-"` - Base64EmbedImages bool `ini:"BASE64_EMBED_IMAGES"` - Base64EmbedImagesMaxSizePerEmail int64 `ini:"BASE64_EMBED_IMAGES_MAX_SIZE_PER_EMAIL"` + Name string `ini:"NAME"` + From string `ini:"FROM"` + EnvelopeFrom string `ini:"ENVELOPE_FROM"` + OverrideEnvelopeFrom bool `ini:"-"` + FromName string `ini:"-"` + FromEmail string `ini:"-"` + SendAsPlainText bool `ini:"SEND_AS_PLAIN_TEXT"` + SubjectPrefix string `ini:"SUBJECT_PREFIX"` + OverrideHeader map[string][]string `ini:"-"` + + // Embed attachment images as inline base64 img src attribute + EmbedAttachmentImages bool // SMTP sender Protocol string `ini:"PROTOCOL"` @@ -152,8 +153,6 @@ func loadMailerFrom(rootCfg ConfigProvider) { sec.Key("SENDMAIL_TIMEOUT").MustDuration(5 * time.Minute) sec.Key("SENDMAIL_CONVERT_CRLF").MustBool(true) sec.Key("FROM").MustString(sec.Key("USER").String()) - sec.Key("BASE64_EMBED_IMAGES").MustBool(false) - sec.Key("BASE64_EMBED_IMAGES_MAX_SIZE_PER_EMAIL").MustInt64(9.5 * 1024 * 1024) // Now map the values on to the MailService MailService = &Mailer{} diff --git a/services/mailer/mail.go b/services/mailer/mail.go index c93f7a1cfee92..f58dd41034075 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -6,13 +6,14 @@ package mailer import ( "bytes" + "code.gitea.io/gitea/modules/httplib" + "code.gitea.io/gitea/modules/typesniffer" "context" "encoding/base64" "fmt" "html/template" "io" "mime" - "net/http" "regexp" "strings" texttmpl "text/template" @@ -54,22 +55,23 @@ func sanitizeSubject(subject string) string { } type mailAttachmentBase64Embedder struct { - doer *user_model.User - repo *repo_model.Repository - maxSize int64 + doer *user_model.User + repo *repo_model.Repository + maxSize int64 + estimateSize int64 } func newMailAttachmentBase64Embedder(doer *user_model.User, repo *repo_model.Repository, maxSize int64) *mailAttachmentBase64Embedder { return &mailAttachmentBase64Embedder{doer: doer, repo: repo, maxSize: maxSize} } -func (b64embedder *mailAttachmentBase64Embedder) Base64InlineImages(ctx context.Context, body string) (string, error) { - doc, err := html.Parse(strings.NewReader(body)) +func (b64embedder *mailAttachmentBase64Embedder) Base64InlineImages(ctx context.Context, body template.HTML) (template.HTML, error) { + doc, err := html.Parse(strings.NewReader(string(body))) if err != nil { - return "", fmt.Errorf("%w", err) + return "", fmt.Errorf("html.Parse failed: %w", err) } - var totalEmbeddedImagesSize int64 + b64embedder.estimateSize = int64(len(string(body))) var processNode func(*html.Node) processNode = func(n *html.Node) { @@ -77,19 +79,19 @@ func (b64embedder *mailAttachmentBase64Embedder) Base64InlineImages(ctx context. if n.Data == "img" { for i, attr := range n.Attr { if attr.Key == "src" { - attachmentPath := attr.Val - dataURI, err := b64embedder.AttachmentSrcToBase64DataURI(ctx, attachmentPath, &totalEmbeddedImagesSize) + attachmentSrc := attr.Val + dataURI, err := b64embedder.AttachmentSrcToBase64DataURI(ctx, attachmentSrc) if err != nil { - log.Trace("attachmentSrcToDataURI not possible: %v", err) // Not an error, just skip. This is probably an image from outside the gitea instance. - continue + // Not an error, just skip. This is probably an image from outside the gitea instance. + log.Trace("Unable to embed attachment %q to mail body: %v", attachmentSrc, err) + } else { + n.Attr[i].Val = dataURI } - n.Attr[i].Val = dataURI break } } } } - for c := n.FirstChild; c != nil; c = c.NextSibling { processNode(c) } @@ -100,22 +102,24 @@ func (b64embedder *mailAttachmentBase64Embedder) Base64InlineImages(ctx context. var buf bytes.Buffer err = html.Render(&buf, doc) if err != nil { - log.Error("Failed to render modified HTML: %v", err) - return "", err + return "", fmt.Errorf("html.Render failed: %w", err) } - return buf.String(), nil + return template.HTML(buf.String()), nil } -func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ctx context.Context, attachmentPath string, totalEmbeddedImagesSize *int64) (string, error) { - if !strings.HasPrefix(attachmentPath, setting.AppURL) { // external image - return "", fmt.Errorf("external image") - } - parts := strings.Split(attachmentPath, "/attachments/") - if len(parts) <= 1 { - return "", fmt.Errorf("invalid attachment path: %s", attachmentPath) +func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ctx context.Context, attachmentSrc string) (string, error) { + parsedSrc := httplib.ParseGiteaSiteURL(ctx, attachmentSrc) + var attachmentUUID string + if parsedSrc != nil { + var ok bool + attachmentUUID, ok = strings.CutPrefix(parsedSrc.RoutePath, "/attachments/") + if !ok { + attachmentUUID, ok = strings.CutPrefix(parsedSrc.RepoSubPath, "/attachments/") + } + if !ok { + return "", fmt.Errorf("not an attachment") + } } - - attachmentUUID := parts[len(parts)-1] attachment, err := repo_model.GetAttachmentByUUID(ctx, attachmentUUID) if err != nil { return "", err @@ -124,6 +128,10 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct if attachment.RepoID != b64embedder.repo.ID { return "", fmt.Errorf("attachment does not belong to the repository") } + if attachment.Size+b64embedder.estimateSize > b64embedder.maxSize { + return "", fmt.Errorf("total embedded images exceed max limit") + } + b64embedder.estimateSize += attachment.Size fr, err := storage.Attachments.Open(attachment.RelativePath()) if err != nil { @@ -134,26 +142,16 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct lr := &io.LimitedReader{R: fr, N: b64embedder.maxSize + 1} content, err := io.ReadAll(lr) if err != nil { - return "", err - } - if int64(len(content)) > b64embedder.maxSize { - return "", fmt.Errorf("file size exceeds the embedded image max limit \\(%d bytes\\)", b64embedder.maxSize) - } - - if *totalEmbeddedImagesSize+int64(len(content)) > setting.MailService.Base64EmbedImagesMaxSizePerEmail { - return "", fmt.Errorf("total embedded images exceed max limit: %d > %d", *totalEmbeddedImagesSize+int64(len(content)), setting.MailService.Base64EmbedImagesMaxSizePerEmail) + return "", fmt.Errorf("LimitedReader ReadAll: %w", err) } - *totalEmbeddedImagesSize += int64(len(content)) - - mimeType := http.DetectContentType(content) - if !strings.HasPrefix(mimeType, "image/") { + mimeType := typesniffer.DetectContentType(content) + if !mimeType.IsImage() { return "", fmt.Errorf("not an image") } encoded := base64.StdEncoding.EncodeToString(content) dataURI := fmt.Sprintf("data:%s;base64,%s", mimeType, encoded) - return dataURI, nil } diff --git a/services/mailer/mail_issue_common.go b/services/mailer/mail_issue_common.go index d5611cee31099..85fe7c1f9adf4 100644 --- a/services/mailer/mail_issue_common.go +++ b/services/mailer/mail_issue_common.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "fmt" - "html/template" "strconv" "strings" "time" @@ -26,6 +25,10 @@ import ( "code.gitea.io/gitea/services/mailer/token" ) +// maxEmailBodySize is the approximate maximum size of an email body in bytes +// Many e-mail service providers have limitations on the size of the email body, it's usually from 10MB to 25MB +const maxEmailBodySize = 9_000_000 + func fallbackMailSubject(issue *issues_model.Issue) string { return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.FullName(), issue.Title, issue.Index) } @@ -65,19 +68,19 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient // This is the body of the new issue or comment, not the mail body rctx := renderhelper.NewRenderContextRepoComment(ctx.Context, ctx.Issue.Repo).WithUseAbsoluteLink(true) - body, err := markdown.RenderString(rctx, - ctx.Content) + body, err := markdown.RenderString(rctx, ctx.Content) if err != nil { return nil, err } - if setting.MailService.Base64EmbedImages { - bodyStr := string(body) - bodyStr, err = Base64InlineImages(bodyStr, ctx) + if setting.MailService.EmbedAttachmentImages { + attEmbedder := newMailAttachmentBase64Embedder(ctx.Doer, ctx.Issue.Repo, maxEmailBodySize) + bodyAfterEmbedding, err := attEmbedder.Base64InlineImages(ctx, body) if err != nil { - return nil, err + log.Error("Failed to embed images in mail body: %v", err) + } else { + body = bodyAfterEmbedding } - body = template.HTML(bodyStr) } actType, actName, tplName := actionToTemplate(ctx.Issue, ctx.ActionType, commentType, reviewType) From 821c4a7422c78f617908d13caa6fe543dd5307fa Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 5 Mar 2025 10:17:30 +0800 Subject: [PATCH 08/10] fix tests --- services/mailer/mail.go | 6 +- services/mailer/mail_test.go | 155 +++++++++++++++++------------------ 2 files changed, 78 insertions(+), 83 deletions(-) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index f58dd41034075..f48f16c3a7a1f 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -6,8 +6,6 @@ package mailer import ( "bytes" - "code.gitea.io/gitea/modules/httplib" - "code.gitea.io/gitea/modules/typesniffer" "context" "encoding/base64" "fmt" @@ -20,9 +18,11 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/typesniffer" sender_service "code.gitea.io/gitea/services/mailer/sender" "golang.org/x/net/html" @@ -151,7 +151,7 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct } encoded := base64.StdEncoding.EncodeToString(content) - dataURI := fmt.Sprintf("data:%s;base64,%s", mimeType, encoded) + dataURI := fmt.Sprintf("data:%s;base64,%s", mimeType.GetMimeType(), encoded) return dataURI, nil } diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 70f2f75fcbe05..dd4b2b395ec2f 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -6,6 +6,7 @@ package mailer import ( "bytes" "context" + "encoding/base64" "fmt" "html/template" "io" @@ -23,6 +24,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/services/attachment" sender_service "code.gitea.io/gitea/services/mailer/sender" @@ -67,11 +69,11 @@ func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Re return doer, repo, issue, comment } -func prepareMailerBase64Test(t *testing.T) (doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, att *repo_model.Attachment) { +func prepareMailerBase64Test(t *testing.T) (doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, att1, att2 *repo_model.Attachment) { user, repo, issue, comment := prepareMailerTest(t) - setting.MailService.Base64EmbedImages = true - setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 * 1024 * 1024 - att, err := attachment.NewAttachment(t.Context(), &repo_model.Attachment{ + setting.MailService.EmbedAttachmentImages = true + + att1, err := attachment.NewAttachment(t.Context(), &repo_model.Attachment{ RepoID: repo.ID, IssueID: issue.ID, UploaderID: user.ID, @@ -79,9 +81,17 @@ func prepareMailerBase64Test(t *testing.T) (doer *user_model.User, repo *repo_mo Name: "test.png", }, bytes.NewReader([]byte("\x89\x50\x4e\x47\x0d\x0a\x1a\x0a")), 8) require.NoError(t, err) - issue.Content = fmt.Sprintf(`MSG-BEFORE MSG-AFTER`, att.UUID) - require.NoError(t, issues_model.UpdateIssueCols(t.Context(), issue, "content")) - return user, repo, issue, att + + att2, err = attachment.NewAttachment(t.Context(), &repo_model.Attachment{ + RepoID: repo.ID, + IssueID: issue.ID, + UploaderID: user.ID, + CommentID: comment.ID, + Name: "test.png", + }, bytes.NewReader([]byte("\x89\x50\x4e\x47\x0d\x0a\x1a\x0a"+strings.Repeat("\x00", 1024))), 8+1024) + require.NoError(t, err) + + return user, repo, issue, att1, att2 } func TestComposeIssueComment(t *testing.T) { @@ -468,90 +478,75 @@ func TestFromDisplayName(t *testing.T) { }) } -func TestEmbedBase64ImagesInEmail(t *testing.T) { - doer, _, issue, _ := prepareMailerBase64Test(t) - subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) - bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) - - recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} - msgs, err := composeIssueCommentMessages(&mailCommentContext{ - Context: t.Context(), - Issue: issue, - Doer: doer, - ActionType: activities_model.ActionCreateIssue, - Content: strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), - }, "en-US", recipients, false, "issue create") - require.NoError(t, err) - - mailBody := msgs[0].Body - re := regexp.MustCompile(`MSG-BEFORE.*MSG-AFTER`) - matches := re.FindStringSubmatch(mailBody) - require.NotEmpty(t, matches) - mailBody = matches[0] - assert.Equal(t, `MSG-BEFORE MSG-AFTER`, mailBody) -} - func TestEmbedBase64Images(t *testing.T) { - user, repo, issue, att := prepareMailerBase64Test(t) + user, repo, issue, att1, att2 := prepareMailerBase64Test(t) ctx := &mailCommentContext{Context: t.Context(), Issue: issue, Doer: user} - img1ExternalURL := "https://via.placeholder.com/10" - img1ExternalImg := "" - - img2InternalURL := setting.AppURL + repo.Owner.Name + "/" + repo.Name + "/attachments/" + att.UUID - img2InternalImg := "" - img2InternalBase64 := "data:image/png;base64,iVBORw0KGgo=" - img2InternalBase64Img := "" + imgExternalURL := "https://via.placeholder.com/10" + imgExternalImg := fmt.Sprintf(``, imgExternalURL) - // 1st Test: convert internal image to base64 - t.Run("replaceSpecifiedBase64ImagesInternal", func(t *testing.T) { - totalEmbeddedImagesSize := int64(0) + att1URL := setting.AppURL + repo.Owner.Name + "/" + repo.Name + "/attachments/" + att1.UUID + att1Img := fmt.Sprintf(``, att1URL) + att1Base64 := "data:image/png;base64,iVBORw0KGgo=" + att1ImgBase64 := fmt.Sprintf(``, att1Base64) - resultImg1Internal, err := AttachmentSrcToBase64DataURI(img2InternalURL, ctx, &totalEmbeddedImagesSize) - assert.NoError(t, err) - assert.Equal(t, img2InternalBase64, resultImg1Internal) // replace cause internal image - }) - - // 2nd Test: convert external image to base64 -> abort cause external image - t.Run("replaceSpecifiedBase64ImagesExternal", func(t *testing.T) { - totalEmbeddedImagesSize := int64(0) + att2URL := setting.AppURL + repo.Owner.Name + "/" + repo.Name + "/attachments/" + att2.UUID + att2Img := fmt.Sprintf(``, att2URL) + att2File, err := storage.Attachments.Open(att2.RelativePath()) + require.NoError(t, err) + defer att2File.Close() + att2Bytes, err := io.ReadAll(att2File) + require.NoError(t, err) + require.Greater(t, len(att2Bytes), 1024) + att2Base64 := "data:image/png;base64," + base64.StdEncoding.EncodeToString(att2Bytes) + att2ImgBase64 := fmt.Sprintf(``, att2Base64) - resultImg1External, err := AttachmentSrcToBase64DataURI(img1ExternalURL, ctx, &totalEmbeddedImagesSize) - assert.Error(t, err) - assert.Equal(t, "", resultImg1External) // don't replace cause external image - }) + t.Run("ComposeMessage", func(t *testing.T) { + issue.Content = fmt.Sprintf(`MSG-BEFORE MSG-AFTER`, att1.UUID) + require.NoError(t, issues_model.UpdateIssueCols(t.Context(), issue, "content")) - // 3rd Test: generate email body with 1 internal and 1 external image, expect the result to have the internal image replaced with base64 data and the external not replaced - t.Run("generateEmailBody", func(t *testing.T) { - mailBody := "

Test1

" + img1ExternalImg + "

Test2

" + img2InternalImg + "

Test3

" - expectedMailBody := "

Test1

" + img1ExternalImg + "

Test2

" + img2InternalBase64Img + "

Test3

" - resultMailBody, err := Base64InlineImages(mailBody, ctx) + subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) + bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) - assert.NoError(t, err) - assert.Equal(t, expectedMailBody, resultMailBody) + recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} + msgs, err := composeIssueCommentMessages(&mailCommentContext{ + Context: t.Context(), + Issue: issue, + Doer: user, + ActionType: activities_model.ActionCreateIssue, + Content: issue.Content, // strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), + }, "en-US", recipients, false, "issue create") + require.NoError(t, err) + + mailBody := msgs[0].Body + re := regexp.MustCompile(`MSG-BEFORE.*MSG-AFTER`) + matches := re.FindStringSubmatch(mailBody) + require.NotEmpty(t, matches) + mailBody = matches[0] + assert.Regexp(t, `MSG-BEFORE ]+> MSG-AFTER`, mailBody) }) - // 4th Test, generate email body with 2 internal images, but set Mailer.Base64EmbedImagesMaxSizePerEmail to the size of the first image (+1), expect the first image to be replaced and the second not - t.Run("generateEmailBodyWithMaxSize", func(t *testing.T) { - setting.MailService.Base64EmbedImagesMaxSizePerEmail = 10 - - mailBody := "

Test1

" + img2InternalImg + "

Test2

" + img2InternalImg + "

Test3

" - expectedMailBody := "

Test1

" + img2InternalBase64Img + "

Test2

" + img2InternalImg + "

Test3

" - resultMailBody, err := Base64InlineImages(mailBody, ctx) - - assert.NoError(t, err) - assert.Equal(t, expectedMailBody, resultMailBody) + t.Run("EmbedInstanceImageSkipExternalImage", func(t *testing.T) { + mailBody := "

Test1

" + imgExternalImg + "

Test2

" + att1Img + "

Test3

" + expectedMailBody := "

Test1

" + imgExternalImg + "

Test2

" + att1ImgBase64 + "

Test3

" + b64embedder := newMailAttachmentBase64Embedder(user, repo, 1024) + resultMailBody, err := b64embedder.Base64InlineImages(ctx, template.HTML(mailBody)) + require.NoError(t, err) + assert.Equal(t, expectedMailBody, string(resultMailBody)) }) - // 5th Test, generate email body with 3 internal images, but set Mailer.Base64EmbedImagesMaxSizePerEmail to the size of all 3 images (+1), expect all images to be replaced - t.Run("generateEmailBodyWith3Images", func(t *testing.T) { - setting.MailService.Base64EmbedImagesMaxSizePerEmail = int64(len(img2InternalBase64)*3 + 1) - - mailBody := "

Test1

" + img2InternalImg + "

Test2

" + img2InternalImg + "

Test3

" + img2InternalImg + "" - expectedMailBody := "

Test1

" + img2InternalBase64Img + "

Test2

" + img2InternalBase64Img + "

Test3

" + img2InternalBase64Img + "" - resultMailBody, err := Base64InlineImages(mailBody, ctx) - - assert.NoError(t, err) - assert.Equal(t, expectedMailBody, resultMailBody) + t.Run("LimitedEmailBodySize", func(t *testing.T) { + mailBody := fmt.Sprintf("%s%s", att1Img, att2Img) + b64embedder := newMailAttachmentBase64Embedder(user, repo, 1024) + resultMailBody, err := b64embedder.Base64InlineImages(ctx, template.HTML(mailBody)) + require.NoError(t, err) + expected := fmt.Sprintf("%s%s", att1ImgBase64, att2Img) + assert.Equal(t, expected, string(resultMailBody)) + + b64embedder = newMailAttachmentBase64Embedder(user, repo, 4096) + resultMailBody, err = b64embedder.Base64InlineImages(ctx, template.HTML(mailBody)) + require.NoError(t, err) + expected = fmt.Sprintf("%s%s", att1ImgBase64, att2ImgBase64) + assert.Equal(t, expected, string(resultMailBody)) }) } From dfaef36dd44dddbb58fb86d0da7dbf5dffe54166 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 5 Mar 2025 10:23:53 +0800 Subject: [PATCH 09/10] optimize tests --- services/mailer/mail_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index dd4b2b395ec2f..85ee3455459d8 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -502,27 +502,23 @@ func TestEmbedBase64Images(t *testing.T) { att2ImgBase64 := fmt.Sprintf(``, att2Base64) t.Run("ComposeMessage", func(t *testing.T) { - issue.Content = fmt.Sprintf(`MSG-BEFORE MSG-AFTER`, att1.UUID) - require.NoError(t, issues_model.UpdateIssueCols(t.Context(), issue, "content")) - subjectTemplates = texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) bodyTemplates = template.Must(template.New("issue/new").Parse(bodyTpl)) + issue.Content = fmt.Sprintf(`MSG-BEFORE MSG-AFTER`, att1.UUID) + require.NoError(t, issues_model.UpdateIssueCols(t.Context(), issue, "content")) + recipients := []*user_model.User{{Name: "Test", Email: "test@gitea.com"}} msgs, err := composeIssueCommentMessages(&mailCommentContext{ Context: t.Context(), Issue: issue, Doer: user, ActionType: activities_model.ActionCreateIssue, - Content: issue.Content, // strings.ReplaceAll(issue.Content, `src="`, `src="`+setting.AppURL), + Content: issue.Content, }, "en-US", recipients, false, "issue create") require.NoError(t, err) mailBody := msgs[0].Body - re := regexp.MustCompile(`MSG-BEFORE.*MSG-AFTER`) - matches := re.FindStringSubmatch(mailBody) - require.NotEmpty(t, matches) - mailBody = matches[0] assert.Regexp(t, `MSG-BEFORE ]+> MSG-AFTER`, mailBody) }) From 8f40b9f97f427e8440d7934f31e0135f6ebb2341 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 5 Mar 2025 10:29:29 +0800 Subject: [PATCH 10/10] fix size calc --- services/mailer/mail.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index f48f16c3a7a1f..f7e5b0c9f029f 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -131,7 +131,6 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct if attachment.Size+b64embedder.estimateSize > b64embedder.maxSize { return "", fmt.Errorf("total embedded images exceed max limit") } - b64embedder.estimateSize += attachment.Size fr, err := storage.Attachments.Open(attachment.RelativePath()) if err != nil { @@ -152,6 +151,7 @@ func (b64embedder *mailAttachmentBase64Embedder) AttachmentSrcToBase64DataURI(ct encoded := base64.StdEncoding.EncodeToString(content) dataURI := fmt.Sprintf("data:%s;base64,%s", mimeType.GetMimeType(), encoded) + b64embedder.estimateSize += int64(len(dataURI)) return dataURI, nil }