issue: fix redirect to random issue if index does not exist (#4315)

10 files changed, 42 insertions, 54 deletions

diff --git a/gogs.go b/gogs.go
index 452a4c6b..64ef8546 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 d04f6d3d..d830a36f 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 0e327d15..72edb560 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 4ea898fb..903cc977 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 32e20b65..6bcda7a9 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 97b058a9..ffb52b9c 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 c359754b..dec64f41 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 3305b8ed..bd60890a 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 9184c5e8..aa61dcd9 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 bf9d4171..dd724fd0 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