diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index ac4b3c69da..6f8cf863e4 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012 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 @@ -12,25 +12,21 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +// We want UINT32_MAX macro to be defined in stdint.h +#define __STDC_LIMIT_MACROS + +#include +#include +#include +#include +#include +#include +#include + #include #include #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include using namespace std; using namespace isc::data; @@ -43,17 +39,11 @@ namespace dhcp { typedef pair ConfigPair; /// @brief a factory method that will create a parser for a given element name -typedef DhcpConfigParser* ParserFactory(const std::string& config_id); +typedef Dhcp4ConfigParser* ParserFactory(const std::string& config_id); /// @brief a collection of factories that creates parsers for specified element names typedef std::map FactoryMap; -/// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900) -typedef std::map Uint32Storage; - -/// @brief a collection of elements that store string values -typedef std::map StringStorage; - /// @brief a collection of pools /// /// That type is used as intermediate storage, when pools are parsed, but there is @@ -72,12 +62,12 @@ StringStorage string_defaults; /// will accept any configuration and will just print it out /// on commit. Useful for debugging existing configurations and /// adding new ones. -class DebugParser : public DhcpConfigParser { +class DebugParser : public Dhcp4ConfigParser { public: /// @brief Constructor /// - /// See \ref DhcpConfigParser class for details. + /// See \ref Dhcp4ConfigParser class for details. /// /// @param param_name name of the parsed parameter DebugParser(const std::string& param_name) @@ -86,7 +76,7 @@ public: /// @brief builds parameter value /// - /// See \ref DhcpConfigParser class for details. + /// See \ref Dhcp4ConfigParser class for details. /// /// @param new_config pointer to the new configuration virtual void build(ConstElementPtr new_config) { @@ -100,7 +90,7 @@ public: /// This is a method required by base class. It pretends to apply the /// configuration, but in fact it only prints the parameter out. /// - /// See \ref DhcpConfigParser class for details. + /// See \ref Dhcp4ConfigParser class for details. virtual void commit() { // Debug message. The whole DebugParser class is used only for parser // debugging, and is not used in production code. It is very convenient @@ -112,11 +102,11 @@ public: /// @brief factory that constructs DebugParser objects /// /// @param param_name name of the parameter to be parsed - static DhcpConfigParser* Factory(const std::string& param_name) { + static Dhcp4ConfigParser* Factory(const std::string& param_name) { return (new DebugParser(param_name)); } -protected: +private: /// name of the parsed parameter std::string param_name_; @@ -131,11 +121,11 @@ protected: /// (uint32_defaults). If used in smaller scopes (e.g. to parse parameters /// in subnet config), it can be pointed to a different storage, using /// setStorage() method. This class follows the parser interface, laid out -/// in its base class, \ref DhcpConfigParser. +/// in its base class, \ref Dhcp4ConfigParser. /// /// For overview of usability of this generic purpose parser, see -/// \ref dhcpv4-config-inherit page. -class Uint32Parser : public DhcpConfigParser { +/// \ref dhcp4-config-inherit page. +class Uint32Parser : public Dhcp4ConfigParser { public: /// @brief constructor for Uint32Parser @@ -150,13 +140,30 @@ public: /// \ref setStorage() for details. /// /// @param value pointer to the content of parsed values + /// @throw BadValue if supplied value could not be base to uint32_t virtual void build(ConstElementPtr value) { + int64_t check; + string x = value->str(); try { - value_ = boost::lexical_cast(value->str()); + check = boost::lexical_cast(x); } catch (const boost::bad_lexical_cast &) { isc_throw(BadValue, "Failed to parse value " << value->str() << " as unsigned 32-bit integer."); } + if (check > UINT32_MAX) { + isc_throw(BadValue, "Value " << value->str() << "is too large" + << " for unsigned 32-bit integer."); + } + if (check < 0) { + isc_throw(BadValue, "Value " << value->str() << "is negative." + << " Only 0 or larger are allowed for unsigned 32-bit integer."); + } + + // value is small enough to fit + value_ = static_cast(check); + + /// @todo: check if there is no such name in the map. Otherwise + /// the insert will fail silently storage_->insert(pair(param_name_, value_)); } @@ -166,7 +173,7 @@ public: /// is not commited anywhere. Higher level parsers are expected to /// use values stored in the storage, e.g. renew-timer for a given /// subnet is stored in subnet-specific storage. It is not commited - /// here, but is rather used by \ref Subnet4Parser when constructing + /// here, but is rather used by \ref Subnet4ConfigParser when constructing /// the subnet. virtual void commit() { } @@ -174,13 +181,13 @@ public: /// @brief factory that constructs Uint32Parser objects /// /// @param param_name name of the parameter to be parsed - static DhcpConfigParser* Factory(const std::string& param_name) { + static Dhcp4ConfigParser* Factory(const std::string& param_name) { return (new Uint32Parser(param_name)); } /// @brief sets storage for value of this parameter /// - /// See \ref dhcpv4-config-inherit for details. + /// See \ref dhcp4-config-inherit for details. /// /// @param storage pointer to the storage container void setStorage(Uint32Storage* storage) { @@ -205,11 +212,11 @@ protected: /// (string_defaults). If used in smaller scopes (e.g. to parse parameters /// in subnet config), it can be pointed to a different storage, using /// setStorage() method. This class follows the parser interface, laid out -/// in its base class, \ref DhcpConfigParser. +/// in its base class, \ref Dhcp4ConfigParser. /// /// For overview of usability of this generic purpose parser, see -/// \ref dhcpv4-config-inherit page. -class StringParser : public DhcpConfigParser { +/// \ref dhcp4-config-inherit page. +class StringParser : public Dhcp4ConfigParser { public: /// @brief constructor for StringParser @@ -227,6 +234,8 @@ public: virtual void build(ConstElementPtr value) { value_ = value->str(); boost::erase_all(value_, "\""); + /// @todo: check if there is no such name in the map. Otherwise + /// the insert will fail silently storage_->insert(pair(param_name_, value_)); } @@ -244,13 +253,13 @@ public: /// @brief factory that constructs StringParser objects /// /// @param param_name name of the parameter to be parsed - static DhcpConfigParser* Factory(const std::string& param_name) { + static Dhcp4ConfigParser* Factory(const std::string& param_name) { return (new StringParser(param_name)); } /// @brief sets storage for value of this parameter /// - /// See \ref dhcpv4-config-inherit for details. + /// See \ref dhcp4-config-inherit for details. /// /// @param storage pointer to the storage container void setStorage(StringStorage* storage) { @@ -277,7 +286,7 @@ protected: /// designates all interfaces. /// /// It is useful for parsing Dhcp4/interface parameter. -class InterfaceListConfigParser : public DhcpConfigParser { +class InterfaceListConfigParser : public Dhcp4ConfigParser { public: /// @brief constructor @@ -286,17 +295,18 @@ public: /// "interface" parameter only. All other types will throw exception. /// /// @param param_name name of the configuration parameter being parsed + /// @throw BadValue if supplied parameter name is not "interface" InterfaceListConfigParser(const std::string& param_name) { if (param_name != "interface") { - isc_throw(NotImplemented, "Internal error. Interface configuration " + isc_throw(BadValue, "Internal error. Interface configuration " "parser called for the wrong parameter: " << param_name); } } /// @brief parses parameters value /// - /// Parses configuration entry (list of parameters) and stores it in - /// storage. See \ref setStorage() for details. + /// Parses configuration entry (list of parameters) and adds each element + /// to the interfaces list. /// /// @param value pointer to the content of parsed values virtual void build(ConstElementPtr value) { @@ -314,7 +324,7 @@ public: /// @brief factory that constructs InterfaceListConfigParser objects /// /// @param param_name name of the parameter to be parsed - static DhcpConfigParser* Factory(const std::string& param_name) { + static Dhcp4ConfigParser* Factory(const std::string& param_name) { return (new InterfaceListConfigParser(param_name)); } @@ -333,7 +343,7 @@ protected: /// before build(). Otherwise exception will be thrown. /// /// It is useful for parsing Dhcp4/subnet4[X]/pool parameters. -class PoolParser : public DhcpConfigParser { +class PoolParser : public Dhcp4ConfigParser { public: /// @brief constructor. @@ -347,10 +357,13 @@ public: /// This method parses the actual list of interfaces. /// No validation is done at this stage, everything is interpreted as /// interface name. + /// @param pools_list list of pools defined for a subnet + /// @throw InvalidOperation if storage was not specified (setStorage() not called) + /// @throw Dhcp4ConfigError when pool parsing fails void build(ConstElementPtr pools_list) { // setStorage() should have been called before build if (!pools_) { - isc_throw(NotImplemented, "Parser logic error. No pool storage set," + isc_throw(InvalidOperation, "Parser logic error. No pool storage set," " but pool parser asked to parse pools"); } @@ -416,7 +429,7 @@ public: /// @brief sets storage for value of this parameter /// - /// See \ref dhcpv4-config-inherit for details. + /// See \ref dhcp4-config-inherit for details. /// /// @param storage pointer to the storage container void setStorage(PoolStorage* storage) { @@ -433,7 +446,7 @@ public: /// @brief factory that constructs PoolParser objects /// /// @param param_name name of the parameter to be parsed - static DhcpConfigParser* Factory(const std::string& param_name) { + static Dhcp4ConfigParser* Factory(const std::string& param_name) { return (new PoolParser(param_name)); } @@ -449,7 +462,7 @@ protected: /// /// This class parses the whole subnet definition. It creates parsers /// for received configuration parameters as needed. -class Subnet4ConfigParser : public DhcpConfigParser { +class Subnet4ConfigParser : public Dhcp4ConfigParser { public: /// @brief constructor @@ -469,22 +482,22 @@ public: // if this is an Uint32 parser, tell it to store the values // in values_, rather than in global storage - boost::shared_ptr uintParser = + boost::shared_ptr uint_parser = boost::dynamic_pointer_cast(parser); - if (uintParser) { - uintParser->setStorage(&uint32_values_); + if (uint_parser) { + uint_parser->setStorage(&uint32_values_); } else { - boost::shared_ptr stringParser = + boost::shared_ptr string_parser = boost::dynamic_pointer_cast(parser); - if (stringParser) { - stringParser->setStorage(&string_values_); + if (string_parser) { + string_parser->setStorage(&string_values_); } else { - boost::shared_ptr poolParser = + boost::shared_ptr pool_parser = boost::dynamic_pointer_cast(parser); - if (poolParser) { - poolParser->setStorage(&pools_); + if (pool_parser) { + pool_parser->setStorage(&pools_); } } } @@ -502,6 +515,7 @@ public: /// storing the values that are actually consumed here. Pool definitions /// created in other parsers are used here and added to newly created Subnet4 /// objects. Subnet4 are then added to DHCP CfgMgr. + /// @throw Dhcp4ConfigError if there are any issues encountered during commit void commit() { StringStorage::const_iterator it = string_values_.find("subnet"); @@ -549,7 +563,8 @@ protected: /// /// @param config_id name od the entry /// @return parser object for specified entry name - DhcpConfigParser* createSubnet4ConfigParser(const std::string& config_id) { + /// @throw NotImplemented if trying to create a parser for unknown config element + Dhcp4ConfigParser* createSubnet4ConfigParser(const std::string& config_id) { FactoryMap factories; factories.insert(pair( @@ -585,6 +600,7 @@ protected: /// /// @param name name of the parameter /// @return triplet with the parameter name + /// @throw Dhcp4ConfigError when requested parameter is not present Triplet getParam(const std::string& name) { uint32_t value = 0; bool found = false; @@ -627,7 +643,7 @@ protected: /// This is a wrapper parser that handles the whole list of Subnet4 /// definitions. It iterates over all entries and creates Subnet4ConfigParser /// for each entry. -class Subnets4ListConfigParser : public DhcpConfigParser { +class Subnets4ListConfigParser : public Dhcp4ConfigParser { public: /// @brief constructor @@ -676,7 +692,7 @@ public: /// @brief Returns Subnet4ListConfigParser object /// @param param_name name of the parameter /// @return Subnets4ListConfigParser object - static DhcpConfigParser* Factory(const std::string& param_name) { + static Dhcp4ConfigParser* Factory(const std::string& param_name) { return (new Subnets4ListConfigParser(param_name)); } @@ -691,7 +707,8 @@ public: /// /// @param config_id pointer to received global configuration entry /// @return parser for specified global DHCPv4 parameter -DhcpConfigParser* createGlobalDhcpConfigParser(const std::string& config_id) { +/// @throw NotImplemented if trying to create a parser for unknown config element +Dhcp4ConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) { FactoryMap factories; factories.insert(pair( @@ -723,25 +740,18 @@ DhcpConfigParser* createGlobalDhcpConfigParser(const std::string& config_id) { /// @brief configures DHCPv4 server /// -/// This function is called every time a new configuration is received. The extra -/// parameter is a reference to DHCPv4 server component. It is currently not used -/// and CfgMgr::instance() is accessed instead. -/// -/// This method does not throw. It catches all exceptions and returns them as -/// reconfiguration statuses. It may return the following response codes: -/// 0 - configuration successful -/// 1 - malformed configuration (parsing failed) -/// 2 - logical error (parsing was successful, but the values are invalid) -/// -/// @param config_set a new configuration for DHCPv4 server -/// @return answer that contains result of reconfiguration isc::data::ConstElementPtr configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) { if (!config_set) { - isc_throw(Dhcp4ConfigError, - "Null pointer is passed to configuration parser"); + ConstElementPtr answer = isc::config::createAnswer(1, + string("Can't parse NULL config")); + return (answer); } + /// Let's wipe previous configuration defaults + uint32_defaults.clear(); + string_defaults.clear(); + /// @todo: append most essential info here (like "2 new subnets configured") string config_details; @@ -751,7 +761,7 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) { try { BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) { - ParserPtr parser(createGlobalDhcpConfigParser(config_pair.first)); + ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first)); parser->build(config_pair.second); parsers.push_back(parser); } diff --git a/src/bin/dhcp4/config_parser.h b/src/bin/dhcp4/config_parser.h index efead3475b..2ee9cf6bb2 100644 --- a/src/bin/dhcp4/config_parser.h +++ b/src/bin/dhcp4/config_parser.h @@ -12,9 +12,9 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include #include #include +#include #ifndef DHCP4_CONFIG_PARSER_H #define DHCP4_CONFIG_PARSER_H @@ -27,21 +27,34 @@ namespace dhcp { class Dhcpv4Srv; +/// @brief a collection of elements that store uint32 values (e.g. renew-timer = 900) +typedef std::map Uint32Storage; + +/// @brief a collection of elements that store string values +typedef std::map StringStorage; + /// An exception that is thrown if an error occurs while configuring an /// \c Dhcpv4Srv object. class Dhcp4ConfigError : public isc::Exception { public: -/// @brief constructor -/// -/// @param file name of the file, where exception occurred -/// @param line line of the file, where exception occurred -/// @param what text description of the issue that caused exception -Dhcp4ConfigError(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) {} + /// @brief constructor + /// + /// @param file name of the file, where exception occurred + /// @param line line of the file, where exception occurred + /// @param what text description of the issue that caused exception + Dhcp4ConfigError(const char* file, size_t line, const char* what) + : isc::Exception(file, line, what) {} }; -class DhcpConfigParser { +/// @brief Base abstract class for all DHCPv4 parsers +/// +/// Each instance of a class derived from this class parses one specific config +/// element. Sometimes elements are simple (e.g. a string) and sometimes quite +/// complex (e.g. a subnet). In such case, it is likely that a parser will +/// spawn child parsers to parse child elements in the configuration. +/// @todo: Merge this class with DhcpConfigParser in src/bin/dhcp6 +class Dhcp4ConfigParser { /// /// \name Constructors and Destructor /// @@ -50,17 +63,21 @@ class DhcpConfigParser { /// pure base class. //@{ private: - DhcpConfigParser(const DhcpConfigParser& source); - DhcpConfigParser& operator=(const DhcpConfigParser& source); + + // Private construtor and assignment operator assures that nobody + // will be able to copy or assign a parser. There are no defined + // bodies for them. + Dhcp4ConfigParser(const Dhcp4ConfigParser& source); + Dhcp4ConfigParser& operator=(const Dhcp4ConfigParser& source); protected: /// \brief The default constructor. /// /// This is intentionally defined as \c protected as this base class should /// never be instantiated (except as part of a derived class). - DhcpConfigParser() {} + Dhcp4ConfigParser() {} public: /// The destructor. - virtual ~DhcpConfigParser() {} + virtual ~Dhcp4ConfigParser() {} //@} /// \brief Prepare configuration value. @@ -107,7 +124,7 @@ public: }; /// @brief a pointer to configuration parser -typedef boost::shared_ptr ParserPtr; +typedef boost::shared_ptr ParserPtr; /// @brief a collection of parsers /// @@ -115,7 +132,7 @@ typedef boost::shared_ptr ParserPtr; typedef std::vector ParserCollection; -/// \brief Configure an \c Dhcpv4Srv object with a set of configuration values. +/// \brief Configure DHCPv4 server (\c Dhcpv4Srv) with a set of configuration values. /// /// This function parses configuration information stored in \c config_set /// and configures the \c server by applying the configuration to it. @@ -126,22 +143,22 @@ typedef std::vector ParserCollection; /// /// If a syntax or semantics level error happens during the configuration /// (such as malformed configuration or invalid configuration parameter), -/// this function throws an exception of class \c Dhcp4ConfigError. -/// If the given configuration requires resource allocation and it fails, -/// a corresponding standard exception will be thrown. -/// Other exceptions may also be thrown, depending on the implementation of -/// the underlying derived class of \c Dhcp4ConfigError. -/// In any case the strong guarantee is provided as described above except -/// in the very rare cases where the \c commit() method of a parser throws -/// an exception. If that happens this function converts the exception -/// into a \c FatalError exception and rethrows it. This exception is -/// expected to be caught at the highest level of the application to terminate -/// the program gracefully. +/// this function returns appropriate error code. /// -/// \param server The \c Dhcpv4Srv object to be configured. -/// \param config_set A JSON style configuration to apply to \c server. +/// This function is called every time a new configuration is received. The extra +/// parameter is a reference to DHCPv4 server component. It is currently not used +/// and CfgMgr::instance() is accessed instead. +/// +/// This method does not throw. It catches all exceptions and returns them as +/// reconfiguration statuses. It may return the following response codes: +/// 0 - configuration successful +/// 1 - malformed configuration (parsing failed) +/// 2 - logical error (parsing was successful, but the values are invalid) +/// +/// @param config_set a new configuration (JSON) for DHCPv4 server +/// @return answer that contains result of reconfiguration isc::data::ConstElementPtr -configureDhcp4Server(Dhcpv4Srv& server, +configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set); }; // end of isc::dhcp namespace diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 01067fa7ca..c9366af833 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -14,9 +14,6 @@ #include -#include -#include - #include #include #include @@ -29,6 +26,8 @@ #include #include #include +#include +#include using namespace isc::asiolink; using namespace isc::cc; @@ -126,7 +125,7 @@ void ControlledDhcpv4Srv::establishSession() { /// Integrate the asynchronous I/O model of BIND 10 configuration /// control with the "select" model of the DHCP server. This is - /// fully explained in \ref dhcpv4Session. + /// fully explained in \ref dhcp4-session. int ctrl_socket = cc_session_->getSocketDesc(); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_CCSESSION_STARTED) .arg(ctrl_socket); diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 688b1db521..76c5d6d1be 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -34,7 +34,7 @@ namespace dhcp { /// embedded environments. /// /// For detailed explanation or relations between main(), ControlledDhcpv4Srv, -/// Dhcpv4Srv and other classes, see \ref dhcpv4Session. +/// Dhcpv4Srv and other classes, see \ref dhcp4-session. class ControlledDhcpv4Srv : public isc::dhcp::Dhcpv4Srv { public: @@ -66,7 +66,7 @@ public: /// @brief Session callback, processes received commands. /// - /// @param command_id text represenation of the command (e.g. "shutdown") + /// @param command text represenation of the command (e.g. "shutdown") /// @param args optional parameters /// /// @return status of the command diff --git a/src/bin/dhcp4/dhcp4.dox b/src/bin/dhcp4/dhcp4.dox index 5dacf63ef6..54f2bd36e8 100644 --- a/src/bin/dhcp4/dhcp4.dox +++ b/src/bin/dhcp4/dhcp4.dox @@ -15,8 +15,6 @@ DHCPv4 server component does not support direct traffic (relayed only), as support for transmission to hosts without IPv4 address assigned is not implemented in IfaceMgr yet. -DHCPv4 server component does not use BIND10 logging yet. - @section dhcp4-session BIND10 message queue integration DHCPv4 server component is now integrated with BIND10 message queue. diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 67943c5e56..aa79ce9cb4 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -17,8 +17,8 @@ #include #include #include -#include #include +#include using namespace isc; using namespace isc::asiolink; diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index f677259947..10470e41b6 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -15,10 +15,10 @@ #ifndef DHCPV4_SRV_H #define DHCPV4_SRV_H -#include #include #include #include +#include #include namespace isc { @@ -34,11 +34,11 @@ namespace dhcp { /// appropriate responses. /// /// This class does not support any controlling mechanisms directly. -/// See derived \ref ControlledDhcv4Srv class for support for +/// See derived \ref ControlledDhcpv4Srv class for support for /// command and configuration updates over msgq. /// /// For detailed explanation or relations between main(), ControlledDhcpv4Srv, -/// Dhcpv4Srv and other classes, see \ref dhcpv4Session. +/// Dhcpv4Srv and other classes, see \ref dhcp4-session. class Dhcpv4Srv : public boost::noncopyable { public: diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 8a8873f0bc..ea7048b430 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -13,9 +13,6 @@ // PERFORMANCE OF THIS SOFTWARE. #include -#include -#include -#include #include #include @@ -25,6 +22,10 @@ #include #include #include +#include +#include +#include +#include using namespace std; using namespace isc; @@ -33,6 +34,12 @@ using namespace isc::asiolink; using namespace isc::data; using namespace isc::config; +namespace isc { +namespace dhcp { +extern Uint32Storage uint32_defaults; +} +} + namespace { class Dhcp4ParserTest : public ::testing::Test { @@ -45,6 +52,26 @@ public: srv_ = new Dhcpv4Srv(0); } + // Checks if global parameter of name have expected_value + void checkGlobalUint32(string name, uint32_t expected_value) { + Uint32Storage::const_iterator it = uint32_defaults.find(name); + if (it == uint32_defaults.end()) { + ADD_FAILURE() << "Expected uint32 with name " << name + << " not found"; + return; + } + EXPECT_EQ(expected_value, it->second); + } + + // Checks if config_result (result of DHCP server configuration) has + // expected code (0 for success, other for failures). + // Also stores result in rcode_ and comment_. + void checkResult(ConstElementPtr status, int expected_code) { + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + EXPECT_EQ(expected_code, rcode_); + } + ~Dhcp4ParserTest() { delete srv_; }; @@ -66,9 +93,7 @@ TEST_F(Dhcp4ParserTest, version) { Element::fromJSON("{\"version\": 0}"))); // returned value must be 0 (configuration accepted) - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + checkResult(x, 0); } /// The goal of this test is to verify that the code accepts only @@ -81,15 +106,13 @@ TEST_F(Dhcp4ParserTest, bogus_command) { Element::fromJSON("{\"bogus\": 5}"))); // returned value must be 1 (configuration parse error) - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(1, rcode_); + checkResult(x, 1); } /// The goal of this test is to verify if wrongly defined subnet will /// be rejected. Properly defined subnet must include at least one /// pool definition. -TEST_F(Dhcp4ParserTest, empty_subnet) { +TEST_F(Dhcp4ParserTest, emptySubnet) { ConstElementPtr status; @@ -101,9 +124,11 @@ TEST_F(Dhcp4ParserTest, empty_subnet) { "\"valid-lifetime\": 4000 }"))); // returned value should be 0 (success) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); + + checkGlobalUint32("rebind-timer", 2000); + checkGlobalUint32("renew-timer", 1000); + checkGlobalUint32("valid-lifetime", 4000); } /// The goal of this test is to verify if defined subnet uses global @@ -126,9 +151,7 @@ TEST_F(Dhcp4ParserTest, subnet_global_defaults) { EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // check if returned status is OK - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); // Now check if the configuration was indeed handled and we have // expected pool configured. @@ -141,7 +164,7 @@ TEST_F(Dhcp4ParserTest, subnet_global_defaults) { // This test checks if it is possible to override global values // on a per subnet basis. -TEST_F(Dhcp4ParserTest, subnet_local) { +TEST_F(Dhcp4ParserTest, subnetLocal) { ConstElementPtr status; @@ -162,9 +185,7 @@ TEST_F(Dhcp4ParserTest, subnet_local) { EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); // returned value should be 0 (configuration success) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200")); ASSERT_TRUE(subnet); @@ -175,7 +196,7 @@ TEST_F(Dhcp4ParserTest, subnet_local) { // Test verifies that a subnet with pool values that do not belong to that // pool are rejected. -TEST_F(Dhcp4ParserTest, pool_out_of_subnet) { +TEST_F(Dhcp4ParserTest, poolOutOfSubnet) { ConstElementPtr status; @@ -194,17 +215,15 @@ TEST_F(Dhcp4ParserTest, pool_out_of_subnet) { // returned value must be 2 (values error) // as the pool does not belong to that subnet - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(2, rcode_); + checkResult(status, 2); } // Goal of this test is to verify if pools can be defined // using prefix/length notation. There is no separate test for min-max // notation as it was tested in several previous tests. -TEST_F(Dhcp4ParserTest, pool_prefix_len) { +TEST_F(Dhcp4ParserTest, poolPrefixLen) { - ConstElementPtr x; + ConstElementPtr status; string config = "{ \"interface\": [ \"all\" ]," "\"rebind-timer\": 2000, " @@ -217,12 +236,10 @@ TEST_F(Dhcp4ParserTest, pool_prefix_len) { ElementPtr json = Element::fromJSON(config); - EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); - // returned value must be 1 (configuration parse error) - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + // returned value must be 0 (configuration accepted) + checkResult(status, 0); Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200")); ASSERT_TRUE(subnet); @@ -231,4 +248,47 @@ TEST_F(Dhcp4ParserTest, pool_prefix_len) { EXPECT_EQ(4000, subnet->getValid()); } +/// This test checks if Uint32Parser can really parse the whole range +/// and properly err of out of range values. As we can't call Uint32Parser +/// directly, we are exploiting the fact that it is used to parse global +/// parameter renew-timer and the results are stored in uint32_defaults. +TEST_F(Dhcp4ParserTest, Uint32Parser) { + + ConstElementPtr status; + + // CASE 1: 0 - minimum value, should work + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, + Element::fromJSON("{\"version\": 0," + "\"renew-timer\": 0}"))); + + // returned value must be ok (0 is a proper value) + checkResult(status, 0); + checkGlobalUint32("renew-timer", 0); + + // CASE 2: 4294967295U (UINT_MAX) should work as well + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, + Element::fromJSON("{\"version\": 0," + "\"renew-timer\": 4294967295}"))); + + // returned value must be ok (0 is a proper value) + checkResult(status, 0); + checkGlobalUint32("renew-timer", 4294967295U); + + // CASE 3: 4294967296U (UINT_MAX + 1) should not work + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, + Element::fromJSON("{\"version\": 0," + "\"renew-timer\": 4294967296}"))); + + // returned value must be rejected (1 configuration error) + checkResult(status, 1); + + // CASE 4: -1 (UINT_MIN -1 ) should not work + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, + Element::fromJSON("{\"version\": 0," + "\"renew-timer\": -1}"))); + + // returned value must be rejected (1 configuration error) + checkResult(status, 1); +} + };