[1575] introduced an abstract base class of NSEC3Hash with a factory method

as suggested in review.  hid the concrete derived class in the implementation.
adjusted tests accordingly.

src/lib/dns/nsec3hash.cc

@@ -18,6 +18,8 @@
 #include <string>
 #include <vector>
 
+#include <boost/noncopyable.hpp>
+
 #include <exceptions/exceptions.h>
 
 #include <util/buffer.h>
@@ -32,19 +34,19 @@ using namespace std;
 using namespace isc::util;
 using namespace isc::util::encode;
 using namespace isc::util::hash;
+using namespace isc::dns;
 using namespace isc::dns::rdata;
 
 namespace {
+// This is the algorithm number for SHA1/NSEC3 as defined in RFC5155.
 // Currently the only pre-defined algorithm is SHA1.  So we don't
 // over-generalize it at the moment, and rather hardocde it and
 // assume that specific algorithm.
 const uint8_t NSEC3_HASH_SHA1 = 1;
-}
 
-namespace isc {
-namespace dns {
-struct NSEC3Hash::NSEC3HashImpl {
-    NSEC3HashImpl(const generic::NSEC3PARAM& param) :
+class NSEC3HashRFC5155 : boost::noncopyable, public NSEC3Hash {
+public:
+    NSEC3HashRFC5155(const generic::NSEC3PARAM& param) :
         algorithm_(param.getHashalg()),
         iterations_(param.getIterations()),
         salt_(param.getSalt()), digest_(SHA1_HASHSIZE), obuf_(Name::MAX_WIRE)
@@ -55,6 +57,10 @@ struct NSEC3Hash::NSEC3HashImpl {
         }
         SHA1Reset(&sha1_ctx_);
     }
+
+    virtual std::string calculate(const Name& name) const;
+
+private:
     const uint8_t algorithm_;
     const uint16_t iterations_;
     const vector<uint8_t> salt_;
@@ -67,15 +73,6 @@ struct NSEC3Hash::NSEC3HashImpl {
     mutable OutputBuffer obuf_;
 };
 
-NSEC3Hash::NSEC3Hash(const generic::NSEC3PARAM& param) :
-    impl_(new NSEC3HashImpl(param))
-{}
-
-NSEC3Hash::~NSEC3Hash() {
-    delete impl_;
-}
-
-namespace {
 inline void
 iterateSHA1(SHA1Context* ctx, const uint8_t* input, size_t inlength,
             const uint8_t* salt, size_t saltlen,
@@ -86,31 +83,37 @@ iterateSHA1(SHA1Context* ctx, const uint8_t* input, size_t inlength,
     SHA1Input(ctx, salt, saltlen); // this works whether saltlen == or > 0
     SHA1Result(ctx, output);
 }
-}
 
 string
-NSEC3Hash::calculate(const Name& name) const {
+NSEC3HashRFC5155::calculate(const Name& name) const {
     // We first need to normalize the name by converting all upper case
     // characters in the labels to lower ones.
-    impl_->obuf_.clear();
+    obuf_.clear();
     Name name_copy(name);
     name_copy.downcase();
-    name_copy.toWire(impl_->obuf_);
-
-    const uint8_t saltlen = impl_->salt_.size();
-    const uint8_t* const salt = (saltlen > 0) ? &impl_->salt_[0] : NULL;
-    uint8_t* const digest = &impl_->digest_[0];
-    assert(impl_->digest_.size() == SHA1_HASHSIZE);
-
-    iterateSHA1(&impl_->sha1_ctx_,
-                static_cast<const uint8_t*>(impl_->obuf_.getData()),
-                impl_->obuf_.getLength(), salt, saltlen, digest);
-    for (unsigned int n = 0; n < impl_->iterations_; ++n) {
-        iterateSHA1(&impl_->sha1_ctx_, digest, SHA1_HASHSIZE,
-                    salt, saltlen, digest);
+    name_copy.toWire(obuf_);
+
+    const uint8_t saltlen = salt_.size();
+    const uint8_t* const salt = (saltlen > 0) ? &salt_[0] : NULL;
+    uint8_t* const digest = &digest_[0];
+    assert(digest_.size() == SHA1_HASHSIZE);
+
+    iterateSHA1(&sha1_ctx_, static_cast<const uint8_t*>(obuf_.getData()),
+                obuf_.getLength(), salt, saltlen, digest);
+    for (unsigned int n = 0; n < iterations_; ++n) {
+        iterateSHA1(&sha1_ctx_, digest, SHA1_HASHSIZE, salt, saltlen, digest);
     }
 
-    return (encodeBase32Hex(impl_->digest_));
+    return (encodeBase32Hex(digest_));
+}
+} // end of unnamed namespace
+
+namespace isc {
+namespace dns {
+
+NSEC3Hash*
+NSEC3Hash::create(const generic::NSEC3PARAM& param) {
+    return (new NSEC3HashRFC5155(param));
 }
 
 } // namespace dns

src/lib/dns/nsec3hash.h

@@ -17,8 +17,6 @@
 
 #include <string>
 
-#include <boost/noncopyable.hpp>
-
 #include <exceptions/exceptions.h>
 
 namespace isc {
@@ -80,7 +78,10 @@ public:
 /// - Allow producing hash value as binary data
 /// - Allow updating NSEC3 parameters of a class object so we can still reuse
 ///   the internal resources for different sets of parameters.
-class NSEC3Hash : public boost::noncopyable {
+class NSEC3Hash {
+protected:
+    NSEC3Hash() {}
+
 public:
     /// \brief Constructor from NSEC3PARAM RDATA.
     ///
@@ -93,10 +94,10 @@ public:
     /// \throw std::bad_alloc Internal resource allocation failure.
     ///
     /// \param param NSEC3 parameters used for subsequent calculation.
-    NSEC3Hash(const rdata::generic::NSEC3PARAM& param);
+    static NSEC3Hash* create(const rdata::generic::NSEC3PARAM& param);
 
     /// \brief The destructor.
-    ~NSEC3Hash();
+    virtual ~NSEC3Hash() {}
 
     /// \brief Calculate the NSEC3 hash.
     ///
@@ -109,12 +110,9 @@ public:
     /// \param name The domain name for which the hash value is to be
     /// calculated.
     /// \return Base32hex-encoded string of the hash value.
-    std::string calculate(const Name& name) const;
-
-private:
-    struct NSEC3HashImpl;
-    NSEC3HashImpl* impl_;
+    virtual std::string calculate(const Name& name) const = 0;
 };
+
 }
 }
 #endif  // __NSEC3HASH_H

src/lib/dns/tests/nsec3hash_unittest.cc

@@ -14,51 +14,60 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/scoped_ptr.hpp>
+
 #include <dns/nsec3hash.h>
 #include <dns/rdataclass.h>
 
+using boost::scoped_ptr;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
 namespace {
+typedef scoped_ptr<NSEC3Hash> NSEC3HashPtr;
+
 class NSEC3HashTest : public ::testing::Test {
 protected:
-    NSEC3HashTest() : test_hash(generic::NSEC3PARAM("1 0 12 aabbccdd"))
+    NSEC3HashTest() :
+        test_hash(NSEC3Hash::create(generic::NSEC3PARAM("1 0 12 aabbccdd")))
     {}
 
     // An NSEC3Hash object commonly used in tests.  Parameters are borrowed
     // from the RFC5155 example.  Construction of this object implicitly
-    // checks a successful case of the constructor.
-    const NSEC3Hash test_hash;
+    // checks a successful case of the creation.
+    NSEC3HashPtr test_hash;
 };
 
 TEST_F(NSEC3HashTest, unknownAlgorithm) {
-    EXPECT_THROW(NSEC3Hash(generic::NSEC3PARAM("2 0 12 aabbccdd")),
-                 UnknownNSEC3HashAlgorithm);
+    EXPECT_THROW(NSEC3HashPtr(
+                     NSEC3Hash::create(
+                         generic::NSEC3PARAM("2 0 12 aabbccdd"))),
+                     UnknownNSEC3HashAlgorithm);
 }
 
 TEST_F(NSEC3HashTest, calculate) {
     // A couple of normal cases from the RFC5155 example.
     EXPECT_EQ("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM",
-              test_hash.calculate(Name("example")));
+              test_hash->calculate(Name("example")));
     EXPECT_EQ("35MTHGPGCU1QG68FAB165KLNSNK3DPVL",
-              test_hash.calculate(Name("a.example")));
+              test_hash->calculate(Name("a.example")));
 
     // Check case-insensitiveness
     EXPECT_EQ("0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM",
-              test_hash.calculate(Name("EXAMPLE")));
+              test_hash->calculate(Name("EXAMPLE")));
 
     // Some boundary cases: 0-iteration and empty salt.  Borrowed from the
     // .com zone data.
     EXPECT_EQ("CK0POJMG874LJREF7EFN8430QVIT8BSM",
-              NSEC3Hash(generic::NSEC3PARAM("1 0 0 -")).
-              calculate(Name("com")));
+              NSEC3HashPtr(NSEC3Hash::create(generic::NSEC3PARAM("1 0 0 -")))
+              ->calculate(Name("com")));
 
     // Using unusually large iterations, something larger than the 8-bit range.
     // (expected hash value generated by BIND 9's dnssec-signzone)
     EXPECT_EQ("COG6A52MJ96MNMV3QUCAGGCO0RHCC2Q3",
-              NSEC3Hash(generic::NSEC3PARAM("1 0 256 AABBCCDD")).
-              calculate(Name("example.org")));
+              NSEC3HashPtr(NSEC3Hash::create(
+                               generic::NSEC3PARAM("1 0 256 AABBCCDD")))
+              ->calculate(Name("example.org")));
 }
 
 } // end namespace