From bd8ac0ce36dee77dc4865239576745db7588db60 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 13 Dec 2012 15:39:51 +0100 Subject: [PATCH 1/8] [2313] Added new class to represent option space. --- src/lib/dhcp/Makefile.am | 12 ++-- src/lib/dhcpsrv/Makefile.am | 1 + src/lib/dhcpsrv/option_space.cc | 25 +++++++ src/lib/dhcpsrv/option_space.h | 69 +++++++++++++++++++ src/lib/dhcpsrv/tests/Makefile.am | 1 + .../dhcpsrv/tests/option_space_unittest.cc | 41 +++++++++++ 6 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 src/lib/dhcpsrv/option_space.cc create mode 100644 src/lib/dhcpsrv/option_space.h create mode 100644 src/lib/dhcpsrv/tests/option_space_unittest.cc diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index 571f11d398..7305e0c50d 100644 --- a/src/lib/dhcp/Makefile.am +++ b/src/lib/dhcp/Makefile.am @@ -14,23 +14,23 @@ CLEANFILES = *.gcno *.gcda lib_LTLIBRARIES = libb10-dhcp++.la libb10_dhcp___la_SOURCES = +libb10_dhcp___la_SOURCES += dhcp6.h dhcp4.h libb10_dhcp___la_SOURCES += duid.cc duid.h libb10_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc libb10_dhcp___la_SOURCES += iface_mgr_linux.cc libb10_dhcp___la_SOURCES += iface_mgr_sun.cc libb10_dhcp___la_SOURCES += libdhcp++.cc libdhcp++.h -libb10_dhcp___la_SOURCES += option.cc option.h -libb10_dhcp___la_SOURCES += option_data_types.cc option_data_types.h -libb10_dhcp___la_SOURCES += option_definition.cc option_definition.h -libb10_dhcp___la_SOURCES += option_custom.cc option_custom.h +libb10_dhcp___la_SOURCES += option4_addrlst.cc option4_addrlst.h libb10_dhcp___la_SOURCES += option6_ia.cc option6_ia.h libb10_dhcp___la_SOURCES += option6_iaaddr.cc option6_iaaddr.h libb10_dhcp___la_SOURCES += option6_addrlst.cc option6_addrlst.h -libb10_dhcp___la_SOURCES += option4_addrlst.cc option4_addrlst.h libb10_dhcp___la_SOURCES += option_int.h libb10_dhcp___la_SOURCES += option_int_array.h -libb10_dhcp___la_SOURCES += dhcp6.h dhcp4.h +libb10_dhcp___la_SOURCES += option.cc option.h +libb10_dhcp___la_SOURCES += option_custom.cc option_custom.h +libb10_dhcp___la_SOURCES += option_data_types.cc option_data_types.h +libb10_dhcp___la_SOURCES += option_definition.cc option_definition.h libb10_dhcp___la_SOURCES += pkt6.cc pkt6.h libb10_dhcp___la_SOURCES += pkt4.cc pkt4.h libb10_dhcp___la_SOURCES += std_option_defs.h diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index b44fa7d415..fc888f6b67 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -27,6 +27,7 @@ libb10_dhcpsrv_la_SOURCES += memfile_lease_mgr.cc memfile_lease_mgr.h if HAVE_MYSQL libb10_dhcpsrv_la_SOURCES += mysql_lease_mgr.cc mysql_lease_mgr.h endif +libb10_dhcpsrv_la_SOURCES += option_space.cc option_space.h libb10_dhcpsrv_la_SOURCES += pool.cc pool.h libb10_dhcpsrv_la_SOURCES += subnet.cc subnet.h libb10_dhcpsrv_la_SOURCES += triplet.h diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc new file mode 100644 index 0000000000..297b8a57e5 --- /dev/null +++ b/src/lib/dhcpsrv/option_space.cc @@ -0,0 +1,25 @@ +// 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 +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include + +namespace isc { +namespace dhcp { + +OptionSpace::OptionSpace(const std::string& name, const bool vendor_space) + : name_(name), vendor_space_(vendor_space) { +} + +} // end of isc::dhcp namespace +} // end of isc namespace diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h new file mode 100644 index 0000000000..3a25c1c2fa --- /dev/null +++ b/src/lib/dhcpsrv/option_space.h @@ -0,0 +1,69 @@ +// 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 +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef OPTION_SPACE_H +#define OPTION_SPACE_H + +#include +#include + +namespace isc { +namespace dhcp { + +class OptionSpace; + +typedef boost::shared_ptr OptionSpacePtr; + +/// @brief Option code space. +/// +/// This class represents single option space. The option spaces are used +/// to group DHCP options having unique option codes. The special type +/// of the option space is so called "vendor option space". It groups +/// sub-options being sent within Vendor Encapsulated Options. These are +/// option with code 43 for DHCPv4 and option with code 17 for DHCPv6. +/// The option spaces are assigned to option instances represented by +/// isc::dhcp::Option and other classes derived from it. Each particular +/// option may belong to multiple option spaces. +class OptionSpace { +public: + + /// @brief Constructor. + /// + /// @param name option space name. + /// @param vendor_space boolean value that indicates that the object + /// describes the vendor space. + OptionSpace(const std::string& name, const bool vendor_space = false); + + /// @brief Return option space name. + /// + /// @return option space name. + const std::string& getName() const { return (name_); } + + /// @brief Check if option space is a vendor space. + /// + /// @return boolean value that indicates if the object describes + /// the vendor space. + bool isVendorSpace() const { return (vendor_space_); } + +private: + std::string name_; ///< Holds option space name. + + bool vendor_space_; ///< Is this the vendor space? + +}; + +} // namespace isc::dhcp +} // namespace isc + +#endif // OPTION_SPACE_H diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 11e1d75c1d..dfe3e785c5 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -36,6 +36,7 @@ libdhcpsrv_unittests_SOURCES += memfile_lease_mgr_unittest.cc if HAVE_MYSQL libdhcpsrv_unittests_SOURCES += mysql_lease_mgr_unittest.cc endif +libdhcpsrv_unittests_SOURCES += option_space_unittest.cc libdhcpsrv_unittests_SOURCES += pool_unittest.cc libdhcpsrv_unittests_SOURCES += schema_copy.h libdhcpsrv_unittests_SOURCES += subnet_unittest.cc diff --git a/src/lib/dhcpsrv/tests/option_space_unittest.cc b/src/lib/dhcpsrv/tests/option_space_unittest.cc new file mode 100644 index 0000000000..011b4db073 --- /dev/null +++ b/src/lib/dhcpsrv/tests/option_space_unittest.cc @@ -0,0 +1,41 @@ +// 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 +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include + +#include + +#include + +using namespace isc::dhcp; +using namespace isc; + +namespace { + +// The purpose of this test is to verify that the constructor +// creates an object with members initialized to correct values. +TEST(OptionSpaceTest, constructor) { + // Create some option space. + OptionSpace space("isc", true); + EXPECT_EQ("isc", space.getName()); + EXPECT_TRUE(space.isVendorSpace()); + + // Create another object with different values + // to check that the values will change. + OptionSpace space2("abc", false); + EXPECT_EQ("abc", space2.getName()); + EXPECT_FALSE(space2.isVendorSpace()); +} + +}; // end of anonymous namespace From d4eda56344420aafa49d1c0d91d1a6a95c7a4707 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 13 Dec 2012 16:48:56 +0100 Subject: [PATCH 2/8] [2313] Add option spaces into configuration manager. --- src/lib/dhcpsrv/cfgmgr.cc | 33 ++++++++++-- src/lib/dhcpsrv/cfgmgr.h | 37 +++++++++++++ src/lib/dhcpsrv/option_space.h | 15 +++++- src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 66 ++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 7dc5f555d9..beb2df2157 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -21,15 +21,42 @@ using namespace isc::util; namespace isc { namespace dhcp { - - - CfgMgr& CfgMgr::instance() { static CfgMgr cfg_mgr; return (cfg_mgr); } +void +CfgMgr::addOptionSpace4(const OptionSpacePtr& space) { + if (!space) { + isc_throw(InvalidOptionSpace, + "provided option space object is NULL."); + } + OptionSpaceCollection::iterator it = spaces4_.find(space->getName()); + if (it != spaces4_.end()) { + isc_throw(InvalidOptionSpace, "option space " << space->getName() + << " already added."); + } + spaces4_.insert(std::pair(space->getName(), space)); +} + +void +CfgMgr::addOptionSpace6(const OptionSpacePtr& space) { + if (!space) { + isc_throw(InvalidOptionSpace, + "provided option space object is NULL."); + } + OptionSpaceCollection::iterator it = spaces6_.find(space->getName()); + if (it != spaces6_.end()) { + isc_throw(InvalidOptionSpace, "option space " << space->getName() + << " already added."); + } + spaces6_.insert(std::pair(space->getName(), space)); +} + Subnet6Ptr CfgMgr::getSubnet6(const isc::asiolink::IOAddress& hint) { diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index ac1b3f59a4..523cd101fd 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -77,6 +78,36 @@ public: /// accessing it. static CfgMgr& instance(); + /// @brief Adds new DHCPv4 option space to the collection. + /// + /// @param space option space to be added. + /// + /// @throw isc::dhcp::InvalidOptionSpace invalid option space + /// has been specified. + void addOptionSpace4(const OptionSpacePtr& space); + + /// @brief Adds new DHCPv6 option space to the collection. + /// + /// @param space option space to be added. + /// + /// @throw isc::dhcp::InvalidOptionSpace invalid option space + /// has been specified. + void addOptionSpace6(const OptionSpacePtr& space); + + /// @brief Return option spaces for DHCPv4. + /// + /// @return A collection of option spaces. + const OptionSpaceCollection& getOptionSpaces4() const { + return (spaces4_); + } + + /// @brief Return option spaces for DHCPv6. + /// + /// @return A collection of option spaces. + const OptionSpaceCollection& getOptionSpaces6() const { + return (spaces6_); + } + /// @brief get IPv6 subnet by address /// /// Finds a matching subnet, based on an address. This can be used @@ -169,6 +200,12 @@ protected: /// pattern will use calling inRange() method on each subnet until /// a match is found. Subnet4Collection subnets4_; + + /// @brief Container for defined DHCPv6 option spaces. + OptionSpaceCollection spaces6_; + + /// @brief Container for defined DHCPv4 option spaces. + OptionSpaceCollection spaces4_; }; } // namespace isc::dhcp diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h index 3a25c1c2fa..5eac1a8ddb 100644 --- a/src/lib/dhcpsrv/option_space.h +++ b/src/lib/dhcpsrv/option_space.h @@ -15,15 +15,28 @@ #ifndef OPTION_SPACE_H #define OPTION_SPACE_H +#include #include +#include #include namespace isc { namespace dhcp { -class OptionSpace; +/// @brief Exception to be thrown when invalid option space +/// is specified. +class InvalidOptionSpace : public Exception { +public: + InvalidOptionSpace(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; +/// OptionSpace forward declaration. +class OptionSpace; +/// A pointer to OptionSpace object. typedef boost::shared_ptr OptionSpacePtr; +/// A collection of option spaces. +typedef std::map OptionSpaceCollection; /// @brief Option code space. /// diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 7b23bcf409..7d3d9caa71 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -112,4 +112,70 @@ TEST_F(CfgMgrTest, subnet6) { EXPECT_EQ(Subnet6Ptr(), cfg_mgr.getSubnet6(IOAddress("4000::123"))); } +// This test verifies that new DHCPv4 option spaces can be added to +// the configuration manager and that duplicated option space is +// rejected. +TEST_F(CfgMgrTest, optionSpace4) { + CfgMgr& cfg_mgr = CfgMgr::instance(); + + // Create some option spaces. + OptionSpacePtr space1(new OptionSpace("isc", false)); + OptionSpacePtr space2(new OptionSpace("xyz", true)); + + // Add option spaces with different names and expect they + // are accepted. + ASSERT_NO_THROW(cfg_mgr.addOptionSpace4(space1)); + ASSERT_NO_THROW(cfg_mgr.addOptionSpace4(space2)); + + // Validate that the option spaces have been added correctly. + const OptionSpaceCollection& spaces = cfg_mgr.getOptionSpaces4(); + + ASSERT_EQ(2, spaces.size()); + EXPECT_FALSE(spaces.find("isc") == spaces.end()); + EXPECT_FALSE(spaces.find("xyz") == spaces.end()); + + // Create another option space with the name that duplicates + // the existing option space. + OptionSpacePtr space3(new OptionSpace("isc", true)); + // Expect that the duplicate option space is rejected. + ASSERT_THROW( + cfg_mgr.addOptionSpace4(space3), isc::dhcp::InvalidOptionSpace + ); + + // @todo decode if a duplicate vendor space is allowed. +} + +// This test verifies that new DHCPv6 option spaces can be added to +// the configuration manager and that duplicated option space is +// rejected. +TEST_F(CfgMgrTest, optionSpace6) { + CfgMgr& cfg_mgr = CfgMgr::instance(); + + // Create some option spaces. + OptionSpacePtr space1(new OptionSpace("isc", false)); + OptionSpacePtr space2(new OptionSpace("xyz", true)); + + // Add option spaces with different names and expect they + // are accepted. + ASSERT_NO_THROW(cfg_mgr.addOptionSpace6(space1)); + ASSERT_NO_THROW(cfg_mgr.addOptionSpace6(space2)); + + // Validate that the option spaces have been added correctly. + const OptionSpaceCollection& spaces = cfg_mgr.getOptionSpaces6(); + + ASSERT_EQ(2, spaces.size()); + EXPECT_FALSE(spaces.find("isc") == spaces.end()); + EXPECT_FALSE(spaces.find("xyz") == spaces.end()); + + // Create another option space with the name that duplicates + // the existing option space. + OptionSpacePtr space3(new OptionSpace("isc", true)); + // Expect that the duplicate option space is rejected. + ASSERT_THROW( + cfg_mgr.addOptionSpace6(space3), isc::dhcp::InvalidOptionSpace + ); + + // @todo decide if a duplicate vendor space is allowed. +} + } // end of anonymous namespace From 85da7d68f1eff606608bd46df43a8f57436a0131 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 2 Jan 2013 15:58:59 +0100 Subject: [PATCH 3/8] [2313] Validate option space name. --- src/lib/dhcpsrv/option_space.cc | 20 ++++++- src/lib/dhcpsrv/option_space.h | 19 ++++++- .../dhcpsrv/tests/option_space_unittest.cc | 53 ++++++++++++++++++- 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc index 297b8a57e5..2b81b2ae9d 100644 --- a/src/lib/dhcpsrv/option_space.cc +++ b/src/lib/dhcpsrv/option_space.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012, 2013 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 @@ -13,12 +13,30 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include +#include namespace isc { namespace dhcp { OptionSpace::OptionSpace(const std::string& name, const bool vendor_space) : name_(name), vendor_space_(vendor_space) { + if (!validateName(name_)) { + isc_throw(InvalidOptionSpace, "Invalid option space name " + << name_); + } +} + +bool +OptionSpace::validateName(const std::string& name) { + if (boost::algorithm::all(name, boost::is_from_range('a', 'z') || + boost::is_from_range('A', 'Z') || + boost::is_digit() || + boost::is_any_of("-_")) && + !name.empty()) { + return (true); + } + return (false); } } // end of isc::dhcp namespace diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h index 5eac1a8ddb..d05317f636 100644 --- a/src/lib/dhcpsrv/option_space.h +++ b/src/lib/dhcpsrv/option_space.h @@ -1,4 +1,4 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012, 2013 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 @@ -69,6 +69,23 @@ public: /// the vendor space. bool isVendorSpace() const { return (vendor_space_); } + /// @brief Mark option space as vendor space or non-vendor space. + /// + /// @param a boolean value indicating that this option space is + /// a vendor space (true) or non-vendor space (false). + void setVendorSpace(const bool vendor_space) { + vendor_space_ = vendor_space; + } + + /// @brief Checks that the provided option space name is valid. + /// + /// It is expected that option space name consists of upper or + /// lower case letters or digits. All other characters are + /// prohibited. + /// + /// @param name option space name to be validated. + static bool validateName(const std::string& name); + private: std::string name_; ///< Holds option space name. diff --git a/src/lib/dhcpsrv/tests/option_space_unittest.cc b/src/lib/dhcpsrv/tests/option_space_unittest.cc index 011b4db073..0c520e0090 100644 --- a/src/lib/dhcpsrv/tests/option_space_unittest.cc +++ b/src/lib/dhcpsrv/tests/option_space_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012, 2013 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 @@ -36,6 +36,57 @@ TEST(OptionSpaceTest, constructor) { OptionSpace space2("abc", false); EXPECT_EQ("abc", space2.getName()); EXPECT_FALSE(space2.isVendorSpace()); + + // Verify that constructor throws exception if invalid + // option space name is provided. + EXPECT_THROW(OptionSpace("invalid%space.name"), InvalidOptionSpace); +} + +// The purpose of this test is to verify that the vendor-space flag +// can be overriden. +TEST(OptionSpaceTest, setVendorSpace) { + OptionSpace space("isc", true); + EXPECT_EQ("isc", space.getName()); + EXPECT_TRUE(space.isVendorSpace()); + + // Override the vendor space flag. + space.setVendorSpace(false); + EXPECT_FALSE(space.isVendorSpace()); +} + +// The purpose of this test is to verify that the static function +// to validate the option space name works correctly. +TEST(OptionSpaceTest, validateName) { + // Positive test scenarios: letters, digits, dashes, underscores + // lower/upper case allowed. + EXPECT_TRUE(OptionSpace::validateName("abc")); + EXPECT_TRUE(OptionSpace::validateName("dash-allowed")); + EXPECT_TRUE(OptionSpace::validateName("two-dashes-allowed")); + EXPECT_TRUE(OptionSpace::validateName("underscore_allowed")); + EXPECT_TRUE(OptionSpace::validateName("underscore_three_times_allowed")); + EXPECT_TRUE(OptionSpace::validateName("digits0912")); + EXPECT_TRUE(OptionSpace::validateName("1234")); + EXPECT_TRUE(OptionSpace::validateName("UPPER_CASE_allowed")); + + // Negative test scenarions: empty strings, dots, spaces are not + // allowed + EXPECT_FALSE(OptionSpace::validateName("")); + EXPECT_FALSE(OptionSpace::validateName(" ")); + EXPECT_FALSE(OptionSpace::validateName(" isc ")); + EXPECT_FALSE(OptionSpace::validateName("isc ")); + EXPECT_FALSE(OptionSpace::validateName(" isc")); + EXPECT_FALSE(OptionSpace::validateName("isc with-space")); + + // Test other special characters + const char specials[] = { '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', + '+', '=', '[', ']', '{', '}', ';', ':', '"', '\'', + '\\', '|', '<','>', ',', '.', '?', '~', '`' }; + for (int i = 0; i < sizeof(specials); ++i) { + std::ostringstream stream; + stream << "abc" << specials[i]; + EXPECT_FALSE(OptionSpace::validateName(stream.str())) + << "Test failed for special character '" << specials[i] << "'."; + } } }; // end of anonymous namespace From 810c95b11f458b18c196a39cba8f3d47ac1d6cb2 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 2 Jan 2013 17:07:58 +0100 Subject: [PATCH 4/8] [2313] Improved comments for OptionSpace class. --- src/lib/dhcpsrv/option_space.cc | 3 +++ src/lib/dhcpsrv/option_space.h | 16 ++++++++++++---- src/lib/dhcpsrv/tests/option_space_unittest.cc | 3 +++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc index 2b81b2ae9d..671245ef95 100644 --- a/src/lib/dhcpsrv/option_space.cc +++ b/src/lib/dhcpsrv/option_space.cc @@ -21,6 +21,7 @@ namespace dhcp { OptionSpace::OptionSpace(const std::string& name, const bool vendor_space) : name_(name), vendor_space_(vendor_space) { + // Check that provided option space name is valid. if (!validateName(name_)) { isc_throw(InvalidOptionSpace, "Invalid option space name " << name_); @@ -29,6 +30,8 @@ OptionSpace::OptionSpace(const std::string& name, const bool vendor_space) bool OptionSpace::validateName(const std::string& name) { + // Allowed digits are: lower or upper case letters, digits, + // underscores and dashes. Empty option spaces are not allowed. if (boost::algorithm::all(name, boost::is_from_range('a', 'z') || boost::is_from_range('A', 'Z') || boost::is_digit() || diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h index d05317f636..c722f4d44a 100644 --- a/src/lib/dhcpsrv/option_space.h +++ b/src/lib/dhcpsrv/option_space.h @@ -56,6 +56,11 @@ public: /// @param name option space name. /// @param vendor_space boolean value that indicates that the object /// describes the vendor space. + /// + /// @throw isc::dhcp::InvalidOptionSpace if given option space name + /// contains invalid characters or is empty. This constructor uses + /// \ref validateName function to check that the specified name is + /// correct. OptionSpace(const std::string& name, const bool vendor_space = false); /// @brief Return option space name. @@ -71,8 +76,8 @@ public: /// @brief Mark option space as vendor space or non-vendor space. /// - /// @param a boolean value indicating that this option space is - /// a vendor space (true) or non-vendor space (false). + /// @param vendor_space a boolean value indicating that this option + /// space is a vendor space (true) or non-vendor space (false). void setVendorSpace(const bool vendor_space) { vendor_space_ = vendor_space; } @@ -80,10 +85,13 @@ public: /// @brief Checks that the provided option space name is valid. /// /// It is expected that option space name consists of upper or - /// lower case letters or digits. All other characters are - /// prohibited. + /// lower case letters or digits. Also, it may contain underscores + /// or dashes. Other characters are prohibited. The empty option + /// space names are invalid. /// /// @param name option space name to be validated. + /// + /// @return true if the option space is valid, else it returns false. static bool validateName(const std::string& name); private: diff --git a/src/lib/dhcpsrv/tests/option_space_unittest.cc b/src/lib/dhcpsrv/tests/option_space_unittest.cc index 0c520e0090..92e188dc36 100644 --- a/src/lib/dhcpsrv/tests/option_space_unittest.cc +++ b/src/lib/dhcpsrv/tests/option_space_unittest.cc @@ -83,6 +83,9 @@ TEST(OptionSpaceTest, validateName) { '\\', '|', '<','>', ',', '.', '?', '~', '`' }; for (int i = 0; i < sizeof(specials); ++i) { std::ostringstream stream; + // Concatenate valid option space name: "abc" with an invalid character. + // That way we get option space names like: "abc!", "abc$" etc. It is + // expected that the validating function fails form them. stream << "abc" << specials[i]; EXPECT_FALSE(OptionSpace::validateName(stream.str())) << "Test failed for special character '" << specials[i] << "'."; From a146bf1575760b55554d3d77520ea0900b9a4d51 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 2 Jan 2013 19:58:20 +0100 Subject: [PATCH 5/8] [2313] Added OptionSpace6 class that holds enterprise numbers. --- src/lib/dhcpsrv/option_space.cc | 17 ++++++ src/lib/dhcpsrv/option_space.h | 57 +++++++++++++++++-- .../dhcpsrv/tests/option_space_unittest.cc | 50 +++++++++++++++- 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc index 671245ef95..b5a451814d 100644 --- a/src/lib/dhcpsrv/option_space.cc +++ b/src/lib/dhcpsrv/option_space.cc @@ -42,5 +42,22 @@ OptionSpace::validateName(const std::string& name) { return (false); } +OptionSpace6::OptionSpace6(const std::string& name) + : OptionSpace(name), + enterprise_number_(0) { +} + +OptionSpace6::OptionSpace6(const std::string& name, + const uint32_t enterprise_id) + : OptionSpace(name, true), + enterprise_number_(enterprise_id) { +} + +void +OptionSpace6::setVendorSpace(const uint32_t enterprise_id) { + enterprise_number_ = enterprise_id; + OptionSpace::setVendorSpace(); +} + } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h index c722f4d44a..adc2a9f2b0 100644 --- a/src/lib/dhcpsrv/option_space.h +++ b/src/lib/dhcpsrv/option_space.h @@ -18,6 +18,7 @@ #include #include #include +#include #include namespace isc { @@ -63,6 +64,11 @@ public: /// correct. OptionSpace(const std::string& name, const bool vendor_space = false); + /// @brief Mark option space as non-vendor space. + void clearVendorSpace() { + vendor_space_ = false; + } + /// @brief Return option space name. /// /// @return option space name. @@ -74,12 +80,9 @@ public: /// the vendor space. bool isVendorSpace() const { return (vendor_space_); } - /// @brief Mark option space as vendor space or non-vendor space. - /// - /// @param vendor_space a boolean value indicating that this option - /// space is a vendor space (true) or non-vendor space (false). - void setVendorSpace(const bool vendor_space) { - vendor_space_ = vendor_space; + /// @brief Mark option space as vendor space. + void setVendorSpace() { + vendor_space_ = true; } /// @brief Checks that the provided option space name is valid. @@ -101,6 +104,48 @@ private: }; +/// @brief DHCPv6 option space. +/// +/// This class extends the base class with support for enterprise numbers. +/// The enterprise numbers are assigned by IANA to various organizations +/// and they are carried as uint32_t integers in DHCPv6 Vendor Specific +/// Information Options (VSIO). For more information refer to RFC3315. +/// All option spaces that group VSIO options must have enterprise number +/// set. It can be set using a constructor or \ref setVendorSpace function. +class OptionSpace6 : public OptionSpace { +public: + + /// @brief Constructor for non-vendor-specific options. + /// + /// This constructor marks option space as non-vendor specific. + /// + /// @param name option space name. + OptionSpace6(const std::string& name); + + /// @brief Constructor for vendor-specific options. + /// + /// This constructor marks option space as vendor specific and sets + /// enterprise number to a given value. + /// + /// @param name option space name. + /// @param enterprise_number enterprise number. + OptionSpace6(const std::string& name, const uint32_t enterprise_number); + + /// @brief Return enterprise number for the option space. + /// + /// @return enterprise number. + uint32_t getEnterpriseNumber() const { return (enterprise_number_); } + + /// @brief Mark option space as VSIO option space. + /// + /// @param enterprise_number enterprise number. + void setVendorSpace(const uint32_t enterprise_number); + +private: + + uint32_t enterprise_number_; ///< IANA assigned enterprise number. +}; + } // namespace isc::dhcp } // namespace isc diff --git a/src/lib/dhcpsrv/tests/option_space_unittest.cc b/src/lib/dhcpsrv/tests/option_space_unittest.cc index 92e188dc36..6c0653a788 100644 --- a/src/lib/dhcpsrv/tests/option_space_unittest.cc +++ b/src/lib/dhcpsrv/tests/option_space_unittest.cc @@ -50,7 +50,7 @@ TEST(OptionSpaceTest, setVendorSpace) { EXPECT_TRUE(space.isVendorSpace()); // Override the vendor space flag. - space.setVendorSpace(false); + space.clearVendorSpace(); EXPECT_FALSE(space.isVendorSpace()); } @@ -92,4 +92,52 @@ TEST(OptionSpaceTest, validateName) { } } +// The purpose of this test is to verify that the constructors of the +// OptionSpace6 class set the class members to correct values. +TEST(OptionSpace6Test, constructor) { + // Create some option space and do not specify enterprise number. + // In such case the vendor space flag is expected to be + // set to false. + OptionSpace6 space1("abcd"); + EXPECT_EQ("abcd", space1.getName()); + EXPECT_FALSE(space1.isVendorSpace()); + + // Create an option space and specify an enterprise number. In this + // case the vendor space flag is expected to be set to true and the + // enterprise number should be set to a desired value. + OptionSpace6 space2("abcd", 2145); + EXPECT_EQ("abcd", space2.getName()); + EXPECT_TRUE(space2.isVendorSpace()); + EXPECT_EQ(2145, space2.getEnterpriseNumber()); + + // Verify that constructors throw an exception when invalid option + // space name has been specified. + EXPECT_THROW(OptionSpace6("isc dhcp"), InvalidOptionSpace); + EXPECT_THROW(OptionSpace6("isc%dhcp", 2145), InvalidOptionSpace); +} + +// The purpose of this test is to verify an option space can be marked +// vendor option space and enterprise number can be set. +TEST(OptionSpace6Test, setVendorSpace) { + OptionSpace6 space("isc"); + EXPECT_EQ("isc", space.getName()); + EXPECT_FALSE(space.isVendorSpace()); + + // Mark it vendor option space and set enterprise id. + space.setVendorSpace(1234); + EXPECT_TRUE(space.isVendorSpace()); + EXPECT_EQ(1234, space.getEnterpriseNumber()); + + // Override the enterprise number to make sure and make sure that + // the new number is returned by the object. + space.setVendorSpace(2345); + EXPECT_TRUE(space.isVendorSpace()); + EXPECT_EQ(2345, space.getEnterpriseNumber()); + + // Clear the vendor option space flag. + space.clearVendorSpace(); + EXPECT_FALSE(space.isVendorSpace()); +} + + }; // end of anonymous namespace From 3e100854e0a85d215d614729f6d429340be1ead2 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 2 Jan 2013 21:01:41 +0100 Subject: [PATCH 6/8] [2313] Improved doxygen documentation for OptionSpaceX classes. --- src/lib/dhcpsrv/option_space.cc | 8 +++--- src/lib/dhcpsrv/option_space.h | 49 +++++++++++++++++++++++---------- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc index b5a451814d..88f05bed55 100644 --- a/src/lib/dhcpsrv/option_space.cc +++ b/src/lib/dhcpsrv/option_space.cc @@ -48,14 +48,14 @@ OptionSpace6::OptionSpace6(const std::string& name) } OptionSpace6::OptionSpace6(const std::string& name, - const uint32_t enterprise_id) + const uint32_t enterprise_number) : OptionSpace(name, true), - enterprise_number_(enterprise_id) { + enterprise_number_(enterprise_number) { } void -OptionSpace6::setVendorSpace(const uint32_t enterprise_id) { - enterprise_number_ = enterprise_id; +OptionSpace6::setVendorSpace(const uint32_t enterprise_number) { + enterprise_number_ = enterprise_number; OptionSpace::setVendorSpace(); } diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h index adc2a9f2b0..0a07f27013 100644 --- a/src/lib/dhcpsrv/option_space.h +++ b/src/lib/dhcpsrv/option_space.h @@ -39,16 +39,20 @@ typedef boost::shared_ptr OptionSpacePtr; /// A collection of option spaces. typedef std::map OptionSpaceCollection; -/// @brief Option code space. +/// @brief DHCP option space. /// /// This class represents single option space. The option spaces are used /// to group DHCP options having unique option codes. The special type -/// of the option space is so called "vendor option space". It groups -/// sub-options being sent within Vendor Encapsulated Options. These are -/// option with code 43 for DHCPv4 and option with code 17 for DHCPv6. -/// The option spaces are assigned to option instances represented by -/// isc::dhcp::Option and other classes derived from it. Each particular -/// option may belong to multiple option spaces. +/// of the option space is so called "vendor specific option space". +/// It groups sub-options being sent within Vendor Encapsulated Options. +/// For DHCPv4 it is the option with code 43. The option spaces are +/// assigned to option instances represented by isc::dhcp::Option and +/// other classes derived from it. Each particular option may belong to +/// multiple option spaces. +/// This class may be used to represent any DHCPv4 option space. In theory +/// this class can be also used to represent non-vendor specific DHCPv6 +/// option space but this is disencouraged. For DHCPv6 option spaces the +/// OptionSpace6 class should be used instead. class OptionSpace { public: @@ -56,7 +60,7 @@ public: /// /// @param name option space name. /// @param vendor_space boolean value that indicates that the object - /// describes the vendor space. + /// describes the vendor specific option space. /// /// @throw isc::dhcp::InvalidOptionSpace if given option space name /// contains invalid characters or is empty. This constructor uses @@ -74,13 +78,13 @@ public: /// @return option space name. const std::string& getName() const { return (name_); } - /// @brief Check if option space is a vendor space. + /// @brief Check if option space is vendor specific. /// /// @return boolean value that indicates if the object describes - /// the vendor space. + /// the vendor specific option space. bool isVendorSpace() const { return (vendor_space_); } - /// @brief Mark option space as vendor space. + /// @brief Mark option space as vendor specific. void setVendorSpace() { vendor_space_ = true; } @@ -104,14 +108,21 @@ private: }; -/// @brief DHCPv6 option space. +/// @brief DHCPv6 option space with enterprise number assigned. /// -/// This class extends the base class with support for enterprise numbers. +/// This class extends the base class with the support for enterprise numbers. /// The enterprise numbers are assigned by IANA to various organizations /// and they are carried as uint32_t integers in DHCPv6 Vendor Specific /// Information Options (VSIO). For more information refer to RFC3315. /// All option spaces that group VSIO options must have enterprise number /// set. It can be set using a constructor or \ref setVendorSpace function. +/// The extra functionality of this class (enterprise numbers) allows to +/// represent DHCPv6 vendor-specific option spaces but this class is also +/// intended to be used for all other DHCPv6 option spaces. That way all +/// DHCPv6 option spaces can be stored in the container holding OptionSpace6 +/// objects. Also, it is easy to mark vendor-specific option space as non-vendor +/// specific option space (and the other way around) without a need to cast +/// between OptionSpace and OptionSpace6 types. class OptionSpace6 : public OptionSpace { public: @@ -120,6 +131,11 @@ public: /// This constructor marks option space as non-vendor specific. /// /// @param name option space name. + /// + /// @throw isc::dhcp::InvalidOptionSpace if given option space name + /// contains invalid characters or is empty. This constructor uses + /// \ref OptionSpace::validateName function to check that the specified + /// name is correct. OptionSpace6(const std::string& name); /// @brief Constructor for vendor-specific options. @@ -129,6 +145,11 @@ public: /// /// @param name option space name. /// @param enterprise_number enterprise number. + /// + /// @throw isc::dhcp::InvalidOptionSpace if given option space name + /// contains invalid characters or is empty. This constructor uses + /// \ref OptionSpace::validateName function to check that the specified + /// name is correct. OptionSpace6(const std::string& name, const uint32_t enterprise_number); /// @brief Return enterprise number for the option space. @@ -136,7 +157,7 @@ public: /// @return enterprise number. uint32_t getEnterpriseNumber() const { return (enterprise_number_); } - /// @brief Mark option space as VSIO option space. + /// @brief Mark option space as vendor specific. /// /// @param enterprise_number enterprise number. void setVendorSpace(const uint32_t enterprise_number); From f8ad67f7166f71b5f78721cc2d7df85c2a859bbc Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 8 Jan 2013 15:44:03 +0100 Subject: [PATCH 7/8] [2313] Changes as a result of the review. --- src/lib/dhcpsrv/option_space.cc | 20 +++++++---- src/lib/dhcpsrv/option_space.h | 34 ++++++++++++++----- .../dhcpsrv/tests/option_space_unittest.cc | 7 ++++ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc index 88f05bed55..c244268ca0 100644 --- a/src/lib/dhcpsrv/option_space.cc +++ b/src/lib/dhcpsrv/option_space.cc @@ -30,14 +30,22 @@ OptionSpace::OptionSpace(const std::string& name, const bool vendor_space) bool OptionSpace::validateName(const std::string& name) { - // Allowed digits are: lower or upper case letters, digits, + + using namespace boost::algorithm; + + // Allowed characters are: lower or upper case letters, digits, // underscores and dashes. Empty option spaces are not allowed. - if (boost::algorithm::all(name, boost::is_from_range('a', 'z') || - boost::is_from_range('A', 'Z') || - boost::is_digit() || - boost::is_any_of("-_")) && - !name.empty()) { + if (all(name, boost::is_from_range('a', 'z') || + boost::is_from_range('A', 'Z') || + boost::is_digit() || + boost::is_any_of("-_")) && + !name.empty() && + // Hyphens are not allowed at the beginning and at + // the end of the option space name. + !all(find_head(name, 1), boost::is_any_of("-_")) && + !all(find_tail(name, 1), boost::is_any_of("-_"))) { return (true); + } return (false); } diff --git a/src/lib/dhcpsrv/option_space.h b/src/lib/dhcpsrv/option_space.h index 0a07f27013..9eebd76df9 100644 --- a/src/lib/dhcpsrv/option_space.h +++ b/src/lib/dhcpsrv/option_space.h @@ -49,10 +49,26 @@ typedef std::map OptionSpaceCollection; /// assigned to option instances represented by isc::dhcp::Option and /// other classes derived from it. Each particular option may belong to /// multiple option spaces. -/// This class may be used to represent any DHCPv4 option space. In theory -/// this class can be also used to represent non-vendor specific DHCPv6 -/// option space but this is disencouraged. For DHCPv6 option spaces the -/// OptionSpace6 class should be used instead. +/// This class may be used to represent any DHCPv4 option space. If the +/// option space is to group DHCPv4 Vendor Encapsulated Options then +/// "vendor space" flag must be set using \ref OptionSpace::setVendorSpace +/// or the argument passed to the constructor. In theory, this class can +/// be also used to represent non-vendor specific DHCPv6 option space +/// but this is discouraged. For DHCPv6 option spaces the OptionSpace6 +/// class should be used instead. +/// +/// @note this class is intended to be used to represent DHCPv4 option +/// spaces only. However, it hasn't been called OptionSpace4 (that would +/// suggest that it is specific to DHCPv4) because it can be also +/// used to represent some DHCPv6 option spaces and is a base class +/// for \ref OptionSpace6. Thus, if one declared the container as follows: +/// @code +/// std::vector container; +/// @endcode +/// it would suggest that the container holds DHCPv4 option spaces while +/// it could hold both DHCPv4 and DHCPv6 option spaces as the OptionSpace6 +/// object could be upcast to OptionSpace4. This confusion does not appear +/// when OptionSpace is used as a name for the base class. class OptionSpace { public: @@ -68,16 +84,16 @@ public: /// correct. OptionSpace(const std::string& name, const bool vendor_space = false); - /// @brief Mark option space as non-vendor space. - void clearVendorSpace() { - vendor_space_ = false; - } - /// @brief Return option space name. /// /// @return option space name. const std::string& getName() const { return (name_); } + /// @brief Mark option space as non-vendor space. + void clearVendorSpace() { + vendor_space_ = false; + } + /// @brief Check if option space is vendor specific. /// /// @return boolean value that indicates if the object describes diff --git a/src/lib/dhcpsrv/tests/option_space_unittest.cc b/src/lib/dhcpsrv/tests/option_space_unittest.cc index 6c0653a788..9ee6fcaaf5 100644 --- a/src/lib/dhcpsrv/tests/option_space_unittest.cc +++ b/src/lib/dhcpsrv/tests/option_space_unittest.cc @@ -77,6 +77,13 @@ TEST(OptionSpaceTest, validateName) { EXPECT_FALSE(OptionSpace::validateName(" isc")); EXPECT_FALSE(OptionSpace::validateName("isc with-space")); + // Hyphens are not allowed at the beginning and at the end + // of the option space name. + EXPECT_FALSE(OptionSpace::validateName("-isc")); + EXPECT_FALSE(OptionSpace::validateName("isc-")); + EXPECT_FALSE(OptionSpace::validateName("_isc")); + EXPECT_FALSE(OptionSpace::validateName("isc_")); + // Test other special characters const char specials[] = { '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '+', '=', '[', ']', '{', '}', ';', ':', '"', '\'', From b3a9b362643eddb59ef78ff392cf091d0c199163 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 8 Jan 2013 17:26:42 +0100 Subject: [PATCH 8/8] [2313] Trivial: updates to some comments as a result of the second review. --- src/lib/dhcpsrv/option_space.cc | 6 +++--- src/lib/dhcpsrv/tests/option_space_unittest.cc | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/dhcpsrv/option_space.cc b/src/lib/dhcpsrv/option_space.cc index c244268ca0..6036e542e6 100644 --- a/src/lib/dhcpsrv/option_space.cc +++ b/src/lib/dhcpsrv/option_space.cc @@ -34,14 +34,14 @@ OptionSpace::validateName(const std::string& name) { using namespace boost::algorithm; // Allowed characters are: lower or upper case letters, digits, - // underscores and dashes. Empty option spaces are not allowed. + // underscores and hyphens. Empty option spaces are not allowed. if (all(name, boost::is_from_range('a', 'z') || boost::is_from_range('A', 'Z') || boost::is_digit() || boost::is_any_of("-_")) && !name.empty() && - // Hyphens are not allowed at the beginning and at - // the end of the option space name. + // Hyphens and underscores are not allowed at the beginning + // and at the end of the option space name. !all(find_head(name, 1), boost::is_any_of("-_")) && !all(find_tail(name, 1), boost::is_any_of("-_"))) { return (true); diff --git a/src/lib/dhcpsrv/tests/option_space_unittest.cc b/src/lib/dhcpsrv/tests/option_space_unittest.cc index 9ee6fcaaf5..f8d75c8f47 100644 --- a/src/lib/dhcpsrv/tests/option_space_unittest.cc +++ b/src/lib/dhcpsrv/tests/option_space_unittest.cc @@ -77,8 +77,8 @@ TEST(OptionSpaceTest, validateName) { EXPECT_FALSE(OptionSpace::validateName(" isc")); EXPECT_FALSE(OptionSpace::validateName("isc with-space")); - // Hyphens are not allowed at the beginning and at the end - // of the option space name. + // Hyphens and underscores are not allowed at the beginning + // and at the end of the option space name. EXPECT_FALSE(OptionSpace::validateName("-isc")); EXPECT_FALSE(OptionSpace::validateName("isc-")); EXPECT_FALSE(OptionSpace::validateName("_isc"));