From 8dc655dad25b0b04f271cb66ba73fd504db3195d Mon Sep 17 00:00:00 2001
From: Ethan Buchman <ethan@coinculture.info>
Date: Tue, 24 Jul 2018 20:35:36 -0400
Subject: [PATCH] rpc: fix /blockchain OOM #2049

---
 rpc/core/blocks.go      | 51 +++++++++++++++++++++++++-----------
 rpc/core/blocks_test.go | 58 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 15 deletions(-)
 create mode 100644 rpc/core/blocks_test.go

diff --git a/rpc/core/blocks.go b/rpc/core/blocks.go
index 5815b60e3..671250db8 100644
--- a/rpc/core/blocks.go
+++ b/rpc/core/blocks.go
@@ -64,25 +64,15 @@ import (
 //
 // <aside class="notice">Returns at most 20 items.</aside>
 func BlockchainInfo(minHeight, maxHeight int64) (*ctypes.ResultBlockchainInfo, error) {
-	if minHeight == 0 {
-		minHeight = 1
-	}
-
-	if maxHeight == 0 {
-		maxHeight = blockStore.Height()
-	} else {
-		maxHeight = cmn.MinInt64(blockStore.Height(), maxHeight)
-	}
 
 	// maximum 20 block metas
 	const limit int64 = 20
-	minHeight = cmn.MaxInt64(minHeight, maxHeight-limit)
-
-	logger.Debug("BlockchainInfoHandler", "maxHeight", maxHeight, "minHeight", minHeight)
-
-	if minHeight > maxHeight {
-		return nil, fmt.Errorf("min height %d can't be greater than max height %d", minHeight, maxHeight)
+	var err error
+	minHeight, maxHeight, err = filterMinMax(blockStore.Height(), minHeight, maxHeight, limit)
+	if err != nil {
+		return nil, err
 	}
+	logger.Debug("BlockchainInfoHandler", "maxHeight", maxHeight, "minHeight", minHeight)
 
 	blockMetas := []*types.BlockMeta{}
 	for height := maxHeight; height >= minHeight; height-- {
@@ -93,6 +83,37 @@ func BlockchainInfo(minHeight, maxHeight int64) (*ctypes.ResultBlockchainInfo, e
 	return &ctypes.ResultBlockchainInfo{blockStore.Height(), blockMetas}, nil
 }
 
+// error if either min or max are negative or min < max
+// if 0, use 1 for min, latest block height for max
+// enforce limit.
+// error if min > max
+func filterMinMax(height, min, max, limit int64) (int64, int64, error) {
+	// filter negatives
+	if min < 0 || max < 0 {
+		return min, max, fmt.Errorf("heights must be non-negative")
+	}
+
+	// adjust for default values
+	if min == 0 {
+		min = 1
+	}
+	if max == 0 {
+		max = height
+	}
+
+	// limit max to the height
+	max = cmn.MinInt64(height, max)
+
+	// limit min to within `limit` of max
+	// so the total number of blocks returned will be `limit`
+	min = cmn.MaxInt64(min, max-limit+1)
+
+	if min > max {
+		return min, max, fmt.Errorf("min height %d can't be greater than max height %d", min, max)
+	}
+	return min, max, nil
+}
+
 // Get block at a given height.
 // If no height is provided, it will fetch the latest block.
 //
diff --git a/rpc/core/blocks_test.go b/rpc/core/blocks_test.go
new file mode 100644
index 000000000..da3920c22
--- /dev/null
+++ b/rpc/core/blocks_test.go
@@ -0,0 +1,58 @@
+package core
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestBlockchainInfo(t *testing.T) {
+
+	cases := []struct {
+		min, max     int64
+		height       int64
+		limit        int64
+		resultLength int64
+		wantErr      bool
+	}{
+
+		// min > max
+		{0, 0, 0, 10, 0, true},  // min set to 1
+		{0, 1, 0, 10, 0, true},  // max set to height (0)
+		{0, 0, 1, 10, 1, false}, // max set to height (1)
+		{2, 0, 1, 10, 0, true},  // max set to height (1)
+		{2, 1, 5, 10, 0, true},
+
+		// negative
+		{1, 10, 14, 10, 10, false}, // control
+		{-1, 10, 14, 10, 0, true},
+		{1, -10, 14, 10, 0, true},
+		{-9223372036854775808, -9223372036854775788, 100, 20, 0, true},
+
+		// check limit and height
+		{1, 1, 1, 10, 1, false},
+		{1, 1, 5, 10, 1, false},
+		{2, 2, 5, 10, 1, false},
+		{1, 2, 5, 10, 2, false},
+		{1, 5, 1, 10, 1, false},
+		{1, 5, 10, 10, 5, false},
+		{1, 15, 10, 10, 10, false},
+		{1, 15, 15, 10, 10, false},
+		{1, 15, 15, 20, 15, false},
+		{1, 20, 15, 20, 15, false},
+		{1, 20, 20, 20, 20, false},
+	}
+
+	for i, c := range cases {
+		caseString := fmt.Sprintf("test %d failed", i)
+		min, max, err := filterMinMax(c.height, c.min, c.max, c.limit)
+		if c.wantErr {
+			require.Error(t, err, caseString)
+		} else {
+			require.NoError(t, err, caseString)
+			require.Equal(t, 1+max-min, c.resultLength, caseString)
+		}
+	}
+
+}