From: Marcin Siodelski Date: Wed, 6 Mar 2019 08:10:19 +0000 (+0100) Subject: [#488,!259] Extracted common parsing functions to a new class. X-Git-Tag: 494-dhcp4configparser-sharednetworkssanitychecks-is-buggy_base~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=09c5653d82ec22f97cb69932dfe22103b020b644;p=thirdparty%2Fkea.git [#488,!259] Extracted common parsing functions to a new class. --- diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index e72952892e..0237391da8 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -169,6 +169,8 @@ libkea_dhcpsrv_la_SOURCES += utils.h libkea_dhcpsrv_la_SOURCES += writable_host_data_source.h # Configuration parsers +libkea_dhcpsrv_la_SOURCES += parsers/base_network_parser.cc +libkea_dhcpsrv_la_SOURCES += parsers/base_network_parser.h libkea_dhcpsrv_la_SOURCES += parsers/client_class_def_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/client_class_def_parser.h libkea_dhcpsrv_la_SOURCES += parsers/dhcp_parsers.cc @@ -343,6 +345,7 @@ endif # Specify parsers' headers for copying into installation directory tree. libkea_dhcpsrv_parsers_includedir = $(pkgincludedir)/dhcpsrv/parsers libkea_dhcpsrv_parsers_include_HEADERS = \ + parsers/base_network_parser.h \ parsers/client_class_def_parser.h \ parsers/dhcp_parsers.h \ parsers/duid_config_parser.h \ diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.cc b/src/lib/dhcpsrv/parsers/base_network_parser.cc new file mode 100644 index 0000000000..01f74ff490 --- /dev/null +++ b/src/lib/dhcpsrv/parsers/base_network_parser.cc @@ -0,0 +1,82 @@ +// Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include + +using namespace isc::data; +using namespace isc::util; + +namespace isc { +namespace dhcp { + +void +BaseNetworkParser::parseCommonTimers(const ConstElementPtr& shared_network_data, + NetworkPtr& network) { + Triplet t1; + if (shared_network_data->contains("renew-timer")) { + network->setT1(getInteger(shared_network_data, "renew-timer")); + } + + Triplet t2; + if (shared_network_data->contains("rebind-timer")) { + network->setT2(getInteger(shared_network_data, "rebind-timer")); + } + + Triplet valid; + if (shared_network_data->contains("valid-lifetime")) { + network->setValid(getInteger(shared_network_data, "valid-lifetime")); + } +} + +void +BaseNetworkParser::parseTeePercents(const ConstElementPtr& shared_network_data, + NetworkPtr& network) { + bool calculate_tee_times = network->getCalculateTeeTimes(); + if (shared_network_data->contains("calculate-tee-times")) { + bool calculate_tee_times = getBoolean(shared_network_data, "calculate-tee-times"); + network->setCalculateTeeTimes(calculate_tee_times); + } + + Optional t2_percent; + if (shared_network_data->contains("t2-percent")) { + t2_percent = getDouble(shared_network_data, "t2-percent"); + } + + Optional t1_percent; + if (shared_network_data->contains("t1-percent")) { + t1_percent = getDouble(shared_network_data, "t1-percent"); + } + if (calculate_tee_times) { + if (!t2_percent.unspecified() && ((t2_percent.get() <= 0.0) || + (t2_percent.get() >= 1.0))) { + isc_throw(DhcpConfigError, "t2-percent: " << t2_percent.get() + << " is invalid, it must be greater than 0.0 and less than 1.0"); + } + + if (!t1_percent.unspecified() && ((t1_percent.get() <= 0.0) || + (t1_percent.get() >= 1.0))) { + isc_throw(DhcpConfigError, "t1-percent: " << t1_percent.get() + << " is invalid it must be greater than 0.0 and less than 1.0"); + } + + if (!t1_percent.unspecified() && !t2_percent.unspecified() && + (t1_percent.get() >= t2_percent.get())) { + isc_throw(DhcpConfigError, "t1-percent: " << t1_percent.get() + << " is invalid, it must be less than t2-percent: " + << t2_percent.get()); + } + } + + network->setT2Percent(t2_percent); + network->setT1Percent(t1_percent); +} + + +} // end of namespace isc::dhcp +} // end of namespace isc diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.h b/src/lib/dhcpsrv/parsers/base_network_parser.h new file mode 100644 index 0000000000..8f3862f43b --- /dev/null +++ b/src/lib/dhcpsrv/parsers/base_network_parser.h @@ -0,0 +1,57 @@ +// Copyright (C) 2019 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef BASE_NETWORK_PARSER_H +#define BASE_NETWORK_PARSER_H + +#include +#include +#include + +namespace isc { +namespace dhcp { + +/// @brief Common configuration parser for shared networks +/// and subnets. +class BaseNetworkParser : public data::SimpleParser { +protected: + + /// @brief Parses common DHCP timers. + /// + /// The parsed parameters are: + /// - renew-timer, + /// - rebind-timer, + /// - valid-lifetime + /// + /// @param shared_network_data Data element holding shared network + /// configuration to be parsed. + /// @param [out] network Pointer to a network in which parsed data is + /// to be stored. + void parseCommonTimers(const data::ConstElementPtr& shared_network_data, + NetworkPtr& network); + + /// @brief Parses parameters related to "percent" timers settngs. + /// + /// The parsed parameters are: + /// - calculate-tee-times, + /// - t1-percent, + /// - t2-percent. + /// + /// @param shared_network_data Data element holding shared network + /// configuration to be parsed. + /// @param [out] network Pointer to a network in which parsed data is + /// to be stored. + /// + /// @throw DhcpConfigError if configuration of these parameters is + /// invalid. + void parseTeePercents(const data::ConstElementPtr& shared_network_data, + NetworkPtr& network); +}; + +} // end of namespace isc::dhcp +} // end of namespace isc + +#endif diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index c698534400..048064ecf2 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -670,24 +670,6 @@ Subnet4ConfigParser::parse(ConstElementPtr subnet) { void Subnet4ConfigParser::initSubnet(data::ConstElementPtr params, asiolink::IOAddress addr, uint8_t len) { - // The renew-timer and rebind-timer are optional. If not set, the - // option 58 and 59 will not be sent to a client. In this case the - // client should formulate default values based on the valid-lifetime. - Triplet t1; - if (params->contains("renew-timer")) { - t1 = getInteger(params, "renew-timer"); - } - - Triplet t2; - if (params->contains("rebind-timer")) { - t2 = getInteger(params, "rebind-timer"); - } - - Triplet valid; - if (params->contains("valid-lifetime")) { - valid = getInteger(params, "valid-lifetime"); - } - // Subnet ID is optional. If it is not supplied the value of 0 is used, // which means autogenerate. SubnetID subnet_id = 0; @@ -695,22 +677,29 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params, subnet_id = static_cast(getInteger(params, "id")); } + Subnet4Ptr subnet4(new Subnet4(addr, len, Triplet(), + Triplet(), Triplet(), + subnet_id)); + subnet_ = subnet4; + + NetworkPtr network = boost::dynamic_pointer_cast(subnet4); + parseCommonTimers(params, network); + stringstream s; s << addr << "/" << static_cast(len) << " with params: "; // t1 and t2 are optional may be not specified. - if (!t1.unspecified()) { - s << "t1=" << t1 << ", "; + if (!subnet4->getT1().unspecified()) { + s << "t1=" << subnet4->getT1().get() << ", "; } - if (!t2.unspecified()) { - s << "t2=" << t2 << ", "; + if (!subnet4->getT2().unspecified()) { + s << "t2=" << subnet4->getT2().get() << ", "; + } + if (!subnet4->getValid().unspecified()) { + s << "valid-lifetime=" << subnet4->getValid().get(); } - s <<"valid-lifetime=" << valid; LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_NEW_SUBNET4).arg(s.str()); - Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid, subnet_id)); - subnet_ = subnet4; - // Set the match-client-id value for the subnet. if (params->contains("match-client-id")) { bool match_client_id = getBoolean(params, "match-client-id"); @@ -882,44 +871,7 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params, // Copy options to the subnet configuration. options_->copyTo(*subnet4->getCfgOption()); - bool calculate_tee_times = subnet4->getCalculateTeeTimes(); - if (params->contains("calculate-tee-times")) { - bool calculate_tee_times = getBoolean(params, "calculate-tee-times"); - subnet4->setCalculateTeeTimes(calculate_tee_times); - } - - Optional t2_percent; - if (params->contains("t2-percent")) { - t2_percent = getDouble(params, "t2-percent"); - } - - Optional t1_percent; - if (params->contains("t1-percent")) { - t1_percent = getDouble(params, "t1-percent"); - } - if (calculate_tee_times) { - if (!t2_percent.unspecified() && ((t2_percent.get() <= 0.0) || - (t2_percent.get() >= 1.0))) { - isc_throw(DhcpConfigError, "t2-percent: " << t2_percent.get() - << " is invalid, it must be greater than 0.0 and less than 1.0"); - } - - if (!t1_percent.unspecified() && ((t1_percent.get() <= 0.0) || - (t1_percent.get() >= 1.0))) { - isc_throw(DhcpConfigError, "t1-percent: " << t1_percent.get() - << " is invalid it must be greater than 0.0 and less than 1.0"); - } - - if (!t1_percent.unspecified() && !t2_percent.unspecified() && - (t1_percent.get() >= t2_percent.get())) { - isc_throw(DhcpConfigError, "t1-percent: " << t1_percent.get() - << " is invalid, it must be less than t2-percent: " << t2_percent.get()); - } - - } - - subnet4->setT2Percent(t2_percent); - subnet4->setT1Percent(t1_percent); + parseTeePercents(params, network); } //**************************** Subnets4ListConfigParser ********************** @@ -1099,7 +1051,7 @@ PdPoolsListParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_list) { //**************************** Subnet6ConfigParser *********************** Subnet6ConfigParser::Subnet6ConfigParser() - :SubnetConfigParser(AF_INET6) { + : SubnetConfigParser(AF_INET6) { } Subnet6Ptr @@ -1162,27 +1114,6 @@ Subnet6ConfigParser::duplicate_option_warning(uint32_t code, void Subnet6ConfigParser::initSubnet(data::ConstElementPtr params, asiolink::IOAddress addr, uint8_t len) { - // Get all 'time' parameters. - Triplet t1; - if (params->contains("renew-timer")) { - t1 = getInteger(params, "renew-timer"); - } - - Triplet t2; - if (params->contains("rebind-timer")) { - t2 = getInteger(params, "rebind-timer"); - } - - Triplet pref; - if (params->contains("preferred-lifetime")) { - pref = getInteger(params, "preferred-lifetime"); - } - - Triplet valid; - if (params->contains("valid-lifetime")) { - valid = getInteger(params, "valid-lifetime"); - } - // Subnet ID is optional. If it is not supplied the value of 0 is used, // which means autogenerate. The value was inserted earlier by calling // SimpleParser6::setAllDefaults. @@ -1195,20 +1126,34 @@ Subnet6ConfigParser::initSubnet(data::ConstElementPtr params, rapid_commit = getBoolean(params, "rapid-commit"); } + // Parse preferred lifetime as it is not parsed by the common function. + Triplet pref; + if (params->contains("preferred-lifetime")) { + pref = getInteger(params, "preferred-lifetime"); + } + + // Create a new subnet. + Subnet6* subnet6 = new Subnet6(addr, len, Triplet(), + Triplet(), + pref, + Triplet(), + subnet_id); + subnet_.reset(subnet6); + std::ostringstream output; output << addr << "/" << static_cast(len) - << " with params t1=" << t1 << ", t2=" - << t2 << ", preferred-lifetime=" << pref - << ", valid-lifetime=" << valid + << " with params t1=" << subnet6->getT1().get() + << ", t2=" << subnet6->getT2().get() + << ", preferred-lifetime=" << pref.get() + << ", valid-lifetime=" << subnet6->getValid().get() << ", rapid-commit is " << (rapid_commit ? "enabled" : "disabled"); LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_NEW_SUBNET6).arg(output.str()); - // Create a new subnet. - Subnet6* subnet6 = new Subnet6(addr, len, t1, t2, pref, valid, - subnet_id); - subnet_.reset(subnet6); + // Parse timers that are common for v4 and v6. + NetworkPtr network = boost::dynamic_pointer_cast(subnet_); + parseCommonTimers(params, network); // Enable or disable Rapid Commit option support for the subnet. if (!rapid_commit.unspecified()) { diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 8258bbe5e4..234075621d 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -449,8 +450,7 @@ private: /// instantiated here and family specific parameters are set) /// 5. Control returns to createSubnet() (step 3) and common parameters /// are set. - -class SubnetConfigParser : public isc::data::SimpleParser { +class SubnetConfigParser : public BaseNetworkParser { public: /// @brief constructor diff --git a/src/lib/dhcpsrv/parsers/shared_network_parser.cc b/src/lib/dhcpsrv/parsers/shared_network_parser.cc index 75086f0c20..283ad68ecf 100644 --- a/src/lib/dhcpsrv/parsers/shared_network_parser.cc +++ b/src/lib/dhcpsrv/parsers/shared_network_parser.cc @@ -4,9 +4,6 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. -#ifndef SHARED_NETWORK_PARSER_H -#define SHARED_NETWORK_PARSER_H - #include #include @@ -15,78 +12,14 @@ #include #include #include -#include #include #include using namespace isc::data; -using namespace isc::util; namespace isc { namespace dhcp { -void -SharedNetworkParser::parseCommonTimers(const ConstElementPtr& shared_network_data, - NetworkPtr& network) { - Triplet t1; - if (shared_network_data->contains("renew-timer")) { - network->setT1(getInteger(shared_network_data, "renew-timer")); - } - - Triplet t2; - if (shared_network_data->contains("rebind-timer")) { - network->setT2(getInteger(shared_network_data, "rebind-timer")); - } - - Triplet valid; - if (shared_network_data->contains("valid-lifetime")) { - network->setValid(getInteger(shared_network_data, "valid-lifetime")); - } -} - -void -SharedNetworkParser::parseTeePercents(const ConstElementPtr& shared_network_data, - NetworkPtr& network) { - bool calculate_tee_times = network->getCalculateTeeTimes(); - if (shared_network_data->contains("calculate-tee-times")) { - bool calculate_tee_times = getBoolean(shared_network_data, "calculate-tee-times"); - network->setCalculateTeeTimes(calculate_tee_times); - } - - Optional t2_percent; - if (shared_network_data->contains("t2-percent")) { - t2_percent = getDouble(shared_network_data, "t2-percent"); - } - - Optional t1_percent; - if (shared_network_data->contains("t1-percent")) { - t1_percent = getDouble(shared_network_data, "t1-percent"); - } - if (calculate_tee_times) { - if (!t2_percent.unspecified() && ((t2_percent.get() <= 0.0) || - (t2_percent.get() >= 1.0))) { - isc_throw(DhcpConfigError, "t2-percent: " << t2_percent.get() - << " is invalid, it must be greater than 0.0 and less than 1.0"); - } - - if (!t1_percent.unspecified() && ((t1_percent.get() <= 0.0) || - (t1_percent.get() >= 1.0))) { - isc_throw(DhcpConfigError, "t1-percent: " << t1_percent.get() - << " is invalid it must be greater than 0.0 and less than 1.0"); - } - - if (!t1_percent.unspecified() && !t2_percent.unspecified() && - (t1_percent.get() >= t2_percent.get())) { - isc_throw(DhcpConfigError, "t1-percent: " << t1_percent.get() - << " is invalid, it must be less than t2-percent: " - << t2_percent.get()); - } - } - - network->setT2Percent(t2_percent); - network->setT1Percent(t1_percent); -} - SharedNetwork4Ptr SharedNetwork4Parser::parse(const data::ConstElementPtr& shared_network_data) { SharedNetwork4Ptr shared_network; @@ -290,4 +223,3 @@ SharedNetwork6Parser::parse(const data::ConstElementPtr& shared_network_data) { } // end of namespace isc::dhcp } // end of namespace isc -#endif // SHARED_NETWORK_PARSER_H diff --git a/src/lib/dhcpsrv/parsers/shared_network_parser.h b/src/lib/dhcpsrv/parsers/shared_network_parser.h index 88e3f4bb0f..d88e1b2d4d 100644 --- a/src/lib/dhcpsrv/parsers/shared_network_parser.h +++ b/src/lib/dhcpsrv/parsers/shared_network_parser.h @@ -12,51 +12,13 @@ #include #include #include +#include namespace isc { namespace dhcp { -/// @brief Common parser for IPv4 and IPv6 shared networks. -/// -/// It contains common functions for parsing the shared networks. -/// DHCPv4 and DHCPv6 specific implementations derive from it. -class SharedNetworkParser : public isc::data::SimpleParser { -protected: - - /// @brief Parses common DHCP timers. - /// - /// The parsed parameters are: - /// - renew-timer, - /// - rebind-timer, - /// - valid-lifetime - /// - /// @param shared_network_data Data element holding shared network - /// configuration to be parsed. - /// @param [out] network Pointer to a network in which parsed data is - /// to be stored. - void parseCommonTimers(const data::ConstElementPtr& shared_network_data, - NetworkPtr& network); - - /// @brief Parses parameters related to "percent" timers settngs. - /// - /// The parsed parameters are: - /// - calculate-tee-times, - /// - t1-percent, - /// - t2-percent. - /// - /// @param shared_network_data Data element holding shared network - /// configuration to be parsed. - /// @param [out] network Pointer to a network in which parsed data is - /// to be stored. - /// - /// @throw DhcpConfigError if configuration of these parameters is - /// invalid. - void parseTeePercents(const data::ConstElementPtr& shared_network_data, - NetworkPtr& network); -}; - /// @brief Implements parser for IPv4 shared networks. -class SharedNetwork4Parser : public SharedNetworkParser { +class SharedNetwork4Parser : public BaseNetworkParser { public: /// @brief Parses shared configuration information for IPv4 shared network. @@ -71,7 +33,7 @@ public: }; /// @brief Implements parser for IPv6 shared networks. -class SharedNetwork6Parser : public SharedNetworkParser { +class SharedNetwork6Parser : public BaseNetworkParser { public: /// @brief Parses shared configuration information for IPv6 shared network.