BIND 10 trac3274, updated. d8683cbfed08e4bdb0716d6576f8bc7e2e8a3c83 [3274] Changes after review:
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Feb 7 16:26:27 UTC 2014
The branch, trac3274 has been updated
via d8683cbfed08e4bdb0716d6576f8bc7e2e8a3c83 (commit)
from f2c1e775f7b2281655da68327fcae2be3a5fd2ca (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 d8683cbfed08e4bdb0716d6576f8bc7e2e8a3c83
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Fri Feb 7 17:26:03 2014 +0100
[3274] Changes after review:
- RelayInfoParser now takes Option::Universe as last parameter;
- Extra tests for checking invalid parameters in RelayInfoParser
- Comments in subnet.h improved
- Sanity checks in RelayInfoParser implemented, doc improved
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/config_parser.cc | 2 +-
src/bin/dhcp6/config_parser.cc | 2 +-
src/lib/dhcpsrv/dhcp_parsers.cc | 24 ++++++++++---
src/lib/dhcpsrv/dhcp_parsers.h | 22 +++++++++---
src/lib/dhcpsrv/subnet.h | 14 ++++++--
src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc | 44 ++++++++++++++++++++++--
6 files changed, 92 insertions(+), 16 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index 5809739..ac67faf 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -211,7 +211,7 @@ protected:
} else if (config_id.compare("pool") == 0) {
parser = new Pool4Parser(config_id, pools_);
} else if (config_id.compare("relay") == 0) {
- parser = new RelayInfoParser(config_id, relay_info_, IOAddress("0.0.0.0"));
+ parser = new RelayInfoParser(config_id, relay_info_, Option::V4);
} else if (config_id.compare("option-data") == 0) {
parser = new OptionDataListParser(config_id, options_,
global_context_,
diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc
index e809a67..38b1233 100644
--- a/src/bin/dhcp6/config_parser.cc
+++ b/src/bin/dhcp6/config_parser.cc
@@ -416,7 +416,7 @@ protected:
} else if (config_id.compare("pool") == 0) {
parser = new Pool6Parser(config_id, pools_);
} else if (config_id.compare("relay") == 0) {
- parser = new RelayInfoParser(config_id, relay_info_, IOAddress("::"));
+ parser = new RelayInfoParser(config_id, relay_info_, Option::V6);
} else if (config_id.compare("pd-pools") == 0) {
parser = new PdPoolListParser(config_id, pools_);
} else if (config_id.compare("option-data") == 0) {
diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc
index 1cf41df..694d616 100644
--- a/src/lib/dhcpsrv/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/dhcp_parsers.cc
@@ -814,9 +814,10 @@ OptionDefListParser::commit() {
//****************************** RelayInfoParser ********************************
RelayInfoParser::RelayInfoParser(const std::string&,
const isc::dhcp::Subnet::RelayInfoPtr& relay_info,
- const asiolink::IOAddress& default_addr)
- :storage_(relay_info), local_(default_addr),
- string_values_(new StringStorage()) {
+ const Option::Universe family)
+ :storage_(relay_info), local_(isc::asiolink::IOAddress(
+ family == Option::V4 ? "0.0.0.0" : "::")),
+ string_values_(new StringStorage()), family_(family) {
if (!relay_info) {
isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:"
<< "relay-info storage may not be NULL");
@@ -834,9 +835,22 @@ RelayInfoParser::build(ConstElementPtr relay_info) {
}
// Get the IP address
- asiolink::IOAddress ip(string_values_->getParam("ip-address"));
+ boost::scoped_ptr<asiolink::IOAddress> ip;
+ try {
+ ip.reset(new asiolink::IOAddress(string_values_->getParam("ip-address")));
+ } catch (...) {
+ isc_throw(DhcpConfigError, "Failed to parse ip-address "
+ "value: " << string_values_->getParam("ip-address"));
+ }
+
+ 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"));
+ }
- local_.addr_ = ip;
+ local_.addr_ = *ip;
}
isc::dhcp::ParserPtr
diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h
index 0e4f049..a48cbed 100644
--- a/src/lib/dhcpsrv/dhcp_parsers.h
+++ b/src/lib/dhcpsrv/dhcp_parsers.h
@@ -765,14 +765,13 @@ class RelayInfoParser : public DhcpConfigParser {
public:
/// @brief constructor
- /// @param dummy first argument is ignored, all Parser constructors
+ /// @param unused first argument is ignored, all Parser constructors
/// accept string as first argument.
/// @param relay_info is the storage in which to store the parsed
- /// @param default_addr 0.0.0.0 for IPv4 or :: for IPv6
- /// relay information upon "commit".
- RelayInfoParser(const std::string& dummy,
+ /// @param family specifies protocol family (IPv4 or IPv6)
+ RelayInfoParser(const std::string& unused,
const isc::dhcp::Subnet::RelayInfoPtr& relay_info,
- const asiolink::IOAddress& default_addr);
+ const isc::dhcp::Option::Universe family);
/// @brief parses the actual relay parameters
/// @param relay_info JSON strcture holding relay parameters to parse
@@ -782,6 +781,16 @@ public:
virtual void commit();
protected:
+
+ /// @brief Creates a parser for the given "relay" member element id.
+ ///
+ /// The elements currently supported are:
+ /// -# ip-address
+ ///
+ /// @param config_id is the "item_name" for a specific member element of
+ /// the "relay" specification.
+ ///
+ /// @return returns a pointer to newly created parser.
isc::dhcp::ParserPtr
createConfigParser(const std::string& parser);
@@ -793,6 +802,9 @@ protected:
/// Storage for subnet-specific string values.
StringStoragePtr string_values_;
+
+ /// Protocol family (IPv4 or IPv6)
+ Option::Universe family_;
};
/// @brief this class parses a single subnet
diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h
index 316556b..7b9bce7 100644
--- a/src/lib/dhcpsrv/subnet.h
+++ b/src/lib/dhcpsrv/subnet.h
@@ -396,7 +396,7 @@ public:
static_id_ = 1;
}
- /// @brief Sets address of the relay
+ /// @brief Sets information about relay
///
/// In some situations where there are shared subnets (i.e. two different
/// subnets are available on the same physical link), there is only one
@@ -411,8 +411,10 @@ public:
/// from different subnet, so users won't tinker with their modems.
///
/// Setting this parameter is not needed in most deployments.
+ /// This structure holds IP address only for now, but it is expected to
+ /// be extended in the future.
///
- /// @param relay IP address of the relay
+ /// @param relay structure that contains relay information
void setRelay(const isc::dhcp::Subnet::RelayInfo& relay);
/// @brief Relay information
@@ -439,6 +441,8 @@ public:
/// @brief adds class class_name to the list of supported classes
///
+ /// Also see explanation note in @ref white_list_.
+ ///
/// @param class_name client class to be supported by this subnet
void
allowClientClass(const isc::dhcp::ClientClass& class_name);
@@ -575,6 +579,12 @@ protected:
/// If defined, only clients belonging to that class will be allowed to use
/// this particular subnet. The default value for this is "", which means
/// that any client is allowed, regardless of its class.
+ ///
+ /// @todo This is just a single class for now, but it will eventually be
+ /// list of classes (only classes on the list are allowed, the rest is
+ /// rejected) and we'll also have black-list (only classes on the list are
+ /// rejected, the rest are allowed). Implementing this will require
+ /// fancy parser logic, so it may be a while until we support this.
ClientClass white_list_;
private:
diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
index 9c95eca..44d587c 100644
--- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
+++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc
@@ -1278,6 +1278,20 @@ TEST_F(ParseConfigTest, validRelayInfo4) {
" }";
ElementPtr json = Element::fromJSON(config_str);
+ // Invalid config (wrong family type of the ip-address field)
+ std::string config_str_bogus1 =
+ " {"
+ " \"ip-address\" : \"2001:db8::1\""
+ " }";
+ ElementPtr json_bogus1 = Element::fromJSON(config_str_bogus1);
+
+ // Invalid config (that thing is not an IPv4 address)
+ std::string config_str_bogus2 =
+ " {"
+ " \"ip-address\" : \"256.345.123.456\""
+ " }";
+ ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2);
+
// We need to set the default ip-address to something.
Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("0.0.0.0")));
@@ -1285,11 +1299,17 @@ TEST_F(ParseConfigTest, validRelayInfo4) {
// Subnet4 parser will pass 0.0.0.0 to the RelayInfoParser
EXPECT_NO_THROW(parser.reset(new RelayInfoParser("ignored", result,
- asiolink::IOAddress("0.0.0.0"))));
+ Option::V4)));
EXPECT_NO_THROW(parser->build(json));
EXPECT_NO_THROW(parser->commit());
EXPECT_EQ("192.0.2.1", result->addr_.toText());
+
+ // Let's check negative scenario (wrong family type)
+ EXPECT_THROW(parser->build(json_bogus1), DhcpConfigError);
+
+ // Let's check negative scenario (too large byte values in pseudo-IPv4 addr)
+ EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError);
}
/// @brief Checks that a valid relay info structure for IPv6 can be handled
@@ -1302,15 +1322,35 @@ TEST_F(ParseConfigTest, validRelayInfo6) {
" }";
ElementPtr json = Element::fromJSON(config_str);
+ // Invalid config (wrong family type of the ip-address field
+ std::string config_str_bogus1 =
+ " {"
+ " \"ip-address\" : \"192.0.2.1\""
+ " }";
+ ElementPtr json_bogus1 = Element::fromJSON(config_str_bogus1);
+
+ // That IPv6 address doesn't look right
+ std::string config_str_bogus2 =
+ " {"
+ " \"ip-address\" : \"2001:db8:::4\""
+ " }";
+ ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2);
+
// We need to set the default ip-address to something.
Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("::")));
boost::shared_ptr<RelayInfoParser> parser;
// Subnet4 parser will pass :: to the RelayInfoParser
EXPECT_NO_THROW(parser.reset(new RelayInfoParser("ignored", result,
- asiolink::IOAddress("::"))));
+ Option::V6)));
EXPECT_NO_THROW(parser->build(json));
EXPECT_NO_THROW(parser->commit());
EXPECT_EQ("2001:db8::1", result->addr_.toText());
+
+ // Let's check negative scenario (wrong family type)
+ EXPECT_THROW(parser->build(json_bogus1), DhcpConfigError);
+
+ // Unparseable text that looks like IPv6 address, but has too many colons
+ EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError);
}
More information about the bind10-changes
mailing list