[#3463] Addressed review comments

/src/hooks/dhcp/lease_cmds/lease_cmds.cc
    Add null lease checks

/src/lib/dhcpsrv/lease.cc
    Lease4::decline() - wipe user context

/src/lib/dhcpsrv/tests/lease_unittest.cc
    TEST_F(Lease4Test, decline) - verify user context
    is wiped

diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
index 0a19bea1b67ca75364c3de8d634b32a3c5504204..72f93761a8e25aafdcd0bba8bf6d8dacd5a0e374 100644 (file)
--- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc
+++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc
@@ -2794,6 +2794,10 @@ LeaseCmdsImpl::leases4Committed(CalloutHandle& callout_handle,
     callout_handle.getArgument("response4", response);
     callout_handle.getArgument("leases4", leases);
 
+    if (!leases) {
+        isc_throw(Unexpected, "leases4Committed - leases is null");
+    }
+
     // In some cases we may have no lease, e.g. DHCPNAK,
     // or no response e.g. DHCPRELEASE.
     if (leases->empty() || !response || (response->getType() != DHCPACK)) {
@@ -2802,7 +2806,7 @@ LeaseCmdsImpl::leases4Committed(CalloutHandle& callout_handle,
 
     Lease4Ptr lease = (*leases)[0];
     if (!lease) {
-        isc_throw(Unexpected, "leases4Committed - no lease!");
+        isc_throw(Unexpected, "leases4Committed - lease is null");
     }
 
     try {
@@ -2832,6 +2836,10 @@ LeaseCmdsImpl::leases6Committed(CalloutHandle& callout_handle,
     callout_handle.getArgument("response6", response);
     callout_handle.getArgument("leases6", leases);
 
+    if (!leases) {
+        isc_throw(Unexpected, "leases6Committed - leases is null");
+    }
+
     // In some cases we may have no active leases or no response.
     if (leases->empty() || !response) {
         return;
@@ -2841,6 +2849,10 @@ LeaseCmdsImpl::leases6Committed(CalloutHandle& callout_handle,
     int failed = 0;
     for (auto lease : *leases) {
         try {
+            if (!lease) {
+                isc_throw(Unexpected, "leases6Committed - lease is null");
+            }
+
             /// @todo - Users might want to only update NA or PD leases.
             /// This could be done via adding a lease type to the variable.
             /// V4 would not use it, for V6 it would restrict a variable

diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc
index a3a7568dff0b9d4e38b699d2bb4e2e0aa281dcb5..d28580c3de99715ef9615c982d6bee140fc68998 100644 (file)
--- a/src/lib/dhcpsrv/lease.cc
+++ b/src/lib/dhcpsrv/lease.cc
@@ -383,6 +383,7 @@ Lease4::decline(uint32_t probation_period) {
     fqdn_rev_ = false;
     state_ = STATE_DECLINED;
     valid_lft_ = probation_period;
+    setContext(ElementPtr());
 }
 
 isc::data::ElementPtr

diff --git a/src/lib/dhcpsrv/tests/lease_unittest.cc b/src/lib/dhcpsrv/tests/lease_unittest.cc
index 024af5185750b4adbc7f0f52a2c6fda7f1d22687..98222e760c29780af60882549e07cced2197403e 100644 (file)
--- a/src/lib/dhcpsrv/tests/lease_unittest.cc
+++ b/src/lib/dhcpsrv/tests/lease_unittest.cc
@@ -593,6 +593,9 @@ TEST_F(Lease4Test, decline) {
     lease.fqdn_rev_ = true;
 
     time_t now = time(0);
+    ElementPtr context = Element::createMap();
+    context->set("bogus", Element::create("true"));
+    lease.setContext(context);
 
     // Move lease to declined state and set its valid-lifetime to 123 seconds
     lease.decline(123);