BIND 10 trac3191, updated. 7abead307c37c5d1f2a3ef3540e0648985997297 [3191] Changes after review:

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Oct 17 17:11:13 UTC 2013


The branch, trac3191 has been updated
       via  7abead307c37c5d1f2a3ef3540e0648985997297 (commit)
      from  e98ccfe63db14be0d9a027797ffd75885b296969 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 7abead307c37c5d1f2a3ef3540e0648985997297
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Thu Oct 17 19:10:50 2013 +0200

    [3191] Changes after review:
    
     - new negative unit-test for next-server written
     - comments update (capitalized!)
     - indents corrected
     - no more raw pointers
     - ChangeLog added

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                     |    6 +++
 doc/guide/bind10-guide.xml                    |    2 +-
 src/bin/dhcp4/config_parser.cc                |    8 ++--
 src/bin/dhcp4/dhcp4.spec                      |    6 +--
 src/bin/dhcp4/tests/config_parser_unittest.cc |   51 +++++++++++++++++++++++++
 src/lib/dhcpsrv/subnet.cc                     |    2 +-
 src/lib/dhcpsrv/subnet.h                      |    5 ++-
 src/lib/dhcpsrv/tests/subnet_unittest.cc      |    2 +-
 8 files changed, 70 insertions(+), 12 deletions(-)

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index a8af862..ab0fb54 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+6XX.	[func]		tomek
+	b10-dhcp4: It is now possible to specify value of siaddr field
+	in DHCPv4 responses. It is used to point out to the next
+	server in the boot process (that typically is TFTP server).
+	(Trac #3191, git ABCD)
+
 690.	[bug]		tomek
 	b10-dhcp4: Relay Agent Info option is now echoed back in
 	DHCPv4 responses.
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index 36fd6d5..fe166ee 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -4390,7 +4390,7 @@ Dhcp4/subnet4	[]	list	(default)
 
     <section id="dhcp4-next-server">
       <title>Next server (siaddr)</title>
-      <para>In some cases, clients want to obtain configuration form the TFTP server.
+      <para>In some cases, clients want to obtain configuration from the TFTP server.
       Although there is a dedicated option for it, some devices may use siaddr field
       in the DHCPv4 packet for that purpose. That specific field can be configured
       using next-server directive. It is possible to define it in global scope or
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index 6c5bbee..9c7ca51 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -264,15 +264,15 @@ protected:
 
         LOG_INFO(dhcp4_logger, DHCP4_CONFIG_NEW_SUBNET).arg(tmp.str());
 
-        Subnet4* subnet4 = new Subnet4(addr, len, t1, t2, valid);
-        subnet_.reset(subnet4);
+        Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid));
+        subnet_ = subnet4;
 
         // Try global value first
         try {
             string next_server = globalContext()->string_values_->getParam("next-server");
             subnet4->setSiaddr(IOAddress(next_server));
         } catch (const DhcpConfigError&) {
-            // don't care. next_server is optional. We can live without it
+            // Don't care. next_server is optional. We can live without it
         }
 
         // Try subnet specific value if it's available
@@ -280,7 +280,7 @@ protected:
             string next_server = string_values_->getParam("next-server");
             subnet4->setSiaddr(IOAddress(next_server));
         } catch (const DhcpConfigError&) {
-            // don't care. next_server is optional. We can live without it
+            // Don't care. next_server is optional. We can live without it
         }
     }
 };
diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec
index bedbc7c..6fe26d6 100644
--- a/src/bin/dhcp4/dhcp4.spec
+++ b/src/bin/dhcp4/dhcp4.spec
@@ -226,9 +226,9 @@
                 },
 
                 { "item_name": "next-server",
-                    "item_type": "string",
-                    "item_optional": true,
-                    "item_default": "0.0.0.0"
+                  "item_type": "string",
+                  "item_optional": true,
+                  "item_default": "0.0.0.0"
                 },
 
                 { "item_name": "pool",
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index ee769e3..162c837 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -469,6 +469,57 @@ TEST_F(Dhcp4ParserTest, nextServerSubnet) {
     EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
 }
 
+// Test checks several negative scenarios for next-server configuration: bogus
+// address, IPv6 adddress and empty string.
+TEST_F(Dhcp4ParserTest, nextServerNegative) {
+
+    ConstElementPtr status;
+
+    // Config with junk instead of next-server address
+    string config_bogus1 = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"next-server\": \"a.b.c.d\", "
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    // Config with IPv6 next server address
+    string config_bogus2 = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"next-server\": \"2001:db8::1\", "
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    // Config with empty next server address
+    string config_bogus3 = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"next-server\": \"\", "
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json1 = Element::fromJSON(config_bogus1);
+    ElementPtr json2 = Element::fromJSON(config_bogus2);
+    ElementPtr json3 = Element::fromJSON(config_bogus2);
+
+    // check if returned status is always a failure
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json1));
+    checkResult(status, 1);
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json2));
+    checkResult(status, 1);
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json3));
+    checkResult(status, 1);
+}
+
 // Checks if the next-server defined as global value is overridden by subnet
 // specific value.
 TEST_F(Dhcp4ParserTest, nextServerOverride) {
diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc
index a4dcc56..5488f82 100644
--- a/src/lib/dhcpsrv/subnet.cc
+++ b/src/lib/dhcpsrv/subnet.cc
@@ -154,7 +154,7 @@ Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
 
 void Subnet4::setSiaddr(const isc::asiolink::IOAddress& siaddr) {
     if (!siaddr.isV4()) {
-        isc_throw(BadValue, "Can't set siaddr to non-IPv4 addr "
+        isc_throw(BadValue, "Can't set siaddr to non-IPv4 address "
                   << siaddr.toText());
     }
     siaddr_ = siaddr;
diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h
index dd8f102..6c49f60 100644
--- a/src/lib/dhcpsrv/subnet.h
+++ b/src/lib/dhcpsrv/subnet.h
@@ -466,14 +466,15 @@ public:
             const Triplet<uint32_t>& t2,
             const Triplet<uint32_t>& valid_lifetime);
 
-    /// @brief sets siaddr for the Subnet4
+    /// @brief Sets siaddr for the Subnet4
     ///
     /// Will be used for siaddr field (the next server) that typically is used
     /// as TFTP server. If not specified, the default value of 0.0.0.0 is
     /// used.
     void setSiaddr(const isc::asiolink::IOAddress& siaddr);
 
-    /// @brief returns siaddr for this subnet
+    /// @brief Returns siaddr for this subnet
+    ///
     /// @return siaddr value
     isc::asiolink::IOAddress getSiaddr() const;
 
diff --git a/src/lib/dhcpsrv/tests/subnet_unittest.cc b/src/lib/dhcpsrv/tests/subnet_unittest.cc
index 760e3d6..60142d4 100644
--- a/src/lib/dhcpsrv/tests/subnet_unittest.cc
+++ b/src/lib/dhcpsrv/tests/subnet_unittest.cc
@@ -58,7 +58,7 @@ TEST(Subnet4Test, in_range) {
     EXPECT_FALSE(subnet.inRange(IOAddress("255.255.255.255")));
 }
 
-// Checks whether siaddr field is handle correctly
+// Checks whether siaddr field can be set and retrieved correctly.
 TEST(Subnet4Test, siaddr) {
     Subnet4 subnet(IOAddress("192.0.2.1"), 24, 1000, 2000, 3000);
 



More information about the bind10-changes mailing list