Update /<count>/ endpoints to use a '?count=' query parameter instead

In most RESTful APIs, path parameters are used to represent resources, and
query parameters are used to control how these resources are being filtered/sorted/...

The old /<count>/ functionality is kept alive to maintain backwards compatibility,
but new paths with query parameters are introduced and documented as the default
interface so future API methods don't break consistency by using query parameters.

Author: stickies-v

doc/REST-interface.md

@@ -47,18 +47,24 @@ The HTTP request and response are both handled entirely in-memory.
 With the /notxdetails/ option JSON response will only contain the transaction hash instead of the complete transaction details. The option only affects the JSON response.
 
 #### Blockheaders
-`GET /rest/headers/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`
+`GET /rest/headers/<BLOCK-HASH>.<bin|hex|json>?count=<COUNT=5>`
 
 Given a block hash: returns <COUNT> amount of blockheaders in upward direction.
 Returns empty if the block doesn't exist or it isn't in the active chain.
 
+*Deprecated (but not removed) since 24.0:*
+`GET /rest/headers/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`
+
 #### Blockfilter Headers
-`GET /rest/blockfilterheaders/<FILTERTYPE>/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`
+`GET /rest/blockfilterheaders/<FILTERTYPE>/<BLOCK-HASH>.<bin|hex|json>?count=<COUNT=5>`
 
 Given a block hash: returns <COUNT> amount of blockfilter headers in upward
 direction for the filter type <FILTERTYPE>.
 Returns empty if the block doesn't exist or it isn't in the active chain.
 
+*Deprecated (but not removed) since 24.0:*
+`GET /rest/blockfilterheaders/<FILTERTYPE>/<COUNT>/<BLOCK-HASH>.<bin|hex|json>`
+
 #### Blockfilters
 `GET /rest/blockfilter/<FILTERTYPE>/<BLOCK-HASH>.<bin|hex|json>`
 

src/rest.cpp

@@ -194,15 +194,25 @@ static bool rest_headers(const std::any& context,
     std::vector<std::string> path;
     boost::split(path, param, boost::is_any_of("/"));
 
-    if (path.size() != 2)
-        return RESTERR(req, HTTP_BAD_REQUEST, "No header count specified. Use /rest/headers/<count>/<hash>.<ext>.");
-
-    const auto parsed_count{ToIntegral<size_t>(path[0])};
+    std::string raw_count;
+    std::string hashStr;
+    if (path.size() == 2) {
+        // deprecated path: /rest/headers/<count>/<hash>
+        hashStr = path[1];
+        raw_count = path[0];
+    } else if (path.size() == 1) {
+        // new path with query parameter: /rest/headers/<hash>?count=<count>
+        hashStr = path[0];
+        raw_count = req->GetQueryParameter("count").value_or("5");
+    } else {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/headers/<hash>.<ext>?count=<count>");
+    }
+
+    const auto parsed_count{ToIntegral<size_t>(raw_count)};
     if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, path[0]));
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, raw_count));
     }
 
-    std::string hashStr = path[1];
     uint256 hash;
     if (!ParseHashStr(hashStr, hash))
         return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr);
@@ -354,13 +364,28 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
 
     std::vector<std::string> uri_parts;
     boost::split(uri_parts, param, boost::is_any_of("/"));
-    if (uri_parts.size() != 3) {
-        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders/<filtertype>/<count>/<blockhash>");
+    std::string raw_count;
+    std::string raw_blockhash;
+    if (uri_parts.size() == 3) {
+        // deprecated path: /rest/blockfilterheaders/<filtertype>/<count>/<blockhash>
+        raw_blockhash = uri_parts[2];
+        raw_count = uri_parts[1];
+    } else if (uri_parts.size() == 2) {
+        // new path with query parameter: /rest/blockfilterheaders/<filtertype>/<blockhash>?count=<count>
+        raw_blockhash = uri_parts[1];
+        raw_count = req->GetQueryParameter("count").value_or("5");
+    } else {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/blockfilterheaders/<filtertype>/<blockhash>.<ext>?count=<count>");
+    }
+
+    const auto parsed_count{ToIntegral<size_t>(raw_count)};
+    if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
+        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, raw_count));
     }
 
     uint256 block_hash;
-    if (!ParseHashStr(uri_parts[2], block_hash)) {
-        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + uri_parts[2]);
+    if (!ParseHashStr(raw_blockhash, block_hash)) {
+        return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + raw_blockhash);
     }
 
     BlockFilterType filtertype;
@@ -373,11 +398,6 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const
         return RESTERR(req, HTTP_BAD_REQUEST, "Index is not enabled for filtertype " + uri_parts[0]);
     }
 
-    const auto parsed_count{ToIntegral<size_t>(uri_parts[1])};
-    if (!parsed_count.has_value() || *parsed_count < 1 || *parsed_count > MAX_REST_HEADERS_RESULTS) {
-        return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Header count is invalid or out of acceptable range (1-%u): %s", MAX_REST_HEADERS_RESULTS, uri_parts[1]));
-    }
-
     std::vector<const CBlockIndex*> headers;
     headers.reserve(*parsed_count);
     {

test/functional/interface_rest.py

@@ -10,6 +10,7 @@
 from io import BytesIO
 import json
 from struct import pack, unpack
+import typing
 import urllib.parse
 
 
@@ -57,14 +58,21 @@ def set_test_params(self):
             args.append("[email protected]")
         self.supports_cli = False
 
-    def test_rest_request(self, uri, http_method='GET', req_type=ReqType.JSON, body='', status=200, ret_type=RetType.JSON):
+    def test_rest_request(
+            self,
+            uri: str,
+            http_method: str = 'GET',
+            req_type: ReqType = ReqType.JSON,
+            body: str = '',
+            status: int = 200,
+            ret_type: RetType = RetType.JSON,
+            query_params: typing.Dict[str, typing.Any] = None,
+            ) -> typing.Union[http.client.HTTPResponse, bytes, str, None]:
         rest_uri = '/rest' + uri
-        if req_type == ReqType.JSON:
-            rest_uri += '.json'
-        elif req_type == ReqType.BIN:
-            rest_uri += '.bin'
-        elif req_type == ReqType.HEX:
-            rest_uri += '.hex'
+        if req_type in ReqType:
+            rest_uri += f'.{req_type.name.lower()}'
+        if query_params:
+            rest_uri += f'?{urllib.parse.urlencode(query_params)}'
 
         conn = http.client.HTTPConnection(self.url.hostname, self.url.port)
         self.log.debug(f'{http_method} {rest_uri} {body}')
@@ -83,6 +91,8 @@ def test_rest_request(self, uri, http_method='GET', req_type=ReqType.JSON, body=
         elif ret_type == RetType.JSON:
             return json.loads(resp.read().decode('utf-8'), parse_float=Decimal)
 
+        return None
+
     def run_test(self):
         self.url = urllib.parse.urlparse(self.nodes[0].url)
         self.wallet = MiniWallet(self.nodes[0])
@@ -213,12 +223,12 @@ def run_test(self):
         bb_hash = self.nodes[0].getbestblockhash()
 
         # Check result if block does not exists
-        assert_equal(self.test_rest_request(f"/headers/1/{UNKNOWN_PARAM}"), [])
+        assert_equal(self.test_rest_request(f"/headers/{UNKNOWN_PARAM}", query_params={"count": 1}), [])
         self.test_rest_request(f"/block/{UNKNOWN_PARAM}", status=404, ret_type=RetType.OBJ)
 
         # Check result if block is not in the active chain
         self.nodes[0].invalidateblock(bb_hash)
-        assert_equal(self.test_rest_request(f'/headers/1/{bb_hash}'), [])
+        assert_equal(self.test_rest_request(f'/headers/{bb_hash}', query_params={'count': 1}), [])
         self.test_rest_request(f'/block/{bb_hash}')
         self.nodes[0].reconsiderblock(bb_hash)
 
@@ -228,7 +238,7 @@ def run_test(self):
         response_bytes = response.read()
 
         # Compare with block header
-        response_header = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ)
+        response_header = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ, query_params={"count": 1})
         assert_equal(int(response_header.getheader('content-length')), BLOCK_HEADER_SIZE)
         response_header_bytes = response_header.read()
         assert_equal(response_bytes[:BLOCK_HEADER_SIZE], response_header_bytes)
@@ -240,7 +250,7 @@ def run_test(self):
         assert_equal(response_bytes.hex().encode(), response_hex_bytes)
 
         # Compare with hex block header
-        response_header_hex = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ)
+        response_header_hex = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ, query_params={"count": 1})
         assert_greater_than(int(response_header_hex.getheader('content-length')), BLOCK_HEADER_SIZE*2)
         response_header_hex_bytes = response_header_hex.read(BLOCK_HEADER_SIZE*2)
         assert_equal(response_bytes[:BLOCK_HEADER_SIZE].hex().encode(), response_header_hex_bytes)
@@ -267,7 +277,7 @@ def run_test(self):
         self.test_rest_request("/blockhashbyheight/", ret_type=RetType.OBJ, status=400)
 
         # Compare with json block header
-        json_obj = self.test_rest_request(f"/headers/1/{bb_hash}")
+        json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1})
         assert_equal(len(json_obj), 1)  # ensure that there is one header in the json response
         assert_equal(json_obj[0]['hash'], bb_hash)  # request/response hash should be the same
 
@@ -278,9 +288,9 @@ def run_test(self):
 
         # See if we can get 5 headers in one response
         self.generate(self.nodes[1], 5)
-        json_obj = self.test_rest_request(f"/headers/5/{bb_hash}")
+        json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 5})
         assert_equal(len(json_obj), 5)  # now we should have 5 header objects
-        json_obj = self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}")
+        json_obj = self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 5})
         first_filter_header = json_obj[0]
         assert_equal(len(json_obj), 5)  # now we should have 5 filter header objects
         json_obj = self.test_rest_request(f"/blockfilter/basic/{bb_hash}")
@@ -294,7 +304,7 @@ def run_test(self):
         for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']:
             assert_equal(
                 bytes(f'Header count is invalid or out of acceptable range (1-2000): {num}\r\n', 'ascii'),
-                self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400),
+                self.test_rest_request(f"/headers/{bb_hash}", ret_type=RetType.BYTES, status=400, query_params={"count": num}),
             )
 
         self.log.info("Test tx inclusion in the /mempool and /block URIs")
@@ -351,6 +361,11 @@ def run_test(self):
         json_obj = self.test_rest_request("/chaininfo")
         assert_equal(json_obj['bestblockhash'], bb_hash)
 
+        # Test compatibility of deprecated and newer endpoints
+        self.log.info("Test compatibility of deprecated and newer endpoints")
+        assert_equal(self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/headers/1/{bb_hash}"))
+        assert_equal(self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}"))
+
 
 if __name__ == '__main__':
     RESTTest().main()