From 3f3447a1ea8ed0d89ed862b7da506c97030d670e Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Sat, 14 Nov 2020 15:05:40 +0100
Subject: [PATCH] API: Fix GetQueryBeforeSince (#13559)

---
 routers/api/v1/notify/repo.go             |  2 +-
 routers/api/v1/notify/user.go             |  2 +-
 routers/api/v1/repo/issue_comment.go      |  4 +-
 routers/api/v1/repo/issue_tracked_time.go |  6 +--
 routers/api/v1/utils/utils.go             | 56 +++++++++++++++--------
 5 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/routers/api/v1/notify/repo.go b/routers/api/v1/notify/repo.go
index 49b493aa4f..cc66e0f743 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 9c3f9b1472..373c88d372 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 2da2f29063..245c90d49a 100644
--- a/routers/api/v1/repo/issue_comment.go
+++ b/routers/api/v1/repo/issue_comment.go
@@ -57,7 +57,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"))
@@ -133,7 +133,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 fd437753ab..67c47d858a 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 092ea3dbb6..ad1a136db4 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,30 +16,49 @@ 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
 	}
 
-	qCreatedAfter := strings.Trim(ctx.Query("since"), " ")
-	if qCreatedAfter != "" {
-		createdAfter, err := time.Parse(time.RFC3339, qCreatedAfter)
-		if err != nil {
-			return 0, 0, err
-		}
-		if !createdAfter.IsZero() {
-			since = createdAfter.Unix()
-		}
+	qCreatedSince, err := prepareQueryArg(ctx, "since")
+	if err != nil {
+		return 0, 0, err
+	}
+
+	before, err = parseTime(qCreatedBefore)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	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, err
+		}
+		if !t.IsZero() {
+			return t.Unix(), 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
 func GetListOptions(ctx *context.APIContext) models.ListOptions {
 	return models.ListOptions{