diff options
| author | kolaente <konrad@kola-entertainments.de> | 2019-06-12 21:41:28 +0200 |
|---|---|---|
| committer | techknowlogick <techknowlogick@gitea.io> | 2019-06-12 15:41:28 -0400 |
| commit | f9ec2f89f2265bc1371a6c62359de9816534fa6b (patch) | |
| tree | f48b138a457e5ac6cf843bbb38400926704370f7 | |
| parent | 5832f8d90df2d72cb38698c3e9050f2b29717dc7 (diff) | |
| download | gitea-f9ec2f89f2265bc1371a6c62359de9816534fa6b.tar.gz gitea-f9ec2f89f2265bc1371a6c62359de9816534fa6b.zip | |
Add golangci (#6418)
147 files changed, 1046 insertions, 774 deletions
diff --git a/.drone.yml b/.drone.yml index 0b8c8422b6..ec90c158b1 100644 --- a/.drone.yml +++ b/.drone.yml @@ -74,12 +74,10 @@ pipeline: commands: - make clean - make generate - - make vet - - make lint - - make fmt-check + - make golangci-lint + - make revive - make swagger-check - make swagger-validate - - make misspell-check - make test-vendor - make build when: diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..82d0e46694 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,97 @@ +linters: + enable: + - gosimple + - deadcode + - typecheck + - govet + - errcheck + - staticcheck + - unused + - structcheck + - varcheck + - golint + - dupl + #- gocyclo # The cyclomatic complexety of a lot of functions is too high, we should refactor those another time. + - gofmt + - misspell + - gocritic + enable-all: false + disable-all: true + fast: false + +linters-settings: + gocritic: + disabled-checks: + - ifElseChain + - singleCaseSwitch # Every time this occured in the code, there was no other way. + +issues: + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - gocyclo + - errcheck + - dupl + - gosec + - unparam + - staticcheck + - path: models/migrations/v + linters: + - gocyclo + - errcheck + - dupl + - gosec + - linters: + - dupl + text: "webhook" + - linters: + - gocritic + text: "`ID' should not be capitalized" + - path: modules/templates/helper.go + linters: + - gocritic + - linters: + - unused + - deadcode + text: "swagger" + - path: contrib/pr/checkout.go + linters: + - errcheck + - path: models/issue.go + linters: + - errcheck + - path: models/migrations/ + linters: + - errcheck + - path: modules/log/ + linters: + - errcheck + - path: routers/routes/routes.go + linters: + - dupl + - path: routers/repo/view.go + linters: + - dupl + - path: models/migrations/ + linters: + - unused + - linters: + - staticcheck + text: "argument x is overwritten before first use" + - path: modules/httplib/httplib.go + linters: + - staticcheck + # Enabling this would require refactoring the methods and how they are called. + - path: models/issue_comment_list.go + linters: + - dupl + # "Destroy" is misspelled in github.com/go-macaron/session/session.go:213 so it's not our responsability to fix it + - path: modules/session/virtual.go + linters: + - misspell + text: '`Destory` is a misspelling of `Destroy`' + - path: modules/session/memory.go + linters: + - misspell + text: '`Destory` is a misspelling of `Destroy`' @@ -135,6 +135,10 @@ errcheck: .PHONY: lint lint: + @echo 'make lint is depricated. Use "make revive" if you want to use the old lint tool, or "make golangci-lint" to run a complete code check.' + +.PHONY: revive +revive: @hash revive > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ $(GO) get -u github.com/mgechev/revive; \ fi @@ -461,3 +465,10 @@ generate-images: .PHONY: pr pr: $(GO) run contrib/pr/checkout.go $(PR) + +.PHONY: golangci-lint +golangci-lint: + @hash golangci-lint > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ + curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(GOPATH)/bin v1.16.0; \ + fi + golangci-lint run diff --git a/cmd/admin.go b/cmd/admin.go index ecb4eb48a6..6234ab828d 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -481,7 +481,7 @@ func runUpdateOauth(c *cli.Context) error { } // update custom URL mapping - var customURLMapping *oauth2.CustomURLMapping + var customURLMapping = &oauth2.CustomURLMapping{} if oAuth2Config.CustomURLMapping != nil { customURLMapping.TokenURL = oAuth2Config.CustomURLMapping.TokenURL diff --git a/cmd/cert.go b/cmd/cert.go index 46473c0042..b62319f808 100644 --- a/cmd/cert.go +++ b/cmd/cert.go @@ -170,17 +170,28 @@ func runCert(c *cli.Context) error { if err != nil { log.Fatalf("Failed to open cert.pem for writing: %v", err) } - pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) - certOut.Close() + err = pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) + if err != nil { + log.Fatalf("Failed to encode certificate: %v", err) + } + err = certOut.Close() + if err != nil { + log.Fatalf("Failed to write cert: %v", err) + } log.Println("Written cert.pem") keyOut, err := os.OpenFile("key.pem", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) if err != nil { log.Fatalf("Failed to open key.pem for writing: %v", err) } - pem.Encode(keyOut, pemBlockForKey(priv)) - keyOut.Close() + err = pem.Encode(keyOut, pemBlockForKey(priv)) + if err != nil { + log.Fatalf("Failed to encode key: %v", err) + } + err = keyOut.Close() + if err != nil { + log.Fatalf("Failed to write key: %v", err) + } log.Println("Written key.pem") - return nil } diff --git a/cmd/serv.go b/cmd/serv.go index c0cb3cd50f..c1c8fd3a97 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -30,7 +30,6 @@ import ( ) const ( - accessDenied = "Repository does not exist or you do not have access" lfsAuthenticateVerb = "git-lfs-authenticate" ) @@ -67,7 +66,7 @@ func checkLFSVersion() { } func setup(logPath string) { - log.DelLogger("console") + _ = log.DelLogger("console") setting.NewContext() checkLFSVersion() } @@ -112,7 +111,9 @@ func runServ(c *cli.Context) error { } if len(c.Args()) < 1 { - cli.ShowSubcommandHelp(c) + if err := cli.ShowSubcommandHelp(c); err != nil { + fmt.Printf("error showing subcommand help: %v\n", err) + } return nil } diff --git a/cmd/web.go b/cmd/web.go index e6d0300a15..e211674b4d 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -178,11 +178,16 @@ func runWeb(ctx *cli.Context) error { } err = runHTTPS(listenAddr, setting.CertFile, setting.KeyFile, context2.ClearHandler(m)) case setting.FCGI: - listener, err := net.Listen("tcp", listenAddr) + var listener net.Listener + listener, err = net.Listen("tcp", listenAddr) if err != nil { log.Fatal("Failed to bind %s: %v", listenAddr, err) } - defer listener.Close() + defer func() { + if err := listener.Close(); err != nil { + log.Fatal("Failed to stop server: %v", err) + } + }() err = fcgi.Serve(listener, context2.ClearHandler(m)) case setting.UnixSocket: if err := os.Remove(listenAddr); err != nil && !os.IsNotExist(err) { diff --git a/contrib/pr/checkout.go b/contrib/pr/checkout.go index 880c029510..a837cb5812 100644 --- a/contrib/pr/checkout.go +++ b/contrib/pr/checkout.go @@ -91,8 +91,7 @@ func runPR() { routers.NewServices() //x, err = xorm.NewEngine("sqlite3", "file::memory:?cache=shared") - var helper testfixtures.Helper - helper = &testfixtures.SQLite{} + var helper testfixtures.Helper = &testfixtures.SQLite{} models.NewEngine(func(_ *xorm.Engine) error { return nil }) diff --git a/integrations/branches_test.go b/integrations/branches_test.go index 01c6dd2a4b..e74f338aa2 100644 --- a/integrations/branches_test.go +++ b/integrations/branches_test.go @@ -62,7 +62,7 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) { req = NewRequestWithValues(t, "POST", link, map[string]string{ "_csrf": getCsrf(t, htmlDoc.doc), }) - resp = session.MakeRequest(t, req, http.StatusOK) + session.MakeRequest(t, req, http.StatusOK) url, err := url.Parse(link) assert.NoError(t, err) diff --git a/integrations/editor_test.go b/integrations/editor_test.go index 8e6effe7eb..a46712293e 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -34,7 +34,7 @@ func TestCreateFile(t *testing.T) { "content": "Content", "commit_choice": "direct", }) - resp = session.MakeRequest(t, req, http.StatusFound) + session.MakeRequest(t, req, http.StatusFound) }) } @@ -48,7 +48,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { "_csrf": csrf, "protected": "on", }) - resp := session.MakeRequest(t, req, http.StatusFound) + session.MakeRequest(t, req, http.StatusFound) // Check if master branch has been locked successfully flashCookie := session.GetCookie("macaron_flash") assert.NotNil(t, flashCookie) @@ -56,7 +56,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { // Request editor page req = NewRequest(t, "GET", "/user2/repo1/_new/master/") - resp = session.MakeRequest(t, req, http.StatusOK) + resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) lastCommit := doc.GetInputValueByName("last_commit") diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 80a42efb5c..e9b46ffcb9 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -42,7 +42,7 @@ type NilResponseRecorder struct { } func (n *NilResponseRecorder) Write(b []byte) (int, error) { - n.Length = n.Length + len(b) + n.Length += len(b) return len(b), nil } @@ -141,8 +141,7 @@ func initIntegrationTest() { if err != nil { log.Fatalf("sql.Open: %v", err) } - rows, err := db.Query(fmt.Sprintf("SELECT 1 FROM pg_database WHERE datname = '%s'", - models.DbCfg.Name)) + rows, err := db.Query(fmt.Sprintf("SELECT 1 FROM pg_database WHERE datname = '%s'", models.DbCfg.Name)) if err != nil { log.Fatalf("db.Query: %v", err) } @@ -210,7 +209,7 @@ func (s *TestSession) MakeRequest(t testing.TB, req *http.Request, expectedStatu resp := MakeRequest(t, req, expectedStatus) ch := http.Header{} - ch.Add("Cookie", strings.Join(resp.HeaderMap["Set-Cookie"], ";")) + ch.Add("Cookie", strings.Join(resp.Header()["Set-Cookie"], ";")) cr := http.Request{Header: ch} s.jar.SetCookies(baseURL, cr.Cookies()) @@ -226,7 +225,7 @@ func (s *TestSession) MakeRequestNilResponseRecorder(t testing.TB, req *http.Req resp := MakeRequestNilResponseRecorder(t, req, expectedStatus) ch := http.Header{} - ch.Add("Cookie", strings.Join(resp.HeaderMap["Set-Cookie"], ";")) + ch.Add("Cookie", strings.Join(resp.Header()["Set-Cookie"], ";")) cr := http.Request{Header: ch} s.jar.SetCookies(baseURL, cr.Cookies()) @@ -266,7 +265,7 @@ func loginUserWithPassword(t testing.TB, userName, password string) *TestSession resp = MakeRequest(t, req, http.StatusFound) ch := http.Header{} - ch.Add("Cookie", strings.Join(resp.HeaderMap["Set-Cookie"], ";")) + ch.Add("Cookie", strings.Join(resp.Header()["Set-Cookie"], ";")) cr := http.Request{Header: ch} session := emptyTestSession(t) diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 8f01d712a3..567cf13b45 100644 --- a/integrations/lfs_getobject_test.go +++ b/integrations/lfs_getobject_test.go @@ -45,7 +45,7 @@ func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string lfsMetaObject = &models.LFSMetaObject{Oid: oid, Size: int64(len(*content)), RepositoryID: repositoryID} } - lfsID = lfsID + 1 + lfsID++ lfsMetaObject, err = models.NewLFSMetaObject(lfsMetaObject) assert.NoError(t, err) contentStore := &lfs.ContentStore{BasePath: setting.LFS.ContentPath} diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index f168424865..15b086c6e6 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -57,21 +57,6 @@ func initMigrationTest(t *testing.T) { setting.NewLogServices(true) } -func getDialect() string { - dialect := "sqlite" - switch { - case setting.UseSQLite3: - dialect = "sqlite" - case setting.UseMySQL: - dialect = "mysql" - case setting.UsePostgreSQL: - dialect = "pgsql" - case setting.UseMSSQL: - dialect = "mssql" - } - return dialect -} - func availableVersions() ([]string, error) { migrationsDir, err := os.Open("integrations/migration-test") if err != nil { diff --git a/integrations/testlogger.go b/integrations/testlogger.go index c50daead9b..43a1471f66 100644 --- a/integrations/testlogger.go +++ b/integrations/testlogger.go @@ -73,7 +73,7 @@ func PrintCurrentTest(t testing.TB, skip ...int) { _, filename, line, _ := runtime.Caller(actualSkip) if log.CanColorStdout { - fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", log.NewColoredValue(t.Name()), strings.TrimPrefix(filename, prefix), line) + fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", fmt.Formatter(log.NewColoredValue(t.Name())), strings.TrimPrefix(filename, prefix), line) } else { fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", t.Name(), strings.TrimPrefix(filename, prefix), line) } @@ -42,7 +42,7 @@ var ( func init() { setting.AppVer = Version - setting.AppBuiltWith = formatBuiltWith(Tags) + setting.AppBuiltWith = formatBuiltWith() // Grab the original help templates originalAppHelpTemplate = cli.AppHelpTemplate @@ -56,7 +56,7 @@ func main() { app.Usage = "A painless self-hosted Git service" app.Description = `By default, gitea will start serving using the webserver with no arguments - which can alternatively be run by running the subcommand web.` - app.Version = Version + formatBuiltWith(Tags) + app.Version = Version + formatBuiltWith() app.Commands = []cli.Command{ cmd.CmdWeb, cmd.CmdServ, @@ -179,7 +179,7 @@ DEFAULT CONFIGURATION: `, originalTemplate, setting.CustomPath, overrided, setting.CustomConf, setting.AppPath, setting.AppWorkPath) } -func formatBuiltWith(makeTags string) string { +func formatBuiltWith() string { var version = runtime.Version() if len(MakeVersion) > 0 { version = MakeVersion + ", " + runtime.Version() diff --git a/models/access_test.go b/models/access_test.go index d6a1c92b90..db6f655a67 100644 --- a/models/access_test.go +++ b/models/access_test.go @@ -10,13 +10,6 @@ import ( "github.com/stretchr/testify/assert" ) -var accessModes = []AccessMode{ - AccessModeRead, - AccessModeWrite, - AccessModeAdmin, - AccessModeOwner, -} - func TestAccessLevel(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) diff --git a/models/branches.go b/models/branches.go index abefa60f48..df3b69aa21 100644 --- a/models/branches.go +++ b/models/branches.go @@ -126,14 +126,14 @@ func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) } // GetProtectedBranchByRepoID getting protected branch by repo ID -func GetProtectedBranchByRepoID(RepoID int64) ([]*ProtectedBranch, error) { +func GetProtectedBranchByRepoID(repoID int64) ([]*ProtectedBranch, error) { protectedBranches := make([]*ProtectedBranch, 0) - return protectedBranches, x.Where("repo_id = ?", RepoID).Desc("updated_unix").Find(&protectedBranches) + return protectedBranches, x.Where("repo_id = ?", repoID).Desc("updated_unix").Find(&protectedBranches) } // GetProtectedBranchBy getting protected branch by ID/Name -func GetProtectedBranchBy(repoID int64, BranchName string) (*ProtectedBranch, error) { - rel := &ProtectedBranch{RepoID: repoID, BranchName: BranchName} +func GetProtectedBranchBy(repoID int64, branchName string) (*ProtectedBranch, error) { + rel := &ProtectedBranch{RepoID: repoID, BranchName: branchName} has, err := x.Get(rel) if err != nil { return nil, err diff --git a/models/git_blame.go b/models/git_blame.go index 7b4fb64a70..2b439a23b9 100644 --- a/models/git_blame.go +++ b/models/git_blame.go @@ -40,7 +40,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { scanner := r.scanner if r.lastSha != nil { - blamePart = &BlamePart{*r.lastSha, make([]string, 0, 0)} + blamePart = &BlamePart{*r.lastSha, make([]string, 0)} } for scanner.Scan() { @@ -56,7 +56,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { sha1 := lines[1] if blamePart == nil { - blamePart = &BlamePart{sha1, make([]string, 0, 0)} + blamePart = &BlamePart{sha1, make([]string, 0)} } if blamePart.Sha != sha1 { diff --git a/models/git_diff.go b/models/git_diff.go index ac2a5f90d7..a6ea7306d4 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -384,13 +384,9 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi // headers + hunk header newHunk := make([]string, headerLines) // transfer existing headers - for idx, lof := range hunk[:headerLines] { - newHunk[idx] = lof - } + copy(newHunk, hunk[:headerLines]) // transfer last n lines - for _, lof := range hunk[len(hunk)-numbersOfLine-1:] { - newHunk = append(newHunk, lof) - } + newHunk = append(newHunk, hunk[len(hunk)-numbersOfLine-1:]...) // calculate newBegin, ... by counting lines for i := len(hunk) - 1; i >= len(hunk)-numbersOfLine; i-- { switch hunk[i][0] { @@ -582,7 +578,10 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D diff.Files = append(diff.Files, curFile) if len(diff.Files) >= maxFiles { diff.IsIncomplete = true - io.Copy(ioutil.Discard, reader) + _, err := io.Copy(ioutil.Discard, reader) + if err != nil { + return nil, fmt.Errorf("Copy: %v", err) + } break } curFileLinesCount = 0 diff --git a/models/git_diff_test.go b/models/git_diff_test.go index 2111e9044f..deca7c8d4a 100644 --- a/models/git_diff_test.go +++ b/models/git_diff_test.go @@ -17,12 +17,6 @@ func assertEqual(t *testing.T, s1 string, s2 template.HTML) { } } -func assertLineEqual(t *testing.T, d1 *DiffLine, d2 *DiffLine) { - if d1 != d2 { - t.Errorf("%v should be equal %v", d1, d2) - } -} - func TestDiffToHTML(t *testing.T) { assertEqual(t, "+foo <span class=\"added-code\">bar</span> biz", diffToHTML([]dmp.Diff{ {Type: dmp.DiffEqual, Text: "foo "}, diff --git a/models/issue.go b/models/issue.go index 999bd2f7a9..27298b8a86 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1330,7 +1330,7 @@ func sortIssuesSession(sess *xorm.Session, sortType string) { } } -func (opts *IssuesOptions) setupSession(sess *xorm.Session) error { +func (opts *IssuesOptions) setupSession(sess *xorm.Session) { if opts.Page >= 0 && opts.PageSize > 0 { var start int if opts.Page == 0 { @@ -1389,7 +1389,6 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) error { fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID)) } } - return nil } // CountIssuesByRepo map from repoID to number of issues matching the options @@ -1397,9 +1396,7 @@ func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) { sess := x.NewSession() defer sess.Close() - if err := opts.setupSession(sess); err != nil { - return nil, err - } + opts.setupSession(sess) countsSlice := make([]*struct { RepoID int64 @@ -1424,9 +1421,7 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) { sess := x.NewSession() defer sess.Close() - if err := opts.setupSession(sess); err != nil { - return nil, err - } + opts.setupSession(sess) sortIssuesSession(sess, opts.SortType) issues := make([]*Issue, 0, setting.UI.IssuePagingNum) diff --git a/models/issue_comment.go b/models/issue_comment.go index 0d2e917f85..d75d9d7db1 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -171,17 +171,6 @@ func (c *Comment) loadPoster(e Engine) (err error) { return err } -func (c *Comment) loadAttachments(e Engine) (err error) { - if len(c.Attachments) > 0 { - return - } - c.Attachments, err = getAttachmentsByCommentID(e, c.ID) - if err != nil { - log.Error("getAttachmentsByCommentID[%d]: %v", c.ID, err) - } - return err -} - // AfterDelete is invoked from XORM after the object is deleted. func (c *Comment) AfterDelete() { if c.ID <= 0 { @@ -463,7 +452,7 @@ func (c *Comment) LoadReview() error { return c.loadReview(x) } -func (c *Comment) checkInvalidation(e Engine, doer *User, repo *git.Repository, branch string) error { +func (c *Comment) checkInvalidation(doer *User, repo *git.Repository, branch string) error { // FIXME differentiate between previous and proposed line commit, err := repo.LineBlame(branch, repo.Path, c.TreePath, uint(c.UnsignedLine())) if err != nil { @@ -479,7 +468,7 @@ func (c *Comment) checkInvalidation(e Engine, doer *User, repo *git.Repository, // CheckInvalidation checks if the line of code comment got changed by another commit. // If the line got changed the comment is going to be invalidated. func (c *Comment) CheckInvalidation(repo *git.Repository, doer *User, branch string) error { - return c.checkInvalidation(x, doer, repo, branch) + return c.checkInvalidation(doer, repo, branch) } // DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes. @@ -915,7 +904,7 @@ func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, tree commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line)) if err == nil { commitID = commit.ID.String() - } else if err != nil && !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") { + } else if !strings.Contains(err.Error(), "exit status 128 - fatal: no such path") { return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err) } } diff --git a/models/issue_comment_list.go b/models/issue_comment_list.go index a8c8123280..ae2a89a01a 100644 --- a/models/issue_comment_list.go +++ b/models/issue_comment_list.go @@ -36,7 +36,7 @@ func (comments CommentList) loadPosters(e Engine) error { if err != nil { return err } - left = left - limit + left -= limit posterIDs = posterIDs[limit:] } @@ -94,13 +94,13 @@ func (comments CommentList) loadLabels(e Engine) error { var label Label err = rows.Scan(&label) if err != nil { - rows.Close() + _ = rows.Close() return err } commentLabels[label.ID] = &label } - rows.Close() - left = left - limit + _ = rows.Close() + left -= limit labelIDs = labelIDs[limit:] } @@ -143,7 +143,7 @@ func (comments CommentList) loadMilestones(e Engine) error { if err != nil { return err } - left = left - limit + left -= limit milestoneIDs = milestoneIDs[limit:] } @@ -186,7 +186,7 @@ func (comments CommentList) loadOldMilestones(e Engine) error { if err != nil { return err } - left = left - limit + left -= limit milestoneIDs = milestoneIDs[limit:] } @@ -236,9 +236,9 @@ func (comments CommentList) loadAssignees(e Engine) error { assignees[user.ID] = &user } - rows.Close() + _ = rows.Close() - left = left - limit + left -= limit assigneeIDs = assigneeIDs[limit:] } @@ -310,9 +310,9 @@ func (comments CommentList) loadIssues(e Engine) error { issues[issue.ID] = &issue } - rows.Close() + _ = rows.Close() - left = left - limit + left -= limit issueIDs = issueIDs[limit:] } @@ -361,15 +361,15 @@ func (comments CommentList) loadDependentIssues(e Engine) error { var issue Issue err = rows.Scan(&issue) if err != nil { - rows.Close() + _ = rows.Close() return err } issues[issue.ID] = &issue } - rows.Close() + _ = rows.Close() - left = left - limit + left -= limit issueIDs = issueIDs[limit:] } @@ -406,14 +406,14 @@ func (comments CommentList) loadAttachments(e Engine) (err error) { var attachment Attachment err = rows.Scan(&attachment) if err != nil { - rows.Close() + _ = rows.Close() return err } attachments[attachment.CommentID] = append(attachments[attachment.CommentID], &attachment) } - rows.Close() - left = left - limit + _ = rows.Close() + left -= limit commentsIDs = commentsIDs[limit:] } @@ -457,15 +457,15 @@ func (comments CommentList) loadReviews(e Engine) error { var review Review err = rows.Scan(&review) if err != nil { - rows.Close() + _ = rows.Close() return err } reviews[review.ID] = &review } - rows.Close() + _ = rows.Close() - left = left - limit + left -= limit reviewIDs = reviewIDs[limit:] } diff --git a/models/issue_label.go b/models/issue_label.go index 38266f3e7c..363d4bb814 100644 --- a/models/issue_label.go +++ b/models/issue_label.go @@ -401,14 +401,6 @@ func NewIssueLabels(issue *Issue, labels []*Label, doer *User) (err error) { return sess.Commit() } -func getIssueLabels(e Engine, issueID int64) ([]*IssueLabel, error) { - issueLabels := make([]*IssueLabel, 0, 10) - return issueLabels, e. - Where("issue_id=?", issueID). - Asc("label_id"). - Find(&issueLabels) -} - func deleteIssueLabel(e *xorm.Session, issue *Issue, label *Label, doer *User) (err error) { if count, err := e.Delete(&IssueLabel{ IssueID: issue.ID, diff --git a/models/issue_list.go b/models/issue_list.go index a1aab488fc..4ddb32da13 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -7,6 +7,8 @@ package models import ( "fmt" + "code.gitea.io/gitea/modules/log" + "github.com/go-xorm/builder" ) @@ -47,7 +49,7 @@ func (issues IssueList) loadRepositories(e Engine) ([]*Repository, error) { if err != nil { return nil, fmt.Errorf("find repository: %v", err) } - left = left - limit + left -= limit repoIDs = repoIDs[limit:] } @@ -91,7 +93,7 @@ func (issues IssueList) loadPosters(e Engine) error { if err != nil { return err } - left = left - limit + left -= limit posterIDs = posterIDs[limit:] } @@ -146,13 +148,21 @@ func (issues IssueList) loadLabels(e Engine) error { var labelIssue LabelIssue err = rows.Scan(&labelIssue) if err != nil { - rows.Close() + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadLabels: Close: %v", err) + } return err } issueLabels[labelIssue.IssueLabel.IssueID] = append(issueLabels[labelIssue.IssueLabel.IssueID], labelIssue.Label) } - rows.Close() - left = left - limit + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadLabels: Close: %v", err) + } + left -= limit issueIDs = issueIDs[limit:] } @@ -191,7 +201,7 @@ func (issues IssueList) loadMilestones(e Engine) error { if err != nil { return err } - left = left - limit + left -= limit milestoneIDs = milestoneIDs[limit:] } @@ -231,15 +241,22 @@ func (issues IssueList) loadAssignees(e Engine) error { var assigneeIssue AssigneeIssue err = rows.Scan(&assigneeIssue) if err != nil { - rows.Close() + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadAssignees: Close: %v", err) + } return err } assignees[assigneeIssue.IssueAssignee.IssueID] = append(assignees[assigneeIssue.IssueAssignee.IssueID], assigneeIssue.Assignee) } - rows.Close() - - left = left - limit + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadAssignees: Close: %v", err) + } + left -= limit issueIDs = issueIDs[limit:] } @@ -283,14 +300,21 @@ func (issues IssueList) loadPullRequests(e Engine) error { var pr PullRequest err = rows.Scan(&pr) if err != nil { - rows.Close() + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadPullRequests: Close: %v", err) + } return err } pullRequestMaps[pr.IssueID] = &pr } - - rows.Close() - left = left - limit + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadPullRequests: Close: %v", err) + } + left -= limit issuesIDs = issuesIDs[limit:] } @@ -325,14 +349,21 @@ func (issues IssueList) loadAttachments(e Engine) (err error) { var attachment Attachment err = rows.Scan(&attachment) if err != nil { - rows.Close() + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadAttachments: Close: %v", err) + } return err } attachments[attachment.IssueID] = append(attachments[attachment.IssueID], &attachment) } - - rows.Close() - left = left - limit + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadAttachments: Close: %v", err) + } + left -= limit issuesIDs = issuesIDs[limit:] } @@ -368,13 +399,21 @@ func (issues IssueList) loadComments(e Engine, cond builder.Cond) (err error) { var comment Comment err = rows.Scan(&comment) if err != nil { - rows.Close() + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadComments: Close: %v", err) + } return err } comments[comment.IssueID] = append(comments[comment.IssueID], &comment) } - rows.Close() - left = left - limit + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadComments: Close: %v", err) + } + left -= limit issuesIDs = issuesIDs[limit:] } @@ -422,13 +461,21 @@ func (issues IssueList) loadTotalTrackedTimes(e Engine) (err error) { var totalTime totalTimesByIssue err = rows.Scan(&totalTime) if err != nil { - rows.Close() + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadTotalTrackedTimes: Close: %v", err) + } return err } trackedTimes[totalTime.IssueID] = totalTime.Time } - rows.Close() - left = left - limit + // When there are no rows left and we try to close it, xorm will complain with an error. + // Since that is not relevant for us, we can safely ignore it. + if err := rows.Close(); err != nil { + log.Error("IssueList.loadTotalTrackedTimes: Close: %v", err) + } + left -= limit ids = ids[limit:] } @@ -439,33 +486,33 @@ func (issues IssueList) loadTotalTrackedTimes(e Engine) (err error) { } // loadAttributes loads all attributes, expect for attachments and comments -func (issues IssueList) loadAttributes(e Engine) (err error) { - if _, err = issues.loadRepositories(e); err != nil { - return +func (issues IssueList) loadAttributes(e Engine) error { + if _, err := issues.loadRepositories(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadRepositories: %v", err) } - if err = issues.loadPosters(e); err != nil { - return + if err := issues.loadPosters(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadPosters: %v", err) } - if err = issues.loadLabels(e); err != nil { - return + if err := issues.loadLabels(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadLabels: %v", err) } - if err = issues.loadMilestones(e); err != nil { - return + if err := issues.loadMilestones(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadMilestones: %v", err) } - if err = issues.loadAssignees(e); err != nil { - return + if err := issues.loadAssignees(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadAssignees: %v", err) } - if err = issues.loadPullRequests(e); err != nil { - return + if err := issues.loadPullRequests(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadPullRequests: %v", err) } - if err = issues.loadTotalTrackedTimes(e); err != nil { - return + if err := issues.loadTotalTrackedTimes(e); err != nil { + return fmt.Errorf("issue.loadAttributes: loadTotalTrackedTimes: %v", err) } return nil diff --git a/models/log.go b/models/log.go index 4994545c5f..38d6caf07c 100644 --- a/models/log.go +++ b/models/log.go @@ -15,7 +15,6 @@ import ( // XORMLogBridge a logger bridge from Logger to xorm type XORMLogBridge struct { showSQL bool - level core.LogLevel logger *log.Logger } @@ -34,42 +33,42 @@ func (l *XORMLogBridge) Log(skip int, level log.Level, format string, v ...inter // Debug show debug log func (l *XORMLogBridge) Debug(v ...interface{}) { - l.Log(2, log.DEBUG, fmt.Sprint(v...)) + _ = l.Log(2, log.DEBUG, fmt.Sprint(v...)) } // Debugf show debug log func (l *XORMLogBridge) Debugf(format string, v ...interface{}) { - l.Log(2, log.DEBUG, format, v...) + _ = l.Log(2, log.DEBUG, format, v...) } // Error show error log func (l *XORMLogBridge) Error(v ...interface{}) { - l.Log(2, log.ERROR, fmt.Sprint(v...)) + _ = l.Log(2, log.ERROR, fmt.Sprint(v...)) } // Errorf show error log func (l *XORMLogBridge) Errorf(format string, v ...interface{}) { - l.Log(2, log.ERROR, format, v...) + _ = l.Log(2, log.ERROR, format, v...) } // Info show information level log func (l *XORMLogBridge) Info(v ...interface{}) { - l.Log(2, log.INFO, fmt.Sprint(v...)) + _ = l.Log(2, log.INFO, fmt.Sprint(v...)) } // Infof show information level log func (l *XORMLogBridge) Infof(format string, v ...interface{}) { - l.Log(2, log.INFO, format, v...) + _ = l.Log(2, log.INFO, format, v...) } // Warn show warning log func (l *XORMLogBridge) Warn(v ...interface{}) { - l.Log(2, log.WARN, fmt.Sprint(v...)) + _ = l.Log(2, log.WARN, fmt.Sprint(v...)) } // Warnf show warnning log func (l *XORMLogBridge) Warnf(format string, v ...interface{}) { - l.Log(2, log.WARN, format, v...) + _ = l.Log(2, log.WARN, format, v...) } // Level get logger level diff --git a/models/login_source.go b/models/login_source.go index 9b8173b84d..8eefec4ae5 100644 --- a/models/login_source.go +++ b/models/login_source.go @@ -164,8 +164,7 @@ func Cell2Int64(val xorm.Cell) int64 { // BeforeSet is invoked from XORM before setting the value of a field of this object. func (source *LoginSource) BeforeSet(colName string, val xorm.Cell) { - switch colName { - case "type": + if colName == "type" { switch LoginType(Cell2Int64(val)) { case LoginLDAP, LoginDLDAP: source.Cfg = new(LDAPConfig) @@ -282,10 +281,12 @@ func CreateLoginSource(source *LoginSource) error { oAuth2Config := source.OAuth2() err = oauth2.RegisterProvider(source.Name, oAuth2Config.Provider, oAuth2Config.ClientID, oAuth2Config.ClientSecret, oAuth2Config.OpenIDConnectAutoDiscoveryURL, oAuth2Config.CustomURLMapping) err = wrapOpenIDConnectInitializeError(err, source.Name, oAuth2Config) - if err != nil { // remove the LoginSource in case of errors while registering OAuth2 providers - x.Delete(source) + if _, err := x.Delete(source); err != nil { + log.Error("CreateLoginSource: Error while wrapOpenIDConnectInitializeError: %v", err) + } + return err } } return err @@ -325,10 +326,12 @@ func UpdateSource(source *LoginSource) error { oAuth2Config := source.OAuth2() err = oauth2.RegisterProvider(source.Name, oAuth2Config.Provider, oAuth2Config.ClientID, oAuth2Config.ClientSecret, oAuth2Config.OpenIDConnectAutoDiscoveryURL, oAuth2Config.CustomURLMapping) err = wrapOpenIDConnectInitializeError(err, source.Name, oAuth2Config) - if err != nil { // restore original values since we cannot update the provider it self - x.ID(source.ID).AllCols().Update(originalLoginSource) + if _, err := x.ID(source.ID).AllCols().Update(originalLoginSource); err != nil { + log.Error("UpdateSource: Error while wrapOpenIDConnectInitializeError: %v", err) + } + return err } } return err @@ -385,7 +388,7 @@ func composeFullName(firstname, surname, username string) string { } var ( - alphaDashDotPattern = regexp.MustCompile("[^\\w-\\.]") + alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`) ) // LoginViaLDAP queries if login/password is valid against the LDAP directory pool, @@ -401,7 +404,7 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource, autoR if !autoRegister { if isAttributeSSHPublicKeySet && synchronizeLdapSSHPublicKeys(user, source, sr.SSHPublicKey) { - RewriteAllPublicKeys() + return user, RewriteAllPublicKeys() } return user, nil @@ -435,7 +438,7 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource, autoR err := CreateUser(user) if err == nil && isAttributeSSHPublicKeySet && addLdapSSHPublicKeys(user, source, sr.SSHPublicKey) { - RewriteAllPublicKeys() + err = RewriteAllPublicKeys() } return user, err diff --git a/models/mail.go b/models/mail.go index 6be0df95ba..2bb07607a4 100644 --- a/models/mail.go +++ b/models/mail.go @@ -157,10 +157,13 @@ func composeTplData(subject, body, link string) map[string]interface{} { func composeIssueCommentMessage(issue *Issue, doer *User, content string, comment *Comment, tplName base.TplName, tos []string, info string) *mailer.Message { subject := issue.mailSubject() - issue.LoadRepo() + err := issue.LoadRepo() + if err != nil { + log.Error("LoadRepo: %v", err) + } body := string(markup.RenderByType(markdown.MarkupName, []byte(content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) - data := make(map[string]interface{}, 10) + var data = make(map[string]interface{}, 10) if comment != nil { data = composeTplData(subject, body, issue.HTMLURL()+"#"+comment.HashTag()) } else { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index b95a74c362..e8fb42c492 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -399,7 +399,7 @@ func trimCommitActionAppURLPrefix(x *xorm.Engine) error { return fmt.Errorf("marshal action content[%d]: %v", actID, err) } - if _, err = sess.Id(actID).Update(&Action{ + if _, err = sess.ID(actID).Update(&Action{ Content: string(p), }); err != nil { return fmt.Errorf("update action[%d]: %v", actID, err) @@ -503,7 +503,7 @@ func attachmentRefactor(x *xorm.Engine) error { // Update database first because this is where error happens the most often. for _, attach := range attachments { - if _, err = sess.Id(attach.ID).Update(attach); err != nil { + if _, err = sess.ID(attach.ID).Update(attach); err != nil { return err } @@ -581,7 +581,7 @@ func renamePullRequestFields(x *xorm.Engine) (err error) { if pull.Index == 0 { continue } - if _, err = sess.Id(pull.ID).Update(pull); err != nil { + if _, err = sess.ID(pull.ID).Update(pull); err != nil { return err } } @@ -661,7 +661,7 @@ func generateOrgRandsAndSalt(x *xorm.Engine) (err error) { if org.Salt, err = generate.GetRandomString(10); err != nil { return err } - if _, err = sess.Id(org.ID).Update(org); err != nil { + if _, err = sess.ID(org.ID).Update(org); err != nil { return err } } diff --git a/models/migrations/v27.go b/models/migrations/v27.go index bd115cf937..e87c7ab68f 100644 --- a/models/migrations/v27.go +++ b/models/migrations/v27.go @@ -58,13 +58,13 @@ func convertIntervalToDuration(x *xorm.Engine) (err error) { return fmt.Errorf("Query repositories: %v", err) } for _, mirror := range mirrors { - mirror.Interval = mirror.Interval * time.Hour + mirror.Interval *= time.Hour if mirror.Interval < setting.Mirror.MinInterval { log.Info("Mirror interval less than Mirror.MinInterval, setting default interval: repo id %v", mirror.RepoID) mirror.Interval = setting.Mirror.DefaultInterval } log.Debug("Mirror interval set to %v for repo id %v", mirror.Interval, mirror.RepoID) - _, err := sess.Id(mirror.ID).Cols("interval").Update(mirror) + _, err := sess.ID(mirror.ID).Cols("interval").Update(mirror) if err != nil { return fmt.Errorf("update mirror interval failed: %v", err) } diff --git a/models/migrations/v78.go b/models/migrations/v78.go index 7ca112dbd5..310c479d01 100644 --- a/models/migrations/v78.go +++ b/models/migrations/v78.go @@ -48,6 +48,9 @@ func renameRepoIsBareToIsEmpty(x *xorm.Engine) error { if len(indexes) >= 1 { _, err = sess.Exec("DROP INDEX IDX_repository_is_bare ON repository") + if err != nil { + return fmt.Errorf("Drop index failed: %v", err) + } } } else { _, err = sess.Exec("DROP INDEX IDX_repository_is_bare ON repository") diff --git a/models/migrations/v85.go b/models/migrations/v85.go index 1fe85ac408..d511628b8d 100644 --- a/models/migrations/v85.go +++ b/models/migrations/v85.go @@ -58,6 +58,9 @@ func hashAppToken(x *xorm.Engine) error { if len(indexes) >= 1 { _, err = sess.Exec("DROP INDEX UQE_access_token_sha1 ON access_token") + if err != nil { + return err + } } } else { _, err = sess.Exec("DROP INDEX UQE_access_token_sha1 ON access_token") diff --git a/models/models.go b/models/models.go index c1d4c100d0..5752a8edd6 100644 --- a/models/models.go +++ b/models/models.go @@ -48,6 +48,7 @@ type Engine interface { Join(joinOperator string, tablename interface{}, condition string, args ...interface{}) *xorm.Session SQL(interface{}, ...interface{}) *xorm.Session Where(interface{}, ...interface{}) *xorm.Session + Asc(colNames ...string) *xorm.Session } var ( @@ -181,14 +182,14 @@ func parsePostgreSQLHostPort(info string) (string, string) { return host, port } -func getPostgreSQLConnectionString(DBHost, DBUser, DBPasswd, DBName, DBParam, DBSSLMode string) (connStr string) { - host, port := parsePostgreSQLHostPort(DBHost) +func getPostgreSQLConnectionString(dbHost, dbUser, dbPasswd, dbName, dbParam, dbsslMode string) (connStr string) { + host, port := parsePostgreSQLHostPort(dbHost) if host[0] == '/' { // looks like a unix socket connStr = fmt.Sprintf("postgres://%s:%s@:%s/%s%ssslmode=%s&host=%s", - url.PathEscape(DBUser), url.PathEscape(DBPasswd), port, DBName, DBParam, DBSSLMode, host) + url.PathEscape(dbUser), url.PathEscape(dbPasswd), port, dbName, dbParam, dbsslMode, host) } else { connStr = fmt.Sprintf("postgres://%s:%s@%s:%s/%s%ssslmode=%s", - url.PathEscape(DBUser), url.PathEscape(DBPasswd), host, port, DBName, DBParam, DBSSLMode) + url.PathEscape(dbUser), url.PathEscape(dbPasswd), host, port, dbName, dbParam, dbsslMode) } return } diff --git a/models/notification.go b/models/notification.go index cda2916fae..f83fe63e5a 100644 --- a/models/notification.go +++ b/models/notification.go @@ -119,7 +119,10 @@ func createOrUpdateIssueNotifications(e Engine, issue *Issue, notificationAuthor } } - issue.loadRepo(e) + err = issue.loadRepo(e) + if err != nil { + return err + } for _, watch := range watches { issue.Repo.Units = nil diff --git a/models/oauth2.go b/models/oauth2.go index 10bce31924..bf4446229a 100644 --- a/models/oauth2.go +++ b/models/oauth2.go @@ -106,7 +106,10 @@ func InitOAuth2() error { for _, source := range loginSources { oAuth2Config := source.OAuth2() - oauth2.RegisterProvider(source.Name, oAuth2Config.Provider, oAuth2Config.ClientID, oAuth2Config.ClientSecret, oAuth2Config.OpenIDConnectAutoDiscoveryURL, oAuth2Config.CustomURLMapping) + err := oauth2.RegisterProvider(source.Name, oAuth2Config.Provider, oAuth2Config.ClientID, oAuth2Config.ClientSecret, oAuth2Config.OpenIDConnectAutoDiscoveryURL, oAuth2Config.CustomURLMapping) + if err != nil { + return err + } } return nil } diff --git a/models/oauth2_application.go b/models/oauth2_application.go index 1e69dd6430..63d2e7ce5e 100644 --- a/models/oauth2_application.go +++ b/models/oauth2_application.go @@ -142,6 +142,9 @@ func GetOAuth2ApplicationByID(id int64) (app *OAuth2Application, err error) { func getOAuth2ApplicationByID(e Engine, id int64) (app *OAuth2Application, err error) { app = new(OAuth2Application) has, err := e.ID(id).Get(app) + if err != nil { + return nil, err + } if !has { return nil, ErrOAuthApplicationNotFound{ID: id} } @@ -295,10 +298,10 @@ func (code *OAuth2AuthorizationCode) invalidate(e Engine) error { // ValidateCodeChallenge validates the given verifier against the saved code challenge. This is part of the PKCE implementation. func (code *OAuth2AuthorizationCode) ValidateCodeChallenge(verifier string) bool { - return code.validateCodeChallenge(x, verifier) + return code.validateCodeChallenge(verifier) } -func (code *OAuth2AuthorizationCode) validateCodeChallenge(e Engine, verifier string) bool { +func (code *OAuth2AuthorizationCode) validateCodeChallenge(verifier string) bool { switch code.CodeChallengeMethod { case "S256": // base64url(SHA256(verifier)) see https://tools.ietf.org/html/rfc7636#section-4.6 diff --git a/models/org.go b/models/org.go index 6511072e2b..65002eadff 100644 --- a/models/org.go +++ b/models/org.go @@ -172,7 +172,9 @@ func CreateOrganization(org, owner *User) (err error) { } if _, err = sess.Insert(&units); err != nil { - sess.Rollback() + if err := sess.Rollback(); err != nil { + log.Error("CreateOrganization: sess.Rollback: %v", err) + } return err } @@ -376,10 +378,7 @@ func HasOrgVisible(org *User, user *User) bool { func hasOrgVisible(e Engine, org *User, user *User) bool { // Not SignedUser if user == nil { - if org.Visibility == structs.VisibleTypePublic { - return true - } - return false + return org.Visibility == structs.VisibleTypePublic } if user.IsAdmin { @@ -485,10 +484,14 @@ func AddOrgUser(orgID, uid int64) error { } if _, err := sess.Insert(ou); err != nil { - sess.Rollback() + if err := sess.Rollback(); err != nil { + log.Error("AddOrgUser: sess.Rollback: %v", err) + } return err } else if _, err = sess.Exec("UPDATE `user` SET num_members = num_members + 1 WHERE id = ?", orgID); err != nil { - sess.Rollback() + if err := sess.Rollback(); err != nil { + log.Error("AddOrgUser: sess.Rollback: %v", err) + } return err } diff --git a/models/org_team.go b/models/org_team.go index 49d06896e5..dcf0743740 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -287,7 +287,8 @@ func NewTeam(t *Team) (err error) { has, err := x.ID(t.OrgID).Get(new(User)) if err != nil { return err - } else if !has { + } + if !has { return ErrOrgNotExist{t.OrgID, ""} } @@ -298,7 +299,8 @@ func NewTeam(t *Team) (err error) { Get(new(Team)) if err != nil { return err - } else if has { + } + if has { return ErrTeamAlreadyExist{t.OrgID, t.LowerName} } @@ -309,7 +311,10 @@ func NewTeam(t *Team) (err error) { } if _, err = sess.Insert(t); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("NewTeam sess.Rollback: %v", errRollback) + } return err } @@ -319,14 +324,20 @@ func NewTeam(t *Team) (err error) { unit.TeamID = t.ID } if _, err = sess.Insert(&t.Units); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("NewTeam sess.Rollback: %v", errRollback) + } return err } } // Update organization number of teams. if _, err = sess.Exec("UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("NewTeam sess.Rollback: %v", errRollback) + } return err } return sess.Commit() @@ -412,7 +423,10 @@ func UpdateTeam(t *Team, authChanged bool) (err error) { } if _, err = sess.Insert(&t.Units); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("UpdateTeam sess.Rollback: %v", errRollback) + } return err } } @@ -841,7 +855,10 @@ func UpdateTeamUnits(team *Team, units []TeamUnit) (err error) { } if _, err = sess.Insert(units); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("UpdateTeamUnits sess.Rollback: %v", errRollback) + } return err } diff --git a/models/org_test.go b/models/org_test.go index b484208be1..a2ebf1f60b 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -242,10 +242,10 @@ func TestGetOrgByName(t *testing.T) { assert.EqualValues(t, 3, org.ID) assert.Equal(t, "user3", org.Name) - org, err = GetOrgByName("user2") // user2 is an individual + _, err = GetOrgByName("user2") // user2 is an individual assert.True(t, IsErrOrgNotExist(err)) - org, err = GetOrgByName("") // corner case + _, err = GetOrgByName("") // corner case assert.True(t, IsErrOrgNotExist(err)) } @@ -499,7 +499,7 @@ func TestAccessibleReposEnv_CountRepos(t *testing.T) { func TestAccessibleReposEnv_RepoIDs(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) - testSuccess := func(userID, page, pageSize int64, expectedRepoIDs []int64) { + testSuccess := func(userID, _, pageSize int64, expectedRepoIDs []int64) { env, err := org.AccessibleReposEnv(userID) assert.NoError(t, err) repoIDs, err := env.RepoIDs(1, 100) diff --git a/models/pull.go b/models/pull.go index 1f03dd9b0f..7a168181e2 100644 --- a/models/pull.go +++ b/models/pull.go @@ -192,15 +192,19 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest { } } if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { + log.Error("pr.BaseRepo.GetBranch[%d]: %v", pr.BaseBranch, err) return nil } if baseCommit, err = baseBranch.GetCommit(); err != nil { + log.Error("baseBranch.GetCommit[%d]: %v", pr.ID, err) return nil } if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil { + log.Error("pr.HeadRepo.GetBranch[%d]: %v", pr.HeadBranch, err) return nil } if headCommit, err = headBranch.GetCommit(); err != nil { + log.Error("headBranch.GetCommit[%d]: %v", pr.ID, err) return nil } apiBaseBranchInfo := &api.PRBranchInfo{ @@ -218,7 +222,10 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest { Repository: pr.HeadRepo.innerAPIFormat(e, AccessModeNone, false), } - pr.Issue.loadRepo(e) + if err = pr.Issue.loadRepo(e); err != nil { + log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err) + return nil + } apiPullRequest := &api.PullRequest{ ID: pr.ID, @@ -420,7 +427,11 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle if err != nil { return err } - defer RemoveTemporaryPath(tmpBasePath) + defer func() { + if err := RemoveTemporaryPath(tmpBasePath); err != nil { + log.Error("Merge: RemoveTemporaryPath: %s", err) + } + }() headRepoPath := RepoPath(pr.HeadUserName, pr.HeadRepo.Name) @@ -1142,7 +1153,9 @@ func (pr *PullRequest) UpdatePatch() (err error) { return fmt.Errorf("AddRemote: %v", err) } defer func() { - headGitRepo.RemoveRemote(tmpRemote) + if err := headGitRepo.RemoveRemote(tmpRemote); err != nil { + log.Error("UpdatePatch: RemoveRemote: %s", err) + } }() pr.MergeBase, _, err = headGitRepo.GetMergeBase(tmpRemote, pr.BaseBranch, pr.HeadBranch) if err != nil { @@ -1180,7 +1193,11 @@ func (pr *PullRequest) PushToBaseRepo() (err error) { return fmt.Errorf("headGitRepo.AddRemote: %v", err) } // Make sure to remove the remote even if the push fails - defer headGitRepo.RemoveRemote(tmpRemoteName) + defer func() { + if err := headGitRepo.RemoveRemote(tmpRemoteName); err != nil { + log.Error("PushToBaseRepo: RemoveRemote: %s", err) + } + }() headFile := pr.GetGitRefName() diff --git a/models/pull_test.go b/models/pull_test.go index 1dad664077..5a53474ac4 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -94,7 +94,7 @@ func TestGetUnmergedPullRequest(t *testing.T) { assert.NoError(t, err) assert.Equal(t, int64(2), pr.ID) - pr, err = GetUnmergedPullRequest(1, 9223372036854775807, "branch1", "master") + _, err = GetUnmergedPullRequest(1, 9223372036854775807, "branch1", "master") assert.Error(t, err) assert.True(t, IsErrPullRequestNotExist(err)) } @@ -128,7 +128,7 @@ func TestGetPullRequestByIndex(t *testing.T) { assert.Equal(t, int64(1), pr.BaseRepoID) assert.Equal(t, int64(2), pr.Index) - pr, err = GetPullRequestByIndex(9223372036854775807, 9223372036854775807) + _, err = GetPullRequestByIndex(9223372036854775807, 9223372036854775807) assert.Error(t, err) assert.True(t, IsErrPullRequestNotExist(err)) } @@ -151,7 +151,7 @@ func TestGetPullRequestByIssueID(t *testing.T) { assert.NoError(t, err) assert.Equal(t, int64(2), pr.IssueID) - pr, err = GetPullRequestByIssueID(9223372036854775807) + _, err = GetPullRequestByIssueID(9223372036854775807) assert.Error(t, err) assert.True(t, IsErrPullRequestNotExist(err)) } diff --git a/models/release.go b/models/release.go index b7ec4461f2..28a2891013 100644 --- a/models/release.go +++ b/models/release.go @@ -50,12 +50,12 @@ func (r *Release) loadAttributes(e Engine) error { } } if r.Publisher == nil { - r.Publisher, err = GetUserByID(r.PublisherID) + r.Publisher, err = getUserByID(e, r.PublisherID) if err != nil { return err } } - return GetReleaseAttachments(r) + return getReleaseAttachments(e, r) } // LoadAttributes load repo and publisher attributes for a release @@ -316,6 +316,10 @@ func (s releaseMetaSearch) Less(i, j int) bool { // GetReleaseAttachments retrieves the attachments for releases func GetReleaseAttachments(rels ...*Release) (err error) { + return getReleaseAttachments(x, rels...) +} + +func getReleaseAttachments(e Engine, rels ...*Release) (err error) { if len(rels) == 0 { return } @@ -335,11 +339,10 @@ func GetReleaseAttachments(rels ...*Release) (err error) { sort.Sort(sortedRels) // Select attachments - err = x. + err = e. Asc("release_id"). In("release_id", sortedRels.ID). Find(&attachments, Attachment{}) - if err != nil { return err } @@ -354,7 +357,6 @@ func GetReleaseAttachments(rels ...*Release) (err error) { } return - } type releaseSorter struct { @@ -493,7 +495,7 @@ func SyncReleasesWithTags(repo *Repository, gitRepo *git.Repository) error { return fmt.Errorf("GetTagCommitID: %v", err) } if git.IsErrNotExist(err) || commitID != rel.Sha1 { - if err := pushUpdateDeleteTag(repo, gitRepo, rel.TagName); err != nil { + if err := pushUpdateDeleteTag(repo, rel.TagName); err != nil { return fmt.Errorf("pushUpdateDeleteTag: %v", err) } } else { diff --git a/models/repo.go b/models/repo.go index a855c84939..a4a7521aa4 100644 --- a/models/repo.go +++ b/models/repo.go @@ -20,7 +20,6 @@ import ( "os" "path" "path/filepath" - "regexp" "sort" "strconv" "strings" @@ -744,10 +743,6 @@ func (repo *Repository) getUsersWithAccessMode(e Engine, mode AccessMode) (_ []* return users, nil } -var ( - descPattern = regexp.MustCompile(`https?://\S+`) -) - // DescriptionHTML does special handles to description and return HTML string. func (repo *Repository) DescriptionHTML() template.HTML { desc, err := markup.RenderDescriptionHTML([]byte(repo.Description), repo.HTMLURL(), repo.ComposeMetas()) @@ -1333,11 +1328,9 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err return fmt.Errorf("prepareWebhooks: %v", err) } go HookQueue.Add(repo.ID) - } else { + } else if err = repo.recalculateAccesses(e); err != nil { // Organization automatically called this in addRepository method. - if err = repo.recalculateAccesses(e); err != nil { - return fmt.Errorf("recalculateAccesses: %v", err) - } + return fmt.Errorf("recalculateAccesses: %v", err) } if setting.Service.AutoWatchNewRepos { @@ -1512,11 +1505,9 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error } else if err = t.addRepository(sess, repo); err != nil { return fmt.Errorf("add to owner team: %v", err) } - } else { + } else if err = repo.recalculateAccesses(sess); err != nil { // Organization called this in addRepository method. - if err = repo.recalculateAccesses(sess); err != nil { - return fmt.Errorf("recalculateAccesses: %v", err) - } + return fmt.Errorf("recalculateAccesses: %v", err) } // Update repository count. @@ -1864,7 +1855,10 @@ func DeleteRepository(doer *User, uid, repoID int64) error { repoPath := repo.repoPath(sess) removeAllWithNotice(sess, "Delete repository files", repoPath) - repo.deleteWiki(sess) + err = repo.deleteWiki(sess) + if err != nil { + return err + } // Remove attachment files. for i := range attachmentPaths { @@ -2522,7 +2516,7 @@ func (repo *Repository) GetUserFork(userID int64) (*Repository, error) { // CustomAvatarPath returns repository custom avatar file path. func (repo *Repository) CustomAvatarPath() string { // Avatar empty by default - if len(repo.Avatar) <= 0 { + if len(repo.Avatar) == 0 { return "" } return filepath.Join(setting.RepositoryAvatarUploadPath, repo.Avatar) @@ -2562,10 +2556,7 @@ func (repo *Repository) generateRandomAvatar(e Engine) error { // RemoveRandomAvatars removes the randomly generated avatars that were created for repositories func RemoveRandomAvatars() error { - var ( - err error - ) - err = x. + return x. Where("id > 0").BufferSize(setting.IterateBufferSize). Iterate(new(Repository), func(idx int, bean interface{}) error { @@ -2576,7 +2567,6 @@ func RemoveRandomAvatars() error { } return nil }) - return err } // RelAvatarLink returns a relative link to the repository's avatar. @@ -2587,7 +2577,7 @@ func (repo *Repository) RelAvatarLink() string { func (repo *Repository) relAvatarLink(e Engine) string { // If no avatar - path is empty avatarPath := repo.CustomAvatarPath() - if len(avatarPath) <= 0 || !com.IsFile(avatarPath) { + if len(avatarPath) == 0 || !com.IsFile(avatarPath) { switch mode := setting.RepositoryAvatarFallback; mode { case "image": return setting.RepositoryAvatarFallbackImage diff --git a/models/repo_activity.go b/models/repo_activity.go index fb1385a54b..04612ae1ef 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -114,7 +114,7 @@ func GetActivityStatsTopAuthors(repo *Repository, timeFrom time.Time, count int) v = append(v, u) } - sort.Slice(v[:], func(i, j int) bool { + sort.Slice(v, func(i, j int) bool { return v[i].Commits < v[j].Commits }) diff --git a/models/repo_branch.go b/models/repo_branch.go index 08c881fc24..dee6ef3d7e 100644 --- a/models/repo_branch.go +++ b/models/repo_branch.go @@ -75,7 +75,11 @@ func (repo *Repository) CreateNewBranch(doer *User, oldBranchName, branchName st if err != nil { return err } - defer RemoveTemporaryPath(basePath) + defer func() { + if err := RemoveTemporaryPath(basePath); err != nil { + log.Error("CreateNewBranch: RemoveTemporaryPath: %s", err) + } + }() if err := git.Clone(repo.RepoPath(), basePath, git.CloneRepoOptions{ Bare: true, @@ -117,7 +121,11 @@ func (repo *Repository) CreateNewBranchFromCommit(doer *User, commit, branchName if err != nil { return err } - defer RemoveTemporaryPath(basePath) + defer func() { + if err := RemoveTemporaryPath(basePath); err != nil { + log.Error("CreateNewBranchFromCommit: RemoveTemporaryPath: %s", err) + } + }() if err := git.Clone(repo.RepoPath(), basePath, git.CloneRepoOptions{ Bare: true, diff --git a/models/repo_collaboration.go b/models/repo_collaboration.go index 9d2935d581..0797f50430 100644 --- a/models/repo_collaboration.go +++ b/models/repo_collaboration.go @@ -142,7 +142,7 @@ func (repo *Repository) ChangeCollaborationAccessMode(uid int64, mode AccessMode } if _, err = sess. - Id(collaboration.ID). + ID(collaboration.ID). Cols("mode"). Update(collaboration); err != nil { return fmt.Errorf("update collaboration: %v", err) diff --git a/models/repo_list.go b/models/repo_list.go index 9686676eae..5655404f7c 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -148,19 +148,19 @@ func (s SearchOrderBy) String() string { // Strings for sorting result const ( SearchOrderByAlphabetically SearchOrderBy = "name ASC" - SearchOrderByAlphabeticallyReverse = "name DESC" - SearchOrderByLeastUpdated = "updated_unix ASC" - SearchOrderByRecentUpdated = "updated_unix DESC" - SearchOrderByOldest = "created_unix ASC" - SearchOrderByNewest = "created_unix DESC" - SearchOrderBySize = "size ASC" - SearchOrderBySizeReverse = "size DESC" - SearchOrderByID = "id ASC" - SearchOrderByIDReverse = "id DESC" - SearchOrderByStars = "num_stars ASC" - SearchOrderByStarsReverse = "num_stars DESC" - SearchOrderByForks = "num_forks ASC" - SearchOrderByForksReverse = "num_forks DESC" + SearchOrderByAlphabeticallyReverse SearchOrderBy = "name DESC" + SearchOrderByLeastUpdated SearchOrderBy = "updated_unix ASC" + SearchOrderByRecentUpdated SearchOrderBy = "updated_unix DESC" + SearchOrderByOldest SearchOrderBy = "created_unix ASC" + SearchOrderByNewest SearchOrderBy = "created_unix DESC" + SearchOrderBySize SearchOrderBy = "size ASC" + SearchOrderBySizeReverse SearchOrderBy = "size DESC" + SearchOrderByID SearchOrderBy = "id ASC" + SearchOrderByIDReverse SearchOrderBy = "id DESC" + SearchOrderByStars SearchOrderBy = "num_stars ASC" + SearchOrderByStarsReverse SearchOrderBy = "num_stars DESC" + SearchOrderByForks SearchOrderBy = "num_forks ASC" + SearchOrderByForksReverse SearchOrderBy = "num_forks DESC" ) // SearchRepositoryByName takes keyword and part of repository name to search, diff --git a/models/repo_redirect.go b/models/repo_redirect.go index 813b3e6c9e..8847a0889c 100644 --- a/models/repo_redirect.go +++ b/models/repo_redirect.go @@ -4,7 +4,10 @@ package models -import "strings" +import ( + "code.gitea.io/gitea/modules/log" + "strings" +) // RepoRedirect represents that a repo name should be redirected to another type RepoRedirect struct { @@ -38,7 +41,10 @@ func NewRepoRedirect(ownerID, repoID int64, oldRepoName, newRepoName string) err } if err := deleteRepoRedirect(sess, ownerID, newRepoName); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("NewRepoRedirect sess.Rollback: %v", errRollback) + } return err } @@ -47,7 +53,10 @@ func NewRepoRedirect(ownerID, repoID int64, oldRepoName, newRepoName string) err LowerName: oldRepoName, RedirectRepoID: repoID, }); err != nil { - sess.Rollback() + errRollback := sess.Rollback() + if errRollback != nil { + log.Error("NewRepoRedirect sess.Rollback: %v", errRollback) + } return err } return sess.Commit() diff --git a/models/ssh_key.go b/models/ssh_key.go index fb5f9f399b..1f2288b13e 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -142,7 +142,7 @@ func parseKeyString(content string) (string, error) { if continuationLine || strings.ContainsAny(line, ":-") { continuationLine = strings.HasSuffix(line, "\\") } else { - keyContent = keyContent + line + keyContent += line } } @@ -392,7 +392,7 @@ func addKey(e Engine, key *PublicKey) (err error) { } // AddPublicKey adds new public key to database and authorized_keys file. -func AddPublicKey(ownerID int64, name, content string, LoginSourceID int64) (*PublicKey, error) { +func AddPublicKey(ownerID int64, name, content string, loginSourceID int64) (*PublicKey, error) { log.Trace(content) fingerprint, err := calcFingerprint(content) @@ -427,7 +427,7 @@ func AddPublicKey(ownerID int64, name, content string, LoginSourceID int64) (*Pu Content: content, Mode: AccessModeWrite, Type: KeyTypeUser, - LoginSourceID: LoginSourceID, + LoginSourceID: loginSourceID, } if err = addKey(sess, key); err != nil { return nil, fmt.Errorf("addKey: %v", err) @@ -491,10 +491,10 @@ func ListPublicKeys(uid int64) ([]*PublicKey, error) { } // ListPublicLdapSSHKeys returns a list of synchronized public ldap ssh keys belongs to given user and login source. -func ListPublicLdapSSHKeys(uid int64, LoginSourceID int64) ([]*PublicKey, error) { +func ListPublicLdapSSHKeys(uid int64, loginSourceID int64) ([]*PublicKey, error) { keys := make([]*PublicKey, 0, 5) return keys, x. - Where("owner_id = ? AND login_source_id = ?", uid, LoginSourceID). + Where("owner_id = ? AND login_source_id = ?", uid, loginSourceID). Find(&keys) } diff --git a/models/status.go b/models/status.go index a3db47f455..384f5693dc 100644 --- a/models/status.go +++ b/models/status.go @@ -87,7 +87,7 @@ func (status *CommitStatus) loadRepo(e Engine) (err error) { // APIURL returns the absolute APIURL to this commit-status. func (status *CommitStatus) APIURL() string { - status.loadRepo(x) + _ = status.loadRepo(x) return fmt.Sprintf("%sapi/v1/%s/statuses/%s", setting.AppURL, status.Repo.FullName(), status.SHA) } @@ -95,7 +95,7 @@ func (status *CommitStatus) APIURL() string { // APIFormat assumes some fields assigned with values: // Required - Repo, Creator func (status *CommitStatus) APIFormat() *api.Status { - status.loadRepo(x) + _ = status.loadRepo(x) apiStatus := &api.Status{ Created: status.CreatedUnix.AsTime(), Updated: status.CreatedUnix.AsTime(), @@ -219,7 +219,9 @@ func newCommitStatus(sess *xorm.Session, opts NewCommitStatusOptions) error { } has, err := sess.Desc("index").Limit(1).Get(lastCommitStatus) if err != nil { - sess.Rollback() + if err := sess.Rollback(); err != nil { + log.Error("newCommitStatus: sess.Rollback: %v", err) + } return fmt.Errorf("newCommitStatus[%s, %s]: %v", repoPath, opts.SHA, err) } if has { @@ -231,7 +233,9 @@ func newCommitStatus(sess *xorm.Session, opts NewCommitStatusOptions) error { // Insert new CommitStatus if _, err = sess.Insert(opts.CommitStatus); err != nil { - sess.Rollback() + if err := sess.Rollback(); err != nil { + log.Error("newCommitStatus: sess.Rollback: %v", err) + } return fmt.Errorf("newCommitStatus[%s, %s]: %v", repoPath, opts.SHA, err) } diff --git a/models/token_test.go b/models/token_test.go index 9f2699a168..a74de8f818 100644 --- a/models/token_test.go +++ b/models/token_test.go @@ -36,11 +36,11 @@ func TestGetAccessTokenBySHA(t *testing.T) { assert.Equal(t, "2b3668e11cb82d3af8c6e4524fc7841297668f5008d1626f0ad3417e9fa39af84c268248b78c481daa7e5dc437784003494f", token.TokenHash) assert.Equal(t, "e4efbf36", token.TokenLastEight) - token, err = GetAccessTokenBySHA("notahash") + _, err = GetAccessTokenBySHA("notahash") assert.Error(t, err) assert.True(t, IsErrAccessTokenNotExist(err)) - token, err = GetAccessTokenBySHA("") + _, err = GetAccessTokenBySHA("") assert.Error(t, err) assert.True(t, IsErrAccessTokenEmpty(err)) } diff --git a/models/update.go b/models/update.go index 0883cb0e01..3eb0990d3d 100644 --- a/models/update.go +++ b/models/update.go @@ -84,7 +84,7 @@ func PushUpdate(branch string, opt PushUpdateOptions) error { return nil } -func pushUpdateDeleteTag(repo *Repository, gitRepo *git.Repository, tagName string) error { +func pushUpdateDeleteTag(repo *Repository, tagName string) error { rel, err := GetRelease(repo.ID, tagName) if err != nil { if IsErrReleaseNotExist(err) { @@ -223,7 +223,7 @@ func pushUpdate(opts PushUpdateOptions) (repo *Repository, err error) { // If is tag reference tagName := opts.RefFullName[len(git.TagPrefix):] if isDelRef { - err = pushUpdateDeleteTag(repo, gitRepo, tagName) + err = pushUpdateDeleteTag(repo, tagName) if err != nil { return nil, fmt.Errorf("pushUpdateDeleteTag: %v", err) } diff --git a/models/user.go b/models/user.go index e29cf5b32a..2820d2edbc 100644 --- a/models/user.go +++ b/models/user.go @@ -1072,7 +1072,10 @@ func deleteUser(e *xorm.Session, u *User) error { if _, err = e.Delete(&PublicKey{OwnerID: u.ID}); err != nil { return fmt.Errorf("deletePublicKeys: %v", err) } - rewriteAllPublicKeys(e) + err = rewriteAllPublicKeys(e) + if err != nil { + return err + } // ***** END: PublicKey ***** // ***** START: GPGPublicKey ***** @@ -1401,8 +1404,7 @@ func (opts *SearchUserOptions) toConds() builder.Cond { } else { exprCond = builder.Expr("org_user.org_id = \"user\".id") } - var accessCond = builder.NewCond() - accessCond = builder.Or( + accessCond := builder.Or( builder.In("id", builder.Select("org_id").From("org_user").LeftJoin("`user`", exprCond).Where(builder.And(builder.Eq{"uid": opts.OwnerID}, builder.Eq{"visibility": structs.VisibleTypePrivate}))), builder.In("visibility", structs.VisibleTypePublic, structs.VisibleTypeLimited)) cond = cond.And(accessCond) @@ -1512,9 +1514,9 @@ func deleteKeysMarkedForDeletion(keys []string) (bool, error) { } // addLdapSSHPublicKeys add a users public keys. Returns true if there are changes. -func addLdapSSHPublicKeys(usr *User, s *LoginSource, SSHPublicKeys []string) bool { +func addLdapSSHPublicKeys(usr *User, s *LoginSource, sshPublicKeys []string) bool { var sshKeysNeedUpdate bool - for _, sshKey := range SSHPublicKeys { + for _, sshKey := range sshPublicKeys { _, _, _, _, err := ssh.ParseAuthorizedKey([]byte(sshKey)) if err == nil { sshKeyName := fmt.Sprintf("%s-%s", s.Name, sshKey[0:40]) @@ -1536,7 +1538,7 @@ func addLdapSSHPublicKeys(usr *User, s *LoginSource, SSHPublicKeys []string) boo } // synchronizeLdapSSHPublicKeys updates a users public keys. Returns true if there are changes. -func synchronizeLdapSSHPublicKeys(usr *User, s *LoginSource, SSHPublicKeys []string) bool { +func synchronizeLdapSSHPublicKeys(usr *User, s *LoginSource, sshPublicKeys []string) bool { var sshKeysNeedUpdate bool log.Trace("synchronizeLdapSSHPublicKeys[%s]: Handling LDAP Public SSH Key synchronization for user %s", s.Name, usr.Name) @@ -1554,7 +1556,7 @@ func synchronizeLdapSSHPublicKeys(usr *User, s *LoginSource, SSHPublicKeys []str // Get Public Keys from LDAP and skip duplicate keys var ldapKeys []string - for _, v := range SSHPublicKeys { + for _, v := range sshPublicKeys { sshKeySplit := strings.Split(v, " ") if len(sshKeySplit) > 1 { ldapKey := strings.Join(sshKeySplit[:2], " ") @@ -1634,9 +1636,13 @@ func SyncExternalUsers() { // Find all users with this login type var users []*User - x.Where("login_type = ?", LoginLDAP). + err = x.Where("login_type = ?", LoginLDAP). And("login_source = ?", s.ID). Find(&users) + if err != nil { + log.Error("SyncExternalUsers: %v", err) + return + } sr := s.LDAP().SearchEntries() for _, su := range sr { @@ -1694,7 +1700,7 @@ func SyncExternalUsers() { // Check if user data has changed if (len(s.LDAP().AdminFilter) > 0 && usr.IsAdmin != su.IsAdmin) || - strings.ToLower(usr.Email) != strings.ToLower(su.Mail) || + !strings.EqualFold(usr.Email, su.Mail) || usr.FullName != fullName || !usr.IsActive { @@ -1718,7 +1724,10 @@ func SyncExternalUsers() { // Rewrite authorized_keys file if LDAP Public SSH Key attribute is set and any key was added or removed if sshKeysNeedUpdate { - RewriteAllPublicKeys() + err = RewriteAllPublicKeys() + if err != nil { + log.Error("RewriteAllPublicKeys: %v", err) + } } // Deactivate users not present in LDAP diff --git a/models/user_mail.go b/models/user_mail.go index 39d1070c35..d929ba5a5d 100644 --- a/models/user_mail.go +++ b/models/user_mail.go @@ -134,7 +134,7 @@ func (email *EmailAddress) Activate() error { email.IsActivated = true if _, err := sess. - Id(email.ID). + ID(email.ID). Cols("is_activated"). Update(email); err != nil { return err diff --git a/models/user_openid_test.go b/models/user_openid_test.go index 711f92b9ff..18f84bef76 100644 --- a/models/user_openid_test.go +++ b/models/user_openid_test.go @@ -31,12 +31,12 @@ func TestGetUserOpenIDs(t *testing.T) { func TestGetUserByOpenID(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - user, err := GetUserByOpenID("https://unknown") + _, err := GetUserByOpenID("https://unknown") if assert.Error(t, err) { assert.True(t, IsErrUserNotExist(err)) } - user, err = GetUserByOpenID("https://user1.domain1.tld") + user, err := GetUserByOpenID("https://user1.domain1.tld") if assert.NoError(t, err) { assert.Equal(t, user.ID, int64(1)) } diff --git a/models/webhook.go b/models/webhook.go index 7a28e37958..e3e11e5963 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -700,7 +700,10 @@ func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType, log.Error("prepareWebhooks.JSONPayload: %v", err) } sig := hmac.New(sha256.New, []byte(w.Secret)) - sig.Write(data) + _, err = sig.Write(data) + if err != nil { + log.Error("prepareWebhooks.sigWrite: %v", err) + } signature = hex.EncodeToString(sig.Sum(nil)) } @@ -930,8 +933,7 @@ func InitDeliverHooks() { return nil, err } - conn.SetDeadline(time.Now().Add(timeout)) - return conn, nil + return conn, conn.SetDeadline(time.Now().Add(timeout)) }, }, diff --git a/models/webhook_discord.go b/models/webhook_discord.go index 0029e94fca..d7a2de0d11 100644 --- a/models/webhook_discord.go +++ b/models/webhook_discord.go @@ -490,7 +490,7 @@ func getDiscordReleasePayload(p *api.ReleasePayload, meta *DiscordMeta) (*Discor Embeds: []DiscordEmbed{ { Title: title, - Description: fmt.Sprintf("%s", p.Release.Note), + Description: p.Release.Note, URL: url, Color: color, Author: DiscordEmbedAuthor{ diff --git a/models/wiki.go b/models/wiki.go index bcf97c0765..9ae3386333 100644 --- a/models/wiki.go +++ b/models/wiki.go @@ -115,7 +115,11 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiName, newWikiName, con if err != nil { return err } - defer RemoveTemporaryPath(basePath) + defer func() { + if err := RemoveTemporaryPath(basePath); err != nil { + log.Error("Merge: RemoveTemporaryPath: %s", err) + } + }() cloneOpts := git.CloneRepoOptions{ Bare: true, @@ -246,7 +250,11 @@ func (repo *Repository) DeleteWikiPage(doer *User, wikiName string) (err error) if err != nil { return err } - defer RemoveTemporaryPath(basePath) + defer func() { + if err := RemoveTemporaryPath(basePath); err != nil { + log.Error("Merge: RemoveTemporaryPath: %s", err) + } + }() if err := git.Clone(repo.WikiPath(), basePath, git.CloneRepoOptions{ Bare: true, diff --git a/modules/auth/auth.go b/modules/auth/auth.go index edb596c240..2a2ee40492 100644 --- a/modules/auth/auth.go +++ b/modules/auth/auth.go @@ -214,10 +214,8 @@ func SignedInUser(ctx *macaron.Context, sess session.Store) (*models.User, bool) if err = models.UpdateAccessToken(token); err != nil { log.Error("UpdateAccessToken: %v", err) } - } else { - if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { - log.Error("GetAccessTokenBySha: %v", err) - } + } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { + log.Error("GetAccessTokenBySha: %v", err) } if u == nil { @@ -301,12 +299,6 @@ func GetInclude(field reflect.StructField) string { return getRuleBody(field, "Include(") } -// FIXME: struct contains a struct -func validateStruct(obj interface{}) binding.Errors { - - return nil -} - func validate(errs binding.Errors, data map[string]interface{}, f Form, l macaron.Locale) binding.Errors { if errs.Len() == 0 { return errs diff --git a/modules/auth/oauth2/oauth2.go b/modules/auth/oauth2/oauth2.go index 5684f44a89..a2d7116211 100644 --- a/modules/auth/oauth2/oauth2.go +++ b/modules/auth/oauth2/oauth2.go @@ -220,8 +220,7 @@ func GetDefaultProfileURL(provider string) string { // GetDefaultEmailURL return the default email url for the given provider func GetDefaultEmailURL(provider string) string { - switch provider { - case "github": + if provider == "github" { return github.EmailURL } return "" diff --git a/modules/auth/openid/discovery_cache_test.go b/modules/auth/openid/discovery_cache_test.go index 2e37058cc3..931e5c7945 100644 --- a/modules/auth/openid/discovery_cache_test.go +++ b/modules/auth/openid/discovery_cache_test.go @@ -39,7 +39,7 @@ func TestTimedDiscoveryCache(t *testing.T) { t.Errorf("Expected nil, got %v", di) } - // Sleep one second and try retrive again + // Sleep one second and try retrieve again time.Sleep(1 * time.Second) if di := dc.Get("foo"); di != nil { diff --git a/modules/auth/user_form.go b/modules/auth/user_form.go index 8b9e5877d9..0c8bd30abc 100644 --- a/modules/auth/user_form.go +++ b/modules/auth/user_form.go @@ -253,7 +253,7 @@ func (f UpdateThemeForm) IsThemeExists() bool { var exists bool for _, v := range setting.UI.Themes { - if strings.ToLower(v) == strings.ToLower(f.Theme) { + if strings.EqualFold(v, f.Theme) { exists = true break } diff --git a/modules/base/tool.go b/modules/base/tool.go index dcf9155a07..4893abff71 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -44,21 +44,21 @@ var UTF8BOM = []byte{'\xef', '\xbb', '\xbf'} // EncodeMD5 encodes string to md5 hex value. func EncodeMD5(str string) string { m := md5.New() - m.Write([]byte(str)) + _, _ = m.Write([]byte(str)) return hex.EncodeToString(m.Sum(nil)) } // EncodeSha1 string to sha1 hex value. func EncodeSha1(str string) string { h := sha1.New() - h.Write([]byte(str)) + _, _ = h.Write([]byte(str)) return hex.EncodeToString(h.Sum(nil)) } // EncodeSha256 string to sha1 hex value. func EncodeSha256(str string) string { h := sha256.New() - h.Write([]byte(str)) + _, _ = h.Write([]byte(str)) return hex.EncodeToString(h.Sum(nil)) } @@ -193,7 +193,7 @@ func CreateTimeLimitCode(data string, minutes int, startInf interface{}) string // create sha1 encode string sh := sha1.New() - sh.Write([]byte(data + setting.SecretKey + startStr + endStr + com.ToStr(minutes))) + _, _ = sh.Write([]byte(data + setting.SecretKey + startStr + endStr + com.ToStr(minutes))) encoded := hex.EncodeToString(sh.Sum(nil)) code := fmt.Sprintf("%s%06d%s", startStr, minutes, encoded) @@ -425,16 +425,6 @@ const ( EByte = PByte * 1024 ) -var bytesSizeTable = map[string]uint64{ - "b": Byte, - "kb": KByte, - "mb": MByte, - "gb": GByte, - "tb": TByte, - "pb": PByte, - "eb": EByte, -} - func logn(n, b float64) float64 { return math.Log(n) / math.Log(b) } @@ -582,27 +572,27 @@ func IsTextFile(data []byte) bool { if len(data) == 0 { return true } - return strings.Index(http.DetectContentType(data), "text/") != -1 + return strings.Contains(http.DetectContentType(data), "text/") } // IsImageFile detects if data is an image format func IsImageFile(data []byte) bool { - return strings.Index(http.DetectContentType(data), "image/") != -1 + return strings.Contains(http.DetectContentType(data), "image/") } // IsPDFFile detects if data is a pdf format func IsPDFFile(data []byte) bool { - return strings.Index(http.DetectContentType(data), "application/pdf") != -1 + return strings.Contains(http.DetectContentType(data), "application/pdf") } // IsVideoFile detects if data is an video format func IsVideoFile(data []byte) bool { - return strings.Index(http.DetectContentType(data), "video/") != -1 + return strings.Contains(http.DetectContentType(data), "video/") } // IsAudioFile detects if data is an video format func IsAudioFile(data []byte) bool { - return strings.Index(http.DetectContentType(data), "audio/") != -1 + return strings.Contains(http.DetectContentType(data), "audio/") } // EntryIcon returns the octicon class for displaying files/directories diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index ec9bc1eb52..fa61e5dfb1 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -287,20 +287,19 @@ func TestHtmlTimeSince(t *testing.T) { } func TestFileSize(t *testing.T) { - var size int64 - size = 512 + var size int64 = 512 assert.Equal(t, "512B", FileSize(size)) - size = size * 1024 + size *= 1024 assert.Equal(t, "512KB", FileSize(size)) - size = size * 1024 + size *= 1024 assert.Equal(t, "512MB", FileSize(size)) - size = size * 1024 + size *= 1024 assert.Equal(t, "512GB", FileSize(size)) - size = size * 1024 + size *= 1024 assert.Equal(t, "512TB", FileSize(size)) - size = size * 1024 + size *= 1024 assert.Equal(t, "512PB", FileSize(size)) - size = size * 4 + size *= 4 assert.Equal(t, "2.0EB", FileSize(size)) } diff --git a/modules/cache/cache.go b/modules/cache/cache.go index a2d6c724c8..a7412f109f 100644 --- a/modules/cache/cache.go +++ b/modules/cache/cache.go @@ -43,7 +43,10 @@ func GetInt(key string, getFunc func() (int, error)) (int, error) { if value, err = getFunc(); err != nil { return value, err } - conn.Put(key, value, int64(setting.CacheService.TTL.Seconds())) + err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds())) + if err != nil { + return 0, err + } } switch value := conn.Get(key).(type) { case int: @@ -72,7 +75,10 @@ func GetInt64(key string, getFunc func() (int64, error)) (int64, error) { if value, err = getFunc(); err != nil { return value, err } - conn.Put(key, value, int64(setting.CacheService.TTL.Seconds())) + err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds())) + if err != nil { + return 0, err + } } switch value := conn.Get(key).(type) { case int64: @@ -93,5 +99,5 @@ func Remove(key string) { if conn == nil { return } - conn.Delete(key) + _ = conn.Delete(key) } diff --git a/modules/context/context.go b/modules/context/context.go index 1699d7aecc..b7c77ac460 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -130,7 +130,6 @@ func (ctx *Context) RedirectToFirst(location ...string) { } ctx.Redirect(setting.AppSubURL + "/") - return } // HTML calls Context.HTML and converts template name to string. @@ -266,7 +265,7 @@ func Contexter() macaron.Handler { } c.Header().Set("Content-Type", "text/html") c.WriteHeader(http.StatusOK) - c.Write([]byte(com.Expand(`<!doctype html> + _, _ = c.Write([]byte(com.Expand(`<!doctype html> <html> <head> <meta name="go-import" content="{GoGetImport} git {CloneLink}"> diff --git a/modules/context/pagination.go b/modules/context/pagination.go index 4795f650fb..390b4dbdd7 100644 --- a/modules/context/pagination.go +++ b/modules/context/pagination.go @@ -39,7 +39,7 @@ func (p *Pagination) AddParam(ctx *Context, paramKey string, ctxKey string) { // GetParams returns the configured URL params func (p *Pagination) GetParams() template.URL { - return template.URL(strings.Join(p.urlParams[:], "&")) + return template.URL(strings.Join(p.urlParams, "&")) } // SetDefaultParams sets common pagination params that are often used diff --git a/modules/context/repo.go b/modules/context/repo.go index 0908340879..096f3c0a5d 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -455,15 +455,13 @@ func RepoAssignment() macaron.Handler { ctx.Repo.PullRequest.BaseRepo = repo.BaseRepo ctx.Repo.PullRequest.Allowed = true ctx.Repo.PullRequest.HeadInfo = ctx.Repo.Owner.Name + ":" + ctx.Repo.BranchName - } else { + } else if repo.AllowsPulls() { // Or, this is repository accepts pull requests between branches. - if repo.AllowsPulls() { - ctx.Data["BaseRepo"] = repo - ctx.Repo.PullRequest.BaseRepo = repo - ctx.Repo.PullRequest.Allowed = true - ctx.Repo.PullRequest.SameRepo = true - ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName - } + ctx.Data["BaseRepo"] = repo + ctx.Repo.PullRequest.BaseRepo = repo + ctx.Repo.PullRequest.Allowed = true + ctx.Repo.PullRequest.SameRepo = true + ctx.Repo.PullRequest.HeadInfo = ctx.Repo.BranchName } } ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequest diff --git a/modules/git/blob.go b/modules/git/blob.go index 171b4a1010..73ac89dfdf 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -50,12 +50,12 @@ func (b *Blob) GetBlobContentBase64() (string, error) { go func() { _, err := io.Copy(encoder, dataRc) - encoder.Close() + _ = encoder.Close() if err != nil { - pw.CloseWithError(err) + _ = pw.CloseWithError(err) } else { - pw.Close() + _ = pw.Close() } }() diff --git a/modules/git/commit.go b/modules/git/commit.go index 7b64a300ab..c86ece9848 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -133,7 +133,7 @@ func (c *Commit) ParentCount() int { func isImageFile(data []byte) (string, bool) { contentType := http.DetectContentType(data) - if strings.Index(contentType, "image/") != -1 { + if strings.Contains(contentType, "image/") { return contentType, true } return contentType, false @@ -206,8 +206,7 @@ func CommitChanges(repoPath string, opts CommitChangesOptions) error { } func commitsCount(repoPath, revision, relpath string) (int64, error) { - var cmd *Command - cmd = NewCommand("rev-list", "--count") + cmd := NewCommand("rev-list", "--count") cmd.AddArguments(revision) if len(relpath) > 0 { cmd.AddArguments("--", relpath) @@ -263,7 +262,7 @@ type SearchCommitsOptions struct { All bool } -// NewSearchCommitsOptions contruct a SearchCommitsOption from a space-delimited search string +// NewSearchCommitsOptions construct a SearchCommitsOption from a space-delimited search string func NewSearchCommitsOptions(searchString string, forAllRefs bool) SearchCommitsOptions { var keywords, authors, committers []string var after, before string diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go index da430a21cd..43723d169b 100644 --- a/modules/git/commit_info.go +++ b/modules/git/commit_info.go @@ -87,16 +87,6 @@ func getCommitTree(c *object.Commit, treePath string) (*object.Tree, error) { return tree, nil } -func getFullPath(treePath, path string) string { - if treePath != "" { - if path != "" { - return treePath + "/" + path - } - return treePath - } - return path -} - func getFileHashes(c *object.Commit, treePath string, paths []string) (map[string]plumbing.Hash, error) { tree, err := getCommitTree(c, treePath) if err == object.ErrDirectoryNotFound { diff --git a/modules/git/repo.go b/modules/git/repo.go index 4be3164130..f5d7ee63bb 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -58,21 +58,21 @@ func (repo *Repository) parsePrettyFormatLogToList(logs []byte) (*list.List, err // IsRepoURLAccessible checks if given repository URL is accessible. func IsRepoURLAccessible(url string) bool { _, err := NewCommand("ls-remote", "-q", "-h", url, "HEAD").Run() - if err != nil { - return false - } - return true + return err == nil } // InitRepository initializes a new Git repository. func InitRepository(repoPath string, bare bool) error { - os.MkdirAll(repoPath, os.ModePerm) + err := os.MkdirAll(repoPath, os.ModePerm) + if err != nil { + return err + } cmd := NewCommand("init") if bare { cmd.AddArguments("--bare") } - _, err := cmd.RunInDir(repoPath) + _, err = cmd.RunInDir(repoPath) return err } diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index 116bdbee82..05eba1e30e 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -29,10 +29,7 @@ func IsBranchExist(repoPath, name string) bool { // IsBranchExist returns true if given branch exists in current repository. func (repo *Repository) IsBranchExist(name string) bool { _, err := repo.gogitRepo.Reference(plumbing.ReferenceName(BranchPrefix+name), true) - if err != nil { - return false - } - return true + return err == nil } // Branch represents a Git branch. @@ -77,7 +74,7 @@ func (repo *Repository) GetBranches() ([]string, error) { return nil, err } - branches.ForEach(func(branch *plumbing.Reference) error { + _ = branches.ForEach(func(branch *plumbing.Reference) error { branchNames = append(branchNames, strings.TrimPrefix(branch.Name().String(), BranchPrefix)) return nil }) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 501ea88e40..8ea2a33145 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -31,10 +31,7 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) { func (repo *Repository) IsCommitExist(name string) bool { hash := plumbing.NewHash(name) _, err := repo.gogitRepo.CommitObject(hash) - if err != nil { - return false - } - return true + return err == nil } // GetBranchCommitID returns last commit ID string of given branch. diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 42f0b9ad0c..ddc8109720 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -13,6 +13,8 @@ import ( "strconv" "strings" "time" + + logger "code.gitea.io/gitea/modules/log" ) // CompareInfo represents needed information for comparing references. @@ -55,7 +57,11 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string) if err = repo.AddRemote(tmpRemote, basePath, true); err != nil { return nil, fmt.Errorf("AddRemote: %v", err) } - defer repo.RemoveRemote(tmpRemote) + defer func() { + if err := repo.RemoveRemote(tmpRemote); err != nil { + logger.Error("GetPullRequestInfo: RemoveRemote: %v", err) + } + }() } compareInfo := new(CompareInfo) diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index 08d66262c1..df49e9acd6 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -24,10 +24,7 @@ func IsTagExist(repoPath, name string) bool { // IsTagExist returns true if given tag exists in the repository. func (repo *Repository) IsTagExist(name string) bool { _, err := repo.gogitRepo.Reference(plumbing.ReferenceName(TagPrefix+name), true) - if err != nil { - return false - } - return true + return err == nil } // CreateTag create one tag in the repository @@ -221,7 +218,7 @@ func (repo *Repository) GetTags() ([]string, error) { return nil, err } - tags.ForEach(func(tag *plumbing.Reference) error { + _ = tags.ForEach(func(tag *plumbing.Reference) error { tagNames = append(tagNames, strings.TrimPrefix(tag.Name().String(), TagPrefix)) return nil }) diff --git a/modules/git/utils.go b/modules/git/utils.go index 8f010321cf..83cd21f34e 100644 --- a/modules/git/utils.go +++ b/modules/git/utils.go @@ -7,7 +7,6 @@ package git import ( "fmt" "os" - "path/filepath" "strings" "sync" ) @@ -75,13 +74,6 @@ func concatenateError(err error, stderr string) error { return fmt.Errorf("%v - %s", err, stderr) } -// If the object is stored in its own file (i.e not in a pack file), -// this function returns the full path to the object file. -// It does not test if the file exists. -func filepathFromSHA1(rootdir, sha1 string) string { - return filepath.Join(rootdir, "objects", sha1[:2], sha1[2:]) -} - // RefEndName return the end name of a ref name func RefEndName(refStr string) string { if strings.HasPrefix(refStr, BranchPrefix) { diff --git a/modules/gzip/gzip.go b/modules/gzip/gzip.go index 4a4a797c7a..0d10071830 100644 --- a/modules/gzip/gzip.go +++ b/modules/gzip/gzip.go @@ -74,7 +74,6 @@ func (wp *WriterPool) Put(w *gzip.Writer) { } var writerPool WriterPool -var regex regexp.Regexp // Options represents the configuration for the gzip middleware type Options struct { @@ -116,7 +115,7 @@ func Middleware(options ...Options) macaron.Handler { if rangeHdr := ctx.Req.Header.Get(rangeHeader); rangeHdr != "" { match := regex.FindStringSubmatch(rangeHdr) - if match != nil && len(match) > 1 { + if len(match) > 1 { return } } @@ -270,9 +269,8 @@ func (proxy *ProxyResponseWriter) Close() error { if proxy.writer == nil { err := proxy.startPlain() - if err != nil { - err = fmt.Errorf("GzipMiddleware: write to regular responseWriter at close gets error: %q", err.Error()) + return fmt.Errorf("GzipMiddleware: write to regular responseWriter at close gets error: %q", err.Error()) } } diff --git a/modules/httplib/httplib.go b/modules/httplib/httplib.go index c96e04c35f..90bbe8f12a 100644 --- a/modules/httplib/httplib.go +++ b/modules/httplib/httplib.go @@ -263,7 +263,7 @@ func (r *Request) getResponse() (*http.Response, error) { } if r.req.Method == "GET" && len(paramBody) > 0 { - if strings.Index(r.url, "?") != -1 { + if strings.Contains(r.url, "?") { r.url += "&" + paramBody } else { r.url = r.url + "?" + paramBody @@ -290,10 +290,13 @@ func (r *Request) getResponse() (*http.Response, error) { } } for k, v := range r.params { - bodyWriter.WriteField(k, v) + err := bodyWriter.WriteField(k, v) + if err != nil { + log.Fatal(err) + } } - bodyWriter.Close() - pw.Close() + _ = bodyWriter.Close() + _ = pw.Close() }() r.Header("Content-Type", bodyWriter.FormDataContentType()) r.req.Body = ioutil.NopCloser(pr) @@ -323,18 +326,15 @@ func (r *Request) getResponse() (*http.Response, error) { Proxy: proxy, Dial: TimeoutDialer(r.setting.ConnectTimeout, r.setting.ReadWriteTimeout), } - } else { - // if r.transport is *http.Transport then set the settings. - if t, ok := trans.(*http.Transport); ok { - if t.TLSClientConfig == nil { - t.TLSClientConfig = r.setting.TLSClientConfig - } - if t.Proxy == nil { - t.Proxy = r.setting.Proxy - } - if t.Dial == nil { - t.Dial = TimeoutDialer(r.setting.ConnectTimeout, r.setting.ReadWriteTimeout) - } + } else if t, ok := trans.(*http.Transport); ok { + if t.TLSClientConfig == nil { + t.TLSClientConfig = r.setting.TLSClientConfig + } + if t.Proxy == nil { + t.Proxy = r.setting.Proxy + } + if t.Dial == nil { + t.Dial = TimeoutDialer(r.setting.ConnectTimeout, r.setting.ReadWriteTimeout) } } @@ -461,7 +461,6 @@ func TimeoutDialer(cTimeout time.Duration, rwTimeout time.Duration) func(net, ad if err != nil { return nil, err } - conn.SetDeadline(time.Now().Add(rwTimeout)) - return conn, nil + return conn, conn.SetDeadline(time.Now().Add(rwTimeout)) } } diff --git a/modules/indexer/indexer.go b/modules/indexer/indexer.go index 9e12a7f501..29261c693b 100644 --- a/modules/indexer/indexer.go +++ b/modules/indexer/indexer.go @@ -5,7 +5,6 @@ package indexer import ( - "fmt" "os" "strconv" @@ -24,15 +23,6 @@ func indexerID(id int64) string { return strconv.FormatInt(id, 36) } -// idOfIndexerID the integer id associated with an indexer id -func idOfIndexerID(indexerID string) (int64, error) { - id, err := strconv.ParseInt(indexerID, 36, 64) - if err != nil { - return 0, fmt.Errorf("Unexpected indexer ID %s: %v", indexerID, err) - } - return id, nil -} - // numericEqualityQuery a numeric equality query for the given value and field func numericEqualityQuery(value int64, field string) *query.NumericRangeQuery { f := float64(value) @@ -42,13 +32,6 @@ func numericEqualityQuery(value int64, field string) *query.NumericRangeQuery { return q } -func newMatchPhraseQuery(matchPhrase, field, analyzer string) *query.MatchPhraseQuery { - q := bleve.NewMatchPhraseQuery(matchPhrase) - q.FieldVal = field - q.Analyzer = analyzer - return q -} - const unicodeNormalizeName = "unicodeNormalize" func addUnicodeNormalizeTokenFilter(m *mapping.IndexMappingImpl) error { diff --git a/modules/indexer/issues/indexer.go b/modules/indexer/issues/indexer.go index 75e6893b87..df8bfd6305 100644 --- a/modules/indexer/issues/indexer.go +++ b/modules/indexer/issues/indexer.go @@ -101,7 +101,12 @@ func InitIssueIndexer(syncReindex bool) error { return fmt.Errorf("Unsupported indexer queue type: %v", setting.Indexer.IssueQueueType) } - go issueIndexerQueue.Run() + go func() { + err = issueIndexerQueue.Run() + if err != nil { + log.Error("issueIndexerQueue.Run: %v", err) + } + }() if populate { if syncReindex { @@ -161,7 +166,7 @@ func UpdateIssueIndexer(issue *models.Issue) { comments = append(comments, comment.Content) } } - issueIndexerQueue.Push(&IndexerData{ + _ = issueIndexerQueue.Push(&IndexerData{ ID: issue.ID, RepoID: issue.RepoID, Title: issue.Title, @@ -179,11 +184,11 @@ func DeleteRepoIssueIndexer(repo *models.Repository) { return } - if len(ids) <= 0 { + if len(ids) == 0 { return } - issueIndexerQueue.Push(&IndexerData{ + _ = issueIndexerQueue.Push(&IndexerData{ IDs: ids, IsDelete: true, }) diff --git a/modules/indexer/issues/queue_channel.go b/modules/indexer/issues/queue_channel.go index bd92f6b7b1..b6458d3eb5 100644 --- a/modules/indexer/issues/queue_channel.go +++ b/modules/indexer/issues/queue_channel.go @@ -34,20 +34,20 @@ func (c *ChannelQueue) Run() error { select { case data := <-c.queue: if data.IsDelete { - c.indexer.Delete(data.IDs...) + _ = c.indexer.Delete(data.IDs...) continue } datas = append(datas, data) if len(datas) >= c.batchNumber { - c.indexer.Index(datas) + _ = c.indexer.Index(datas) // TODO: save the point datas = make([]*IndexerData, 0, c.batchNumber) } case <-time.After(time.Millisecond * 100): i++ if i >= 3 && len(datas) > 0 { - c.indexer.Index(datas) + _ = c.indexer.Index(datas) // TODO: save the point datas = make([]*IndexerData, 0, c.batchNumber) } diff --git a/modules/indexer/issues/queue_disk.go b/modules/indexer/issues/queue_disk.go index cf9e6aee22..e5ac2a7981 100644 --- a/modules/indexer/issues/queue_disk.go +++ b/modules/indexer/issues/queue_disk.go @@ -44,7 +44,7 @@ func (l *LevelQueue) Run() error { for { i++ if len(datas) > l.batchNumber || (len(datas) > 0 && i > 3) { - l.indexer.Index(datas) + _ = l.indexer.Index(datas) datas = make([]*IndexerData, 0, l.batchNumber) i = 0 continue @@ -59,7 +59,7 @@ func (l *LevelQueue) Run() error { continue } - if len(bs) <= 0 { + if len(bs) == 0 { time.Sleep(time.Millisecond * 100) continue } diff --git a/modules/indexer/issues/queue_redis.go b/modules/indexer/issues/queue_redis.go index a9434c4f92..aeccd7920c 100644 --- a/modules/indexer/issues/queue_redis.go +++ b/modules/indexer/issues/queue_redis.go @@ -96,12 +96,12 @@ func (r *RedisQueue) Run() error { i++ if len(datas) > r.batchNumber || (len(datas) > 0 && i > 3) { - r.indexer.Index(datas) + _ = r.indexer.Index(datas) datas = make([]*IndexerData, 0, r.batchNumber) i = 0 } - if len(bs) <= 0 { + if len(bs) == 0 { time.Sleep(time.Millisecond * 100) continue } diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index b1ca2f094a..4516ba01ae 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -17,7 +17,7 @@ import ( ) //checkIsValidRequest check if it a valid request in case of bad request it write the response to ctx. -func checkIsValidRequest(ctx *context.Context, post bool) bool { +func checkIsValidRequest(ctx *context.Context) bool { if !setting.LFS.StartServer { writeStatus(ctx, 404) return false @@ -35,13 +35,6 @@ func checkIsValidRequest(ctx *context.Context, post bool) bool { } ctx.User = user } - if post { - mediaParts := strings.Split(ctx.Req.Header.Get("Content-Type"), ";") - if mediaParts[0] != metaMediaType { - writeStatus(ctx, 400) - return false - } - } return true } @@ -71,7 +64,7 @@ func handleLockListOut(ctx *context.Context, repo *models.Repository, lock *mode // GetListLockHandler list locks func GetListLockHandler(ctx *context.Context) { - if !checkIsValidRequest(ctx, false) { + if !checkIsValidRequest(ctx) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) @@ -135,7 +128,7 @@ func GetListLockHandler(ctx *context.Context) { // PostLockHandler create lock func PostLockHandler(ctx *context.Context) { - if !checkIsValidRequest(ctx, false) { + if !checkIsValidRequest(ctx) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) @@ -198,7 +191,7 @@ func PostLockHandler(ctx *context.Context) { // VerifyLockHandler list locks for verification func VerifyLockHandler(ctx *context.Context) { - if !checkIsValidRequest(ctx, false) { + if !checkIsValidRequest(ctx) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) @@ -249,7 +242,7 @@ func VerifyLockHandler(ctx *context.Context) { // UnLockHandler delete locks func UnLockHandler(ctx *context.Context) { - if !checkIsValidRequest(ctx, false) { + if !checkIsValidRequest(ctx) { return } ctx.Resp.Header().Set("Content-Type", metaMediaType) diff --git a/modules/lfs/server.go b/modules/lfs/server.go index 7e20aa8515..bf5355acfc 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -152,7 +152,7 @@ func getContentHandler(ctx *context.Context) { if rangeHdr := ctx.Req.Header.Get("Range"); rangeHdr != "" { regex := regexp.MustCompile(`bytes=(\d+)\-.*`) match := regex.FindStringSubmatch(rangeHdr) - if match != nil && len(match) > 1 { + if len(match) > 1 { statusCode = 206 fromByte, _ = strconv.ParseInt(match[1], 10, 32) ctx.Resp.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", fromByte, meta.Size-1, meta.Size-fromByte)) @@ -178,8 +178,8 @@ func getContentHandler(ctx *context.Context) { } ctx.Resp.WriteHeader(statusCode) - io.Copy(ctx.Resp, content) - content.Close() + _, _ = io.Copy(ctx.Resp, content) + _ = content.Close() logRequest(ctx.Req, statusCode) } @@ -196,7 +196,7 @@ func getMetaHandler(ctx *context.Context) { if ctx.Req.Method == "GET" { enc := json.NewEncoder(ctx.Resp) - enc.Encode(Represent(rv, meta, true, false)) + _ = enc.Encode(Represent(rv, meta, true, false)) } logRequest(ctx.Req, 200) @@ -249,7 +249,7 @@ func PostHandler(ctx *context.Context) { ctx.Resp.WriteHeader(sentStatus) enc := json.NewEncoder(ctx.Resp) - enc.Encode(Represent(rv, meta, meta.Existing, true)) + _ = enc.Encode(Represent(rv, meta, meta.Existing, true)) logRequest(ctx.Req, sentStatus) } @@ -313,7 +313,7 @@ func BatchHandler(ctx *context.Context) { respobj := &BatchResponse{Objects: responseObjects} enc := json.NewEncoder(ctx.Resp) - enc.Encode(respobj) + _ = enc.Encode(respobj) logRequest(ctx.Req, 200) } diff --git a/modules/log/colors.go b/modules/log/colors.go index 0ec8ce4ba8..c29741634f 100644 --- a/modules/log/colors.go +++ b/modules/log/colors.go @@ -208,7 +208,7 @@ normalLoop: if i > lasti { written, err := c.w.Write(bytes[lasti:i]) - totalWritten = totalWritten + written + totalWritten += written if err != nil { return totalWritten, err } @@ -243,7 +243,7 @@ normalLoop: if bytes[j] == 'm' { if c.mode == allowColor { written, err := c.w.Write(bytes[i : j+1]) - totalWritten = totalWritten + written + totalWritten += written if err != nil { return totalWritten, err } @@ -278,7 +278,7 @@ func ColorSprintf(format string, args ...interface{}) string { } return fmt.Sprintf(format, v...) } - return fmt.Sprintf(format) + return format } // ColorFprintf will write to the provided writer similar to ColorSprintf @@ -290,7 +290,7 @@ func ColorFprintf(w io.Writer, format string, args ...interface{}) (int, error) } return fmt.Fprintf(w, format, v...) } - return fmt.Fprintf(w, format) + return fmt.Fprint(w, format) } // ColorFormatted structs provide their own colored string when formatted with ColorSprintf diff --git a/modules/log/conn.go b/modules/log/conn.go index bd76855168..8816664526 100644 --- a/modules/log/conn.go +++ b/modules/log/conn.go @@ -67,7 +67,10 @@ func (i *connWriter) connect() error { } if tcpConn, ok := conn.(*net.TCPConn); ok { - tcpConn.SetKeepAlive(true) + err = tcpConn.SetKeepAlive(true) + if err != nil { + return err + } } i.innerWriter = conn diff --git a/modules/log/conn_test.go b/modules/log/conn_test.go index 380a115d96..cc3d758fa9 100644 --- a/modules/log/conn_test.go +++ b/modules/log/conn_test.go @@ -24,7 +24,6 @@ func listenReadAndClose(t *testing.T, l net.Listener, expected string) { assert.NoError(t, err) assert.Equal(t, expected, string(written)) - return } func TestConnLogger(t *testing.T) { diff --git a/modules/log/event.go b/modules/log/event.go index 2ec1f9587d..37efa3c230 100644 --- a/modules/log/event.go +++ b/modules/log/event.go @@ -79,7 +79,7 @@ func (l *ChannelledLog) Start() { return } l.loggerProvider.Flush() - case _, _ = <-l.close: + case <-l.close: l.closeLogger() return } @@ -104,7 +104,6 @@ func (l *ChannelledLog) closeLogger() { l.loggerProvider.Flush() l.loggerProvider.Close() l.closed <- true - return } // Close this ChannelledLog @@ -228,7 +227,6 @@ func (m *MultiChannelledLog) closeLoggers() { } m.mutex.Unlock() m.closed <- true - return } // Start processing the MultiChannelledLog diff --git a/modules/log/file.go b/modules/log/file.go index cdda85d626..877820b8be 100644 --- a/modules/log/file.go +++ b/modules/log/file.go @@ -223,7 +223,7 @@ func compressOldLogFile(fname string, compressionLevel int) error { func (log *FileLogger) deleteOldLog() { dir := filepath.Dir(log.Filename) - filepath.Walk(dir, func(path string, info os.FileInfo, err error) (returnErr error) { + _ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) (returnErr error) { defer func() { if r := recover(); r != nil { returnErr = fmt.Errorf("Unable to delete old log '%s', error: %+v", path, r) @@ -246,7 +246,7 @@ func (log *FileLogger) deleteOldLog() { // there are no buffering messages in file logger in memory. // flush file means sync file from disk. func (log *FileLogger) Flush() { - log.mw.fd.Sync() + _ = log.mw.fd.Sync() } // GetName returns the default name for this implementation diff --git a/modules/log/file_test.go b/modules/log/file_test.go index 648db6f393..38279315ab 100644 --- a/modules/log/file_test.go +++ b/modules/log/file_test.go @@ -103,7 +103,7 @@ func TestFileLogger(t *testing.T) { assert.Equal(t, expected, string(logData)) event.level = WARN - expected = expected + fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) + expected += fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) fileLogger.LogEvent(&event) fileLogger.Flush() logData, err = ioutil.ReadFile(filename) @@ -130,7 +130,7 @@ func TestFileLogger(t *testing.T) { err = realFileLogger.DoRotate() assert.Error(t, err) - expected = expected + fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) + expected += fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) fileLogger.LogEvent(&event) fileLogger.Flush() logData, err = ioutil.ReadFile(filename) @@ -138,7 +138,7 @@ func TestFileLogger(t *testing.T) { assert.Equal(t, expected, string(logData)) // Should fail to rotate - expected = expected + fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) + expected += fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) fileLogger.LogEvent(&event) fileLogger.Flush() logData, err = ioutil.ReadFile(filename) @@ -188,7 +188,7 @@ func TestCompressFileLogger(t *testing.T) { assert.Equal(t, expected, string(logData)) event.level = WARN - expected = expected + fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) + expected += fmt.Sprintf("%s%s %s:%d:%s [%c] %s\n", prefix, dateString, event.filename, event.line, event.caller, strings.ToUpper(event.level.String())[0], event.msg) fileLogger.LogEvent(&event) fileLogger.Flush() logData, err = ioutil.ReadFile(filename) diff --git a/modules/log/flags.go b/modules/log/flags.go index 928d42b965..992fc62ddb 100644 --- a/modules/log/flags.go +++ b/modules/log/flags.go @@ -57,7 +57,7 @@ func FlagsFromString(from string) int { for _, flag := range strings.Split(strings.ToLower(from), ",") { f, ok := flagFromString[strings.TrimSpace(flag)] if ok { - flags = flags | f + flags |= f } } return flags diff --git a/modules/log/log.go b/modules/log/log.go index 8698e9eed3..0ca0f3adc5 100644 --- a/modules/log/log.go +++ b/modules/log/log.go @@ -218,7 +218,7 @@ func (l *LoggerAsWriter) Write(p []byte) (int, error) { func (l *LoggerAsWriter) Log(msg string) { for _, logger := range l.ourLoggers { // Set the skip to reference the call just above this - logger.Log(1, l.level, msg) + _ = logger.Log(1, l.level, msg) } } diff --git a/modules/log/smtp.go b/modules/log/smtp.go index f77d716d94..f912299a73 100644 --- a/modules/log/smtp.go +++ b/modules/log/smtp.go @@ -11,10 +11,6 @@ import ( "strings" ) -const ( - subjectPhrase = "Diagnostic message from server" -) - type smtpWriter struct { owner *SMTPLogger } diff --git a/modules/log/writer.go b/modules/log/writer.go index 22ef0b9047..2503f04d76 100644 --- a/modules/log/writer.go +++ b/modules/log/writer.go @@ -252,10 +252,7 @@ func (logger *WriterLogger) Match(event *Event) bool { mode: removeColor, }).Write([]byte(event.msg)) msg = baw - if logger.regexp.Match(msg) { - return true - } - return false + return logger.regexp.Match(msg) } // Close the base logger diff --git a/modules/mailer/mailer.go b/modules/mailer/mailer.go index 411d6eafd8..d19ae7b2f4 100644 --- a/modules/mailer/mailer.go +++ b/modules/mailer/mailer.go @@ -258,15 +258,12 @@ func (s *dummySender) Send(from string, to []string, msg io.WriterTo) error { } func processMailQueue() { - for { - select { - case msg := <-mailQueue: - log.Trace("New e-mail sending request %s: %s", msg.GetHeader("To"), msg.Info) - if err := gomail.Send(Sender, msg.Message); err != nil { - log.Error("Failed to send emails %s: %s - %v", msg.GetHeader("To"), msg.Info, err) - } else { - log.Trace("E-mails sent %s: %s", msg.GetHeader("To"), msg.Info) - } + for msg := range mailQueue { + log.Trace("New e-mail sending request %s: %s", msg.GetHeader("To"), msg.Info) + if err := gomail.Send(Sender, msg.Message); err != nil { + log.Error("Failed to send emails %s: %s - %v", msg.GetHeader("To"), msg.Info, err) + } else { + log.Trace("E-mails sent %s: %s", msg.GetHeader("To"), msg.Info) } } } diff --git a/modules/markup/html.go b/modules/markup/html.go index 91913b0679..dbfc8dbe85 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -108,24 +108,6 @@ func FindAllMentions(content string) []string { return ret } -// cutoutVerbosePrefix cutouts URL prefix including sub-path to -// return a clean unified string of request URL path. -func cutoutVerbosePrefix(prefix string) string { - if len(prefix) == 0 || prefix[0] != '/' { - return prefix - } - count := 0 - for i := 0; i < len(prefix); i++ { - if prefix[i] == '/' { - count++ - } - if count >= 3+setting.AppSubURLDepth { - return prefix[:i] - } - } - return prefix -} - // IsSameDomain checks if given url string has the same hostname as current Gitea instance func IsSameDomain(s string) bool { if strings.HasPrefix(s, "/") { @@ -146,7 +128,7 @@ type postProcessError struct { } func (p *postProcessError) Error() string { - return "PostProcess: " + p.context + ", " + p.Error() + return "PostProcess: " + p.context + ", " + p.err.Error() } type processor func(ctx *postProcessCtx, node *html.Node) @@ -304,20 +286,6 @@ func (ctx *postProcessCtx) visitNode(node *html.Node) { // ignore everything else } -func (ctx *postProcessCtx) visitNodeForShortLinks(node *html.Node) { - switch node.Type { - case html.TextNode: - shortLinkProcessorFull(ctx, node, true) - case html.ElementNode: - if node.Data == "code" || node.Data == "pre" || node.Data == "a" { - return - } - for n := node.FirstChild; n != nil; n = n.NextSibling { - ctx.visitNodeForShortLinks(n) - } - } -} - // textNode runs the passed node through various processors, in order to handle // all kinds of special links handled by the post-processing. func (ctx *postProcessCtx) textNode(node *html.Node) { diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 135a8e103c..10bc4973cd 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -29,11 +29,6 @@ func numericIssueLink(baseURL string, index int) string { return link(util.URLJoin(baseURL, strconv.Itoa(index)), fmt.Sprintf("#%d", index)) } -// urlContentsLink an HTML link whose contents is the target URL -func urlContentsLink(href string) string { - return link(href, href) -} - // link an HTML link func link(href, contents string) string { return fmt.Sprintf("<a href=\"%s\">%s</a>", href, contents) diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index a31591c2a7..22089158f5 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -35,12 +35,9 @@ func NewNotifier() base.Notifier { } func (ns *notificationService) Run() { - for { - select { - case opts := <-ns.issueQueue: - if err := models.CreateOrUpdateIssueNotifications(opts.issue, opts.notificationAuthorID); err != nil { - log.Error("Was unable to create issue notification: %v", err) - } + for opts := range ns.issueQueue { + if err := models.CreateOrUpdateIssueNotifications(opts.issue, opts.notificationAuthorID); err != nil { + log.Error("Was unable to create issue notification: %v", err) } } } diff --git a/modules/pprof/pprof.go b/modules/pprof/pprof.go index b63904e713..80ad67be3a 100644 --- a/modules/pprof/pprof.go +++ b/modules/pprof/pprof.go @@ -9,6 +9,8 @@ import ( "io/ioutil" "runtime" "runtime/pprof" + + "code.gitea.io/gitea/modules/log" ) // DumpMemProfileForUsername dumps a memory profile at pprofDataPath as memprofile_<username>_<temporary id> @@ -30,9 +32,15 @@ func DumpCPUProfileForUsername(pprofDataPath, username string) (func(), error) { return nil, err } - pprof.StartCPUProfile(f) + err = pprof.StartCPUProfile(f) + if err != nil { + log.Fatal("StartCPUProfile: %v", err) + } return func() { pprof.StopCPUProfile() - f.Close() + err = f.Close() + if err != nil { + log.Fatal("StopCPUProfile Close: %v", err) + } }, nil } diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go index 09a4dbb44c..91910fa860 100644 --- a/modules/repofiles/delete.go +++ b/modules/repofiles/delete.go @@ -53,11 +53,9 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo BranchName: opts.NewBranch, } } - } else { - if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { - return nil, models.ErrUserCannotCommit{ - UserName: doer.LowerName, - } + } else if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { + return nil, models.ErrUserCannotCommit{ + UserName: doer.LowerName, } } @@ -74,10 +72,10 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo author, committer := GetAuthorAndCommitterUsers(opts.Committer, opts.Author, doer) t, err := NewTemporaryUploadRepository(repo) - defer t.Close() if err != nil { return nil, err } + defer t.Close() if err := t.Clone(opts.OldBranch); err != nil { return nil, err } diff --git a/modules/repofiles/file.go b/modules/repofiles/file.go index de3ee71dba..70fd57bba0 100644 --- a/modules/repofiles/file.go +++ b/modules/repofiles/file.go @@ -86,7 +86,7 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *models // If only one of the two are provided, we set both of them to it. // If neither are provided, both are the doer. if committer != nil && committer.Email != "" { - if doer != nil && strings.ToLower(doer.Email) == strings.ToLower(committer.Email) { + if doer != nil && strings.EqualFold(doer.Email, committer.Email) { committerUser = doer // the committer is the doer, so will use their user object if committer.Name != "" { committerUser.FullName = committer.Name @@ -99,7 +99,7 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *models } } if author != nil && author.Email != "" { - if doer != nil && strings.ToLower(doer.Email) == strings.ToLower(author.Email) { + if doer != nil && strings.EqualFold(doer.Email, author.Email) { authorUser = doer // the author is the doer, so will use their user object if authorUser.Name != "" { authorUser.FullName = author.Name diff --git a/modules/repofiles/tree.go b/modules/repofiles/tree.go index 4eb54a2598..318a5d152d 100644 --- a/modules/repofiles/tree.go +++ b/modules/repofiles/tree.go @@ -16,6 +16,9 @@ import ( // GetTreeBySHA get the GitTreeResponse of a repository using a sha hash. func GetTreeBySHA(repo *models.Repository, sha string, page, perPage int, recursive bool) (*api.GitTreeResponse, error) { gitRepo, err := git.OpenRepository(repo.RepoPath()) + if err != nil { + return nil, err + } gitTree, err := gitRepo.GetTree(sha) if err != nil || gitTree == nil { return nil, models.ErrSHANotFound{ @@ -39,12 +42,12 @@ func GetTreeBySHA(repo *models.Repository, sha string, page, perPage int, recurs // 51 is len(sha1) + len("/git/blobs/"). 40 + 11. blobURL := make([]byte, apiURLLen+51) - copy(blobURL[:], apiURL) + copy(blobURL, apiURL) copy(blobURL[apiURLLen:], "/git/blobs/") // 51 is len(sha1) + len("/git/trees/"). 40 + 11. treeURL := make([]byte, apiURLLen+51) - copy(treeURL[:], apiURL) + copy(treeURL, apiURL) copy(treeURL[apiURLLen:], "/git/trees/") // 40 is the size of the sha1 hash in hexadecimal format. @@ -83,10 +86,10 @@ func GetTreeBySHA(repo *models.Repository, sha string, page, perPage int, recurs if entries[e].IsDir() { copy(treeURL[copyPos:], entries[e].ID.String()) - tree.Entries[i].URL = string(treeURL[:]) + tree.Entries[i].URL = string(treeURL) } else { copy(blobURL[copyPos:], entries[e].ID.String()) - tree.Entries[i].URL = string(blobURL[:]) + tree.Entries[i].URL = string(blobURL) } } return tree, nil diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index 569c89ac51..f011017a5e 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -99,6 +99,10 @@ func detectEncodingAndBOM(entry *git.TreeEntry, repo *models.Repository) (string } result, n, err := transform.String(charsetEncoding.NewDecoder(), string(buf)) + if err != nil { + // return default + return "UTF-8", false + } if n > 2 { return encoding, bytes.Equal([]byte(result)[0:3], base.UTF8BOM) @@ -135,10 +139,8 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up if err != nil && !git.IsErrBranchNotExist(err) { return nil, err } - } else { - if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { - return nil, models.ErrUserCannotCommit{UserName: doer.LowerName} - } + } else if protected, _ := repo.IsProtectedBranchForPush(opts.OldBranch, doer); protected { + return nil, models.ErrUserCannotCommit{UserName: doer.LowerName} } // If FromTreePath is not set, set it to the opts.TreePath @@ -166,10 +168,10 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up author, committer := GetAuthorAndCommitterUsers(opts.Committer, opts.Author, doer) t, err := NewTemporaryUploadRepository(repo) - defer t.Close() if err != nil { - return nil, err + log.Error("%v", err) } + defer t.Close() if err := t.Clone(opts.OldBranch); err != nil { return nil, err } diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index 5f428c3139..2da101c64d 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -57,10 +57,10 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep } t, err := NewTemporaryUploadRepository(repo) - defer t.Close() if err != nil { return err } + defer t.Close() if err := t.Clone(opts.OldBranch); err != nil { return err } @@ -108,10 +108,8 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep } infos[i] = uploadInfo - } else { - if objectHash, err = t.HashObject(file); err != nil { - return err - } + } else if objectHash, err = t.HashObject(file); err != nil { + return err } // Add the object to the index diff --git a/modules/repofiles/verification.go b/modules/repofiles/verification.go index be56f9b8b8..9fc084daaf 100644 --- a/modules/repofiles/verification.go +++ b/modules/repofiles/verification.go @@ -20,10 +20,8 @@ func GetPayloadCommitVerification(commit *git.Commit) *structs.PayloadCommitVeri } if verification.Reason != "" { verification.Reason = commitVerification.Reason - } else { - if verification.Verified { - verification.Reason = "unsigned" - } + } else if verification.Verified { + verification.Reason = "unsigned" } return verification } diff --git a/modules/session/virtual.go b/modules/session/virtual.go index b90f03417a..b8ddd2f71b 100644 --- a/modules/session/virtual.go +++ b/modules/session/virtual.go @@ -21,10 +21,8 @@ import ( // VirtualSessionProvider represents a shadowed session provider implementation. type VirtualSessionProvider struct { - lock sync.RWMutex - maxlifetime int64 - rootPath string - provider session.Provider + lock sync.RWMutex + provider session.Provider } // Init initializes the cookie session provider with given root path. diff --git a/modules/setting/log.go b/modules/setting/log.go index 9f4bbf9d87..5e2d2d769d 100644 --- a/modules/setting/log.go +++ b/modules/setting/log.go @@ -150,8 +150,6 @@ func generateNamedLogger(key string, options defaultLogOptions) *LogDescription sections := strings.Split(Cfg.Section("log").Key(strings.ToUpper(key)).MustString(""), ",") - //description.Configs = make([]string, len(description.Sections)) - for i := 0; i < len(sections); i++ { sections[i] = strings.TrimSpace(sections[i]) } @@ -167,7 +165,10 @@ func generateNamedLogger(key string, options defaultLogOptions) *LogDescription provider, config, levelName := generateLogConfig(sec, name, options) - log.NewNamedLogger(key, options.bufferLength, name, provider, config) + if err := log.NewNamedLogger(key, options.bufferLength, name, provider, config); err != nil { + // Maybe panic here? + log.Error("Could not create new named logger: %v", err.Error()) + } description.SubLogDescriptions = append(description.SubLogDescriptions, SubLogDescription{ Name: name, @@ -242,7 +243,10 @@ func newLogService() { } if !useConsole { - log.DelLogger("console") + err := log.DelLogger("console") + if err != nil { + log.Fatal("DelLogger: %v", err) + } } for _, name := range sections { diff --git a/modules/setting/setting.go b/modules/setting/setting.go index ff53e9a375..b550836bc1 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -545,13 +545,14 @@ func NewContext() { AppName = Cfg.Section("").Key("APP_NAME").MustString("Gitea: Git with a cup of tea") Protocol = HTTP - if sec.Key("PROTOCOL").String() == "https" { + switch sec.Key("PROTOCOL").String() { + case "https": Protocol = HTTPS CertFile = sec.Key("CERT_FILE").String() KeyFile = sec.Key("KEY_FILE").String() - } else if sec.Key("PROTOCOL").String() == "fcgi" { + case "fcgi": Protocol = FCGI - } else if sec.Key("PROTOCOL").String() == "unix" { + case "unix": Protocol = UnixSocket UnixSocketPermissionRaw := sec.Key("UNIX_SOCKET_PERMISSION").MustString("666") UnixSocketPermissionParsed, err := strconv.ParseUint(UnixSocketPermissionRaw, 8, 32) diff --git a/modules/ssh/ssh.go b/modules/ssh/ssh.go index 98ff50bfec..c5251ef23a 100644 --- a/modules/ssh/ssh.go +++ b/modules/ssh/ssh.go @@ -37,7 +37,10 @@ func cleanCommand(cmd string) string { func handleServerConn(keyID string, chans <-chan ssh.NewChannel) { for newChan := range chans { if newChan.ChannelType() != "session" { - newChan.Reject(ssh.UnknownChannelType, "unknown channel type") + err := newChan.Reject(ssh.UnknownChannelType, "unknown channel type") + if err != nil { + log.Error("Error rejecting channel: %v", err) + } continue } @@ -48,7 +51,11 @@ func handleServerConn(keyID string, chans <-chan ssh.NewChannel) { } go func(in <-chan *ssh.Request) { - defer ch.Close() + defer func() { + if err = ch.Close(); err != nil { + log.Error("Close: %v", err) + } + }() for req := range in { payload := cleanCommand(string(req.Payload)) switch req.Type { @@ -87,17 +94,34 @@ func handleServerConn(keyID string, chans <-chan ssh.NewChannel) { return } - req.Reply(true, nil) - go io.Copy(input, ch) - io.Copy(ch, stdout) - io.Copy(ch.Stderr(), stderr) + err = req.Reply(true, nil) + if err != nil { + log.Error("SSH: Reply: %v", err) + } + go func() { + _, err = io.Copy(input, ch) + if err != nil { + log.Error("SSH: Copy: %v", err) + } + }() + _, err = io.Copy(ch, stdout) + if err != nil { + log.Error("SSH: Copy: %v", err) + } + _, err = io.Copy(ch.Stderr(), stderr) + if err != nil { + log.Error("SSH: Copy: %v", err) + } if err = cmd.Wait(); err != nil { log.Error("SSH: Wait: %v", err) return } - ch.SendRequest("exit-status", false, []byte{0, 0, 0, 0}) + _, err = ch.SendRequest("exit-status", false, []byte{0, 0, 0, 0}) + if err != nil { + log.Error("SSH: SendRequest: %v", err) + } return default: } @@ -203,7 +227,11 @@ func GenKeyPair(keyPath string) error { if err != nil { return err } - defer f.Close() + defer func() { + if err = f.Close(); err != nil { + log.Error("Close: %v", err) + } + }() if err := pem.Encode(f, privateKeyPEM); err != nil { return err @@ -220,7 +248,11 @@ func GenKeyPair(keyPath string) error { if err != nil { return err } - defer p.Close() + defer func() { + if err = p.Close(); err != nil { + log.Error("Close: %v", err) + } + }() _, err = p.Write(public) return err } diff --git a/modules/structs/user_search.go b/modules/structs/user_search.go deleted file mode 100644 index 1650cf736a..0000000000 --- a/modules/structs/user_search.go +++ /dev/null @@ -1,5 +0,0 @@ -package structs - -type searchUsersResponse struct { - Users []*User `json:"data"` -} diff --git a/modules/structs/utils.go b/modules/structs/utils.go index 1b9d689562..aaeb653d03 100644 --- a/modules/structs/utils.go +++ b/modules/structs/utils.go @@ -4,12 +4,6 @@ package structs -import ( - "net/http" -) - -var jsonHeader = http.Header{"content-type": []string{"application/json"}} - // Bool return address of bool value func Bool(v bool) *bool { return &v diff --git a/modules/templates/dynamic.go b/modules/templates/dynamic.go index dbd75221d2..d7c04ccb09 100644 --- a/modules/templates/dynamic.go +++ b/modules/templates/dynamic.go @@ -83,12 +83,15 @@ func Mailer() *template.Template { continue } - templates.New( + _, err = templates.New( strings.TrimSuffix( filePath, ".tmpl", ), ).Parse(string(content)) + if err != nil { + log.Warn("Failed to parse template %v", err) + } } } } @@ -113,12 +116,15 @@ func Mailer() *template.Template { continue } - templates.New( + _, err = templates.New( strings.TrimSuffix( filePath, ".tmpl", ), ).Parse(string(content)) + if err != nil { + log.Warn("Failed to parse template %v", err) + } } } } diff --git a/modules/templates/helper.go b/modules/templates/helper.go index ef4a68add0..c4551bb4be 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -343,7 +343,7 @@ func ReplaceLeft(s, oldS, newS string) string { // allocating space for the new string curLen := n*newLen + len(s[i:]) - replacement := make([]byte, curLen, curLen) + replacement := make([]byte, curLen) j := 0 for ; j < n*newLen; j += newLen { diff --git a/modules/user/user_test.go b/modules/user/user_test.go index ae7460281f..fadcbde7bf 100644 --- a/modules/user/user_test.go +++ b/modules/user/user_test.go @@ -13,7 +13,7 @@ func getWhoamiOutput() (string, error) { if err != nil { return "", err } - return strings.TrimSpace(string(output[:])), nil + return strings.TrimSpace(string(output)), nil } func TestCurrentUsername(t *testing.T) { diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index 7bc41ac395..f55b09266d 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -26,12 +26,6 @@ type ( expectedErrors binding.Errors } - handlerFunc func(interface{}, ...interface{}) macaron.Handler - - modeler interface { - Model() string - } - TestForm struct { BranchName string `form:"BranchName" binding:"GitRefName"` URL string `form:"ValidUrl" binding:"ValidUrl"` diff --git a/routers/admin/admin.go b/routers/admin/admin.go index 5107e18b7d..b4eac2c677 100644 --- a/routers/admin/admin.go +++ b/routers/admin/admin.go @@ -261,10 +261,6 @@ func Config(ctx *context.Context) { } ctx.Data["EnvVars"] = envVars - - type logger struct { - Mode, Config string - } ctx.Data["Loggers"] = setting.LogDescriptions ctx.Data["RedirectMacaronLog"] = setting.RedirectMacaronLog ctx.Data["EnableAccessLog"] = setting.EnableAccessLog diff --git a/routers/api/v1/misc/markdown.go b/routers/api/v1/misc/markdown.go index 06e344a15b..b00b00c499 100644 --- a/routers/api/v1/misc/markdown.go +++ b/routers/api/v1/misc/markdown.go @@ -5,6 +5,7 @@ package misc import ( + "net/http" "strings" api "code.gitea.io/gitea/modules/structs" @@ -42,7 +43,7 @@ func Markdown(ctx *context.APIContext, form api.MarkdownOption) { } if len(form.Text) == 0 { - ctx.Write([]byte("")) + _, _ = ctx.Write([]byte("")) return } @@ -63,12 +64,24 @@ func Markdown(ctx *context.APIContext, form api.MarkdownOption) { meta = ctx.Repo.Repository.ComposeMetas() } if form.Wiki { - ctx.Write([]byte(markdown.RenderWiki(md, urlPrefix, meta))) + _, err := ctx.Write([]byte(markdown.RenderWiki(md, urlPrefix, meta))) + if err != nil { + ctx.Error(http.StatusInternalServerError, "", err) + return + } } else { - ctx.Write(markdown.Render(md, urlPrefix, meta)) + _, err := ctx.Write(markdown.Render(md, urlPrefix, meta)) + if err != nil { + ctx.Error(http.StatusInternalServerError, "", err) + return + } } default: - ctx.Write(markdown.RenderRaw([]byte(form.Text), "", false)) + _, err := ctx.Write(markdown.RenderRaw([]byte(form.Text), "", false)) + if err != nil { + ctx.Error(http.StatusInternalServerError, "", err) + return + } } } @@ -98,5 +111,9 @@ func MarkdownRaw(ctx *context.APIContext) { ctx.Error(422, "", err) return } - ctx.Write(markdown.RenderRaw(body, "", false)) + _, err = ctx.Write(markdown.RenderRaw(body, "", false)) + if err != nil { + ctx.Error(http.StatusInternalServerError, "", err) + return + } } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 0e1db144b1..b14b0b02b8 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -353,7 +353,11 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) { return } - pr.LoadIssue() + err = pr.LoadIssue() + if err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssue", err) + return + } issue := pr.Issue issue.Repo = ctx.Repo.Repository @@ -547,7 +551,11 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { return } - pr.LoadIssue() + err = pr.LoadIssue() + if err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssue", err) + return + } pr.Issue.Repo = ctx.Repo.Repository if ctx.IsSigned { diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index f8df3e9fa1..26cfff51ce 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -631,15 +631,6 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err return nil } -func unitTypeInTypes(unitType models.UnitType, unitTypes []models.UnitType) bool { - for _, tp := range unitTypes { - if unitType == tp { - return true - } - } - return false -} - // updateRepoUnits updates repo units: Issue settings, Wiki settings, PR settings func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { owner := ctx.Repo.Owner diff --git a/routers/api/v1/user/gpg_key.go b/routers/api/v1/user/gpg_key.go index c2c55e9b92..7bf43c5822 100644 --- a/routers/api/v1/user/gpg_key.go +++ b/routers/api/v1/user/gpg_key.go @@ -9,14 +9,9 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers/api/v1/convert" ) -func composePublicGPGKeysAPILink() string { - return setting.AppURL + "api/v1/user/gpg_keys/" -} - func listGPGKeys(ctx *context.APIContext, uid int64) { keys, err := models.ListGPGKeys(uid) if err != nil { diff --git a/routers/init.go b/routers/init.go index b3078b478a..e6b23cf8b8 100644 --- a/routers/init.go +++ b/routers/init.go @@ -41,7 +41,7 @@ func checkRunMode() { func NewServices() { setting.NewServices() mailer.NewContext() - cache.NewContext() + _ = cache.NewContext() } // In case of problems connecting to DB, retry connection. Eg, PGSQL in Docker Container on Synology diff --git a/routers/org/teams.go b/routers/org/teams.go index 27db14e4b5..f662bd92e2 100644 --- a/routers/org/teams.go +++ b/routers/org/teams.go @@ -5,6 +5,7 @@ package org import ( + "net/http" "path" "strings" @@ -287,7 +288,11 @@ func EditTeamPost(ctx *context.Context, form auth.CreateTeamForm) { Type: tp, }) } - models.UpdateTeamUnits(t, units) + err := models.UpdateTeamUnits(t, units) + if err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssue", err.Error()) + return + } } if ctx.HasError() { diff --git a/routers/private/hook.go b/routers/private/hook.go index a5985f161e..3da5e38edb 100644 --- a/routers/private/hook.go +++ b/routers/private/hook.go @@ -214,5 +214,4 @@ func HookPostReceive(ctx *macaron.Context) { ctx.JSON(http.StatusOK, map[string]interface{}{ "message": false, }) - return } diff --git a/routers/private/serv.go b/routers/private/serv.go index 68e4361e56..90579a3dcc 100644 --- a/routers/private/serv.go +++ b/routers/private/serv.go @@ -62,7 +62,6 @@ func ServNoCommand(ctx *macaron.Context) { results.Owner = user } ctx.JSON(http.StatusOK, &results) - return } // ServCommand returns information about the provided keyid @@ -282,5 +281,4 @@ func ServCommand(ctx *macaron.Context) { ctx.JSON(http.StatusOK, results) // We will update the keys in a different call. - return } diff --git a/routers/repo/blame.go b/routers/repo/blame.go index 964fdc8746..2b2f45f0bb 100644 --- a/routers/repo/blame.go +++ b/routers/repo/blame.go @@ -192,7 +192,7 @@ func RefBlame(ctx *context.Context) { func renderBlame(ctx *context.Context, blameParts []models.BlamePart, commitNames map[string]models.UserCommit) { repoLink := ctx.Repo.RepoLink - var lines = make([]string, 0, 0) + var lines = make([]string, 0) var commitInfo bytes.Buffer var lineNumbers bytes.Buffer diff --git a/routers/repo/commit.go b/routers/repo/commit.go index dde6d8f321..4dbedea2a0 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -261,6 +261,9 @@ func Diff(ctx *context.Context) { } ctx.Data["RawPath"] = setting.AppSubURL + "/" + path.Join(userName, repoName, "raw", "commit", commitID) ctx.Data["BranchName"], err = commit.GetBranchName() + if err != nil { + ctx.ServerError("commit.GetBranchName", err) + } ctx.HTML(200, tplCommitPage) } diff --git a/routers/repo/download.go b/routers/repo/download.go index 41c4a18102..2da8b109ca 100644 --- a/routers/repo/download.go +++ b/routers/repo/download.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/log" ) // ServeData download file from io.Reader @@ -39,8 +40,11 @@ func ServeData(ctx *context.Context, name string, reader io.Reader) error { ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s"`, name)) } - ctx.Resp.Write(buf) - _, err := io.Copy(ctx.Resp, reader) + _, err := ctx.Resp.Write(buf) + if err != nil { + return err + } + _, err = io.Copy(ctx.Resp, reader) return err } @@ -50,7 +54,11 @@ func ServeBlob(ctx *context.Context, blob *git.Blob) error { if err != nil { return err } - defer dataRc.Close() + defer func() { + if err = dataRc.Close(); err != nil { + log.Error("ServeBlob: Close: %v", err) + } + }() return ServeData(ctx, ctx.Repo.TreePath, dataRc) } @@ -61,7 +69,11 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob) error { if err != nil { return err } - defer dataRc.Close() + defer func() { + if err = dataRc.Close(); err != nil { + log.Error("ServeBlobOrLFS: Close: %v", err) + } + }() if meta, _ := lfs.ReadPointerFile(dataRc); meta != nil { meta, _ = ctx.Repo.Repository.GetLFSMetaObjectByOid(meta.Oid) diff --git a/routers/repo/editor.go b/routers/repo/editor.go index 46f12d66d2..062ecfebf7 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -118,9 +118,7 @@ func editFile(ctx *context.Context, isNewFile bool) { d, _ := ioutil.ReadAll(dataRc) buf = append(buf, d...) if content, err := templates.ToUTF8WithErr(buf); err != nil { - if err != nil { - log.Error("ToUTF8WithErr: %v", err) - } + log.Error("ToUTF8WithErr: %v", err) ctx.Data["FileContent"] = string(buf) } else { ctx.Data["FileContent"] = content @@ -235,16 +233,12 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo switch fileErr.Type { case git.EntryModeSymlink: ctx.RenderWithErr(ctx.Tr("repo.editor.file_is_a_symlink", fileErr.Path), tplEditFile, &form) - break case git.EntryModeTree: ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_a_directory", fileErr.Path), tplEditFile, &form) - break case git.EntryModeBlob: ctx.RenderWithErr(ctx.Tr("repo.editor.directory_is_a_file", fileErr.Path), tplEditFile, &form) - break default: ctx.Error(500, err.Error()) - break } } else { ctx.Error(500, err.Error()) @@ -403,16 +397,12 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) { switch fileErr.Type { case git.EntryModeSymlink: ctx.RenderWithErr(ctx.Tr("repo.editor.file_is_a_symlink", fileErr.Path), tplEditFile, &form) - break case git.EntryModeTree: ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_a_directory", fileErr.Path), tplEditFile, &form) - break case git.EntryModeBlob: ctx.RenderWithErr(ctx.Tr("repo.editor.directory_is_a_file", fileErr.Path), tplEditFile, &form) - break default: ctx.ServerError("DeleteRepoFile", err) - break } } else { ctx.ServerError("DeleteRepoFile", err) diff --git a/routers/repo/http.go b/routers/repo/http.go index 214e2f3411..3072209448 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -206,10 +206,8 @@ func HTTP(ctx *context.Context) { if err = models.UpdateAccessToken(token); err != nil { ctx.ServerError("UpdateAccessToken", err) } - } else { - if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { - log.Error("GetAccessTokenBySha: %v", err) - } + } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { + log.Error("GetAccessTokenBySha: %v", err) } if authUser == nil { @@ -332,17 +330,17 @@ type route struct { } var routes = []route{ - {regexp.MustCompile("(.*?)/git-upload-pack$"), "POST", serviceUploadPack}, - {regexp.MustCompile("(.*?)/git-receive-pack$"), "POST", serviceReceivePack}, - {regexp.MustCompile("(.*?)/info/refs$"), "GET", getInfoRefs}, - {regexp.MustCompile("(.*?)/HEAD$"), "GET", getTextFile}, - {regexp.MustCompile("(.*?)/objects/info/alternates$"), "GET", getTextFile}, - {regexp.MustCompile("(.*?)/objects/info/http-alternates$"), "GET", getTextFile}, - {regexp.MustCompile("(.*?)/objects/info/packs$"), "GET", getInfoPacks}, - {regexp.MustCompile("(.*?)/objects/info/[^/]*$"), "GET", getTextFile}, - {regexp.MustCompile("(.*?)/objects/[0-9a-f]{2}/[0-9a-f]{38}$"), "GET", getLooseObject}, - {regexp.MustCompile("(.*?)/objects/pack/pack-[0-9a-f]{40}\\.pack$"), "GET", getPackFile}, - {regexp.MustCompile("(.*?)/objects/pack/pack-[0-9a-f]{40}\\.idx$"), "GET", getIdxFile}, + {regexp.MustCompile(`(.*?)/git-upload-pack$`), "POST", serviceUploadPack}, + {regexp.MustCompile(`(.*?)/git-receive-pack$`), "POST", serviceReceivePack}, + {regexp.MustCompile(`(.*?)/info/refs$`), "GET", getInfoRefs}, + {regexp.MustCompile(`(.*?)/HEAD$`), "GET", getTextFile}, + {regexp.MustCompile(`(.*?)/objects/info/alternates$`), "GET", getTextFile}, + {regexp.MustCompile(`(.*?)/objects/info/http-alternates$`), "GET", getTextFile}, + {regexp.MustCompile(`(.*?)/objects/info/packs$`), "GET", getInfoPacks}, + {regexp.MustCompile(`(.*?)/objects/info/[^/]*$`), "GET", getTextFile}, + {regexp.MustCompile(`(.*?)/objects/[0-9a-f]{2}/[0-9a-f]{38}$`), "GET", getLooseObject}, + {regexp.MustCompile(`(.*?)/objects/pack/pack-[0-9a-f]{40}\.pack$`), "GET", getPackFile}, + {regexp.MustCompile(`(.*?)/objects/pack/pack-[0-9a-f]{40}\.idx$`), "GET", getIdxFile}, } // FIXME: use process module @@ -393,7 +391,12 @@ func hasAccess(service string, h serviceHandler, checkContentType bool) bool { } func serviceRPC(h serviceHandler, service string) { - defer h.r.Body.Close() + defer func() { + if err := h.r.Body.Close(); err != nil { + log.Error("serviceRPC: Close: %v", err) + } + + }() if !hasAccess(service, h, true) { h.w.WriteHeader(http.StatusUnauthorized) @@ -469,9 +472,9 @@ func getInfoRefs(h serviceHandler) { h.w.Header().Set("Content-Type", fmt.Sprintf("application/x-git-%s-advertisement", service)) h.w.WriteHeader(http.StatusOK) - h.w.Write(packetWrite("# service=git-" + service + "\n")) - h.w.Write([]byte("0000")) - h.w.Write(refs) + _, _ = h.w.Write(packetWrite("# service=git-" + service + "\n")) + _, _ = h.w.Write([]byte("0000")) + _, _ = h.w.Write(refs) } else { updateServerInfo(h.dir) h.sendFile("text/plain; charset=utf-8") @@ -524,16 +527,25 @@ func HTTPBackend(ctx *context.Context, cfg *serviceConfig) http.HandlerFunc { if m := route.reg.FindStringSubmatch(r.URL.Path); m != nil { if setting.Repository.DisableHTTPGit { w.WriteHeader(http.StatusForbidden) - w.Write([]byte("Interacting with repositories by HTTP protocol is not allowed")) + _, err := w.Write([]byte("Interacting with repositories by HTTP protocol is not allowed")) + if err != nil { + log.Error(err.Error()) + } return } if route.method != r.Method { if r.Proto == "HTTP/1.1" { w.WriteHeader(http.StatusMethodNotAllowed) - w.Write([]byte("Method Not Allowed")) + _, err := w.Write([]byte("Method Not Allowed")) + if err != nil { + log.Error(err.Error()) + } } else { w.WriteHeader(http.StatusBadRequest) - w.Write([]byte("Bad Request")) + _, err := w.Write([]byte("Bad Request")) + if err != nil { + log.Error(err.Error()) + } } return } @@ -552,6 +564,5 @@ func HTTPBackend(ctx *context.Context, cfg *serviceConfig) http.HandlerFunc { } ctx.NotFound("HTTPBackend", nil) - return } } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index cd384da0d6..3904d29532 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -945,7 +945,15 @@ func ViewIssue(ctx *context.Context) { // Get Dependencies ctx.Data["BlockedByDependencies"], err = issue.BlockedByDependencies() + if err != nil { + ctx.ServerError("BlockedByDependencies", err) + return + } ctx.Data["BlockingDependencies"], err = issue.BlockingDependencies() + if err != nil { + ctx.ServerError("BlockingDependencies", err) + return + } ctx.Data["Participants"] = participants ctx.Data["NumParticipants"] = len(participants) @@ -1226,7 +1234,8 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) { if form.Status == "reopen" && issue.IsPull { pull := issue.PullRequest - pr, err := models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch) + var err error + pr, err = models.GetUnmergedPullRequest(pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch) if err != nil { if !models.IsErrPullRequestNotExist(err) { ctx.ServerError("GetUnmergedPullRequest", err) diff --git a/routers/repo/issue_label.go b/routers/repo/issue_label.go index 556a24c33e..cae6535c79 100644 --- a/routers/repo/issue_label.go +++ b/routers/repo/issue_label.go @@ -129,7 +129,6 @@ func DeleteLabel(ctx *context.Context) { ctx.JSON(200, map[string]interface{}{ "redirect": ctx.Repo.RepoLink + "/labels", }) - return } // UpdateIssueLabel change issue's labels diff --git a/routers/repo/milestone.go b/routers/repo/milestone.go index 644f7e043b..3ad638e60a 100644 --- a/routers/repo/milestone.go +++ b/routers/repo/milestone.go @@ -19,7 +19,6 @@ import ( const ( tplMilestone base.TplName = "repo/issue/milestones" tplMilestoneNew base.TplName = "repo/issue/milestone_new" - tplMilestoneEdit base.TplName = "repo/issue/milestone_edit" tplMilestoneIssues base.TplName = "repo/issue/milestone_issues" ) @@ -57,7 +56,7 @@ func Milestones(ctx *context.Context) { return } if ctx.Repo.Repository.IsTimetrackerEnabled() { - if miles.LoadTotalTrackedTimes(); err != nil { + if err := miles.LoadTotalTrackedTimes(); err != nil { ctx.ServerError("LoadTotalTrackedTimes", err) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 71c684356d..5be8aa57c1 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -793,10 +793,10 @@ func CleanUpPullRequest(ctx *context.Context) { // Forked repository has already been deleted ctx.NotFound("CleanUpPullRequest", nil) return - } else if pr.GetBaseRepo(); err != nil { + } else if err = pr.GetBaseRepo(); err != nil { ctx.ServerError("GetBaseRepo", err) return - } else if pr.HeadRepo.GetOwner(); err != nil { + } else if err = pr.HeadRepo.GetOwner(); err != nil { ctx.ServerError("HeadRepo.GetOwner", err) return } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 767cdacde0..757295069e 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -419,7 +419,10 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { return } - repo.DeleteWiki() + err := repo.DeleteWiki() + if err != nil { + log.Error("Delete Wiki: %v", err.Error()) + } log.Trace("Repository wiki deleted: %s/%s", ctx.Repo.Owner.Name, repo.Name) ctx.Flash.Success(ctx.Tr("repo.settings.wiki_deletion_success")) diff --git a/routers/repo/view.go b/routers/repo/view.go index 3483a53a0d..edaf24017c 100644 --- a/routers/repo/view.go +++ b/routers/repo/view.go @@ -294,9 +294,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st // Building code view blocks with line number on server side. var fileContent string if content, err := templates.ToUTF8WithErr(buf); err != nil { - if err != nil { - log.Error("ToUTF8WithErr: %v", err) - } + log.Error("ToUTF8WithErr: %v", err) fileContent = string(buf) } else { fileContent = content diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go index 8daf721b50..20a3a45c18 100644 --- a/routers/repo/webhook.go +++ b/routers/repo/webhook.go @@ -197,12 +197,20 @@ func WebHooksNewPost(ctx *context.Context, form auth.NewWebhookForm) { } // GogsHooksNewPost response for creating webhook -func GogsHooksNewPost(ctx *context.Context, form auth.NewGogshookForm) { +func GogsHooksNewPost(ctx *context.Context, form auth.NewWebhookForm) { + newGenericWebhookPost(ctx, form, models.GOGS) +} + +func newGenericWebhookPost(ctx *context.Context, form auth.NewWebhookForm, kind models.HookTaskType) { ctx.Data["Title"] = ctx.Tr("repo.settings.add_webhook") ctx.Data["PageIsSettingsHooks"] = true ctx.Data["PageIsSettingsHooksNew"] = true ctx.Data["Webhook"] = models.Webhook{HookEvent: &models.HookEvent{}} - ctx.Data["HookType"] = "gogs" + + ctx.Data["HookType"] = "gitea" + if kind == models.GOGS { + ctx.Data["HookType"] = "gogs" + } orCtx, err := getOrgRepoCtx(ctx) if err != nil { @@ -228,7 +236,7 @@ func GogsHooksNewPost(ctx *context.Context, form auth.NewGogshookForm) { Secret: form.Secret, HookEvent: ParseHookEvent(form.WebhookForm), IsActive: form.Active, - HookTaskType: models.GOGS, + HookTaskType: kind, OrgID: orCtx.OrgID, } if err := w.UpdateEvent(); err != nil { diff --git a/routers/routes/routes.go b/routers/routes/routes.go index f7ccfc43d2..744088a9d7 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -47,19 +47,6 @@ import ( macaron "gopkg.in/macaron.v1" ) -/*func giteaLogger(l *log.LoggerAsWriter) macaron.Handler { - return func(ctx *macaron.Context) { - start := time.Now() - - l.Log(fmt.Sprintf("[Macaron] Started %s %s for %s", ctx.Req.Method, ctx.Req.RequestURI, ctx.RemoteAddr())) - - ctx.Next() - - rw := ctx.Resp.(macaron.ResponseWriter) - l.Log(fmt.Sprintf("[Macaron] Completed %s %s %v %s in %v", ctx.Req.Method, ctx.Req.RequestURI, rw.Status(), http.StatusText(rw.Status()), time.Since(start))) - } -}*/ - type routerLoggerOptions struct { Ctx *macaron.Context Identity *string @@ -83,14 +70,20 @@ func setupAccessLogger(m *macaron.Macaron) { rw := ctx.Resp.(macaron.ResponseWriter) buf := bytes.NewBuffer([]byte{}) - logTemplate.Execute(buf, routerLoggerOptions{ + err := logTemplate.Execute(buf, routerLoggerOptions{ Ctx: ctx, Identity: &identity, Start: &start, ResponseWriter: &rw, }) + if err != nil { + log.Error("Could not set up macaron access logger: %v", err.Error()) + } - logger.SendLog(log.INFO, "", "", 0, buf.String(), "") + err = logger.SendLog(log.INFO, "", "", 0, buf.String(), "") + if err != nil { + log.Error("Could not set up macaron access logger: %v", err.Error()) + } }) } @@ -99,13 +92,13 @@ func RouterHandler(level log.Level) func(ctx *macaron.Context) { return func(ctx *macaron.Context) { start := time.Now() - log.GetLogger("router").Log(0, level, "Started %s %s for %s", log.ColoredMethod(ctx.Req.Method), ctx.Req.RequestURI, ctx.RemoteAddr()) + _ = log.GetLogger("router").Log(0, level, "Started %s %s for %s", log.ColoredMethod(ctx.Req.Method), ctx.Req.RequestURI, ctx.RemoteAddr()) rw := ctx.Resp.(macaron.ResponseWriter) ctx.Next() status := rw.Status() - log.GetLogger("router").Log(0, level, "Completed %s %s %v %s in %v", log.ColoredMethod(ctx.Req.Method), ctx.Req.RequestURI, log.ColoredStatus(status), log.ColoredStatus(status, http.StatusText(rw.Status())), log.ColoredTime(time.Since(start))) + _ = log.GetLogger("router").Log(0, level, "Completed %s %s %v %s in %v", log.ColoredMethod(ctx.Req.Method), ctx.Req.RequestURI, log.ColoredStatus(status), log.ColoredStatus(status, http.StatusText(rw.Status())), log.ColoredTime(time.Since(start))) } } @@ -443,14 +436,14 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/delete", admin.DeleteDefaultWebhook) m.Get("/:type/new", repo.WebhooksNew) m.Post("/gitea/new", bindIgnErr(auth.NewWebhookForm{}), repo.WebHooksNewPost) - m.Post("/gogs/new", bindIgnErr(auth.NewGogshookForm{}), repo.GogsHooksNewPost) + m.Post("/gogs/new", bindIgnErr(auth.NewWebhookForm{}), repo.GogsHooksNewPost) m.Post("/slack/new", bindIgnErr(auth.NewSlackHookForm{}), repo.SlackHooksNewPost) m.Post("/discord/new", bindIgnErr(auth.NewDiscordHookForm{}), repo.DiscordHooksNewPost) m.Post("/dingtalk/new", bindIgnErr(auth.NewDingtalkHookForm{}), repo.DingtalkHooksNewPost) m.Post("/msteams/new", bindIgnErr(auth.NewMSTeamsHookForm{}), repo.MSTeamsHooksNewPost) m.Get("/:id", repo.WebHooksEdit) m.Post("/gitea/:id", bindIgnErr(auth.NewWebhookForm{}), repo.WebHooksEditPost) - m.Post("/gogs/:id", bindIgnErr(auth.NewGogshookForm{}), repo.GogsHooksEditPost) + m.Post("/gogs/:id", bindIgnErr(auth.NewWebhookForm{}), repo.GogsHooksEditPost) m.Post("/slack/:id", bindIgnErr(auth.NewSlackHookForm{}), repo.SlackHooksEditPost) m.Post("/discord/:id", bindIgnErr(auth.NewDiscordHookForm{}), repo.DiscordHooksEditPost) m.Post("/dingtalk/:id", bindIgnErr(auth.NewDingtalkHookForm{}), repo.DingtalkHooksEditPost) @@ -582,7 +575,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/delete", org.DeleteWebhook) m.Get("/:type/new", repo.WebhooksNew) m.Post("/gitea/new", bindIgnErr(auth.NewWebhookForm{}), repo.WebHooksNewPost) - m.Post("/gogs/new", bindIgnErr(auth.NewGogshookForm{}), repo.GogsHooksNewPost) + m.Post("/gogs/new", bindIgnErr(auth.NewWebhookForm{}), repo.GogsHooksNewPost) m.Post("/slack/new", bindIgnErr(auth.NewSlackHookForm{}), repo.SlackHooksNewPost) m.Post("/discord/new", bindIgnErr(auth.NewDiscordHookForm{}), repo.DiscordHooksNewPost) m.Post("/dingtalk/new", bindIgnErr(auth.NewDingtalkHookForm{}), repo.DingtalkHooksNewPost) @@ -640,7 +633,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/delete", repo.DeleteWebhook) m.Get("/:type/new", repo.WebhooksNew) m.Post("/gitea/new", bindIgnErr(auth.NewWebhookForm{}), repo.WebHooksNewPost) - m.Post("/gogs/new", bindIgnErr(auth.NewGogshookForm{}), repo.GogsHooksNewPost) + m.Post("/gogs/new", bindIgnErr(auth.NewWebhookForm{}), repo.GogsHooksNewPost) m.Post("/slack/new", bindIgnErr(auth.NewSlackHookForm{}), repo.SlackHooksNewPost) m.Post("/discord/new", bindIgnErr(auth.NewDiscordHookForm{}), repo.DiscordHooksNewPost) m.Post("/dingtalk/new", bindIgnErr(auth.NewDingtalkHookForm{}), repo.DingtalkHooksNewPost) diff --git a/routers/user/auth.go b/routers/user/auth.go index b8f697b3ca..0731e34675 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -77,8 +77,14 @@ func AutoSignIn(ctx *context.Context) (bool, error) { } isSucceed = true - ctx.Session.Set("uid", u.ID) - ctx.Session.Set("uname", u.Name) + err = ctx.Session.Set("uid", u.ID) + if err != nil { + return false, err + } + err = ctx.Session.Set("uname", u.Name) + if err != nil { + return false, err + } ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) return true, nil } @@ -191,8 +197,16 @@ func SignInPost(ctx *context.Context, form auth.SignInForm) { } // User needs to use 2FA, save data and redirect to 2FA page. - ctx.Session.Set("twofaUid", u.ID) - ctx.Session.Set("twofaRemember", form.Remember) + err = ctx.Session.Set("twofaUid", u.ID) + if err != nil { + ctx.ServerError("UserSignIn", err) + return + } + err = ctx.Session.Set("twofaRemember", form.Remember) + if err != nil { + ctx.ServerError("UserSignIn", err) + return + } regs, err := models.GetU2FRegistrationsByUID(u.ID) if err == nil && len(regs) > 0 { @@ -383,6 +397,10 @@ func U2FChallenge(ctx *context.Context) { return } challenge, err := u2f.NewChallenge(setting.U2F.AppID, setting.U2F.TrustedFacets) + if err != nil { + ctx.ServerError("u2f.NewChallenge", err) + return + } if err = ctx.Session.Set("u2fChallenge", challenge); err != nil { ctx.ServerError("UserSignIn", err) return @@ -462,16 +480,22 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR setting.CookieRememberName, u.Name, days, setting.AppSubURL, "", setting.SessionConfig.Secure, true) } - ctx.Session.Delete("openid_verified_uri") - ctx.Session.Delete("openid_signin_remember") - ctx.Session.Delete("openid_determined_email") - ctx.Session.Delete("openid_determined_username") - ctx.Session.Delete("twofaUid") - ctx.Session.Delete("twofaRemember") - ctx.Session.Delete("u2fChallenge") - ctx.Session.Delete("linkAccount") - ctx.Session.Set("uid", u.ID) - ctx.Session.Set("uname", u.Name) + _ = ctx.Session.Delete("openid_verified_uri") + _ = ctx.Session.Delete("openid_signin_remember") + _ = ctx.Session.Delete("openid_determined_email") + _ = ctx.Session.Delete("openid_determined_username") + _ = ctx.Session.Delete("twofaUid") + _ = ctx.Session.Delete("twofaRemember") + _ = ctx.Session.Delete("u2fChallenge") + _ = ctx.Session.Delete("linkAccount") + err := ctx.Session.Set("uid", u.ID) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } + err = ctx.Session.Set("uname", u.Name) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } // Language setting of the user overwrites the one previously set // If the user does not have a locale set, we save the current one. @@ -563,7 +587,10 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context if u == nil { // no existing user is found, request attach or new account - ctx.Session.Set("linkAccountGothUser", gothUser) + err = ctx.Session.Set("linkAccountGothUser", gothUser) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } ctx.Redirect(setting.AppSubURL + "/user/link_account") return } @@ -573,8 +600,14 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context _, err = models.GetTwoFactorByUID(u.ID) if err != nil { if models.IsErrTwoFactorNotEnrolled(err) { - ctx.Session.Set("uid", u.ID) - ctx.Session.Set("uname", u.Name) + err = ctx.Session.Set("uid", u.ID) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } + err = ctx.Session.Set("uname", u.Name) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } // Clear whatever CSRF has right now, force to generate a new one ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) @@ -600,8 +633,14 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context } // User needs to use 2FA, save data and redirect to 2FA page. - ctx.Session.Set("twofaUid", u.ID) - ctx.Session.Set("twofaRemember", false) + err = ctx.Session.Set("twofaUid", u.ID) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } + err = ctx.Session.Set("twofaRemember", false) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } // If U2F is enrolled -> Redirect to U2F instead regs, err := models.GetU2FRegistrationsByUID(u.ID) @@ -760,9 +799,18 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) { } // User needs to use 2FA, save data and redirect to 2FA page. - ctx.Session.Set("twofaUid", u.ID) - ctx.Session.Set("twofaRemember", signInForm.Remember) - ctx.Session.Set("linkAccount", true) + err = ctx.Session.Set("twofaUid", u.ID) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } + err = ctx.Session.Set("twofaRemember", signInForm.Remember) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } + err = ctx.Session.Set("linkAccount", true) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } // If U2F is enrolled -> Redirect to U2F instead regs, err := models.GetU2FRegistrationsByUID(u.ID) @@ -897,11 +945,11 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au } func handleSignOut(ctx *context.Context) { - ctx.Session.Delete("uid") - ctx.Session.Delete("uname") - ctx.Session.Delete("socialId") - ctx.Session.Delete("socialName") - ctx.Session.Delete("socialEmail") + _ = ctx.Session.Delete("uid") + _ = ctx.Session.Delete("uname") + _ = ctx.Session.Delete("socialId") + _ = ctx.Session.Delete("socialName") + _ = ctx.Session.Delete("socialEmail") ctx.SetCookie(setting.CookieUserName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) @@ -1086,8 +1134,14 @@ func Activate(ctx *context.Context) { log.Trace("User activated: %s", user.Name) - ctx.Session.Set("uid", user.ID) - ctx.Session.Set("uname", user.Name) + err = ctx.Session.Set("uid", user.ID) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } + err = ctx.Session.Set("uname", user.Name) + if err != nil { + log.Error(fmt.Sprintf("Error setting session: %v", err)) + } ctx.Flash.Success(ctx.Tr("auth.account_activated")) ctx.Redirect(setting.AppSubURL + "/") return @@ -1113,7 +1167,6 @@ func ActivateEmail(ctx *context.Context) { } ctx.Redirect(setting.AppSubURL + "/user/settings/email") - return } // ForgotPasswd render the forget pasword page diff --git a/routers/user/auth_openid.go b/routers/user/auth_openid.go index 1351ca040b..f98c07acd7 100644 --- a/routers/user/auth_openid.go +++ b/routers/user/auth_openid.go @@ -126,7 +126,10 @@ func SignInOpenIDPost(ctx *context.Context, form auth.SignInOpenIDForm) { url += "&openid.sreg.optional=nickname%2Cemail" log.Trace("Form-passed openid-remember: %t", form.Remember) - ctx.Session.Set("openid_signin_remember", form.Remember) + err = ctx.Session.Set("openid_signin_remember", form.Remember) + if err != nil { + log.Error("SignInOpenIDPost: Could not set session: %v", err.Error()) + } ctx.Redirect(url) } @@ -152,7 +155,7 @@ func signInOpenIDVerify(ctx *context.Context) { /* Now we should seek for the user and log him in, or prompt * to register if not found */ - u, _ := models.GetUserByOpenID(id) + u, err := models.GetUserByOpenID(id) if err != nil { if !models.IsErrUserNotExist(err) { ctx.RenderWithErr(err.Error(), tplSignInOpenID, &auth.SignInOpenIDForm{ @@ -160,6 +163,7 @@ func signInOpenIDVerify(ctx *context.Context) { }) return } + log.Error("signInOpenIDVerify: %v", err) } if u != nil { log.Trace("User exists, logging in") @@ -191,7 +195,7 @@ func signInOpenIDVerify(ctx *context.Context) { log.Trace("User has email=" + email + " and nickname=" + nickname) if email != "" { - u, _ = models.GetUserByEmail(email) + u, err = models.GetUserByEmail(email) if err != nil { if !models.IsErrUserNotExist(err) { ctx.RenderWithErr(err.Error(), tplSignInOpenID, &auth.SignInOpenIDForm{ @@ -199,6 +203,7 @@ func signInOpenIDVerify(ctx *context.Context) { }) return } + log.Error("signInOpenIDVerify: %v", err) } if u != nil { log.Trace("Local user " + u.LowerName + " has OpenID provided email " + email) @@ -220,15 +225,24 @@ func signInOpenIDVerify(ctx *context.Context) { } } - ctx.Session.Set("openid_verified_uri", id) + err = ctx.Session.Set("openid_verified_uri", id) + if err != nil { + log.Error("signInOpenIDVerify: Could not set session: %v", err.Error()) + } - ctx.Session.Set("openid_determined_email", email) + err = ctx.Session.Set("openid_determined_email", email) + if err != nil { + log.Error("signInOpenIDVerify: Could not set session: %v", err.Error()) + } if u != nil { nickname = u.LowerName } - ctx.Session.Set("openid_determined_username", nickname) + err = ctx.Session.Set("openid_determined_username", nickname) + if err != nil { + log.Error("signInOpenIDVerify: Could not set session: %v", err.Error()) + } if u != nil || !setting.Service.EnableOpenIDSignUp { ctx.Redirect(setting.AppSubURL + "/user/openid/connect") @@ -350,7 +364,11 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si } if setting.Service.EnableCaptcha && setting.Service.CaptchaType == setting.ReCaptcha { - ctx.Req.ParseForm() + err := ctx.Req.ParseForm() + if err != nil { + ctx.ServerError("", err) + return + } valid, _ := recaptcha.Verify(form.GRecaptchaResponse) if !valid { ctx.Data["Err_Captcha"] = true diff --git a/routers/user/oauth.go b/routers/user/oauth.go index b85ea8125e..aaad26201b 100644 --- a/routers/user/oauth.go +++ b/routers/user/oauth.go @@ -7,12 +7,10 @@ package user import ( "encoding/base64" "fmt" + "github.com/go-macaron/binding" "net/url" "strings" - "github.com/dgrijalva/jwt-go" - "github.com/go-macaron/binding" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/base" @@ -20,6 +18,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" + + "github.com/dgrijalva/jwt-go" ) const ( @@ -164,6 +164,14 @@ func newAccessTokenResponse(grant *models.OAuth2Grant) (*AccessTokenResponse, *A func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) { errs := binding.Errors{} errs = form.Validate(ctx.Context, errs) + if len(errs) > 0 { + errstring := "" + for _, e := range errs { + errstring += e.Error() + "\n" + } + ctx.ServerError("AuthorizeOAuth: Validate: ", fmt.Errorf("errors occured during validation: %s", errstring)) + return + } app, err := models.GetOAuth2ApplicationByClientID(form.ClientID) if err != nil { @@ -221,7 +229,6 @@ func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) { }, form.RedirectURI) return } - break case "": break default: @@ -262,9 +269,24 @@ func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) { ctx.Data["ApplicationUserLink"] = "<a href=\"" + setting.AppURL + app.User.LowerName + "\">@" + app.User.Name + "</a>" ctx.Data["ApplicationRedirectDomainHTML"] = "<strong>" + form.RedirectURI + "</strong>" // TODO document SESSION <=> FORM - ctx.Session.Set("client_id", app.ClientID) - ctx.Session.Set("redirect_uri", form.RedirectURI) - ctx.Session.Set("state", form.State) + err = ctx.Session.Set("client_id", app.ClientID) + if err != nil { + handleServerError(ctx, form.State, form.RedirectURI) + log.Error(err.Error()) + return + } + err = ctx.Session.Set("redirect_uri", form.RedirectURI) + if err != nil { + handleServerError(ctx, form.State, form.RedirectURI) + log.Error(err.Error()) + return + } + err = ctx.Session.Set("state", form.State) + if err != nil { + handleServerError(ctx, form.State, form.RedirectURI) + log.Error(err.Error()) + return + } ctx.HTML(200, tplGrantAccess) } diff --git a/routers/user/profile.go b/routers/user/profile.go index bda29522d9..7df92d44f5 100644 --- a/routers/user/profile.go +++ b/routers/user/profile.go @@ -20,7 +20,6 @@ import ( const ( tplFollowers base.TplName = "user/meta/followers" - tplStars base.TplName = "user/meta/stars" ) // GetUserByName get user by name diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index ac5c4c97fb..163bc869b4 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -141,13 +141,11 @@ func UpdateAvatarSetting(ctx *context.Context, form auth.AvatarForm, ctxUser *mo if err = ctxUser.UploadAvatar(data); err != nil { return fmt.Errorf("UploadAvatar: %v", err) } - } else { + } else if ctxUser.UseCustomAvatar && !com.IsFile(ctxUser.CustomAvatarPath()) { // No avatar is uploaded but setting has been changed to enable, // generate a random one when needed. - if ctxUser.UseCustomAvatar && !com.IsFile(ctxUser.CustomAvatarPath()) { - if err := ctxUser.GenerateRandomAvatar(); err != nil { - log.Error("GenerateRandomAvatar[%d]: %v", ctxUser.ID, err) - } + if err := ctxUser.GenerateRandomAvatar(); err != nil { + log.Error("GenerateRandomAvatar[%d]: %v", ctxUser.ID, err) } } diff --git a/routers/user/setting/security_twofa.go b/routers/user/setting/security_twofa.go index fca1151a04..6e3516dbba 100644 --- a/routers/user/setting/security_twofa.go +++ b/routers/user/setting/security_twofa.go @@ -73,6 +73,10 @@ func twofaGenerateSecretAndQr(ctx *context.Context) bool { uri := ctx.Session.Get("twofaUri") if uri != nil { otpKey, err = otp.NewKeyFromURL(uri.(string)) + if err != nil { + ctx.ServerError("SettingsTwoFactor: NewKeyFromURL: ", err) + return false + } } // Filter unsafe character ':' in issuer issuer := strings.Replace(setting.AppName+" ("+setting.Domain+")", ":", "", -1) @@ -103,8 +107,16 @@ func twofaGenerateSecretAndQr(ctx *context.Context) bool { } ctx.Data["QrUri"] = template.URL("data:image/png;base64," + base64.StdEncoding.EncodeToString(imgBytes.Bytes())) - ctx.Session.Set("twofaSecret", otpKey.Secret()) - ctx.Session.Set("twofaUri", otpKey.String()) + err = ctx.Session.Set("twofaSecret", otpKey.Secret()) + if err != nil { + ctx.ServerError("SettingsTwoFactor", err) + return false + } + err = ctx.Session.Set("twofaUri", otpKey.String()) + if err != nil { + ctx.ServerError("SettingsTwoFactor", err) + return false + } return true } @@ -184,8 +196,16 @@ func EnrollTwoFactorPost(ctx *context.Context, form auth.TwoFactorAuthForm) { return } - ctx.Session.Delete("twofaSecret") - ctx.Session.Delete("twofaUri") + err = ctx.Session.Delete("twofaSecret") + if err != nil { + ctx.ServerError("SettingsTwoFactor", err) + return + } + err = ctx.Session.Delete("twofaUri") + if err != nil { + ctx.ServerError("SettingsTwoFactor", err) + return + } ctx.Flash.Success(ctx.Tr("settings.twofa_enrolled", token)) ctx.Redirect(setting.AppSubURL + "/user/settings/security") } diff --git a/routers/user/setting/security_u2f.go b/routers/user/setting/security_u2f.go index c1d6eab967..b733467b84 100644 --- a/routers/user/setting/security_u2f.go +++ b/routers/user/setting/security_u2f.go @@ -42,7 +42,11 @@ func U2FRegister(ctx *context.Context, form auth.U2FRegistrationForm) { return } } - ctx.Session.Set("u2fName", form.Name) + err = ctx.Session.Set("u2fName", form.Name) + if err != nil { + ctx.ServerError("", err) + return + } ctx.JSON(200, u2f.NewWebRegisterRequest(challenge, regs.ToRegistrations())) } @@ -95,5 +99,4 @@ func U2FDelete(ctx *context.Context, form auth.U2FDeleteForm) { ctx.JSON(200, map[string]interface{}{ "redirect": setting.AppSubURL + "/user/settings/security", }) - return } diff --git a/templates/user/meta/stars.tmpl b/templates/user/meta/stars.tmpl deleted file mode 100644 index e69de29bb2..0000000000 --- a/templates/user/meta/stars.tmpl +++ /dev/null |