From f122aaf9ff627515922a68782339725e2d7c079a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Nov 2024 18:41:59 -0800 Subject: [PATCH 01/23] Use better name for userinfo structure (#32544) --- routers/web/auth/oauth2_provider.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index faea34959f..d844d42421 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -80,12 +80,12 @@ func (err errCallback) Error() string { } type userInfoResponse struct { - Sub string `json:"sub"` - Name string `json:"name"` - Username string `json:"preferred_username"` - Email string `json:"email"` - Picture string `json:"picture"` - Groups []string `json:"groups"` + Sub string `json:"sub"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` + Email string `json:"email"` + Picture string `json:"picture"` + Groups []string `json:"groups"` } // InfoOAuth manages request for userinfo endpoint @@ -97,11 +97,11 @@ func InfoOAuth(ctx *context.Context) { } response := &userInfoResponse{ - Sub: fmt.Sprint(ctx.Doer.ID), - Name: ctx.Doer.FullName, - Username: ctx.Doer.Name, - Email: ctx.Doer.Email, - Picture: ctx.Doer.AvatarLink(ctx), + Sub: fmt.Sprint(ctx.Doer.ID), + Name: ctx.Doer.FullName, + PreferredUsername: ctx.Doer.Name, + Email: ctx.Doer.Email, + Picture: ctx.Doer.AvatarLink(ctx), } groups, err := oauth2_provider.GetOAuthGroupsForUser(ctx, ctx.Doer) From 4f879a00df029e09b40f64bf8de0572704766115 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Nov 2024 19:06:25 -0800 Subject: [PATCH 02/23] Refactor find forks and fix possible bugs that weak permissions check (#32528) - Move models/GetForks to services/FindForks - Add doer as a parameter of FindForks to check permissions - Slight performance optimization for get forks API with batch loading of repository units - Add tests for forking repository to organizations --------- Co-authored-by: wxiaoguang --- models/repo/fork.go | 15 ------ models/repo/repo_list.go | 26 +++++++--- routers/api/v1/repo/fork.go | 15 ++++-- routers/web/repo/view.go | 23 ++++----- services/repository/fork.go | 24 +++++++++ templates/repo/forks.tmpl | 8 +-- tests/integration/api_fork_test.go | 80 +++++++++++++++++++++++++++++ tests/integration/repo_fork_test.go | 52 +++++++++++++++++++ 8 files changed, 202 insertions(+), 41 deletions(-) diff --git a/models/repo/fork.go b/models/repo/fork.go index 07cd31c269..1c75e86458 100644 --- a/models/repo/fork.go +++ b/models/repo/fork.go @@ -54,21 +54,6 @@ func GetUserFork(ctx context.Context, repoID, userID int64) (*Repository, error) return &forkedRepo, nil } -// GetForks returns all the forks of the repository -func GetForks(ctx context.Context, repo *Repository, listOptions db.ListOptions) ([]*Repository, error) { - sess := db.GetEngine(ctx) - - var forks []*Repository - if listOptions.Page == 0 { - forks = make([]*Repository, 0, repo.NumForks) - } else { - forks = make([]*Repository, 0, listOptions.PageSize) - sess = db.SetSessionPagination(sess, &listOptions) - } - - return forks, sess.Find(&forks, &Repository{ForkID: repo.ID}) -} - // IncrementRepoForkNum increment repository fork number func IncrementRepoForkNum(ctx context.Context, repoID int64) error { _, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET num_forks=num_forks+1 WHERE id=?", repoID) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 1bffadbf0a..9bed2e9197 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -98,8 +98,7 @@ func (repos RepositoryList) IDs() []int64 { return repoIDs } -// LoadAttributes loads the attributes for the given RepositoryList -func (repos RepositoryList) LoadAttributes(ctx context.Context) error { +func (repos RepositoryList) LoadOwners(ctx context.Context) error { if len(repos) == 0 { return nil } @@ -107,10 +106,6 @@ func (repos RepositoryList) LoadAttributes(ctx context.Context) error { userIDs := container.FilterSlice(repos, func(repo *Repository) (int64, bool) { return repo.OwnerID, true }) - repoIDs := make([]int64, len(repos)) - for i := range repos { - repoIDs[i] = repos[i].ID - } // Load owners. users := make(map[int64]*user_model.User, len(userIDs)) @@ -123,12 +118,19 @@ func (repos RepositoryList) LoadAttributes(ctx context.Context) error { for i := range repos { repos[i].Owner = users[repos[i].OwnerID] } + return nil +} + +func (repos RepositoryList) LoadLanguageStats(ctx context.Context) error { + if len(repos) == 0 { + return nil + } // Load primary language. stats := make(LanguageStatList, 0, len(repos)) if err := db.GetEngine(ctx). Where("`is_primary` = ? AND `language` != ?", true, "other"). - In("`repo_id`", repoIDs). + In("`repo_id`", repos.IDs()). Find(&stats); err != nil { return fmt.Errorf("find primary languages: %w", err) } @@ -141,10 +143,18 @@ func (repos RepositoryList) LoadAttributes(ctx context.Context) error { } } } - return nil } +// LoadAttributes loads the attributes for the given RepositoryList +func (repos RepositoryList) LoadAttributes(ctx context.Context) error { + if err := repos.LoadOwners(ctx); err != nil { + return err + } + + return repos.LoadLanguageStats(ctx) +} + // SearchRepoOptions holds the search options type SearchRepoOptions struct { db.ListOptions diff --git a/routers/api/v1/repo/fork.go b/routers/api/v1/repo/fork.go index a1e3c9804b..14a1a8d1c4 100644 --- a/routers/api/v1/repo/fork.go +++ b/routers/api/v1/repo/fork.go @@ -55,11 +55,20 @@ func ListForks(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - forks, err := repo_model.GetForks(ctx, ctx.Repo.Repository, utils.GetListOptions(ctx)) + forks, total, err := repo_service.FindForks(ctx, ctx.Repo.Repository, ctx.Doer, utils.GetListOptions(ctx)) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetForks", err) + ctx.Error(http.StatusInternalServerError, "FindForks", err) return } + if err := repo_model.RepositoryList(forks).LoadOwners(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadOwners", err) + return + } + if err := repo_model.RepositoryList(forks).LoadUnits(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadUnits", err) + return + } + apiForks := make([]*api.Repository, len(forks)) for i, fork := range forks { permission, err := access_model.GetUserRepoPermission(ctx, fork, ctx.Doer) @@ -70,7 +79,7 @@ func ListForks(ctx *context.APIContext) { apiForks[i] = convert.ToRepo(ctx, fork, permission) } - ctx.SetTotalCountHeader(int64(ctx.Repo.Repository.NumForks)) + ctx.SetTotalCountHeader(total) ctx.JSON(http.StatusOK, apiForks) } diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 7030f6d8a9..5d68ace29b 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -1151,26 +1151,25 @@ func Forks(ctx *context.Context) { if page <= 0 { page = 1 } + pageSize := setting.ItemsPerPage - pager := context.NewPagination(ctx.Repo.Repository.NumForks, setting.ItemsPerPage, page, 5) - ctx.Data["Page"] = pager - - forks, err := repo_model.GetForks(ctx, ctx.Repo.Repository, db.ListOptions{ - Page: pager.Paginater.Current(), - PageSize: setting.ItemsPerPage, + forks, total, err := repo_service.FindForks(ctx, ctx.Repo.Repository, ctx.Doer, db.ListOptions{ + Page: page, + PageSize: pageSize, }) if err != nil { - ctx.ServerError("GetForks", err) + ctx.ServerError("FindForks", err) return } - for _, fork := range forks { - if err = fork.LoadOwner(ctx); err != nil { - ctx.ServerError("LoadOwner", err) - return - } + if err := repo_model.RepositoryList(forks).LoadOwners(ctx); err != nil { + ctx.ServerError("LoadAttributes", err) + return } + pager := context.NewPagination(int(total), pageSize, page, 5) + ctx.Data["Page"] = pager + ctx.Data["Forks"] = forks ctx.HTML(http.StatusOK, tplForks) diff --git a/services/repository/fork.go b/services/repository/fork.go index 5b24015a03..bc4fdf8562 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" 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/git" "code.gitea.io/gitea/modules/gitrepo" @@ -20,6 +21,8 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" + + "xorm.io/builder" ) // ErrForkAlreadyExist represents a "ForkAlreadyExist" kind of error. @@ -247,3 +250,24 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit return err } + +type findForksOptions struct { + db.ListOptions + RepoID int64 + Doer *user_model.User +} + +func (opts findForksOptions) ToConds() builder.Cond { + return builder.Eq{"fork_id": opts.RepoID}.And( + repo_model.AccessibleRepositoryCondition(opts.Doer, unit.TypeInvalid), + ) +} + +// FindForks returns all the forks of the repository +func FindForks(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, listOptions db.ListOptions) ([]*repo_model.Repository, int64, error) { + return db.FindAndCount[repo_model.Repository](ctx, findForksOptions{ + ListOptions: listOptions, + RepoID: repo.ID, + Doer: doer, + }) +} diff --git a/templates/repo/forks.tmpl b/templates/repo/forks.tmpl index 412c59b60e..725b67c651 100644 --- a/templates/repo/forks.tmpl +++ b/templates/repo/forks.tmpl @@ -5,12 +5,14 @@

{{ctx.Locale.Tr "repo.forks"}}

+
{{range .Forks}} -
- {{ctx.AvatarUtils.Avatar .Owner}} - {{.Owner.Name}} / {{.Name}} +
+ {{ctx.AvatarUtils.Avatar .Owner}} + {{.Owner.Name}} / {{.Name}}
{{end}} +
{{template "base/paginate" .}} diff --git a/tests/integration/api_fork_test.go b/tests/integration/api_fork_test.go index 7c231415a3..357dd27f86 100644 --- a/tests/integration/api_fork_test.go +++ b/tests/integration/api_fork_test.go @@ -7,8 +7,16 @@ import ( "net/http" "testing" + "code.gitea.io/gitea/models" + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + org_model "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" ) func TestCreateForkNoLogin(t *testing.T) { @@ -16,3 +24,75 @@ func TestCreateForkNoLogin(t *testing.T) { req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{}) MakeRequest(t, req, http.StatusUnauthorized) } + +func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user1Sess := loginUser(t, "user1") + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user1"}) + + // fork into a limited org + limitedOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22}) + assert.EqualValues(t, api.VisibleTypeLimited, limitedOrg.Visibility) + + ownerTeam1, err := org_model.OrgFromUser(limitedOrg).GetOwnerTeam(db.DefaultContext) + assert.NoError(t, err) + assert.NoError(t, models.AddTeamMember(db.DefaultContext, ownerTeam1, user1)) + user1Token := getTokenForLoggedInUser(t, user1Sess, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteOrganization) + req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{ + Organization: &limitedOrg.Name, + }).AddTokenAuth(user1Token) + MakeRequest(t, req, http.StatusAccepted) + + // fork into a private org + user4Sess := loginUser(t, "user4") + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user4"}) + privateOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23}) + assert.EqualValues(t, api.VisibleTypePrivate, privateOrg.Visibility) + + ownerTeam2, err := org_model.OrgFromUser(privateOrg).GetOwnerTeam(db.DefaultContext) + assert.NoError(t, err) + assert.NoError(t, models.AddTeamMember(db.DefaultContext, ownerTeam2, user4)) + user4Token := getTokenForLoggedInUser(t, user4Sess, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteOrganization) + req = NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/forks", &api.CreateForkOption{ + Organization: &privateOrg.Name, + }).AddTokenAuth(user4Token) + MakeRequest(t, req, http.StatusAccepted) + + t.Run("Anonymous", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/forks") + resp := MakeRequest(t, req, http.StatusOK) + + var forks []*api.Repository + DecodeJSON(t, resp, &forks) + + assert.Empty(t, forks) + assert.EqualValues(t, "0", resp.Header().Get("X-Total-Count")) + }) + + t.Run("Logged in", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/forks").AddTokenAuth(user1Token) + resp := MakeRequest(t, req, http.StatusOK) + + var forks []*api.Repository + DecodeJSON(t, resp, &forks) + + assert.Len(t, forks, 1) + assert.EqualValues(t, "1", resp.Header().Get("X-Total-Count")) + + assert.NoError(t, models.AddTeamMember(db.DefaultContext, ownerTeam2, user1)) + + req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/forks").AddTokenAuth(user1Token) + resp = MakeRequest(t, req, http.StatusOK) + + forks = []*api.Repository{} + DecodeJSON(t, resp, &forks) + + assert.Len(t, forks, 2) + assert.EqualValues(t, "2", resp.Header().Get("X-Total-Count")) + }) +} diff --git a/tests/integration/repo_fork_test.go b/tests/integration/repo_fork_test.go index feebebf062..52b55888b9 100644 --- a/tests/integration/repo_fork_test.go +++ b/tests/integration/repo_fork_test.go @@ -9,8 +9,12 @@ import ( "net/http/httptest" "testing" + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + org_model "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -74,3 +78,51 @@ func TestRepoForkToOrg(t *testing.T) { _, exists := htmlDoc.doc.Find(`a.ui.button[href*="/fork"]`).Attr("href") assert.False(t, exists, "Forking should not be allowed anymore") } + +func TestForkListLimitedAndPrivateRepos(t *testing.T) { + defer tests.PrepareTestEnv(t)() + forkItemSelector := ".repo-fork-item" + + user1Sess := loginUser(t, "user1") + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user1"}) + + // fork to a limited org + limitedOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22}) + assert.EqualValues(t, structs.VisibleTypeLimited, limitedOrg.Visibility) + ownerTeam1, err := org_model.OrgFromUser(limitedOrg).GetOwnerTeam(db.DefaultContext) + assert.NoError(t, err) + assert.NoError(t, models.AddTeamMember(db.DefaultContext, ownerTeam1, user1)) + testRepoFork(t, user1Sess, "user2", "repo1", limitedOrg.Name, "repo1", "") + + // fork to a private org + user4Sess := loginUser(t, "user4") + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user4"}) + privateOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23}) + assert.EqualValues(t, structs.VisibleTypePrivate, privateOrg.Visibility) + ownerTeam2, err := org_model.OrgFromUser(privateOrg).GetOwnerTeam(db.DefaultContext) + assert.NoError(t, err) + assert.NoError(t, models.AddTeamMember(db.DefaultContext, ownerTeam2, user4)) + testRepoFork(t, user4Sess, "user2", "repo1", privateOrg.Name, "repo1", "") + + t.Run("Anonymous", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + req := NewRequest(t, "GET", "/user2/repo1/forks") + resp := MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + assert.EqualValues(t, 0, htmlDoc.Find(forkItemSelector).Length()) + }) + + t.Run("Logged in", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user2/repo1/forks") + resp := user1Sess.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + assert.EqualValues(t, 1, htmlDoc.Find(forkItemSelector).Length()) + + assert.NoError(t, models.AddTeamMember(db.DefaultContext, ownerTeam2, user1)) + resp = user1Sess.MakeRequest(t, req, http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + assert.EqualValues(t, 2, htmlDoc.Find(forkItemSelector).Length()) + }) +} From 8a20fba8eb1ac01a0de9355eff84af69d4636d96 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 18 Nov 2024 13:25:42 +0800 Subject: [PATCH 03/23] Refactor markup render system (#32533) Remove unmaintainable sanitizer rules. No need to add special "class" regexp rules anymore, use RenderInternal.SafeAttr instead, more details (and examples) are in the tests --- modules/html/html.go | 25 --- modules/htmlutil/html.go | 48 +++++ modules/htmlutil/html_test.go | 15 ++ modules/markup/asciicast/asciicast.go | 15 +- modules/markup/common/html.go | 16 -- modules/markup/common/linkify.go | 18 +- modules/markup/console/console.go | 7 +- modules/markup/csv/csv.go | 11 +- modules/markup/external/external.go | 9 +- modules/markup/html.go | 110 ++++------ modules/markup/html_codepreview.go | 13 +- modules/markup/html_email.go | 2 +- modules/markup/html_emoji.go | 16 +- modules/markup/html_issue.go | 14 +- modules/markup/html_link.go | 6 +- modules/markup/html_mention.go | 4 +- modules/markup/internal/finalprocessor.go | 30 +++ modules/markup/internal/internal_test.go | 61 ++++++ modules/markup/internal/renderinternal.go | 82 ++++++++ modules/markup/markdown/ast.go | 30 +-- modules/markup/markdown/goldmark.go | 27 ++- modules/markup/markdown/markdown.go | 192 ++++++++---------- modules/markup/markdown/markdown_test.go | 14 ++ .../markup/markdown/math/block_renderer.go | 12 +- .../markup/markdown/math/inline_renderer.go | 12 +- modules/markup/markdown/math/math.go | 39 +--- modules/markup/markdown/meta_test.go | 6 +- .../markup/markdown/transform_blockquote.go | 7 +- modules/markup/markdown/transform_codespan.go | 3 +- modules/markup/markdown/transform_list.go | 2 +- modules/markup/render.go | 79 ++++--- modules/markup/sanitizer_custom.go | 10 +- modules/markup/sanitizer_default.go | 63 +----- modules/markup/sanitizer_default_test.go | 3 - modules/setting/markup.go | 21 +- modules/svg/svg.go | 2 +- modules/templates/helper.go | 21 +- modules/templates/helper_test.go | 4 - modules/templates/util_avatar.go | 2 +- modules/templates/util_render.go | 5 +- modules/templates/util_render_test.go | 18 +- routers/web/repo/wiki.go | 2 +- 42 files changed, 568 insertions(+), 508 deletions(-) delete mode 100644 modules/html/html.go create mode 100644 modules/htmlutil/html.go create mode 100644 modules/htmlutil/html_test.go delete mode 100644 modules/markup/common/html.go create mode 100644 modules/markup/internal/finalprocessor.go create mode 100644 modules/markup/internal/internal_test.go create mode 100644 modules/markup/internal/renderinternal.go diff --git a/modules/html/html.go b/modules/html/html.go deleted file mode 100644 index b1ebd584c6..0000000000 --- a/modules/html/html.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package html - -// ParseSizeAndClass get size and class from string with default values -// If present, "others" expects the new size first and then the classes to use -func ParseSizeAndClass(defaultSize int, defaultClass string, others ...any) (int, string) { - size := defaultSize - if len(others) >= 1 { - if v, ok := others[0].(int); ok && v != 0 { - size = v - } - } - class := defaultClass - if len(others) >= 2 { - if v, ok := others[1].(string); ok && v != "" { - if class != "" { - class += " " - } - class += v - } - } - return size, class -} diff --git a/modules/htmlutil/html.go b/modules/htmlutil/html.go new file mode 100644 index 0000000000..9b5f5a92d8 --- /dev/null +++ b/modules/htmlutil/html.go @@ -0,0 +1,48 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package htmlutil + +import ( + "fmt" + "html/template" + "slices" +) + +// ParseSizeAndClass get size and class from string with default values +// If present, "others" expects the new size first and then the classes to use +func ParseSizeAndClass(defaultSize int, defaultClass string, others ...any) (int, string) { + size := defaultSize + if len(others) >= 1 { + if v, ok := others[0].(int); ok && v != 0 { + size = v + } + } + class := defaultClass + if len(others) >= 2 { + if v, ok := others[1].(string); ok && v != "" { + if class != "" { + class += " " + } + class += v + } + } + return size, class +} + +func HTMLFormat(s string, rawArgs ...any) template.HTML { + args := slices.Clone(rawArgs) + for i, v := range args { + switch v := v.(type) { + case nil, bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64, template.HTML: + // for most basic types (including template.HTML which is safe), just do nothing and use it + case string: + args[i] = template.HTMLEscapeString(v) + case fmt.Stringer: + args[i] = template.HTMLEscapeString(v.String()) + default: + args[i] = template.HTMLEscapeString(fmt.Sprint(v)) + } + } + return template.HTML(fmt.Sprintf(s, args...)) +} diff --git a/modules/htmlutil/html_test.go b/modules/htmlutil/html_test.go new file mode 100644 index 0000000000..5ff05d75b3 --- /dev/null +++ b/modules/htmlutil/html_test.go @@ -0,0 +1,15 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package htmlutil + +import ( + "html/template" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHTMLFormat(t *testing.T) { + assert.Equal(t, template.HTML("< < 1"), HTMLFormat("%s %s %d", "<", template.HTML("<"), 1)) +} diff --git a/modules/markup/asciicast/asciicast.go b/modules/markup/asciicast/asciicast.go index 0678062340..e92b78a4bc 100644 --- a/modules/markup/asciicast/asciicast.go +++ b/modules/markup/asciicast/asciicast.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "net/url" - "regexp" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" @@ -38,10 +37,7 @@ const ( // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { - return []setting.MarkupSanitizerRule{ - {Element: "div", AllowAttr: "class", Regexp: regexp.MustCompile(playerClassName)}, - {Element: "div", AllowAttr: playerSrcAttr}, - } + return []setting.MarkupSanitizerRule{{Element: "div", AllowAttr: playerSrcAttr}} } // Render implements markup.Renderer @@ -53,12 +49,5 @@ func (Renderer) Render(ctx *markup.RenderContext, _ io.Reader, output io.Writer) ctx.Metas["BranchNameSubURL"], url.PathEscape(ctx.RelativePath), ) - - _, err := io.WriteString(output, fmt.Sprintf( - `
`, - playerClassName, - playerSrcAttr, - rawURL, - )) - return err + return ctx.RenderInternal.FormatWithSafeAttrs(output, `
`, playerClassName, playerSrcAttr, rawURL) } diff --git a/modules/markup/common/html.go b/modules/markup/common/html.go deleted file mode 100644 index 5658839c6f..0000000000 --- a/modules/markup/common/html.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package common - -import ( - "mvdan.cc/xurls/v2" -) - -// NOTE: All below regex matching do not perform any extra validation. -// Thus a link is produced even if the linked entity does not exist. -// While fast, this is also incorrect and lead to false positives. -// TODO: fix invalid linking issue - -// LinkRegex is a regexp matching a valid link -var LinkRegex, _ = xurls.StrictMatchingScheme("https?://") diff --git a/modules/markup/common/linkify.go b/modules/markup/common/linkify.go index f84680205e..be6ab22b55 100644 --- a/modules/markup/common/linkify.go +++ b/modules/markup/common/linkify.go @@ -9,15 +9,27 @@ package common import ( "bytes" "regexp" + "sync" "github.com/yuin/goldmark" "github.com/yuin/goldmark/ast" "github.com/yuin/goldmark/parser" "github.com/yuin/goldmark/text" "github.com/yuin/goldmark/util" + "mvdan.cc/xurls/v2" ) -var wwwURLRegxp = regexp.MustCompile(`^www\.[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}((?:/|[#?])[-a-zA-Z0-9@:%_\+.~#!?&//=\(\);,'">\^{}\[\]` + "`" + `]*)?`) +type GlobalVarsType struct { + wwwURLRegxp *regexp.Regexp + LinkRegex *regexp.Regexp // fast matching a URL link, no any extra validation. +} + +var GlobalVars = sync.OnceValue[*GlobalVarsType](func() *GlobalVarsType { + v := &GlobalVarsType{} + v.wwwURLRegxp = regexp.MustCompile(`^www\.[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}((?:/|[#?])[-a-zA-Z0-9@:%_\+.~#!?&//=\(\);,'">\^{}\[\]` + "`" + `]*)?`) + v.LinkRegex, _ = xurls.StrictMatchingScheme("https?://") + return v +}) type linkifyParser struct{} @@ -60,10 +72,10 @@ func (s *linkifyParser) Parse(parent ast.Node, block text.Reader, pc parser.Cont var protocol []byte typ := ast.AutoLinkURL if bytes.HasPrefix(line, protoHTTP) || bytes.HasPrefix(line, protoHTTPS) || bytes.HasPrefix(line, protoFTP) { - m = LinkRegex.FindSubmatchIndex(line) + m = GlobalVars().LinkRegex.FindSubmatchIndex(line) } if m == nil && bytes.HasPrefix(line, domainWWW) { - m = wwwURLRegxp.FindSubmatchIndex(line) + m = GlobalVars().wwwURLRegxp.FindSubmatchIndex(line) protocol = []byte("http") } if m != nil { diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go index d991527b80..06f3acfa68 100644 --- a/modules/markup/console/console.go +++ b/modules/markup/console/console.go @@ -6,8 +6,7 @@ package console import ( "bytes" "io" - "path/filepath" - "regexp" + "path" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" @@ -36,7 +35,7 @@ func (Renderer) Extensions() []string { // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{ - {Element: "span", AllowAttr: "class", Regexp: regexp.MustCompile(`^term-((fg[ix]?|bg)\d+|container)$`)}, + {Element: "span", AllowAttr: "class", Regexp: `^term-((fg[ix]?|bg)\d+|container)$`}, } } @@ -46,7 +45,7 @@ func (Renderer) CanRender(filename string, input io.Reader) bool { if err != nil { return false } - if enry.GetLanguage(filepath.Base(filename), buf) != enry.OtherLanguage { + if enry.GetLanguage(path.Base(filename), buf) != enry.OtherLanguage { return false } return bytes.ContainsRune(buf, '\x1b') diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 3d952b0de4..a3e6bbaac6 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -7,7 +7,6 @@ import ( "bufio" "html" "io" - "regexp" "strconv" "code.gitea.io/gitea/modules/csv" @@ -37,9 +36,9 @@ func (Renderer) Extensions() []string { // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{ - {Element: "table", AllowAttr: "class", Regexp: regexp.MustCompile(`data-table`)}, - {Element: "th", AllowAttr: "class", Regexp: regexp.MustCompile(`line-num`)}, - {Element: "td", AllowAttr: "class", Regexp: regexp.MustCompile(`line-num`)}, + {Element: "table", AllowAttr: "class", Regexp: `^data-table$`}, + {Element: "th", AllowAttr: "class", Regexp: `^line-num$`}, + {Element: "td", AllowAttr: "class", Regexp: `^line-num$`}, } } @@ -51,13 +50,13 @@ func writeField(w io.Writer, element, class, field string) error { return err } if len(class) > 0 { - if _, err := io.WriteString(w, " class=\""); err != nil { + if _, err := io.WriteString(w, ` class="`); err != nil { return err } if _, err := io.WriteString(w, class); err != nil { return err } - if _, err := io.WriteString(w, "\""); err != nil { + if _, err := io.WriteString(w, `"`); err != nil { return err } } diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 122517ed11..d28dc9fa5d 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -102,7 +102,7 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io. _, err = io.Copy(f, input) if err != nil { - f.Close() + _ = f.Close() return fmt.Errorf("%s write data to temp file when rendering %s failed: %w", p.Name(), p.Command, err) } @@ -113,10 +113,9 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io. args = append(args, f.Name()) } - if ctx == nil || ctx.Ctx == nil { - if ctx == nil { - log.Warn("RenderContext not provided defaulting to empty ctx") - ctx = &markup.RenderContext{} + if ctx.Ctx == nil { + if !setting.IsProd || setting.IsInTesting { + panic("RenderContext did not provide context") } log.Warn("RenderContext did not provide context, defaulting to Shutdown context") ctx.Ctx = graceful.GetManager().ShutdownContext() diff --git a/modules/markup/html.go b/modules/markup/html.go index 16ccd4b406..e8799c401c 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -25,9 +25,6 @@ const ( IssueNameStyleRegexp = "regexp" ) -// CSS class for action keywords (e.g. "closes: #1") -const keywordClass = "issue-keyword" - type globalVarsType struct { hashCurrentPattern *regexp.Regexp shortLinkPattern *regexp.Regexp @@ -39,6 +36,7 @@ type globalVarsType struct { emojiShortCodeRegex *regexp.Regexp issueFullPattern *regexp.Regexp filesChangedFullPattern *regexp.Regexp + codePreviewPattern *regexp.Regexp tagCleaner *regexp.Regexp nulCleaner *strings.Replacer @@ -88,6 +86,9 @@ var globalVars = sync.OnceValue[*globalVarsType](func() *globalVarsType { // example: https://domain/org/repo/pulls/27/files#hash v.filesChangedFullPattern = regexp.MustCompile(`https?://(?:\S+/)[\w_.-]+/[\w_.-]+/pulls/((?:\w{1,10}-)?[1-9][0-9]*)/files([\?|#](\S+)?)?\b`) + // codePreviewPattern matches "http://domain/.../{owner}/{repo}/src/commit/{commit}/{filepath}#L10-L20" + v.codePreviewPattern = regexp.MustCompile(`https?://\S+/([^\s/]+)/([^\s/]+)/src/commit/([0-9a-f]{7,64})(/\S+)#(L\d+(-L\d+)?)`) + v.tagCleaner = regexp.MustCompile(`<((?:/?\w+/\w+)|(?:/[\w ]+/)|(/?[hH][tT][mM][lL]\b)|(/?[hH][eE][aA][dD]\b))`) v.nulCleaner = strings.NewReplacer("\000", "") return v @@ -129,7 +130,7 @@ func CustomLinkURLSchemes(schemes []string) { } withAuth = append(withAuth, s) } - common.LinkRegex, _ = xurls.StrictMatchingScheme(strings.Join(withAuth, "|")) + common.GlobalVars().LinkRegex, _ = xurls.StrictMatchingScheme(strings.Join(withAuth, "|")) } type postProcessError struct { @@ -164,11 +165,7 @@ var defaultProcessors = []processor{ // emails with HTML links, parsing shortlinks in the format of [[Link]], like // MediaWiki, linking issues in the format #ID, and mentions in the format // @user, and others. -func PostProcess( - ctx *RenderContext, - input io.Reader, - output io.Writer, -) error { +func PostProcess(ctx *RenderContext, input io.Reader, output io.Writer) error { return postProcess(ctx, defaultProcessors, input, output) } @@ -189,10 +186,7 @@ var commitMessageProcessors = []processor{ // RenderCommitMessage will use the same logic as PostProcess, but will disable // the shortLinkProcessor and will add a defaultLinkProcessor if defaultLink is // set, which changes every text node into a link to the passed default link. -func RenderCommitMessage( - ctx *RenderContext, - content string, -) (string, error) { +func RenderCommitMessage(ctx *RenderContext, content string) (string, error) { procs := commitMessageProcessors return renderProcessString(ctx, procs, content) } @@ -219,10 +213,7 @@ var emojiProcessors = []processor{ // RenderCommitMessage, but will disable the shortLinkProcessor and // emailAddressProcessor, will add a defaultLinkProcessor if defaultLink is set, // which changes every text node into a link to the passed default link. -func RenderCommitMessageSubject( - ctx *RenderContext, - defaultLink, content string, -) (string, error) { +func RenderCommitMessageSubject(ctx *RenderContext, defaultLink, content string) (string, error) { procs := slices.Clone(commitMessageSubjectProcessors) procs = append(procs, func(ctx *RenderContext, node *html.Node) { ch := &html.Node{Parent: node, Type: html.TextNode, Data: node.Data} @@ -236,10 +227,7 @@ func RenderCommitMessageSubject( } // RenderIssueTitle to process title on individual issue/pull page -func RenderIssueTitle( - ctx *RenderContext, - title string, -) (string, error) { +func RenderIssueTitle(ctx *RenderContext, title string) (string, error) { // do not render other issue/commit links in an issue's title - which in most cases is already a link. return renderProcessString(ctx, []processor{ emojiShortCodeProcessor, @@ -257,10 +245,7 @@ func renderProcessString(ctx *RenderContext, procs []processor, content string) // RenderDescriptionHTML will use similar logic as PostProcess, but will // use a single special linkProcessor. -func RenderDescriptionHTML( - ctx *RenderContext, - content string, -) (string, error) { +func RenderDescriptionHTML(ctx *RenderContext, content string) (string, error) { return renderProcessString(ctx, []processor{ descriptionLinkProcessor, emojiShortCodeProcessor, @@ -270,10 +255,7 @@ func RenderDescriptionHTML( // RenderEmoji for when we want to just process emoji and shortcodes // in various places it isn't already run through the normal markdown processor -func RenderEmoji( - ctx *RenderContext, - content string, -) (string, error) { +func RenderEmoji(ctx *RenderContext, content string) (string, error) { return renderProcessString(ctx, emojiProcessors, content) } @@ -333,6 +315,17 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output return nil } +func isEmojiNode(node *html.Node) bool { + if node.Type == html.ElementNode && node.Data == atom.Span.String() { + for _, attr := range node.Attr { + if (attr.Key == "class" || attr.Key == "data-attr-class") && strings.Contains(attr.Val, "emoji") { + return true + } + } + } + return false +} + func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Node { // Add user-content- to IDs and "#" links if they don't already have them for idx, attr := range node.Attr { @@ -346,47 +339,27 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Nod if attr.Key == "href" && strings.HasPrefix(attr.Val, "#") && notHasPrefix { node.Attr[idx].Val = "#user-content-" + val } - - if attr.Key == "class" && attr.Val == "emoji" { - procs = nil - } } switch node.Type { case html.TextNode: - processTextNodes(ctx, procs, node) + for _, proc := range procs { + proc(ctx, node) // it might add siblings + } + case html.ElementNode: - if node.Data == "code" || node.Data == "pre" { - // ignore code and pre nodes + if isEmojiNode(node) { + // TextNode emoji will be converted to ``, then the next iteration will visit the "span" + // if we don't stop it, it will go into the TextNode again and create an infinite recursion return node.NextSibling + } else if node.Data == "code" || node.Data == "pre" { + return node.NextSibling // ignore code and pre nodes } else if node.Data == "img" { return visitNodeImg(ctx, node) } else if node.Data == "video" { return visitNodeVideo(ctx, node) } else if node.Data == "a" { - // Restrict text in links to emojis - procs = emojiProcessors - } else if node.Data == "i" { - for _, attr := range node.Attr { - if attr.Key != "class" { - continue - } - classes := strings.Split(attr.Val, " ") - for i, class := range classes { - if class == "icon" { - classes[0], classes[i] = classes[i], classes[0] - attr.Val = strings.Join(classes, " ") - - // Remove all children of icons - child := node.FirstChild - for child != nil { - node.RemoveChild(child) - child = node.FirstChild - } - break - } - } - } + procs = emojiProcessors // Restrict text in links to emojis } for n := node.FirstChild; n != nil; { n = visitNode(ctx, procs, n) @@ -396,22 +369,17 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Nod return node.NextSibling } -// processTextNodes runs the passed node through various processors, in order to handle -// all kinds of special links handled by the post-processing. -func processTextNodes(ctx *RenderContext, procs []processor, node *html.Node) { - for _, p := range procs { - p(ctx, node) - } -} - // createKeyword() renders a highlighted version of an action keyword -func createKeyword(content string) *html.Node { +func createKeyword(ctx *RenderContext, content string) *html.Node { + // CSS class for action keywords (e.g. "closes: #1") + const keywordClass = "issue-keyword" + span := &html.Node{ Type: html.ElementNode, Data: atom.Span.String(), Attr: []html.Attribute{}, } - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: keywordClass}) + span.Attr = append(span.Attr, ctx.RenderInternal.NodeSafeAttr("class", keywordClass)) text := &html.Node{ Type: html.TextNode, @@ -422,7 +390,7 @@ func createKeyword(content string) *html.Node { return span } -func createLink(href, content, class string) *html.Node { +func createLink(ctx *RenderContext, href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, Data: atom.A.String(), @@ -432,7 +400,7 @@ func createLink(href, content, class string) *html.Node { a.Attr = append(a.Attr, html.Attribute{Key: "data-markdown-generated-content"}) } if class != "" { - a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) + a.Attr = append(a.Attr, ctx.RenderInternal.NodeSafeAttr("class", class)) } text := &html.Node{ diff --git a/modules/markup/html_codepreview.go b/modules/markup/html_codepreview.go index 5ab9290b3e..5c88481d76 100644 --- a/modules/markup/html_codepreview.go +++ b/modules/markup/html_codepreview.go @@ -6,7 +6,6 @@ package markup import ( "html/template" "net/url" - "regexp" "strconv" "strings" @@ -16,9 +15,6 @@ import ( "golang.org/x/net/html" ) -// codePreviewPattern matches "http://domain/.../{owner}/{repo}/src/commit/{commit}/{filepath}#L10-L20" -var codePreviewPattern = regexp.MustCompile(`https?://\S+/([^\s/]+)/([^\s/]+)/src/commit/([0-9a-f]{7,64})(/\S+)#(L\d+(-L\d+)?)`) - type RenderCodePreviewOptions struct { FullURL string OwnerName string @@ -30,7 +26,7 @@ type RenderCodePreviewOptions struct { } func renderCodeBlock(ctx *RenderContext, node *html.Node) (urlPosStart, urlPosStop int, htm template.HTML, err error) { - m := codePreviewPattern.FindStringSubmatchIndex(node.Data) + m := globalVars().codePreviewPattern.FindStringSubmatchIndex(node.Data) if m == nil { return 0, 0, "", nil } @@ -66,8 +62,8 @@ func codePreviewPatternProcessor(ctx *RenderContext, node *html.Node) { node = node.NextSibling continue } - urlPosStart, urlPosEnd, h, err := renderCodeBlock(ctx, node) - if err != nil || h == "" { + urlPosStart, urlPosEnd, renderedCodeBlock, err := renderCodeBlock(ctx, node) + if err != nil || renderedCodeBlock == "" { if err != nil { log.Error("Unable to render code preview: %v", err) } @@ -84,7 +80,8 @@ func codePreviewPatternProcessor(ctx *RenderContext, node *html.Node) { // then it is resolved as: "

{TextBefore}

{TextAfter}

", // so unless it could correctly replace the parent "p/li" node, it is very difficult to eliminate the "TextBefore" empty node. node.Data = textBefore - node.Parent.InsertBefore(&html.Node{Type: html.RawNode, Data: string(h)}, next) + renderedCodeNode := &html.Node{Type: html.RawNode, Data: string(ctx.RenderInternal.ProtectSafeAttrs(renderedCodeBlock))} + node.Parent.InsertBefore(renderedCodeNode, next) if textAfter != "" { node.Parent.InsertBefore(&html.Node{Type: html.TextNode, Data: textAfter}, next) } diff --git a/modules/markup/html_email.go b/modules/markup/html_email.go index 32d0285eb4..cbfae8b829 100644 --- a/modules/markup/html_email.go +++ b/modules/markup/html_email.go @@ -15,7 +15,7 @@ func emailAddressProcessor(ctx *RenderContext, node *html.Node) { } mail := node.Data[m[2]:m[3]] - replaceContent(node, m[2], m[3], createLink("mailto:"+mail, mail, "mailto")) + replaceContent(node, m[2], m[3], createLink(ctx, "mailto:"+mail, mail, "" /*mailto*/)) node = node.NextSibling.NextSibling } } diff --git a/modules/markup/html_emoji.go b/modules/markup/html_emoji.go index 6eacb2067f..c638065425 100644 --- a/modules/markup/html_emoji.go +++ b/modules/markup/html_emoji.go @@ -13,15 +13,13 @@ import ( "golang.org/x/net/html/atom" ) -func createEmoji(content, class, name string) *html.Node { +func createEmoji(ctx *RenderContext, content, name string) *html.Node { span := &html.Node{ Type: html.ElementNode, Data: atom.Span.String(), Attr: []html.Attribute{}, } - if class != "" { - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: class}) - } + span.Attr = append(span.Attr, ctx.RenderInternal.NodeSafeAttr("class", "emoji")) if name != "" { span.Attr = append(span.Attr, html.Attribute{Key: "aria-label", Val: name}) } @@ -35,13 +33,13 @@ func createEmoji(content, class, name string) *html.Node { return span } -func createCustomEmoji(alias string) *html.Node { +func createCustomEmoji(ctx *RenderContext, alias string) *html.Node { span := &html.Node{ Type: html.ElementNode, Data: atom.Span.String(), Attr: []html.Attribute{}, } - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: "emoji"}) + span.Attr = append(span.Attr, ctx.RenderInternal.NodeSafeAttr("class", "emoji")) span.Attr = append(span.Attr, html.Attribute{Key: "aria-label", Val: alias}) img := &html.Node{ @@ -77,7 +75,7 @@ func emojiShortCodeProcessor(ctx *RenderContext, node *html.Node) { if converted == nil { // check if this is a custom reaction if _, exist := setting.UI.CustomEmojisMap[alias]; exist { - replaceContent(node, m[0], m[1], createCustomEmoji(alias)) + replaceContent(node, m[0], m[1], createCustomEmoji(ctx, alias)) node = node.NextSibling.NextSibling start = 0 continue @@ -85,7 +83,7 @@ func emojiShortCodeProcessor(ctx *RenderContext, node *html.Node) { continue } - replaceContent(node, m[0], m[1], createEmoji(converted.Emoji, "emoji", converted.Description)) + replaceContent(node, m[0], m[1], createEmoji(ctx, converted.Emoji, converted.Description)) node = node.NextSibling.NextSibling start = 0 } @@ -107,7 +105,7 @@ func emojiProcessor(ctx *RenderContext, node *html.Node) { start = m[1] val := emoji.FromCode(codepoint) if val != nil { - replaceContent(node, m[0], m[1], createEmoji(codepoint, "emoji", val.Description)) + replaceContent(node, m[0], m[1], createEmoji(ctx, codepoint, val.Description)) node = node.NextSibling.NextSibling start = 0 } diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go index 2acf154ad2..7341af7eb6 100644 --- a/modules/markup/html_issue.go +++ b/modules/markup/html_issue.go @@ -57,10 +57,10 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { matchRepo := linkParts[len(linkParts)-3] if matchOrg == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] { - replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) + replaceContent(node, m[0], m[1], createLink(ctx, link, text, "ref-issue")) } else { text = matchOrg + "/" + matchRepo + text - replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) + replaceContent(node, m[0], m[1], createLink(ctx, link, text, "ref-issue")) } node = node.NextSibling.NextSibling } @@ -129,16 +129,16 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { log.Error("unable to expand template vars for ref %s, err: %v", ref.Issue, err) } - link = createLink(res, reftext, "ref-issue ref-external-issue") + link = createLink(ctx, res, reftext, "ref-issue ref-external-issue") } else { // Path determines the type of link that will be rendered. It's unknown at this point whether // the linked item is actually a PR or an issue. Luckily it's of no real consequence because // Gitea will redirect on click as appropriate. issuePath := util.Iif(ref.IsPull, "pulls", "issues") if ref.Owner == "" { - link = createLink(util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], issuePath, ref.Issue), reftext, "ref-issue") + link = createLink(ctx, util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], issuePath, ref.Issue), reftext, "ref-issue") } else { - link = createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, issuePath, ref.Issue), reftext, "ref-issue") + link = createLink(ctx, util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, issuePath, ref.Issue), reftext, "ref-issue") } } @@ -151,7 +151,7 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { // Decorate action keywords if actionable var keyword *html.Node if references.IsXrefActionable(ref, hasExtTrackFormat) { - keyword = createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) + keyword = createKeyword(ctx, node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) } else { keyword = &html.Node{ Type: html.TextNode, @@ -177,7 +177,7 @@ func commitCrossReferencePatternProcessor(ctx *RenderContext, node *html.Node) { } reftext := ref.Owner + "/" + ref.Name + "@" + base.ShortSha(ref.CommitSha) - link := createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") + link := createLink(ctx, util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) node = node.NextSibling.NextSibling diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index b7562d0aa6..32aa7dc614 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -189,13 +189,13 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) { func linkProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling for node != nil && node != next { - m := common.LinkRegex.FindStringIndex(node.Data) + m := common.GlobalVars().LinkRegex.FindStringIndex(node.Data) if m == nil { return } uri := node.Data[m[0]:m[1]] - replaceContent(node, m[0], m[1], createLink(uri, uri, "link")) + replaceContent(node, m[0], m[1], createLink(ctx, uri, uri, "" /*link*/)) node = node.NextSibling.NextSibling } } @@ -204,7 +204,7 @@ func linkProcessor(ctx *RenderContext, node *html.Node) { func descriptionLinkProcessor(ctx *RenderContext, node *html.Node) { next := node.NextSibling for node != nil && node != next { - m := common.LinkRegex.FindStringIndex(node.Data) + m := common.GlobalVars().LinkRegex.FindStringIndex(node.Data) if m == nil { return } diff --git a/modules/markup/html_mention.go b/modules/markup/html_mention.go index 3f0692e05f..f7e2ad50f1 100644 --- a/modules/markup/html_mention.go +++ b/modules/markup/html_mention.go @@ -33,7 +33,7 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { if ok && strings.Contains(mention, "/") { mentionOrgAndTeam := strings.Split(mention, "/") if mentionOrgAndTeam[0][1:] == ctx.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "mention")) + replaceContent(node, loc.Start, loc.End, createLink(ctx, util.URLJoin(ctx.Links.Prefix(), "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "" /*mention*/)) node = node.NextSibling.NextSibling start = 0 continue @@ -44,7 +44,7 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { mentionedUsername := mention[1:] if DefaultProcessorHelper.IsUsernameMentionable != nil && DefaultProcessorHelper.IsUsernameMentionable(ctx.Ctx, mentionedUsername) { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), mentionedUsername), mention, "mention")) + replaceContent(node, loc.Start, loc.End, createLink(ctx, util.URLJoin(ctx.Links.Prefix(), mentionedUsername), mention, "" /*mention*/)) node = node.NextSibling.NextSibling start = 0 } else { diff --git a/modules/markup/internal/finalprocessor.go b/modules/markup/internal/finalprocessor.go new file mode 100644 index 0000000000..14d46a161f --- /dev/null +++ b/modules/markup/internal/finalprocessor.go @@ -0,0 +1,30 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package internal + +import ( + "bytes" + "io" +) + +type finalProcessor struct { + renderInternal *RenderInternal + + output io.Writer + buf bytes.Buffer +} + +func (p *finalProcessor) Write(data []byte) (int, error) { + p.buf.Write(data) + return len(data), nil +} + +func (p *finalProcessor) Close() error { + // TODO: reading the whole markdown isn't a problem at the moment, + // because "postProcess" already does so. In the future we could optimize the code to process data on the fly. + buf := p.buf.Bytes() + buf = bytes.ReplaceAll(buf, []byte(` data-attr-class="`+p.renderInternal.secureIDPrefix), []byte(` class="`)) + _, err := p.output.Write(buf) + return err +} diff --git a/modules/markup/internal/internal_test.go b/modules/markup/internal/internal_test.go new file mode 100644 index 0000000000..98ff3bc079 --- /dev/null +++ b/modules/markup/internal/internal_test.go @@ -0,0 +1,61 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package internal + +import ( + "bytes" + "html/template" + "io" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRenderInternal(t *testing.T) { + cases := []struct { + input, protected, recovered string + }{ + { + input: `
class="content"
`, + protected: `
class="content"
`, + recovered: `
class="content"
`, + }, + { + input: "
", + protected: `
`, + recovered: `
`, + }, + } + for _, c := range cases { + var r RenderInternal + out := &bytes.Buffer{} + in := r.init("sec", out) + protected := r.ProtectSafeAttrs(template.HTML(c.input)) + assert.EqualValues(t, c.protected, protected) + _, _ = io.WriteString(in, string(protected)) + _ = in.Close() + assert.EqualValues(t, c.recovered, out.String()) + } + + var r1, r2 RenderInternal + protected := r1.ProtectSafeAttrs(`
`) + assert.EqualValues(t, `
`, protected, "non-initialized RenderInternal should not protect any attributes") + _ = r1.init("sec", nil) + protected = r1.ProtectSafeAttrs(`
`) + assert.EqualValues(t, `
`, protected) + assert.EqualValues(t, "data-attr-class", r1.SafeAttr("class")) + assert.EqualValues(t, "sec:val", r1.SafeValue("val")) + recovered, ok := r1.RecoverProtectedValue("sec:val") + assert.True(t, ok) + assert.EqualValues(t, "val", recovered) + recovered, ok = r1.RecoverProtectedValue("other:val") + assert.False(t, ok) + assert.Empty(t, recovered) + + out2 := &bytes.Buffer{} + in2 := r2.init("sec-other", out2) + _, _ = io.WriteString(in2, string(protected)) + _ = in2.Close() + assert.EqualValues(t, `
`, out2.String(), "different secureID should not recover the value") +} diff --git a/modules/markup/internal/renderinternal.go b/modules/markup/internal/renderinternal.go new file mode 100644 index 0000000000..4775fecfc7 --- /dev/null +++ b/modules/markup/internal/renderinternal.go @@ -0,0 +1,82 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package internal + +import ( + "crypto/rand" + "encoding/base64" + "html/template" + "io" + "regexp" + "strings" + "sync" + + "code.gitea.io/gitea/modules/htmlutil" + + "golang.org/x/net/html" +) + +var reAttrClass = sync.OnceValue[*regexp.Regexp](func() *regexp.Regexp { + // TODO: it isn't a problem at the moment because our HTML contents are always well constructed + return regexp.MustCompile(`(<[^>]+)\s+class="([^"]+)"([^>]*>)`) +}) + +// RenderInternal also works without initialization +// If no initialization (no secureID), it will not protect any attributes and return the original name&value +type RenderInternal struct { + secureID string + secureIDPrefix string +} + +func (r *RenderInternal) Init(output io.Writer) io.WriteCloser { + buf := make([]byte, 12) + _, err := rand.Read(buf) + if err != nil { + panic("unable to generate secure id") + } + return r.init(base64.URLEncoding.EncodeToString(buf), output) +} + +func (r *RenderInternal) init(secID string, output io.Writer) io.WriteCloser { + r.secureID = secID + r.secureIDPrefix = r.secureID + ":" + return &finalProcessor{renderInternal: r, output: output} +} + +func (r *RenderInternal) RecoverProtectedValue(v string) (string, bool) { + if !strings.HasPrefix(v, r.secureIDPrefix) { + return "", false + } + return v[len(r.secureIDPrefix):], true +} + +func (r *RenderInternal) SafeAttr(name string) string { + if r.secureID == "" { + return name + } + return "data-attr-" + name +} + +func (r *RenderInternal) SafeValue(val string) string { + if r.secureID == "" { + return val + } + return r.secureID + ":" + val +} + +func (r *RenderInternal) NodeSafeAttr(attr, val string) html.Attribute { + return html.Attribute{Key: r.SafeAttr(attr), Val: r.SafeValue(val)} +} + +func (r *RenderInternal) ProtectSafeAttrs(content template.HTML) template.HTML { + if r.secureID == "" { + return content + } + return template.HTML(reAttrClass().ReplaceAllString(string(content), `$1 data-attr-class="`+r.secureIDPrefix+`$2"$3`)) +} + +func (r *RenderInternal) FormatWithSafeAttrs(w io.Writer, fmt string, a ...any) error { + _, err := w.Write([]byte(r.ProtectSafeAttrs(htmlutil.HTMLFormat(fmt, a...)))) + return err +} diff --git a/modules/markup/markdown/ast.go b/modules/markup/markdown/ast.go index 624c35d945..ca165b1ba0 100644 --- a/modules/markup/markdown/ast.go +++ b/modules/markup/markdown/ast.go @@ -34,13 +34,6 @@ func NewDetails() *Details { } } -// IsDetails returns true if the given node implements the Details interface, -// otherwise false. -func IsDetails(node ast.Node) bool { - _, ok := node.(*Details) - return ok -} - // Summary is a block that contains the summary of details block type Summary struct { ast.BaseBlock @@ -66,13 +59,6 @@ func NewSummary() *Summary { } } -// IsSummary returns true if the given node implements the Summary interface, -// otherwise false. -func IsSummary(node ast.Node) bool { - _, ok := node.(*Summary) - return ok -} - // TaskCheckBoxListItem is a block that represents a list item of a markdown block with a checkbox type TaskCheckBoxListItem struct { *ast.ListItem @@ -103,14 +89,7 @@ func NewTaskCheckBoxListItem(listItem *ast.ListItem) *TaskCheckBoxListItem { } } -// IsTaskCheckBoxListItem returns true if the given node implements the TaskCheckBoxListItem interface, -// otherwise false. -func IsTaskCheckBoxListItem(node ast.Node) bool { - _, ok := node.(*TaskCheckBoxListItem) - return ok -} - -// Icon is an inline for a fomantic icon +// Icon is an inline for a Fomantic UI icon type Icon struct { ast.BaseInline Name []byte @@ -139,13 +118,6 @@ func NewIcon(name string) *Icon { } } -// IsIcon returns true if the given node implements the Icon interface, -// otherwise false. -func IsIcon(node ast.Node) bool { - _, ok := node.(*Icon) - return ok -} - // ColorPreview is an inline for a color preview type ColorPreview struct { ast.BaseInline diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index c837b21e78..47dcfa8b5a 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -7,9 +7,11 @@ import ( "fmt" "regexp" "strings" + "sync" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/internal" "code.gitea.io/gitea/modules/setting" "github.com/yuin/goldmark/ast" @@ -23,11 +25,13 @@ import ( // ASTTransformer is a default transformer of the goldmark tree. type ASTTransformer struct { + renderInternal *internal.RenderInternal attentionTypes container.Set[string] } -func NewASTTransformer() *ASTTransformer { +func NewASTTransformer(renderInternal *internal.RenderInternal) *ASTTransformer { return &ASTTransformer{ + renderInternal: renderInternal, attentionTypes: container.SetOf("note", "tip", "important", "warning", "caution"), } } @@ -109,12 +113,16 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa } } -// NewHTMLRenderer creates a HTMLRenderer to render -// in the gitea form. -func NewHTMLRenderer(opts ...html.Option) renderer.NodeRenderer { +// it is copied from old code, which is quite doubtful whether it is correct +var reValidIconName = sync.OnceValue[*regexp.Regexp](func() *regexp.Regexp { + return regexp.MustCompile(`^[-\w]+$`) // old: regexp.MustCompile("^[a-z ]+$") +}) + +// NewHTMLRenderer creates a HTMLRenderer to render in the gitea form. +func NewHTMLRenderer(renderInternal *internal.RenderInternal, opts ...html.Option) renderer.NodeRenderer { r := &HTMLRenderer{ - Config: html.NewConfig(), - reValidName: regexp.MustCompile("^[a-z ]+$"), + renderInternal: renderInternal, + Config: html.NewConfig(), } for _, opt := range opts { opt.SetHTMLOption(&r.Config) @@ -126,7 +134,7 @@ func NewHTMLRenderer(opts ...html.Option) renderer.NodeRenderer { // renders gitea specific features. type HTMLRenderer struct { html.Config - reValidName *regexp.Regexp + renderInternal *internal.RenderInternal } // RegisterFuncs implements renderer.NodeRenderer.RegisterFuncs. @@ -214,12 +222,13 @@ func (r *HTMLRenderer) renderIcon(w util.BufWriter, source []byte, node ast.Node return ast.WalkContinue, nil } - if !r.reValidName.MatchString(name) { + if !reValidIconName().MatchString(name) { // skip this return ast.WalkContinue, nil } - _, err := w.WriteString(fmt.Sprintf(``, name)) + // FIXME: the "icon xxx" is from Fomantic UI, it's really questionable whether it still works correctly + err := r.renderInternal.FormatWithSafeAttrs(w, ``, name) if err != nil { return ast.WalkStop, err } diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index 6af0deb27b..a3915ad439 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -9,7 +9,6 @@ import ( "html/template" "io" "strings" - "sync" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" @@ -29,11 +28,6 @@ import ( "github.com/yuin/goldmark/util" ) -var ( - specMarkdown goldmark.Markdown - specMarkdownOnce sync.Once -) - var ( renderContextKey = parser.NewContextKey() renderConfigKey = parser.NewContextKey() @@ -68,85 +62,95 @@ func newParserContext(ctx *markup.RenderContext) parser.Context { return pc } -// SpecializedMarkdown sets up the Gitea specific markdown extensions -func SpecializedMarkdown() goldmark.Markdown { - specMarkdownOnce.Do(func() { - specMarkdown = goldmark.New( - goldmark.WithExtensions( - extension.NewTable( - extension.WithTableCellAlignMethod(extension.TableCellAlignAttribute)), - extension.Strikethrough, - extension.TaskList, - extension.DefinitionList, - common.FootnoteExtension, - highlighting.NewHighlighting( - highlighting.WithFormatOptions( - chromahtml.WithClasses(true), - chromahtml.PreventSurroundingPre(true), - ), - highlighting.WithWrapperRenderer(func(w util.BufWriter, c highlighting.CodeBlockContext, entering bool) { - if entering { - language, _ := c.Language() - if language == nil { - language = []byte("text") - } +type GlodmarkRender struct { + ctx *markup.RenderContext - languageStr := string(language) - - preClasses := []string{"code-block"} - if languageStr == "mermaid" || languageStr == "math" { - preClasses = append(preClasses, "is-loading") - } - - _, err := w.WriteString(`
`)
-							if err != nil {
-								return
-							}
-
-							// include language-x class as part of commonmark spec
-							// the "display" class is used by "js/markup/math.js" to render the code element as a block
-							_, err = w.WriteString(``)
-							if err != nil {
-								return
-							}
-						} else {
-							_, err := w.WriteString("
") - if err != nil { - return - } - } - }), - ), - math.NewExtension( - math.Enabled(setting.Markdown.EnableMath), - ), - meta.Meta, - ), - goldmark.WithParserOptions( - parser.WithAttribute(), - parser.WithAutoHeadingID(), - parser.WithASTTransformers( - util.Prioritized(NewASTTransformer(), 10000), - ), - ), - goldmark.WithRendererOptions( - html.WithUnsafe(), - ), - ) - - // Override the original Tasklist renderer! - specMarkdown.Renderer().AddOptions( - renderer.WithNodeRenderers( - util.Prioritized(NewHTMLRenderer(), 10), - ), - ) - }) - return specMarkdown + goldmarkMarkdown goldmark.Markdown } -// actualRender renders Markdown to HTML without handling special links. -func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - converter := SpecializedMarkdown() +func (r *GlodmarkRender) Convert(source []byte, writer io.Writer, opts ...parser.ParseOption) error { + return r.goldmarkMarkdown.Convert(source, writer, opts...) +} + +func (r *GlodmarkRender) Renderer() renderer.Renderer { + return r.goldmarkMarkdown.Renderer() +} + +func (r *GlodmarkRender) highlightingRenderer(w util.BufWriter, c highlighting.CodeBlockContext, entering bool) { + if entering { + language, _ := c.Language() + if language == nil { + language = []byte("text") + } + + languageStr := string(language) + + preClasses := []string{"code-block"} + if languageStr == "mermaid" || languageStr == "math" { + preClasses = append(preClasses, "is-loading") + } + + err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `
`, strings.Join(preClasses, " "))
+		if err != nil {
+			return
+		}
+
+		// include language-x class as part of commonmark spec
+		// the "display" class is used by "js/markup/math.js" to render the code element as a block
+		err = r.ctx.RenderInternal.FormatWithSafeAttrs(w, ``, string(language))
+		if err != nil {
+			return
+		}
+	} else {
+		_, err := w.WriteString("
") + if err != nil { + return + } + } +} + +// SpecializedMarkdown sets up the Gitea specific markdown extensions +func SpecializedMarkdown(ctx *markup.RenderContext) *GlodmarkRender { + // TODO: it could use a pool to cache the renderers to reuse them with different contexts + // at the moment it is fast enough (see the benchmarks) + r := &GlodmarkRender{ctx: ctx} + r.goldmarkMarkdown = goldmark.New( + goldmark.WithExtensions( + extension.NewTable(extension.WithTableCellAlignMethod(extension.TableCellAlignAttribute)), + extension.Strikethrough, + extension.TaskList, + extension.DefinitionList, + common.FootnoteExtension, + highlighting.NewHighlighting( + highlighting.WithFormatOptions( + chromahtml.WithClasses(true), + chromahtml.PreventSurroundingPre(true), + ), + highlighting.WithWrapperRenderer(r.highlightingRenderer), + ), + math.NewExtension(&ctx.RenderInternal, math.Enabled(setting.Markdown.EnableMath)), + meta.Meta, + ), + goldmark.WithParserOptions( + parser.WithAttribute(), + parser.WithAutoHeadingID(), + parser.WithASTTransformers(util.Prioritized(NewASTTransformer(&ctx.RenderInternal), 10000)), + ), + goldmark.WithRendererOptions(html.WithUnsafe()), + ) + + // Override the original Tasklist renderer! + r.goldmarkMarkdown.Renderer().AddOptions( + renderer.WithNodeRenderers(util.Prioritized(NewHTMLRenderer(&ctx.RenderInternal), 10)), + ) + + return r +} + +// render calls goldmark render to convert Markdown to HTML +// NOTE: The output of this method MUST get sanitized separately!!! +func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { + converter := SpecializedMarkdown(ctx) lw := &limitWriter{ w: output, limit: setting.UI.MaxDisplayFileSize * 3, @@ -160,8 +164,8 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) } log.Warn("Unable to render markdown due to panic in goldmark: %v", err) - if log.IsDebug() { - log.Debug("Panic in markdown: %v\n%s", err, log.Stack(2)) + if (!setting.IsProd && !setting.IsInTesting) || log.IsDebug() { + log.Error("Panic in markdown: %v\n%s", err, log.Stack(2)) } }() @@ -200,26 +204,6 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer) return nil } -// Note: The output of this method must get sanitized. -func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - defer func() { - err := recover() - if err == nil { - return - } - - log.Warn("Unable to render markdown due to panic in goldmark - will return raw bytes") - if log.IsDebug() { - log.Debug("Panic in markdown: %v\n%s", err, log.Stack(2)) - } - _, err = io.Copy(output, input) - if err != nil { - log.Error("io.Copy failed: %v", err) - } - }() - return actualRender(ctx, input, output) -} - // MarkupName describes markup's name var MarkupName = "markdown" diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 780df8727f..e4889a75e5 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -1051,3 +1051,17 @@ func TestAttention(t *testing.T) { // legacy GitHub style test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n") } + +func BenchmarkSpecializedMarkdown(b *testing.B) { + // 240856 4719 ns/op + for i := 0; i < b.N; i++ { + markdown.SpecializedMarkdown(&markup.RenderContext{}) + } +} + +func BenchmarkMarkdownRender(b *testing.B) { + // 23202 50840 ns/op + for i := 0; i < b.N; i++ { + _, _ = markdown.RenderString(&markup.RenderContext{Ctx: context.Background()}, "https://example.com\n- a\n- b\n") + } +} diff --git a/modules/markup/markdown/math/block_renderer.go b/modules/markup/markdown/math/block_renderer.go index 84817ef1e4..0d2a966102 100644 --- a/modules/markup/markdown/math/block_renderer.go +++ b/modules/markup/markdown/math/block_renderer.go @@ -4,17 +4,21 @@ package math import ( + "code.gitea.io/gitea/modules/markup/internal" + gast "github.com/yuin/goldmark/ast" "github.com/yuin/goldmark/renderer" "github.com/yuin/goldmark/util" ) // BlockRenderer represents a renderer for math Blocks -type BlockRenderer struct{} +type BlockRenderer struct { + renderInternal *internal.RenderInternal +} // NewBlockRenderer creates a new renderer for math Blocks -func NewBlockRenderer() renderer.NodeRenderer { - return &BlockRenderer{} +func NewBlockRenderer(renderInternal *internal.RenderInternal) renderer.NodeRenderer { + return &BlockRenderer{renderInternal: renderInternal} } // RegisterFuncs registers the renderer for math Blocks @@ -33,7 +37,7 @@ func (r *BlockRenderer) writeLines(w util.BufWriter, source []byte, n gast.Node) func (r *BlockRenderer) renderBlock(w util.BufWriter, source []byte, node gast.Node, entering bool) (gast.WalkStatus, error) { n := node.(*Block) if entering { - _, _ = w.WriteString(`
`)
+		_ = r.renderInternal.FormatWithSafeAttrs(w, `
`)
 		r.writeLines(w, source, n)
 	} else {
 		_, _ = w.WriteString(`
` + "\n") diff --git a/modules/markup/markdown/math/inline_renderer.go b/modules/markup/markdown/math/inline_renderer.go index 96848099cc..0cff4f1e74 100644 --- a/modules/markup/markdown/math/inline_renderer.go +++ b/modules/markup/markdown/math/inline_renderer.go @@ -6,17 +6,21 @@ package math import ( "bytes" + "code.gitea.io/gitea/modules/markup/internal" + "github.com/yuin/goldmark/ast" "github.com/yuin/goldmark/renderer" "github.com/yuin/goldmark/util" ) // InlineRenderer is an inline renderer -type InlineRenderer struct{} +type InlineRenderer struct { + renderInternal *internal.RenderInternal +} // NewInlineRenderer returns a new renderer for inline math -func NewInlineRenderer() renderer.NodeRenderer { - return &InlineRenderer{} +func NewInlineRenderer(renderInternal *internal.RenderInternal) renderer.NodeRenderer { + return &InlineRenderer{renderInternal: renderInternal} } func (r *InlineRenderer) renderInline(w util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) { @@ -25,7 +29,7 @@ func (r *InlineRenderer) renderInline(w util.BufWriter, source []byte, n ast.Nod if _, ok := n.(*InlineBlock); ok { extraClass = "display " } - _, _ = w.WriteString(``) + _ = r.renderInternal.FormatWithSafeAttrs(w, ``, extraClass) for c := n.FirstChild(); c != nil; c = c.NextSibling() { segment := c.(*ast.Text).Segment value := util.EscapeHTML(segment.Value(source)) diff --git a/modules/markup/markdown/math/math.go b/modules/markup/markdown/math/math.go index 3d9f376bc6..7e8defcd4a 100644 --- a/modules/markup/markdown/math/math.go +++ b/modules/markup/markdown/math/math.go @@ -4,6 +4,8 @@ package math import ( + "code.gitea.io/gitea/modules/markup/internal" + "github.com/yuin/goldmark" "github.com/yuin/goldmark/parser" "github.com/yuin/goldmark/renderer" @@ -12,6 +14,7 @@ import ( // Extension is a math extension type Extension struct { + renderInternal *internal.RenderInternal enabled bool parseDollarInline bool parseDollarBlock bool @@ -39,38 +42,10 @@ func Enabled(enable ...bool) Option { }) } -// WithInlineDollarParser enables or disables the parsing of $...$ -func WithInlineDollarParser(enable ...bool) Option { - value := true - if len(enable) > 0 { - value = enable[0] - } - return extensionFunc(func(e *Extension) { - e.parseDollarInline = value - }) -} - -// WithBlockDollarParser enables or disables the parsing of $$...$$ -func WithBlockDollarParser(enable ...bool) Option { - value := true - if len(enable) > 0 { - value = enable[0] - } - return extensionFunc(func(e *Extension) { - e.parseDollarBlock = value - }) -} - -// Math represents a math extension with default rendered delimiters -var Math = &Extension{ - enabled: true, - parseDollarBlock: true, - parseDollarInline: true, -} - // NewExtension creates a new math extension with the provided options -func NewExtension(opts ...Option) *Extension { +func NewExtension(renderInternal *internal.RenderInternal, opts ...Option) *Extension { r := &Extension{ + renderInternal: renderInternal, enabled: true, parseDollarBlock: true, parseDollarInline: true, @@ -102,7 +77,7 @@ func (e *Extension) Extend(m goldmark.Markdown) { m.Parser().AddOptions(parser.WithInlineParsers(inlines...)) m.Renderer().AddOptions(renderer.WithNodeRenderers( - util.Prioritized(NewBlockRenderer(), 501), - util.Prioritized(NewInlineRenderer(), 502), + util.Prioritized(NewBlockRenderer(e.renderInternal), 501), + util.Prioritized(NewInlineRenderer(e.renderInternal), 502), )) } diff --git a/modules/markup/markdown/meta_test.go b/modules/markup/markdown/meta_test.go index 6949966328..278c33f1d2 100644 --- a/modules/markup/markdown/meta_test.go +++ b/modules/markup/markdown/meta_test.go @@ -11,10 +11,8 @@ import ( "github.com/stretchr/testify/assert" ) -/* -IssueTemplate is a legacy to keep the unit tests working. -Copied from structs.IssueTemplate, the original type has been changed a lot to support yaml template. -*/ +// IssueTemplate is a legacy to keep the unit tests working. +// Copied from structs.IssueTemplate, the original type has been changed a lot to support yaml template. type IssueTemplate struct { Name string `json:"name" yaml:"name"` Title string `json:"title" yaml:"title"` diff --git a/modules/markup/markdown/transform_blockquote.go b/modules/markup/markdown/transform_blockquote.go index 92dc500e69..2651d44a69 100644 --- a/modules/markup/markdown/transform_blockquote.go +++ b/modules/markup/markdown/transform_blockquote.go @@ -32,7 +32,8 @@ func (r *HTMLRenderer) renderAttention(w util.BufWriter, source []byte, node ast default: // including "note" octiconName = "info" } - _, _ = w.WriteString(string(svg.RenderHTML("octicon-"+octiconName, 16, "attention-icon attention-"+n.AttentionType))) + svgHTML := svg.RenderHTML("octicon-"+octiconName, 16, "attention-icon attention-"+n.AttentionType) + _, _ = w.WriteString(string(r.renderInternal.ProtectSafeAttrs(svgHTML))) } return ast.WalkContinue, nil } @@ -128,13 +129,13 @@ func (g *ASTTransformer) transformBlockquote(v *ast.Blockquote, reader text.Read } // color the blockquote - v.SetAttributeString("class", []byte("attention-header attention-"+attentionType)) + v.SetAttributeString(g.renderInternal.SafeAttr("class"), []byte(g.renderInternal.SafeValue("attention-header attention-"+attentionType))) // create an emphasis to make it bold attentionParagraph := ast.NewParagraph() g.applyElementDir(attentionParagraph) emphasis := ast.NewEmphasis(2) - emphasis.SetAttributeString("class", []byte("attention-"+attentionType)) + emphasis.SetAttributeString(g.renderInternal.SafeAttr("class"), []byte(g.renderInternal.SafeValue("attention-"+attentionType))) attentionAstString := ast.NewString([]byte(cases.Title(language.English).String(attentionType))) diff --git a/modules/markup/markdown/transform_codespan.go b/modules/markup/markdown/transform_codespan.go index ff7d24eec9..bccc43aad2 100644 --- a/modules/markup/markdown/transform_codespan.go +++ b/modules/markup/markdown/transform_codespan.go @@ -5,7 +5,6 @@ package markdown import ( "bytes" - "fmt" "strings" "code.gitea.io/gitea/modules/markup" @@ -40,7 +39,7 @@ func (r *HTMLRenderer) renderCodeSpan(w util.BufWriter, source []byte, n ast.Nod r.Writer.RawWrite(w, value) } case *ColorPreview: - _, _ = w.WriteString(fmt.Sprintf(``, string(v.Color))) + _ = r.renderInternal.FormatWithSafeAttrs(w, ``, string(v.Color)) } } return ast.WalkSkipChildren, nil diff --git a/modules/markup/markdown/transform_list.go b/modules/markup/markdown/transform_list.go index b982fd4a83..c89ad2f2cf 100644 --- a/modules/markup/markdown/transform_list.go +++ b/modules/markup/markdown/transform_list.go @@ -72,7 +72,7 @@ func (g *ASTTransformer) transformList(_ *markup.RenderContext, v *ast.List, rc } newChild := NewTaskCheckBoxListItem(listItem) newChild.IsChecked = taskCheckBox.IsChecked - newChild.SetAttributeString("class", []byte("task-list-item")) + newChild.SetAttributeString(g.renderInternal.SafeAttr("class"), []byte(g.renderInternal.SafeValue("task-list-item"))) segments := newChild.FirstChild().Lines() if segments.Len() > 0 { segment := segments.At(0) diff --git a/modules/markup/render.go b/modules/markup/render.go index 1977dc73f5..f05cb62626 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -9,14 +9,15 @@ import ( "io" "net/url" "strings" - "sync" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/markup/internal" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "github.com/yuin/goldmark/ast" + "golang.org/x/sync/errgroup" ) type RenderMetaMode string @@ -65,6 +66,8 @@ type RenderContext struct { SidebarTocNode ast.Node RenderMetaAs RenderMetaMode InStandalonePage bool // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page + + RenderInternal internal.RenderInternal } // Cancel runs any cleanup functions that have been registered for this Ctx @@ -156,59 +159,53 @@ sandbox="allow-scripts" return err } -func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { - var wg sync.WaitGroup - var err error +func pipes() (io.ReadCloser, io.WriteCloser, func()) { pr, pw := io.Pipe() - defer func() { + return pr, pw, func() { _ = pr.Close() _ = pw.Close() - }() + } +} - var pr2 io.ReadCloser - var pw2 io.WriteCloser +func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { + finalProcessor := ctx.RenderInternal.Init(output) + defer finalProcessor.Close() - var sanitizerDisabled bool - if r, ok := renderer.(ExternalRenderer); ok { - sanitizerDisabled = r.SanitizerDisabled() + // input -> (pw1=pr1) -> renderer -> (pw2=pr2) -> SanitizeReader -> finalProcessor -> output + // no sanitizer: input -> (pw1=pr1) -> renderer -> pw2(finalProcessor) -> output + pr1, pw1, close1 := pipes() + defer close1() + + eg, _ := errgroup.WithContext(ctx.Ctx) + var pw2 io.WriteCloser = util.NopCloser{Writer: finalProcessor} + + if r, ok := renderer.(ExternalRenderer); !ok || !r.SanitizerDisabled() { + var pr2 io.ReadCloser + var close2 func() + pr2, pw2, close2 = pipes() + defer close2() + eg.Go(func() error { + defer pr2.Close() + return SanitizeReader(pr2, renderer.Name(), finalProcessor) + }) } - if !sanitizerDisabled { - pr2, pw2 = io.Pipe() - defer func() { - _ = pr2.Close() - _ = pw2.Close() - }() - - wg.Add(1) - go func() { - err = SanitizeReader(pr2, renderer.Name(), output) - _ = pr2.Close() - wg.Done() - }() - } else { - pw2 = util.NopCloser{Writer: output} - } - - wg.Add(1) - go func() { + eg.Go(func() (err error) { if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() { - err = PostProcess(ctx, pr, pw2) + err = PostProcess(ctx, pr1, pw2) } else { - _, err = io.Copy(pw2, pr) + _, err = io.Copy(pw2, pr1) } - _ = pr.Close() - _ = pw2.Close() - wg.Done() - }() + _, _ = pr1.Close(), pw2.Close() + return err + }) - if err1 := renderer.Render(ctx, input, pw); err1 != nil { - return err1 + if err := renderer.Render(ctx, input, pw1); err != nil { + return err } - _ = pw.Close() + _ = pw1.Close() - wg.Wait() - return err + return eg.Wait() } // Init initializes the render global variables diff --git a/modules/markup/sanitizer_custom.go b/modules/markup/sanitizer_custom.go index 7978973166..7f96556fd7 100644 --- a/modules/markup/sanitizer_custom.go +++ b/modules/markup/sanitizer_custom.go @@ -4,6 +4,9 @@ package markup import ( + "regexp" + "strings" + "code.gitea.io/gitea/modules/setting" "github.com/microcosm-cc/bluemonday" @@ -15,8 +18,11 @@ func (st *Sanitizer) addSanitizerRules(policy *bluemonday.Policy, rules []settin policy.AllowDataURIImages() } if rule.Element != "" { - if rule.Regexp != nil { - policy.AllowAttrs(rule.AllowAttr).Matching(rule.Regexp).OnElements(rule.Element) + if rule.Regexp != "" { + if !strings.HasPrefix(rule.Regexp, "^") || !strings.HasSuffix(rule.Regexp, "$") { + panic("Markup sanitizer rule regexp must start with ^ and end with $ to be strict") + } + policy.AllowAttrs(rule.AllowAttr).Matching(regexp.MustCompile(rule.Regexp)).OnElements(rule.Element) } else { policy.AllowAttrs(rule.AllowAttr).OnElements(rule.Element) } diff --git a/modules/markup/sanitizer_default.go b/modules/markup/sanitizer_default.go index 476ae5e26f..0fa54efd45 100644 --- a/modules/markup/sanitizer_default.go +++ b/modules/markup/sanitizer_default.go @@ -16,37 +16,12 @@ import ( func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { policy := bluemonday.UGCPolicy() - // For JS code copy and Mermaid loading state - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^code-block( is-loading)?$`)).OnElements("pre") + // NOTICE: DO NOT add special "class" regexp rules here anymore, use RenderInternal.SafeAttr instead - // For code preview - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^code-preview-[-\w]+( file-content)?$`)).Globally() - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^lines-num$`)).OnElements("td") - policy.AllowAttrs("data-line-number").OnElements("span") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^lines-code chroma$`)).OnElements("td") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^code-inner$`)).OnElements("div") - - // For code preview (unicode escape) - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^file-view( unicode-escaped)?$`)).OnElements("table") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^lines-escape$`)).OnElements("td") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^toggle-escape-button btn interact-bg$`)).OnElements("a") // don't use button, button might submit a form - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^(ambiguous-code-point|escaped-code-point|broken-code-point)$`)).OnElements("span") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^char$`)).OnElements("span") - policy.AllowAttrs("data-tooltip-content", "data-escaped").OnElements("span") - - // For color preview - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^color-preview$`)).OnElements("span") - - // For attention - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^attention-header attention-\w+$`)).OnElements("blockquote") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^attention-\w+$`)).OnElements("strong") - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^attention-icon attention-\w+ svg octicon-[\w-]+$`)).OnElements("svg") - policy.AllowAttrs("viewBox", "width", "height", "aria-hidden").OnElements("svg") + // General safe SVG attributes + policy.AllowAttrs("viewBox", "width", "height", "aria-hidden", "data-attr-class").OnElements("svg") policy.AllowAttrs("fill-rule", "d").OnElements("path") - // For Chroma markdown plugin - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^(chroma )?language-[\w-]+( display)?( is-loading)?$`)).OnElements("code") - // Checkboxes policy.AllowAttrs("type").Matching(regexp.MustCompile(`^checkbox$`)).OnElements("input") policy.AllowAttrs("checked", "disabled", "data-source-position").OnElements("input") @@ -66,28 +41,15 @@ func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { policy.AllowURLSchemeWithCustomPolicy("data", disallowScheme) } - // Allow classes for anchors - policy.AllowAttrs("class").Matching(regexp.MustCompile(`ref-issue( ref-external-issue)?`)).OnElements("a") - - // Allow classes for task lists - policy.AllowAttrs("class").Matching(regexp.MustCompile(`task-list-item`)).OnElements("li") - // Allow classes for org mode list item status. policy.AllowAttrs("class").Matching(regexp.MustCompile(`^(unchecked|checked|indeterminate)$`)).OnElements("li") - // Allow icons - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^icon(\s+[\p{L}\p{N}_-]+)+$`)).OnElements("i") - - // Allow classes for emojis - policy.AllowAttrs("class").Matching(regexp.MustCompile(`emoji`)).OnElements("img") - - // Allow icons, emojis, chroma syntax and keyword markup on span - policy.AllowAttrs("class").Matching(regexp.MustCompile(`^((icon(\s+[\p{L}\p{N}_-]+)+)|(emoji)|(language-math display)|(language-math inline))$|^([a-z][a-z0-9]{0,2})$|^` + keywordClass + `$`)).OnElements("span") - // Allow 'color' and 'background-color' properties for the style attribute on text elements. policy.AllowStyles("color", "background-color").OnElements("span", "p") - // Allow generally safe attributes + policy.AllowAttrs("src", "autoplay", "controls").OnElements("video") + + // Allow generally safe attributes (reference: https://github.com/jch/html-pipeline) generalSafeAttrs := []string{ "abbr", "accept", "accept-charset", "accesskey", "action", "align", "alt", @@ -106,10 +68,9 @@ func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { "selected", "shape", "size", "span", "start", "summary", "tabindex", "target", "title", "type", "usemap", "valign", "value", - "vspace", "width", "itemprop", - "data-markdown-generated-content", + "vspace", "width", "itemprop", "itemscope", "itemtype", + "data-markdown-generated-content", "data-attr-class", } - generalSafeElements := []string{ "h1", "h2", "h3", "h4", "h5", "h6", "h7", "h8", "br", "b", "i", "strong", "em", "a", "pre", "code", "img", "tt", "div", "ins", "del", "sup", "sub", "p", "ol", "ul", "table", "thead", "tbody", "tfoot", "blockquote", "label", @@ -117,14 +78,8 @@ func (st *Sanitizer) createDefaultPolicy() *bluemonday.Policy { "details", "caption", "figure", "figcaption", "abbr", "bdo", "cite", "dfn", "mark", "small", "span", "time", "video", "wbr", } - - policy.AllowAttrs(generalSafeAttrs...).OnElements(generalSafeElements...) - - policy.AllowAttrs("src", "autoplay", "controls").OnElements("video") - - policy.AllowAttrs("itemscope", "itemtype").OnElements("div") - // FIXME: Need to handle longdesc in img but there is no easy way to do it + policy.AllowAttrs(generalSafeAttrs...).OnElements(generalSafeElements...) // Custom keyword markup defaultSanitizer.addSanitizerRules(policy, setting.ExternalSanitizerRules) diff --git a/modules/markup/sanitizer_default_test.go b/modules/markup/sanitizer_default_test.go index 20370509c1..c5c43695ea 100644 --- a/modules/markup/sanitizer_default_test.go +++ b/modules/markup/sanitizer_default_test.go @@ -19,7 +19,6 @@ func TestSanitizer(t *testing.T) { // Code highlighting class ``, ``, ``, ``, - ``, ``, // Input checkbox ``, ``, @@ -38,10 +37,8 @@ func TestSanitizer(t *testing.T) { // tags `Ctrl + C`, `Ctrl + C`, `NAUGHTY`, `NAUGHTY`, - ``, ``, `unchecked`, `unchecked`, `NAUGHTY`, `NAUGHTY`, - `contents`, `contents`, // Color property `Hello World`, `Hello World`, diff --git a/modules/setting/markup.go b/modules/setting/markup.go index 6c2246342b..dfce8afa77 100644 --- a/modules/setting/markup.go +++ b/modules/setting/markup.go @@ -54,7 +54,7 @@ type MarkupRenderer struct { type MarkupSanitizerRule struct { Element string AllowAttr string - Regexp *regexp.Regexp + Regexp string AllowDataURIImages bool } @@ -117,15 +117,24 @@ func createMarkupSanitizerRule(name string, sec ConfigSection) (MarkupSanitizerR regexpStr := sec.Key("REGEXP").Value() if regexpStr != "" { - // Validate when parsing the config that this is a valid regular - // expression. Then we can use regexp.MustCompile(...) later. - compiled, err := regexp.Compile(regexpStr) + hasPrefix := strings.HasPrefix(regexpStr, "^") + hasSuffix := strings.HasSuffix(regexpStr, "$") + if !hasPrefix || !hasSuffix { + log.Error("In markup.%s: REGEXP must start with ^ and end with $ to be strict", name) + // to avoid breaking existing user configurations and satisfy the strict requirement in addSanitizerRules + if !hasPrefix { + regexpStr = "^.*" + regexpStr + } + if !hasSuffix { + regexpStr += ".*$" + } + } + _, err := regexp.Compile(regexpStr) if err != nil { log.Error("In markup.%s: REGEXP (%s) failed to compile: %v", name, regexpStr, err) return rule, false } - - rule.Regexp = compiled + rule.Regexp = regexpStr } ok = true diff --git a/modules/svg/svg.go b/modules/svg/svg.go index 8132978cac..fded9d0873 100644 --- a/modules/svg/svg.go +++ b/modules/svg/svg.go @@ -9,7 +9,7 @@ import ( "path" "strings" - gitea_html "code.gitea.io/gitea/modules/html" + gitea_html "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/public" ) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 3ef11772dc..d5b32358da 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -10,12 +10,12 @@ import ( "html/template" "net/url" "reflect" - "slices" "strings" "time" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/svg" @@ -39,7 +39,7 @@ func NewFuncMap() template.FuncMap { "Iif": iif, "Eval": evalTokens, "SafeHTML": safeHTML, - "HTMLFormat": HTMLFormat, + "HTMLFormat": htmlutil.HTMLFormat, "HTMLEscape": htmlEscape, "QueryEscape": queryEscape, "JSEscape": jsEscapeSafe, @@ -184,23 +184,6 @@ func NewFuncMap() template.FuncMap { } } -func HTMLFormat(s string, rawArgs ...any) template.HTML { - args := slices.Clone(rawArgs) - for i, v := range args { - switch v := v.(type) { - case nil, bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64, template.HTML: - // for most basic types (including template.HTML which is safe), just do nothing and use it - case string: - args[i] = template.HTMLEscapeString(v) - case fmt.Stringer: - args[i] = template.HTMLEscapeString(v.String()) - default: - args[i] = template.HTMLEscapeString(fmt.Sprint(v)) - } - } - return template.HTML(fmt.Sprintf(s, args...)) -} - // safeHTML render raw as HTML func safeHTML(s any) template.HTML { switch v := s.(type) { diff --git a/modules/templates/helper_test.go b/modules/templates/helper_test.go index b9fabb7016..3e17e86c66 100644 --- a/modules/templates/helper_test.go +++ b/modules/templates/helper_test.go @@ -61,10 +61,6 @@ func TestJSEscapeSafe(t *testing.T) { assert.EqualValues(t, `\u0026\u003C\u003E\'\"`, jsEscapeSafe(`&<>'"`)) } -func TestHTMLFormat(t *testing.T) { - assert.Equal(t, template.HTML("< < 1"), HTMLFormat("%s %s %d", "<", template.HTML("<"), 1)) -} - func TestSanitizeHTML(t *testing.T) { assert.Equal(t, template.HTML(`link xss
inline
`), SanitizeHTML(`link xss
inline
`)) } diff --git a/modules/templates/util_avatar.go b/modules/templates/util_avatar.go index afc1091516..f7dd408ee2 100644 --- a/modules/templates/util_avatar.go +++ b/modules/templates/util_avatar.go @@ -14,7 +14,7 @@ import ( "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - gitea_html "code.gitea.io/gitea/modules/html" + gitea_html "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/setting" ) diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 8e443446bd..5776eefced 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -16,6 +16,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/modules/emoji" + "code.gitea.io/gitea/modules/htmlutil" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" @@ -140,7 +141,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { if labelScope == "" { // Regular label - return HTMLFormat(`
%s
`, + return htmlutil.HTMLFormat(`
%s
`, extraCSSClasses, textColor, label.Color, descriptionText, ut.RenderEmoji(label.Name)) } @@ -174,7 +175,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { itemColor := "#" + hex.EncodeToString(itemBytes) scopeColor := "#" + hex.EncodeToString(scopeBytes) - return HTMLFormat(``+ + return htmlutil.HTMLFormat(``+ `
%s
`+ `
%s
`+ `
`, diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index 529507e7ea..cf6d839cbf 100644 --- a/modules/templates/util_render_test.go +++ b/modules/templates/util_render_test.go @@ -113,34 +113,34 @@ func TestRenderCommitBody(t *testing.T) { } expected := `/just/a/path.bin -https://example.com/file.bin +https://example.com/file.bin [local link](file.bin) -[remote link](https://example.com) +[remote link](https://example.com) [[local link|file.bin]] -[[remote link|https://example.com]] +[[remote link|https://example.com]] ![local image](image.jpg) -![remote image](https://example.com/image.jpg) +![remote image](https://example.com/image.jpg) [[local image|image.jpg]] -[[remote link|https://example.com/image.jpg]] +[[remote link|https://example.com/image.jpg]] 88fc37a3c0...12fc37a3c0 (hash) com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a58247f42fb pare 88fc37a3c0 com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit 👍 -mail@domain.com -@mention-user test +mail@domain.com +@mention-user test #123 space` assert.EqualValues(t, expected, string(newTestRenderUtils().RenderCommitBody(testInput(), testMetas))) } func TestRenderCommitMessage(t *testing.T) { - expected := `space @mention-user ` + expected := `space @mention-user ` assert.EqualValues(t, expected, newTestRenderUtils().RenderCommitMessage(testInput(), testMetas)) } func TestRenderCommitMessageLinkSubject(t *testing.T) { - expected := `space @mention-user` + expected := `space @mention-user` assert.EqualValues(t, expected, newTestRenderUtils().RenderCommitMessageLinkSubject(testInput(), "https://example.com/link", testMetas)) } diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 13f6b69493..2732a67e71 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -326,7 +326,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { if rctx.SidebarTocNode != nil { sb := &strings.Builder{} - err = markdown.SpecializedMarkdown().Renderer().Render(sb, nil, rctx.SidebarTocNode) + err = markdown.SpecializedMarkdown(rctx).Renderer().Render(sb, nil, rctx.SidebarTocNode) if err != nil { log.Error("Failed to render wiki sidebar TOC: %v", err) } else { From 696fbe60365d59a2d979f977b5ae6f13c52f9188 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Nov 2024 21:59:04 -0800 Subject: [PATCH 04/23] Refactor push mirror find and add check for updating push mirror (#32539) Co-authored-by: wxiaoguang --- models/db/collation.go | 3 +- models/repo/pushmirror.go | 50 +++++++---- routers/web/repo/setting/setting.go | 51 ++++------- services/forms/repo_form.go | 2 +- services/mirror/mirror.go | 10 +-- services/mirror/queue.go | 11 ++- tests/integration/db_collation_test.go | 5 +- tests/integration/mirror_push_test.go | 116 ++++++++++++++++--------- 8 files changed, 141 insertions(+), 107 deletions(-) diff --git a/models/db/collation.go b/models/db/collation.go index c128cf5029..a7db9f5442 100644 --- a/models/db/collation.go +++ b/models/db/collation.go @@ -68,7 +68,8 @@ func CheckCollations(x *xorm.Engine) (*CheckCollationsResult, error) { var candidateCollations []string if x.Dialect().URI().DBType == schemas.MYSQL { - if _, err = x.SQL("SELECT @@collation_database").Get(&res.DatabaseCollation); err != nil { + _, err = x.SQL("SELECT DEFAULT_COLLATION_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = ?", setting.Database.Name).Get(&res.DatabaseCollation) + if err != nil { return nil, err } res.IsCollationCaseSensitive = func(s string) bool { diff --git a/models/repo/pushmirror.go b/models/repo/pushmirror.go index bf134abfb1..55e8f3a068 100644 --- a/models/repo/pushmirror.go +++ b/models/repo/pushmirror.go @@ -9,15 +9,13 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) -// ErrPushMirrorNotExist mirror does not exist error -var ErrPushMirrorNotExist = util.NewNotExistErrorf("PushMirror does not exist") - // PushMirror represents mirror information of a repository. type PushMirror struct { ID int64 `xorm:"pk autoincr"` @@ -96,26 +94,46 @@ func DeletePushMirrors(ctx context.Context, opts PushMirrorOptions) error { return util.NewInvalidArgumentErrorf("repoID required and must be set") } +type findPushMirrorOptions struct { + db.ListOptions + RepoID int64 + SyncOnCommit optional.Option[bool] +} + +func (opts findPushMirrorOptions) ToConds() builder.Cond { + cond := builder.NewCond() + if opts.RepoID > 0 { + cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) + } + if opts.SyncOnCommit.Has() { + cond = cond.And(builder.Eq{"sync_on_commit": opts.SyncOnCommit.Value()}) + } + return cond +} + // GetPushMirrorsByRepoID returns push-mirror information of a repository. func GetPushMirrorsByRepoID(ctx context.Context, repoID int64, listOptions db.ListOptions) ([]*PushMirror, int64, error) { - sess := db.GetEngine(ctx).Where("repo_id = ?", repoID) - if listOptions.Page != 0 { - sess = db.SetSessionPagination(sess, &listOptions) - mirrors := make([]*PushMirror, 0, listOptions.PageSize) - count, err := sess.FindAndCount(&mirrors) - return mirrors, count, err + return db.FindAndCount[PushMirror](ctx, findPushMirrorOptions{ + ListOptions: listOptions, + RepoID: repoID, + }) +} + +func GetPushMirrorByIDAndRepoID(ctx context.Context, id, repoID int64) (*PushMirror, bool, error) { + var pushMirror PushMirror + has, err := db.GetEngine(ctx).Where("id = ?", id).And("repo_id = ?", repoID).Get(&pushMirror) + if !has || err != nil { + return nil, has, err } - mirrors := make([]*PushMirror, 0, 10) - count, err := sess.FindAndCount(&mirrors) - return mirrors, count, err + return &pushMirror, true, nil } // GetPushMirrorsSyncedOnCommit returns push-mirrors for this repo that should be updated by new commits func GetPushMirrorsSyncedOnCommit(ctx context.Context, repoID int64) ([]*PushMirror, error) { - mirrors := make([]*PushMirror, 0, 10) - return mirrors, db.GetEngine(ctx). - Where("repo_id = ? AND sync_on_commit = ?", repoID, true). - Find(&mirrors) + return db.Find[PushMirror](ctx, findPushMirrorOptions{ + RepoID: repoID, + SyncOnCommit: optional.Some(true), + }) } // PushMirrorsIterate iterates all push-mirror repositories. diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 485bd927fa..e30129bb44 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "strconv" "strings" "time" @@ -290,8 +289,8 @@ func SettingsPost(ctx *context.Context) { return } - m, err := selectPushMirrorByForm(ctx, form, repo) - if err != nil { + m, _, _ := repo_model.GetPushMirrorByIDAndRepoID(ctx, form.PushMirrorID, repo.ID) + if m == nil { ctx.NotFound("", nil) return } @@ -317,15 +316,13 @@ func SettingsPost(ctx *context.Context) { return } - id, err := strconv.ParseInt(form.PushMirrorID, 10, 64) - if err != nil { - ctx.ServerError("UpdatePushMirrorIntervalPushMirrorID", err) + m, _, _ := repo_model.GetPushMirrorByIDAndRepoID(ctx, form.PushMirrorID, repo.ID) + if m == nil { + ctx.NotFound("", nil) return } - m := &repo_model.PushMirror{ - ID: id, - Interval: interval, - } + + m.Interval = interval if err := repo_model.UpdatePushMirrorInterval(ctx, m); err != nil { ctx.ServerError("UpdatePushMirrorInterval", err) return @@ -334,7 +331,10 @@ func SettingsPost(ctx *context.Context) { // If we observed its implementation in the context of `push-mirror-sync` where it // is evident that pushing to the queue is necessary for updates. // So, there are updates within the given interval, it is necessary to update the queue accordingly. - mirror_service.AddPushMirrorToQueue(m.ID) + if !ctx.FormBool("push_mirror_defer_sync") { + // push_mirror_defer_sync is mainly for testing purpose, we do not really want to sync the push mirror immediately + mirror_service.AddPushMirrorToQueue(m.ID) + } ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) ctx.Redirect(repo.Link() + "/settings") @@ -348,18 +348,18 @@ func SettingsPost(ctx *context.Context) { // as an error on the UI for this action ctx.Data["Err_RepoName"] = nil - m, err := selectPushMirrorByForm(ctx, form, repo) - if err != nil { + m, _, _ := repo_model.GetPushMirrorByIDAndRepoID(ctx, form.PushMirrorID, repo.ID) + if m == nil { ctx.NotFound("", nil) return } - if err = mirror_service.RemovePushMirrorRemote(ctx, m); err != nil { + if err := mirror_service.RemovePushMirrorRemote(ctx, m); err != nil { ctx.ServerError("RemovePushMirrorRemote", err) return } - if err = repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ID: m.ID, RepoID: m.RepoID}); err != nil { + if err := repo_model.DeletePushMirrors(ctx, repo_model.PushMirrorOptions{ID: m.ID, RepoID: m.RepoID}); err != nil { ctx.ServerError("DeletePushMirrorByID", err) return } @@ -995,24 +995,3 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R } ctx.RenderWithErr(ctx.Tr("repo.mirror_address_url_invalid"), tplSettingsOptions, form) } - -func selectPushMirrorByForm(ctx *context.Context, form *forms.RepoSettingForm, repo *repo_model.Repository) (*repo_model.PushMirror, error) { - id, err := strconv.ParseInt(form.PushMirrorID, 10, 64) - if err != nil { - return nil, err - } - - pushMirrors, _, err := repo_model.GetPushMirrorsByRepoID(ctx, repo.ID, db.ListOptions{}) - if err != nil { - return nil, err - } - - for _, m := range pushMirrors { - if m.ID == id { - m.Repo = repo - return m, nil - } - } - - return nil, fmt.Errorf("PushMirror[%v] not associated to repository %v", id, repo) -} diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index d27bbca894..8e663084f8 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -122,7 +122,7 @@ type RepoSettingForm struct { MirrorPassword string LFS bool `form:"mirror_lfs"` LFSEndpoint string `form:"mirror_lfs_endpoint"` - PushMirrorID string + PushMirrorID int64 PushMirrorAddress string PushMirrorUsername string PushMirrorPassword string diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index 44218d6fb3..e029bbb1d6 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -8,7 +8,6 @@ import ( "fmt" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/setting" @@ -119,14 +118,7 @@ func Update(ctx context.Context, pullLimit, pushLimit int) error { return nil } -func queueHandler(items ...*SyncRequest) []*SyncRequest { - for _, req := range items { - doMirrorSync(graceful.GetManager().ShutdownContext(), req) - } - return nil -} - // InitSyncMirrors initializes a go routine to sync the mirrors func InitSyncMirrors() { - StartSyncMirrors(queueHandler) + StartSyncMirrors() } diff --git a/services/mirror/queue.go b/services/mirror/queue.go index 0d9a624730..ca5e2c7272 100644 --- a/services/mirror/queue.go +++ b/services/mirror/queue.go @@ -28,12 +28,19 @@ type SyncRequest struct { ReferenceID int64 // RepoID for pull mirror, MirrorID for push mirror } +func queueHandler(items ...*SyncRequest) []*SyncRequest { + for _, req := range items { + doMirrorSync(graceful.GetManager().ShutdownContext(), req) + } + return nil +} + // StartSyncMirrors starts a go routine to sync the mirrors -func StartSyncMirrors(queueHandle func(data ...*SyncRequest) []*SyncRequest) { +func StartSyncMirrors() { if !setting.Mirror.Enabled { return } - mirrorQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "mirror", queueHandle) + mirrorQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "mirror", queueHandler) if mirrorQueue == nil { log.Fatal("Unable to create mirror queue") } diff --git a/tests/integration/db_collation_test.go b/tests/integration/db_collation_test.go index 75a4c1594f..acec4aa5d1 100644 --- a/tests/integration/db_collation_test.go +++ b/tests/integration/db_collation_test.go @@ -73,9 +73,12 @@ func TestDatabaseCollation(t *testing.T) { t.Run("Convert tables to utf8mb4_bin", func(t *testing.T) { defer test.MockVariableValue(&setting.Database.CharsetCollation, "utf8mb4_bin")() - assert.NoError(t, db.ConvertDatabaseTable()) r, err := db.CheckCollations(x) assert.NoError(t, err) + assert.EqualValues(t, "utf8mb4_bin", r.ExpectedCollation) + assert.NoError(t, db.ConvertDatabaseTable()) + r, err = db.CheckCollations(x) + assert.NoError(t, err) assert.Equal(t, "utf8mb4_bin", r.DatabaseCollation) assert.True(t, r.CollationEquals(r.ExpectedCollation, r.DatabaseCollation)) assert.Empty(t, r.InconsistentCollationColumns) diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 6b1c808cf4..9ff4669bef 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -9,7 +9,9 @@ import ( "net/http" "net/url" "strconv" + "strings" "testing" + "time" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -32,11 +34,10 @@ func TestMirrorPush(t *testing.T) { } func testMirrorPush(t *testing.T, u *url.URL) { - defer tests.PrepareTestEnv(t)() - setting.Migrations.AllowLocalNetworks = true assert.NoError(t, migrations.Init()) + _ = db.TruncateBeans(db.DefaultContext, &repo_model.PushMirror{}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) srcRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) @@ -45,9 +46,10 @@ func testMirrorPush(t *testing.T, u *url.URL) { }) assert.NoError(t, err) - ctx := NewAPITestContext(t, user.LowerName, srcRepo.Name) + session := loginUser(t, user.Name) - doCreatePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name)), user.LowerName, userPassword)(t) + pushMirrorURL := fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(user.Name), url.PathEscape(mirrorRepo.Name)) + testCreatePushMirror(t, session, user.Name, srcRepo.Name, pushMirrorURL, user.LowerName, userPassword, "0") mirrors, _, err := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{}) assert.NoError(t, err) @@ -73,49 +75,81 @@ func testMirrorPush(t *testing.T, u *url.URL) { assert.Equal(t, srcCommit.ID, mirrorCommit.ID) // Cleanup - doRemovePushMirror(ctx, fmt.Sprintf("%s%s/%s", u.String(), url.PathEscape(ctx.Username), url.PathEscape(mirrorRepo.Name)), user.LowerName, userPassword, int(mirrors[0].ID))(t) + assert.True(t, doRemovePushMirror(t, session, user.Name, srcRepo.Name, mirrors[0].ID)) mirrors, _, err = repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{}) assert.NoError(t, err) assert.Len(t, mirrors, 0) } -func doCreatePushMirror(ctx APITestContext, address, username, password string) func(t *testing.T) { - return func(t *testing.T) { - csrf := GetUserCSRFToken(t, ctx.Session) +func testCreatePushMirror(t *testing.T, session *TestSession, owner, repo, address, username, password, interval string) { + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(owner), url.PathEscape(repo)), map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "action": "push-mirror-add", + "push_mirror_address": address, + "push_mirror_username": username, + "push_mirror_password": password, + "push_mirror_interval": interval, + }) + session.MakeRequest(t, req, http.StatusSeeOther) - req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{ - "_csrf": csrf, - "action": "push-mirror-add", - "push_mirror_address": address, - "push_mirror_username": username, - "push_mirror_password": password, - "push_mirror_interval": "0", - }) - ctx.Session.MakeRequest(t, req, http.StatusSeeOther) - - flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.Contains(t, flashCookie.Value, "success") - } + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Contains(t, flashCookie.Value, "success") } -func doRemovePushMirror(ctx APITestContext, address, username, password string, pushMirrorID int) func(t *testing.T) { - return func(t *testing.T) { - csrf := GetUserCSRFToken(t, ctx.Session) - - req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame)), map[string]string{ - "_csrf": csrf, - "action": "push-mirror-remove", - "push_mirror_id": strconv.Itoa(pushMirrorID), - "push_mirror_address": address, - "push_mirror_username": username, - "push_mirror_password": password, - "push_mirror_interval": "0", - }) - ctx.Session.MakeRequest(t, req, http.StatusSeeOther) - - flashCookie := ctx.Session.GetCookie(gitea_context.CookieNameFlash) - assert.NotNil(t, flashCookie) - assert.Contains(t, flashCookie.Value, "success") - } +func doRemovePushMirror(t *testing.T, session *TestSession, owner, repo string, pushMirrorID int64) bool { + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", url.PathEscape(owner), url.PathEscape(repo)), map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "action": "push-mirror-remove", + "push_mirror_id": strconv.FormatInt(pushMirrorID, 10), + }) + resp := session.MakeRequest(t, req, NoExpectedStatus) + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + return resp.Code == http.StatusSeeOther && flashCookie != nil && strings.Contains(flashCookie.Value, "success") +} + +func doUpdatePushMirror(t *testing.T, session *TestSession, owner, repo string, pushMirrorID int64, interval string) bool { + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings", owner, repo), map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "action": "push-mirror-update", + "push_mirror_id": strconv.FormatInt(pushMirrorID, 10), + "push_mirror_interval": interval, + "push_mirror_defer_sync": "true", + }) + resp := session.MakeRequest(t, req, NoExpectedStatus) + return resp.Code == http.StatusSeeOther +} + +func TestRepoSettingPushMirrorUpdate(t *testing.T) { + defer tests.PrepareTestEnv(t)() + setting.Migrations.AllowLocalNetworks = true + assert.NoError(t, migrations.Init()) + + session := loginUser(t, "user2") + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + testCreatePushMirror(t, session, "user2", "repo2", "https://127.0.0.1/user1/repo1.git", "", "", "24h") + + pushMirrors, cnt, err := repo_model.GetPushMirrorsByRepoID(db.DefaultContext, repo2.ID, db.ListOptions{}) + assert.NoError(t, err) + assert.EqualValues(t, 1, cnt) + assert.EqualValues(t, 24*time.Hour, pushMirrors[0].Interval) + repo2PushMirrorID := pushMirrors[0].ID + + // update repo2 push mirror + assert.True(t, doUpdatePushMirror(t, session, "user2", "repo2", repo2PushMirrorID, "10m0s")) + pushMirror := unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID}) + assert.EqualValues(t, 10*time.Minute, pushMirror.Interval) + + // avoid updating repo2 push mirror from repo1 + assert.False(t, doUpdatePushMirror(t, session, "user2", "repo1", repo2PushMirrorID, "20m0s")) + pushMirror = unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID}) + assert.EqualValues(t, 10*time.Minute, pushMirror.Interval) // not changed + + // avoid deleting repo2 push mirror from repo1 + assert.False(t, doRemovePushMirror(t, session, "user2", "repo1", repo2PushMirrorID)) + unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID}) + + // delete repo2 push mirror + assert.True(t, doRemovePushMirror(t, session, "user2", "repo2", repo2PushMirrorID)) + unittest.AssertNotExistsBean(t, &repo_model.PushMirror{ID: repo2PushMirrorID}) } From 896314c7a2be7987b07f5e7ca7aa4b0a82c97c71 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 17 Nov 2024 22:24:49 -0800 Subject: [PATCH 05/23] Fix some places which doesn't repsect org full name setting (#32243) Partially fix #31345 --- templates/admin/org/list.tmpl | 2 +- templates/user/dashboard/repolist.tmpl | 2 +- web_src/js/components/DashboardRepoList.vue | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/admin/org/list.tmpl b/templates/admin/org/list.tmpl index d0805c85bc..d5e09939c5 100644 --- a/templates/admin/org/list.tmpl +++ b/templates/admin/org/list.tmpl @@ -52,7 +52,7 @@ {{.ID}} - {{.Name}} + {{if and DefaultShowFullName .FullName}}{{.FullName}} ({{.Name}}){{else}}{{.Name}}{{end}} {{if .Visibility.IsPrivate}} {{svg "octicon-lock"}} {{end}} diff --git a/templates/user/dashboard/repolist.tmpl b/templates/user/dashboard/repolist.tmpl index be710675d5..a2764ba608 100644 --- a/templates/user/dashboard/repolist.tmpl +++ b/templates/user/dashboard/repolist.tmpl @@ -45,7 +45,7 @@ data.teamId = {{.Team.ID}}; {{end}} {{if not .ContextUser.IsOrganization}} -data.organizations = [{{range .Orgs}}{'name': {{.Name}}, 'num_repos': {{.NumRepos}}, 'org_visibility': {{.Visibility}}},{{end}}]; +data.organizations = [{{range .Orgs}}{'name': {{.Name}}, 'full_name': {{.FullName}}, 'num_repos': {{.NumRepos}}, 'org_visibility': {{.Visibility}}},{{end}}]; data.isOrganization = false; data.organizationsTotalCount = {{.UserOrgsCount}}; data.canCreateOrganization = {{.SignedUser.CanCreateOrganization}}; diff --git a/web_src/js/components/DashboardRepoList.vue b/web_src/js/components/DashboardRepoList.vue index a6a8ccd2d1..3d3ac2fc69 100644 --- a/web_src/js/components/DashboardRepoList.vue +++ b/web_src/js/components/DashboardRepoList.vue @@ -471,7 +471,7 @@ export default sfc; // activate the IDE's Vue plugin
  • -
    {{ org.name }}
    +
    {{ org.full_name ? `${org.full_name} (${org.name})` : org.name }}
    {{ org.org_visibility === 'limited' ? textOrgVisibilityLimited: textOrgVisibilityPrivate }} From 5eb0ee49a101bfc899892f4879d5da4a4bcfbfbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baltaz=C3=A1r=20Radics?= Date: Mon, 18 Nov 2024 12:24:17 +0100 Subject: [PATCH 06/23] Use user.FullName in Oauth2 id_token response (#32542) This makes `/login/oauth/authorize` behave the same way as the `/login/oauth/userinfo` endpoint. --- routers/web/auth/oauth2_provider.go | 2 +- routers/web/auth/oauth_test.go | 21 +-------------------- services/oauth2_provider/access_token.go | 2 +- 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index d844d42421..2ccc4a2253 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -98,7 +98,7 @@ func InfoOAuth(ctx *context.Context) { response := &userInfoResponse{ Sub: fmt.Sprint(ctx.Doer.ID), - Name: ctx.Doer.FullName, + Name: ctx.Doer.DisplayName(), PreferredUsername: ctx.Doer.Name, Email: ctx.Doer.Email, Picture: ctx.Doer.AvatarLink(ctx), diff --git a/routers/web/auth/oauth_test.go b/routers/web/auth/oauth_test.go index 78af97fa9c..8d9365fab4 100644 --- a/routers/web/auth/oauth_test.go +++ b/routers/web/auth/oauth_test.go @@ -10,7 +10,6 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/services/oauth2_provider" "github.com/golang-jwt/jwt/v5" @@ -66,25 +65,7 @@ func TestNewAccessTokenResponse_OIDCToken(t *testing.T) { // Scopes: openid profile email oidcToken = createAndParseToken(t, grants[0]) - assert.Equal(t, user.Name, oidcToken.Name) - assert.Equal(t, user.Name, oidcToken.PreferredUsername) - assert.Equal(t, user.HTMLURL(), oidcToken.Profile) - assert.Equal(t, user.AvatarLink(db.DefaultContext), oidcToken.Picture) - assert.Equal(t, user.Website, oidcToken.Website) - assert.Equal(t, user.UpdatedUnix, oidcToken.UpdatedAt) - assert.Equal(t, user.Email, oidcToken.Email) - assert.Equal(t, user.IsActive, oidcToken.EmailVerified) - - // set DefaultShowFullName to true - oldDefaultShowFullName := setting.UI.DefaultShowFullName - setting.UI.DefaultShowFullName = true - defer func() { - setting.UI.DefaultShowFullName = oldDefaultShowFullName - }() - - // Scopes: openid profile email - oidcToken = createAndParseToken(t, grants[0]) - assert.Equal(t, user.FullName, oidcToken.Name) + assert.Equal(t, user.DisplayName(), oidcToken.Name) assert.Equal(t, user.Name, oidcToken.PreferredUsername) assert.Equal(t, user.HTMLURL(), oidcToken.Profile) assert.Equal(t, user.AvatarLink(db.DefaultContext), oidcToken.Picture) diff --git a/services/oauth2_provider/access_token.go b/services/oauth2_provider/access_token.go index f79afa4b30..dd3f24eeef 100644 --- a/services/oauth2_provider/access_token.go +++ b/services/oauth2_provider/access_token.go @@ -148,7 +148,7 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server Nonce: grant.Nonce, } if grant.ScopeContains("profile") { - idToken.Name = user.GetDisplayName() + idToken.Name = user.DisplayName() idToken.PreferredUsername = user.Name idToken.Profile = user.HTMLURL() idToken.Picture = user.AvatarLink(ctx) From 32456b6f314f993efdc65fc90248b6fd1a8d55ef Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Tue, 19 Nov 2024 14:57:55 +0800 Subject: [PATCH 07/23] Fix a compilation error in the Gitpod environment (#32559) When opening the latest code in **Gitpod** and running `make lint-backend`, the following error occurs: ```bash gitpod /workspace/gitea (main) $ make lint-backend go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.3 run # internal/profilerecord compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/goarch compile: version "go1.23.1" does not match go tool version "go1.22.9" # unicode/utf8 compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/coverage/rtcov compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/byteorder compile: version "go1.23.1" does not match go tool version "go1.22.9" # cmp compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/itoa compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/race compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/goos compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/unsafeheader compile: version "go1.23.1" does not match go tool version "go1.22.9" # unicode compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/godebugs compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/asan compile: version "go1.23.1" does not match go tool version "go1.22.9" # math/bits compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/goexperiment compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/msan compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/runtime/atomic compile: version "go1.23.1" does not match go tool version "go1.22.9" # sync/atomic compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/runtime/syscall compile: version "go1.23.1" does not match go tool version "go1.22.9" # crypto/internal/alias compile: version "go1.23.1" does not match go tool version "go1.22.9" # encoding compile: version "go1.23.1" does not match go tool version "go1.22.9" # log/internal compile: version "go1.23.1" does not match go tool version "go1.22.9" # vendor/golang.org/x/crypto/cryptobyte/asn1 compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/golangci/golangci-lint/pkg/exitcodes compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/cpu compile: version "go1.23.1" does not match go tool version "go1.22.9" # unicode/utf16 compile: version "go1.23.1" does not match go tool version "go1.22.9" # container/list compile: version "go1.23.1" does not match go tool version "go1.22.9" # crypto/subtle compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/goversion compile: version "go1.23.1" does not match go tool version "go1.22.9" # golang.org/x/exp/maps compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/ccojocar/zxcvbn-go/match compile: version "go1.23.1" does not match go tool version "go1.22.9" # golang.org/x/exp/constraints compile: version "go1.23.1" does not match go tool version "go1.22.9" # golang.org/x/tools/internal/packagesinternal compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/quasilyte/go-ruleguard/dsl/types compile: version "go1.23.1" does not match go tool version "go1.22.9" # vendor/golang.org/x/crypto/internal/alias compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/nettrace compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/google/go-cmp/cmp/internal/flags compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/gobwas/glob/util/runes compile: version "go1.23.1" does not match go tool version "go1.22.9" # internal/platform compile: version "go1.23.1" does not match go tool version "go1.22.9" # crypto/internal/boring/sig compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/quasilyte/gogrep/internal/stdinfo compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/daixiang0/gci/pkg/utils compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/quasilyte/stdinfo compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/Antonboom/testifylint/internal/testify compile: version "go1.23.1" does not match go tool version "go1.22.9" # hash/maphash compile: version "go1.23.1" does not match go tool version "go1.22.9" # github.com/nunnatsa/ginkgolinter/version compile: version "go1.23.1" does not match go tool version "go1.22.9" # google.golang.org/protobuf/internal/flags compile: version "go1.23.1" does not match go tool version "go1.22.9" make: *** [Makefile:413: lint-go] Error 1 ``` --- flake.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/flake.nix b/flake.nix index 2c9d74c137..e3655b627e 100644 --- a/flake.nix +++ b/flake.nix @@ -29,7 +29,6 @@ poetry # backend - go_1_22 gofumpt sqlite ]; From 0d5abd9b3e04a09f5d7de720c99e3451723e028e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 19 Nov 2024 08:21:13 -0800 Subject: [PATCH 08/23] Remove unnecessary code (#32560) PushMirrors only be used in the repository setting page. So it should not be loaded on every repository page. --- services/context/repo.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/services/context/repo.go b/services/context/repo.go index e7b32d6283..1eafb7ca48 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -393,14 +393,7 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { } } - pushMirrors, _, err := repo_model.GetPushMirrorsByRepoID(ctx, repo.ID, db.ListOptions{}) - if err != nil { - ctx.ServerError("GetPushMirrorsByRepoID", err) - return - } - ctx.Repo.Repository = repo - ctx.Data["PushMirrors"] = pushMirrors ctx.Data["RepoName"] = ctx.Repo.Repository.Name ctx.Data["IsEmptyRepo"] = ctx.Repo.Repository.IsEmpty From 69268ee19f7fc33f6d3ea3a3a7eb4a000e825863 Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Wed, 20 Nov 2024 08:39:57 +0800 Subject: [PATCH 09/23] Optimize installation-page experience (#32558) ![3000-gogitea-gitea-kiagpwhqbx1 ws-us116 gitpod io_ (1)](https://github.com/user-attachments/assets/7f9ff835-7122-420e-83a9-218a1b9c7030) Highlight the path of the configuration file with a label-style emphasis and provide a quick copy button. --- options/locale/locale_en-US.ini | 1 + templates/install.tmpl | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c3639fb72e..122b70924a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -352,6 +352,7 @@ enable_update_checker = Enable Update Checker enable_update_checker_helper = Checks for new version releases periodically by connecting to gitea.io. env_config_keys = Environment Configuration env_config_keys_prompt = The following environment variables will also be applied to your configuration file: +config_write_file_prompt = These configuration options will be written into: [home] nav_menu = Navigation Menu diff --git a/templates/install.tmpl b/templates/install.tmpl index 5055031a90..ea4023d409 100644 --- a/templates/install.tmpl +++ b/templates/install.tmpl @@ -338,7 +338,7 @@
    - These configuration options will be written into: {{.CustomConfFile}} + {{ctx.Locale.Tr "install.config_write_file_prompt"}} {{.CustomConfFile}}
    From 355889dbc2432554f0bcdb22f918488849f0016c Mon Sep 17 00:00:00 2001 From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com> Date: Tue, 19 Nov 2024 17:05:06 -0800 Subject: [PATCH 10/23] Remove duplicate empty repo check in delete branch API (#32569) Found while working on #32433. This branch will never be executed because we have would have already made the same check a couple lines above. --- routers/api/v1/repo/branch.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index bb16858c81..1cea7d8c72 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -133,11 +133,6 @@ func DeleteBranch(ctx *context.APIContext) { branchName := ctx.PathParam("*") - if ctx.Repo.Repository.IsEmpty { - ctx.Error(http.StatusForbidden, "", "Git Repository is empty.") - return - } - // check whether branches of this repository has been synced totalNumOfBranches, err := db.Count[git_model.Branch](ctx, git_model.FindBranchOptions{ RepoID: ctx.Repo.Repository.ID, From 56bff7ae234ee21d0e4524e401a49385c383ccaf Mon Sep 17 00:00:00 2001 From: Marcell Mars Date: Wed, 20 Nov 2024 15:22:48 +0100 Subject: [PATCH 11/23] Support HTTP POST requests to `/userinfo`, aligning to OpenID Core specification (#32578) This PR adds support for the HTTP POST requests to `/userinfo` endpoint. While the OpenID Core specification says both are supported and recommends using HTTP GET. ref: https://openid.net/specs/openid-connect-core-1_0.html#UserInfo --- routers/web/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index 137c677306..b96d06ed66 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -561,7 +561,7 @@ func registerRoutes(m *web.Router) { m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) }, optSignInIgnoreCsrf, reqSignIn) - m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), optSignInIgnoreCsrf, auth.InfoOAuth) + m.Methods("GET, POST, OPTIONS", "/userinfo", optionsCorsHandler(), optSignInIgnoreCsrf, auth.InfoOAuth) m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), optSignInIgnoreCsrf, auth.AccessTokenOAuth) m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), optSignInIgnoreCsrf, auth.OIDCKeys) m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), optSignInIgnoreCsrf, auth.IntrospectOAuth) From 407b6e6dfc7ee9ebb8a16c7f1a786e4c24d0516e Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 20 Nov 2024 09:24:09 -0600 Subject: [PATCH 12/23] allow the actions user to login via the jwt token (#32527) We have some actions that leverage the Gitea API that began receiving 401 errors, with a message that the user was not found. These actions use the `ACTIONS_RUNTIME_TOKEN` env var in the actions job to authenticate with the Gitea API. The format of this env var in actions jobs changed with go-gitea/gitea/pull/28885 to be a JWT (with a corresponding update to `act_runner`) Since it was a JWT, the OAuth parsing logic attempted to parse it as an OAuth token, and would return user not found, instead of falling back to look up the running task and assigning it to the actions user. Make ACTIONS_RUNTIME_TOKEN in action runners could be used, attempting to parse Oauth JWTs. The code to parse potential old `ACTION_RUNTIME_TOKEN` was kept in case someone is running an older version of act_runner that doesn't support the Actions JWT. --- models/fixtures/action_task.yml | 19 ++++++++++++ services/actions/auth.go | 11 +++++-- services/auth/oauth2.go | 23 ++++++++++++++ services/auth/oauth2_test.go | 55 +++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 services/auth/oauth2_test.go diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index 443effe08c..d88a8ed8a9 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -1,3 +1,22 @@ +- + id: 46 + attempt: 3 + runner_id: 1 + status: 3 # 3 is the status code for "cancelled" + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: 6d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2aaaaa + token_salt: eeeeeeee + token_last_eight: eeeeeeee + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 - id: 47 job_id: 192 diff --git a/services/actions/auth.go b/services/actions/auth.go index 8e934d89a8..1ef21f6e0e 100644 --- a/services/actions/auth.go +++ b/services/actions/auth.go @@ -83,7 +83,12 @@ func ParseAuthorizationToken(req *http.Request) (int64, error) { return 0, fmt.Errorf("split token failed") } - token, err := jwt.ParseWithClaims(parts[1], &actionsClaims{}, func(t *jwt.Token) (any, error) { + return TokenToTaskID(parts[1]) +} + +// TokenToTaskID returns the TaskID associated with the provided JWT token +func TokenToTaskID(token string) (int64, error) { + parsedToken, err := jwt.ParseWithClaims(token, &actionsClaims{}, func(t *jwt.Token) (any, error) { if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok { return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"]) } @@ -93,8 +98,8 @@ func ParseAuthorizationToken(req *http.Request) (int64, error) { return 0, err } - c, ok := token.Claims.(*actionsClaims) - if !token.Valid || !ok { + c, ok := parsedToken.Claims.(*actionsClaims) + if !parsedToken.Valid || !ok { return 0, fmt.Errorf("invalid token claim") } diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index d0aec085b1..251ae5a244 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/actions" "code.gitea.io/gitea/services/oauth2_provider" ) @@ -54,6 +55,18 @@ func CheckOAuthAccessToken(ctx context.Context, accessToken string) int64 { return grant.UserID } +// CheckTaskIsRunning verifies that the TaskID corresponds to a running task +func CheckTaskIsRunning(ctx context.Context, taskID int64) bool { + // Verify the task exists + task, err := actions_model.GetTaskByID(ctx, taskID) + if err != nil { + return false + } + + // Verify that it's running + return task.Status == actions_model.StatusRunning +} + // OAuth2 implements the Auth interface and authenticates requests // (API requests only) by looking for an OAuth token in query parameters or the // "Authorization" header. @@ -97,6 +110,16 @@ func parseToken(req *http.Request) (string, bool) { func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store DataStore) int64 { // Let's see if token is valid. if strings.Contains(tokenSHA, ".") { + // First attempt to decode an actions JWT, returning the actions user + if taskID, err := actions.TokenToTaskID(tokenSHA); err == nil { + if CheckTaskIsRunning(ctx, taskID) { + store.GetData()["IsActionsToken"] = true + store.GetData()["ActionsTaskID"] = taskID + return user_model.ActionsUserID + } + } + + // Otherwise, check if this is an OAuth access token uid := CheckOAuthAccessToken(ctx, tokenSHA) if uid != 0 { store.GetData()["IsApiToken"] = true diff --git a/services/auth/oauth2_test.go b/services/auth/oauth2_test.go new file mode 100644 index 0000000000..75c231ff7a --- /dev/null +++ b/services/auth/oauth2_test.go @@ -0,0 +1,55 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "context" + "testing" + + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/actions" + + "github.com/stretchr/testify/assert" +) + +func TestUserIDFromToken(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Actions JWT", func(t *testing.T) { + const RunningTaskID = 47 + token, err := actions.CreateAuthorizationToken(RunningTaskID, 1, 2) + assert.NoError(t, err) + + ds := make(middleware.ContextData) + + o := OAuth2{} + uid := o.userIDFromToken(context.Background(), token, ds) + assert.Equal(t, int64(user_model.ActionsUserID), uid) + assert.Equal(t, ds["IsActionsToken"], true) + assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID)) + }) +} + +func TestCheckTaskIsRunning(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + cases := map[string]struct { + TaskID int64 + Expected bool + }{ + "Running": {TaskID: 47, Expected: true}, + "Missing": {TaskID: 1, Expected: false}, + "Cancelled": {TaskID: 46, Expected: false}, + } + + for name := range cases { + c := cases[name] + t.Run(name, func(t *testing.T) { + actual := CheckTaskIsRunning(context.Background(), c.TaskID) + assert.Equal(t, c.Expected, actual) + }) + } +} From 33850a83fe4ebd23a762a7aac81614c42e303bfa Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 20 Nov 2024 11:26:12 -0800 Subject: [PATCH 13/23] Fix submodule parsing (#32571) Fix #32568, parse `.gitmodules` correctly --------- Co-authored-by: wxiaoguang --- modules/git/commit.go | 66 +---- modules/git/commit_info.go | 2 +- modules/git/commit_info_gogit.go | 4 +- modules/git/commit_info_nogogit.go | 2 +- modules/git/commit_submodule.go | 47 ++++ ...{submodule.go => commit_submodule_file.go} | 24 +- ..._test.go => commit_submodule_file_test.go} | 2 +- modules/git/commit_test.go | 4 +- modules/git/config.go | 187 +++++++++++++++ modules/git/config_submodule.go | 75 ++++++ modules/git/config_submodule_test.go | 49 ++++ modules/git/config_test.go | 66 +++++ modules/git/fsck.go | 14 ++ modules/git/git.go | 227 ++---------------- modules/git/git_test.go | 53 ---- modules/git/repo_base_gogit.go | 4 +- modules/git/repo_base_nogogit.go | 4 +- modules/git/repo_tag_gogit.go | 2 +- modules/git/repo_tag_nogogit.go | 2 +- modules/git/utils.go | 18 +- 20 files changed, 492 insertions(+), 360 deletions(-) create mode 100644 modules/git/commit_submodule.go rename modules/git/{submodule.go => commit_submodule_file.go} (83%) rename modules/git/{submodule_test.go => commit_submodule_file_test.go} (97%) create mode 100644 modules/git/config.go create mode 100644 modules/git/config_submodule.go create mode 100644 modules/git/config_submodule_test.go create mode 100644 modules/git/config_test.go create mode 100644 modules/git/fsck.go diff --git a/modules/git/commit.go b/modules/git/commit.go index 86adaa79a6..010b56948e 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "errors" - "fmt" "io" "os/exec" "strconv" @@ -29,7 +28,7 @@ type Commit struct { Signature *CommitSignature Parents []ObjectID // ID strings - submoduleCache *ObjectCache + submoduleCache *ObjectCache[*SubModule] } // CommitSignature represents a git commit signature part. @@ -357,69 +356,6 @@ func (c *Commit) GetFileContent(filename string, limit int) (string, error) { return string(bytes), nil } -// GetSubModules get all the sub modules of current revision git tree -func (c *Commit) GetSubModules() (*ObjectCache, error) { - if c.submoduleCache != nil { - return c.submoduleCache, nil - } - - entry, err := c.GetTreeEntryByPath(".gitmodules") - if err != nil { - if _, ok := err.(ErrNotExist); ok { - return nil, nil - } - return nil, err - } - - rd, err := entry.Blob().DataAsync() - if err != nil { - return nil, err - } - - defer rd.Close() - scanner := bufio.NewScanner(rd) - c.submoduleCache = newObjectCache() - var ismodule bool - var path string - for scanner.Scan() { - if strings.HasPrefix(scanner.Text(), "[submodule") { - ismodule = true - continue - } - if ismodule { - fields := strings.Split(scanner.Text(), "=") - k := strings.TrimSpace(fields[0]) - if k == "path" { - path = strings.TrimSpace(fields[1]) - } else if k == "url" { - c.submoduleCache.Set(path, &SubModule{path, strings.TrimSpace(fields[1])}) - ismodule = false - } - } - } - if err = scanner.Err(); err != nil { - return nil, fmt.Errorf("GetSubModules scan: %w", err) - } - - return c.submoduleCache, nil -} - -// GetSubModule get the sub module according entryname -func (c *Commit) GetSubModule(entryname string) (*SubModule, error) { - modules, err := c.GetSubModules() - if err != nil { - return nil, err - } - - if modules != nil { - module, has := modules.Get(entryname) - if has { - return module.(*SubModule), nil - } - } - return nil, nil -} - // GetBranchName gets the closest branch name (as returned by 'git name-rev --name-only') func (c *Commit) GetBranchName() (string, error) { cmd := NewCommand(c.repo.Ctx, "name-rev") diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go index c740a4e13e..545081275b 100644 --- a/modules/git/commit_info.go +++ b/modules/git/commit_info.go @@ -7,5 +7,5 @@ package git type CommitInfo struct { Entry *TreeEntry Commit *Commit - SubModuleFile *SubModuleFile + SubModuleFile *CommitSubModuleFile } diff --git a/modules/git/commit_info_gogit.go b/modules/git/commit_info_gogit.go index 31ffc9aec1..11b44f7c35 100644 --- a/modules/git/commit_info_gogit.go +++ b/modules/git/commit_info_gogit.go @@ -71,7 +71,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath commitsInfo[i].Commit = entryCommit } - // If the entry if a submodule add a submodule file for this + // If the entry is a submodule add a submodule file for this if entry.IsSubModule() { subModuleURL := "" var fullPath string @@ -85,7 +85,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath } else if subModule != nil { subModuleURL = subModule.URL } - subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String()) + subModuleFile := NewCommitSubModuleFile(subModuleURL, entry.ID.String()) commitsInfo[i].SubModuleFile = subModuleFile } } diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go index cfde64a033..20d586f0ff 100644 --- a/modules/git/commit_info_nogogit.go +++ b/modules/git/commit_info_nogogit.go @@ -79,7 +79,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath } else if subModule != nil { subModuleURL = subModule.URL } - subModuleFile := NewSubModuleFile(commitsInfo[i].Commit, subModuleURL, entry.ID.String()) + subModuleFile := NewCommitSubModuleFile(subModuleURL, entry.ID.String()) commitsInfo[i].SubModuleFile = subModuleFile } } diff --git a/modules/git/commit_submodule.go b/modules/git/commit_submodule.go new file mode 100644 index 0000000000..6603061da2 --- /dev/null +++ b/modules/git/commit_submodule.go @@ -0,0 +1,47 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +// GetSubModules get all the submodules of current revision git tree +func (c *Commit) GetSubModules() (*ObjectCache[*SubModule], error) { + if c.submoduleCache != nil { + return c.submoduleCache, nil + } + + entry, err := c.GetTreeEntryByPath(".gitmodules") + if err != nil { + if _, ok := err.(ErrNotExist); ok { + return nil, nil + } + return nil, err + } + + rd, err := entry.Blob().DataAsync() + if err != nil { + return nil, err + } + defer rd.Close() + + // at the moment we do not strictly limit the size of the .gitmodules file because some users would have huge .gitmodules files (>1MB) + c.submoduleCache, err = configParseSubModules(rd) + if err != nil { + return nil, err + } + return c.submoduleCache, nil +} + +// GetSubModule get the submodule according entry name +func (c *Commit) GetSubModule(entryName string) (*SubModule, error) { + modules, err := c.GetSubModules() + if err != nil { + return nil, err + } + + if modules != nil { + if module, has := modules.Get(entryName); has { + return module, nil + } + } + return nil, nil +} diff --git a/modules/git/submodule.go b/modules/git/commit_submodule_file.go similarity index 83% rename from modules/git/submodule.go rename to modules/git/commit_submodule_file.go index b99c81582b..bdec35f682 100644 --- a/modules/git/submodule.go +++ b/modules/git/commit_submodule_file.go @@ -15,24 +15,15 @@ import ( var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+@)?([a-zA-Z0-9._-]+):(.*)$`) -// SubModule submodule is a reference on git repository -type SubModule struct { - Name string - URL string -} - -// SubModuleFile represents a file with submodule type. -type SubModuleFile struct { - *Commit - +// CommitSubModuleFile represents a file with submodule type. +type CommitSubModuleFile struct { refURL string refID string } -// NewSubModuleFile create a new submodule file -func NewSubModuleFile(c *Commit, refURL, refID string) *SubModuleFile { - return &SubModuleFile{ - Commit: c, +// NewCommitSubModuleFile create a new submodule file +func NewCommitSubModuleFile(refURL, refID string) *CommitSubModuleFile { + return &CommitSubModuleFile{ refURL: refURL, refID: refID, } @@ -109,11 +100,12 @@ func getRefURL(refURL, urlPrefix, repoFullName, sshDomain string) string { } // RefURL guesses and returns reference URL. -func (sf *SubModuleFile) RefURL(urlPrefix, repoFullName, sshDomain string) string { +// FIXME: template passes AppURL as urlPrefix, it needs to figure out the correct approach (no hard-coded AppURL anymore) +func (sf *CommitSubModuleFile) RefURL(urlPrefix, repoFullName, sshDomain string) string { return getRefURL(sf.refURL, urlPrefix, repoFullName, sshDomain) } // RefID returns reference ID. -func (sf *SubModuleFile) RefID() string { +func (sf *CommitSubModuleFile) RefID() string { return sf.refID } diff --git a/modules/git/submodule_test.go b/modules/git/commit_submodule_file_test.go similarity index 97% rename from modules/git/submodule_test.go rename to modules/git/commit_submodule_file_test.go index e05f2510c4..473b996b82 100644 --- a/modules/git/submodule_test.go +++ b/modules/git/commit_submodule_file_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGetRefURL(t *testing.T) { +func TestCommitSubModuleFileGetRefURL(t *testing.T) { kases := []struct { refURL string prefixURL string diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index 0ddeb182ef..bf381a5350 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -135,7 +135,7 @@ author KN4CK3R 1711702962 +0100 committer KN4CK3R 1711702962 +0100 encoding ISO-8859-1 gpgsig -----BEGIN PGP SIGNATURE----- - + iQGzBAABCgAdFiEE9HRrbqvYxPT8PXbefPSEkrowAa8FAmYGg7IACgkQfPSEkrow Aa9olwv+P0HhtCM6CRvlUmPaqswRsDPNR4i66xyXGiSxdI9V5oJL7HLiQIM7KrFR gizKa2COiGtugv8fE+TKqXKaJx6uJUJEjaBd8E9Af9PrAzjWj+A84lU6/PgPS8hq @@ -150,7 +150,7 @@ gpgsig -----BEGIN PGP SIGNATURE----- -----END PGP SIGNATURE----- ISO-8859-1` - + commitString = strings.ReplaceAll(commitString, "", " ") sha := &Sha1Hash{0xfe, 0xaf, 0x4b, 0xa6, 0xbc, 0x63, 0x5f, 0xec, 0x44, 0x2f, 0x46, 0xdd, 0xd4, 0x51, 0x24, 0x16, 0xec, 0x43, 0xc2, 0xc2} gitRepo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare")) assert.NoError(t, err) diff --git a/modules/git/config.go b/modules/git/config.go new file mode 100644 index 0000000000..9c36cf1654 --- /dev/null +++ b/modules/git/config.go @@ -0,0 +1,187 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "fmt" + "os" + "regexp" + "runtime" + "strings" + + "code.gitea.io/gitea/modules/setting" +) + +// syncGitConfig only modifies gitconfig, won't change global variables (otherwise there will be data-race problem) +func syncGitConfig() (err error) { + if err = os.MkdirAll(HomeDir(), os.ModePerm); err != nil { + return fmt.Errorf("unable to prepare git home directory %s, err: %w", HomeDir(), err) + } + + // first, write user's git config options to git config file + // user config options could be overwritten by builtin values later, because if a value is builtin, it must have some special purposes + for k, v := range setting.GitConfig.Options { + if err = configSet(strings.ToLower(k), v); err != nil { + return err + } + } + + // Git requires setting user.name and user.email in order to commit changes - old comment: "if they're not set just add some defaults" + // TODO: need to confirm whether users really need to change these values manually. It seems that these values are dummy only and not really used. + // If these values are not really used, then they can be set (overwritten) directly without considering about existence. + for configKey, defaultValue := range map[string]string{ + "user.name": "Gitea", + "user.email": "gitea@fake.local", + } { + if err := configSetNonExist(configKey, defaultValue); err != nil { + return err + } + } + + // Set git some configurations - these must be set to these values for gitea to work correctly + if err := configSet("core.quotePath", "false"); err != nil { + return err + } + + if DefaultFeatures().CheckVersionAtLeast("2.10") { + if err := configSet("receive.advertisePushOptions", "true"); err != nil { + return err + } + } + + if DefaultFeatures().CheckVersionAtLeast("2.18") { + if err := configSet("core.commitGraph", "true"); err != nil { + return err + } + if err := configSet("gc.writeCommitGraph", "true"); err != nil { + return err + } + if err := configSet("fetch.writeCommitGraph", "true"); err != nil { + return err + } + } + + if DefaultFeatures().SupportProcReceive { + // set support for AGit flow + if err := configAddNonExist("receive.procReceiveRefs", "refs/for"); err != nil { + return err + } + } else { + if err := configUnsetAll("receive.procReceiveRefs", "refs/for"); err != nil { + return err + } + } + + // Due to CVE-2022-24765, git now denies access to git directories which are not owned by current user. + // However, some docker users and samba users find it difficult to configure their systems correctly, + // so that Gitea's git repositories are owned by the Gitea user. + // (Possibly Windows Service users - but ownership in this case should really be set correctly on the filesystem.) + // See issue: https://github.com/go-gitea/gitea/issues/19455 + // As Gitea now always use its internal git config file, and access to the git repositories is managed through Gitea, + // it is now safe to set "safe.directory=*" for internal usage only. + // Although this setting is only supported by some new git versions, it is also tolerated by earlier versions + if err := configAddNonExist("safe.directory", "*"); err != nil { + return err + } + + if runtime.GOOS == "windows" { + if err := configSet("core.longpaths", "true"); err != nil { + return err + } + if setting.Git.DisableCoreProtectNTFS { + err = configSet("core.protectNTFS", "false") + } else { + err = configUnsetAll("core.protectNTFS", "false") + } + if err != nil { + return err + } + } + + // By default partial clones are disabled, enable them from git v2.22 + if !setting.Git.DisablePartialClone && DefaultFeatures().CheckVersionAtLeast("2.22") { + if err = configSet("uploadpack.allowfilter", "true"); err != nil { + return err + } + err = configSet("uploadpack.allowAnySHA1InWant", "true") + } else { + if err = configUnsetAll("uploadpack.allowfilter", "true"); err != nil { + return err + } + err = configUnsetAll("uploadpack.allowAnySHA1InWant", "true") + } + + return err +} + +func configSet(key, value string) error { + stdout, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil) + if err != nil && !IsErrorExitCode(err, 1) { + return fmt.Errorf("failed to get git config %s, err: %w", key, err) + } + + currValue := strings.TrimSpace(stdout) + if currValue == value { + return nil + } + + _, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil) + if err != nil { + return fmt.Errorf("failed to set git global config %s, err: %w", key, err) + } + + return nil +} + +func configSetNonExist(key, value string) error { + _, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil) + if err == nil { + // already exist + return nil + } + if IsErrorExitCode(err, 1) { + // not exist, set new config + _, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil) + if err != nil { + return fmt.Errorf("failed to set git global config %s, err: %w", key, err) + } + return nil + } + + return fmt.Errorf("failed to get git config %s, err: %w", key, err) +} + +func configAddNonExist(key, value string) error { + _, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key, regexp.QuoteMeta(value)).RunStdString(nil) + if err == nil { + // already exist + return nil + } + if IsErrorExitCode(err, 1) { + // not exist, add new config + _, _, err = NewCommand(DefaultContext, "config", "--global", "--add").AddDynamicArguments(key, value).RunStdString(nil) + if err != nil { + return fmt.Errorf("failed to add git global config %s, err: %w", key, err) + } + return nil + } + return fmt.Errorf("failed to get git config %s, err: %w", key, err) +} + +func configUnsetAll(key, value string) error { + _, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil) + if err == nil { + // exist, need to remove + _, _, err = NewCommand(DefaultContext, "config", "--global", "--unset-all").AddDynamicArguments(key, regexp.QuoteMeta(value)).RunStdString(nil) + if err != nil { + return fmt.Errorf("failed to unset git global config %s, err: %w", key, err) + } + return nil + } + if IsErrorExitCode(err, 1) { + // not exist + return nil + } + return fmt.Errorf("failed to get git config %s, err: %w", key, err) +} diff --git a/modules/git/config_submodule.go b/modules/git/config_submodule.go new file mode 100644 index 0000000000..fe33208850 --- /dev/null +++ b/modules/git/config_submodule.go @@ -0,0 +1,75 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "bufio" + "fmt" + "io" + "strings" +) + +// SubModule is a reference on git repository +type SubModule struct { + Path string + URL string + Branch string // this field is newly added but not really used +} + +// configParseSubModules this is not a complete parse for gitmodules file, it only +// parses the url and path of submodules. At the moment it only parses well-formed gitmodules files. +// In the future, there should be a complete implementation of https://git-scm.com/docs/git-config#_syntax +func configParseSubModules(r io.Reader) (*ObjectCache[*SubModule], error) { + var subModule *SubModule + subModules := newObjectCache[*SubModule]() + scanner := bufio.NewScanner(r) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + + // Skip empty lines and comments + if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") { + continue + } + + // Section header [section] + if strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") { + if subModule != nil { + subModules.Set(subModule.Path, subModule) + } + if strings.HasPrefix(line, "[submodule") { + subModule = &SubModule{} + } else { + subModule = nil + } + continue + } + + if subModule == nil { + continue + } + + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + continue + } + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + switch key { + case "path": + subModule.Path = value + case "url": + subModule.URL = value + case "branch": + subModule.Branch = value + } + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading file: %w", err) + } + if subModule != nil { + subModules.Set(subModule.Path, subModule) + } + return subModules, nil +} diff --git a/modules/git/config_submodule_test.go b/modules/git/config_submodule_test.go new file mode 100644 index 0000000000..f0846c7bfb --- /dev/null +++ b/modules/git/config_submodule_test.go @@ -0,0 +1,49 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConfigSubmodule(t *testing.T) { + input := ` +[core] +path = test + +[submodule "submodule1"] + path = path1 + url = https://gitea.io/foo/foo + #branch = b1 + +[other1] +branch = master + +[submodule "submodule2"] + path = path2 + url = https://gitea.io/bar/bar + branch = b2 + +[other2] +branch = main + +[submodule "submodule3"] + path = path3 + url = https://gitea.io/xxx/xxx +` + + subModules, err := configParseSubModules(strings.NewReader(input)) + assert.NoError(t, err) + assert.Len(t, subModules.cache, 3) + + sm1, _ := subModules.Get("path1") + assert.Equal(t, &SubModule{Path: "path1", URL: "https://gitea.io/foo/foo", Branch: ""}, sm1) + sm2, _ := subModules.Get("path2") + assert.Equal(t, &SubModule{Path: "path2", URL: "https://gitea.io/bar/bar", Branch: "b2"}, sm2) + sm3, _ := subModules.Get("path3") + assert.Equal(t, &SubModule{Path: "path3", URL: "https://gitea.io/xxx/xxx", Branch: ""}, sm3) +} diff --git a/modules/git/config_test.go b/modules/git/config_test.go new file mode 100644 index 0000000000..59f70c99e2 --- /dev/null +++ b/modules/git/config_test.go @@ -0,0 +1,66 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "os" + "strings" + "testing" + + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +func gitConfigContains(sub string) bool { + if b, err := os.ReadFile(HomeDir() + "/.gitconfig"); err == nil { + return strings.Contains(string(b), sub) + } + return false +} + +func TestGitConfig(t *testing.T) { + assert.False(t, gitConfigContains("key-a")) + + assert.NoError(t, configSetNonExist("test.key-a", "val-a")) + assert.True(t, gitConfigContains("key-a = val-a")) + + assert.NoError(t, configSetNonExist("test.key-a", "val-a-changed")) + assert.False(t, gitConfigContains("key-a = val-a-changed")) + + assert.NoError(t, configSet("test.key-a", "val-a-changed")) + assert.True(t, gitConfigContains("key-a = val-a-changed")) + + assert.NoError(t, configAddNonExist("test.key-b", "val-b")) + assert.True(t, gitConfigContains("key-b = val-b")) + + assert.NoError(t, configAddNonExist("test.key-b", "val-2b")) + assert.True(t, gitConfigContains("key-b = val-b")) + assert.True(t, gitConfigContains("key-b = val-2b")) + + assert.NoError(t, configUnsetAll("test.key-b", "val-b")) + assert.False(t, gitConfigContains("key-b = val-b")) + assert.True(t, gitConfigContains("key-b = val-2b")) + + assert.NoError(t, configUnsetAll("test.key-b", "val-2b")) + assert.False(t, gitConfigContains("key-b = val-2b")) + + assert.NoError(t, configSet("test.key-x", "*")) + assert.True(t, gitConfigContains("key-x = *")) + assert.NoError(t, configSetNonExist("test.key-x", "*")) + assert.NoError(t, configUnsetAll("test.key-x", "*")) + assert.False(t, gitConfigContains("key-x = *")) +} + +func TestSyncConfig(t *testing.T) { + oldGitConfig := setting.GitConfig + defer func() { + setting.GitConfig = oldGitConfig + }() + + setting.GitConfig.Options["sync-test.cfg-key-a"] = "CfgValA" + assert.NoError(t, syncGitConfig()) + assert.True(t, gitConfigContains("[sync-test]")) + assert.True(t, gitConfigContains("cfg-key-a = CfgValA")) +} diff --git a/modules/git/fsck.go b/modules/git/fsck.go new file mode 100644 index 0000000000..cec27f165b --- /dev/null +++ b/modules/git/fsck.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "context" + "time" +) + +// Fsck verifies the connectivity and validity of the objects in the database +func Fsck(ctx context.Context, repoPath string, timeout time.Duration, args TrustedCmdArgs) error { + return NewCommand(ctx, "fsck").AddArguments(args...).Run(&RunOpts{Timeout: timeout, Dir: repoPath}) +} diff --git a/modules/git/git.go b/modules/git/git.go index a19dd7771b..e3e5b83274 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -11,7 +11,6 @@ import ( "os" "os/exec" "path/filepath" - "regexp" "runtime" "strings" "time" @@ -95,17 +94,18 @@ func parseGitVersionLine(s string) (*version.Version, error) { return version.NewVersion(versionString) } -// SetExecutablePath changes the path of git executable and checks the file permission and version. -func SetExecutablePath(path string) error { - // If path is empty, we use the default value of GitExecutable "git" to search for the location of git. - if path != "" { - GitExecutable = path +func checkGitVersionCompatibility(gitVer *version.Version) error { + badVersions := []struct { + Version *version.Version + Reason string + }{ + {version.Must(version.NewVersion("2.43.1")), "regression bug of GIT_FLUSH"}, } - absPath, err := exec.LookPath(GitExecutable) - if err != nil { - return fmt.Errorf("git not found: %w", err) + for _, bad := range badVersions { + if gitVer.Equal(bad.Version) { + return errors.New(bad.Reason) + } } - GitExecutable = absPath return nil } @@ -128,6 +128,20 @@ func ensureGitVersion() error { return nil } +// SetExecutablePath changes the path of git executable and checks the file permission and version. +func SetExecutablePath(path string) error { + // If path is empty, we use the default value of GitExecutable "git" to search for the location of git. + if path != "" { + GitExecutable = path + } + absPath, err := exec.LookPath(GitExecutable) + if err != nil { + return fmt.Errorf("git not found: %w", err) + } + GitExecutable = absPath + return nil +} + // HomeDir is the home dir for git to store the global config file used by Gitea internally func HomeDir() string { if setting.Git.HomePath == "" { @@ -204,196 +218,3 @@ func InitFull(ctx context.Context) (err error) { return syncGitConfig() } - -// syncGitConfig only modifies gitconfig, won't change global variables (otherwise there will be data-race problem) -func syncGitConfig() (err error) { - if err = os.MkdirAll(HomeDir(), os.ModePerm); err != nil { - return fmt.Errorf("unable to prepare git home directory %s, err: %w", HomeDir(), err) - } - - // first, write user's git config options to git config file - // user config options could be overwritten by builtin values later, because if a value is builtin, it must have some special purposes - for k, v := range setting.GitConfig.Options { - if err = configSet(strings.ToLower(k), v); err != nil { - return err - } - } - - // Git requires setting user.name and user.email in order to commit changes - old comment: "if they're not set just add some defaults" - // TODO: need to confirm whether users really need to change these values manually. It seems that these values are dummy only and not really used. - // If these values are not really used, then they can be set (overwritten) directly without considering about existence. - for configKey, defaultValue := range map[string]string{ - "user.name": "Gitea", - "user.email": "gitea@fake.local", - } { - if err := configSetNonExist(configKey, defaultValue); err != nil { - return err - } - } - - // Set git some configurations - these must be set to these values for gitea to work correctly - if err := configSet("core.quotePath", "false"); err != nil { - return err - } - - if DefaultFeatures().CheckVersionAtLeast("2.10") { - if err := configSet("receive.advertisePushOptions", "true"); err != nil { - return err - } - } - - if DefaultFeatures().CheckVersionAtLeast("2.18") { - if err := configSet("core.commitGraph", "true"); err != nil { - return err - } - if err := configSet("gc.writeCommitGraph", "true"); err != nil { - return err - } - if err := configSet("fetch.writeCommitGraph", "true"); err != nil { - return err - } - } - - if DefaultFeatures().SupportProcReceive { - // set support for AGit flow - if err := configAddNonExist("receive.procReceiveRefs", "refs/for"); err != nil { - return err - } - } else { - if err := configUnsetAll("receive.procReceiveRefs", "refs/for"); err != nil { - return err - } - } - - // Due to CVE-2022-24765, git now denies access to git directories which are not owned by current user. - // However, some docker users and samba users find it difficult to configure their systems correctly, - // so that Gitea's git repositories are owned by the Gitea user. - // (Possibly Windows Service users - but ownership in this case should really be set correctly on the filesystem.) - // See issue: https://github.com/go-gitea/gitea/issues/19455 - // As Gitea now always use its internal git config file, and access to the git repositories is managed through Gitea, - // it is now safe to set "safe.directory=*" for internal usage only. - // Although this setting is only supported by some new git versions, it is also tolerated by earlier versions - if err := configAddNonExist("safe.directory", "*"); err != nil { - return err - } - - if runtime.GOOS == "windows" { - if err := configSet("core.longpaths", "true"); err != nil { - return err - } - if setting.Git.DisableCoreProtectNTFS { - err = configSet("core.protectNTFS", "false") - } else { - err = configUnsetAll("core.protectNTFS", "false") - } - if err != nil { - return err - } - } - - // By default partial clones are disabled, enable them from git v2.22 - if !setting.Git.DisablePartialClone && DefaultFeatures().CheckVersionAtLeast("2.22") { - if err = configSet("uploadpack.allowfilter", "true"); err != nil { - return err - } - err = configSet("uploadpack.allowAnySHA1InWant", "true") - } else { - if err = configUnsetAll("uploadpack.allowfilter", "true"); err != nil { - return err - } - err = configUnsetAll("uploadpack.allowAnySHA1InWant", "true") - } - - return err -} - -func checkGitVersionCompatibility(gitVer *version.Version) error { - badVersions := []struct { - Version *version.Version - Reason string - }{ - {version.Must(version.NewVersion("2.43.1")), "regression bug of GIT_FLUSH"}, - } - for _, bad := range badVersions { - if gitVer.Equal(bad.Version) { - return errors.New(bad.Reason) - } - } - return nil -} - -func configSet(key, value string) error { - stdout, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil) - if err != nil && !IsErrorExitCode(err, 1) { - return fmt.Errorf("failed to get git config %s, err: %w", key, err) - } - - currValue := strings.TrimSpace(stdout) - if currValue == value { - return nil - } - - _, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil) - if err != nil { - return fmt.Errorf("failed to set git global config %s, err: %w", key, err) - } - - return nil -} - -func configSetNonExist(key, value string) error { - _, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil) - if err == nil { - // already exist - return nil - } - if IsErrorExitCode(err, 1) { - // not exist, set new config - _, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil) - if err != nil { - return fmt.Errorf("failed to set git global config %s, err: %w", key, err) - } - return nil - } - - return fmt.Errorf("failed to get git config %s, err: %w", key, err) -} - -func configAddNonExist(key, value string) error { - _, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key, regexp.QuoteMeta(value)).RunStdString(nil) - if err == nil { - // already exist - return nil - } - if IsErrorExitCode(err, 1) { - // not exist, add new config - _, _, err = NewCommand(DefaultContext, "config", "--global", "--add").AddDynamicArguments(key, value).RunStdString(nil) - if err != nil { - return fmt.Errorf("failed to add git global config %s, err: %w", key, err) - } - return nil - } - return fmt.Errorf("failed to get git config %s, err: %w", key, err) -} - -func configUnsetAll(key, value string) error { - _, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil) - if err == nil { - // exist, need to remove - _, _, err = NewCommand(DefaultContext, "config", "--global", "--unset-all").AddDynamicArguments(key, regexp.QuoteMeta(value)).RunStdString(nil) - if err != nil { - return fmt.Errorf("failed to unset git global config %s, err: %w", key, err) - } - return nil - } - if IsErrorExitCode(err, 1) { - // not exist - return nil - } - return fmt.Errorf("failed to get git config %s, err: %w", key, err) -} - -// Fsck verifies the connectivity and validity of the objects in the database -func Fsck(ctx context.Context, repoPath string, timeout time.Duration, args TrustedCmdArgs) error { - return NewCommand(ctx, "fsck").AddArguments(args...).Run(&RunOpts{Timeout: timeout, Dir: repoPath}) -} diff --git a/modules/git/git_test.go b/modules/git/git_test.go index fc92bebe04..5472842b76 100644 --- a/modules/git/git_test.go +++ b/modules/git/git_test.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "os" - "strings" "testing" "code.gitea.io/gitea/modules/setting" @@ -43,58 +42,6 @@ func TestMain(m *testing.M) { } } -func gitConfigContains(sub string) bool { - if b, err := os.ReadFile(HomeDir() + "/.gitconfig"); err == nil { - return strings.Contains(string(b), sub) - } - return false -} - -func TestGitConfig(t *testing.T) { - assert.False(t, gitConfigContains("key-a")) - - assert.NoError(t, configSetNonExist("test.key-a", "val-a")) - assert.True(t, gitConfigContains("key-a = val-a")) - - assert.NoError(t, configSetNonExist("test.key-a", "val-a-changed")) - assert.False(t, gitConfigContains("key-a = val-a-changed")) - - assert.NoError(t, configSet("test.key-a", "val-a-changed")) - assert.True(t, gitConfigContains("key-a = val-a-changed")) - - assert.NoError(t, configAddNonExist("test.key-b", "val-b")) - assert.True(t, gitConfigContains("key-b = val-b")) - - assert.NoError(t, configAddNonExist("test.key-b", "val-2b")) - assert.True(t, gitConfigContains("key-b = val-b")) - assert.True(t, gitConfigContains("key-b = val-2b")) - - assert.NoError(t, configUnsetAll("test.key-b", "val-b")) - assert.False(t, gitConfigContains("key-b = val-b")) - assert.True(t, gitConfigContains("key-b = val-2b")) - - assert.NoError(t, configUnsetAll("test.key-b", "val-2b")) - assert.False(t, gitConfigContains("key-b = val-2b")) - - assert.NoError(t, configSet("test.key-x", "*")) - assert.True(t, gitConfigContains("key-x = *")) - assert.NoError(t, configSetNonExist("test.key-x", "*")) - assert.NoError(t, configUnsetAll("test.key-x", "*")) - assert.False(t, gitConfigContains("key-x = *")) -} - -func TestSyncConfig(t *testing.T) { - oldGitConfig := setting.GitConfig - defer func() { - setting.GitConfig = oldGitConfig - }() - - setting.GitConfig.Options["sync-test.cfg-key-a"] = "CfgValA" - assert.NoError(t, syncGitConfig()) - assert.True(t, gitConfigContains("[sync-test]")) - assert.True(t, gitConfigContains("cfg-key-a = CfgValA")) -} - func TestParseGitVersion(t *testing.T) { v, err := parseGitVersionLine("git version 2.29.3") assert.NoError(t, err) diff --git a/modules/git/repo_base_gogit.go b/modules/git/repo_base_gogit.go index a1127f4e6c..0ca1ea79c2 100644 --- a/modules/git/repo_base_gogit.go +++ b/modules/git/repo_base_gogit.go @@ -28,7 +28,7 @@ const isGogit = true type Repository struct { Path string - tagCache *ObjectCache + tagCache *ObjectCache[*Tag] gogitRepo *gogit.Repository gogitStorage *filesystem.Storage @@ -79,7 +79,7 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) { Path: repoPath, gogitRepo: gogitRepo, gogitStorage: storage, - tagCache: newObjectCache(), + tagCache: newObjectCache[*Tag](), Ctx: ctx, objectFormat: ParseGogitHash(plumbing.ZeroHash).Type(), }, nil diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go index 3eb2e2ee6b..477e3b8742 100644 --- a/modules/git/repo_base_nogogit.go +++ b/modules/git/repo_base_nogogit.go @@ -21,7 +21,7 @@ const isGogit = false type Repository struct { Path string - tagCache *ObjectCache + tagCache *ObjectCache[*Tag] gpgSettings *GPGSettings @@ -53,7 +53,7 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) { return &Repository{ Path: repoPath, - tagCache: newObjectCache(), + tagCache: newObjectCache[*Tag](), Ctx: ctx, }, nil } diff --git a/modules/git/repo_tag_gogit.go b/modules/git/repo_tag_gogit.go index 4a7a06e9bd..3e1b4e89ad 100644 --- a/modules/git/repo_tag_gogit.go +++ b/modules/git/repo_tag_gogit.go @@ -72,7 +72,7 @@ func (repo *Repository) getTag(tagID ObjectID, name string) (*Tag, error) { t, ok := repo.tagCache.Get(tagID.String()) if ok { log.Debug("Hit cache: %s", tagID) - tagClone := *t.(*Tag) + tagClone := *t tagClone.Name = name // This is necessary because lightweight tags may have same id return &tagClone, nil } diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go index 8b06a6a1c3..e0a3104249 100644 --- a/modules/git/repo_tag_nogogit.go +++ b/modules/git/repo_tag_nogogit.go @@ -51,7 +51,7 @@ func (repo *Repository) getTag(tagID ObjectID, name string) (*Tag, error) { t, ok := repo.tagCache.Get(tagID.String()) if ok { log.Debug("Hit cache: %s", tagID) - tagClone := *t.(*Tag) + tagClone := *t tagClone.Name = name // This is necessary because lightweight tags may have same id return &tagClone, nil } diff --git a/modules/git/utils.go b/modules/git/utils.go index 53211c6451..56cba9087a 100644 --- a/modules/git/utils.go +++ b/modules/git/utils.go @@ -15,27 +15,25 @@ import ( ) // ObjectCache provides thread-safe cache operations. -type ObjectCache struct { +type ObjectCache[T any] struct { lock sync.RWMutex - cache map[string]any + cache map[string]T } -func newObjectCache() *ObjectCache { - return &ObjectCache{ - cache: make(map[string]any, 10), - } +func newObjectCache[T any]() *ObjectCache[T] { + return &ObjectCache[T]{cache: make(map[string]T, 10)} } -// Set add obj to cache -func (oc *ObjectCache) Set(id string, obj any) { +// Set adds obj to cache +func (oc *ObjectCache[T]) Set(id string, obj T) { oc.lock.Lock() defer oc.lock.Unlock() oc.cache[id] = obj } -// Get get cached obj by id -func (oc *ObjectCache) Get(id string) (any, bool) { +// Get gets cached obj by id +func (oc *ObjectCache[T]) Get(id string) (T, bool) { oc.lock.RLock() defer oc.lock.RUnlock() From 23d0f9083e2d8b735b1b0578c348f73c0e4ae3dc Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Thu, 21 Nov 2024 09:23:50 +0800 Subject: [PATCH 14/23] make search box in issue sidebar dropdown list always show when scrolling (#32576) as title, replace #31597 after #32460 --------- Signed-off-by: a1012112796 <1012112796@qq.com> --- .../repo/issue/sidebar/assignee_list.tmpl | 14 ++++---- templates/repo/issue/sidebar/label_list.tmpl | 36 ++++++++++--------- .../repo/issue/sidebar/milestone_list.tmpl | 36 ++++++++++--------- .../repo/issue/sidebar/project_list.tmpl | 34 +++++++++--------- .../repo/issue/sidebar/reviewer_list.tmpl | 36 ++++++++++--------- web_src/css/repo.css | 6 ++++ 6 files changed, 89 insertions(+), 73 deletions(-) diff --git a/templates/repo/issue/sidebar/assignee_list.tmpl b/templates/repo/issue/sidebar/assignee_list.tmpl index bee6123e52..d8ccd73387 100644 --- a/templates/repo/issue/sidebar/assignee_list.tmpl +++ b/templates/repo/issue/sidebar/assignee_list.tmpl @@ -16,12 +16,14 @@
    {{ctx.Locale.Tr "repo.issues.new.clear_assignees"}}
    - {{range $data.CandidateAssignees}} -
    - {{svg "octicon-check"}} - {{ctx.AvatarUtils.Avatar . 20}} {{template "repo/search_name" .}} - - {{end}} +
    diff --git a/templates/repo/issue/sidebar/label_list.tmpl b/templates/repo/issue/sidebar/label_list.tmpl index ed80047661..526eb1ec04 100644 --- a/templates/repo/issue/sidebar/label_list.tmpl +++ b/templates/repo/issue/sidebar/label_list.tmpl @@ -17,25 +17,27 @@
    {{ctx.Locale.Tr "repo.issues.new.clear_labels"}} - {{$previousExclusiveScope := "_no_scope"}} - {{range $data.RepoLabels}} - {{$exclusiveScope := .ExclusiveScope}} - {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}} -
    + {{end}} diff --git a/templates/repo/issue/sidebar/milestone_list.tmpl b/templates/repo/issue/sidebar/milestone_list.tmpl index 4f2b4cb06f..2d16c6e1b4 100644 --- a/templates/repo/issue/sidebar/milestone_list.tmpl +++ b/templates/repo/issue/sidebar/milestone_list.tmpl @@ -20,25 +20,27 @@
    {{ctx.Locale.Tr "repo.issues.new.clear_milestone"}}
    - {{if $data.OpenMilestones}} -
    -
    {{ctx.Locale.Tr "repo.issues.new.open_milestone"}}
    - {{range $data.OpenMilestones}} - - {{svg "octicon-milestone" 18}} {{.Name}} - + diff --git a/templates/repo/issue/sidebar/project_list.tmpl b/templates/repo/issue/sidebar/project_list.tmpl index ab1243cadd..6ca6156d2c 100644 --- a/templates/repo/issue/sidebar/project_list.tmpl +++ b/templates/repo/issue/sidebar/project_list.tmpl @@ -18,24 +18,26 @@ {{end}}
    {{ctx.Locale.Tr "repo.issues.new.clear_projects"}}
    - {{if $data.OpenProjects}} -
    -
    {{ctx.Locale.Tr "repo.issues.new.open_projects"}}
    - {{range $data.OpenProjects}} - - {{svg .IconName 18}} {{.Title}} - +
    diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index e990fc5afc..16eea23d69 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -17,27 +17,29 @@
    {{end}} - {{range $data.Reviewers}} - {{if .User}} - - {{svg "octicon-check"}} - {{ctx.AvatarUtils.Avatar .User 20}} {{template "repo/search_name" .User}} - - {{end}} - {{end}} - {{if $data.TeamReviewers}} - {{if $data.Reviewers}}
    {{end}} - {{range $data.TeamReviewers}} - {{if .Team}} - + diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 01ddab97e5..7307b97870 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -66,6 +66,12 @@ overflow-x: auto; } +.issue-content-right .dropdown > .menu .item-secondary-info small { + display: block; + text-overflow: ellipsis; + overflow: hidden; +} + @media (max-width: 767.98px) { .issue-content-left, .issue-content-right { From efb55cd8ef484bbe949e95465b4c17d6e31e7270 Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Thu, 21 Nov 2024 10:42:37 +0800 Subject: [PATCH 15/23] Supplement and Improvement for #32558 (#32585) Thank you for @wxiaoguang's reminders and suggestions: https://github.com/go-gitea/gitea/pull/32558#discussion_r1849972913 --- options/locale/locale_en-US.ini | 2 +- templates/install.tmpl | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 122b70924a..d75827be5c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -352,7 +352,7 @@ enable_update_checker = Enable Update Checker enable_update_checker_helper = Checks for new version releases periodically by connecting to gitea.io. env_config_keys = Environment Configuration env_config_keys_prompt = The following environment variables will also be applied to your configuration file: -config_write_file_prompt = These configuration options will be written into: +config_write_file_prompt = These configuration options will be written into: %s [home] nav_menu = Navigation Menu diff --git a/templates/install.tmpl b/templates/install.tmpl index ea4023d409..6c4cc7df01 100644 --- a/templates/install.tmpl +++ b/templates/install.tmpl @@ -338,7 +338,9 @@
    - {{ctx.Locale.Tr "install.config_write_file_prompt"}} {{.CustomConfFile}} + {{$copyBtn := svg "octicon-copy" 14}} + {{$filePath := HTMLFormat `%s ` .CustomConfFile .CustomConfFile $copyBtn}} + {{ctx.Locale.Tr "install.config_write_file_prompt" $filePath}}
    From 07373f1d5dd315966bc2f6085a8dd8cca193ec5c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 21 Nov 2024 11:31:54 +0800 Subject: [PATCH 16/23] Improve issue sidebar UI (#32587) 1. remove duplicate dividers 2. align reviewer items 3. merge & remove unused CSS styles Before:
    ![image](https://github.com/user-attachments/assets/1b3121ee-b5fa-4fe9-b0f2-344d96dc5fbc) ![image](https://github.com/user-attachments/assets/ba8b97e6-114d-488c-adee-48f6c7a3b580)
    After:
    ![image](https://github.com/user-attachments/assets/978eab3e-a5d7-4b68-90ce-079b61994d25) ![image](https://github.com/user-attachments/assets/a8b58a27-dd05-4c8d-be60-816439ce77c6) ![image](https://github.com/user-attachments/assets/b7e6a16c-bf98-4465-a805-9f4a642d366e)
    --- templates/repo/issue/sidebar/label_list.tmpl | 2 +- templates/repo/issue/sidebar/milestone_list.tmpl | 3 +-- templates/repo/issue/sidebar/project_list.tmpl | 3 +-- templates/repo/issue/sidebar/reviewer_list.tmpl | 2 +- web_src/css/base.css | 4 ++++ web_src/css/repo.css | 10 ---------- 6 files changed, 8 insertions(+), 16 deletions(-) diff --git a/templates/repo/issue/sidebar/label_list.tmpl b/templates/repo/issue/sidebar/label_list.tmpl index 526eb1ec04..fb8f1a667e 100644 --- a/templates/repo/issue/sidebar/label_list.tmpl +++ b/templates/repo/issue/sidebar/label_list.tmpl @@ -27,7 +27,7 @@ {{$previousExclusiveScope = $exclusiveScope}} {{template "repo/issue/sidebar/label_list_item" dict "Label" .}} {{end}} -
    + {{if and $data.RepoLabels $data.OrgLabels}}
    {{end}} {{$previousExclusiveScope = "_no_scope"}} {{range $data.OrgLabels}} {{$exclusiveScope := .ExclusiveScope}} diff --git a/templates/repo/issue/sidebar/milestone_list.tmpl b/templates/repo/issue/sidebar/milestone_list.tmpl index 2d16c6e1b4..a5ed0eef55 100644 --- a/templates/repo/issue/sidebar/milestone_list.tmpl +++ b/templates/repo/issue/sidebar/milestone_list.tmpl @@ -22,7 +22,6 @@
    {{ctx.Locale.Tr "repo.issues.new.clear_milestone"}}