From 7b70fa9392cc03121c798407363712d6e5dde536 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 22 Dec 2023 17:20:50 +0100 Subject: [PATCH] [GITEA] Fix /issues/search endpoint - 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 (cherry picked from commit fe71e32ace98461398cffe55f99ad31dc1be0b4e) (cherry picked from commit c89e0735fab6b3994ff1769afafb012d1147972f) (cherry picked from commit 4c04dcfc59c1a23b990f9a81c73de7cbfd95d1e3) (cherry picked from commit 66eae1041c3b6bd4f15bbbaf552678313bcae835) --- routers/web/repo/issue.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index c8c9924a9e..9da62bb00b 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2501,7 +2501,8 @@ func UpdatePullReviewRequest(ctx *context.Context) { func SearchIssues(ctx *context.Context) { before, since, err := context.GetQueryBeforeSince(ctx.Base) if err != nil { - ctx.Error(http.StatusUnprocessableEntity, err.Error()) + log.Error("GetQueryBeforeSince: %v", err) + ctx.Error(http.StatusUnprocessableEntity, "invalid before or since") return } @@ -2538,10 +2539,11 @@ func SearchIssues(ctx *context.Context) { if ctx.FormString("owner") != "" { owner, err := user_model.GetUserByName(ctx, ctx.FormString("owner")) if err != nil { + log.Error("GetUserByName: %v", err) if user_model.IsErrUserNotExist(err) { ctx.Error(http.StatusBadRequest, "Owner not found", err.Error()) } else { - ctx.Error(http.StatusInternalServerError, "GetUserByName", err.Error()) + ctx.Error(http.StatusInternalServerError) } return } @@ -2552,15 +2554,16 @@ func SearchIssues(ctx *context.Context) { } if ctx.FormString("team") != "" { 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 } team, err := organization.GetTeam(ctx, opts.OwnerID, ctx.FormString("team")) if err != nil { + log.Error("GetTeam: %v", err) if organization.IsErrTeamNotExist(err) { - ctx.Error(http.StatusBadRequest, "Team not found", err.Error()) + ctx.Error(http.StatusBadRequest) } else { - ctx.Error(http.StatusInternalServerError, "GetUserByName", err.Error()) + ctx.Error(http.StatusInternalServerError) } return } @@ -2573,7 +2576,8 @@ func SearchIssues(ctx *context.Context) { } repoIDs, _, err = repo_model.SearchRepositoryIDs(ctx, opts) if err != nil { - ctx.Error(http.StatusInternalServerError, "SearchRepositoryIDs", err.Error()) + log.Error("SearchRepositoryIDs: %v", err) + ctx.Error(http.StatusInternalServerError) return } if len(repoIDs) == 0 { @@ -2607,7 +2611,8 @@ func SearchIssues(ctx *context.Context) { } includedAnyLabels, err = issues_model.GetLabelIDsByNames(ctx, includedLabelNames) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetLabelIDsByNames", err.Error()) + log.Error("GetLabelIDsByNames: %v", err) + ctx.Error(http.StatusInternalServerError) return } } @@ -2621,7 +2626,8 @@ func SearchIssues(ctx *context.Context) { } includedMilestones, err = issues_model.GetMilestoneIDsByNames(ctx, includedMilestoneNames) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetMilestoneIDsByNames", err.Error()) + log.Error("GetMilestoneIDsByNames: %v", err) + ctx.Error(http.StatusInternalServerError) return } } @@ -2688,12 +2694,14 @@ func SearchIssues(ctx *context.Context) { ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt) if err != nil { - ctx.Error(http.StatusInternalServerError, "SearchIssues", err.Error()) + log.Error("SearchIssues: %v", err) + ctx.Error(http.StatusInternalServerError) return } issues, err := issues_model.GetIssuesByIDs(ctx, ids, true) if err != nil { - ctx.Error(http.StatusInternalServerError, "FindIssuesByIDs", err.Error()) + log.Error("GetIssuesByIDs: %v", err) + ctx.Error(http.StatusInternalServerError) return }