thepaperpilot/forgejo
1
0
Fork 0
Code Issues Pull requests Releases Activity Actions

[GITEA] Fix /issues/search endpoint

Browse source 
- The endpoint was moved from being an API endpoint to an web endpoint
with JSON result. However the API context isn't the same as the web
context, for example the `ctx.Error` only takes in the first two
arguments into consideration and doesn't do logging, which is not the
same behavior as the API context where there's three arguments and does
do logging and only reveal the function + error if the user is admin.
- Remove any details in the error message and do the logging seperatly,
this is somewhat consistent with how other API endpoints behave.
- Ref: https://codeberg.org/forgejo/forgejo/issues/1998
This commit is contained in:
Gusted 2023-12-22 17:20:50 +01:00
parent caff546249
commit fe71e32ace
No known key found for this signature in database
GPG key ID: FD821B732837125F
1 changed files with 18 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
routers/web/repo/issue.go
View file

@ -2482,7 +2482,8 @@ func UpdatePullReviewRequest(ctx *context.Context) {
func SearchIssues(ctx *context.Context) { func SearchIssues(ctx *context.Context) {
before, since, err := context.GetQueryBeforeSince(ctx.Base) before, since, err := context.GetQueryBeforeSince(ctx.Base)
if err != nil { if err != nil {
ctx.Error(http.StatusUnprocessableEntity, err.Error()) log.Error("GetQueryBeforeSince: %v", err)
ctx.Error(http.StatusUnprocessableEntity, "invalid before or since")
return return
} }
@ -2519,10 +2520,11 @@ func SearchIssues(ctx *context.Context) {
if ctx.FormString("owner") != "" { if ctx.FormString("owner") != "" {
owner, err := user_model.GetUserByName(ctx, ctx.FormString("owner")) owner, err := user_model.GetUserByName(ctx, ctx.FormString("owner"))
if err != nil { if err != nil {
log.Error("GetUserByName: %v", err)
if user_model.IsErrUserNotExist(err) { if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusBadRequest, "Owner not found", err.Error()) ctx.Error(http.StatusBadRequest, "Owner not found", err.Error())
} else { } else {
ctx.Error(http.StatusInternalServerError, "GetUserByName", err.Error()) ctx.Error(http.StatusInternalServerError)
} }
return return
} }
@ -2533,15 +2535,16 @@ func SearchIssues(ctx *context.Context) {
} }
if ctx.FormString("team") != "" { if ctx.FormString("team") != "" {
if ctx.FormString("owner") == "" { if ctx.FormString("owner") == "" {
ctx.Error(http.StatusBadRequest, "", "Owner organisation is required for filtering on team") ctx.Error(http.StatusBadRequest, "Owner organisation is required for filtering on team")
return return
} }
team, err := organization.GetTeam(ctx, opts.OwnerID, ctx.FormString("team")) team, err := organization.GetTeam(ctx, opts.OwnerID, ctx.FormString("team"))
if err != nil { if err != nil {
log.Error("GetTeam: %v", err)
if organization.IsErrTeamNotExist(err) { if organization.IsErrTeamNotExist(err) {
ctx.Error(http.StatusBadRequest, "Team not found", err.Error()) ctx.Error(http.StatusBadRequest)
} else { } else {
ctx.Error(http.StatusInternalServerError, "GetUserByName", err.Error()) ctx.Error(http.StatusInternalServerError)
} }
return return
} }
@ -2554,7 +2557,8 @@ func SearchIssues(ctx *context.Context) {
} }
repoIDs, _, err = repo_model.SearchRepositoryIDs(ctx, opts) repoIDs, _, err = repo_model.SearchRepositoryIDs(ctx, opts)
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "SearchRepositoryIDs", err.Error()) log.Error("SearchRepositoryIDs: %v", err)
ctx.Error(http.StatusInternalServerError)
return return
} }
if len(repoIDs) == 0 { if len(repoIDs) == 0 {
@ -2588,7 +2592,8 @@ func SearchIssues(ctx *context.Context) {
} }
includedAnyLabels, err = issues_model.GetLabelIDsByNames(ctx, includedLabelNames) includedAnyLabels, err = issues_model.GetLabelIDsByNames(ctx, includedLabelNames)
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "GetLabelIDsByNames", err.Error()) log.Error("GetLabelIDsByNames: %v", err)
ctx.Error(http.StatusInternalServerError)
return return
} }
} }
@ -2602,7 +2607,8 @@ func SearchIssues(ctx *context.Context) {
} }
includedMilestones, err = issues_model.GetMilestoneIDsByNames(ctx, includedMilestoneNames) includedMilestones, err = issues_model.GetMilestoneIDsByNames(ctx, includedMilestoneNames)
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "GetMilestoneIDsByNames", err.Error()) log.Error("GetMilestoneIDsByNames: %v", err)
ctx.Error(http.StatusInternalServerError)
return return
} }
} }
@ -2669,12 +2675,14 @@ func SearchIssues(ctx *context.Context) {
ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt) ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt)
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "SearchIssues", err.Error()) log.Error("SearchIssues: %v", err)
ctx.Error(http.StatusInternalServerError)
return return
} }
issues, err := issues_model.GetIssuesByIDs(ctx, ids, true) issues, err := issues_model.GetIssuesByIDs(ctx, ids, true)
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "FindIssuesByIDs", err.Error()) log.Error("GetIssuesByIDs: %v", err)
ctx.Error(http.StatusInternalServerError)
return return
} }