From 685d43b2b3ac7a11300e668bfeae5e5e5c05ee7b Mon Sep 17 00:00:00 2001
From: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>
Date: Fri, 7 Jan 2022 15:01:23 +0100
Subject: [PATCH] Ensure store response never exceeds `MaxPageSize` (#811)

* Never exceed maxPageSize, even with no specified pagination

* fix test

* Changelog

* Forgot to stage everything...

* Account for timing difference in Windows
---
 CHANGELOG.md                               |  1 +
 tests/v2/test_jsonrpc_waku.nim             |  2 +-
 tests/v2/test_waku_store.nim               | 12 ++++++++----
 waku/v2/protocol/waku_store/waku_store.nim | 13 ++++++-------
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8bbabd01e..13a44e72a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ which is a sequence of string.
 - Metrics: added counters for protocol messages
 
 ### Fixes
+- All `HistoryResponse` messages are now auto-paginated to a maximum of 100 messages per response
 - Increased maximum length for reading from a libp2p input stream to allow largest possible protocol messages, including `HistoryResponse` messages at max size.
 
 ## 2021-11-05 v0.6
diff --git a/tests/v2/test_jsonrpc_waku.nim b/tests/v2/test_jsonrpc_waku.nim
index e1e0f0f94..c74a466f5 100644
--- a/tests/v2/test_jsonrpc_waku.nim
+++ b/tests/v2/test_jsonrpc_waku.nim
@@ -257,7 +257,7 @@ procSuite "Waku v2 JSON-RPC API":
     let response = await client.get_waku_v2_store_v1_messages(some(defaultTopic), some(@[HistoryContentFilter(contentTopic: defaultContentTopic)]), some(0.float64), some(9.float64), some(StorePagingOptions()))
     check:
       response.messages.len() == 8
-      response.pagingOptions.isNone
+      response.pagingOptions.isSome()
       
     server.stop()
     server.close()
diff --git a/tests/v2/test_waku_store.nim b/tests/v2/test_waku_store.nim
index d5c56aeff..642a4ae11 100644
--- a/tests/v2/test_waku_store.nim
+++ b/tests/v2/test_waku_store.nim
@@ -190,8 +190,8 @@ procSuite "Waku Store":
       key = PrivateKey.random(ECDSA, rng[]).get()
       peer = PeerInfo.new(key)
       msg1 = WakuMessage(payload: @[byte 1, 2, 3], contentTopic: defaultContentTopic)
-      msg2 = WakuMessage(payload: @[byte 1, 2, 3], contentTopic: defaultContentTopic)
-      msg3 = WakuMessage(payload: @[byte 1, 2, 3], contentTopic: defaultContentTopic)
+      msg2 = WakuMessage(payload: @[byte 4, 5, 6], contentTopic: defaultContentTopic)
+      msg3 = WakuMessage(payload: @[byte 7, 8, 9,], contentTopic: defaultContentTopic)
 
     var dialSwitch = newStandardSwitch()
     discard await dialSwitch.start()
@@ -389,7 +389,7 @@ procSuite "Waku Store":
     check:
       (await completionFut.withTimeout(5.seconds)) == true
 
-  asyncTest "handle queries with no pagination":
+  asyncTest "handle queries with no paging info (auto-paginate)":
     let
       key = PrivateKey.random(ECDSA, rng[]).get()
       peer = PeerInfo.new(key)
@@ -424,8 +424,12 @@ procSuite "Waku Store":
 
     proc handler(response: HistoryResponse) {.gcsafe, closure.} =
       check:
+        ## No pagination specified. Response will be auto-paginated with
+        ## up to MaxPageSize messages per page.
         response.messages.len() == 8
-        response.pagingInfo == PagingInfo()
+        response.pagingInfo.pageSize == 8
+        response.pagingInfo.direction == PagingDirection.BACKWARD
+        response.pagingInfo.cursor != Index()
       completionFut.complete(true)
 
     let rpc = HistoryQuery(contentFilters: @[HistoryContentFilter(contentTopic: defaultContentTopic)] )
diff --git a/waku/v2/protocol/waku_store/waku_store.nim b/waku/v2/protocol/waku_store/waku_store.nim
index 4c65e1bdc..ed607f67f 100644
--- a/waku/v2/protocol/waku_store/waku_store.nim
+++ b/waku/v2/protocol/waku_store/waku_store.nim
@@ -287,16 +287,15 @@ proc paginate*(list: seq[IndexedWakuMessage], pinfo: PagingInfo): (seq[IndexedWa
     dir = pinfo.direction
     output: (seq[IndexedWakuMessage], PagingInfo, HistoryResponseError) 
 
-  if pageSize == uint64(0): # pageSize being zero indicates that no pagination is required
-    let output = (list, pinfo, HistoryResponseError.NONE)
-    return output
-
   if list.len == 0: # no pagination is needed for an empty list
     output = (list, PagingInfo(pageSize: 0, cursor:pinfo.cursor, direction: pinfo.direction), HistoryResponseError.NONE)
     return output
   
-  # adjust pageSize
-  if pageSize > MaxPageSize:
+  ## Adjust pageSize:
+  ## - pageSize should not exceed maximum
+  ## - pageSize being zero indicates "no pagination", but we still limit
+  ##   responses to no more than a page of MaxPageSize messages
+  if (pageSize == uint64(0)) or (pageSize > MaxPageSize):
     pageSize = MaxPageSize
 
   # sort the existing messages
@@ -304,7 +303,7 @@ proc paginate*(list: seq[IndexedWakuMessage], pinfo: PagingInfo): (seq[IndexedWa
     msgList = list # makes a copy of the list
     total = uint64(msgList.len)
   # sorts msgList based on the custom comparison proc indexedWakuMessageComparison
-  msgList.sort(indexedWakuMessageComparison)  
+  msgList.sort(indexedWakuMessageComparison)
   
   # set the cursor of the initial paging request
   var isInitialQuery = false