[#2260] Addressed third item

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
index f43a1d70d1d67abd26b7d756ea913a1583e3ad0f..f4552ebcb9d30afba2c72c76146571620efe9473 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
@@ -33,6 +33,7 @@
 
 #include <boost/scoped_ptr.hpp>
 #include <boost/algorithm/string.hpp>
+#include <limits>
 #include <string>
 #include <sstream>
 
@@ -752,7 +753,12 @@ LeaseCmdsImpl::getParameters(bool v6, const ConstElementPtr& params) {
     if (tmp->getType() != Element::integer) {
         isc_throw(BadValue, "'subnet-id' parameter is not integer.");
     }
-    x.subnet_id = tmp->intValue();
+    int64_t tmp64 = tmp->intValue();
+    if ((tmp64 < numeric_limits<uint32_t>::min()) ||
+        (tmp64 > numeric_limits<uint32_t>::max())) {
+        isc_throw(BadValue, "'subnet-id' parameter is not a 32 bit unsigned integer.");
+    }
+    x.subnet_id = static_cast<SubnetID>(tmp64);
 
     if (params->contains("iaid")) {
         x.iaid = params->get("iaid")->intValue();
@@ -923,17 +929,23 @@ LeaseCmdsImpl::leaseGetAllHandler(CalloutHandle& handle) {
                 if (subnet_id->getType() != Element::integer) {
                     isc_throw(BadValue, "listed subnet identifiers must be numbers");
                 }
+                auto subnet_id_ = subnet_id->intValue();
+                if ((subnet_id_ < numeric_limits<uint32_t>::min()) ||
+                    (subnet_id_ > numeric_limits<uint32_t>::max())) {
+                    isc_throw(BadValue, "out of range subnet identifier "
+                              << subnet_id_);
+                }
 
                 if (v4) {
                     Lease4Collection leases =
-                        LeaseMgrFactory::instance().getLeases4(subnet_id->intValue());
+                        LeaseMgrFactory::instance().getLeases4(subnet_id_);
                     for (auto const& lease : leases) {
                         ElementPtr lease_json = lease->toElement();
                         leases_json->add(lease_json);
                     }
                 } else {
                     Lease6Collection leases =
-                        LeaseMgrFactory::instance().getLeases6(subnet_id->intValue());
+                        LeaseMgrFactory::instance().getLeases6(subnet_id_);
                     for (auto const& lease : leases) {
                         ElementPtr lease_json = lease->toElement();
                         leases_json->add(lease_json);
@@ -1316,7 +1328,12 @@ LeaseCmdsImpl::leaseGetByStateHandler(CalloutHandle& handle) {
             if (subnet->getType() != Element::integer) {
                 isc_throw(BadValue, "'subnet-id' parameter must be a number");
             }
-            subnet_id_ = subnet->intValue();
+            auto id64 = subnet->intValue();
+            if ((id64 < numeric_limits<uint32_t>::min()) ||
+                (id64 > numeric_limits<uint32_t>::max())) {
+                isc_throw(BadValue, "'subnet-id' parameter must be a 32 bit unsigned integer");
+            }
+            subnet_id_ = static_cast<SubnetID>(id64);
         }
 
         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 680b69726552843aaecef6df57c38bd9dc0bfa34..40c12c0fe1a975957c26daccb27b1d563413ffb2 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
+++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds4_unittest.cc
@@ -313,6 +313,10 @@ public:
     /// combinations).
     void testLease4GetByStateFindN();
 
+    /// @brief Check that lease4-get-by-state raises an error when
+    /// the argument is bad.
+    void testLease4GetByStateBadArgs();
+
     /// @brief Check that lease4-get-by-hostname can handle a situation when
     /// the query is broken (required parameter is missing).
     void testLease4GetByHostnameParams();
@@ -1199,6 +1203,39 @@ void Lease4CmdsTest::testLease4GetMissingParams() {
     string exp_rsp = "Mandatory 'subnet-id' parameter missing.";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 
+    // Reject not numeric subnet id.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get\",\n"
+        "    \"arguments\": {"
+        "        \"subnet-id\": \"foo\""
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter is not integer.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Reject negative subnet id.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get\",\n"
+        "    \"arguments\": {"
+        "        \"subnet-id\": -1"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Reject too large subnet id.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get\",\n"
+        "    \"arguments\": {"
+        "        \"subnet-id\": 4294967297"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
     // Just the subnet-id won't cut it, either.
     cmd =
         "{\n"
@@ -1616,6 +1653,28 @@ void Lease4CmdsTest::testLease4GetBySubnetIdInvalidArguments() {
         "}";
     exp_rsp = "listed subnet identifiers must be numbers";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnets list must contain positive numbers..
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get-all\",\n"
+        "    \"arguments\": {"
+        "        \"subnets\": [ 1, -1 ]\n"
+        "    }\n"
+        "}";
+    exp_rsp = "out of range subnet identifier -1";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnets list must contain 32 bit unsigned integers..
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get-all\",\n"
+        "    \"arguments\": {"
+        "        \"subnets\": [ 1, 4294967297 ]\n"
+        "    }\n"
+        "}";
+    exp_rsp = "out of range subnet identifier 4294967297";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 }
 
 void Lease4CmdsTest::testLease4GetPaged() {
@@ -2180,6 +2239,44 @@ void Lease4CmdsTest::testLease4GetByStateFindN() {
     }
 }
 
+void Lease4CmdsTest::testLease4GetByStateBadArgs() {
+    // Subnet id must be a number.
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease4-get-by-state\",\n"
+        "    \"arguments\": {"
+        "        \"state\": 3,\n"
+        "        \"subnet-id\": \"foo\"\n"
+        "    }\n"
+        "}";
+    string exp_rsp = "'subnet-id' parameter must be a number";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnet id must be positive.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get-by-state\",\n"
+        "    \"arguments\": {"
+        "        \"state\": 3,\n"
+        "        \"subnet-id\": -1\n"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnet id must be fir into 32 bits.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease4-get-by-state\",\n"
+        "    \"arguments\": {"
+        "        \"state\": 3,\n"
+        "        \"subnet-id\": 4294967297\n"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+}
+
 void Lease4CmdsTest::testLease4GetByHostnameParams() {
     // No parameters whatsoever.
     string cmd =
@@ -4348,6 +4445,15 @@ TEST_F(Lease4CmdsTest, lease4GetByStateFindNMultiThreading) {
     testLease4GetByStateFindN();
 }
 
+TEST_F(Lease4CmdsTest, lease4GetByStateBadArgs) {
+    testLease4GetByStateBadArgs();
+}
+
+TEST_F(Lease4CmdsTest, lease4GetByStateBadArgsMultiThreading) {
+    MultiThreadingTest mt(true);
+    testLease4GetByStateBadArgs();
+}
+
 TEST_F(Lease4CmdsTest, lease4GetByHostnameParams) {
     testLease4GetByHostnameParams();
 }

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 fd4ade3ac53d00499538cc42dfa6ab72c5a51912..9fbe4a7bbbbd82f8a845c11e6f66e3d5d9759257 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc
+++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc
@@ -341,6 +341,10 @@ public:
     /// combinations).
     void testLease6GetByStateFindN();
 
+    /// @brief Check that lease6-get-by-state raises an error when
+    /// the argument is bad.
+    void testLease6GetByStateBadArgs();
+
     /// @brief Check that lease6-get-by-hostname can handle a situation when
     /// the query is broken (required parameter is missing).
     void testLease6GetByHostnameParams();
@@ -1452,6 +1456,39 @@ void Lease6CmdsTest::testLease6GetMissingParams() {
     string exp_rsp = "Mandatory 'subnet-id' parameter missing.";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 
+    // Reject not numeric subnet id.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get\",\n"
+        "    \"arguments\": {"
+        "        \"subnet-id\": \"foo\""
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter is not integer.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Reject negative subnet id.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get\",\n"
+        "    \"arguments\": {"
+        "        \"subnet-id\": -1"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Reject too large subnet id.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get\",\n"
+        "    \"arguments\": {"
+        "        \"subnet-id\": 4294967297"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter is not a 32 bit unsigned integer.";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
     // Just the subnet-id won't cut it, either.
     cmd =
         "{\n"
@@ -1880,6 +1917,28 @@ void Lease6CmdsTest::testLease6GetBySubnetIdInvalidArguments() {
         "}";
     exp_rsp = "listed subnet identifiers must be numbers";
     testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnets list must contain positive numbers..
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get-all\",\n"
+        "    \"arguments\": {"
+        "        \"subnets\": [ 1, -1 ]\n"
+        "    }\n"
+        "}";
+    exp_rsp = "out of range subnet identifier -1";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnets list must contain 32 bit unsigned integers..
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get-all\",\n"
+        "    \"arguments\": {"
+        "        \"subnets\": [ 1, 4294967297 ]\n"
+        "    }\n"
+        "}";
+    exp_rsp = "out of range subnet identifier 4294967297";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
 }
 
 void Lease6CmdsTest::testLease6GetPaged() {
@@ -2450,6 +2509,44 @@ void Lease6CmdsTest::testLease6GetByStateFindN() {
     }
 }
 
+void Lease6CmdsTest::testLease6GetByStateBadArgs() {
+    // Subnet id must be a number.
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease6-get-by-state\",\n"
+        "    \"arguments\": {"
+        "        \"state\": 3,\n"
+        "        \"subnet-id\": \"foo\"\n"
+        "    }\n"
+        "}";
+    string exp_rsp = "'subnet-id' parameter must be a number";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnet id must be positive.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get-by-state\",\n"
+        "    \"arguments\": {"
+        "        \"state\": 3,\n"
+        "        \"subnet-id\": -1\n"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    // Subnet id must be fir into 32 bits.
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-get-by-state\",\n"
+        "    \"arguments\": {"
+        "        \"state\": 3,\n"
+        "        \"subnet-id\": 4294967297\n"
+        "    }\n"
+        "}";
+    exp_rsp = "'subnet-id' parameter must be a 32 bit unsigned integer";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+}
+
 void Lease6CmdsTest::testLease6GetByHostnameParams() {
     // No parameters whatsoever.
     string cmd =
@@ -5411,6 +5508,15 @@ TEST_F(Lease6CmdsTest, lease6GetByStateFindNMultiThreading) {
     testLease6GetByStateFindN();
 }
 
+TEST_F(Lease6CmdsTest, lease6GetByStateBadArgs) {
+    testLease6GetByStateBadArgs();
+}
+
+TEST_F(Lease6CmdsTest, lease6GetByStateBadArgsMultiThreading) {
+    MultiThreadingTest mt(true);
+    testLease6GetByStateBadArgs();
+}
+
 TEST_F(Lease6CmdsTest, lease6GetByHostnameParams) {
     testLease6GetByHostnameParams();
 }