From e57ac841de1fc93df15ba8bef280077bbb733bf1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 28 Feb 2020 00:10:27 +0100 Subject: [PATCH] Fix potential bugs (#10513) * use e if it is an option * potential nil so check err first * check err first * m == nil already checked --- models/action.go | 2 +- models/attachment.go | 2 +- models/issue_comment.go | 6 +++++- models/notification.go | 2 +- modules/markup/common/linkify.go | 2 +- routers/repo/attachment.go | 8 ++++---- routers/repo/commit.go | 2 +- 7 files changed, 14 insertions(+), 10 deletions(-) diff --git a/models/action.go b/models/action.go index 267a19bc62..fd49c6d4ed 100644 --- a/models/action.go +++ b/models/action.go @@ -213,7 +213,7 @@ func (a *Action) getCommentLink(e Engine) string { return "#" } if a.Comment == nil && a.CommentID != 0 { - a.Comment, _ = GetCommentByID(a.CommentID) + a.Comment, _ = getCommentByID(e, a.CommentID) } if a.Comment != nil { return a.Comment.HTMLURL() diff --git a/models/attachment.go b/models/attachment.go index 81f2e15dad..7e59c7eef4 100644 --- a/models/attachment.go +++ b/models/attachment.go @@ -199,7 +199,7 @@ func GetAttachmentsByCommentID(commentID int64) ([]*Attachment, error) { func getAttachmentsByCommentID(e Engine, commentID int64) ([]*Attachment, error) { attachments := make([]*Attachment, 0, 10) - return attachments, x.Where("comment_id=?", commentID).Find(&attachments) + return attachments, e.Where("comment_id=?", commentID).Find(&attachments) } // getAttachmentByReleaseIDFileName return a file based on the the following infos: diff --git a/models/issue_comment.go b/models/issue_comment.go index 4abc3f9c1f..303cf3f9c1 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -765,8 +765,12 @@ func CreateRefComment(doer *User, repo *Repository, issue *Issue, content, commi // GetCommentByID returns the comment by given ID. func GetCommentByID(id int64) (*Comment, error) { + return getCommentByID(x, id) +} + +func getCommentByID(e Engine, id int64) (*Comment, error) { c := new(Comment) - has, err := x.ID(id).Get(c) + has, err := e.ID(id).Get(c) if err != nil { return nil, err } else if !has { diff --git a/models/notification.go b/models/notification.go index c52d6c557a..a7a65c38a4 100644 --- a/models/notification.go +++ b/models/notification.go @@ -396,7 +396,7 @@ func (n *Notification) loadIssue(e Engine) (err error) { func (n *Notification) loadComment(e Engine) (err error) { if n.Comment == nil && n.CommentID > 0 { - n.Comment, err = GetCommentByID(n.CommentID) + n.Comment, err = getCommentByID(e, n.CommentID) if err != nil { return fmt.Errorf("GetCommentByID [%d] for issue ID [%d]: %v", n.CommentID, n.IssueID, err) } diff --git a/modules/markup/common/linkify.go b/modules/markup/common/linkify.go index 6ae70fba34..25621bf801 100644 --- a/modules/markup/common/linkify.go +++ b/modules/markup/common/linkify.go @@ -108,7 +108,7 @@ func (s *linkifyParser) Parse(parent ast.Node, block text.Reader, pc parser.Cont } at := bytes.IndexByte(line, '@') m = []int{0, stop, at, stop - 1} - if m == nil || bytes.IndexByte(line[m[2]:m[3]], '.') < 0 { + if bytes.IndexByte(line[m[2]:m[3]], '.') < 0 { return nil } lastChar := line[m[1]-1] diff --git a/routers/repo/attachment.go b/routers/repo/attachment.go index 96dc28a23a..f938c230ed 100644 --- a/routers/repo/attachment.go +++ b/routers/repo/attachment.go @@ -70,14 +70,14 @@ func UploadAttachment(ctx *context.Context) { func DeleteAttachment(ctx *context.Context) { file := ctx.Query("file") attach, err := models.GetAttachmentByUUID(file) - if !ctx.IsSigned || (ctx.User.ID != attach.UploaderID) { - ctx.Error(403) - return - } if err != nil { ctx.Error(400, err.Error()) return } + if !ctx.IsSigned || (ctx.User.ID != attach.UploaderID) { + ctx.Error(403) + return + } err = models.DeleteAttachment(attach, true) if err != nil { ctx.Error(500, fmt.Sprintf("DeleteAttachment: %v", err)) diff --git a/routers/repo/commit.go b/routers/repo/commit.go index 2767986fda..77439f8873 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -244,11 +244,11 @@ func Diff(ctx *context.Context) { parents := make([]string, commit.ParentCount()) for i := 0; i < commit.ParentCount(); i++ { sha, err := commit.ParentID(i) - parents[i] = sha.String() if err != nil { ctx.NotFound("repo.Diff", err) return } + parents[i] = sha.String() } ctx.Data["CommitID"] = commitID