From 3c2028c1aa4edf2ce0b0d6e55239c28d7c0387a9 Mon Sep 17 00:00:00 2001
From: Razvan Becheriu <razvan@isc.org>
Date: Thu, 20 Aug 2020 21:42:20 +0300
Subject: [PATCH] [#505] addressed review

---
 .../dhcpsrv/parsers/base_network_parser.cc    |  2 +-
 src/lib/dhcpsrv/parsers/dhcp_parsers.cc       |  6 ++--
 .../tests/shared_network_parser_unittest.cc   | 34 +++++++++++++++----
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.cc b/src/lib/dhcpsrv/parsers/base_network_parser.cc
index 89ebfe9df8..4bcd178890 100644
--- a/src/lib/dhcpsrv/parsers/base_network_parser.cc
+++ b/src/lib/dhcpsrv/parsers/base_network_parser.cc
@@ -125,7 +125,7 @@ BaseNetworkParser::parseCommon(const ConstElementPtr& network_data,
 
     if (has_renew && has_rebind && (renew > rebind)) {
         isc_throw(DhcpConfigError, "the value of renew-timer" << " (" << renew
-                  << ") is not less than rebind-timer" << " (" << rebind << ")");
+                  << ") is greater than rebind-timer" << " (" << rebind << ")");
     }
 
     network->setValid(parseLifetime(network_data, "valid-lifetime"));
diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 11265f5999..9f339d69b4 100644
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -760,14 +760,14 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params,
         renew = subnet4->getT1().get();
         output << "t1=" << renew << ", ";
     }
-    if (rebind) {
+    if (has_rebind) {
         rebind = subnet4->getT2().get();
         output << "t2=" << rebind << ", ";
     }
 
     if (has_renew && has_rebind && (renew > rebind)) {
         isc_throw(DhcpConfigError, "the value of renew-timer" << " (" << renew
-                  << ") is not less than rebind-timer" << " (" << rebind << ")");
+                  << ") is greater than rebind-timer" << " (" << rebind << ")");
     }
 
     if (!subnet4->getValid().unspecified()) {
@@ -1262,7 +1262,7 @@ Subnet6ConfigParser::initSubnet(data::ConstElementPtr params,
 
     if (has_renew && has_rebind && (renew > rebind)) {
         isc_throw(DhcpConfigError, "the value of renew-timer" << " (" << renew
-                  << ") is not less than rebind-timer" << " (" << rebind << ")");
+                  << ") is greater than rebind-timer" << " (" << rebind << ")");
     }
 
     if (!subnet6->getPreferred().unspecified()) {
diff --git a/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc b/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
index 6b3db1c9c3..b63017c6ba 100644
--- a/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
@@ -474,14 +474,14 @@ TEST_F(SharedNetwork4ParserTest, iface) {
 TEST_F(SharedNetwork4ParserTest, parseWithInvalidRenewRebind) {
     IfaceMgrTestConfig ifmgr(true);
 
-    // Basic configuration for shared network. A bunch of parameters
-    // have to be specified for subnets because subnet parsers expect
-    // that default and global values are set.
+    // Basic configuration for shared network.
     std::string config = getWorkingConfig();
     ElementPtr config_element = Element::fromJSON(config);
-    ConstElementPtr valid_element = config_element->get("renew-timer");
-    ElementPtr invalid_element = boost::const_pointer_cast<Element>(valid_element);
-    invalid_element->setValue(200);
+    ConstElementPtr valid_element = config_element->get("rebind-timer");
+    int64_t value = valid_element->intValue();
+    valid_element = config_element->get("renew-timer");
+    ElementPtr mutable_element = boost::const_pointer_cast<Element>(valid_element);
+    mutable_element->setValue(value + 1);
 
     // Parse configuration specified above.
     SharedNetwork4Parser parser;
@@ -491,6 +491,28 @@ TEST_F(SharedNetwork4ParserTest, parseWithInvalidRenewRebind) {
     ASSERT_FALSE(network);
 }
 
+// This test verifies that shared network parser for IPv4 works properly
+// when renew and rebind timers are equal.
+TEST_F(SharedNetwork4ParserTest, parseValidWithEqualRenewRebind) {
+    IfaceMgrTestConfig ifmgr(true);
+
+    // Basic configuration for shared network.
+    std::string config = getWorkingConfig();
+    ElementPtr config_element = Element::fromJSON(config);
+    ConstElementPtr valid_element = config_element->get("rebind-timer");
+    int64_t value = valid_element->intValue();
+    valid_element = config_element->get("renew-timer");
+    ElementPtr mutable_element = boost::const_pointer_cast<Element>(valid_element);
+    mutable_element->setValue(value);
+
+    // Parse configuration specified above.
+    SharedNetwork4Parser parser;
+    SharedNetwork4Ptr network;
+
+    ASSERT_NO_THROW(network = parser.parse(config_element));
+    ASSERT_TRUE(network);
+}
+
 /// @brief Test fixture class for SharedNetwork6Parser class.
 class SharedNetwork6ParserTest : public SharedNetworkParserTest {
 public:
-- 
2.47.2