From 756440e0a2774c8943d4f0295648f6648f2132f7 Mon Sep 17 00:00:00 2001
From: Anton Kaliaev <anton.kalyaev@gmail.com>
Date: Tue, 23 Jul 2019 12:25:59 +0400
Subject: [PATCH] =?UTF-8?q?rpc:=20return=20err=20if=20page=20is=20incorrec?=
 =?UTF-8?q?t=20(less=20than=200=20or=20greater=20than=20tot=E2=80=A6=20(#3?=
 =?UTF-8?q?825)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* rpc: return err if page is incorrect (less than 0 or greater than total pages)

Fixes #3813

* fix rpc_test
---
 CHANGELOG_PENDING.md  |  2 ++
 rpc/core/pipe.go      | 20 +++++++++++++-------
 rpc/core/pipe_test.go | 42 ++++++++++++++++++++++++------------------
 rpc/core/tx.go        |  5 ++++-
 4 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
index 721a376c7..ad8e60d35 100644
--- a/CHANGELOG_PENDING.md
+++ b/CHANGELOG_PENDING.md
@@ -27,3 +27,5 @@ program](https://hackerone.com/tendermint).
 - [mempool] \#3826 Make `max_msg_bytes` configurable
 
 ### BUG FIXES:
+
+- [rpc] \#3813 Return err if page is incorrect (less than 0 or greater than total pages)
diff --git a/rpc/core/pipe.go b/rpc/core/pipe.go
index a0fc7b9bb..9581b89d7 100644
--- a/rpc/core/pipe.go
+++ b/rpc/core/pipe.go
@@ -1,6 +1,7 @@
 package core
 
 import (
+	"fmt"
 	"time"
 
 	cfg "github.com/tendermint/tendermint/config"
@@ -145,19 +146,24 @@ func SetConfig(c cfg.RPCConfig) {
 	config = c
 }
 
-func validatePage(page, perPage, totalCount int) int {
+func validatePage(page, perPage, totalCount int) (int, error) {
 	if perPage < 1 {
-		return 1
+		panic(fmt.Sprintf("zero or negative perPage: %d", perPage))
+	}
+
+	if page == 0 {
+		return 1, nil // default
 	}
 
 	pages := ((totalCount - 1) / perPage) + 1
-	if page < 1 {
-		page = 1
-	} else if page > pages {
-		page = pages
+	if pages == 0 {
+		pages = 1 // one page (even if it's empty)
+	}
+	if page < 0 || page > pages {
+		return 1, fmt.Errorf("page should be within [0, %d] range, given %d", pages, page)
 	}
 
-	return page
+	return page, nil
 }
 
 func validatePerPage(perPage int) int {
diff --git a/rpc/core/pipe_test.go b/rpc/core/pipe_test.go
index 19ed11fcc..93aff3e58 100644
--- a/rpc/core/pipe_test.go
+++ b/rpc/core/pipe_test.go
@@ -14,33 +14,39 @@ func TestPaginationPage(t *testing.T) {
 		perPage    int
 		page       int
 		newPage    int
+		expErr     bool
 	}{
-		{0, 0, 1, 1},
+		{0, 10, 1, 1, false},
 
-		{0, 10, 0, 1},
-		{0, 10, 1, 1},
-		{0, 10, 2, 1},
+		{0, 10, 0, 1, false},
+		{0, 10, 1, 1, false},
+		{0, 10, 2, 0, true},
 
-		{5, 10, -1, 1},
-		{5, 10, 0, 1},
-		{5, 10, 1, 1},
-		{5, 10, 2, 1},
-		{5, 10, 2, 1},
+		{5, 10, -1, 0, true},
+		{5, 10, 0, 1, false},
+		{5, 10, 1, 1, false},
+		{5, 10, 2, 0, true},
+		{5, 10, 2, 0, true},
 
-		{5, 5, 1, 1},
-		{5, 5, 2, 1},
-		{5, 5, 3, 1},
+		{5, 5, 1, 1, false},
+		{5, 5, 2, 0, true},
+		{5, 5, 3, 0, true},
 
-		{5, 3, 2, 2},
-		{5, 3, 3, 2},
+		{5, 3, 2, 2, false},
+		{5, 3, 3, 0, true},
 
-		{5, 2, 2, 2},
-		{5, 2, 3, 3},
-		{5, 2, 4, 3},
+		{5, 2, 2, 2, false},
+		{5, 2, 3, 3, false},
+		{5, 2, 4, 0, true},
 	}
 
 	for _, c := range cases {
-		p := validatePage(c.page, c.perPage, c.totalCount)
+		p, err := validatePage(c.page, c.perPage, c.totalCount)
+		if c.expErr {
+			assert.Error(t, err)
+			continue
+		}
+
 		assert.Equal(t, c.newPage, p, fmt.Sprintf("%v", c))
 	}
 
diff --git a/rpc/core/tx.go b/rpc/core/tx.go
index 575553f85..dba457c30 100644
--- a/rpc/core/tx.go
+++ b/rpc/core/tx.go
@@ -202,7 +202,10 @@ func TxSearch(ctx *rpctypes.Context, query string, prove bool, page, perPage int
 
 	totalCount := len(results)
 	perPage = validatePerPage(perPage)
-	page = validatePage(page, perPage, totalCount)
+	page, err = validatePage(page, perPage, totalCount)
+	if err != nil {
+		return nil, err
+	}
 	skipCount := validateSkipCount(page, perPage)
 
 	apiResults := make([]*ctypes.ResultTx, cmn.MinInt(perPage, totalCount-skipCount))