API: Fix GetQueryBeforeSince (#13559) (#13560)

* API: Fix GetQueryBeforeSince

* CI.restart()

Co-authored-by: zeripath <art27@cantab.net>

5 files changed, 43 insertions, 23 deletions

diff --git a/routers/api/v1/notify/repo.go b/routers/api/v1/notify/repo.go
index ca9aea256..a37dfd8b4 100644
--- a/routers/api/v1/notify/repo.go
+++ b/routers/api/v1/notify/repo.go
@@ -101,7 +101,7 @@ func ListRepoNotifications(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 	opts := models.FindNotificationOptions{
diff --git a/routers/api/v1/notify/user.go b/routers/api/v1/notify/user.go
index 08a2f0841..57127c96f 100644
--- a/routers/api/v1/notify/user.go
+++ b/routers/api/v1/notify/user.go
@@ -63,7 +63,7 @@ func ListNotifications(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 	opts := models.FindNotificationOptions{
diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go
index 6b7c2beac..064a37bc1 100644
--- a/routers/api/v1/repo/issue_comment.go
+++ b/routers/api/v1/repo/issue_comment.go
@@ -56,7 +56,7 @@ func ListIssueComments(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "GetQueryBeforeSince", err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
@@ -132,7 +132,7 @@ func ListRepoIssueComments(ctx *context.APIContext) {
 
 	before, since, err := utils.GetQueryBeforeSince(ctx)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "GetQueryBeforeSince", err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 
diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go
index 19242d1fa..765bfc0b6 100644
--- a/routers/api/v1/repo/issue_tracked_time.go
+++ b/routers/api/v1/repo/issue_tracked_time.go
@@ -86,7 +86,7 @@ func ListTrackedTimes(ctx *context.APIContext) {
 	}
 
 	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 
@@ -491,7 +491,7 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) {
 
 	var err error
 	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 
@@ -554,7 +554,7 @@ func ListMyTrackedTimes(ctx *context.APIContext) {
 
 	var err error
 	if opts.CreatedBeforeUnix, opts.CreatedAfterUnix, err = utils.GetQueryBeforeSince(ctx); err != nil {
-		ctx.InternalServerError(err)
+		ctx.Error(http.StatusUnprocessableEntity, "GetQueryBeforeSince", err)
 		return
 	}
 
diff --git a/routers/api/v1/utils/utils.go b/routers/api/v1/utils/utils.go
index 092ea3dbb..ad1a136db 100644
--- a/routers/api/v1/utils/utils.go
+++ b/routers/api/v1/utils/utils.go
@@ -5,6 +5,7 @@
 package utils
 
 import (
+	"net/url"
 	"strings"
 	"time"
 
@@ -15,28 +16,47 @@ import (
 
 // GetQueryBeforeSince return parsed time (unix format) from URL query's before and since
 func GetQueryBeforeSince(ctx *context.APIContext) (before, since int64, err error) {
-	qCreatedBefore := strings.Trim(ctx.Query("before"), " ")
-	if qCreatedBefore != "" {
-		createdBefore, err := time.Parse(time.RFC3339, qCreatedBefore)
-		if err != nil {
-			return 0, 0, err
-		}
-		if !createdBefore.IsZero() {
-			before = createdBefore.Unix()
-		}
+	qCreatedBefore, err := prepareQueryArg(ctx, "before")
+	if err != nil {
+		return 0, 0, err
+	}
+
+	qCreatedSince, err := prepareQueryArg(ctx, "since")
+	if err != nil {
+		return 0, 0, err
+	}
+
+	before, err = parseTime(qCreatedBefore)
+	if err != nil {
+		return 0, 0, err
 	}
 
-	qCreatedAfter := strings.Trim(ctx.Query("since"), " ")
-	if qCreatedAfter != "" {
-		createdAfter, err := time.Parse(time.RFC3339, qCreatedAfter)
+	since, err = parseTime(qCreatedSince)
+	if err != nil {
+		return 0, 0, err
+	}
+	return before, since, nil
+}
+
+// parseTime parse time and return unix timestamp
+func parseTime(value string) (int64, error) {
+	if len(value) != 0 {
+		t, err := time.Parse(time.RFC3339, value)
 		if err != nil {
-			return 0, 0, err
+			return 0, err
 		}
-		if !createdAfter.IsZero() {
-			since = createdAfter.Unix()
+		if !t.IsZero() {
+			return t.Unix(), nil
 		}
 	}
-	return before, since, nil
+	return 0, nil
+}
+
+// prepareQueryArg unescape and trim a query arg
+func prepareQueryArg(ctx *context.APIContext, name string) (value string, err error) {
+	value, err = url.PathUnescape(ctx.Query(name))
+	value = strings.Trim(value, " ")
+	return
 }
 
 // GetListOptions returns list options using the page and limit parameters