From 85a050fca70446ea7a9e10266bbe083d5d8b63ea Mon Sep 17 00:00:00 2001 From: Unknwon Date: Wed, 22 Mar 2017 13:20:29 -0400 Subject: [PATCH] issue: fix redirect to random issue if index does not exist (#4315) --- gogs.go | 2 +- models/action.go | 6 +++--- models/error.go | 22 ---------------------- models/errors/issue.go | 15 +++++++++++++++ models/issue.go | 4 ++-- routers/api/v1/repo/issue.go | 4 ++-- routers/api/v1/repo/issue_label.go | 11 ++++++----- routers/repo/issue.go | 24 +++++++++++------------- routers/repo/pull.go | 6 +----- templates/.VERSION | 2 +- 10 files changed, 42 insertions(+), 54 deletions(-) diff --git a/gogs.go b/gogs.go index 452a4c6b3..64ef85465 100644 --- a/gogs.go +++ b/gogs.go @@ -16,7 +16,7 @@ import ( "github.com/gogits/gogs/modules/setting" ) -const APP_VER = "0.10.24.0320" +const APP_VER = "0.10.25.0322" func init() { setting.AppVer = APP_VER diff --git a/models/action.go b/models/action.go index d04f6d3dc..d830a36fe 100644 --- a/models/action.go +++ b/models/action.go @@ -343,7 +343,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err issue, err := GetIssueByRef(ref) if err != nil { - if IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { continue } return err @@ -386,7 +386,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err issue, err := GetIssueByRef(ref) if err != nil { - if IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { continue } return err @@ -426,7 +426,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err issue, err := GetIssueByRef(ref) if err != nil { - if IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { continue } return err diff --git a/models/error.go b/models/error.go index 0e327d157..72edb560e 100644 --- a/models/error.go +++ b/models/error.go @@ -436,28 +436,6 @@ func (err ErrBranchNotExist) Error() string { return fmt.Sprintf("branch does not exist [name: %s]", err.Name) } -// .___ -// | | ______ ________ __ ____ -// | |/ ___// ___/ | \_/ __ \ -// | |\___ \ \___ \| | /\ ___/ -// |___/____ >____ >____/ \___ > -// \/ \/ \/ - -type ErrIssueNotExist struct { - ID int64 - RepoID int64 - Index int64 -} - -func IsErrIssueNotExist(err error) bool { - _, ok := err.(ErrIssueNotExist) - return ok -} - -func (err ErrIssueNotExist) Error() string { - return fmt.Sprintf("issue does not exist [id: %d, repo_id: %d, index: %d]", err.ID, err.RepoID, err.Index) -} - // __________ .__ .__ __________ __ // \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_ // | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ diff --git a/models/errors/issue.go b/models/errors/issue.go index 4ea898fb9..903cc977e 100644 --- a/models/errors/issue.go +++ b/models/errors/issue.go @@ -6,6 +6,21 @@ package errors import "fmt" +type IssueNotExist struct { + ID int64 + RepoID int64 + Index int64 +} + +func IsIssueNotExist(err error) bool { + _, ok := err.(IssueNotExist) + return ok +} + +func (err IssueNotExist) Error() string { + return fmt.Sprintf("issue does not exist [id: %d, repo_id: %d, index: %d]", err.ID, err.RepoID, err.Index) +} + type InvalidIssueReference struct { Ref string } diff --git a/models/issue.go b/models/issue.go index 32e20b65b..6bcda7a99 100644 --- a/models/issue.go +++ b/models/issue.go @@ -827,7 +827,7 @@ func GetRawIssueByIndex(repoID, index int64) (*Issue, error) { if err != nil { return nil, err } else if !has { - return nil, ErrIssueNotExist{0, repoID, index} + return nil, errors.IssueNotExist{0, repoID, index} } return issue, nil } @@ -847,7 +847,7 @@ func getRawIssueByID(e Engine, id int64) (*Issue, error) { if err != nil { return nil, err } else if !has { - return nil, ErrIssueNotExist{id, 0, 0} + return nil, errors.IssueNotExist{id, 0, 0} } return issue, nil } diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 97b058a9d..ffb52b9c3 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -64,7 +64,7 @@ func ListIssues(ctx *context.APIContext) { func GetIssue(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) @@ -126,7 +126,7 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) { func EditIssue(ctx *context.APIContext, form api.EditIssueOption) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index c359754b2..dec64f413 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -8,13 +8,14 @@ import ( api "github.com/gogits/go-gogs-client" "github.com/gogits/gogs/models" + "github.com/gogits/gogs/models/errors" "github.com/gogits/gogs/modules/context" ) func ListIssueLabels(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) @@ -37,7 +38,7 @@ func AddIssueLabels(ctx *context.APIContext, form api.IssueLabelsOption) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) @@ -77,7 +78,7 @@ func DeleteIssueLabel(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) @@ -111,7 +112,7 @@ func ReplaceIssueLabels(ctx *context.APIContext, form api.IssueLabelsOption) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) @@ -151,7 +152,7 @@ func ClearIssueLabels(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { + if errors.IsIssueNotExist(err) { ctx.Status(404) } else { ctx.Error(500, "GetIssueByIndex", err) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 3305b8ed0..bd60890ad 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -5,7 +5,6 @@ package repo import ( - "errors" "fmt" "io" "io/ioutil" @@ -19,6 +18,7 @@ import ( log "gopkg.in/clog.v1" "github.com/gogits/gogs/models" + "github.com/gogits/gogs/models/errors" "github.com/gogits/gogs/modules/base" "github.com/gogits/gogs/modules/context" "github.com/gogits/gogs/modules/form" @@ -497,13 +497,15 @@ func ViewIssue(ctx *context.Context) { ctx.Data["RequireDropzone"] = true renderAttachmentSettings(ctx) - issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + index := ctx.ParamsInt64(":index") + if index <= 0 { + ctx.NotFound() + return + } + + issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, index) if err != nil { - if models.IsErrIssueNotExist(err) { - ctx.Handle(404, "GetIssueByIndex", err) - } else { - ctx.Handle(500, "GetIssueByIndex", err) - } + ctx.NotFoundOrServerError("GetIssueByIndex", errors.IsIssueNotExist, err) return } ctx.Data["Title"] = issue.Title @@ -653,11 +655,7 @@ func ViewIssue(ctx *context.Context) { func getActionIssue(ctx *context.Context) *models.Issue { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { - ctx.Error(404, "GetIssueByIndex") - } else { - ctx.Handle(500, "GetIssueByIndex", err) - } + ctx.NotFoundOrServerError("GetIssueByIndex", errors.IsIssueNotExist, err) return nil } return issue @@ -807,7 +805,7 @@ func UpdateIssueAssignee(ctx *context.Context) { func NewComment(ctx *context.Context, f form.CreateComment) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - ctx.NotFoundOrServerError("GetIssueByIndex", models.IsErrIssueNotExist, err) + ctx.NotFoundOrServerError("GetIssueByIndex", errors.IsIssueNotExist, err) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 9184c5e8e..aa61dcd93 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -142,11 +142,7 @@ func ForkPost(ctx *context.Context, f form.CreateRepo) { func checkPullInfo(ctx *context.Context) *models.Issue { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - if models.IsErrIssueNotExist(err) { - ctx.Handle(404, "GetIssueByIndex", err) - } else { - ctx.Handle(500, "GetIssueByIndex", err) - } + ctx.NotFoundOrServerError("GetIssueByIndex", errors.IsIssueNotExist, err) return nil } ctx.Data["Title"] = issue.Title diff --git a/templates/.VERSION b/templates/.VERSION index bf9d41714..dd724fd08 100644 --- a/templates/.VERSION +++ b/templates/.VERSION @@ -1 +1 @@ -0.10.24.0320 \ No newline at end of file +0.10.25.0322 \ No newline at end of file