From: Francis Dupont Date: Thu, 26 Jan 2017 19:34:45 +0000 (+0100) Subject: [5123] Moved DhcpConfigError, added new templates, updated simpleparser code X-Git-Tag: trac5126_base~2^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6bde0a8daaa155a21e66dad0af6a041ee83c1e75;p=thirdparty%2Fkea.git [5123] Moved DhcpConfigError, added new templates, updated simpleparser code --- diff --git a/src/lib/cc/Makefile.am b/src/lib/cc/Makefile.am index 02c46d179a..cbce1325e1 100644 --- a/src/lib/cc/Makefile.am +++ b/src/lib/cc/Makefile.am @@ -6,6 +6,7 @@ AM_CXXFLAGS = $(KEA_CXXFLAGS) lib_LTLIBRARIES = libkea-cc.la libkea_cc_la_SOURCES = data.cc data.h +libkea_cc_la_SOURCES += dhcp_config_error.h libkea_cc_la_SOURCES += command_interpreter.cc command_interpreter.h libkea_cc_la_SOURCES += simple_parser.cc simple_parser.h @@ -17,7 +18,7 @@ libkea_cc_la_LDFLAGS = -no-undefined -version-info 1:0:0 # Since data.h is now used in the hooks interface, it needs to be # installed on target system. libkea_cc_includedir = $(pkgincludedir)/cc -libkea_cc_include_HEADERS = data.h +libkea_cc_include_HEADERS = data.h dhcp_config_error.h EXTRA_DIST = cc.dox diff --git a/src/lib/cc/dhcp_config_error.h b/src/lib/cc/dhcp_config_error.h new file mode 100644 index 0000000000..6b205fbcab --- /dev/null +++ b/src/lib/cc/dhcp_config_error.h @@ -0,0 +1,33 @@ +// Copyright (C) 2017 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 DHCP_CONFIG_ERROR_H +#define DHCP_CONFIG_ERROR_H + +#include + +namespace isc { +namespace dhcp { + +/// An exception that is thrown if an error occurs while configuring +/// DHCP server. +class DhcpConfigError : public isc::Exception { +public: + + /// @brief constructor + /// + /// @param file name of the file, where exception occurred + /// @param line line of the file, where exception occurred + /// @param what text description of the issue that caused exception + DhcpConfigError(const char* file, size_t line, const char* what) + : isc::Exception(file, line, what) {} +}; + +}; // end of isc::dhcp namespace +}; // end of isc namespace + +#endif // DHCP_CONFIG_ERROR_H + diff --git a/src/lib/cc/simple_parser.cc b/src/lib/cc/simple_parser.cc index 6c0d74a474..6e6dfd26a4 100644 --- a/src/lib/cc/simple_parser.cc +++ b/src/lib/cc/simple_parser.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2017 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 @@ -11,6 +11,7 @@ #include using namespace std; +using isc::dhcp::DhcpConfigError; namespace isc { namespace data { @@ -19,12 +20,14 @@ std::string SimpleParser::getString(isc::data::ConstElementPtr scope, const std::string& name) { ConstElementPtr x = scope->get(name); if (!x) { - isc_throw(BadValue, "String parameter " << name << " not found" - << "(" << scope->getPosition() << ")"); + isc_throw(DhcpConfigError, + "missing parameter '" << name << "' (" + << scope->getPosition() << ")"); } if (x->getType() != Element::string) { - isc_throw(BadValue, "Element " << name << " found, but is not a string" - << "(" << x->getPosition() << ")"); + isc_throw(DhcpConfigError, + "invalid type specified for parameter '" << name + << "' (" << x->getPosition() << ")"); } return (x->stringValue()); @@ -34,12 +37,14 @@ int64_t SimpleParser::getInteger(isc::data::ConstElementPtr scope, const std::string& name) { ConstElementPtr x = scope->get(name); if (!x) { - isc_throw(BadValue, "Integer parameter " << name << " not found " - << "(" << scope->getPosition() << ")"); + isc_throw(DhcpConfigError, + "missing parameter '" << name << "' (" + << scope->getPosition() << ")"); } if (x->getType() != Element::integer) { - isc_throw(BadValue, "Element " << name << " found, but is not an integer" - << "(" << x->getPosition() << ")"); + isc_throw(DhcpConfigError, + "invalid type specified for parameter '" << name + << "' (" << x->getPosition() << ")"); } return (x->intValue()); @@ -49,13 +54,14 @@ bool SimpleParser::getBoolean(isc::data::ConstElementPtr scope, const std::string& name) { ConstElementPtr x = scope->get(name); if (!x) { - isc_throw(BadValue, "Boolean element " << name << " not found " - << "(" << scope->getPosition() << ")"); - + isc_throw(DhcpConfigError, + "missing parameter '" << name << "' (" + << scope->getPosition() << ")"); } if (x->getType() != Element::boolean) { - isc_throw(BadValue, "Element " << name << " found, but is not a boolean" - << "(" << x->getPosition() << ")"); + isc_throw(DhcpConfigError, + "invalid type specified for parameter '" << name + << "' (" << x->getPosition() << ")"); } return (x->boolValue()); @@ -112,7 +118,8 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope, } else if (def_value.value_ == string("false")) { bool_value = false; } else { - isc_throw(BadValue, "Internal error. Boolean value specified as " + isc_throw(DhcpConfigError, + "Internal error. Boolean value specified as " << def_value.value_ << ", expected true or false"); } x.reset(new BoolElement(bool_value, pos)); @@ -125,7 +132,8 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope, } default: // No default values for null, list or map - isc_throw(BadValue, "Internal error. Incorrect default value type."); + isc_throw(DhcpConfigError, + "Internal error. Incorrect default value type."); } // ... and insert it into the provided Element tree. diff --git a/src/lib/cc/simple_parser.h b/src/lib/cc/simple_parser.h index 6d14cd0701..6f758f98a8 100644 --- a/src/lib/cc/simple_parser.h +++ b/src/lib/cc/simple_parser.h @@ -8,6 +8,7 @@ #define SIMPLE_PARSER_H #include +#include #include #include #include @@ -148,6 +149,59 @@ protected: static bool getBoolean(isc::data::ConstElementPtr scope, const std::string& name); + /// @brief Returns an integer value with range checking from a scope + /// + /// This template should be instantied in parsers when useful + /// + /// @tparam int_type the integer type e.g. uint32_t + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter for error report + /// @return a value of int_type + template int_type + getIntType(isc::data::ConstElementPtr scope, + const std::string& name) { + int64_t val_int = getInteger(scope, name); + if ((val_int < std::numeric_limits::min()) || + (val_int > std::numeric_limits::max())) { + isc_throw(isc::dhcp::DhcpConfigError, + "out of range value (" << val_int + << ") specified for parameter '" << name + << "' (" << getPosition(name, scope) << ")"); + } + return (static_cast(val_int)); + } + + /// @brief Returns a converted value from a scope + /// + /// This template should be instantied in parsers when useful + /// + /// @tparam target_type the type of the result + /// @tparam convert the conversion function std::string -> target_type + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter for error report + /// @param type_name name of target_type for error report + /// @param value value of the parameter + /// @return a converted value of target_type + /// @throw isc::data::TypeError when the value is not an integer + /// @throw exception_type when the value cannot be converted + template target_type + getAndConvert(isc::data::ConstElementPtr scope, + const std::string& name, + const std::string& type_name) { + std::string str = getString(scope, name); + try { + return (convert(str)); + } catch (const std::exception&) { + isc_throw(isc::dhcp::DhcpConfigError, + "invalid " << type_name << " (" << str + << ") specified for parameter '" << name + << "' (" << getPosition(name, scope) << ")"); + } + } + + /// @todo remove this when they'll be no longer used. + /// @brief Returns an integer value with range checking /// /// This template should be instantied in parsers when useful diff --git a/src/lib/cc/tests/simple_parser_unittest.cc b/src/lib/cc/tests/simple_parser_unittest.cc index 163d19210c..dfe384c990 100644 --- a/src/lib/cc/tests/simple_parser_unittest.cc +++ b/src/lib/cc/tests/simple_parser_unittest.cc @@ -9,6 +9,7 @@ #include using namespace isc::data; +using isc::dhcp::DhcpConfigError; /// This table defines sample default values. Although these are DHCPv6 /// specific, the mechanism is generic and can be used by any other component. @@ -57,23 +58,22 @@ public: class SimpleParserClassTest : public SimpleParser { public: - /// @brief Instantiation of extractInt for uint8_t + /// @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 - /// @param value value of the parameter /// @return an uint8_t value - uint8_t extractUint8(const std::string& name, ConstElementPtr value) const { - return (extractInt(name, value)); + uint8_t getUint8(ConstElementPtr scope, const std::string& name) { + return (getIntType(scope, name)); } - /// @brief Instantiation of extractConvert + /// @brief Instantiation of getAndConvert /// + /// @param scope specified parameter will be extracted from this scope /// @param name name of the parameter for error report - /// @param value value of the parameter /// @return a bool value - bool extractBool(const std::string& name, ConstElementPtr value) const { - return (extractConvert - (name, "boolean", value)); + bool getAsBool(ConstElementPtr scope, const std::string& name) { + return (getAndConvert(scope, name, "boolean")); } /// @brief Convert to boolean @@ -169,45 +169,53 @@ TEST_F(SimpleParserTest, setListDefaults) { checkIntegerValue(third, "renew-timer", 900); } -// This test exercises the extractInt template -TEST_F(SimpleParserTest, extractInt) { +// This test exercises the getIntType template +TEST_F(SimpleParserTest, getIntType) { SimpleParserClassTest parser; - // extractInt checks if it is an integer - ConstElementPtr not_int(new StringElement("xyz")); - EXPECT_THROW(parser.extractUint8("foo", not_int), TypeError); + // getIntType checks it can be found + ElementPtr not_found = Element::fromJSON("{ \"bar\": 1 }"); + EXPECT_THROW(parser.getUint8(not_found, "foo"), DhcpConfigError); - // extractInt checks bounds - ConstElementPtr negative(new IntElement(-1)); - EXPECT_THROW(parser.extractUint8("foo", negative), isc::OutOfRange); - ConstElementPtr too_large(new IntElement(1024)); - EXPECT_THROW(parser.extractUint8("foo", too_large),isc::OutOfRange); + // getIntType checks if it is an integer + ElementPtr not_int = Element::fromJSON("{ \"foo\": \"xyz\" }"); + EXPECT_THROW(parser.getUint8(not_int, "foo"), DhcpConfigError); - // checks if extractInt can return the expected value - ConstElementPtr hundred(new IntElement(100)); + // getIntType checks bounds + ElementPtr negative = Element::fromJSON("{ \"foo\": -1 }"); + EXPECT_THROW(parser.getUint8(negative, "foo"), DhcpConfigError); + ElementPtr too_large = Element::fromJSON("{ \"foo\": 1024 }"); + EXPECT_THROW(parser.getUint8(too_large, "foo"), DhcpConfigError); + + // checks if getIntType can return the expected value + ElementPtr hundred = Element::fromJSON("{ \"foo\": 100 }"); uint8_t val = 0; - EXPECT_NO_THROW(val = parser.extractUint8("foo", hundred)); + EXPECT_NO_THROW(val = parser.getUint8(hundred, "foo")); EXPECT_EQ(100, val); } -// This test exercises the extractConvert template -TEST_F(SimpleParserTest, extractConvert) { +// This test exercises the getAndConvert template +TEST_F(SimpleParserTest, getAndConvert) { SimpleParserClassTest parser; - // extractConvert checks if it is a string - ConstElementPtr not_bool(new IntElement(1)); - EXPECT_THROW(parser.extractBool("foo", not_bool), TypeError); + // getAndConvert checks it can be found + ElementPtr not_found = Element::fromJSON("{ \"bar\": \"true\" }"); + EXPECT_THROW(parser.getAsBool(not_found, "foo"), DhcpConfigError); + + // getAndConvert checks if it is a string + ElementPtr not_bool = Element::fromJSON("{ \"foo\": 1 }"); + EXPECT_THROW(parser.getAsBool(not_bool, "foo"), DhcpConfigError); - // checks if extractConvert can return the expected value - ConstElementPtr a_bool(new StringElement("false")); + // checks if getAndConvert can return the expected value + ElementPtr a_bool = Element::fromJSON("{ \"foo\": \"false\" }"); bool val = true; - EXPECT_NO_THROW(val = parser.extractBool("foo", a_bool)); + EXPECT_NO_THROW(val = parser.getAsBool(a_bool, "foo")); EXPECT_FALSE(val); - // extractConvert checks convertion - ConstElementPtr bad_bool(new StringElement("foo")); - EXPECT_THROW(parser.extractBool("bar", bad_bool), isc::BadValue); + // getAndConvert checks convertion + ElementPtr bad_bool = Element::fromJSON("{ \"foo\": \"bar\" }"); + EXPECT_THROW(parser.getAsBool(bad_bool, "bar"), DhcpConfigError); } diff --git a/src/lib/dhcpsrv/parsers/dhcp_config_parser.h b/src/lib/dhcpsrv/parsers/dhcp_config_parser.h index d0c759b3b6..92e53413e9 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_config_parser.h +++ b/src/lib/dhcpsrv/parsers/dhcp_config_parser.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2015,2017 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 @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -16,20 +17,6 @@ namespace isc { namespace dhcp { -/// An exception that is thrown if an error occurs while configuring -/// DHCP server. -class DhcpConfigError : public isc::Exception { -public: - - /// @brief constructor - /// - /// @param file name of the file, where exception occurred - /// @param line line of the file, where exception occurred - /// @param what text description of the issue that caused exception - DhcpConfigError(const char* file, size_t line, const char* what) - : isc::Exception(file, line, what) {} -}; - /// @brief Forward declaration to DhcpConfigParser class. /// /// It is only needed here to define types that are