[trac998] Just the undefined abstract methods to worry about

src/lib/acl/ip_check.h

@@ -16,10 +16,12 @@
 #define __IP_CHECK_H
 
 #include <cassert>
+#include <iterator>
 #include <utility>
 #include <vector>
 
 #include <boost/lexical_cast.hpp>
+#include <boost/scoped_ptr.hpp>
 
 #include <stdint.h>
 #include <arpa/inet.h>
@@ -136,6 +138,27 @@ splitIpAddress(const std::string& addrmask, uint32_t maxmask) {
 }
 
 
+/// \brief IP Base
+///
+/// Base class for both the Ipv4 and Ipv4 check classes.  The only thing this
+/// supplies over and above the Check class is a clone() method, to return
+/// a pointer to a newly-allocated copy of itself.
+
+template <typename Context>
+class IpBaseCheck : public Check<Context> {
+public:
+    /// \brief Clone Object
+    ///
+    /// Implemented by derived classes, this causes a class to create a copy
+    /// of itself and return a pointer to the object.
+    ///
+    /// \return Pointer to the cloned object.  It is the caller's responsibility
+    ///         to delete this object.
+    virtual IpBaseCheck* clone() const = 0;
+};
+
+
+
 /// \brief IPV4 Check
 ///
 /// This class performs a match between an IPv4 address specified in an ACL
@@ -145,7 +168,7 @@ splitIpAddress(const std::string& addrmask, uint32_t maxmask) {
 /// \param Context Structure holding address to be matched.
 
 template <typename Context>
-class Ipv4Check : public Check<Context> {
+class Ipv4Check : public IpBaseCheck<Context> {
 public:
     /// \brief IPV4 Constructor
     ///
@@ -167,8 +190,6 @@ public:
         setNetmask();
     }
 
-
-
     /// \brief String Constructor
     ///
     /// Constructs an IPv4 Check object from a network address and size of mask
@@ -199,13 +220,12 @@ public:
         setNetmask();
     }
 
-
+    // Default copy constructor and assignment operator are correct for this
+    // class.
 
     /// \brief Destructor
     virtual ~Ipv4Check() {}
 
-
-
     /// \brief The check itself
     ///
     /// Matches the passed argument to the condition stored here.  Different
@@ -215,8 +235,6 @@ public:
     /// \param context Information to be matched
     virtual bool matches(const Context& context) const = 0;
 
-
-
     /// \brief Estimated cost
     ///
     /// Assume that the cost of the match is linear and depends on the number
@@ -225,8 +243,6 @@ public:
         return (1);             // Single check on a 32-bit word
     }
 
-
-
     ///@{
     /// Access methods - mainly for testing
 
@@ -310,7 +326,6 @@ public:
     size_t      masksize_;  ///< Mask size passed to constructor
     bool        inverse_;   ///< Test for equality or inequality
     uint32_t    netmask_;   ///< Network mask applied to match
-
 };
 
 
@@ -331,12 +346,18 @@ bool isNonZero(uint8_t i) {
 /// \param Context Structure holding address to be matched.
 
 template <typename Context>
-class Ipv6Check : public Check<Context> {
+class Ipv6Check : public IpBaseCheck<Context> {
 public:
 
     // Size of array to old IPV6 address.
     static const size_t IPV6_SIZE = sizeof(struct in6_addr);
 
+    /// \brief Default Constructor
+    Ipv6Check() :
+        address_(IPV6_SIZE, 0), masksize_(0),
+        inverse_(false), netmask_(IPV6_SIZE, 0)
+    {}
+
     /// \brief IPV6 Constructor
     ///
     /// Constructs an IPv6 Check object from a network address given as a
@@ -354,27 +375,25 @@ public:
     Ipv6Check(const uint8_t* address, size_t masksize = 8 * IPV6_SIZE,
               bool inverse = false) :
         address_(address, address + IPV6_SIZE), masksize_(masksize),
-        inverse_(inverse), netmask_(IPV6_SIZE, 0), netcount_(0)
+        inverse_(inverse), netmask_(IPV6_SIZE, 0)
     {
         setNetmask();
     }
 
-
-
     /// \brief String Constructor
     ///
     /// Constructs an IPv6 Check object from a network address and size of mask
     /// given as a string of the form "a.b.c.d/n", where the "/n" part is
     /// optional.
     ///
-    /// \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 "<v6-address>/n"
+    ///        (where the "/n" part is optional).
     /// \param inverse If false (the default), matches() returns true if the
     ///        condition matches.  If true, matches() returns true if the
     ///        condition does not match.
     Ipv6Check(const std::string& address, bool inverse = false) :
         address_(IPV6_SIZE, 0), masksize_(8 * IPV6_SIZE),
-        inverse_(inverse), netmask_(IPV6_SIZE, 0), netcount_(0)
+        inverse_(inverse), netmask_(IPV6_SIZE, 0)
     {
         // Split the address into address part and mask.
         std::pair<std::string, uint32_t> result =
@@ -392,13 +411,35 @@ public:
         setNetmask();
     }
 
-
+    /// \brief Copy constructor
+    Ipv6Check(const Ipv6Check& other) : IpBaseCheck<Context>(other),
+        address_(other.address_.begin(), other.address_.end()),
+        masksize_(other.masksize_), inverse_(other.inverse_),
+        netmask_(other.netmask_.begin(), other.netmask_.end())
+    {}
+
+    /// \brief Assignment operator
+    Ipv6Check& operator=(const Ipv6Check& other) {
+        if (this != &other) {
+            // Copy parent
+            IpBaseCheck<Context>::operator=(other);
+
+            // Copy this object
+            address_.clear();
+            std::copy(other.address_.begin(), other.address_.end(),
+                      std::back_inserter(address_));
+            masksize_ = other.masksize_;
+            inverse_ = other.inverse_;
+            netmask_.clear();
+            std::copy(other.netmask_.begin(), other.netmask_.end(),
+                      std::back_inserter(netmask_));
+        }
+        return (*this);
+    }
 
     /// \brief Destructor
     virtual ~Ipv6Check() {}
 
-
-
     /// \brief The check itself
     ///
     /// Matches the passed argument to the condition stored here.  Different
@@ -408,8 +449,6 @@ public:
     /// \param context Information to be matched
     virtual bool matches(const Context& context) const = 0;
 
-
-
     /// \brief Estimated cost
     ///
     /// Assume that the cost of the match is linear and depends on the number
@@ -418,8 +457,6 @@ public:
         return (IPV6_SIZE);             // Up to 16 checks
     }
 
-
-
     ///@{
     /// Access methods - mainly for testing
 
@@ -456,24 +493,21 @@ public:
             // Loop, setting the bits in the set of mask bytes until all the
             // specified bits have been used up.  The netmask array was
             // initialized to zero in the constructor.
-            assert(netcount_ == 0);     // Set in constructor
             assert(std::find_if(netmask_.begin(), netmask_.end(), isNonZero) ==
                    netmask_.end());
 
             size_t bits_left = masksize_;   // Bits remaining to set
+            int i = -1;                     // To allow pre-incrementing
             while (bits_left > 0) {
                 if (bits_left >= 8) {
-                    netmask_[netcount_] = ~0;  // All bits set
+                    netmask_[++i] = ~0;  // All bits set
                     bits_left -= 8;
 
                 } else if (bits_left > 0) {
-                    netmask_[netcount_] = createNetmask<uint8_t>(bits_left);
+                    netmask_[++i] = createNetmask<uint8_t>(bits_left);
                     bits_left = 0;
 
                 }
-
-                // One more byte to test against
-                ++netcount_;
             }
         } else {
             isc_throw(isc::OutOfRange,
@@ -489,12 +523,12 @@ public:
     /// to this class with the matching information in the class itself.  It is
     /// expected to be called from matches().
     ///
-    /// \param address Address (in network byte order) to match against the
-    ///                check condition in the class.  This is expected to
-    ///                be IPV6_SIZE bytes long.
+    /// \param testaddr Address (in network byte order) to test against the
+    ///                 check condition in the class.  This is expected to
+    ///                 be IPV6_SIZE bytes long.
     ///
     /// \return true if the address matches, false if it does not.
-    virtual bool compare(const uint8_t* address) const {
+    virtual bool compare(const uint8_t* testaddr) const {
 
         // To check that the address given matches the stored network address
         // and netmask, we check the simple condition that:
@@ -502,10 +536,16 @@ public:
         //     address_given & netmask_ == stored_address & netmask_
         //
         // The result is checked for all bytes for which there are bits set in
-        // the network mask.  We stop at the first non-match.
+        // the network mask.  We stop at the first non-match (or when we run
+        // out of bits in the network mask). (Note that the network mask
+        // represents a contiguous set of bits.  As such, as soon as we find
+        // a netmask byte of zeroes, we have run part the part of the address
+        // where we need to match.
+
         bool match = true;
-        for (size_t i = 0; (i < netcount_) && match; ++i) {
-            match = ((address[i] & netmask_[i]) == (address_[i] & netmask_[i]));
+        for (int i = 0; match && (i < IPV6_SIZE) && (netmask_[i] != 0); ++i){
+             match = ((testaddr[i] & netmask_[i]) ==
+                      (address_[i] & netmask_[i]));
         }
 
         // As with the V4 check, return the XOR with the inverse flag.
@@ -518,10 +558,90 @@ public:
     size_t                  masksize_;  ///< Mask size passed to constructor
     bool                    inverse_;   ///< Test for equality or inequality
     std::vector<uint8_t>    netmask_;   ///< Network mask applied to match
-    size_t                  netcount_;  ///< Number of bytes in mask to test
 
 };
 
+
+/// \brief Generic IP Address Check Object
+///
+/// This class performs a match between an IP address specified in an ACL
+/// (IP address (either V4 of V6), network mask and a flag indicating whether
+/// the check should be for a match or for a non-match) and a given IP address.
+///
+/// \param Context Structure holding address to be matched.
+
+template <typename Context>
+class IpCheck : public IpBaseCheck<Context> {
+public:
+
+    /// \brief String Constructor
+    ///
+    /// Constructs an IP Check object from a network address and size of mask
+    /// given as a string of the form "<address>/n", where the "/n" part is
+    /// optional.
+    ///
+    /// \param address IP address and netmask in the form "<address>/n"
+    ///        (where the "/n" part is optional).
+    /// \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(const std::string& address, bool inverse = false) {
+        try {
+            check_.reset(new Ipv4Check<Context>(address, inverse));
+            isv4_ = true;
+        } catch (isc::Exception&) {
+            check_.reset(new Ipv6Check<Context>(address, inverse));
+            isv4_ = false;
+        }
+    }
+
+    /// \brief Copy constructor
+    IpCheck(const IpCheck<Context>& other) : IpBaseCheck<Context>(other),
+                                             check_(other.check_->clone())
+    {}
+
+    /// \brief Assignment operator
+    IpCheck& operator=(const IpCheck& other) {
+        if (this != &other) {
+            IpBaseCheck<Context>::operator=(other);
+            check_.reset(other.check_->clone());
+        }
+    }
+
+    /// \brief Clone
+    IpBaseCheck<Context>* clone() const {
+        return (new IpCheck<Context>(*this));
+    }
+
+    /// \brief Destructor
+    virtual ~IpCheck() {}
+
+    /// \brief The check itself
+    ///
+    /// Matches the passed argument to the condition stored here.  Different
+    /// specialisations are provided for different argument types, so the
+    /// link will fail if used for a type for which no match is provided.
+    ///
+    /// \param context Information to be matched
+    virtual bool matches(const Context& context) const {
+        return (check_->matches(context));
+    }
+
+    /// \brief Estimated cost
+    ///
+    /// Assume that the cost of the match is linear and depends on the number
+    /// of comparison operations.
+    virtual unsigned cost() const {
+        return (check_->cost());
+    }
+
+private:
+    // Member variables
+
+    bool                                      isv4_;  ///< true if V4 address
+    boost::scoped_ptr<IpBaseCheck<Context> >  check_; ///< Check object
+};
+
 } // namespace acl
 } // namespace isc
 

src/lib/acl/tests/ip_check_unittest.cc

@@ -15,16 +15,70 @@
 
 #include <gtest/gtest.h>
 #include <acl/ip_check.h>
+#include <boost/scoped_ptr.hpp>
 
 using namespace isc::acl;
 using namespace std;
 
-// Declare a derived class to allow the abstract function to be declared
-// as a concrete one.
+/// *** Free Function Tests ***
+
+TEST(IpFunctionCheck, CreateNetmask) {
+
+    // 8-bit tests: invalid arguments should throw.
+    EXPECT_THROW(createNetmask<uint8_t>(9), isc::OutOfRange);
+
+    // Check on all possible 8-bit values.  Use a signed type to generate a
+    // variable with the most significant bits set, as right-shifting it is
+    // guaranteed to introduce additional bits.
+    int8_t expected8 = 0x80;
+    for (size_t i = 1; i <= 8; ++i, expected8 >>= 1) {
+        EXPECT_EQ(static_cast<uint8_t>(expected8), createNetmask<uint8_t>(i));
+    }
+    EXPECT_EQ(0, createNetmask<uint8_t>(0));
+
+    // Do the same for 32 bits.
+    EXPECT_THROW(createNetmask<int32_t>(33), isc::OutOfRange);
+
+    // Check on all possible 8-bit values
+    int32_t expected32 = 0x80000000;
+    for (size_t i = 1; i <= 32; ++i, expected32 >>= 1) {
+        EXPECT_EQ(static_cast<uint32_t>(expected32),
+                  createNetmask<uint32_t>(i));
+    }
+    EXPECT_EQ(0, createNetmask<uint32_t>(0));
+}
+
+TEST(IpFunctionCheck, SplitIp) {
+    pair<string, uint32_t> result;
+
+    result = splitIpAddress("192.0.2.1/24", 32);
+    EXPECT_EQ(string("192.0.2.1"), result.first);
+    EXPECT_EQ(24, result.second);
+
+    result = splitIpAddress("192.0.2.1/32", 32);
+    EXPECT_EQ(string("192.0.2.1"), result.first);
+    EXPECT_EQ(32, result.second);
+
+    result = splitIpAddress("2001:db8::/128", 128);
+    EXPECT_EQ(string("2001:db8::"), result.first);
+    EXPECT_EQ(128, result.second);
+
+    EXPECT_THROW(splitIpAddress("2001:db8::/129", 128), isc::OutOfRange);
+    EXPECT_THROW(splitIpAddress("2001:db8::/xxxx", 32), isc::InvalidParameter);
+    EXPECT_THROW(splitIpAddress("2001:db8::/32/s", 32), isc::InvalidParameter);
+}
+
+// *** IPV4 Tests ***
+
+// Declare a derived class to allow a definition of the match() method to be
+// provided.
+//
+// In this case, the match will check a uint32_t variable representing an IPV4
+// address.
 
 class DerivedV4Check : public Ipv4Check<uint32_t> {
 public:
-    // Basic constructor
+    // Basic (and default) constructor
     DerivedV4Check(uint32_t address = 1, size_t masksize = 32,
                    bool inverse = false) :
                    Ipv4Check<uint32_t>(address, masksize, inverse)
@@ -35,6 +89,23 @@ public:
         Ipv4Check<uint32_t>(address, inverse)
     {}
 
+    // Copy constructor
+    DerivedV4Check(const DerivedV4Check& other) : Ipv4Check<uint32_t>(other)
+    {}
+
+    // Assignment operator
+    DerivedV4Check& operator=(const DerivedV4Check& other) {
+        if (this != &other) {
+            Ipv4Check<uint32_t>::operator=(other);
+        }
+        return (*this);
+    }
+
+    // Clone method
+    virtual IpBaseCheck<uint32_t>* clone() const {
+        return (new DerivedV4Check(*this));
+    }
+
     // Destructor
     virtual ~DerivedV4Check()
     {}
@@ -45,54 +116,31 @@ public:
     }
 };
 
+// Check that a default constructor can be instantiated.
 
-/// Tests of the free functions.
-
-TEST(Ipv4Check, CreateNetmask) {
-    size_t  i;
-
-    // 8-bit tests.
-
-    // Invalid arguments should throw.
-    EXPECT_THROW(createNetmask<uint8_t>(9), isc::OutOfRange);
-
-    // Check on all possible 8-bit values.  Use a signed type to generate a
-    // variable with the most significant bits set, as right-shifting it is
-    // guaranteed to introduce additional bits.
-    int8_t  expected8;
-    for (i = 1, expected8 = 0x80; i <= 8; ++i, expected8 >>= 1) {
-        EXPECT_EQ(static_cast<uint8_t>(expected8),
-                  createNetmask<uint8_t>(i));
-    }
-    EXPECT_EQ(0, createNetmask<uint8_t>(0));
+TEST(Ipv4Check, V4DefaultConstructor) {
+    DerivedV4Check acl1;
 
-    // Do the same for 32 bits.
-    EXPECT_THROW(createNetmask<int32_t>(33), isc::OutOfRange);
-
-    // Check on all possible 8-bit values
-    int32_t expected32;
-    for (i = 1, expected32 = 0x80000000; i <= 32; ++i, expected32 >>= 1) {
-        EXPECT_EQ(static_cast<uint32_t>(expected32),
-                  createNetmask<uint32_t>(i));
-    }
-    EXPECT_EQ(0, createNetmask<uint32_t>(0));
+    // The test is needed to avoid the unused variable causing a warning or
+    // getting optimised away.
+    EXPECT_EQ(1, acl1.getAddress());
 }
-// IPV4 tests
 
-// Check that the constructor expands the network mask and stores the elements
-// correctly.  For these tests, we don't worry about the type of the context,
-// so we declare it as an int.
+// Check that the constructor stores the elements correctly.  As the address
+// is provided in the correct (network-byte order) format, the real test is
+// checking that the network mask is stored in the correct byte order.
 
 TEST(Ipv4Check, V4ConstructorAddress) {
     DerivedV4Check acl1(0x12345678);
     EXPECT_EQ(0x12345678, acl1.getAddress());
 }
 
-// The mask is stored in network byte order, so the pattern expected must
-// also be converted to network byte order for the comparison to succeed.
 TEST(Ipv4Check, V4ConstructorMask) {
     // Valid values. Address of "1" is used as a placeholder
     DerivedV4Check acl1(1, 1);
+
+    // The mask is stored in network byte order, so the pattern expected must
+    // also be converted to network byte order for the comparison to succeed.
     uint32_t expected = htonl(0x80000000);
     EXPECT_EQ(expected, acl1.getNetmask());
     EXPECT_EQ(1, acl1.getMasksize());
@@ -120,29 +168,61 @@ TEST(Ipv4Check, V4ConstructorInverse) {
 }
 
 TEST(Ipv4Check, V4StringConstructor) {
-    DerivedV4Check acl1("127.0.0.1");
-    uint32_t expected = htonl(0x7f000001);
+    DerivedV4Check acl1("192.168.132.255");
+    uint32_t expected = htonl(0xc0a884ff);
     EXPECT_EQ(expected, acl1.getAddress());
     EXPECT_EQ(32, acl1.getMasksize());
 
-    DerivedV4Check acl2("255.255.255.0/24");
-    expected = htonl(0xffffff00);
+    DerivedV4Check acl2("192.0.2.0/24");
+    expected = htonl(0xc0000200);
     EXPECT_EQ(expected, acl2.getAddress());
     EXPECT_EQ(24, acl2.getMasksize());
 
-    EXPECT_THROW(DerivedV4Check("255.255.255.0/0"), isc::OutOfRange);
-    EXPECT_THROW(DerivedV4Check("255.255.255.0/33"), isc::OutOfRange);
-    EXPECT_THROW(DerivedV4Check("255.255.255.0/24/3"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV4Check("255.255.255.0/ww"), isc::InvalidParameter);
+    EXPECT_THROW(DerivedV4Check("192.0.2.0/0"), isc::OutOfRange);
+    EXPECT_THROW(DerivedV4Check("192.0.2.0/33"), isc::OutOfRange);
+    EXPECT_THROW(DerivedV4Check("192.0.2.0/24/3"), isc::InvalidParameter);
+    EXPECT_THROW(DerivedV4Check("192.0.2.0/ww"), isc::InvalidParameter);
     EXPECT_THROW(DerivedV4Check("aa.255.255.0/ww"), isc::InvalidParameter);
 }
 
+TEST(Ipv4Check, V4CopyConstructor) {
+    DerivedV4Check acl1("127.0.0.1/16", true);
+    DerivedV4Check acl2(acl1);
+
+    EXPECT_EQ(acl1.getAddress(), acl2.getAddress());
+    EXPECT_EQ(acl1.getMasksize(), acl2.getMasksize());
+    EXPECT_EQ(acl1.getNetmask(), acl2.getNetmask());
+    EXPECT_EQ(acl1.getInverse(), acl2.getInverse());
+}
+
+TEST(Ipv4Check, V4AssignmentOperator) {
+    DerivedV4Check acl1("127.0.0.1/16", true);
+    DerivedV4Check acl2("192.1.2.3/12", false);
+
+    acl2 = acl1;
+    EXPECT_EQ(acl1.getAddress(), acl2.getAddress());
+    EXPECT_EQ(acl1.getMasksize(), acl2.getMasksize());
+    EXPECT_EQ(acl1.getNetmask(), acl2.getNetmask());
+    EXPECT_EQ(acl1.getInverse(), acl2.getInverse());
+}
+
+TEST(IPv4Check, V4Clone) {
+    DerivedV4Check acl1("127.0.0.1/16", true);
+
+    boost::scoped_ptr<DerivedV4Check> acl2(
+            static_cast<DerivedV4Check*>(acl1.clone()));
+    EXPECT_EQ(acl1.getAddress(), acl2->getAddress());
+    EXPECT_EQ(acl1.getMasksize(), acl2->getMasksize());
+    EXPECT_EQ(acl1.getNetmask(), acl2->getNetmask());
+    EXPECT_EQ(acl1.getInverse(), acl2->getInverse());
+}
+
 // Check that the comparison works - note that "matches" just calls the
 // internal compare() code.
 //
-// Note that addresses passed to the class are expected to be in network-
-// byte order.  Therefore for the comparisons to work as expected, we must
-// convert the values to network-byte order first.
+// As before, note that addresses passed to the class are expected to be in
+// network-byte order.  Therefore for the comparisons to work as expected, we
+// must convert the values to network-byte order first.
 
 TEST(Ipv4Check, V4Compare) {
     // Exact address - match if given address matches stored address.
@@ -172,17 +252,24 @@ TEST(Ipv4Check, V4Compare) {
     EXPECT_FALSE(acl4.matches(htonl(0x2345ffff)));
     EXPECT_TRUE(acl4.matches(htonl(0x23460000)));
     EXPECT_TRUE(acl4.matches(htonl(0x2346ffff)));
-
 }
 
 
-// IPV6 tests
 
-// Declare a derived class to allow the abstract function to be declared
-// as a concrete one.
+// *** IPV6 Tests ***
+//
+// Declare a derived class to allow a definition of the match() method to be
+// provided.
+//
+// In this case, the match will check a vector of uint8_t s representing an IPV6
+// address.
 
 class DerivedV6Check : public Ipv6Check<vector<uint8_t> > {
 public:
+    // default constructor
+    DerivedV6Check() : Ipv6Check<vector<uint8_t> >()
+    {}
+
     // Basic constructor
     DerivedV6Check(const uint8_t* address, size_t masksize = 128,
                    bool inverse = false) :
@@ -194,6 +281,11 @@ public:
         Ipv6Check<vector<uint8_t> >(address, inverse)
     {}
 
+    // Clone method
+    virtual IpBaseCheck<vector<uint8_t> >* clone() const {
+        return (new DerivedV6Check(*this));
+    }
+
     // Destructor
     virtual ~DerivedV6Check()
     {}
@@ -262,9 +354,15 @@ static const uint8_t MASK_128[] = {
     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
 };
 
-// Check that the constructor expands the network mask and stores the elements
-// correctly.  For these tests, we don't worry about the type of the context,
-// so we declare it as an int.
+// Check that a default constructor can be instantiated.
+
+TEST(Ipv6Check, V6DefaultConstructor) {
+    DerivedV6Check acl1;
+
+    // The test is needed to avoid the unused variable causing a warning or
+    // getting optimised away.
+    EXPECT_EQ(0, acl1.getMasksize());
+}
 
 TEST(Ipv6Check, V6ConstructorAddress) {
     DerivedV6Check acl1(V6ADDR_1);
@@ -273,11 +371,9 @@ TEST(Ipv6Check, V6ConstructorAddress) {
     EXPECT_TRUE(equal(stored.begin(), stored.end(), V6ADDR_1));
 }
 
-
-// The mask is stored in network byte order, so the pattern expected must
-// also be converted to network byte order for the comparison to succeed.
 TEST(Ipv6Check, V6ConstructorMask) {
-    // Valid values. 
+
+    // Valid masks...
     DerivedV6Check acl1(V6ADDR_1, 1);
     vector<uint8_t> stored = acl1.getNetmask();
     EXPECT_EQ(sizeof(MASK_1), stored.size());
@@ -304,7 +400,6 @@ TEST(Ipv6Check, V6ConstructorMask) {
     EXPECT_THROW(DerivedV6Check(V6ADDR_1, 129), isc::OutOfRange);
 }
 
-
 TEST(Ipv6Check, V6ConstructorInverse) {
     // Valid values. Address/mask of "1" is used as a placeholder
     DerivedV6Check acl1(V6ADDR_1, 1);
@@ -336,25 +431,90 @@ TEST(Ipv6Check, V6StringConstructor) {
     EXPECT_THROW(DerivedV6Check("::1/0"), isc::OutOfRange);
     EXPECT_THROW(DerivedV6Check("::1/129"), isc::OutOfRange);
     EXPECT_THROW(DerivedV6Check("::1/24/3"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV6Check("2001:0db8::dead:beef/ww"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV6Check("2xx1:0db8::dead:beef/32"), isc::InvalidParameter);
+    EXPECT_THROW(DerivedV6Check("2001:0db8::abcd/ww"), isc::InvalidParameter);
+    EXPECT_THROW(DerivedV6Check("2xx1:0db8::abcd/32"), isc::InvalidParameter);
 }
 
+TEST(Ipv6Check, V6CopyConstructor) {
+    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/52"));
+    DerivedV6Check acl2(acl1);
 
+    vector<uint8_t> acl1_address = acl1.getAddress();
+    vector<uint8_t> acl2_address = acl1.getAddress();
+    EXPECT_EQ(sizeof(V6ADDR_1), acl1_address.size());
+    EXPECT_EQ(acl1_address.size(), acl2_address.size());
+    EXPECT_TRUE(equal(acl1_address.begin(), acl1_address.end(),
+                acl2_address.begin()));
 
-// Check that the comparison works - note that "matches" just calls the
-// internal compare() code.
-//
-// Note that addresses passed to the class are expected to be in network-
-// byte order.  Therefore for the comparisons to work as expected, we must
-// convert the values to network-byte order first.
+    EXPECT_EQ(acl1.getMasksize(), acl2.getMasksize());
+
+    vector<uint8_t> acl1_netmask = acl1.getNetmask();
+    vector<uint8_t> acl2_netmask = acl1.getNetmask();
+    EXPECT_EQ(sizeof(V6ADDR_1), acl1_netmask.size());
+    EXPECT_EQ(acl1_netmask.size(), acl2_netmask.size());
+    EXPECT_TRUE(equal(acl1_netmask.begin(), acl1_netmask.end(),
+                acl2_netmask.begin()));
+
+    EXPECT_EQ(acl1.getInverse(), acl2.getInverse());
+}
+
+TEST(Ipv6Check, V6AssignmentOperator) {
+    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/52"));
+    DerivedV6Check acl2(string(V6ADDR_1_STRING) + string("/48"));
+
+    acl2 = acl1;
+
+    vector<uint8_t> acl1_address = acl1.getAddress();
+    vector<uint8_t> acl2_address = acl2.getAddress();
+    EXPECT_EQ(sizeof(V6ADDR_1), acl1_address.size());
+    EXPECT_EQ(acl1_address.size(), acl2_address.size());
+    EXPECT_TRUE(equal(acl1_address.begin(), acl1_address.end(),
+                acl2_address.begin()));
+
+    EXPECT_EQ(acl1.getMasksize(), acl2.getMasksize());
+
+    vector<uint8_t> acl1_netmask = acl1.getNetmask();
+    vector<uint8_t> acl2_netmask = acl2.getNetmask();
+    EXPECT_EQ(sizeof(V6ADDR_1), acl1_netmask.size());
+    EXPECT_EQ(acl1_netmask.size(), acl2_netmask.size());
+    EXPECT_TRUE(equal(acl1_netmask.begin(), acl1_netmask.end(),
+                acl2_netmask.begin()));
+
+    EXPECT_EQ(acl1.getInverse(), acl2.getInverse());
+}
+
+TEST(Ipv6Check, V6Clone) {
+    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/52"));
+    boost::scoped_ptr<DerivedV6Check> acl2(
+        static_cast<DerivedV6Check*>(acl1.clone()));
+
+    vector<uint8_t> acl1_address = acl1.getAddress();
+    vector<uint8_t> acl2_address = acl2->getAddress();
+    EXPECT_EQ(sizeof(V6ADDR_1), acl1_address.size());
+    EXPECT_EQ(acl1_address.size(), acl2_address.size());
+    EXPECT_TRUE(equal(acl1_address.begin(), acl1_address.end(),
+                acl2_address.begin()));
+
+    EXPECT_EQ(acl1.getMasksize(), acl2->getMasksize());
+
+    vector<uint8_t> acl1_netmask = acl1.getNetmask();
+    vector<uint8_t> acl2_netmask = acl2->getNetmask();
+    EXPECT_EQ(sizeof(V6ADDR_1), acl1_netmask.size());
+    EXPECT_EQ(acl1_netmask.size(), acl2_netmask.size());
+    EXPECT_TRUE(equal(acl1_netmask.begin(), acl1_netmask.end(),
+                acl2_netmask.begin()));
+
+    EXPECT_EQ(acl1.getInverse(), acl2->getInverse());
+}
 
 TEST(Ipv6Check, V6Compare) {
     // Set up some data.  The constant doesn't depend on the template parameter,
     // so use a type name that's short.
     vector<uint8_t> v6addr_2(V6ADDR_2, V6ADDR_2 + DerivedV6Check::IPV6_SIZE);
-    vector<uint8_t> v6addr_2_48(V6ADDR_2_48, V6ADDR_2_48 + DerivedV6Check::IPV6_SIZE);
-    vector<uint8_t> v6addr_2_52(V6ADDR_2_52, V6ADDR_2_52 + DerivedV6Check::IPV6_SIZE);
+    vector<uint8_t> v6addr_2_48(V6ADDR_2_48,
+                                V6ADDR_2_48 + DerivedV6Check::IPV6_SIZE);
+    vector<uint8_t> v6addr_2_52(V6ADDR_2_52,
+                                V6ADDR_2_52 + DerivedV6Check::IPV6_SIZE);
     vector<uint8_t> v6addr_3(V6ADDR_3, V6ADDR_3 + DerivedV6Check::IPV6_SIZE);
 
     // Exact address - match if given address matches stored address.
@@ -385,3 +545,83 @@ TEST(Ipv6Check, V6Compare) {
     EXPECT_TRUE(acl4.matches(v6addr_2_48));
     EXPECT_TRUE(acl4.matches(v6addr_3));
 }
+
+// *** IP Tests ***
+
+// Provide specializations for a simple joint struct holding both an
+// IPV4 address and an IPV6 address
+
+typedef struct {
+    bool        isv4;
+    uint32_t    v4addr;
+    uint8_t     v6addr[16];
+} GeneralAddress;
+
+namespace isc  {
+namespace acl {
+template <>
+bool Ipv4Check<GeneralAddress>::matches(const GeneralAddress& addr) const {
+    return (addr.isv4 ? compare(addr.v4addr) : false);
+}
+
+template <>
+bool Ipv6Check<GeneralAddress>::matches(const GeneralAddress& addr) const {
+    return (addr.isv4 ? false : compare(addr.v6addr));
+}
+} // namespace acl
+} // namespace isc
+
+TEST(IpCheck, V4Test) {
+    IpCheck<GeneralAddress> acl("192.168.132.255/16");
+                               //c0  a8  84  ff
+
+    GeneralAddress test;
+    test.isv4 = true;
+    test.v4addr = htonl(0xc0a884ff);
+    EXPECT_TRUE(acl.matches(test));
+
+    test.v4addr = htonl(0xc0a884ee);    // Correct to 24 bits
+    EXPECT_TRUE(acl.matches(test));
+
+    test.v4addr = htonl(0xc0a8ffee);    // Correct to 16 bits
+    EXPECT_TRUE(acl.matches(test));
+
+    test.v4addr = htonl(0xc000ffee);    // Incorrect
+    EXPECT_FALSE(acl.matches(test));
+
+    test.isv4 = false;
+    test.v4addr = htonl(0xc0a884ff);    // Correct IPV4 address
+    EXPECT_FALSE(acl.matches(test));
+
+    // Quick check for negative match
+    IpCheck<GeneralAddress> acl2("192.168.132.255/16", true);
+    test.isv4 = true;
+    test.v4addr = htonl(0xc0a884ff);
+    EXPECT_FALSE(acl.matches(test));
+}
+
+TEST(IpCheck, V6Test) {
+    IpCheck<GeneralAddress> acl(string(V6ADDR_2_STRING) + string("/52"));
+
+    GeneralAddress test;
+    test.isv4 = false;
+    copy(V6ADDR_2, V6ADDR_2 + sizeof(V6ADDR_2), test.v6addr);
+    EXPECT_TRUE(acl.matches(test));
+
+    copy(V6ADDR_2, V6ADDR_2_52 + sizeof(V6ADDR_2_52), test.v6addr);
+    EXPECT_TRUE(acl.matches(test));
+
+    copy(V6ADDR_2, V6ADDR_2_48 + sizeof(V6ADDR_2_48), test.v6addr);
+    EXPECT_FALSE(acl.matches(test));
+
+    test.isv4 = true;
+    copy(V6ADDR_2, V6ADDR_2 + sizeof(V6ADDR_2), test.v6addr);
+                                        // Correct V6 address
+    EXPECT_FALSE(acl.matches(test));
+
+    // Quick check for negative match
+    IpCheck<GeneralAddress> acl2(string(V6ADDR_2_STRING) + string("/52"), true);
+    test.isv4 = false;
+    copy(V6ADDR_2, V6ADDR_2 + sizeof(V6ADDR_2), test.v6addr);
+    EXPECT_FALSE(acl.matches(test));
+}