[#2260] check page size limits

diff --git a/src/bin/keactrl/tests/keactrl_tests.sh.in b/src/bin/keactrl/tests/keactrl_tests.sh.in
index dac42232e6ddbd2f4d83cc9c84c93dab744783cb..83315ef3290e0383f24c435c4f93310517a7ba00 100755 (executable)
--- a/src/bin/keactrl/tests/keactrl_tests.sh.in
+++ b/src/bin/keactrl/tests/keactrl_tests.sh.in
@@ -645,7 +645,7 @@ Expected wait_for_message return %d, returned %d."
 # servers are not started.
 start_v6_server_test() {
     # Create configuration file for keactrl. This configuration enables
-    # DHCPv6 server but disables other servers..
+    # DHCPv6 server but disables other servers.
     keactrl_config="${CFG_FILES}\ndhcp4=no\ndhcp6=yes\n\
 dhcp_ddns=no\nnetconf=no\n\
 kea_verbose=no\n${keactrl_fixed_config}"

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
index 150edd12c90795e2567787d7358b7456d39d37e6..57cb211c7e98706cdde8fbd51e78576c177f4f70 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
@@ -1071,8 +1071,17 @@ LeaseCmdsImpl::leaseGetPageHandler(CalloutHandle& handle) {
             isc_throw(BadValue, "'limit' parameter must be a number");
         }
 
+        auto tmp128 = page_limit->intValue();
+        if ((tmp128 < uint32_min) || (tmp128 > uint32_max)) {
+            isc_throw(BadValue, "'limit' parameter is not a 32 bit unsigned integer.");
+        }
+
+        if (tmp128 == 0) {
+            isc_throw(BadValue, "'limit' parameter must not be 0.");
+        }
+
         // Retrieve the desired page size.
-        size_t page_limit_value = static_cast<size_t>(page_limit->intValue());
+        size_t page_limit_value = static_cast<size_t>(tmp128);
 
         ElementPtr leases_json = Element::createList();
 

diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
index 14573233e171595a4dccd0625af11b63a7d9e4db..4674b9064302fd7e5a576cc818b85cab00a2dd84 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
+++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
@@ -273,6 +273,9 @@ public:
     /// @brief Verifies that the limit of 0 is rejected.
     void testLease4GetPagedLimitIsZero();
 
+    /// @brief Verifies that the limit of negative value is rejected.
+    void testLease4GetPagedLimitIsNegative();
+
     /// @brief Check that lease4-get-by-hw-address can handle a situation when
     /// the query is broken (required parameter is missing).
     void testLease4GetByHwAddressParams();
@@ -1699,7 +1702,7 @@ void Lease4CmdsTest::testLease4GetBySubnetIdInvalidArguments() {
     exp_rsp = "listed subnet identifiers must be numbers";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 
-    // Subnets list must contain positive numbers..
+    // Subnets list must contain positive numbers.
     cmd =
         "{\n"
         "    \"command\": \"lease4-get-all\",\n"
@@ -1710,7 +1713,7 @@ void Lease4CmdsTest::testLease4GetBySubnetIdInvalidArguments() {
     exp_rsp = "out of range subnet identifier -1";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 
-    // Subnets list must contain 32 bit unsigned integers..
+    // Subnets list must contain 32 bit unsigned integers.
     cmd =
         "{\n"
         "    \"command\": \"lease4-get-all\",\n"
@@ -1920,7 +1923,25 @@ void Lease4CmdsTest::testLease4GetPagedLimitIsZero() {
         "    }"
         "}";
 
-    string exp_rsp = "page size of retrieved leases must not be 0";
+    string exp_rsp = "'limit' parameter must not be 0.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+}
+
+void Lease4CmdsTest::testLease4GetPagedLimitIsNegative() {
+    // Initialize lease manager (false = v6, true = add leases)
+    initLeaseMgr(false, true);
+
+    // Query for a page of leases.
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease4-get-page\",\n"
+        "    \"arguments\": {"
+        "        \"from\": \"start\","
+        "        \"limit\": -1"
+        "    }"
+        "}";
+
+    string exp_rsp = "'limit' parameter is not a 32 bit unsigned integer.";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 }
 
@@ -2295,7 +2316,7 @@ void Lease4CmdsTest::testLease4GetByStateBadArgs() {
             "'subnet-id' parameter must be a 32 bit unsigned integer"
         },
         {
-            "Subnet id must be fit into 32 bits",
+            "Subnet id must fit into 32 bits",
             "\"subnet-id\": 4294967297",
             "'subnet-id' parameter must be a 32 bit unsigned integer"
         }
@@ -4396,6 +4417,15 @@ TEST_F(Lease4CmdsTest, lease4GetPagedLimitIsZeroMultiThreading) {
     testLease4GetPagedLimitIsZero();
 }
 
+TEST_F(Lease4CmdsTest, lease4GetPagedLimitIsNegative) {
+    testLease4GetPagedLimitIsNegative();
+}
+
+TEST_F(Lease4CmdsTest, lease4GetPagedLimitIsNegativeMultiThreading) {
+    MultiThreadingTest mt(true);
+    testLease4GetPagedLimitIsNegative();
+}
+
 TEST_F(Lease4CmdsTest, lease4GetByHwAddressParams) {
     testLease4GetByHwAddressParams();
 }

diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc
index f7f57c6f182a75bc62d60f3ce4a95ddf05e4bea2..f3091d928f1e4c6fbc1ebfa80f36b33b86ec5144 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc
+++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc
@@ -302,6 +302,9 @@ public:
     /// @brief Verifies that the limit of 0 is rejected.
     void testLease6GetPagedLimitIsZero();
 
+    /// @brief Verifies that the limit of negative value is rejected.
+    void testLease6GetPagedLimitIsNegative();
+
     /// @brief Check that lease6-get-by-hw-address can handle a situation when
     /// the query is broken (required parameter is missing).
     void testLease6GetByHwAddressParams();
@@ -1975,7 +1978,7 @@ void Lease6CmdsTest::testLease6GetBySubnetIdInvalidArguments() {
     exp_rsp = "listed subnet identifiers must be numbers";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 
-    // Subnets list must contain positive numbers..
+    // Subnets list must contain positive numbers.
     cmd =
         "{\n"
         "    \"command\": \"lease6-get-all\",\n"
@@ -1986,7 +1989,7 @@ void Lease6CmdsTest::testLease6GetBySubnetIdInvalidArguments() {
     exp_rsp = "out of range subnet identifier -1";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 
-    // Subnets list must contain 32 bit unsigned integers..
+    // Subnets list must contain 32 bit unsigned integers.
     cmd =
         "{\n"
         "    \"command\": \"lease6-get-all\",\n"
@@ -2197,7 +2200,25 @@ void Lease6CmdsTest::testLease6GetPagedLimitIsZero() {
         "    }"
         "}";
 
-    string exp_rsp = "page size of retrieved leases must not be 0";
+    string exp_rsp = "'limit' parameter must not be 0.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+}
+
+void Lease6CmdsTest::testLease6GetPagedLimitIsNegative() {
+    // Initialize lease manager (true = v6, true = add leases)
+    initLeaseMgr(true, true);
+
+    // Query for a page of leases.
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease6-get-page\",\n"
+        "    \"arguments\": {"
+        "        \"from\": \"start\","
+        "        \"limit\": -1"
+        "    }"
+        "}";
+
+    string exp_rsp = "'limit' parameter is not a 32 bit unsigned integer.";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 }
 
@@ -2577,7 +2598,7 @@ void Lease6CmdsTest::testLease6GetByStateBadArgs() {
             "'subnet-id' parameter must be a 32 bit unsigned integer"
         },
         {
-            "Subnet id must be fit into 32 bits",
+            "Subnet id must fit into 32 bits",
             "\"subnet-id\": 4294967297",
             "'subnet-id' parameter must be a 32 bit unsigned integer"
         }
@@ -5470,6 +5491,15 @@ TEST_F(Lease6CmdsTest, lease6GetPagedLimitIsZeroMultiThreading) {
     testLease6GetPagedLimitIsZero();
 }
 
+TEST_F(Lease6CmdsTest, lease6GetPagedLimitIsNegative) {
+    testLease6GetPagedLimitIsNegative();
+}
+
+TEST_F(Lease6CmdsTest, lease6GetPagedLimitIsNegativeMultiThreading) {
+    MultiThreadingTest mt(true);
+    testLease6GetPagedLimitIsNegative();
+}
+
 TEST_F(Lease6CmdsTest, lease6GetByHwAddressParams) {
     testLease6GetByHwAddressParams();
 }

diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.cc b/src/lib/dhcpsrv/dhcpsrv_messages.cc
index 8b04b3bbf0ca8f9b52d0568456072230cd079dd3..5f15a05fe9943cbd9ffa194b471af564ea1545ad 100644 (file)
--- a/src/lib/dhcpsrv/dhcpsrv_messages.cc
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.cc
@@ -231,7 +231,7 @@ const char* values[] = {
     "DHCPSRV_CFGMGR_USE_ADDRESS", "listening on address %1, on interface %2",
     "DHCPSRV_CFGMGR_USE_ALLOCATOR", "using the %1 allocator for %2 leases in subnet %3",
     "DHCPSRV_CFGMGR_USE_UNICAST", "listening on unicast address %1, on interface %2",
-    "DHCPSRV_CLASS_BAD_NAME", "class name '%1' includes problematic characters: its escaped form is '%2'",
+    "DHCPSRV_CLASS_BAD_NAME", "class name '%1' includes problematic characters: suggest to use '%2' instead",
     "DHCPSRV_CLASS_WITH_ADDITIONAL_AND_LIFETIMES", "class: %1 has 'only-in-additional-list' true while specifying one or more lease life time values. Life time values will be ignored.",
     "DHCPSRV_CLIENT_CLASS_DEPRECATED", "The parameter 'client-class' is deprecated. Use 'client-classes' list parameter instead",
     "DHCPSRV_CLOSE_DB", "closing currently open %1 database",

diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
index be29d5a04816ea62fc7eb245e1851f1d4bc9908b..ca285ec2b77a128d8ee94df412da2a00eb7783c7 100644 (file)
--- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
@@ -604,7 +604,7 @@ TEST_F(ClientClassDefParserTest, escapedName) {
     // A warning should be in logs.
     std::string warn = "DHCPSRV_CLASS_BAD_NAME class name 'foo bar' ";
     warn += "includes problematic characters: ";
-    warn += "its escaped form is 'foo%20bar'";
+    warn += "suggest to use 'foo%20bar' instead";
     EXPECT_EQ(1U, countFile(warn));
 }
 
@@ -626,7 +626,7 @@ TEST_F(ClientClassDefParserTest, escapedName2) {
     // A warning should not be in logs.
     std::string warn = "DHCPSRV_CLASS_BAD_NAME class name 'foo%bar' ";
     warn += "includes problematic characters: ";
-    warn += "its escaped form is 'foo%%bar'";
+    warn += "suggest to use 'foo%%bar' instead";
     EXPECT_EQ(0U, countFile(warn));
 }
 

diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
index ac7f49d9d4f46e00771f4805eb11c4bb1d2f70d0..2fc7450b79b31e40b5c078c19fd62cc695506854 100644 (file)
--- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
@@ -1332,7 +1332,7 @@ GenericHostDataSourceTest::testGet4ByIdentifier(
     HostPtr host1 = HostDataSourceUtils::initializeHost4("192.0.2.1", identifier_type);
     HostPtr host2 = HostDataSourceUtils::initializeHost4("192.0.2.2", identifier_type);
 
-    // Sanity check: make sure the hosts have different identifiers..
+    // Sanity check: make sure the hosts have different identifiers.
     ASSERT_FALSE(host1->getIdentifier() == host2->getIdentifier());
 
     // Try to add both of them to the host data source.