From fff771ee864975cee8c831651239bac95503c37a Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Tue, 18 Jan 2022 16:37:54 +0000
Subject: Handle query string when parsing data format

URLs may contain a query string (prefixed with '?') and this should be ignored when parsing
the data format.

To facilitate testing this functionality, ParseDataFormat has been made non-static.
---
 src/Makefile.test.include |  1 +
 src/rest.cpp              | 23 +++++++++++++----------
 src/rest.h                |  9 +++++++++
 src/test/rest_tests.cpp   | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 10 deletions(-)
 create mode 100644 src/test/rest_tests.cpp

(limited to 'src')

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 801745d0c6..dccf20749b 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -116,6 +116,7 @@ BITCOIN_TESTS =\
   test/prevector_tests.cpp \
   test/raii_event_tests.cpp \
   test/random_tests.cpp \
+  test/rest_tests.cpp \
   test/reverselock_tests.cpp \
   test/rpc_tests.cpp \
   test/sanity_tests.cpp \
diff --git a/src/rest.cpp b/src/rest.cpp
index 4daf995b01..ec9295b283 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -135,23 +135,26 @@ static ChainstateManager* GetChainman(const std::any& context, HTTPRequest* req)
 
 RESTResponseFormat ParseDataFormat(std::string& param, const std::string& strReq)
 {
-    const std::string::size_type pos = strReq.rfind('.');
-    if (pos == std::string::npos)
-    {
-        param = strReq;
+    // Remove query string (if any, separated with '?') as it should not interfere with
+    // parsing param and data format
+    param = strReq.substr(0, strReq.rfind('?'));
+    const std::string::size_type pos_format{param.rfind('.')};
+
+    // No format string is found
+    if (pos_format == std::string::npos) {
         return rf_names[0].rf;
     }
 
-    param = strReq.substr(0, pos);
-    const std::string suff(strReq, pos + 1);
-
+    // Match format string to available formats
+    const std::string suffix(param, pos_format + 1);
     for (const auto& rf_name : rf_names) {
-        if (suff == rf_name.name)
+        if (suffix == rf_name.name) {
+            param.erase(pos_format);
             return rf_name.rf;
+        }
     }
 
-    /* If no suffix is found, return original string.  */
-    param = strReq;
+    // If no suffix is found, return RESTResponseFormat::UNDEF and original string without query string
     return rf_names[0].rf;
 }
 
diff --git a/src/rest.h b/src/rest.h
index e09541c47f..49b1c333d0 100644
--- a/src/rest.h
+++ b/src/rest.h
@@ -14,6 +14,15 @@ enum class RESTResponseFormat {
     JSON,
 };
 
+/**
+ * Parse a URI to get the data format and URI without data format
+ * and query string.
+ *
+ * @param[out]  param   The strReq without the data format string and
+ *                      without the query string (if any).
+ * @param[in]   strReq  The URI to be parsed.
+ * @return      RESTResponseFormat that was parsed from the URI.
+ */
 RESTResponseFormat ParseDataFormat(std::string& param, const std::string& strReq);
 
 #endif // BITCOIN_REST_H
diff --git a/src/test/rest_tests.cpp b/src/test/rest_tests.cpp
new file mode 100644
index 0000000000..20dfe4b41a
--- /dev/null
+++ b/src/test/rest_tests.cpp
@@ -0,0 +1,48 @@
+// Copyright (c) 2012-2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <rest.h>
+#include <test/util/setup_common.h>
+
+#include <boost/test/unit_test.hpp>
+
+#include <string>
+
+BOOST_FIXTURE_TEST_SUITE(rest_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(test_query_string)
+{
+    std::string param;
+    RESTResponseFormat rf;
+    // No query string
+    rf = ParseDataFormat(param, "/rest/endpoint/someresource.json");
+    BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource");
+    BOOST_CHECK_EQUAL(rf, RESTResponseFormat::JSON);
+
+    // Query string with single parameter
+    rf = ParseDataFormat(param, "/rest/endpoint/someresource.bin?p1=v1");
+    BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource");
+    BOOST_CHECK_EQUAL(rf, RESTResponseFormat::BINARY);
+
+    // Query string with multiple parameters
+    rf = ParseDataFormat(param, "/rest/endpoint/someresource.hex?p1=v1&p2=v2");
+    BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource");
+    BOOST_CHECK_EQUAL(rf, RESTResponseFormat::HEX);
+
+    // Incorrectly formed query string will not be handled
+    rf = ParseDataFormat(param, "/rest/endpoint/someresource.json&p1=v1");
+    BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource.json&p1=v1");
+    BOOST_CHECK_EQUAL(rf, RESTResponseFormat::UNDEF);
+
+    // Omitted data format with query string should return UNDEF and hide query string
+    rf = ParseDataFormat(param, "/rest/endpoint/someresource?p1=v1");
+    BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource");
+    BOOST_CHECK_EQUAL(rf, RESTResponseFormat::UNDEF);
+
+    // Data format specified after query string
+    rf = ParseDataFormat(param, "/rest/endpoint/someresource?p1=v1.json");
+    BOOST_CHECK_EQUAL(param, "/rest/endpoint/someresource");
+    BOOST_CHECK_EQUAL(rf, RESTResponseFormat::UNDEF);
+}
+BOOST_AUTO_TEST_SUITE_END()
-- 
cgit v1.2.3