From: Francis Dupont Date: Sat, 4 Mar 2017 20:57:52 +0000 (+0100) Subject: [5145b] Reverted trac5126 merge X-Git-Tag: trac5152_base~11^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bd67a84ff8af524b00bfed139a2291631e97165e;p=thirdparty%2Fkea.git [5145b] Reverted trac5126 merge --- diff --git a/configure.ac b/configure.ac index 84edaa5fee..2d7b098732 100644 --- a/configure.ac +++ b/configure.ac @@ -201,21 +201,6 @@ for retry in "none" "--std=c++11" "--std=c++0x" "--std=c++1x" "fail"; do [AC_MSG_RESULT([no]) continue]) - AC_MSG_CHECKING(template alias) - feature="template alias" - AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM( - [template - class I { - public: int get() { return i; }; - }; - using Zero = I<0>;], - [Zero Z; - return Z.get();])], - [AC_MSG_RESULT([yes])], - [AC_MSG_RESULT([no]) - continue]) - AC_MSG_CHECKING(lambda support) feature="lambda" AC_COMPILE_IFELSE( @@ -1482,11 +1467,11 @@ AC_ARG_ENABLE(generate_parser, [AC_HELP_STRING([--enable-generate-parser], enable_generate_parser=$enableval, enable_generate_parser=no) # Check if flex is avaible. Flex is not needed for building Kea sources, -# unless you want to regenerate grammars +# unless you want to regenerate grammar in src/lib/eval AC_PROG_LEX # Check if bison is available. Bison is not needed for building Kea sources, -# unless you want to regenerate grammars +# unless you want to regenerate grammar in src/lib/eval AC_PROG_YACC if test "x$enable_generate_parser" != "xno"; then @@ -1500,7 +1485,7 @@ if test "x$enable_generate_parser" != "xno"; then fi # Ok, let's check if we have at least 3.0.0 version of the bison. The code used -# to generate parsers is roughly based on bison 3.0 examples. +# to generate src/lib/eval parser is roughly based on bison 3.0 examples. cat > bisontest.y << EOF %require "3.0.0" %token X @@ -1614,28 +1599,20 @@ AM_COND_IF([HAVE_OPTRESET], [AC_DEFINE([HAVE_OPTRESET], [1], [Check for optreset AC_DEFINE([CONFIG_H_WAS_INCLUDED], [1], [config.h inclusion marker]) -AC_CONFIG_FILES([Makefile - compatcheck/Makefile +AC_CONFIG_FILES([compatcheck/Makefile dns++.pc - doc/Makefile - doc/design/Makefile doc/design/datasrc/Makefile + doc/design/Makefile doc/guide/Makefile + doc/Makefile doc/version.ent - ext/Makefile ext/coroutine/Makefile ext/gtest/Makefile + ext/Makefile m4macros/Makefile + Makefile src/Makefile src/bin/Makefile - src/bin/admin/Makefile - src/bin/admin/kea-admin - src/bin/admin/tests/Makefile - src/bin/admin/tests/cql_tests.sh - src/bin/admin/tests/data/Makefile - src/bin/admin/tests/memfile_tests.sh - src/bin/admin/tests/mysql_tests.sh - src/bin/admin/tests/pgsql_tests.sh src/bin/agent/Makefile src/bin/agent/tests/Makefile src/bin/agent/tests/ca_process_tests.sh @@ -1667,6 +1644,15 @@ AC_CONFIG_FILES([Makefile src/bin/perfdhcp/Makefile src/bin/perfdhcp/tests/Makefile src/bin/perfdhcp/tests/testdata/Makefile + src/bin/admin/Makefile + src/bin/admin/kea-admin + src/bin/admin/tests/Makefile + src/bin/admin/tests/data/Makefile + src/bin/admin/tests/memfile_tests.sh + src/bin/admin/tests/mysql_tests.sh + src/bin/admin/tests/pgsql_tests.sh + src/bin/admin/tests/cql_tests.sh + src/bin/agent/tests/test_libraries.h src/hooks/Makefile src/hooks/dhcp/Makefile src/hooks/dhcp/user_chk/Makefile @@ -1699,8 +1685,6 @@ AC_CONFIG_FILES([Makefile src/lib/dns/gen-rdatacode.py src/lib/dns/tests/Makefile src/lib/dns/tests/testdata/Makefile - src/lib/eval/Makefile - src/lib/eval/tests/Makefile src/lib/exceptions/Makefile src/lib/exceptions/tests/Makefile src/lib/hooks/Makefile @@ -1726,10 +1710,10 @@ AC_CONFIG_FILES([Makefile src/lib/process/spec_config.h.pre src/lib/process/tests/Makefile src/lib/process/testutils/Makefile - src/lib/stats/Makefile - src/lib/stats/tests/Makefile src/lib/testutils/Makefile src/lib/testutils/dhcp_test_lib.sh + src/lib/stats/Makefile + src/lib/stats/tests/Makefile src/lib/util/Makefile src/lib/util/io/Makefile src/lib/util/python/Makefile @@ -1739,10 +1723,11 @@ AC_CONFIG_FILES([Makefile src/lib/util/threads/Makefile src/lib/util/threads/tests/Makefile src/lib/util/unittests/Makefile + src/lib/eval/Makefile + src/lib/eval/tests/Makefile src/share/Makefile src/share/database/Makefile src/share/database/scripts/Makefile - src/share/database/scripts/cql/Makefile src/share/database/scripts/mysql/Makefile src/share/database/scripts/mysql/upgrade_1.0_to_2.0.sh src/share/database/scripts/mysql/upgrade_2.0_to_3.0.sh @@ -1752,16 +1737,17 @@ AC_CONFIG_FILES([Makefile src/share/database/scripts/pgsql/Makefile src/share/database/scripts/pgsql/upgrade_1.0_to_2.0.sh src/share/database/scripts/pgsql/upgrade_2.0_to_3.0.sh + src/share/database/scripts/cql/Makefile tools/Makefile tools/path_replacer.sh ]) -AC_CONFIG_COMMANDS([permissions], [ - chmod +x src/bin/admin/kea-admin + AC_CONFIG_COMMANDS([permissions], [ chmod +x src/bin/dhcp4/tests/dhcp4_process_tests.sh chmod +x src/bin/dhcp6/tests/dhcp6_process_tests.sh chmod +x src/bin/keactrl/keactrl chmod +x src/bin/keactrl/tests/keactrl_tests.sh + chmod +x src/bin/admin/kea-admin chmod +x src/lib/dns/gen-rdatacode.py chmod +x src/lib/log/tests/console_test.sh chmod +x src/lib/log/tests/destination_test.sh diff --git a/src/bin/dhcp4/dhcp4to6_ipc.cc b/src/bin/dhcp4/dhcp4to6_ipc.cc index b219207265..573078cb3e 100644 --- a/src/bin/dhcp4/dhcp4to6_ipc.cc +++ b/src/bin/dhcp4/dhcp4to6_ipc.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 @@ -35,7 +35,7 @@ Dhcp4to6Ipc& Dhcp4to6Ipc::instance() { } void Dhcp4to6Ipc::open() { - uint16_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port(); + uint32_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port(); if (port == 0) { Dhcp4o6IpcBase::close(); return; @@ -45,7 +45,8 @@ void Dhcp4to6Ipc::open() { } int old_fd = socket_fd_; - socket_fd_ = Dhcp4o6IpcBase::open(port, ENDPOINT_TYPE_V4); + socket_fd_ = Dhcp4o6IpcBase::open(static_cast(port), + ENDPOINT_TYPE_V4); if ((old_fd == -1) && (socket_fd_ != old_fd)) { IfaceMgr::instance().addExternalSocket(socket_fd_, Dhcp4to6Ipc::handler); diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index fafa485174..f809d742b6 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -341,9 +341,24 @@ public: cfg->setDeclinePeriod(probation_period); // Set the DHCPv4-over-DHCPv6 interserver port. - uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port"); + // @todo Change for uint16_t + uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port"); cfg->setDhcp4o6Port(dhcp4o6_port); } + +private: + + /// @brief Returns a value converted to uint32_t + /// + /// Instantiation of getIntType() to uint32_t + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return an uint32_t value + uint32_t getUint32(isc::data::ConstElementPtr scope, + const std::string& name) { + return (getIntType(scope, name)); + } }; } // anonymous namespace diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 3f281b6770..61844b2978 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -4557,6 +4557,14 @@ TEST_F(Dhcp4ParserTest, invalidClientClassDictionary) { " } ] \n" "} \n"; + ConstElementPtr json; + ASSERT_NO_THROW(json = parseJSON(config)); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + ASSERT_TRUE(status); + checkResult(status, 1); + EXPECT_THROW(parseDHCP4(config), Dhcp4ParseError); } diff --git a/src/bin/dhcp6/dhcp6to4_ipc.cc b/src/bin/dhcp6/dhcp6to4_ipc.cc index 7ec9397973..72001a4f87 100644 --- a/src/bin/dhcp6/dhcp6to4_ipc.cc +++ b/src/bin/dhcp6/dhcp6to4_ipc.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 @@ -34,7 +34,7 @@ Dhcp6to4Ipc& Dhcp6to4Ipc::instance() { } void Dhcp6to4Ipc::open() { - uint16_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port(); + uint32_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port(); if (port == 0) { Dhcp4o6IpcBase::close(); return; @@ -44,7 +44,8 @@ void Dhcp6to4Ipc::open() { } int old_fd = socket_fd_; - socket_fd_ = Dhcp4o6IpcBase::open(port, ENDPOINT_TYPE_V6); + socket_fd_ = Dhcp4o6IpcBase::open(static_cast(port), + ENDPOINT_TYPE_V6); if ((old_fd == -1) && (socket_fd_ != old_fd)) { IfaceMgr::instance().addExternalSocket(socket_fd_, Dhcp6to4Ipc::handler); diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 7e11de03a6..59b021d65f 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -214,6 +214,18 @@ public: private: + /// @brief Get an uint8_t value + /// + /// Instantiation of getIntType() to uint8_t + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return uint8_t value + /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t + uint8_t getUint8(ConstElementPtr scope, const std::string& name) { + return (getIntType(scope, name)); + } + /// Pointer to the created pool object. isc::dhcp::Pool6Ptr pool_; @@ -535,9 +547,24 @@ public: srv_config->setDeclinePeriod(probation_period); // Set the DHCPv4-over-DHCPv6 interserver port. - uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port"); + // @todo Change for uint16_t + uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port"); srv_config->setDhcp4o6Port(dhcp4o6_port); } + +private: + + /// @brief Returns a value converted to uint32_t + /// + /// Instantiation of getIntType() to uint32_t + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return an uint32_t value + uint32_t getUint32(isc::data::ConstElementPtr scope, + const std::string& name) { + return (getIntType(scope, name)); + } }; } // anonymous namespace diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 1d4ec0bfe2..93f8c047af 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -4969,6 +4969,13 @@ TEST_F(Dhcp6ParserTest, invalidClientClassDictionary) { " } ] \n" "} \n"; + ConstElementPtr json = parseJSON(config); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + ASSERT_TRUE(status); + checkResult(status, 1); + EXPECT_THROW(parseDHCP6(config), Dhcp6ParseError); } diff --git a/src/lib/cc/simple_parser.cc b/src/lib/cc/simple_parser.cc index 240576d3ba..f5d91dc966 100644 --- a/src/lib/cc/simple_parser.cc +++ b/src/lib/cc/simple_parser.cc @@ -74,7 +74,7 @@ SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr p } ConstElementPtr elem = parent->get(name); if (!elem) { - return (parent->getPosition()); + return (data::Element::ZERO_POSITION()); } return (elem->getPosition()); } diff --git a/src/lib/cc/simple_parser.h b/src/lib/cc/simple_parser.h index 3ff60b7bb8..a7352a5ed3 100644 --- a/src/lib/cc/simple_parser.h +++ b/src/lib/cc/simple_parser.h @@ -104,9 +104,9 @@ class SimpleParser { /// @brief Utility method that returns position of an element /// - /// It's mostly useful for logging. If the element is missing - /// the parent position is returned or ZERO_POSITION if parent - /// is null. + /// It's mostly useful for logging. When any necessary parameter is + /// missing (either parent is null or it doesn't contain specified + /// name) ZERO_POSITION is returned. /// /// @param name position of that element will be returned /// @param parent parent element (optional) @@ -203,44 +203,6 @@ protected: << "' (" << getPosition(name, scope) << ")"); } } - - /// @brief Returns a value converted to uint32_t - /// - /// Instantiation of getIntType() to uint32_t - /// - /// @param scope specified parameter will be extracted from this scope - /// @param name name of the parameter - /// @return an uint32_t value - /// @throw isc::dhcp::DhcpConfigError when it is not an uint32_t - uint32_t getUint32(isc::data::ConstElementPtr scope, - const std::string& name) { - return (getIntType(scope, name)); - } - - /// @brief Returns a value converted to uint16_t - /// - /// Instantiation of getIntType() to uint16_t - /// - /// @param scope specified parameter will be extracted from this scope - /// @param name name of the parameter - /// @return an uint16_t value - /// @throw isc::dhcp::DhcpConfigError when it is not an uint16_t - uint16_t getUint16(isc::data::ConstElementPtr scope, - const std::string& name) { - return (getIntType(scope, name)); - } - - /// @brief Get an uint8_t value - /// - /// Instantiation of getIntType() to uint8_t - /// - /// @param scope specified parameter will be extracted from this scope - /// @param name name of the parameter - /// @return uint8_t value - /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t - uint8_t getUint8(ConstElementPtr scope, const std::string& name) { - return (getIntType(scope, name)); - } }; }; diff --git a/src/lib/cc/tests/simple_parser_unittest.cc b/src/lib/cc/tests/simple_parser_unittest.cc index 0a8bf57988..dfe384c990 100644 --- a/src/lib/cc/tests/simple_parser_unittest.cc +++ b/src/lib/cc/tests/simple_parser_unittest.cc @@ -58,9 +58,14 @@ public: class SimpleParserClassTest : public SimpleParser { public: - - /// Make getUint8 public - using SimpleParser::getUint8; + /// @brief Instantiation of getIntType for uint8_t + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter for error report + /// @return an uint8_t value + uint8_t getUint8(ConstElementPtr scope, const std::string& name) { + return (getIntType(scope, name)); + } /// @brief Instantiation of getAndConvert /// diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc index a16eb6f4cc..2194cffc31 100644 --- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc +++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc @@ -62,87 +62,86 @@ void ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary, ConstElementPtr class_def_cfg, uint16_t family) { - // name is now mandatory - std::string name = getString(class_def_cfg, "name"); - if (name.empty()) { - isc_throw(DhcpConfigError, - "not empty parameter 'name' is required " - << getPosition("name", class_def_cfg) << ")"); - } - // Parse matching expression - ExpressionPtr match_expr; - ConstElementPtr test = class_def_cfg->get("test"); - if (test) { - ExpressionParser parser; - parser.parse(match_expr, test, family); - } + try { + std::string name; + std::string next_server_txt = "0.0.0.0"; + std::string sname; + std::string filename; + ExpressionPtr match_expr; + CfgOptionPtr options(new CfgOption()); + + // Parse the elements that make up the client class definition. + BOOST_FOREACH(ConfigPair param, class_def_cfg->mapValue()) { + std::string entry(param.first); + ConstElementPtr value(param.second); + + if (entry == "name") { + name = value->stringValue(); + + } else if (entry == "test") { + ExpressionParser parser; + parser.parse(match_expr, value, family); + + } else if (entry == "option-data") { + OptionDataListParser opts_parser(family); + opts_parser.parse(options, value); + + } else if (entry == "next-server") { + next_server_txt = value->stringValue(); + + } else if (entry == "server-hostname") { + sname = value->stringValue(); + + } else if (entry == "boot-file-name") { + filename = value->stringValue(); + + } else { + isc_throw(DhcpConfigError, "invalid parameter '" << entry + << "' (" << value->getPosition() << ")"); + } + } - // Parse option data - CfgOptionPtr options(new CfgOption()); - ConstElementPtr option_data = class_def_cfg->get("option-data"); - if (option_data) { - OptionDataListParser opts_parser(family); - opts_parser.parse(options, option_data); - } + // name is now mandatory + if (name.empty()) { + isc_throw(DhcpConfigError, + "not empty parameter 'name' is required"); + } - // Let's try to parse the next-server field - IOAddress next_server("0.0.0.0"); - if (class_def_cfg->contains("next-server")) { - std::string next_server_txt = getString(class_def_cfg, "next-server"); + // Let's parse the next-server field + IOAddress next_server("0.0.0.0"); try { next_server = IOAddress(next_server_txt); } catch (const IOError& ex) { - isc_throw(DhcpConfigError, - "Invalid next-server value specified: '" - << next_server_txt << "' (" - << getPosition("next-server", class_def_cfg) << ")"); + isc_throw(DhcpConfigError, "Invalid next-server value specified: '" + << next_server_txt); } if (next_server.getFamily() != AF_INET) { isc_throw(DhcpConfigError, "Invalid next-server value: '" - << next_server_txt - << "', must be IPv4 address (" - << getPosition("next-server", class_def_cfg) << ")"); + << next_server_txt << "', must be IPv4 address"); } if (next_server.isV4Bcast()) { isc_throw(DhcpConfigError, "Invalid next-server value: '" - << next_server_txt - << "', must not be a broadcast (" - << getPosition("next-server", class_def_cfg) << ")"); + << next_server_txt << "', must not be a broadcast"); } - } - - // Let's try to parse server-hostname - std::string sname; - if (class_def_cfg->contains("server-hostname")) { - sname = getString(class_def_cfg, "server-hostname"); + // Let's try to parse server-hostname if (sname.length() >= Pkt4::MAX_SNAME_LEN) { isc_throw(DhcpConfigError, "server-hostname must be at most " << Pkt4::MAX_SNAME_LEN - 1 << " bytes long, it is " - << sname.length() << " (" - << getPosition("server-hostname", class_def_cfg) << ")"); + << sname.length()); } - } - - // Let's try to parse boot-file-name - std::string filename; - if (class_def_cfg->contains("boot-file-name")) { - filename = getString(class_def_cfg, "boot-file-name"); + // Let's try to parse boot-file-name if (filename.length() > Pkt4::MAX_FILE_LEN) { isc_throw(DhcpConfigError, "boot-file-name must be at most " << Pkt4::MAX_FILE_LEN - 1 << " bytes long, it is " - << filename.length() << " (" - << getPosition("boot-file-name", class_def_cfg) << ")"); + << filename.length()); } - } - - // Add the client class definition - try { + // Add the client class definition class_dictionary->addClass(name, match_expr, options, next_server, sname, filename); } catch (const std::exception& ex) { diff --git a/src/lib/dhcpsrv/parsers/dbaccess_parser.h b/src/lib/dhcpsrv/parsers/dbaccess_parser.h index 4e662d43f1..5460504d79 100644 --- a/src/lib/dhcpsrv/parsers/dbaccess_parser.h +++ b/src/lib/dhcpsrv/parsers/dbaccess_parser.h @@ -43,7 +43,7 @@ public: /// @brief Constructor /// /// @param db_type Specifies database type (lease or hosts) - explicit DbAccessParser(DBType db_type); + DbAccessParser(DBType db_type); /// The destructor. virtual ~DbAccessParser() diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index cbe8d4b6db..0073a35d17 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -295,7 +295,10 @@ OptionDataParser::extractSpace(ConstElementPtr parent) const { } } catch (std::exception& ex) { - // Append position of the option space parameter. + // Append position of the option space parameter. Note, that in the case + // when 'space' was not specified a default value will be used and we + // should never get here. Therefore, it is ok to call getPosition for + // the space parameter here as this parameter will always be specified. isc_throw(DhcpConfigError, ex.what() << " (" << getPosition("space", parent) << ")"); } @@ -597,31 +600,57 @@ RelayInfoParser::RelayInfoParser(const Option::Universe& family) : family_(family) { }; -// Can't use a constructor as a function -namespace { -IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); } -}; - -IOAddress -RelayInfoParser::getIOAddress(ConstElementPtr scope, - const std::string& name) { - return (getAndConvert(scope, name, "address")); -} - void RelayInfoParser::parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg, ConstElementPtr relay_info) { - // There is only one parameter which is mandatory - IOAddress ip = getIOAddress(relay_info, "ip-address"); - - // Check if the address family matches. - if ((ip.isV4() && family_ != Option::V4) || - (ip.isV6() && family_ != Option::V6) ) { - isc_throw(DhcpConfigError, "ip-address field " << ip.toText() - << " does not have IP address of expected family type: " - << (family_ == Option::V4 ? "IPv4" : "IPv6") - << " (" << getPosition("ip-address", relay_info) << ")"); + // Let's start with some sanity checks. + if (!relay_info || !cfg) { + isc_throw(DhcpConfigError, "Logic error: RelayInfoParser::parse() called " + "with at least one NULL parameter."); + } + + if (relay_info->getType() != Element::map) { + isc_throw(DhcpConfigError, "Configuration error: RelayInfoParser::parse() " + "called with non-map parameter"); + } + + // Now create the default value. + isc::asiolink::IOAddress ip(family_ == Option::V4 ? IOAddress::IPV4_ZERO_ADDRESS() + : IOAddress::IPV6_ZERO_ADDRESS()); + + // Now iterate over all parameters. Currently there's only one supported + // parameter, so it should be an easy thing to check. + bool ip_address_specified = false; + BOOST_FOREACH(ConfigPair param, relay_info->mapValue()) { + if (param.first == "ip-address") { + ip_address_specified = true; + + try { + ip = asiolink::IOAddress(param.second->stringValue()); + } catch (...) { + isc_throw(DhcpConfigError, "Failed to parse ip-address " + "value: " << param.second + << " (" << param.second->getPosition() << ")"); + } + + // Check if the address family matches. + if ( (ip.isV4() && family_ != Option::V4) || + (ip.isV6() && family_ != Option::V6) ) { + isc_throw(DhcpConfigError, "ip-address field " << ip.toText() + << " does not have IP address of expected family type: " + << (family_ == Option::V4 ? "IPv4" : "IPv6") + << " (" << param.second->getPosition() << ")"); + } + } else { + isc_throw(NotImplemented, + "parser error: RelayInfoParser parameter not supported: " + << param.second); + } + } + + if (!ip_address_specified) { + isc_throw(DhcpConfigError, "'relay' specified, but mandatory 'ip-address' " + "paramter in it is missing"); } // Ok, we're done with parsing. Let's store the result in the structure @@ -885,10 +914,17 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) { try { std::string hr_mode = getString(params, "reservation-mode"); subnet_->setHostReservationMode(hrModeFromText(hr_mode)); - } catch (const BadValue& ex) { - isc_throw(DhcpConfigError, "Failed to process specified value " + } catch (const BadValue& ex) { + ConstElementPtr mode = params->get("reservation-mode"); + string pos; + if (mode) { + pos = mode->getPosition().str(); + } else { + pos = params->getPosition().str(); + } + isc_throw(DhcpConfigError, "Failed to process specified value " " of reservation-mode parameter: " << ex.what() - << "(" << getPosition("reservation-mode", params) << ")"); + << "(" << pos << ")"); } // Try setting up client class. @@ -907,6 +943,17 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) { //**************************** D2ClientConfigParser ********************** +uint32_t +D2ClientConfigParser::getUint32(ConstElementPtr scope, + const std::string& name) { + return (getIntType(scope, name)); +} + +// Can't use a constructor as a function +namespace { +IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); } +}; + IOAddress D2ClientConfigParser::getIOAddress(ConstElementPtr scope, const std::string& name) { diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 6ddf10bf5d..d84d976ec7 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -365,7 +365,7 @@ public: /// @brief Constructor. /// /// @param address_family Address family: @c AF_INET or @c AF_INET6. - explicit OptionDataParser(const uint16_t address_family); + OptionDataParser(const uint16_t address_family); /// @brief Parses ElementPtr containing option definition /// @@ -477,7 +477,7 @@ public: /// @brief Constructor. /// /// @param address_family Address family: @c AF_INET or AF_INET6 - explicit OptionDataListParser(const uint16_t address_family); + OptionDataListParser(const uint16_t address_family); /// @brief Parses a list of options, instantiates them and stores in cfg /// @@ -620,7 +620,7 @@ public: /// @brief constructor /// @param family specifies protocol family (IPv4 or IPv6) - explicit RelayInfoParser(const isc::dhcp::Option::Universe& family); + RelayInfoParser(const isc::dhcp::Option::Universe& family); /// @brief parses the actual relay parameters /// @@ -632,18 +632,7 @@ public: void parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg, isc::data::ConstElementPtr relay_info); -private: - - /// @brief Returns a value converted to IOAddress - /// - /// Instantiation of getAndConvert() to IOAddress - /// - /// @param scope specified parameter will be extracted from this scope - /// @param name name of the parameter - /// @return an IOAddress value - isc::asiolink::IOAddress - getIOAddress(isc::data::ConstElementPtr scope, const std::string& name); - +protected: /// Protocol family (IPv4 or IPv6) Option::Universe family_; }; @@ -675,7 +664,7 @@ public: /// @brief constructor /// /// @param family address family: @c AF_INET or @c AF_INET6 - explicit SubnetConfigParser(uint16_t family); + SubnetConfigParser(uint16_t family); /// @brief virtual destructor (does nothing) virtual ~SubnetConfigParser() { } @@ -787,6 +776,16 @@ public: private: + /// @brief Returns a value converted to uint32_t + /// + /// Instantiation of getIntType() to uint32_t + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return an uint32_t value + uint32_t + getUint32(isc::data::ConstElementPtr scope, const std::string& name); + /// @brief Returns a value converted to IOAddress /// /// Instantiation of getAndConvert() to IOAddress diff --git a/src/lib/dhcpsrv/parsers/duid_config_parser.cc b/src/lib/dhcpsrv/parsers/duid_config_parser.cc index 00116d8d8e..980bd298b8 100644 --- a/src/lib/dhcpsrv/parsers/duid_config_parser.cc +++ b/src/lib/dhcpsrv/parsers/duid_config_parser.cc @@ -24,62 +24,107 @@ namespace isc { namespace dhcp { void -DUIDConfigParser::parse(const CfgDUIDPtr& cfg, - isc::data::ConstElementPtr duid_configuration) { - - std::string param; - try { - param = "type"; - std::string duid_type = getString(duid_configuration, "type"); - // Map DUID type represented as text into numeric value. - DUID::DUIDType numeric_type = DUID::DUID_UNKNOWN; - if (duid_type == "LLT") { - numeric_type = DUID::DUID_LLT; - } else if (duid_type == "EN") { - numeric_type = DUID::DUID_EN; - } else if (duid_type == "LL") { - numeric_type = DUID::DUID_LL; - } else { - isc_throw(BadValue, "unsupported DUID type '" - << duid_type << "'. Expected: LLT, EN or LL"); +DUIDConfigParser::parse(const CfgDUIDPtr& cfg, isc::data::ConstElementPtr duid_configuration) { + if (!cfg) { + isc_throw(DhcpConfigError, "Must provide valid pointer to cfg when parsing duid"); + } + + bool type_present = false; + BOOST_FOREACH(ConfigPair element, duid_configuration->mapValue()) { + try { + if (element.first == "type") { + type_present = true; + setType(cfg, element.second->stringValue()); + } else if (element.first == "identifier") { + setIdentifier(cfg, element.second->stringValue()); + } else if (element.first == "htype") { + setHType(cfg, element.second->intValue()); + } else if (element.first == "time") { + setTime(cfg, element.second->intValue()); + } else if (element.first == "enterprise-id") { + setEnterpriseId(cfg, element.second->intValue()); + } else if (element.first == "persist") { + setPersist(cfg, element.second->boolValue()); + } else { + isc_throw(DhcpConfigError, "unsupported configuration " + "parameter '" << element.first << "'"); + } + } catch (const std::exception& ex) { + // Append position. + isc_throw(DhcpConfigError, ex.what() << " (" + << element.second->getPosition() << ")"); } + } - cfg->setType(static_cast(numeric_type)); + // "type" is mandatory + if (!type_present) { + isc_throw(DhcpConfigError, "mandatory parameter \"type\" not specified" + " for the DUID configuration (" + << duid_configuration->getPosition() << ")"); + } - param = "identifier"; - if (duid_configuration->contains(param)) { - cfg->setIdentifier(getString(duid_configuration, param)); - } + LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIGURE_SERVERID); +} - param = "htype"; - if (duid_configuration->contains(param)) { - cfg->setHType(getUint16(duid_configuration, param)); - } +void +DUIDConfigParser::setType(const CfgDUIDPtr& cfg, const std::string& duid_type) const { + // Map DUID type represented as text into numeric value. + DUID::DUIDType numeric_type = DUID::DUID_UNKNOWN; + if (duid_type == "LLT") { + numeric_type = DUID::DUID_LLT; + } else if (duid_type == "EN") { + numeric_type = DUID::DUID_EN; + } else if (duid_type == "LL") { + numeric_type = DUID::DUID_LL; + } else { + isc_throw(DhcpConfigError, "unsupported DUID type '" + << duid_type << "'. Expected: LLT, EN or LL"); + } - param = "time"; - if (duid_configuration->contains(param)) { - cfg->setTime(getUint32(duid_configuration, param)); - } + cfg->setType(static_cast(numeric_type)); +} - param = "enterprise-id"; - if (duid_configuration->contains(param)) { - cfg->setEnterpriseId(getUint32(duid_configuration, param)); - } +void +DUIDConfigParser::setIdentifier(const CfgDUIDPtr& cfg, const std::string& identifier) const { + cfg->setIdentifier(identifier); +} - param = "persist"; - if (duid_configuration->contains(param)) { - cfg->setPersist(getBoolean(duid_configuration, param)); - } - } catch (const DhcpConfigError&) { - throw; - } catch (const std::exception& ex) { - // Append position. - isc_throw(DhcpConfigError, ex.what() << " (" - << getPosition(param, duid_configuration) << ")"); - } +void +DUIDConfigParser::setHType(const CfgDUIDPtr& cfg, const int64_t htype) const { + checkRange("htype", htype); + cfg->setHType(static_cast(htype)); +} - LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIGURE_SERVERID); +void +DUIDConfigParser::setTime(const CfgDUIDPtr& cfg, const int64_t new_time) const { + checkRange("time", new_time); + cfg->setTime(static_cast(new_time)); +} + +void +DUIDConfigParser::setEnterpriseId(const CfgDUIDPtr& cfg, const int64_t enterprise_id) const { + checkRange("enterprise-id", enterprise_id); + cfg->setEnterpriseId(static_cast(enterprise_id)); } +void +DUIDConfigParser::setPersist(const CfgDUIDPtr& cfg, const bool persist) { + cfg->setPersist(persist); +} + +template +void +DUIDConfigParser::checkRange(const std::string& parameter_name, + const int64_t parameter_value) const { + if ((parameter_value < 0) || + (parameter_value > std::numeric_limits::max())) { + isc_throw(DhcpConfigError, "out of range value '" << parameter_value + << "' specified for parameter '" << parameter_name + << "'; expected value in range of [0.." + << std::numeric_limits::max() << "]"); + } +} + + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/parsers/duid_config_parser.h b/src/lib/dhcpsrv/parsers/duid_config_parser.h index 8780031147..e745a050ff 100644 --- a/src/lib/dhcpsrv/parsers/duid_config_parser.h +++ b/src/lib/dhcpsrv/parsers/duid_config_parser.h @@ -33,6 +33,57 @@ public: /// /// @throw DhcpConfigError If the configuration is invalid. void parse(const CfgDUIDPtr& cfg, isc::data::ConstElementPtr duid_configuration); +private: + + /// @brief Validate and set DUID type. + /// + /// @param cfg parsed information will be stored here + /// @param duid_type DUID type in textual format. + void setType(const CfgDUIDPtr& cfg, const std::string& duid_type) const; + + /// @brief Validate and set identifier. + /// + /// @param cfg parsed information will be stored here + /// @param identifier Identifier. + void setIdentifier(const CfgDUIDPtr& cfg, const std::string& identifier) const; + + /// @brief Validate and set hardware type. + /// + /// @param cfg parsed information will be stored here + /// @param htype Hardware type. + void setHType(const CfgDUIDPtr& cfg, const int64_t htype) const; + + /// @brief Validate and set time value. + /// + /// @param cfg parsed information will be stored here + /// @param new_time Time value to be used for DUID. + void setTime(const CfgDUIDPtr& cfg, const int64_t new_time) const; + + /// @brief Validate and set enterprise id. + /// + /// @param cfg parsed information will be stored here + /// @param enterprise_id Enterprise id. + void setEnterpriseId(const CfgDUIDPtr& cfg, const int64_t enterprise_id) const; + + /// @brief Set persistence flag. + /// + /// @param cfg parsed information will be stored here + /// @param persist A boolean value indicating if the server + /// identifier should be stored on the disk (if true) or + /// not (if false). + void setPersist(const CfgDUIDPtr& cfg, const bool persist); + + /// @brief Verifies if the specified parameter is in range. + /// + /// Each numeric value must be in range of [0 .. max_value], where + /// max_value is a maximum value for the numeric type used for this + /// parameter. + /// + /// @param parameter_name Parameter name. + /// @tparam Numeric type of the specified parameter. + template + void checkRange(const std::string& parameter_name, + const int64_t parameter_value) const; }; } diff --git a/src/lib/dhcpsrv/parsers/expiration_config_parser.cc b/src/lib/dhcpsrv/parsers/expiration_config_parser.cc index 94e30e9580..170e95abba 100644 --- a/src/lib/dhcpsrv/parsers/expiration_config_parser.cc +++ b/src/lib/dhcpsrv/parsers/expiration_config_parser.cc @@ -20,46 +20,43 @@ void ExpirationConfigParser::parse(ConstElementPtr expiration_config) { CfgExpirationPtr cfg = CfgMgr::instance().getStagingCfg()->getCfgExpiration(); - std::string param; + BOOST_FOREACH(ConfigPair config_element, expiration_config->mapValue()) { - try { - param = "reclaim-timer-wait-time"; - if (expiration_config->contains(param)) { - cfg->setReclaimTimerWaitTime(getInteger(expiration_config, param)); - } + // Get parameter name and value. + std::string param_name = config_element.first; + ConstElementPtr param_value = config_element.second; - param = "flush-reclaimed-timer-wait-time"; - if (expiration_config->contains(param)) { - cfg->setFlushReclaimedTimerWaitTime(getInteger(expiration_config, - param)); - } + try { + // Set configuration parameters. + if (param_name == "reclaim-timer-wait-time") { + cfg->setReclaimTimerWaitTime(param_value->intValue()); - param = "hold-reclaimed-time"; - if (expiration_config->contains(param)) { - cfg->setHoldReclaimedTime(getInteger(expiration_config, param)); - } + } else if (param_name == "flush-reclaimed-timer-wait-time") { + cfg->setFlushReclaimedTimerWaitTime(param_value->intValue()); - param = "max-reclaim-leases"; - if (expiration_config->contains(param)) { - cfg->setMaxReclaimLeases(getInteger(expiration_config, param)); - } + } else if (param_name == "hold-reclaimed-time") { + cfg->setHoldReclaimedTime(param_value->intValue()); - param = "max-reclaim-time"; - if (expiration_config->contains(param)) { - cfg->setMaxReclaimTime(getInteger(expiration_config, param)); - } + } else if (param_name == "max-reclaim-leases") { + cfg->setMaxReclaimLeases(param_value->intValue()); + + } else if (param_name == "max-reclaim-time") { + cfg->setMaxReclaimTime(param_value->intValue()); + + } else if (param_name == "unwarned-reclaim-cycles") { + cfg->setUnwarnedReclaimCycles(param_value->intValue()); + + } else { + isc_throw(DhcpConfigError, "unsupported parameter '" + << param_name << "'"); + } - param = "unwarned-reclaim-cycles"; - if (expiration_config->contains(param)) { - cfg->setUnwarnedReclaimCycles( - getInteger(expiration_config, param)); + } catch (const std::exception& ex) { + // Append position of the configuration parameter to the error + // message. + isc_throw(DhcpConfigError, ex.what() << " (" + << param_value->getPosition() << ")"); } - } catch (const DhcpConfigError&) { - throw; - } catch (const std::exception& ex) { - // Append position of the configuration parameter to the error message. - isc_throw(DhcpConfigError, ex.what() << " (" - << getPosition(param, expiration_config) << ")"); } } diff --git a/src/lib/dhcpsrv/parsers/ifaces_config_parser.h b/src/lib/dhcpsrv/parsers/ifaces_config_parser.h index d23a716dc5..a4c346e36d 100644 --- a/src/lib/dhcpsrv/parsers/ifaces_config_parser.h +++ b/src/lib/dhcpsrv/parsers/ifaces_config_parser.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 @@ -29,7 +29,7 @@ public: /// @brief Constructor /// /// @param protocol AF_INET for DHCPv4 and AF_INET6 for DHCPv6. - explicit IfacesConfigParser(const uint16_t protocol); + IfacesConfigParser(const uint16_t protocol); /// @brief Parses content of the "interfaces-config". /// diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 88db49f7af..6c12548dfd 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -507,7 +507,7 @@ public: /// this socket is bound and connected to this port and port + 1 /// /// @param port port and port + 1 to use - void setDhcp4o6Port(uint16_t port) { + void setDhcp4o6Port(uint32_t port) { /// @todo: Port is supposed to be uint16_t, not uint32_t dhcp4o6_port_ = port; } @@ -516,7 +516,7 @@ public: /// /// See @ref setDhcp4o6Port for brief discussion. /// @return value of DHCP4o6 IPC port - uint16_t getDhcp4o6Port() { + uint32_t getDhcp4o6Port() { return (dhcp4o6_port_); } @@ -624,7 +624,7 @@ private: /// /// DHCPv4-over-DHCPv6 uses a UDP socket for interserver communication, /// this socket is bound and connected to this port and port + 1 - uint16_t dhcp4o6_port_; + uint32_t dhcp4o6_port_; D2ClientConfigPtr d2_client_config_; }; diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc index 13c0217350..17eeb9a97f 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc @@ -459,6 +459,20 @@ TEST_F(ClientClassDefParserTest, blankClassName) { DhcpConfigError); } + +// Verifies that a class with an unknown element, fails to parse. +TEST_F(ClientClassDefParserTest, unknownElement) { + std::string cfg_text = + "{ \n" + " \"name\": \"one\", \n" + " \"bogus\": \"bad\" \n" + "} \n"; + + ClientClassDefPtr cclass; + ASSERT_THROW(cclass = parseClientClassDef(cfg_text, AF_INET), + DhcpConfigError); +} + // Verifies that a class with an invalid expression, fails to parse. TEST_F(ClientClassDefParserTest, invalidExpression) { std::string cfg_text = @@ -551,6 +565,22 @@ TEST_F(ClientClassDefListParserTest, duplicateClass) { DhcpConfigError); } +// Verifies that a class list containing an invalid class entry, fails to +// parse. +TEST_F(ClientClassDefListParserTest, invalidClass) { + std::string cfg_text = + "[ \n" + " { \n" + " \"name\": \"one\", \n" + " \"bogus\": \"bad\" \n" + " } \n" + "] \n"; + + ClientClassDictionaryPtr dictionary; + ASSERT_THROW(dictionary = parseClientClassDefList(cfg_text, AF_INET6), + DhcpConfigError); +} + // Test verifies that without any class specified, the fixed fields have their // default, empty value. // @todo same with AF_INET6 diff --git a/src/lib/dhcpsrv/tests/expiration_config_parser_unittest.cc b/src/lib/dhcpsrv/tests/expiration_config_parser_unittest.cc index fd76507232..53c5953b42 100644 --- a/src/lib/dhcpsrv/tests/expiration_config_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/expiration_config_parser_unittest.cc @@ -221,6 +221,15 @@ TEST_F(ExpirationConfigParserTest, otherParameters) { EXPECT_EQ(20, cfg->getUnwarnedReclaimCycles()); } +// This test verifies that the exception is thrown if unsupported +// parameter is specified. +TEST_F(ExpirationConfigParserTest, invalidParameter) { + addParam("reclaim-timer-wait-time", 20); + addParam("invalid-parameter", 20); + + EXPECT_THROW(renderConfig(), DhcpConfigError); +} + // This test verifies that negative parameter values are not allowed. TEST_F(ExpirationConfigParserTest, outOfRangeValues) { testOutOfRange("reclaim-timer-wait-time",