From 8995046110147ae2c8c98be4e0a8c0b643ccc29c Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 7 Jul 2023 07:31:56 +0200 Subject: [PATCH] Less naked returns (#25713) just a step towards #25655 and some related refactoring --- models/asymkey/gpg_key_commit_verification.go | 9 ++- models/auth/oauth2.go | 7 +- models/migrations/v1_11/v106.go | 8 +- models/migrations/v1_13/v143.go | 16 ++-- models/migrations/v1_15/v180.go | 18 ++--- models/migrations/v1_15/v181.go | 31 ++++---- modules/context/repo.go | 74 +++++++++---------- modules/doctor/fix16961.go | 59 +++++++-------- modules/eventsource/event.go | 10 +-- modules/httpcache/httpcache.go | 6 +- modules/httplib/serve.go | 7 +- modules/httplib/serve_test.go | 3 +- modules/process/manager.go | 2 +- modules/util/util.go | 5 ++ modules/util/util_test.go | 9 +++ routers/api/v1/repo/file.go | 10 +-- routers/api/v1/repo/issue_label.go | 12 +-- routers/common/serve.go | 4 +- routers/web/auth/auth.go | 16 ++-- routers/web/repo/attachment.go | 3 +- routers/web/repo/download.go | 16 ++-- routers/web/repo/http.go | 54 +++++++------- routers/web/repo/issue.go | 9 ++- routers/web/repo/wiki.go | 3 +- routers/web/web.go | 10 +-- services/forms/user_form_hidden_comments.go | 2 +- services/gitdiff/gitdiff.go | 25 +++---- services/issue/assignee.go | 19 +++-- services/issue/issue.go | 10 +-- services/issue/label.go | 6 +- services/pull/review.go | 20 ++--- services/release/release.go | 10 +-- 32 files changed, 254 insertions(+), 239 deletions(-) diff --git a/models/asymkey/gpg_key_commit_verification.go b/models/asymkey/gpg_key_commit_verification.go index db6e78cad..65af0bc94 100644 --- a/models/asymkey/gpg_key_commit_verification.go +++ b/models/asymkey/gpg_key_commit_verification.go @@ -455,9 +455,9 @@ func hashAndVerifyForKeyID(sig *packet.Signature, payload string, committer *use // CalculateTrustStatus will calculate the TrustStatus for a commit verification within a repository // There are several trust models in Gitea -func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error), keyMap *map[string]bool) (err error) { +func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_model.TrustModelType, isOwnerMemberCollaborator func(*user_model.User) (bool, error), keyMap *map[string]bool) error { if !verification.Verified { - return + return nil } // In the Committer trust model a signature is trusted if it matches the committer @@ -475,7 +475,7 @@ func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_ verification.SigningUser.Email == verification.CommittingUser.Email) { verification.TrustStatus = "trusted" } - return + return nil } // Now we drop to the more nuanced trust models... @@ -490,10 +490,11 @@ func CalculateTrustStatus(verification *CommitVerification, repoTrustModel repo_ verification.SigningUser.Email != verification.CommittingUser.Email) { verification.TrustStatus = "untrusted" } - return + return nil } // Check we actually have a GPG SigningKey + var err error if verification.SigningKey != nil { var isMember bool if keyMap != nil { diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 53a5c28b4..0f64b56c1 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -306,9 +306,10 @@ func (code *OAuth2AuthorizationCode) TableName() string { } // GenerateRedirectURI generates a redirect URI for a successful authorization request. State will be used if not empty. -func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (redirect *url.URL, err error) { - if redirect, err = url.Parse(code.RedirectURI); err != nil { - return +func (code *OAuth2AuthorizationCode) GenerateRedirectURI(state string) (*url.URL, error) { + redirect, err := url.Parse(code.RedirectURI) + if err != nil { + return nil, err } q := redirect.Query() if state != "" { diff --git a/models/migrations/v1_11/v106.go b/models/migrations/v1_11/v106.go index 3e06309a8..18d436ae2 100644 --- a/models/migrations/v1_11/v106.go +++ b/models/migrations/v1_11/v106.go @@ -16,10 +16,10 @@ type Watch struct { Mode RepoWatchMode `xorm:"SMALLINT NOT NULL DEFAULT 1"` } -func AddModeColumnToWatch(x *xorm.Engine) (err error) { - if err = x.Sync2(new(Watch)); err != nil { - return +func AddModeColumnToWatch(x *xorm.Engine) error { + if err := x.Sync2(new(Watch)); err != nil { + return err } - _, err = x.Exec("UPDATE `watch` SET `mode` = 1") + _, err := x.Exec("UPDATE `watch` SET `mode` = 1") return err } diff --git a/models/migrations/v1_13/v143.go b/models/migrations/v1_13/v143.go index ad1a8c66a..885768dff 100644 --- a/models/migrations/v1_13/v143.go +++ b/models/migrations/v1_13/v143.go @@ -23,25 +23,25 @@ func RecalculateStars(x *xorm.Engine) (err error) { for start := 0; ; start += batchSize { users := make([]User, 0, batchSize) - if err = sess.Limit(batchSize, start).Where("type = ?", 0).Cols("id").Find(&users); err != nil { - return + if err := sess.Limit(batchSize, start).Where("type = ?", 0).Cols("id").Find(&users); err != nil { + return err } if len(users) == 0 { break } - if err = sess.Begin(); err != nil { - return + if err := sess.Begin(); err != nil { + return err } for _, user := range users { - if _, err = sess.Exec("UPDATE `user` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE uid=?) WHERE id=?", user.ID, user.ID); err != nil { - return + if _, err := sess.Exec("UPDATE `user` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE uid=?) WHERE id=?", user.ID, user.ID); err != nil { + return err } } - if err = sess.Commit(); err != nil { - return + if err := sess.Commit(); err != nil { + return err } } diff --git a/models/migrations/v1_15/v180.go b/models/migrations/v1_15/v180.go index 17163ee2c..c71e77186 100644 --- a/models/migrations/v1_15/v180.go +++ b/models/migrations/v1_15/v180.go @@ -44,25 +44,25 @@ func DeleteMigrationCredentials(x *xorm.Engine) (err error) { for start := 0; ; start += batchSize { tasks := make([]*Task, 0, batchSize) - if err = sess.Limit(batchSize, start).Where(cond, 0).Find(&tasks); err != nil { - return + if err := sess.Limit(batchSize, start).Where(cond, 0).Find(&tasks); err != nil { + return err } if len(tasks) == 0 { break } - if err = sess.Begin(); err != nil { - return + if err := sess.Begin(); err != nil { + return err } for _, t := range tasks { if t.PayloadContent, err = removeCredentials(t.PayloadContent); err != nil { - return + return err } - if _, err = sess.ID(t.ID).Cols("payload_content").Update(t); err != nil { - return + if _, err := sess.ID(t.ID).Cols("payload_content").Update(t); err != nil { + return err } } - if err = sess.Commit(); err != nil { - return + if err := sess.Commit(); err != nil { + return err } } return err diff --git a/models/migrations/v1_15/v181.go b/models/migrations/v1_15/v181.go index e2bb3208c..f4dd1d601 100644 --- a/models/migrations/v1_15/v181.go +++ b/models/migrations/v1_15/v181.go @@ -9,7 +9,7 @@ import ( "xorm.io/xorm" ) -func AddPrimaryEmail2EmailAddress(x *xorm.Engine) (err error) { +func AddPrimaryEmail2EmailAddress(x *xorm.Engine) error { type User struct { ID int64 `xorm:"pk autoincr"` Email string `xorm:"NOT NULL"` @@ -26,12 +26,12 @@ func AddPrimaryEmail2EmailAddress(x *xorm.Engine) (err error) { } // Add lower_email and is_primary columns - if err = x.Table("email_address").Sync2(new(EmailAddress1)); err != nil { - return + if err := x.Table("email_address").Sync2(new(EmailAddress1)); err != nil { + return err } - if _, err = x.Exec("UPDATE email_address SET lower_email=LOWER(email), is_primary=?", false); err != nil { - return + if _, err := x.Exec("UPDATE email_address SET lower_email=LOWER(email), is_primary=?", false); err != nil { + return err } type EmailAddress struct { @@ -44,8 +44,8 @@ func AddPrimaryEmail2EmailAddress(x *xorm.Engine) (err error) { } // change lower_email as unique - if err = x.Sync2(new(EmailAddress)); err != nil { - return + if err := x.Sync2(new(EmailAddress)); err != nil { + return err } sess := x.NewSession() @@ -55,34 +55,33 @@ func AddPrimaryEmail2EmailAddress(x *xorm.Engine) (err error) { for start := 0; ; start += batchSize { users := make([]*User, 0, batchSize) - if err = sess.Limit(batchSize, start).Find(&users); err != nil { - return + if err := sess.Limit(batchSize, start).Find(&users); err != nil { + return err } if len(users) == 0 { break } for _, user := range users { - var exist bool - exist, err = sess.Where("email=?", user.Email).Table("email_address").Exist() + exist, err := sess.Where("email=?", user.Email).Table("email_address").Exist() if err != nil { - return + return err } if !exist { - if _, err = sess.Insert(&EmailAddress{ + if _, err := sess.Insert(&EmailAddress{ UID: user.ID, Email: user.Email, LowerEmail: strings.ToLower(user.Email), IsActivated: user.IsActive, IsPrimary: true, }); err != nil { - return + return err } } else { - if _, err = sess.Where("email=?", user.Email).Cols("is_primary").Update(&EmailAddress{ + if _, err := sess.Where("email=?", user.Email).Cols("is_primary").Update(&EmailAddress{ IsPrimary: true, }); err != nil { - return + return err } } } diff --git a/modules/context/repo.go b/modules/context/repo.go index e99908525..eae71cfb7 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -422,10 +422,10 @@ func RepoIDAssignment() func(ctx *Context) { } // RepoAssignment returns a middleware to handle repository assignment -func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { +func RepoAssignment(ctx *Context) context.CancelFunc { if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce { log.Trace("RepoAssignment was exec already, skipping second call ...") - return + return nil } ctx.Data["repoAssignmentExecuted"] = true @@ -453,7 +453,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { // https://github.com/golang/go/issues/19760 if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) - return + return nil } if redirectUserID, err := user_model.LookupUserRedirect(userName); err == nil { @@ -466,7 +466,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { } else { ctx.ServerError("GetUserByName", err) } - return + return nil } } ctx.Repo.Owner = owner @@ -490,7 +490,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { redirectPath += "?" + ctx.Req.URL.RawQuery } ctx.Redirect(path.Join(setting.AppSubURL, redirectPath)) - return + return nil } // Get repository. @@ -503,7 +503,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { } else if repo_model.IsErrRedirectNotExist(err) { if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) - return + return nil } ctx.NotFound("GetRepositoryByName", nil) } else { @@ -512,13 +512,13 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { } else { ctx.ServerError("GetRepositoryByName", err) } - return + return nil } repo.Owner = owner repoAssignment(ctx, repo) if ctx.Written() { - return + return nil } ctx.Repo.RepoLink = repo.Link() @@ -542,12 +542,12 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { }) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) - return + return nil } ctx.Data["NumReleases"], err = repo_model.GetReleaseCountByRepoID(ctx, ctx.Repo.Repository.ID, repo_model.FindReleasesOptions{}) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) - return + return nil } ctx.Data["Title"] = owner.Name + "/" + repo.Name @@ -563,14 +563,14 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { canSignedUserFork, err := repo_module.CanUserForkRepo(ctx.Doer, ctx.Repo.Repository) if err != nil { ctx.ServerError("CanUserForkRepo", err) - return + return nil } ctx.Data["CanSignedUserFork"] = canSignedUserFork userAndOrgForks, err := repo_model.GetForksByUserAndOrgs(ctx, ctx.Doer, ctx.Repo.Repository) if err != nil { ctx.ServerError("GetForksByUserAndOrgs", err) - return + return nil } ctx.Data["UserAndOrgForks"] = userAndOrgForks @@ -604,14 +604,14 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { if repo.IsFork { RetrieveBaseRepo(ctx, repo) if ctx.Written() { - return + return nil } } if repo.IsGenerated() { RetrieveTemplateRepo(ctx, repo) if ctx.Written() { - return + return nil } } @@ -623,7 +623,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { if !isHomeOrSettings { ctx.Redirect(ctx.Repo.RepoLink) } - return + return nil } gitRepo, err := git.OpenRepository(ctx, repo_model.RepoPath(userName, repoName)) @@ -636,10 +636,10 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { if !isHomeOrSettings { ctx.Redirect(ctx.Repo.RepoLink) } - return + return nil } ctx.ServerError("RepoAssignment Invalid repo "+repo_model.RepoPath(userName, repoName), err) - return + return nil } if ctx.Repo.GitRepo != nil { ctx.Repo.GitRepo.Close() @@ -647,7 +647,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { ctx.Repo.GitRepo = gitRepo // We opened it, we should close it - cancel = func() { + cancel := func() { // If it's been set to nil then assume someone else has closed it. if ctx.Repo.GitRepo != nil { ctx.Repo.GitRepo.Close() @@ -657,13 +657,13 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { // Stop at this point when the repo is empty. if ctx.Repo.Repository.IsEmpty { ctx.Data["BranchName"] = ctx.Repo.Repository.DefaultBranch - return + return cancel } tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) if err != nil { ctx.ServerError("GetTagNamesByRepoID", err) - return + return cancel } ctx.Data["Tags"] = tags @@ -677,7 +677,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { branchesTotal, err := git_model.CountBranches(ctx, branchOpts) if err != nil { ctx.ServerError("CountBranches", err) - return + return cancel } // non empty repo should have at least 1 branch, so this repository's branches haven't been synced yet @@ -685,7 +685,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { branchesTotal, err = repo_module.SyncRepoBranches(ctx, ctx.Repo.Repository.ID, 0) if err != nil { ctx.ServerError("SyncRepoBranches", err) - return + return cancel } } @@ -694,7 +694,7 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { brs, err := git_model.FindBranchNames(ctx, branchOpts) if err != nil { ctx.ServerError("GetBranches", err) - return + return cancel } // always put default branch on the top ctx.Data["Branches"] = append(branchOpts.ExcludeBranchNames, brs...) @@ -741,12 +741,12 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { repoTransfer, err := models.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository) if err != nil { ctx.ServerError("GetPendingRepositoryTransfer", err) - return + return cancel } if err := repoTransfer.LoadAttributes(ctx); err != nil { ctx.ServerError("LoadRecipient", err) - return + return cancel } ctx.Data["RepoTransfer"] = repoTransfer @@ -894,7 +894,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Repo.IsViewBranch = true ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch ctx.Data["TreePath"] = "" - return + return nil } var ( @@ -907,7 +907,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Repo.GitRepo, err = git.OpenRepository(ctx, repoPath) if err != nil { ctx.ServerError("RepoRef Invalid repo "+repoPath, err) - return + return nil } // We opened it, we should close it cancel = func() { @@ -944,7 +944,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Repo.Repository.MarkAsBrokenEmpty() } else { ctx.ServerError("GetBranchCommit", err) - return + return cancel } ctx.Repo.IsViewBranch = true } else { @@ -956,7 +956,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Flash.Info(ctx.Tr("repo.branch.renamed", refName, renamedBranchName)) link := setting.AppSubURL + strings.Replace(ctx.Req.URL.EscapedPath(), util.PathEscapeSegments(refName), util.PathEscapeSegments(renamedBranchName), 1) ctx.Redirect(link) - return + return cancel } if refType.RefTypeIncludesBranches() && ctx.Repo.GitRepo.IsBranchExist(refName) { @@ -966,7 +966,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) if err != nil { ctx.ServerError("GetBranchCommit", err) - return + return cancel } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() @@ -978,10 +978,10 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context if err != nil { if git.IsErrNotExist(err) { ctx.NotFound("GetTagCommit", err) - return + return cancel } ctx.ServerError("GetTagCommit", err) - return + return cancel } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() } else if len(refName) >= 7 && len(refName) <= git.SHAFullLength { @@ -991,7 +991,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) if err != nil { ctx.NotFound("GetCommit", err) - return + return cancel } // If short commit ID add canonical link header if len(refName) < git.SHAFullLength { @@ -1000,10 +1000,10 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context } } else { if len(ignoreNotExistErr) > 0 && ignoreNotExistErr[0] { - return + return cancel } ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName)) - return + return cancel } if refType == RepoRefLegacy { @@ -1015,7 +1015,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context util.PathEscapeSegments(prefix), ctx.Repo.BranchNameSubURL(), util.PathEscapeSegments(ctx.Repo.TreePath))) - return + return cancel } } @@ -1033,7 +1033,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context ctx.Repo.CommitsCount, err = ctx.Repo.GetCommitsCount() if err != nil { ctx.ServerError("GetCommitsCount", err) - return + return cancel } ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount ctx.Repo.GitRepo.LastCommitCache = git.NewLastCommitCache(ctx.Repo.CommitsCount, ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, cache.GetCache()) diff --git a/modules/doctor/fix16961.go b/modules/doctor/fix16961.go index ea14a9b2c..562c78dd7 100644 --- a/modules/doctor/fix16961.go +++ b/modules/doctor/fix16961.go @@ -6,6 +6,7 @@ package doctor import ( "bytes" "context" + "errors" "fmt" "code.gitea.io/gitea/models/db" @@ -40,12 +41,12 @@ func parseBool16961(bs []byte) (bool, error) { func fixUnitConfig16961(bs []byte, cfg *repo_model.UnitConfig) (fixed bool, err error) { err = json.UnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { - return + return false, nil } // Handle #16961 if string(bs) != "&{}" && len(bs) != 0 { - return + return false, err } return true, nil @@ -54,14 +55,14 @@ func fixUnitConfig16961(bs []byte, cfg *repo_model.UnitConfig) (fixed bool, err func fixExternalWikiConfig16961(bs []byte, cfg *repo_model.ExternalWikiConfig) (fixed bool, err error) { err = json.UnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { - return + return false, nil } if len(bs) < 3 { - return + return false, err } if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return + return false, err } cfg.ExternalWikiURL = string(bs[2 : len(bs)-1]) return true, nil @@ -70,20 +71,20 @@ func fixExternalWikiConfig16961(bs []byte, cfg *repo_model.ExternalWikiConfig) ( func fixExternalTrackerConfig16961(bs []byte, cfg *repo_model.ExternalTrackerConfig) (fixed bool, err error) { err = json.UnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { - return + return false, nil } // Handle #16961 if len(bs) < 3 { - return + return false, err } if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return + return false, err } parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) if len(parts) != 3 { - return + return false, err } cfg.ExternalTrackerURL = string(bytes.Join(parts[:len(parts)-2], []byte{' '})) @@ -95,16 +96,16 @@ func fixExternalTrackerConfig16961(bs []byte, cfg *repo_model.ExternalTrackerCon func fixPullRequestsConfig16961(bs []byte, cfg *repo_model.PullRequestsConfig) (fixed bool, err error) { err = json.UnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { - return + return false, nil } // Handle #16961 if len(bs) < 3 { - return + return false, err } if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return + return false, err } // PullRequestsConfig was the following in 1.14 @@ -123,37 +124,37 @@ func fixPullRequestsConfig16961(bs []byte, cfg *repo_model.PullRequestsConfig) ( // DefaultMergeStyle MergeStyle parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) if len(parts) < 7 { - return + return false, err } var parseErr error cfg.IgnoreWhitespaceConflicts, parseErr = parseBool16961(parts[0]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AllowMerge, parseErr = parseBool16961(parts[1]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AllowRebase, parseErr = parseBool16961(parts[2]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AllowRebaseMerge, parseErr = parseBool16961(parts[3]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AllowSquash, parseErr = parseBool16961(parts[4]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AllowManualMerge, parseErr = parseBool16961(parts[5]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AutodetectManualMerge, parseErr = parseBool16961(parts[6]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } // 1.14 unit @@ -162,12 +163,12 @@ func fixPullRequestsConfig16961(bs []byte, cfg *repo_model.PullRequestsConfig) ( } if len(parts) < 9 { - return + return false, err } cfg.DefaultDeleteBranchAfterMerge, parseErr = parseBool16961(parts[7]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.DefaultMergeStyle = repo_model.MergeStyle(string(bytes.Join(parts[8:], []byte{' '}))) @@ -177,34 +178,34 @@ func fixPullRequestsConfig16961(bs []byte, cfg *repo_model.PullRequestsConfig) ( func fixIssuesConfig16961(bs []byte, cfg *repo_model.IssuesConfig) (fixed bool, err error) { err = json.UnmarshalHandleDoubleEncode(bs, &cfg) if err == nil { - return + return false, nil } // Handle #16961 if len(bs) < 3 { - return + return false, err } if bs[0] != '&' || bs[1] != '{' || bs[len(bs)-1] != '}' { - return + return false, err } parts := bytes.Split(bs[2:len(bs)-1], []byte{' '}) if len(parts) != 3 { - return + return false, err } var parseErr error cfg.EnableTimetracker, parseErr = parseBool16961(parts[0]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.AllowOnlyContributorsToTrackTime, parseErr = parseBool16961(parts[1]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } cfg.EnableDependencies, parseErr = parseBool16961(parts[2]) if parseErr != nil { - return + return false, errors.Join(err, parseErr) } return true, nil } diff --git a/modules/eventsource/event.go b/modules/eventsource/event.go index 811f97ff5..ebcca5090 100644 --- a/modules/eventsource/event.go +++ b/modules/eventsource/event.go @@ -15,7 +15,7 @@ import ( func wrapNewlines(w io.Writer, prefix, value []byte) (sum int64, err error) { if len(value) == 0 { - return + return 0, nil } var n int last := 0 @@ -23,24 +23,24 @@ func wrapNewlines(w io.Writer, prefix, value []byte) (sum int64, err error) { n, err = w.Write(prefix) sum += int64(n) if err != nil { - return + return sum, err } n, err = w.Write(value[last : last+j+1]) sum += int64(n) if err != nil { - return + return sum, err } last += j + 1 } n, err = w.Write(prefix) sum += int64(n) if err != nil { - return + return sum, err } n, err = w.Write(value[last:]) sum += int64(n) if err != nil { - return + return sum, err } n, err = w.Write([]byte("\n")) sum += int64(n) diff --git a/modules/httpcache/httpcache.go b/modules/httpcache/httpcache.go index 001ac0641..b57b32183 100644 --- a/modules/httpcache/httpcache.go +++ b/modules/httpcache/httpcache.go @@ -70,11 +70,11 @@ func checkIfNoneMatchIsValid(req *http.Request, etag string) bool { // HandleGenericETagTimeCache handles ETag-based caching with Last-Modified caching for a HTTP request. // It returns true if the request was handled. -func HandleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag string, lastModified time.Time) (handled bool) { +func HandleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag string, lastModified *time.Time) (handled bool) { if len(etag) > 0 { w.Header().Set("Etag", etag) } - if !lastModified.IsZero() { + if lastModified != nil && !lastModified.IsZero() { w.Header().Set("Last-Modified", lastModified.Format(http.TimeFormat)) } @@ -84,7 +84,7 @@ func HandleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag s return true } } - if !lastModified.IsZero() { + if lastModified != nil && !lastModified.IsZero() { ifModifiedSince := req.Header.Get("If-Modified-Since") if ifModifiedSince != "" { t, err := time.Parse(http.TimeFormat, ifModifiedSince) diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index 12d68c2d6..a193ed901 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -206,7 +206,7 @@ func ServeContentByReader(r *http.Request, w http.ResponseWriter, filePath strin _, _ = io.CopyN(w, reader, partialLength) // just like http.ServeContent, not necessary to handle the error } -func ServeContentByReadSeeker(r *http.Request, w http.ResponseWriter, filePath string, modTime time.Time, reader io.ReadSeeker) { +func ServeContentByReadSeeker(r *http.Request, w http.ResponseWriter, filePath string, modTime *time.Time, reader io.ReadSeeker) { buf := make([]byte, mimeDetectionBufferLen) n, err := util.ReadAtMost(reader, buf) if err != nil { @@ -221,5 +221,8 @@ func ServeContentByReadSeeker(r *http.Request, w http.ResponseWriter, filePath s buf = buf[:n] } setServeHeadersByFile(r, w, filePath, buf) - http.ServeContent(w, r, path.Base(filePath), modTime, reader) + if modTime == nil { + modTime = &time.Time{} + } + http.ServeContent(w, r, path.Base(filePath), *modTime, reader) } diff --git a/modules/httplib/serve_test.go b/modules/httplib/serve_test.go index fed4611d2..c2229dffe 100644 --- a/modules/httplib/serve_test.go +++ b/modules/httplib/serve_test.go @@ -11,7 +11,6 @@ import ( "os" "strings" "testing" - "time" "github.com/stretchr/testify/assert" ) @@ -78,7 +77,7 @@ func TestServeContentByReadSeeker(t *testing.T) { defer seekReader.Close() w := httptest.NewRecorder() - ServeContentByReadSeeker(r, w, "test", time.Time{}, seekReader) + ServeContentByReadSeeker(r, w, "test", nil, seekReader) assert.Equal(t, expectedStatusCode, w.Code) if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK { assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length")) diff --git a/modules/process/manager.go b/modules/process/manager.go index 56908c040..9c21f6215 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -211,7 +211,7 @@ func (pm *Manager) nextPID() (start time.Time, pid IDType) { pid = IDType(strconv.FormatInt(start.Unix(), 16)) if pm.next == 1 { - return + return start, pid } pid = IDType(string(pid) + "-" + strconv.FormatInt(pm.next, 10)) return start, pid diff --git a/modules/util/util.go b/modules/util/util.go index 305144954..9d5e6c1e8 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -256,3 +256,8 @@ func ToFloat64(number any) (float64, error) { } return value, nil } + +// ToPointer returns the pointer of a copy of any given value +func ToPointer[T any](val T) *T { + return &val +} diff --git a/modules/util/util_test.go b/modules/util/util_test.go index 8cceafa2f..c5830ce01 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -224,3 +224,12 @@ func TestToTitleCase(t *testing.T) { assert.Equal(t, ToTitleCase(`foo bar baz`), `Foo Bar Baz`) assert.Equal(t, ToTitleCase(`FOO BAR BAZ`), `Foo Bar Baz`) } + +func TestToPointer(t *testing.T) { + assert.Equal(t, "abc", *ToPointer("abc")) + assert.Equal(t, 123, *ToPointer(123)) + abc := "abc" + assert.False(t, &abc == ToPointer(abc)) + val123 := 123 + assert.False(t, &val123 == ToPointer(val123)) +} diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 48f890ee5..bf37532fc 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -219,7 +219,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { common.ServeContentByReadSeeker(ctx.Base, ctx.Repo.TreePath, lastModified, lfsDataRc) } -func getBlobForEntry(ctx *context.APIContext) (blob *git.Blob, entry *git.TreeEntry, lastModified time.Time) { +func getBlobForEntry(ctx *context.APIContext) (blob *git.Blob, entry *git.TreeEntry, lastModified *time.Time) { entry, err := ctx.Repo.Commit.GetTreeEntryByPath(ctx.Repo.TreePath) if err != nil { if git.IsErrNotExist(err) { @@ -227,23 +227,23 @@ func getBlobForEntry(ctx *context.APIContext) (blob *git.Blob, entry *git.TreeEn } else { ctx.Error(http.StatusInternalServerError, "GetTreeEntryByPath", err) } - return + return nil, nil, nil } if entry.IsDir() || entry.IsSubModule() { ctx.NotFound("getBlobForEntry", nil) - return + return nil, nil, nil } info, _, err := git.Entries([]*git.TreeEntry{entry}).GetCommitsInfo(ctx, ctx.Repo.Commit, path.Dir("/" + ctx.Repo.TreePath)[1:]) if err != nil { ctx.Error(http.StatusInternalServerError, "GetCommitsInfo", err) - return + return nil, nil, nil } if len(info) == 1 { // Not Modified - lastModified = info[0].Commit.Committer.When + lastModified = &info[0].Commit.Committer.When } blob = entry.Blob() diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 1f0d21141..9d43c66d8 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -298,26 +298,26 @@ func ClearIssueLabels(ctx *context.APIContext) { ctx.Status(http.StatusNoContent) } -func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) (issue *issues_model.Issue, labels []*issues_model.Label, err error) { - issue, err = issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) +func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) (*issues_model.Issue, []*issues_model.Label, error) { + issue, err := issues_model.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if issues_model.IsErrIssueNotExist(err) { ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } - return + return nil, nil, err } - labels, err = issues_model.GetLabelsByIDs(form.Labels) + labels, err := issues_model.GetLabelsByIDs(form.Labels) if err != nil { ctx.Error(http.StatusInternalServerError, "GetLabelsByIDs", err) - return + return nil, nil, err } if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Status(http.StatusForbidden) - return + return nil, nil, nil } return issue, labels, err diff --git a/routers/common/serve.go b/routers/common/serve.go index 3094ee6a6..8a7f8b333 100644 --- a/routers/common/serve.go +++ b/routers/common/serve.go @@ -15,7 +15,7 @@ import ( ) // ServeBlob download a git.Blob -func ServeBlob(ctx *context.Base, filePath string, blob *git.Blob, lastModified time.Time) error { +func ServeBlob(ctx *context.Base, filePath string, blob *git.Blob, lastModified *time.Time) error { if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { return nil } @@ -38,6 +38,6 @@ func ServeContentByReader(ctx *context.Base, filePath string, size int64, reader httplib.ServeContentByReader(ctx.Req, ctx.Resp, filePath, size, reader) } -func ServeContentByReadSeeker(ctx *context.Base, filePath string, modTime time.Time, reader io.ReadSeeker) { +func ServeContentByReadSeeker(ctx *context.Base, filePath string, modTime *time.Time, reader io.ReadSeeker) { httplib.ServeContentByReadSeeker(ctx.Req, ctx.Resp, filePath, modTime, reader) } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 9f1395225..3bf133f56 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -497,23 +497,23 @@ func createUserInContext(ctx *context.Context, tpl base.TplName, form any, u *us hasUser, err = user_model.GetUser(user) if !hasUser || err != nil { ctx.ServerError("UserLinkAccount", err) - return + return false } } // TODO: probably we should respect 'remember' user's choice... linkAccount(ctx, user, *gothUser, true) - return // user is already created here, all redirects are handled + return false // user is already created here, all redirects are handled } else if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingLogin { showLinkingLogin(ctx, *gothUser) - return // user will be created only after linking login + return false // user will be created only after linking login } } // handle error without template if len(tpl) == 0 { ctx.ServerError("CreateUser", err) - return + return false } // handle error with template @@ -542,7 +542,7 @@ func createUserInContext(ctx *context.Context, tpl base.TplName, form any, u *us default: ctx.ServerError("CreateUser", err) } - return + return false } log.Trace("Account created: %s", u.Name) return true @@ -559,7 +559,7 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. u.SetLastLogin() if err := user_model.UpdateUserCols(ctx, u, "is_admin", "is_active", "last_login_unix"); err != nil { ctx.ServerError("UpdateUser", err) - return + return false } } @@ -577,7 +577,7 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. if setting.Service.RegisterManualConfirm { ctx.Data["ManualActivationOnly"] = true ctx.HTML(http.StatusOK, TplActivate) - return + return false } mailer.SendActivateAccountMail(ctx.Locale, u) @@ -592,7 +592,7 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. log.Error("Set cache(MailResendLimit) fail: %v", err) } } - return + return false } return true diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index af1842ad1..b7be77914 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/upload" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/attachment" repo_service "code.gitea.io/gitea/services/repository" @@ -148,7 +149,7 @@ func ServeAttachment(ctx *context.Context, uuid string) { } defer fr.Close() - common.ServeContentByReadSeeker(ctx.Base, attach.Name, attach.CreatedUnix.AsTime(), fr) + common.ServeContentByReadSeeker(ctx.Base, attach.Name, util.ToPointer(attach.CreatedUnix.AsTime()), fr) } // GetAttachment serve attachments diff --git a/routers/web/repo/download.go b/routers/web/repo/download.go index fd67b82ef..a9e2e2b2f 100644 --- a/routers/web/repo/download.go +++ b/routers/web/repo/download.go @@ -20,7 +20,7 @@ import ( ) // ServeBlobOrLFS download a git.Blob redirecting to LFS if necessary -func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified time.Time) error { +func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Time) error { if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { return nil } @@ -82,7 +82,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified time.Time return common.ServeBlob(ctx.Base, ctx.Repo.TreePath, blob, lastModified) } -func getBlobForEntry(ctx *context.Context) (blob *git.Blob, lastModified time.Time) { +func getBlobForEntry(ctx *context.Context) (blob *git.Blob, lastModified *time.Time) { entry, err := ctx.Repo.Commit.GetTreeEntryByPath(ctx.Repo.TreePath) if err != nil { if git.IsErrNotExist(err) { @@ -90,23 +90,23 @@ func getBlobForEntry(ctx *context.Context) (blob *git.Blob, lastModified time.Ti } else { ctx.ServerError("GetTreeEntryByPath", err) } - return + return nil, nil } if entry.IsDir() || entry.IsSubModule() { ctx.NotFound("getBlobForEntry", nil) - return + return nil, nil } info, _, err := git.Entries([]*git.TreeEntry{entry}).GetCommitsInfo(ctx, ctx.Repo.Commit, path.Dir("/" + ctx.Repo.TreePath)[1:]) if err != nil { ctx.ServerError("GetCommitsInfo", err) - return + return nil, nil } if len(info) == 1 { // Not Modified - lastModified = info[0].Commit.Committer.When + lastModified = &info[0].Commit.Committer.When } blob = entry.Blob() @@ -148,7 +148,7 @@ func DownloadByID(ctx *context.Context) { } return } - if err = common.ServeBlob(ctx.Base, ctx.Repo.TreePath, blob, time.Time{}); err != nil { + if err = common.ServeBlob(ctx.Base, ctx.Repo.TreePath, blob, nil); err != nil { ctx.ServerError("ServeBlob", err) } } @@ -164,7 +164,7 @@ func DownloadByIDOrLFS(ctx *context.Context) { } return } - if err = ServeBlobOrLFS(ctx, blob, time.Time{}); err != nil { + if err = ServeBlobOrLFS(ctx, blob, nil); err != nil { ctx.ServerError("ServeBlob", err) } } diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index f4e9ac86a..0cae9aeda 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -56,13 +56,13 @@ func CorsHandler() func(next http.Handler) http.Handler { } // httpBase implementation git smart HTTP protocol -func httpBase(ctx *context.Context) (h *serviceHandler) { +func httpBase(ctx *context.Context) *serviceHandler { username := ctx.Params(":username") reponame := strings.TrimSuffix(ctx.Params(":reponame"), ".git") if ctx.FormString("go-get") == "1" { context.EarlyResponseForGoGetMeta(ctx) - return + return nil } var isPull, receivePack bool @@ -101,7 +101,7 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { owner := ctx.ContextUser if !owner.IsOrganization() && !owner.IsActive { ctx.PlainText(http.StatusForbidden, "Repository cannot be accessed. You cannot push or open issues/pull-requests.") - return + return nil } repoExist := true @@ -110,19 +110,19 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { if repo_model.IsErrRepoNotExist(err) { if redirectRepoID, err := repo_model.LookupRedirect(owner.ID, reponame); err == nil { context.RedirectToRepo(ctx.Base, redirectRepoID) - return + return nil } repoExist = false } else { ctx.ServerError("GetRepositoryByName", err) - return + return nil } } // Don't allow pushing if the repo is archived if repoExist && repo.IsArchived && !isPull { ctx.PlainText(http.StatusForbidden, "This repo is archived. You can view files and clone it, but cannot push or open issues/pull-requests.") - return + return nil } // Only public pull don't need auth. @@ -136,7 +136,7 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { if isPublicPull { if err := repo.LoadOwner(ctx); err != nil { ctx.ServerError("LoadOwner", err) - return + return nil } askAuth = askAuth || (repo.Owner.Visibility != structs.VisibleTypePublic) @@ -149,12 +149,12 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { // TODO: support digit auth - which would be Authorization header with digit ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"") ctx.Error(http.StatusUnauthorized) - return + return nil } context.CheckRepoScopedToken(ctx, repo, auth_model.GetScopeLevelFromAccessMode(accessMode)) if ctx.Written() { - return + return nil } if ctx.IsBasicAuth && ctx.Data["IsApiToken"] != true && ctx.Data["IsActionsToken"] != true { @@ -162,16 +162,16 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { if err == nil { // TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented ctx.PlainText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page") - return + return nil } else if !auth_model.IsErrTwoFactorNotEnrolled(err) { ctx.ServerError("IsErrTwoFactorNotEnrolled", err) - return + return nil } } if !ctx.Doer.IsActive || ctx.Doer.ProhibitLogin { ctx.PlainText(http.StatusForbidden, "Your account is disabled.") - return + return nil } environ = []string{ @@ -193,23 +193,23 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { task, err := actions_model.GetTaskByID(ctx, taskID) if err != nil { ctx.ServerError("GetTaskByID", err) - return + return nil } if task.RepoID != repo.ID { ctx.PlainText(http.StatusForbidden, "User permission denied") - return + return nil } if task.IsForkPullRequest { if accessMode > perm.AccessModeRead { ctx.PlainText(http.StatusForbidden, "User permission denied") - return + return nil } environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeRead)) } else { if accessMode > perm.AccessModeWrite { ctx.PlainText(http.StatusForbidden, "User permission denied") - return + return nil } environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeWrite)) } @@ -217,18 +217,18 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { p, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return + return nil } if !p.CanAccess(accessMode, unitType) { ctx.PlainText(http.StatusNotFound, "Repository not found") - return + return nil } } if !isPull && repo.IsMirror { ctx.PlainText(http.StatusForbidden, "mirror repository is read-only") - return + return nil } } @@ -246,34 +246,34 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { if !repoExist { if !receivePack { ctx.PlainText(http.StatusNotFound, "Repository not found") - return + return nil } if isWiki { // you cannot send wiki operation before create the repository ctx.PlainText(http.StatusNotFound, "Repository not found") - return + return nil } if owner.IsOrganization() && !setting.Repository.EnablePushCreateOrg { ctx.PlainText(http.StatusForbidden, "Push to create is not enabled for organizations.") - return + return nil } if !owner.IsOrganization() && !setting.Repository.EnablePushCreateUser { ctx.PlainText(http.StatusForbidden, "Push to create is not enabled for users.") - return + return nil } // Return dummy payload if GET receive-pack if ctx.Req.Method == http.MethodGet { dummyInfoRefs(ctx) - return + return nil } repo, err = repo_service.PushCreateRepo(ctx, ctx.Doer, owner, reponame) if err != nil { log.Error("pushCreateRepo: %v", err) ctx.Status(http.StatusNotFound) - return + return nil } } @@ -282,11 +282,11 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { if _, err := repo.GetUnit(ctx, unit.TypeWiki); err != nil { if repo_model.IsErrUnitTypeNotExist(err) { ctx.PlainText(http.StatusForbidden, "repository wiki is disabled") - return + return nil } log.Error("Failed to get the wiki unit in %-v Error: %v", repo, err) ctx.ServerError("GetUnit(UnitTypeWiki) for "+repo.FullName(), err) - return + return nil } } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 6acfc87d5..65a68386a 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1922,9 +1922,12 @@ func ViewIssue(ctx *context.Context) { ctx.HTML(http.StatusOK, tplIssueView) } +// checkBlockedByIssues return canRead and notPermitted func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.DependencyInfo) (canRead, notPermitted []*issues_model.DependencyInfo) { - var lastRepoID int64 - var lastPerm access_model.Permission + var ( + lastRepoID int64 + lastPerm access_model.Permission + ) for i, blocker := range blockers { // Get the permissions for this repository perm := lastPerm @@ -1936,7 +1939,7 @@ func checkBlockedByIssues(ctx *context.Context, blockers []*issues_model.Depende perm, err = access_model.GetUserRepoPermission(ctx, &blocker.Repository, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) - return + return nil, nil } } lastRepoID = blocker.Repository.ID diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 7da691791..4773e25c7 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -12,7 +12,6 @@ import ( "net/url" "path/filepath" "strings" - "time" git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" @@ -671,7 +670,7 @@ func WikiRaw(ctx *context.Context) { } if entry != nil { - if err = common.ServeBlob(ctx.Base, ctx.Repo.TreePath, entry.Blob(), time.Time{}); err != nil { + if err = common.ServeBlob(ctx.Base, ctx.Repo.TreePath, entry.Blob(), nil); err != nil { ctx.ServerError("ServeBlob", err) } return diff --git a/routers/web/web.go b/routers/web/web.go index 45c374e9c..4f5901c0e 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1256,20 +1256,20 @@ func registerRoutes(m *web.Route) { m.Group("/blob_excerpt", func() { m.Get("/{sha}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ExcerptBlob) - }, func(ctx *context.Context) (cancel gocontext.CancelFunc) { + }, func(ctx *context.Context) gocontext.CancelFunc { if ctx.FormBool("wiki") { ctx.Data["PageIsWiki"] = true repo.MustEnableWiki(ctx) - return + return nil } reqRepoCodeReader(ctx) if ctx.Written() { - return + return nil } - cancel = context.RepoRef()(ctx) + cancel := context.RepoRef()(ctx) if ctx.Written() { - return + return cancel } repo.MustBeNotEmpty(ctx) diff --git a/services/forms/user_form_hidden_comments.go b/services/forms/user_form_hidden_comments.go index 7eb800a02..03e629a55 100644 --- a/services/forms/user_form_hidden_comments.go +++ b/services/forms/user_form_hidden_comments.go @@ -90,7 +90,7 @@ func IsUserHiddenCommentTypeGroupChecked(group string, hiddenCommentTypes *big.I commentTypes, ok := hiddenCommentTypeGroups[group] if !ok { log.Critical("the group map for hidden comment types is out of sync, unknown group: %v", group) - return + return false } if hiddenCommentTypes == nil { return false diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 38283680a..9e1db6fd4 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -779,7 +779,7 @@ func skipToNextDiffHead(input *bufio.Reader) (line string, err error) { for { lineBytes, isFragment, err = input.ReadLine() if err != nil { - return + return "", err } if wasFragment { wasFragment = isFragment @@ -795,7 +795,7 @@ func skipToNextDiffHead(input *bufio.Reader) (line string, err error) { var tail string tail, err = input.ReadString('\n') if err != nil { - return + return "", err } line += tail } @@ -821,22 +821,21 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio _, isFragment, err = input.ReadLine() if err != nil { // Now by the definition of ReadLine this cannot be io.EOF - err = fmt.Errorf("unable to ReadLine: %w", err) - return + return nil, false, fmt.Errorf("unable to ReadLine: %w", err) } } sb.Reset() lineBytes, isFragment, err = input.ReadLine() if err != nil { if err == io.EOF { - return + return lineBytes, isFragment, err } err = fmt.Errorf("unable to ReadLine: %w", err) - return + return nil, false, err } if lineBytes[0] == 'd' { // End of hunks - return + return lineBytes, isFragment, err } switch lineBytes[0] { @@ -853,8 +852,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio lineBytes, isFragment, err = input.ReadLine() if err != nil { // Now by the definition of ReadLine this cannot be io.EOF - err = fmt.Errorf("unable to ReadLine: %w", err) - return + return nil, false, fmt.Errorf("unable to ReadLine: %w", err) } _, _ = sb.Write(lineBytes) } @@ -884,8 +882,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio } // This is used only to indicate that the current file does not have a terminal newline if !bytes.Equal(lineBytes, []byte("\\ No newline at end of file")) { - err = fmt.Errorf("unexpected line in hunk: %s", string(lineBytes)) - return + return nil, false, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes)) } // Technically this should be the end the file! // FIXME: we should be putting a marker at the end of the file if there is no terminal new line @@ -953,8 +950,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio curSection.Lines = append(curSection.Lines, diffLine) default: // This is unexpected - err = fmt.Errorf("unexpected line in hunk: %s", string(lineBytes)) - return + return nil, false, fmt.Errorf("unexpected line in hunk: %s", string(lineBytes)) } line := string(lineBytes) @@ -965,8 +961,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio lineBytes, isFragment, err = input.ReadLine() if err != nil { // Now by the definition of ReadLine this cannot be io.EOF - err = fmt.Errorf("unable to ReadLine: %w", err) - return + return lineBytes, isFragment, fmt.Errorf("unable to ReadLine: %w", err) } } } diff --git a/services/issue/assignee.go b/services/issue/assignee.go index 943a761e2..8fe35b560 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -46,13 +46,12 @@ func DeleteNotPassedAssignee(ctx context.Context, issue *issues_model.Issue, doe func ToggleAssignee(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *issues_model.Comment, err error) { removed, comment, err = issues_model.ToggleIssueAssignee(ctx, issue, doer, assigneeID) if err != nil { - return + return false, nil, err } - assignee, err1 := user_model.GetUserByID(ctx, assigneeID) - if err1 != nil { - err = err1 - return + assignee, err := user_model.GetUserByID(ctx, assigneeID) + if err != nil { + return false, nil, err } notification.NotifyIssueChangeAssignee(ctx, doer, issue, assignee, removed, comment) @@ -236,23 +235,23 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use } if err != nil { - return + return nil, err } if comment == nil || !isAdd { - return + return nil, nil } // notify all user in this team - if err = comment.LoadIssue(ctx); err != nil { - return + if err := comment.LoadIssue(ctx); err != nil { + return nil, err } members, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{ TeamID: reviewer.ID, }) if err != nil { - return + return nil, err } for _, member := range members { diff --git a/services/issue/issue.go b/services/issue/issue.go index 61890c75d..b6c6a26cb 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -49,17 +49,17 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo } // ChangeTitle changes the title of this issue, as the given user. -func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, title string) (err error) { +func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, title string) error { oldTitle := issue.Title issue.Title = title - if err = issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil { - return + if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil { + return err } if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) { - if err = issues_model.PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest); err != nil { - return + if err := issues_model.PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest); err != nil { + return err } } diff --git a/services/issue/label.go b/services/issue/label.go index c18abbfcd..ee821a49c 100644 --- a/services/issue/label.go +++ b/services/issue/label.go @@ -12,9 +12,9 @@ import ( ) // ClearLabels clears all of an issue's labels -func ClearLabels(issue *issues_model.Issue, doer *user_model.User) (err error) { - if err = issues_model.ClearIssueLabels(issue, doer); err != nil { - return +func ClearLabels(issue *issues_model.Issue, doer *user_model.User) error { + if err := issues_model.ClearIssueLabels(issue, doer); err != nil { + return err } notification.NotifyIssueClearLabels(db.DefaultContext, doer, issue) diff --git a/services/pull/review.go b/services/pull/review.go index 6feffe4ec..e920bc8c1 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -320,7 +320,7 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos func DismissReview(ctx context.Context, reviewID, repoID int64, message string, doer *user_model.User, isDismiss, dismissPriors bool) (comment *issues_model.Comment, err error) { review, err := issues_model.GetReviewByID(ctx, reviewID) if err != nil { - return + return nil, err } if review.Type != issues_model.ReviewTypeApprove && review.Type != issues_model.ReviewTypeReject { @@ -328,7 +328,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, } // load data for notify - if err = review.LoadAttributes(ctx); err != nil { + if err := review.LoadAttributes(ctx); err != nil { return nil, err } @@ -337,8 +337,8 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, return nil, fmt.Errorf("reviews's repository is not the same as the one we expect") } - if err = issues_model.DismissReview(review, isDismiss); err != nil { - return + if err := issues_model.DismissReview(review, isDismiss); err != nil { + return nil, err } if dismissPriors { @@ -361,11 +361,11 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, return nil, nil } - if err = review.Issue.LoadPullRequest(ctx); err != nil { - return + if err := review.Issue.LoadPullRequest(ctx); err != nil { + return nil, err } - if err = review.Issue.LoadAttributes(ctx); err != nil { - return + if err := review.Issue.LoadAttributes(ctx); err != nil { + return nil, err } comment, err = issue_service.CreateComment(ctx, &issues_model.CreateCommentOptions{ @@ -377,7 +377,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, Repo: review.Issue.Repo, }) if err != nil { - return + return nil, err } comment.Review = review @@ -386,5 +386,5 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string, notification.NotifyPullReviewDismiss(ctx, doer, review, comment) - return comment, err + return comment, nil } diff --git a/services/release/release.go b/services/release/release.go index c1190305b..1ccbd9c81 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -187,7 +187,7 @@ func CreateNewTag(ctx context.Context, doer *user_model.User, repo *repo_model.R // editAttachments accept a map of attachment uuid to new attachment name which will be updated with attachments. func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_model.Release, addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string, -) (err error) { +) error { if rel.ID == 0 { return errors.New("UpdateRelease only accepts an exist release") } @@ -264,8 +264,8 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_mod } } - if err = committer.Commit(); err != nil { - return + if err := committer.Commit(); err != nil { + return err } for _, uuid := range delAttachmentUUIDs { @@ -280,14 +280,14 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *repo_mod if !isCreated { notification.NotifyUpdateRelease(gitRepo.Ctx, doer, rel) - return + return nil } if !rel.IsDraft { notification.NotifyNewRelease(gitRepo.Ctx, rel) } - return err + return nil } // DeleteReleaseByID deletes a release and corresponding Git tag by given ID.