BIND 10 trac2524, updated. 62794b031c00451e7bec0a0b94b4a2dadb42ae0e [2524] Various fixes in response to review

BIND 10 source code commits bind10-changes at lists.isc.org
Sat Dec 29 15:35:19 UTC 2012


The branch, trac2524 has been updated
       via  62794b031c00451e7bec0a0b94b4a2dadb42ae0e (commit)
      from  b07567633570ebffebb13e92ae71d429967a7cb8 (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 62794b031c00451e7bec0a0b94b4a2dadb42ae0e
Author: Stephen Morris <stephen at isc.org>
Date:   Sat Dec 29 15:34:11 2012 +0000

    [2524] Various fixes in response to review

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

Summary of changes:
 src/lib/dhcpsrv/Makefile.am                        |    6 +-
 src/lib/dhcpsrv/dhcpsrv_log.cc                     |    2 +-
 src/lib/dhcpsrv/hwaddr.h                           |    9 +--
 src/lib/dhcpsrv/lease_mgr_factory.cc               |    2 +-
 src/lib/dhcpsrv/tests/cfgmgr_unittest.cc           |   20 +++---
 .../dhcpsrv/tests/lease_mgr_factory_unittest.cc    |   74 ++++++++++++++++----
 6 files changed, 80 insertions(+), 33 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am
index 3e2891b..d2e604a 100644
--- a/src/lib/dhcpsrv/Makefile.am
+++ b/src/lib/dhcpsrv/Makefile.am
@@ -12,9 +12,9 @@ AM_CXXFLAGS = $(B10_CXXFLAGS)
 dhcpsrv_messages.h dhcpsrv_messages.cc: dhcpsrv_messages.mes
 	$(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/dhcpsrv/dhcpsrv_messages.mes
 
-# Tell Automake that the nsas_messages.{cc,h} source files are created in the build
-# process, so it must create these before doing anything else.  Although they
-# are a dependency of the library (so will be created from the message file
+# Tell Automake that the dhcpsrv_messages.{cc,h} source files are created in the
+# build process, so it must create these before doing anything else.  Although
+# they are a dependency of the library (so will be created from the message file
 # anyway), there is no guarantee as to exactly _when_ in the build they will be
 # created.  As the .h file is included in other sources file (so must be
 # present when they are compiled), the safest option is to create it first.
diff --git a/src/lib/dhcpsrv/dhcpsrv_log.cc b/src/lib/dhcpsrv/dhcpsrv_log.cc
index 10ac3ee..6cbfb0d 100644
--- a/src/lib/dhcpsrv/dhcpsrv_log.cc
+++ b/src/lib/dhcpsrv/dhcpsrv_log.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
diff --git a/src/lib/dhcpsrv/hwaddr.h b/src/lib/dhcpsrv/hwaddr.h
index a735792..776f37b 100644
--- a/src/lib/dhcpsrv/hwaddr.h
+++ b/src/lib/dhcpsrv/hwaddr.h
@@ -12,8 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#ifndef __HWADDR_H
-#define __HWADDR_H
+#ifndef HWADDR_H
+#define HWADDR_H
 
 #include <string>
 #include <vector>
@@ -30,9 +30,6 @@ typedef std::vector<uint8_t> HWAddr;
 /// Returns a string containing the hardware address. This is only used for
 /// logging.
 ///
-/// @note Six characters is an arbitrary length, chosen to provide a
-///       suitably wide string.
-///
 /// @todo Create a "hardware address" class of which this will be a member.
 ///
 /// @param hwaddr Hardware address to convert to string form
@@ -44,4 +41,4 @@ hardwareAddressString(const HWAddr& hwaddr);
 };  // namespace dhcp
 };  // namespace isc
 
-#endif // __HWADDR_H
+#endif // HWADDR_H
diff --git a/src/lib/dhcpsrv/lease_mgr_factory.cc b/src/lib/dhcpsrv/lease_mgr_factory.cc
index 06e2f79..23ba12f 100644
--- a/src/lib/dhcpsrv/lease_mgr_factory.cc
+++ b/src/lib/dhcpsrv/lease_mgr_factory.cc
@@ -54,7 +54,7 @@ LeaseMgrFactory::parse(const std::string& dbaccess) {
         // there are cryptic warnings on Debian6 running g++ 4.4 in
         // /usr/include/c++/4.4/bits/stl_algo.h:2178 "array subscript is above
         // array bounds"
-        boost::split(tokens, dbaccess, boost::is_any_of( string("\t ") ));
+        boost::split(tokens, dbaccess, boost::is_any_of(string("\t ")));
         BOOST_FOREACH(std::string token, tokens) {
             size_t pos = token.find("=");
             if (pos != string::npos) {
diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
index bd4eba6..353d833 100644
--- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
@@ -58,7 +58,7 @@ TEST_F(CfgMgrTest, subnet4) {
     Subnet4Ptr subnet3(new Subnet4(IOAddress("192.0.2.128"), 26, 1, 2, 3));
 
     // There shouldn't be any subnet configured at this stage
-    EXPECT_EQ(Subnet4Ptr(), cfg_mgr.getSubnet4(IOAddress("192.0.2.0")));
+    EXPECT_FALSE(cfg_mgr.getSubnet4(IOAddress("192.0.2.0")));
 
     cfg_mgr.addSubnet4(subnet1);
 
@@ -75,13 +75,13 @@ TEST_F(CfgMgrTest, subnet4) {
     EXPECT_EQ(subnet2, cfg_mgr.getSubnet4(IOAddress("192.0.2.85")));
 
     // Try to find an address that does not belong to any subnet
-    EXPECT_EQ(Subnet4Ptr(), cfg_mgr.getSubnet4(IOAddress("192.0.2.192")));
+    EXPECT_FALSE(cfg_mgr.getSubnet4(IOAddress("192.0.2.192")));
 
     // Check that deletion of the subnets works.
     cfg_mgr.deleteSubnets4();
-    EXPECT_EQ(Subnet4Ptr(), cfg_mgr.getSubnet4(IOAddress("192.0.2.191")));
-    EXPECT_EQ(Subnet4Ptr(), cfg_mgr.getSubnet4(IOAddress("192.0.2.15")));
-    EXPECT_EQ(Subnet4Ptr(), cfg_mgr.getSubnet4(IOAddress("192.0.2.85")));
+    EXPECT_FALSE(cfg_mgr.getSubnet4(IOAddress("192.0.2.191")));
+    EXPECT_FALSE(cfg_mgr.getSubnet4(IOAddress("192.0.2.15")));
+    EXPECT_FALSE(cfg_mgr.getSubnet4(IOAddress("192.0.2.85")));
 }
 
 // This test verifies if the configuration manager is able to hold and return
@@ -95,7 +95,7 @@ TEST_F(CfgMgrTest, subnet6) {
     Subnet6Ptr subnet3(new Subnet6(IOAddress("4000::"), 48, 1, 2, 3, 4));
 
     // There shouldn't be any subnet configured at this stage
-    EXPECT_EQ(Subnet6Ptr(), cfg_mgr.getSubnet6(IOAddress("2000::1")));
+    EXPECT_FALSE(cfg_mgr.getSubnet6(IOAddress("2000::1")));
 
     cfg_mgr.addSubnet6(subnet1);
 
@@ -111,13 +111,13 @@ TEST_F(CfgMgrTest, subnet6) {
 
     EXPECT_EQ(subnet3, cfg_mgr.getSubnet6(IOAddress("4000::123")));
     EXPECT_EQ(subnet2, cfg_mgr.getSubnet6(IOAddress("3000::dead:beef")));
-    EXPECT_EQ(Subnet6Ptr(), cfg_mgr.getSubnet6(IOAddress("5000::1")));
+    EXPECT_FALSE(cfg_mgr.getSubnet6(IOAddress("5000::1")));
 
     // Check that deletion of the subnets works.
     cfg_mgr.deleteSubnets6();
-    EXPECT_EQ(Subnet6Ptr(), cfg_mgr.getSubnet6(IOAddress("200::123")));
-    EXPECT_EQ(Subnet6Ptr(), cfg_mgr.getSubnet6(IOAddress("3000::123")));
-    EXPECT_EQ(Subnet6Ptr(), cfg_mgr.getSubnet6(IOAddress("4000::123")));
+    EXPECT_FALSE(cfg_mgr.getSubnet6(IOAddress("200::123")));
+    EXPECT_FALSE(cfg_mgr.getSubnet6(IOAddress("3000::123")));
+    EXPECT_FALSE(cfg_mgr.getSubnet6(IOAddress("4000::123")));
 }
 
 } // end of anonymous namespace
diff --git a/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc b/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc
index e83541d..9924476 100644
--- a/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc
+++ b/src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc
@@ -80,15 +80,45 @@ TEST_F(LeaseMgrFactoryTest, parseInvalid) {
 ///
 /// Checks that the redacted configuration string includes the password only
 /// as a set of asterisks.
-TEST(LeaseMgr, redactAccessString) {
+TEST_F(LeaseMgrFactoryTest, redactAccessString) {
 
-    // To check the redacted string, break it down into its component
-    // parameters and check the results.
-    LeaseMgr::ParameterMap parameters = LeaseMgrFactory::parse(
-        "user=me password=forbidden name=kea type=mysql");
+    LeaseMgr::ParameterMap parameters =
+        LeaseMgrFactory::parse("user=me password=forbidden name=kea type=mysql");
+    EXPECT_EQ(4, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("forbidden", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+
+    // Redact the result.  To check, break the redacted string down into its
+    // components.
     std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
+    parameters = LeaseMgrFactory::parse(redacted);
+
+    EXPECT_EQ(4, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("*****", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+}
 
-    // ... and break the redacted string down into its components.
+/// @brief redactConfigString test - empty password
+///
+/// Checks that the redacted configuration string includes the password only
+/// as a set of asterisks, even if the password is null.
+TEST_F(LeaseMgrFactoryTest, redactAccessStringEmptyPassword) {
+
+    LeaseMgr::ParameterMap parameters =
+        LeaseMgrFactory::parse("user=me name=kea type=mysql password=");
+    EXPECT_EQ(4, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+
+    // Redact the result.  To check, break the redacted string down into its
+    // components.
+    std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
     parameters = LeaseMgrFactory::parse(redacted);
 
     EXPECT_EQ(4, parameters.size());
@@ -97,9 +127,15 @@ TEST(LeaseMgr, redactAccessString) {
     EXPECT_EQ("kea", parameters["name"]);
     EXPECT_EQ("mysql", parameters["type"]);
 
-    // Do the same, but check it works for an empty password.
-    parameters = LeaseMgrFactory::parse(
-        "user=me password=forbidden name=kea type=mysql");
+    // ... and again to check that the position of the empty password in the
+    // string does not matter.
+    parameters = LeaseMgrFactory::parse("user=me password= name=kea type=mysql");
+    EXPECT_EQ(4, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+
     redacted = LeaseMgrFactory::redactedAccessString(parameters);
     parameters = LeaseMgrFactory::parse(redacted);
 
@@ -108,10 +144,24 @@ TEST(LeaseMgr, redactAccessString) {
     EXPECT_EQ("*****", parameters["password"]);
     EXPECT_EQ("kea", parameters["name"]);
     EXPECT_EQ("mysql", parameters["type"]);
+}
 
-    // Do the same, but check it works in the absence of a password token
-    parameters = LeaseMgrFactory::parse("user=me name=kea type=mysql");
-    redacted = LeaseMgrFactory::redactedAccessString(parameters);
+/// @brief redactConfigString test - no password
+///
+/// Checks that the redacted configuration string excludes the password if there
+/// was no password to begion with.
+TEST_F(LeaseMgrFactoryTest, redactAccessStringNoPassword) {
+
+    LeaseMgr::ParameterMap parameters =
+        LeaseMgrFactory::parse("user=me name=kea type=mysql");
+    EXPECT_EQ(3, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+
+    // Redact the result.  To check, break the redacted string down into its
+    // components.
+    std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
     parameters = LeaseMgrFactory::parse(redacted);
 
     EXPECT_EQ(3, parameters.size());



More information about the bind10-changes mailing list