BIND 10 master, updated. fed1aab5a0f813c41637807f8c0c5f8830d71942 [master] Merge branch 'trac2544'
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Dec 19 17:52:21 UTC 2012
The branch, master has been updated
via fed1aab5a0f813c41637807f8c0c5f8830d71942 (commit)
via a5aab271f44cce29af4344ae8c24da02167deb05 (commit)
via f64ff7004ddec6c81538b2fc5d6827de6b79cfb8 (commit)
via 667d94d93788e24158ebbec640f006f49b7d438c (commit)
via fdce9eb73133bb52609ed6ad51fdb77b28447eb3 (commit)
via 393c70b7a308f5d816a87cf21e017351dbf09260 (commit)
via c980c37cce2a445ec40f2c526cd89310cf7c4941 (commit)
via 4114ac95b51b5551e38bc277860abf14775cc817 (commit)
via 16bdc3898aa70d2145872a5277f60711c32f2bd9 (commit)
via 16cc994d2f3e15180fc022976062d616b0c05f38 (commit)
via 870ee769dedab453f6fb65082884cd2b45d1ca14 (commit)
via 2ad9f33c18a74df23e3ca10fd5246289093e2768 (commit)
via 079e629bdec110c1b07376645361fbcb2e467043 (commit)
via 165e2775424f0377b5eb8f574d83c0d0dcd67a4b (commit)
from bd8497df6615f58b2a6de701cf2a8f02c4d57d9b (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 fed1aab5a0f813c41637807f8c0c5f8830d71942
Merge: bd8497d a5aab27
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Dec 19 18:28:57 2012 +0100
[master] Merge branch 'trac2544'
Conflicts:
src/bin/dhcp4/dhcp4_messages.mes
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/Makefile.am | 2 +-
src/bin/dhcp4/config_parser.cc | 481 +++++++++++++++++++++++--
src/bin/dhcp4/config_parser.h | 1 +
src/bin/dhcp4/dhcp4.spec | 68 +++-
src/bin/dhcp4/dhcp4_messages.mes | 5 +
src/bin/dhcp4/tests/Makefile.am | 6 +-
src/bin/dhcp4/tests/config_parser_unittest.cc | 476 ++++++++++++++++++++++++
src/bin/dhcp6/Makefile.am | 12 +-
src/bin/dhcp6/config_parser.cc | 39 +-
src/bin/dhcp6/dhcp6_messages.mes | 4 +-
src/bin/dhcp6/tests/Makefile.am | 9 +-
src/bin/dhcp6/tests/config_parser_unittest.cc | 10 +-
12 files changed, 1037 insertions(+), 76 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/Makefile.am b/src/bin/dhcp4/Makefile.am
index c896591..28f08f7 100644
--- a/src/bin/dhcp4/Makefile.am
+++ b/src/bin/dhcp4/Makefile.am
@@ -58,6 +58,7 @@ b10_dhcp4_CXXFLAGS = -Wno-unused-parameter
endif
b10_dhcp4_LDADD = $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
+b10_dhcp4_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
b10_dhcp4_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
b10_dhcp4_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
b10_dhcp4_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
@@ -65,6 +66,5 @@ b10_dhcp4_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
b10_dhcp4_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
b10_dhcp4_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
-
b10_dhcp4dir = $(pkgdatadir)
b10_dhcp4_DATA = dhcp4.spec
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index 08d16f8..76181c2 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -13,9 +13,12 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <config/ccsession.h>
-#include <dhcpsrv/cfgmgr.h>
#include <dhcp4/config_parser.h>
#include <dhcp4/dhcp4_log.h>
+#include <dhcp/libdhcp++.h>
+#include <dhcp/option_definition.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <util/encode/hex.h>
#include <boost/foreach.hpp>
#include <boost/lexical_cast.hpp>
#include <boost/algorithm/string.hpp>
@@ -46,12 +49,20 @@ typedef std::map<std::string, ParserFactory*> FactoryMap;
/// no subnet object created yet to store them.
typedef std::vector<Pool4Ptr> PoolStorage;
+/// @brief Collection of option descriptors. This container allows searching for
+/// options using the option code or persistency flag. This is useful when merging
+/// existing options with newly configured options.
+typedef Subnet::OptionContainer OptionStorage;
+
/// @brief Global uint32 parameters that will be used as defaults.
Uint32Storage uint32_defaults;
/// @brief global string parameters that will be used as defaults.
StringStorage string_defaults;
+/// @brief Global storage for options that will be used as defaults.
+OptionStorage option_defaults;
+
/// @brief a dummy configuration parser
///
/// It is a debugging parser. It does not configure anything,
@@ -451,6 +462,344 @@ private:
PoolStorage* pools_;
};
+/// @brief Parser for option data value.
+///
+/// This parser parses configuration entries that specify value of
+/// a single option. These entries include option name, option code
+/// and data carried by the option. If parsing is successful then an
+/// instance of an option is created and added to the storage provided
+/// by the calling class.
+///
+/// @todo This class parses and validates the option name. However it is
+/// not used anywhere until support for option spaces is implemented
+/// (see tickets #2319, #2314). When option spaces are implemented
+/// there will be a way to reference the particular option using
+/// its type (code) or option name.
+class OptionDataParser : public Dhcp4ConfigParser {
+public:
+
+ /// @brief Constructor.
+ ///
+ /// Class constructor.
+ OptionDataParser(const std::string&)
+ : options_(NULL),
+ // initialize option to NULL ptr
+ option_descriptor_(false) { }
+
+ /// @brief Parses the single option data.
+ ///
+ /// This method parses the data of a single option from the configuration.
+ /// The option data includes option name, option code and data being
+ /// carried by this option. Eventually it creates the instance of the
+ /// option.
+ ///
+ /// @warning setStorage must be called with valid storage pointer prior
+ /// to calling this method.
+ ///
+ /// @param option_data_entries collection of entries that define value
+ /// for a particular option.
+ /// @throw Dhcp4ConfigError if invalid parameter specified in
+ /// the configuration.
+ /// @throw isc::InvalidOperation if failed to set storage prior to
+ /// calling build.
+ /// @throw isc::BadValue if option data storage is invalid.
+ virtual void build(ConstElementPtr option_data_entries) {
+ if (options_ == NULL) {
+ isc_throw(isc::InvalidOperation, "Parser logic error: storage must be set before "
+ "parsing option data.");
+ }
+ BOOST_FOREACH(ConfigPair param, option_data_entries->mapValue()) {
+ ParserPtr parser;
+ if (param.first == "name") {
+ boost::shared_ptr<StringParser>
+ name_parser(dynamic_cast<StringParser*>(StringParser::Factory(param.first)));
+ if (name_parser) {
+ name_parser->setStorage(&string_values_);
+ parser = name_parser;
+ }
+ } else if (param.first == "code") {
+ boost::shared_ptr<Uint32Parser>
+ code_parser(dynamic_cast<Uint32Parser*>(Uint32Parser::Factory(param.first)));
+ if (code_parser) {
+ code_parser->setStorage(&uint32_values_);
+ parser = code_parser;
+ }
+ } else if (param.first == "data") {
+ boost::shared_ptr<StringParser>
+ value_parser(dynamic_cast<StringParser*>(StringParser::Factory(param.first)));
+ if (value_parser) {
+ value_parser->setStorage(&string_values_);
+ parser = value_parser;
+ }
+ } else {
+ isc_throw(Dhcp4ConfigError,
+ "Parser error: option-data parameter not supported: "
+ << param.first);
+ }
+ parser->build(param.second);
+ }
+ // Try to create the option instance.
+ createOption();
+ }
+
+ /// @brief Commits option value.
+ ///
+ /// This function adds a new option to the storage or replaces an existing option
+ /// with the same code.
+ ///
+ /// @throw isc::InvalidOperation if failed to set pointer to storage or failed
+ /// to call build() prior to commit. If that happens data in the storage
+ /// remain un-modified.
+ virtual void commit() {
+ if (options_ == NULL) {
+ isc_throw(isc::InvalidOperation, "Parser logic error: storage must be set before "
+ "commiting option data.");
+ } else if (!option_descriptor_.option) {
+ // Before we can commit the new option should be configured. If it is not
+ // than somebody must have called commit() before build().
+ isc_throw(isc::InvalidOperation, "Parser logic error: no option has been configured and"
+ " thus there is nothing to commit. Has build() been called?");
+ }
+ uint16_t opt_type = option_descriptor_.option->getType();
+ Subnet::OptionContainerTypeIndex& idx = options_->get<1>();
+ // Try to find options with the particular option code in the main
+ // storage. If found, remove these options because they will be
+ // replaced with new one.
+ Subnet::OptionContainerTypeRange range =
+ idx.equal_range(opt_type);
+ if (std::distance(range.first, range.second) > 0) {
+ idx.erase(range.first, range.second);
+ }
+ // Append new option to the main storage.
+ options_->push_back(option_descriptor_);
+ }
+
+ /// @brief Set storage for the parser.
+ ///
+ /// Sets storage for the parser. This storage points to the
+ /// vector of options and is used by multiple instances of
+ /// OptionDataParser. Each instance creates exactly one object
+ /// of dhcp::Option or derived type and appends it to this
+ /// storage.
+ ///
+ /// @param storage pointer to the options storage
+ void setStorage(OptionStorage* storage) {
+ options_ = storage;
+ }
+
+private:
+
+ /// @brief Create option instance.
+ ///
+ /// Creates an instance of an option and adds it to the provided
+ /// options storage. If the option data parsed by \ref build function
+ /// are invalid or insufficient this function emits an exception.
+ ///
+ /// @warning this function does not check if options_ storage pointer
+ /// is intitialized but this check is not needed here because it is done
+ /// in the \ref build function.
+ ///
+ /// @throw Dhcp4ConfigError if parameters provided in the configuration
+ /// are invalid.
+ void createOption() {
+ // Option code is held in the uint32_t storage but is supposed to
+ // be uint16_t value. We need to check that value in the configuration
+ // does not exceed range of uint16_t and is not zero.
+ uint32_t option_code = getUint32Param("code");
+ if (option_code == 0) {
+ isc_throw(Dhcp4ConfigError, "Parser error: value of 'code' must not"
+ << " be equal to zero. Option code '0' is reserved in"
+ << " DHCPv4.");
+ } else if (option_code > std::numeric_limits<uint16_t>::max()) {
+ isc_throw(Dhcp4ConfigError, "Parser error: value of 'code' must not"
+ << " exceed " << std::numeric_limits<uint16_t>::max());
+ }
+ // Check that the option name has been specified, is non-empty and does not
+ // contain spaces.
+ // @todo possibly some more restrictions apply here?
+ std::string option_name = getStringParam("name");
+ if (option_name.empty()) {
+ isc_throw(Dhcp4ConfigError, "Parser error: option name must not be"
+ << " empty");
+ } else if (option_name.find(" ") != std::string::npos) {
+ isc_throw(Dhcp4ConfigError, "Parser error: option name must not contain"
+ << " spaces");
+ }
+
+ // Get option data from the configuration database ('data' field).
+ // Option data is specified by the user as case insensitive string
+ // of hexadecimal digits for each option.
+ std::string option_data = getStringParam("data");
+ // Transform string of hexadecimal digits into binary format.
+ std::vector<uint8_t> binary;
+ try {
+ util::encode::decodeHex(option_data, binary);
+ } catch (...) {
+ isc_throw(Dhcp4ConfigError, "Parser error: option data is not a valid"
+ << " string of hexadecimal digits: " << option_data);
+ }
+ // Get all existing DHCPv4 option definitions. The one that matches
+ // our option will be picked and used to create it.
+ OptionDefContainer option_defs = LibDHCP::getOptionDefs(Option::V4);
+ // Get search index #1. It allows searching for options definitions
+ // using option type value.
+ const OptionDefContainerTypeIndex& idx = option_defs.get<1>();
+ // Get all option definitions matching option code we want to create.
+ const OptionDefContainerTypeRange& range = idx.equal_range(option_code);
+ size_t num_defs = std::distance(range.first, range.second);
+ OptionPtr option;
+ // Currently we do not allow duplicated definitions and if there are
+ // any duplicates we issue internal server error.
+ if (num_defs > 1) {
+ isc_throw(Dhcp4ConfigError, "Internal error: currently it is not"
+ << " supported to initialize multiple option definitions"
+ << " for the same option code. This will be supported once"
+ << " there option spaces are implemented.");
+ } else if (num_defs == 0) {
+ // @todo We have a limited set of option definitions intiialized at the moment.
+ // In the future we want to initialize option definitions for all options.
+ // Consequently an error will be issued if an option definition does not exist
+ // for a particular option code. For now it is ok to create generic option
+ // if definition does not exist.
+ OptionPtr option(new Option(Option::V4, static_cast<uint16_t>(option_code),
+ binary));
+ // The created option is stored in option_descriptor_ class member until the
+ // commit stage when it is inserted into the main storage. If an option with the
+ // same code exists in main storage already the old option is replaced.
+ option_descriptor_.option = option;
+ option_descriptor_.persistent = false;
+ } else {
+ // We have exactly one option definition for the particular option code
+ // use it to create the option instance.
+ const OptionDefinitionPtr& def = *(range.first);
+ try {
+ OptionPtr option = def->optionFactory(Option::V4, option_code, binary);
+ Subnet::OptionDescriptor desc(option, false);
+ option_descriptor_.option = option;
+ option_descriptor_.persistent = false;
+ } catch (const isc::Exception& ex) {
+ isc_throw(Dhcp4ConfigError, "Parser error: option data does not match"
+ << " option definition (code " << option_code << "): "
+ << ex.what());
+ }
+ }
+ }
+
+ /// @brief Get a parameter from the strings storage.
+ ///
+ /// @param param_id parameter identifier.
+ /// @throw Dhcp4ConfigError if parameter has not been found.
+ std::string getStringParam(const std::string& param_id) const {
+ StringStorage::const_iterator param = string_values_.find(param_id);
+ if (param == string_values_.end()) {
+ isc_throw(Dhcp4ConfigError, "Parser error: option-data parameter"
+ << " '" << param_id << "' not specified");
+ }
+ return (param->second);
+ }
+
+ /// @brief Get a parameter from the uint32 values storage.
+ ///
+ /// @param param_id parameter identifier.
+ /// @throw Dhcp4ConfigError if parameter has not been found.
+ uint32_t getUint32Param(const std::string& param_id) const {
+ Uint32Storage::const_iterator param = uint32_values_.find(param_id);
+ if (param == uint32_values_.end()) {
+ isc_throw(Dhcp4ConfigError, "Parser error: option-data parameter"
+ << " '" << param_id << "' not specified");
+ }
+ return (param->second);
+ }
+
+ /// Storage for uint32 values (e.g. option code).
+ Uint32Storage uint32_values_;
+ /// Storage for string values (e.g. option name or data).
+ StringStorage string_values_;
+ /// Pointer to options storage. This storage is provided by
+ /// the calling class and is shared by all OptionDataParser objects.
+ OptionStorage* options_;
+ /// Option descriptor holds newly configured option.
+ Subnet::OptionDescriptor option_descriptor_;
+};
+
+/// @brief Parser for option data values within a subnet.
+///
+/// This parser iterates over all entries that define options
+/// data for a particular subnet and creates a collection of options.
+/// If parsing is successful, all these options are added to the Subnet
+/// object.
+class OptionDataListParser : public Dhcp4ConfigParser {
+public:
+
+ /// @brief Constructor.
+ ///
+ /// Unless otherwise specified, parsed options will be stored in
+ /// a global option container (option_default). That storage location
+ /// is overriden on a subnet basis.
+ OptionDataListParser(const std::string&)
+ : options_(&option_defaults), local_options_() { }
+
+ /// @brief Parses entries that define options' data for a subnet.
+ ///
+ /// This method iterates over all entries that define option data
+ /// for options within a single subnet and creates options' instances.
+ ///
+ /// @param option_data_list pointer to a list of options' data sets.
+ /// @throw Dhcp4ConfigError if option parsing failed.
+ void build(ConstElementPtr option_data_list) {
+ BOOST_FOREACH(ConstElementPtr option_value, option_data_list->listValue()) {
+ boost::shared_ptr<OptionDataParser> parser(new OptionDataParser("option-data"));
+ // options_ member will hold instances of all options thus
+ // each OptionDataParser takes it as a storage.
+ parser->setStorage(&local_options_);
+ // Build the instance of a single option.
+ parser->build(option_value);
+ // Store a parser as it will be used to commit.
+ parsers_.push_back(parser);
+ }
+ }
+
+ /// @brief Set storage for option instances.
+ ///
+ /// @param storage pointer to options storage.
+ void setStorage(OptionStorage* storage) {
+ options_ = storage;
+ }
+
+
+ /// @brief Commit all option values.
+ ///
+ /// This function invokes commit for all option values.
+ void commit() {
+ BOOST_FOREACH(ParserPtr parser, parsers_) {
+ parser->commit();
+ }
+ // Parsing was successful and we have all configured
+ // options in local storage. We can now replace old values
+ // with new values.
+ std::swap(local_options_, *options_);
+ }
+
+ /// @brief Create DhcpDataListParser object
+ ///
+ /// @param param_name param name.
+ ///
+ /// @return DhcpConfigParser object.
+ static Dhcp4ConfigParser* Factory(const std::string& param_name) {
+ return (new OptionDataListParser(param_name));
+ }
+
+ /// Intermediate option storage. This storage is used by
+ /// lower level parsers to add new options. Values held
+ /// in this storage are assigned to main storage (options_)
+ /// if overall parsing was successful.
+ OptionStorage local_options_;
+ /// Pointer to options instances storage.
+ OptionStorage* options_;
+ /// Collection of parsers;
+ ParserCollection parsers_;
+};
+
/// @brief this class parses a single subnet
///
/// This class parses the whole subnet definition. It creates parsers
@@ -470,35 +819,31 @@ public:
void build(ConstElementPtr subnet) {
BOOST_FOREACH(ConfigPair param, subnet->mapValue()) {
-
ParserPtr parser(createSubnet4ConfigParser(param.first));
-
- // if this is an Uint32 parser, tell it to store the values
- // in values_, rather than in global storage
- boost::shared_ptr<Uint32Parser> uint_parser =
- boost::dynamic_pointer_cast<Uint32Parser>(parser);
- if (uint_parser) {
- uint_parser->setStorage(&uint32_values_);
- } else {
-
- boost::shared_ptr<StringParser> string_parser =
- boost::dynamic_pointer_cast<StringParser>(parser);
- if (string_parser) {
- string_parser->setStorage(&string_values_);
- } else {
-
- boost::shared_ptr<PoolParser> pool_parser =
- boost::dynamic_pointer_cast<PoolParser>(parser);
- if (pool_parser) {
- pool_parser->setStorage(&pools_);
- }
- }
+ // The actual type of the parser is unknown here. We have to discover
+ // the parser type here to invoke the corresponding setStorage function
+ // on it. We discover parser type by trying to cast the parser to various
+ // parser types and checking which one was successful. For this one
+ // a setStorage and build methods are invoked.
+
+ // Try uint32 type parser.
+ if (!buildParser<Uint32Parser, Uint32Storage >(parser, uint32_values_,
+ param.second) &&
+ // Try string type parser.
+ !buildParser<StringParser, StringStorage >(parser, string_values_,
+ param.second) &&
+ // Try pool parser.
+ !buildParser<PoolParser, PoolStorage >(parser, pools_,
+ param.second) &&
+ // Try option data parser.
+ !buildParser<OptionDataListParser, OptionStorage >(parser, options_,
+ param.second)) {
+ // Appropriate parsers are created in the createSubnet6ConfigParser
+ // and they should be limited to those that we check here for. Thus,
+ // if we fail to find a matching parser here it is a programming error.
+ isc_throw(Dhcp4ConfigError, "failed to find suitable parser");
}
-
- parser->build(param.second);
- parsers_.push_back(parser);
}
-
// Ok, we now have subnet parsed
}
@@ -510,6 +855,10 @@ public:
/// objects. Subnet4 are then added to DHCP CfgMgr.
/// @throw Dhcp4ConfigError if there are any issues encountered during commit
void commit() {
+ // Invoke commit on all sub-data parsers.
+ BOOST_FOREACH(ParserPtr parser, parsers_) {
+ parser->commit();
+ }
StringStorage::const_iterator it = string_values_.find("subnet");
if (it == string_values_.end()) {
@@ -545,11 +894,79 @@ public:
subnet->addPool4(*it);
}
+ const Subnet::OptionContainer& options = subnet->getOptions();
+ const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+
+ // Add subnet specific options.
+ BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) {
+ Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
+ if (std::distance(range.first, range.second) > 0) {
+ LOG_WARN(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE)
+ .arg(desc.option->getType()).arg(addr.toText());
+ }
+ subnet->addOption(desc.option);
+ }
+
+ // Check all global options and add them to the subnet object if
+ // they have been configured in the global scope. If they have been
+ // configured in the subnet scope we don't add global option because
+ // the one configured in the subnet scope always takes precedence.
+ BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) {
+ // Get all options specified locally in the subnet and having
+ // code equal to global option's code.
+ Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType());
+ // @todo: In the future we will be searching for options using either
+ // an option code or namespace. Currently we have only the option
+ // code available so if there is at least one option found with the
+ // specific code we don't add the globally configured option.
+ // @todo with this code the first globally configured option
+ // with the given code will be added to a subnet. We may
+ // want to issue a warning about dropping the configuration of
+ // a global option if one already exsists.
+ if (std::distance(range.first, range.second) == 0) {
+ subnet->addOption(desc.option);
+ }
+ }
+
CfgMgr::instance().addSubnet4(subnet);
}
private:
+ /// @brief Set storage for a parser and invoke build.
+ ///
+ /// This helper method casts the provided parser pointer to the specified
+ /// type. If the cast is successful it sets the corresponding storage for
+ /// this parser, invokes build on it and saves the parser.
+ ///
+ /// @tparam T parser type to which parser argument should be cast.
+ /// @tparam Y storage type for the specified parser type.
+ /// @param parser parser on which build must be invoked.
+ /// @param storage reference to a storage that will be set for a parser.
+ /// @param subnet subnet element read from the configuration and being parsed.
+ /// @return true if parser pointer was successfully cast to specialized
+ /// parser type provided as Y.
+ template<typename T, typename Y>
+ bool buildParser(const ParserPtr& parser, Y& storage, const ConstElementPtr& subnet) {
+ // We need to cast to T in order to set storage for the parser.
+ boost::shared_ptr<T> cast_parser = boost::dynamic_pointer_cast<T>(parser);
+ // It is common that this cast is not successful because we try to cast to all
+ // supported parser types as we don't know the type of a parser in advance.
+ if (cast_parser) {
+ // Cast, successful so we go ahead with setting storage and actual parse.
+ cast_parser->setStorage(&storage);
+ parser->build(subnet);
+ parsers_.push_back(parser);
+ // We indicate that cast was successful so as the calling function
+ // may skip attempts to cast to other parser types and proceed to
+ // next element.
+ return (true);
+ }
+ // It was not successful. Indicate that another parser type
+ // should be tried.
+ return (false);
+ }
+
/// @brief creates parsers for entries in subnet definition
///
/// @todo Add subnet-specific things here (e.g. subnet-specific options)
@@ -565,6 +982,7 @@ private:
factories["rebind-timer"] = Uint32Parser::Factory;
factories["subnet"] = StringParser::Factory;
factories["pool"] = PoolParser::Factory;
+ factories["option-data"] = OptionDataListParser::Factory;
FactoryMap::iterator f = factories.find(config_id);
if (f == factories.end()) {
@@ -620,6 +1038,9 @@ private:
/// storage for pools belonging to this subnet
PoolStorage pools_;
+ /// storage for options belonging to this subnet
+ OptionStorage options_;
+
/// parsers are stored here
ParserCollection parsers_;
};
@@ -650,7 +1071,6 @@ public:
// used: Subnet4ConfigParser
BOOST_FOREACH(ConstElementPtr subnet, subnets_list->listValue()) {
-
ParserPtr parser(new Subnet4ConfigParser("subnet"));
parser->build(subnet);
subnets_.push_back(parser);
@@ -702,6 +1122,7 @@ Dhcp4ConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) {
factories["rebind-timer"] = Uint32Parser::Factory;
factories["interface"] = InterfaceListConfigParser::Factory;
factories["subnet4"] = Subnets4ListConfigParser::Factory;
+ factories["option-data"] = OptionDataListParser::Factory;
factories["version"] = StringParser::Factory;
FactoryMap::iterator f = factories.find(config_id);
@@ -739,7 +1160,7 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) {
}
} catch (const isc::Exception& ex) {
ConstElementPtr answer = isc::config::createAnswer(1,
- string("Configuration parsing failed:") + ex.what());
+ string("Configuration parsing failed: ") + ex.what());
return (answer);
} catch (...) {
// for things like bad_cast in boost::lexical_cast
@@ -754,7 +1175,7 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) {
}
catch (const isc::Exception& ex) {
ConstElementPtr answer = isc::config::createAnswer(2,
- string("Configuration commit failed:") + ex.what());
+ string("Configuration commit failed: ") + ex.what());
return (answer);
} catch (...) {
// for things like bad_cast in boost::lexical_cast
diff --git a/src/bin/dhcp4/config_parser.h b/src/bin/dhcp4/config_parser.h
index 2ee9cf6..cc4c690 100644
--- a/src/bin/dhcp4/config_parser.h
+++ b/src/bin/dhcp4/config_parser.h
@@ -14,6 +14,7 @@
#include <exceptions/exceptions.h>
#include <cc/data.h>
+#include <stdint.h>
#include <string>
#ifndef DHCP4_CONFIG_PARSER_H
diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec
index d5066e8..46192de 100644
--- a/src/bin/dhcp4/dhcp4.spec
+++ b/src/bin/dhcp4/dhcp4.spec
@@ -34,6 +34,37 @@
"item_default": 4000
},
+ { "item_name": "option-data",
+ "item_type": "list",
+ "item_optional": false,
+ "item_default": [],
+ "list_item_spec":
+ {
+ "item_name": "single-option-data",
+ "item_type": "map",
+ "item_optional": false,
+ "item_default": {},
+ "map_item_spec": [
+ {
+ "item_name": "name",
+ "item_type": "string",
+ "item_optional": false,
+ "item_default": ""
+ },
+
+ { "item_name": "code",
+ "item_type": "integer",
+ "item_optional": false,
+ "item_default": 0
+ },
+ { "item_name": "data",
+ "item_type": "string",
+ "item_optional": false,
+ "item_default": ""
+ } ]
+ }
+ },
+
{ "item_name": "subnet4",
"item_type": "list",
"item_optional": false,
@@ -80,9 +111,40 @@
"item_optional": false,
"item_default": ""
}
- }
- ]
- }
+ },
+
+ { "item_name": "option-data",
+ "item_type": "list",
+ "item_optional": false,
+ "item_default": [],
+ "list_item_spec":
+ {
+ "item_name": "single-option-data",
+ "item_type": "map",
+ "item_optional": false,
+ "item_default": {},
+ "map_item_spec": [
+ {
+ "item_name": "name",
+ "item_type": "string",
+ "item_optional": false,
+ "item_default": ""
+ },
+ {
+ "item_name": "code",
+ "item_type": "integer",
+ "item_optional": false,
+ "item_default": 0
+ },
+ {
+ "item_name": "data",
+ "item_type": "string",
+ "item_optional": false,
+ "item_default": ""
+ } ]
+ }
+ } ]
+ }
}
],
"commands": [
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index b564590..c7bfeae 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -50,6 +50,11 @@ change is committed by the administrator.
A debug message indicating that the IPv4 DHCP server has received an
updated configuration from the BIND 10 configuration system.
+% DHCP4_CONFIG_OPTION_DUPLICATE multiple options with the code: %1 added to the subnet: %2
+This warning message is issued on an attempt to configure multiple options with the
+same option code for the particular subnet. Adding multiple options is uncommon
+for DHCPv4, but it is not prohibited.
+
% DHCP4_NOT_RUNNING IPv4 DHCP server is not running
A warning message is issued when an attempt is made to shut down the
IPv4 DHCP server but it is not running.
diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am
index e601919..c0ebcb9 100644
--- a/src/bin/dhcp4/tests/Makefile.am
+++ b/src/bin/dhcp4/tests/Makefile.am
@@ -66,13 +66,13 @@ dhcp4_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
dhcp4_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
dhcp4_unittests_LDADD = $(GTEST_LDADD)
dhcp4_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
+dhcp4_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+dhcp4_unittests_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
dhcp4_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
dhcp4_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
-dhcp4_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
-dhcp4_unittests_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
-dhcp4_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+dhcp4_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
endif
noinst_PROGRAMS = $(TESTS)
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index a4ccabd..3dd75d7 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -22,6 +22,7 @@
#include <config/ccsession.h>
#include <dhcpsrv/subnet.h>
#include <dhcpsrv/cfgmgr.h>
+#include <boost/foreach.hpp>
#include <iostream>
#include <fstream>
#include <sstream>
@@ -73,9 +74,188 @@ public:
}
~Dhcp4ParserTest() {
+ resetConfiguration();
delete srv_;
};
+ /// @brief Create the simple configuration with single option.
+ ///
+ /// This function allows to set one of the parameters that configure
+ /// option value. These parameters are: "name", "code" and "data".
+ ///
+ /// @param param_value string holiding option parameter value to be
+ /// injected into the configuration string.
+ /// @param parameter name of the parameter to be configured with
+ /// param value.
+ /// @return configuration string containing custom values of parameters
+ /// describing an option.
+ std::string createConfigWithOption(const std::string& param_value,
+ const std::string& parameter) {
+ std::map<std::string, std::string> params;
+ if (parameter == "name") {
+ params["name"] = param_value;
+ params["code"] = "56";
+ params["data"] = "AB CDEF0105";
+ } else if (parameter == "code") {
+ params["name"] = "option_foo";
+ params["code"] = param_value;
+ params["data"] = "AB CDEF0105";
+ } else if (parameter == "data") {
+ params["name"] = "option_foo";
+ params["code"] = "56";
+ params["data"] = param_value;
+ }
+ return (createConfigWithOption(params));
+ }
+
+ /// @brief Create simple configuration with single option.
+ ///
+ /// This function creates a configuration for a single option with
+ /// custom values for all parameters that describe the option.
+ ///
+ /// @params params map holding parameters and their values.
+ /// @return configuration string containing custom values of parameters
+ /// describing an option.
+ std::string createConfigWithOption(const std::map<std::string, std::string>& params) {
+ std::ostringstream stream;
+ stream << "{ \"interface\": [ \"all\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\", "
+ " \"option-data\": [ {";
+ bool first = true;
+ typedef std::pair<std::string, std::string> ParamPair;
+ BOOST_FOREACH(ParamPair param, params) {
+ if (!first) {
+ stream << ", ";
+ } else {
+ // cppcheck-suppress unreadVariable
+ first = false;
+ }
+ if (param.first == "name") {
+ stream << "\"name\": \"" << param.second << "\"";
+ } else if (param.first == "code") {
+ stream << "\"code\": " << param.second << "";
+ } else if (param.first == "data") {
+ stream << "\"data\": \"" << param.second << "\"";
+ }
+ }
+ stream <<
+ " } ]"
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+ return (stream.str());
+ }
+
+ /// @brief Test invalid option parameter value.
+ ///
+ /// This test function constructs the simple configuration
+ /// string and injects invalid option configuration into it.
+ /// It expects that parser will fail with provided option code.
+ ///
+ /// @param param_value string holding invalid option parameter value
+ /// to be injected into configuration string.
+ /// @param parameter name of the parameter to be configured with
+ /// param_value (can be any of "name", "code", "data")
+ void testInvalidOptionParam(const std::string& param_value,
+ const std::string& parameter) {
+ ConstElementPtr x;
+ std::string config = createConfigWithOption(param_value, parameter);
+ ElementPtr json = Element::fromJSON(config);
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(1, rcode_);
+ }
+
+ /// @brief Test option against given code and data.
+ ///
+ /// @param option_desc option descriptor that carries the option to
+ /// be tested.
+ /// @param expected_code expected code of the option.
+ /// @param expected_data expected data in the option.
+ /// @param expected_data_len length of the reference data.
+ /// @param extra_data if true extra data is allowed in an option
+ /// after tested data.
+ void testOption(const Subnet::OptionDescriptor& option_desc,
+ uint16_t expected_code, const uint8_t* expected_data,
+ size_t expected_data_len,
+ bool extra_data = false) {
+ // Check if option descriptor contains valid option pointer.
+ ASSERT_TRUE(option_desc.option);
+ // Verify option type.
+ EXPECT_EQ(expected_code, option_desc.option->getType());
+ // We may have many different option types being created. Some of them
+ // have dedicated classes derived from Option class. In such case if
+ // we want to verify the option contents against expected_data we have
+ // to prepare raw buffer with the contents of the option. The easiest
+ // way is to call pack() which will prepare on-wire data.
+ util::OutputBuffer buf(option_desc.option->getData().size());
+ option_desc.option->pack(buf);
+ if (extra_data) {
+ // The length of the buffer must be at least equal to size of the
+ // reference data but it can sometimes be greater than that. This is
+ // because some options carry suboptions that increase the overall
+ // length.
+ ASSERT_GE(buf.getLength() - option_desc.option->getHeaderLen(),
+ expected_data_len);
+ } else {
+ ASSERT_EQ(buf.getLength() - option_desc.option->getHeaderLen(),
+ expected_data_len);
+ }
+ // Verify that the data is correct. Do not verify suboptions and a header.
+ const uint8_t* data = static_cast<const uint8_t*>(buf.getData());
+ EXPECT_EQ(0, memcmp(expected_data, data + option_desc.option->getHeaderLen(),
+ expected_data_len));
+ }
+
+ /// @brief Reset configuration database.
+ ///
+ /// This function resets configuration data base by
+ /// removing all subnets and option-data. Reset must
+ /// be performed after each test to make sure that
+ /// contents of the database do not affect result of
+ /// subsequent tests.
+ void resetConfiguration() {
+ ConstElementPtr status;
+
+ string config = "{ \"interface\": [ \"all\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"valid-lifetime\": 4000, "
+ "\"subnet4\": [ ], "
+ "\"option-data\": [ ] }";
+
+ try {
+ ElementPtr json = Element::fromJSON(config);
+ status = configureDhcp4Server(*srv_, json);
+ } catch (const std::exception& ex) {
+ FAIL() << "Fatal error: unable to reset configuration database"
+ << " after the test. The following configuration was used"
+ << " to reset database: " << std::endl
+ << config << std::endl
+ << " and the following error message was returned:"
+ << ex.what() << std::endl;
+ }
+
+ // status object must not be NULL
+ if (!status) {
+ FAIL() << "Fatal error: unable to reset configuration database"
+ << " after the test. Configuration function returned"
+ << " NULL pointer" << std::endl;
+ }
+
+ comment_ = parseAnswer(rcode_, status);
+ // returned value should be 0 (configuration success)
+ if (rcode_ != 0) {
+ FAIL() << "Fatal error: unable to reset configuration database"
+ << " after the test. Configuration function returned"
+ << " error code " << rcode_ << std::endl;
+ }
+ }
+
Dhcpv4Srv* srv_;
int rcode_;
@@ -248,6 +428,302 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) {
EXPECT_EQ(4000, subnet->getValid());
}
+// Goal of this test is to verify that global option
+// data is configured for the subnet if the subnet
+// configuration does not include options configuration.
+TEST_F(Dhcp4ParserTest, optionDataDefaults) {
+ ConstElementPtr x;
+ string config = "{ \"interface\": [ \"all\" ],"
+ "\"rebind-timer\": 2000,"
+ "\"renew-timer\": 1000,"
+ "\"option-data\": [ {"
+ " \"name\": \"option_foo\","
+ " \"code\": 56,"
+ " \"data\": \"AB CDEF0105\""
+ " },"
+ " {"
+ " \"name\": \"option_foo2\","
+ " \"code\": 23,"
+ " \"data\": \"01\""
+ " } ],"
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\""
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ ElementPtr json = Element::fromJSON(config);
+
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
+ ASSERT_TRUE(subnet);
+ const Subnet::OptionContainer& options = subnet->getOptions();
+ ASSERT_EQ(2, options.size());
+
+ // Get the search index. Index #1 is to search using option code.
+ const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+
+ // Get the options for specified index. Expecting one option to be
+ // returned but in theory we may have multiple options with the same
+ // code so we get the range.
+ std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+ Subnet::OptionContainerTypeIndex::const_iterator> range =
+ idx.equal_range(56);
+ // Expect single option with the code equal to 56.
+ ASSERT_EQ(1, std::distance(range.first, range.second));
+ const uint8_t foo_expected[] = {
+ 0xAB, 0xCD, 0xEF, 0x01, 0x05
+ };
+ // Check if option is valid in terms of code and carried data.
+ testOption(*range.first, 56, foo_expected, sizeof(foo_expected));
+
+ range = idx.equal_range(23);
+ ASSERT_EQ(1, std::distance(range.first, range.second));
+ // Do another round of testing with second option.
+ const uint8_t foo2_expected[] = {
+ 0x01
+ };
+ testOption(*range.first, 23, foo2_expected, sizeof(foo2_expected));
+}
+
+// Goal of this test is to verify options configuration
+// for a single subnet. In particular this test checks
+// that local options configuration overrides global
+// option setting.
+TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
+ ConstElementPtr x;
+ string config = "{ \"interface\": [ \"all\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"option-data\": [ {"
+ " \"name\": \"option_foo\","
+ " \"code\": 56,"
+ " \"data\": \"AB\""
+ " } ],"
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\", "
+ " \"option-data\": [ {"
+ " \"name\": \"option_foo\","
+ " \"code\": 56,"
+ " \"data\": \"AB CDEF0105\""
+ " },"
+ " {"
+ " \"name\": \"option_foo2\","
+ " \"code\": 23,"
+ " \"data\": \"01\""
+ " } ]"
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ ElementPtr json = Element::fromJSON(config);
+
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.24"));
+ ASSERT_TRUE(subnet);
+ const Subnet::OptionContainer& options = subnet->getOptions();
+ ASSERT_EQ(2, options.size());
+
+ // Get the search index. Index #1 is to search using option code.
+ const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+
+ // Get the options for specified index. Expecting one option to be
+ // returned but in theory we may have multiple options with the same
+ // code so we get the range.
+ std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+ Subnet::OptionContainerTypeIndex::const_iterator> range =
+ idx.equal_range(56);
+ // Expect single option with the code equal to 100.
+ ASSERT_EQ(1, std::distance(range.first, range.second));
+ const uint8_t foo_expected[] = {
+ 0xAB, 0xCD, 0xEF, 0x01, 0x05
+ };
+ // Check if option is valid in terms of code and carried data.
+ testOption(*range.first, 56, foo_expected, sizeof(foo_expected));
+
+ range = idx.equal_range(23);
+ ASSERT_EQ(1, std::distance(range.first, range.second));
+ // Do another round of testing with second option.
+ const uint8_t foo2_expected[] = {
+ 0x01
+ };
+ testOption(*range.first, 23, foo2_expected, sizeof(foo2_expected));
+}
+
+// Goal of this test is to verify options configuration
+// for multiple subnets.
+TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
+ ConstElementPtr x;
+ string config = "{ \"interface\": [ \"all\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\", "
+ " \"option-data\": [ {"
+ " \"name\": \"option_foo\","
+ " \"code\": 56,"
+ " \"data\": \"0102030405060708090A\""
+ " } ]"
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
+ " \"subnet\": \"192.0.3.0/24\", "
+ " \"option-data\": [ {"
+ " \"name\": \"option_foo2\","
+ " \"code\": 23,"
+ " \"data\": \"FF\""
+ " } ]"
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ ElementPtr json = Element::fromJSON(config);
+
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ Subnet4Ptr subnet1 = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.100"));
+ ASSERT_TRUE(subnet1);
+ const Subnet::OptionContainer& options1 = subnet1->getOptions();
+ ASSERT_EQ(1, options1.size());
+
+ // Get the search index. Index #1 is to search using option code.
+ const Subnet::OptionContainerTypeIndex& idx1 = options1.get<1>();
+
+ // Get the options for specified index. Expecting one option to be
+ // returned but in theory we may have multiple options with the same
+ // code so we get the range.
+ std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+ Subnet::OptionContainerTypeIndex::const_iterator> range1 =
+ idx1.equal_range(56);
+ // Expect single option with the code equal to 56.
+ ASSERT_EQ(1, std::distance(range1.first, range1.second));
+ const uint8_t foo_expected[] = {
+ 0x01, 0x02, 0x03, 0x04, 0x05,
+ 0x06, 0x07, 0x08, 0x09, 0x0A
+ };
+ // Check if option is valid in terms of code and carried data.
+ testOption(*range1.first, 56, foo_expected, sizeof(foo_expected));
+
+ // Test another subnet in the same way.
+ Subnet4Ptr subnet2 = CfgMgr::instance().getSubnet4(IOAddress("192.0.3.102"));
+ ASSERT_TRUE(subnet2);
+ const Subnet::OptionContainer& options2 = subnet2->getOptions();
+ ASSERT_EQ(1, options2.size());
+
+ const Subnet::OptionContainerTypeIndex& idx2 = options2.get<1>();
+ std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+ Subnet::OptionContainerTypeIndex::const_iterator> range2 =
+ idx2.equal_range(23);
+ ASSERT_EQ(1, std::distance(range2.first, range2.second));
+
+ const uint8_t foo2_expected[] = { 0xFF };
+ testOption(*range2.first, 23, foo2_expected, sizeof(foo2_expected));
+}
+
+// Verify that empty option name is rejected in the configuration.
+TEST_F(Dhcp4ParserTest, optionNameEmpty) {
+ // Empty option names not allowed.
+ testInvalidOptionParam("", "name");
+}
+
+// Verify that empty option name with spaces is rejected
+// in the configuration.
+TEST_F(Dhcp4ParserTest, optionNameSpaces) {
+ // Spaces in option names not allowed.
+ testInvalidOptionParam("option foo", "name");
+}
+
+// Verify that negative option code is rejected in the configuration.
+TEST_F(Dhcp4ParserTest, optionCodeNegative) {
+ // Check negative option code -4. This should fail too.
+ testInvalidOptionParam("-4", "code");
+}
+
+// Verify that out of bounds option code is rejected in the configuration.
+TEST_F(Dhcp4ParserTest, optionCodeNonUint8) {
+ // The valid option codes are uint16_t values so passing
+ // uint16_t maximum value incremented by 1 should result
+ // in failure.
+ testInvalidOptionParam("257", "code");
+}
+
+// Verify that zero option code is rejected in the configuration.
+TEST_F(Dhcp4ParserTest, optionCodeZero) {
+ // Option code 0 is reserved and should not be accepted
+ // by configuration parser.
+ testInvalidOptionParam("0", "code");
+}
+
+// Verify that option data which contains non hexadecimal characters
+// is rejected by the configuration.
+TEST_F(Dhcp4ParserTest, optionDataInvalidChar) {
+ // Option code 0 is reserved and should not be accepted
+ // by configuration parser.
+ testInvalidOptionParam("01020R", "data");
+}
+
+// Verify that option data containins '0x' prefix is rejected
+// by the configuration.
+TEST_F(Dhcp4ParserTest, optionDataUnexpectedPrefix) {
+ // Option code 0 is reserved and should not be accepted
+ // by configuration parser.
+ testInvalidOptionParam("0x0102", "data");
+}
+
+// Verify that option data consisting od an odd number of
+// hexadecimal digits is rejected in the configuration.
+TEST_F(Dhcp4ParserTest, optionDataOddLength) {
+ // Option code 0 is reserved and should not be accepted
+ // by configuration parser.
+ testInvalidOptionParam("123", "data");
+}
+
+// Verify that either lower or upper case characters are allowed
+// to specify the option data.
+TEST_F(Dhcp4ParserTest, optionDataLowerCase) {
+ ConstElementPtr x;
+ std::string config = createConfigWithOption("0a0b0C0D", "data");
+ ElementPtr json = Element::fromJSON(config);
+
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"));
+ ASSERT_TRUE(subnet);
+ const Subnet::OptionContainer& options = subnet->getOptions();
+ ASSERT_EQ(1, options.size());
+
+ // Get the search index. Index #1 is to search using option code.
+ const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+
+ // Get the options for specified index. Expecting one option to be
+ // returned but in theory we may have multiple options with the same
+ // code so we get the range.
+ std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+ Subnet::OptionContainerTypeIndex::const_iterator> range =
+ idx.equal_range(56);
+ // Expect single option with the code equal to 100.
+ ASSERT_EQ(1, std::distance(range.first, range.second));
+ const uint8_t foo_expected[] = {
+ 0x0A, 0x0B, 0x0C, 0x0D
+ };
+ // Check if option is valid in terms of code and carried data.
+ testOption(*range.first, 56, foo_expected, sizeof(foo_expected));
+}
+
/// 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
diff --git a/src/bin/dhcp6/Makefile.am b/src/bin/dhcp6/Makefile.am
index 1d9766f..decd986 100644
--- a/src/bin/dhcp6/Makefile.am
+++ b/src/bin/dhcp6/Makefile.am
@@ -59,14 +59,14 @@ if USE_CLANGPP
b10_dhcp6_CXXFLAGS = -Wno-unused-parameter
endif
-b10_dhcp6_LDADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-b10_dhcp6_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
-b10_dhcp6_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
-b10_dhcp6_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
+b10_dhcp6_LDADD = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
+b10_dhcp6_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+b10_dhcp6_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
b10_dhcp6_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
b10_dhcp6_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
-b10_dhcp6_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
-b10_dhcp6_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+b10_dhcp6_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+b10_dhcp6_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
+b10_dhcp6_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
b10_dhcp6dir = $(pkgdatadir)
b10_dhcp6_DATA = dhcp6.spec
diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc
index 4c6fab1..944bb21 100644
--- a/src/bin/dhcp6/config_parser.cc
+++ b/src/bin/dhcp6/config_parser.cc
@@ -496,12 +496,12 @@ private:
///
/// This parser parses configuration entries that specify value of
/// a single option. These entries include option name, option code
-/// and data carried by the option. If parsing is successful than an
+/// and data carried by the option. If parsing is successful then an
/// instance of an option is created and added to the storage provided
/// by the calling class.
///
/// @todo This class parses and validates the option name. However it is
-/// not used anywhere util support for option spaces is implemented
+/// not used anywhere until support for option spaces is implemented
/// (see tickets #2319, #2314). When option spaces are implemented
/// there will be a way to reference the particular option using
/// its type (code) or option name.
@@ -857,26 +857,21 @@ public:
// a setStorage and build methods are invoked.
// Try uint32 type parser.
- if (buildParser<Uint32Parser, Uint32Storage >(parser, uint32_values_,
- param.second)) {
- // Storage set, build invoked on the parser, proceed with
- // next configuration element.
- continue;
- }
- // Try string type parser.
- if (buildParser<StringParser, StringStorage >(parser, string_values_,
- param.second)) {
- continue;
- }
- // Try pools parser.
- if (buildParser<PoolParser, PoolStorage >(parser, pools_,
- param.second)) {
- continue;
- }
- // Try option data parser.
- if (buildParser<OptionDataListParser, OptionStorage >(parser, options_,
- param.second)) {
- continue;
+ if (!buildParser<Uint32Parser, Uint32Storage >(parser, uint32_values_,
+ param.second) &&
+ // Try string type parser.
+ !buildParser<StringParser, StringStorage >(parser, string_values_,
+ param.second) &&
+ // Try pool parser.
+ !buildParser<PoolParser, PoolStorage >(parser, pools_,
+ param.second) &&
+ // Try option data parser.
+ !buildParser<OptionDataListParser, OptionStorage >(parser, options_,
+ param.second)) {
+ // Appropriate parsers are created in the createSubnet6ConfigParser
+ // and they should be limited to those that we check here for. Thus,
+ // if we fail to find a matching parser here it is a programming error.
+ isc_throw(Dhcp6ConfigError, "failed to find suitable parser");
}
}
// Ok, we now have subnet parsed
diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index 74ab401..6ef8455 100644
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/src/bin/dhcp6/dhcp6_messages.mes
@@ -47,9 +47,9 @@ This is an informational message reporting that the configuration has
been extended to include the specified subnet.
% DHCP6_CONFIG_OPTION_DUPLICATE multiple options with the code: %1 added to the subnet: %2
-This warning message is issued on attempt to configure multiple options with the
+This warning message is issued on an attempt to configure multiple options with the
same option code for the particular subnet. Adding multiple options is uncommon
-for DHCPv6, yet it is not prohibited.
+for DHCPv6, but it is not prohibited.
% DHCP6_CONFIG_START DHCPv6 server is processing the following configuration: %1
This is a debug message that is issued every time the server receives a
diff --git a/src/bin/dhcp6/tests/Makefile.am b/src/bin/dhcp6/tests/Makefile.am
index b01b877..d251df3 100644
--- a/src/bin/dhcp6/tests/Makefile.am
+++ b/src/bin/dhcp6/tests/Makefile.am
@@ -63,14 +63,13 @@ dhcp6_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
dhcp6_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
dhcp6_unittests_LDADD = $(GTEST_LDADD)
dhcp6_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
-dhcp6_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
+dhcp6_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
+dhcp6_unittests_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
-dhcp6_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
dhcp6_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-dhcp6_unittests_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
-dhcp6_unittests_LDADD += $(top_builddir)/src/lib/cc/libb10-cc.la
-
+dhcp6_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
+dhcp6_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
endif
noinst_PROGRAMS = $(TESTS)
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 4fd6baa..812eb68 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -97,6 +97,7 @@ public:
if (!first) {
stream << ", ";
} else {
+ // cppcheck-suppress unreadVariable
first = false;
}
if (param.first == "name") {
@@ -144,14 +145,14 @@ public:
<< ex.what() << std::endl;
}
-
- // returned value should be 0 (configuration success)
+ // status object must not be NULL
if (!status) {
FAIL() << "Fatal error: unable to reset configuration database"
<< " after the test. Configuration function returned"
<< " NULL pointer" << std::endl;
}
comment_ = parseAnswer(rcode_, status);
+ // returned value should be 0 (configuration success)
if (rcode_ != 0) {
FAIL() << "Fatal error: unable to reset configuration database"
<< " after the test. Configuration function returned"
@@ -215,9 +216,10 @@ public:
ASSERT_EQ(buf.getLength() - option_desc.option->getHeaderLen(),
expected_data_len);
}
- // Verify that the data is correct. However do not verify suboptions.
+ // Verify that the data is correct. Do not verify suboptions and a header.
const uint8_t* data = static_cast<const uint8_t*>(buf.getData());
- EXPECT_TRUE(memcmp(expected_data, data, expected_data_len));
+ EXPECT_EQ(0, memcmp(expected_data, data + option_desc.option->getHeaderLen(),
+ expected_data_len));
}
Dhcpv6Srv srv_;
More information about the bind10-changes
mailing list