BIND 10 trac998, updated. 287edb431de6ae5d7106dd4e593a193908b9ba9f [trac998] Default constructor now indicates no valid ACL
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Jun 17 11:00:58 UTC 2011
The branch, trac998 has been updated
via 287edb431de6ae5d7106dd4e593a193908b9ba9f (commit)
via 99d7c21284686ba3d021a6d09938b82ea56de783 (commit)
from 309b24ff461b623770e950d6ff12654241bdd39b (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 287edb431de6ae5d7106dd4e593a193908b9ba9f
Author: Stephen Morris <stephen at isc.org>
Date: Fri Jun 17 11:29:33 2011 +0100
[trac998] Default constructor now indicates no valid ACL
commit 99d7c21284686ba3d021a6d09938b82ea56de783
Author: Stephen Morris <stephen at isc.org>
Date: Fri Jun 17 11:07:57 2011 +0100
[trac998] Tidy-up of comments and the like
-----------------------------------------------------------------------
Summary of changes:
src/lib/acl/ip_check.h | 89 +++++++++++++++++++------------
src/lib/acl/tests/ip_check_unittest.cc | 65 ++++++++++++-----------
2 files changed, 89 insertions(+), 65 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/acl/ip_check.h b/src/lib/acl/ip_check.h
index 2e70fd8..a34deb8 100644
--- a/src/lib/acl/ip_check.h
+++ b/src/lib/acl/ip_check.h
@@ -33,15 +33,6 @@
#include <util/strutil.h>
#include <exceptions/exceptions.h>
-// The contents of this file are:
-//
-// 1. Free functions used by other code in the file.
-// 2. Ipv4Check, the class handling the checking of IPV4 addresses.
-// 3. Ipv6Check, the class handling the checking of IPV6 addresses.
-// 4. IpCheck, the "external" interface and the one that should be used in
-// code implementing ACL checks. Detailed documentation of use can be
-// found in this header for that class.
-
namespace isc {
namespace acl {
@@ -107,8 +98,8 @@ T createNetmask(size_t masksize) {
/// string representing the IP address and a number giving the size of the
/// network mask in bits. (In the latter case, the size of the network mask is
/// equal to the width of the data type holding the address.) An exception will
-/// be thrown if the string format is invalid or if the network mask is larger
-/// than a given maximum value.
+/// be thrown if the string format is invalid or if the network mask size is not
+/// an integer.
///
/// N.B. This function does NOT check that the address component is a valid IP
/// address; this is done elsewhere in the address parsing process.
@@ -120,16 +111,23 @@ T createNetmask(size_t masksize) {
/// size value. The second element is -1 if no mask was given.
std::pair<std::string, int>
-splitIpAddress(const std::string& addrmask) {
+splitIPAddress(const std::string& addrmask) {
// Set the default value for the mask size
int masksize = -1;
- // See if a mask size was given
+ // Split string into its components. As the tokenising code ignores
+ // leading, trailing nd consecutive delimiters, be strict here and ensures
+ // that the string contains at most 0 or 1 slashes.
+ if (std::count(addrmask.begin(), addrmask.end(), '/') > 1) {
+ isc_throw(isc::InvalidParameter, "address/masksize of " <<
+ addrmask << " is not valid");
+ }
+
std::vector<std::string> components = isc::util::str::tokens(addrmask, "/");
if (components.size() == 2) {
- // There appears to be, try converting it to a number.
+ // There appears to be a mask, try converting it to a number.
try {
masksize = boost::lexical_cast<int>(components[1]);
} catch (boost::bad_lexical_cast&) {
@@ -138,7 +136,8 @@ splitIpAddress(const std::string& addrmask) {
" is not valid");
}
- // Is it in the valid range?
+ // Ensure that it is positive - a mask size of zero is not a valid
+ // value.
if (masksize <= 0) {
isc_throw(isc::OutOfRange,
"mask size specified in address/masksize " << addrmask <<
@@ -174,13 +173,26 @@ private:
static const size_t IPV6_SIZE8 = sizeof(struct in6_addr);
static const size_t IPV6_SIZE32 = IPV6_SIZE8 / 4;
- // Data type to hold the address, regardless of the address family.
+ // Data type to hold the address, regardless of the address family. The
+ // union allows an IPV4 address to be treated as a sequence of bytes when
+ // necessary.
union AddressData {
uint32_t word[IPV6_SIZE32]; ///< Address in 32-bit words
uint8_t byte[IPV6_SIZE8]; ///< Address in 8-bit bytes
};
public:
+ /// \brief Default Constructor
+ ///
+ /// Constructs an empty IPCheck object. The address family returned will
+ /// be zero.
+ IPCheck() : address_(), netmask_(), masksize_(0), inverse_(false),
+ family_(0), straddr_()
+ {
+ std::fill(address_.word, address_.word + IPV6_SIZE32, 0);
+ std::fill(netmask_.word, netmask_.word + IPV6_SIZE32, 0);
+ }
+
/// \brief IPV4 Constructor
///
/// Constructs an IPCheck object from a network address given as a
@@ -195,7 +207,7 @@ public:
/// \param inverse If false (the default), matches() returns true if the
/// condition matches. If true, matches() returns true if the
/// condition does not match.
- IPCheck(uint32_t address = 1, int masksize = 8 * sizeof(uint32_t),
+ IPCheck(uint32_t address, int masksize = 8 * sizeof(uint32_t),
bool inverse = false):
address_(), netmask_(), masksize_(masksize), inverse_(inverse),
family_(AF_INET), straddr_()
@@ -232,11 +244,12 @@ public:
/// \brief String Constructor
///
- /// Constructs an IPv6 Check object from a network address and size of mask
- /// given as a string of the form <ip6-address>/n".
+ /// Constructs an IP Check object from a network address and size of mask
+ /// given as a string of the form <ip-address>/n".
///
- /// \param address IP address and netmask in the form "<v6-address>/n"
- /// (where the "/n" part is optional).
+ /// \param address IP address and netmask in the form "<ip-address>/n"
+ /// (where the "/n" part is optional and should be valid for the
+ /// address).
/// \param inverse If false (the default), matches() returns true if the
/// condition matches. If true, matches() returns true if the
/// condition does not match.
@@ -244,12 +257,12 @@ public:
address_(), netmask_(), masksize_(0), inverse_(inverse),
family_(0), straddr_(address)
{
- // Initialize addresses.
+ // Initialize.
std::fill(address_.word, address_.word + IPV6_SIZE32, 0);
std::fill(netmask_.word, netmask_.word + IPV6_SIZE32, 0);
// Split the address into address part and mask.
- std::pair<std::string, int> result = splitIpAddress(address);
+ std::pair<std::string, int> result = splitIPAddress(address);
// Try to convert the address. If successful, the result is in
// network-byte order (most significant components at lower addresses).
@@ -261,18 +274,19 @@ public:
family_ = AF_INET;
int status = inet_pton(AF_INET, result.first.c_str(),
address_.word);
-
if (status == 0) {
isc_throw(isc::InvalidParameter, "address/masksize of " <<
address << " is not valid IP address");
}
}
- // All done, so finish initialization.
+ // All done, so set the network mask.
setNetmask(result.second);
}
/// \brief Copy constructor
+ ///
+ /// \param other Object from which the copy is being constructed.
IPCheck(const IPCheck<Context>& other) : address_(), netmask_(),
masksize_(other.masksize_), inverse_(other.inverse_),
family_(other.family_), straddr_(other.straddr_)
@@ -284,6 +298,10 @@ public:
}
/// \brief Assignment operator
+ ///
+ /// \param other Source of the assignment.
+ ///
+ /// \return Reference to current object.
IPCheck& operator=(const IPCheck<Context>& other) {
if (this != &other) {
Check<Context>::operator=(other);
@@ -305,7 +323,7 @@ public:
/// \brief The check itself
///
/// Matches the passed argument to the condition stored here. Different
- /// specialisations are provided for different argument types, so the
+ /// specialisations must be provided for different argument types, and the
/// program will fail to compile if a required specialisation is not
/// provided.
///
@@ -315,8 +333,9 @@ public:
/// \brief Estimated cost
///
/// Assume that the cost of the match is linear and depends on the
- // maximum number of comparison operations.
- /// of comparison operations.
+ /// maximum number of comparison operations.
+ ///
+ /// \return Estimated cost of the comparison
virtual unsigned cost() const {
return ((family_ == AF_INET) ? 1 : IPV6_SIZE32);
}
@@ -405,7 +424,8 @@ private:
///
/// Convenience comparison for an IPV4 address.
///
- /// \param testaddr Address (in network byte order).
+ /// \param testaddr Address (in network byte order) to test against the
+ /// check condition in the class.
///
/// \return true if the address matches, false if it does not.
virtual bool compare(const uint32_t testaddr) const {
@@ -423,13 +443,12 @@ private:
/// family.
///
/// \param requested Requested mask size. If negative, the maximum for
- /// the address family is assumed.
+ /// the address family is assumed. (A negative value will arise
+ /// if the string constructor was used and no mask size was given.)
void setNetmask(int requested) {
// Set the maximum mask size allowed.
int maxmask = 8 * ((family_ == AF_INET) ? sizeof(uint32_t) : IPV6_SIZE8);
-
- // Adjust if negative.
if (requested < 0) {
requested = maxmask;
}
@@ -439,14 +458,14 @@ private:
masksize_ = requested;
// The netmask array was initialized to zero in the constructor,
- // but in debug mode we can check that.
+ // but as an addition check, assert that this is so.
assert(std::find_if(netmask_.word, netmask_.word + IPV6_SIZE32,
std::bind1st(std::not_equal_to<uint32_t>(), 0)) ==
netmask_.word + IPV6_SIZE32);
// Loop, setting the bits in the set of mask bytes until all the
// specified bits have been used up. As both IPV4 and IPV6
- // addressees are stored in network-byte order, this works in
+ // addresses are stored in network-byte order, this works in
// both cases.
size_t bits_left = masksize_; // Bits remaining to set
int i = -1;
@@ -464,7 +483,7 @@ private:
} else {
isc_throw(isc::OutOfRange,
"mask size of " << masksize_ << " is invalid " <<
- "for the data type");
+ "for the givem address");
}
}
diff --git a/src/lib/acl/tests/ip_check_unittest.cc b/src/lib/acl/tests/ip_check_unittest.cc
index e58d5a2..c61252c 100644
--- a/src/lib/acl/tests/ip_check_unittest.cc
+++ b/src/lib/acl/tests/ip_check_unittest.cc
@@ -23,8 +23,8 @@ using namespace std;
// Simple struct holding either an IPV4 or IPV6 address. This is the "Context"
// used for the tests.
//
-// The structure is also convenient for converting an IPV4 address to a
-// four-byte array.
+// The structure is also used for converting an IPV4 address to a four-byte
+// array.
struct GeneralAddress {
bool isv4; // true if it holds a v4 address
union {
@@ -32,19 +32,20 @@ struct GeneralAddress {
uint8_t v6addr[16];
};
+ // Default constructor.
GeneralAddress() : isv4(false) {
fill(v6addr, v6addr + sizeof(v6addr), 0);
}
- // Convenience constructor for V4 address. As it is not marked as
- // explicit, it allows the automatic promotion of a uint32_t to a
- // GeneralAddress data type in calls to matches().
+ // Convenience constructor for V4 address. As it is not marked as explicit,
+ // it allows the automatic promotion of a uint32_t to a GeneralAddress data
+ // type in calls to matches().
GeneralAddress(uint32_t address) : isv4(true), v4addr(address) {
fill(v6addr +sizeof(v4addr), v6addr + sizeof(v6addr), 0);
}
- // Convenience constructor for V6 address. As it is not marked as
- // explicit, it allows the automatic promotion of a vector<uint8_t> to a
+ // Convenience constructor for V6 address. As it is not marked as explicit,
+ // it allows the automatic promotion of a vector<uint8_t> to a
// GeneralAddress data type in calls to matches().
GeneralAddress(const vector<uint8_t>& address) : isv4(false) {
if (address.size() != sizeof(v6addr)) {
@@ -55,10 +56,11 @@ struct GeneralAddress {
copy(address.begin(), address.end(), v6addr);
}
- // A couple of convenience methods for checking equality.
+ // A couple of convenience methods for checking equality with different
+ // representations of an address.
- // Check that the IPV4 address is the same as that given, and that
- // the remainder of the V6 addray is zero.
+ // Check that the IPV4 address is the same as that given, and that the
+ // remainder of the V6 addray is zero.
bool equals(uint32_t address) {
return ((address == v4addr) &&
(find_if(v6addr + sizeof(v4addr), v6addr + sizeof(v6addr),
@@ -73,7 +75,8 @@ struct GeneralAddress {
}
};
- // Provide specializations of the matches() method for the class.
+// Provide a specialisation of the IPCheck::matches() method for the
+// GeneralAddress class.
namespace isc {
namespace acl {
@@ -92,7 +95,7 @@ bool IPCheck<GeneralAddress>::matches(const GeneralAddress& addr) const {
/// *** Free Function Tests ***
// Test the createNetmask() function.
-TEST(IpFunctionCheck, CreateNetmask) {
+TEST(IPFunctionCheck, CreateNetmask) {
// 8-bit tests: invalid arguments should throw.
EXPECT_THROW(createNetmask<uint8_t>(9), isc::OutOfRange);
@@ -117,39 +120,40 @@ TEST(IpFunctionCheck, CreateNetmask) {
EXPECT_EQ(0, createNetmask<uint32_t>(0));
}
-// Test the splitIpAddress() function.
-TEST(IpFunctionCheck, SplitIpAddress) {
+// Test the splitIPAddress() function.
+TEST(IPFunctionCheck, SplitIPAddress) {
pair<string, uint32_t> result;
- result = splitIpAddress("192.0.2.1");
+ result = splitIPAddress("192.0.2.1");
EXPECT_EQ(string("192.0.2.1"), result.first);
EXPECT_EQ(-1, result.second);
- result = splitIpAddress("192.0.2.1/24");
+ result = splitIPAddress("192.0.2.1/24");
EXPECT_EQ(string("192.0.2.1"), result.first);
EXPECT_EQ(24, result.second);
- result = splitIpAddress("2001:db8::/128");
+ result = splitIPAddress("2001:db8::/128");
EXPECT_EQ(string("2001:db8::"), result.first);
EXPECT_EQ(128, result.second);
- EXPECT_THROW(splitIpAddress("192.0.2.43/-1"), isc::OutOfRange);
- EXPECT_THROW(splitIpAddress("2001:db8::/0"), isc::OutOfRange);
- EXPECT_THROW(splitIpAddress("2001:db8::/xxxx"), isc::InvalidParameter);
- EXPECT_THROW(splitIpAddress("2001:db8::/32/s"), isc::InvalidParameter);
+ EXPECT_THROW(splitIPAddress("192.0.2.43/-1"), isc::OutOfRange);
+ EXPECT_THROW(splitIPAddress("192.0.2.43//1"), isc::InvalidParameter);
+ EXPECT_THROW(splitIPAddress("192.0.2.43/1/"), isc::InvalidParameter);
+ EXPECT_THROW(splitIPAddress("/192.0.2.43/1"), isc::InvalidParameter);
+ EXPECT_THROW(splitIPAddress("2001:db8::/0"), isc::OutOfRange);
+ EXPECT_THROW(splitIPAddress("2001:db8::/xxxx"), isc::InvalidParameter);
+ EXPECT_THROW(splitIPAddress("2001:db8::/32/s"), isc::InvalidParameter);
}
-// *** IPV4 Tests ***
-// Check that a default constructor can be instantiated.
+// *** General tests ***
TEST(IPCheck, DefaultConstructor) {
IPCheck<GeneralAddress> acl;
-
- // The test is needed to avoid the unused variable causing a warning or
- // getting optimised away.
- EXPECT_EQ(AF_INET, acl.getFamily());
+ EXPECT_EQ(0, acl.getFamily());
}
+// *** IPV4 Tests ***
+
// Check that the constructor stores the elements correctly and, for the
// address and mask, in network-byte order.
@@ -377,8 +381,6 @@ const uint8_t MASK_128[] = {
} // Anonymous namespace
-// Check that a default constructor can be instantiated.
-
TEST(IPCheck, V6ConstructorAddress) {
IPCheck<GeneralAddress> acl1(V6ADDR_1);
vector<uint8_t> stored = acl1.getAddress();
@@ -540,12 +542,15 @@ TEST(IPCheck, V6Compare) {
// *** Mixed-mode tests - mainly to check that no exception is thrown ***
-TEST(IPCheck, V4Test) {
+TEST(IPCheck, MixedMode) {
+
+ // ACL has a V4 address specified, check against a V6 address.
IPCheck<GeneralAddress> acl1("192.0.2.255/24");
GeneralAddress test1(vector<uint8_t>(V6ADDR_1, V6ADDR_1 + sizeof(V6ADDR_1)));
EXPECT_NO_THROW(acl1.matches(test1));
EXPECT_FALSE(acl1.matches(test1));
+ // Now the reverse - the ACL is specified with a V6 address.
IPCheck<GeneralAddress> acl2(V6ADDR_2_STRING);
GeneralAddress test2(0x12345678);
EXPECT_NO_THROW(acl2.matches(test2));
More information about the bind10-changes
mailing list