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 ]; 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/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/models/fixtures/system_setting.yml b/models/fixtures/system_setting.yml index 30542bc82a..dcad176c89 100644 --- a/models/fixtures/system_setting.yml +++ b/models/fixtures/system_setting.yml @@ -1,7 +1,7 @@ - id: 1 setting_key: 'picture.disable_gravatar' - setting_value: 'false' + setting_value: 'true' version: 1 created: 1653533198 updated: 1653533198 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 1044e487f8..b3bece5589 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -23,9 +23,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar1 + avatar: "" avatar_email: user1@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -60,8 +60,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar2 + avatar: "" avatar_email: user2@example.com + # cause a random avatar to be generated when referenced for test purposes use_custom_avatar: false num_followers: 2 num_following: 1 @@ -97,9 +98,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar3 + avatar: "" avatar_email: org3@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -134,9 +135,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar4 + avatar: "" avatar_email: user4@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 1 num_stars: 0 @@ -171,9 +172,9 @@ allow_import_local: false allow_create_organization: false prohibit_login: false - avatar: avatar5 + avatar: "" avatar_email: user5@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -208,9 +209,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar6 + avatar: "" avatar_email: org6@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -245,9 +246,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar7 + avatar: "" avatar_email: org7@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -282,9 +283,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar8 + avatar: "" avatar_email: user8@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 1 num_following: 1 num_stars: 0 @@ -319,9 +320,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar9 + avatar: "" avatar_email: user9@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -332,6 +333,7 @@ repo_admin_change_team_access: false theme: "" keep_activity_private: false + created_unix: 1730468968 - id: 10 @@ -356,9 +358,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar10 + avatar: "" avatar_email: user10@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 2 @@ -393,9 +395,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar11 + avatar: "" avatar_email: user11@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -430,9 +432,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar12 + avatar: "" avatar_email: user12@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -467,9 +469,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar13 + avatar: "" avatar_email: user13@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -504,9 +506,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar14 + avatar: "" avatar_email: user13@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -541,9 +543,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar15 + avatar: "" avatar_email: user15@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -578,9 +580,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar16 + avatar: "" avatar_email: user16@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -615,9 +617,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar17 + avatar: "" avatar_email: org17@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -652,9 +654,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar18 + avatar: "" avatar_email: user18@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -689,9 +691,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar19 + avatar: "" avatar_email: org19@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -726,9 +728,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar20 + avatar: "" avatar_email: user20@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -763,9 +765,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar21 + avatar: "" avatar_email: user21@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -800,9 +802,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar22 + avatar: "" avatar_email: limited_org@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -837,9 +839,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar23 + avatar: "" avatar_email: privated_org@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -874,9 +876,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar24 + avatar: "" avatar_email: user24@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -911,9 +913,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar25 + avatar: "" avatar_email: org25@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -948,9 +950,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar26 + avatar: "" avatar_email: org26@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -985,9 +987,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar27 + avatar: "" avatar_email: user27@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1022,9 +1024,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar28 + avatar: "" avatar_email: user28@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1059,9 +1061,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar29 + avatar: "" avatar_email: user29@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1096,9 +1098,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar29 + avatar: "" avatar_email: user30@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1133,9 +1135,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar31 + avatar: "" avatar_email: user31@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 1 num_stars: 0 @@ -1170,9 +1172,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar32 + avatar: "" avatar_email: user30@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1207,9 +1209,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar33 + avatar: "" avatar_email: user33@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 1 num_following: 0 num_stars: 0 @@ -1245,7 +1247,7 @@ allow_import_local: false allow_create_organization: false prohibit_login: false - avatar: avatar34 + avatar: "" avatar_email: user34@example.com use_custom_avatar: true num_followers: 0 @@ -1282,9 +1284,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar35 + avatar: "" avatar_email: private_org35@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1319,9 +1321,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar22 + avatar: "" avatar_email: abcde@gitea.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1356,9 +1358,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: true - avatar: avatar29 + avatar: "" avatar_email: user37@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1393,9 +1395,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar38 + avatar: "" avatar_email: user38@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1430,9 +1432,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar39 + avatar: "" avatar_email: user39@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1467,9 +1469,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar40 + avatar: "" avatar_email: user40@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1504,9 +1506,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar41 + avatar: "" avatar_email: org41@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 @@ -1541,9 +1543,9 @@ allow_import_local: false allow_create_organization: true prohibit_login: false - avatar: avatar42 + avatar: "" avatar_email: org42@example.com - use_custom_avatar: false + use_custom_avatar: true num_followers: 0 num_following: 0 num_stars: 0 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/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/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/models/user/user.go b/models/user/user.go index c1cb988e43..a2d9166291 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -48,19 +48,19 @@ const ( UserTypeIndividual UserType = iota // Historic reason to make it starts at 0. // UserTypeOrganization defines an organization - UserTypeOrganization + UserTypeOrganization // 1 // UserTypeUserReserved reserves a (non-existing) user, i.e. to prevent a spam user from re-registering after being deleted, or to reserve the name until the user is actually created later on - UserTypeUserReserved + UserTypeUserReserved // 2 // UserTypeOrganizationReserved reserves a (non-existing) organization, to be used in combination with UserTypeUserReserved - UserTypeOrganizationReserved + UserTypeOrganizationReserved // 3 // UserTypeBot defines a bot user - UserTypeBot + UserTypeBot // 4 // UserTypeRemoteUser defines a remote user for federated users - UserTypeRemoteUser + UserTypeRemoteUser // 5 ) const ( @@ -884,7 +884,13 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error { // GetInactiveUsers gets all inactive users func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) { - var cond builder.Cond = builder.Eq{"is_active": false} + cond := builder.And( + builder.Eq{"is_active": false}, + builder.Or( // only plain user + builder.Eq{"`type`": UserTypeIndividual}, + builder.Eq{"`type`": UserTypeUserReserved}, + ), + ) if olderThan > 0 { cond = cond.And(builder.Lt{"created_unix": time.Now().Add(-olderThan).Unix()}) diff --git a/models/user/user_test.go b/models/user/user_test.go index bc1abc6451..6701be39a5 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -588,3 +588,17 @@ func TestDisabledUserFeatures(t *testing.T) { assert.True(t, user_model.IsFeatureDisabledWithLoginType(user, f)) } } + +func TestGetInactiveUsers(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // all inactive users + // user1's createdunix is 1730468968 + users, err := user_model.GetInactiveUsers(db.DefaultContext, 0) + assert.NoError(t, err) + assert.Len(t, users, 1) + interval := time.Now().Unix() - 1730468968 + 3600*24 + users, err = user_model.GetInactiveUsers(db.DefaultContext, time.Duration(interval*int64(time.Second))) + assert.NoError(t, err) + assert.Len(t, users, 0) +} 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() 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/repository/commits_test.go b/modules/repository/commits_test.go index 248673a907..3afc116e68 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -4,8 +4,6 @@ package repository import ( - "crypto/md5" - "fmt" "strconv" "testing" "time" @@ -125,15 +123,12 @@ func TestPushCommits_AvatarLink(t *testing.T) { }, } - setting.GravatarSource = "https://secure.gravatar.com/avatar" - setting.OfflineMode = true - assert.Equal(t, - "/avatars/avatar2?size="+strconv.Itoa(28*setting.Avatar.RenderedSizeFactor), + "/avatars/ab53a2911ddf9b4817ac01ddcd3d975f?size="+strconv.Itoa(28*setting.Avatar.RenderedSizeFactor), pushCommits.AvatarLink(db.DefaultContext, "user2@example.com")) assert.Equal(t, - fmt.Sprintf("https://secure.gravatar.com/avatar/%x?d=identicon&s=%d", md5.Sum([]byte("nonexistent@example.com")), 28*setting.Avatar.RenderedSizeFactor), + "/assets/img/avatar_default.png", pushCommits.AvatarLink(db.DefaultContext, "nonexistent@example.com")) } 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/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c3639fb72e..b21c376002 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -104,6 +104,7 @@ copy_url = Copy URL copy_hash = Copy hash copy_content = Copy content copy_branch = Copy branch name +copy_path = Copy path copy_success = Copied! copy_error = Copy failed copy_type_unsupported = This file type cannot be copied @@ -352,6 +353,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: %s [home] nav_menu = Navigation Menu 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, 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/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index faea34959f..2ccc4a2253 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.DisplayName(), + PreferredUsername: ctx.Doer.Name, + Email: ctx.Doer.Email, + Picture: ctx.Doer.AvatarLink(ctx), } groups, err := oauth2_provider.GetOAuthGroupsForUser(ctx, ctx.Doer) 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/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/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/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 { 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) 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) + }) + } +} 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 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/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) diff --git a/services/repository/contributors_graph_test.go b/services/repository/contributors_graph_test.go index f22c115276..6db93f6a64 100644 --- a/services/repository/contributors_graph_test.go +++ b/services/repository/contributors_graph_test.go @@ -18,6 +18,7 @@ import ( func TestRepository_ContributorsGraph(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) assert.NoError(t, repo.LoadOwner(db.DefaultContext)) mockCache, err := cache.NewStringCache(setting.Cache{}) @@ -46,7 +47,7 @@ func TestRepository_ContributorsGraph(t *testing.T) { assert.EqualValues(t, &ContributorData{ Name: "Ethan Koenig", - AvatarLink: "https://secure.gravatar.com/avatar/b42fb195faa8c61b8d88abfefe30e9e3?d=identicon", + AvatarLink: "/assets/img/avatar_default.png", TotalCommits: 1, Weeks: map[int64]*WeekData{ 1511654400000: { 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/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/install.tmpl b/templates/install.tmpl index 5055031a90..6c4cc7df01 100644 --- a/templates/install.tmpl +++ b/templates/install.tmpl @@ -338,7 +338,9 @@
- These configuration options will be written into: {{.CustomConfFile}} + {{$copyBtn := svg "octicon-copy" 14}} + {{$filePath := HTMLFormat `%s ` .CustomConfFile .CustomConfFile $copyBtn}} + {{ctx.Locale.Tr "install.config_write_file_prompt" $filePath}}
diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 2f9d4ecab6..26737f110e 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -130,7 +130,7 @@
{{if $file.IsRenamed}}{{$file.OldName}} → {{end}}{{$file.Name}} {{if .IsLFSFile}} ({{ctx.Locale.Tr "repo.stored_lfs"}}){{end}} - + {{if $file.IsGenerated}} {{ctx.Locale.Tr "repo.diff.generated"}} {{end}} 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/templates/repo/home.tmpl b/templates/repo/home.tmpl index 7a6cbbc4e2..0a8391b553 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -106,6 +106,7 @@ / {{- if eq $i $l -}} {{$v}} + {{- else -}} {{$p := index $.Paths $i}}{{$v}} {{- end -}} 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..fb8f1a667e 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..a5ed0eef55 100644 --- a/templates/repo/issue/sidebar/milestone_list.tmpl +++ b/templates/repo/issue/sidebar/milestone_list.tmpl @@ -20,25 +20,26 @@
{{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..0cbbdce760 100644 --- a/templates/repo/issue/sidebar/project_list.tmpl +++ b/templates/repo/issue/sidebar/project_list.tmpl @@ -18,24 +18,25 @@ {{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..b3d9590d58 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/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/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index d6d27e66be..78e42777bb 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -107,7 +107,7 @@ func TestE2e(t *testing.T) { cmd.Stdout = &stdout cmd.Stderr = &stderr - err := cmd.Run() + err = cmd.Run() if err != nil { // Currently colored output is conflicting. Using Printf until that is resolved. fmt.Printf("%v", stdout.String()) 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/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}) } 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()) + }) +} diff --git a/web_src/css/base.css b/web_src/css/base.css index b5a39c7af6..babbf4c89d 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -1397,6 +1397,10 @@ table th[data-sortt-desc] .svg { gap: .5rem; min-width: 0; } +.ui.dropdown .menu.flex-items-menu > .item img, +.ui.dropdown .menu.flex-items-menu > .item svg { + margin: 0; /* use gap, but not margin */ +} .ui.dropdown.ellipsis-items-nowrap > .text { overflow: hidden; diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 01ddab97e5..12cdc4657b 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -53,11 +53,6 @@ .issue-sidebar-combo .ui.dropdown .item:not(.checked) .item-check-mark { visibility: hidden; } -/* ideally, we should move these styles to ".ui.dropdown .menu.flex-items-menu > .item ...", could be done later */ -.issue-sidebar-combo .ui.dropdown .menu > .item > img, -.issue-sidebar-combo .ui.dropdown .menu > .item > svg { - margin: 0; -} .issue-content-right .dropdown > .menu { max-width: 270px; @@ -66,6 +61,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 { @@ -73,11 +74,6 @@ } } -.repository .issue-content-right .filter.menu { - max-height: 500px; - overflow-x: auto; -} - .repository .filter.menu.labels .label-filter .menu .info { display: inline-block; padding: 0.5rem 0; 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 }}