Wrap invalid cursor in claimable balances query in problem.P (#3088)

We do cursor validation, however, if there is an error we return a
plain error instead of a `problem.P` -- this causes horizon to handle
the error as a 500 instead of a 400.

This fix moves cursor parsing to its own method and then use it in the
query handler to return a `problem.P` error.

Fixes #3085.

services/horizon/internal/actions/claimable_balance.go

@@ -155,6 +155,14 @@ func (handler GetClaimableBalancesHandler) GetResourcePage(
 		Claimant:  qp.claimant(),
 	}
 
+	_, _, err = query.Cursor()
+	if err != nil {
+		return nil, problem.MakeInvalidFieldProblem(
+			"cursor",
+			errors.New("First part should be higher than 0 and second part should be valid claimable balance ID"),
+		)
+	}
+
 	historyQ, err := horizonContext.HistoryQFromRequest(r)
 	if err != nil {
 		return nil, err

services/horizon/internal/actions/claimable_balance_test.go

@@ -524,12 +524,54 @@ func TestGetClaimableBalances(t *testing.T) {
 
 	tt.Assert.NoError(err)
 	tt.Assert.Len(response, 2)
-	// for _, resource := range response {
-	// 	tt.Assert.Equal(
-	// 		"GC3C4AKRBQLHOJ45U4XG35ESVWRDECWO5XLDGYADO6DPR3L7KIDVUMML",
-	// 		resource.(protocol.ClaimableBalance).Sponsor,
-	// 	)
-	// }
+}
+
+func TestCursorAndOrderValidation(t *testing.T) {
+	tt := test.Start(t)
+	defer tt.Finish()
+	test.ResetHorizonDB(t, tt.HorizonDB)
+	q := &history.Q{tt.HorizonSession()}
+
+	handler := GetClaimableBalancesHandler{}
+	_, err := handler.GetResourcePage(httptest.NewRecorder(), makeRequest(
+		t,
+		map[string]string{
+			"cursor": "-1-00000043d380c38a2f2cac46ab63674064c56fdce6b977fdef1a278ad50e1a7e6a5e18",
+		},
+		map[string]string{},
+		q.Session,
+	))
+	p := err.(*problem.P)
+	tt.Assert.Equal("bad_request", p.Type)
+	tt.Assert.Equal("cursor", p.Extras["invalid_field"])
+	tt.Assert.Equal("First part should be higher than 0 and second part should be valid claimable balance ID", p.Extras["reason"])
+
+	_, err = handler.GetResourcePage(httptest.NewRecorder(), makeRequest(
+		t,
+		map[string]string{
+			"cursor": "1003529-00000043d380c38a2f2cac46ab63674064c56fdce6b977fdef1a278ad50e1a7e6a5e18",
+		},
+		map[string]string{},
+		q.Session,
+	))
+	p = err.(*problem.P)
+	tt.Assert.Equal("bad_request", p.Type)
+	tt.Assert.Equal("cursor", p.Extras["invalid_field"])
+	tt.Assert.Equal("First part should be higher than 0 and second part should be valid claimable balance ID", p.Extras["reason"])
+
+	_, err = handler.GetResourcePage(httptest.NewRecorder(), makeRequest(
+		t,
+		map[string]string{
+			"order":  "arriba",
+			"cursor": "1003529-00000043d380c38a2f2cac46ab63674064c56fdce6b977fdef1a278ad50e1a7e6a5e18",
+		},
+		map[string]string{},
+		q.Session,
+	))
+	p = err.(*problem.P)
+	tt.Assert.Equal("bad_request", p.Type)
+	tt.Assert.Equal("order", p.Extras["invalid_field"])
+	tt.Assert.Equal("order: invalid value", p.Extras["reason"])
 }
 
 func TestClaimableBalancesQueryURLTemplate(t *testing.T) {

services/horizon/internal/db2/history/claimable_balances.go

@@ -22,10 +22,8 @@ type ClaimableBalancesQuery struct {
 	Claimant  *xdr.AccountId
 }
 
-// ApplyCursor applies cursor to the given sql. For performance reason the limit
-// is not apply here. This allows us to hint the planner later to use the right
-// indexes.
-func (cbq ClaimableBalancesQuery) ApplyCursor(sql sq.SelectBuilder) (sq.SelectBuilder, error) {
+// Cursor validates and returns the query page cursor
+func (cbq ClaimableBalancesQuery) Cursor() (int64, *xdr.ClaimableBalanceId, error) {
 	p := cbq.PageQuery
 	var l int64
 	var r *xdr.ClaimableBalanceId
@@ -34,24 +32,37 @@ func (cbq ClaimableBalancesQuery) ApplyCursor(sql sq.SelectBuilder) (sq.SelectBu
 	if p.Cursor != "" {
 		parts := strings.SplitN(p.Cursor, "-", 2)
 		if len(parts) != 2 {
-			return sql, errors.New("Invalid cursor")
+			return l, r, errors.New("Invalid cursor")
 		}
 
 		l, err = strconv.ParseInt(parts[0], 10, 64)
 		if err != nil {
-			return sql, errors.Wrap(err, "Invalid cursor - first value should be higher than 0")
+			return l, r, errors.Wrap(err, "Invalid cursor - first value should be higher than 0")
 		}
 
 		var balanceID xdr.ClaimableBalanceId
 		if err = xdr.SafeUnmarshalHex(parts[1], &balanceID); err != nil {
-			return sql, errors.Wrap(err, "Invalid cursor - second value should be a valid claimable balance id")
+			return l, r, errors.Wrap(err, "Invalid cursor - second value should be a valid claimable balance id")
 		}
 		r = &balanceID
 		if l < 0 {
-			return sql, errors.Wrap(err, "Invalid cursor - first value should be higher than 0")
+			return l, r, errors.Wrap(err, "Invalid cursor - first value should be higher than 0")
 		}
 	}
 
+	return l, r, nil
+}
+
+// ApplyCursor applies cursor to the given sql. For performance reason the limit
+// is not apply here. This allows us to hint the planner later to use the right
+// indexes.
+func (cbq ClaimableBalancesQuery) ApplyCursor(sql sq.SelectBuilder) (sq.SelectBuilder, error) {
+	p := cbq.PageQuery
+	l, r, err := cbq.Cursor()
+	if err != nil {
+		return sql, err
+	}
+
 	switch p.Order {
 	case db2.OrderAscending:
 		if l > 0 && r != nil {