BIND 10 trac1575, updated. 80a2fbfd0db2b19beb0e18b3b3dc166ccdae76f6 [1575] added some non-compilable note around meta-remarks that should be removed before releasing the wrapper code. Compiler will detect if any of them is forgotten to be cleaned up.
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Jan 27 22:29:27 UTC 2012
The branch, trac1575 has been updated
via 80a2fbfd0db2b19beb0e18b3b3dc166ccdae76f6 (commit)
via 47c87e9c2cb3239284a64f9e029e52693106dccd (commit)
via ea2af1351e60e715bb70fb1b021c87e47add5d24 (commit)
via 45baff8527758f9c3610f75308d9262452c713b0 (commit)
from 81e5f0caa91e4be3b3e97d97ea6f56d92333bc16 (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 80a2fbfd0db2b19beb0e18b3b3dc166ccdae76f6
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Jan 27 14:27:42 2012 -0800
[1575] added some non-compilable note around meta-remarks that should be
removed before releasing the wrapper code. Compiler will detect if
any of them is forgotten to be cleaned up.
commit 47c87e9c2cb3239284a64f9e029e52693106dccd
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Jan 27 14:22:19 2012 -0800
[1575] documentation update to reflect the introdction of base NSEC3Hash class.
(there doesn't seem to be need for updating the python doc)
commit ea2af1351e60e715bb70fb1b021c87e47add5d24
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Jan 27 13:54:14 2012 -0800
[1575] used the NSEC3Hash factory in the python wrapper, too.
commit 45baff8527758f9c3610f75308d9262452c713b0
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Jan 27 13:49:24 2012 -0800
[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.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/nsec3hash.cc | 82 ++++++++++++++++++-------------
src/lib/dns/nsec3hash.h | 80 +++++++++++++++++-------------
src/lib/dns/python/nsec3hash_python.cc | 2 +-
src/lib/dns/tests/nsec3hash_unittest.cc | 33 ++++++++-----
src/lib/util/python/wrapper_template.cc | 17 +++---
5 files changed, 123 insertions(+), 91 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/nsec3hash.cc b/src/lib/dns/nsec3hash.cc
index 1e3eb6a..3217c67 100644
--- a/src/lib/dns/nsec3hash.cc
+++ b/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,28 @@ 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 {
-// 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) :
+/// \brief A derived class of \c NSEC3Hash that implements the standard hash
+/// calculation specified in RFC5155.
+///
+/// Currently the only pre-defined algorithm in the RFC is SHA1. So we don't
+/// over-generalize it at the moment, and rather hardocde it and assume that
+/// specific algorithm.
+///
+/// The implementation details are only open within this file, but to avoid
+/// an accidental error in this implementation we explicitly make it non
+/// copyable.
+class NSEC3HashRFC5155 : boost::noncopyable, public NSEC3Hash {
+private:
+ // This is the algorithm number for SHA1/NSEC3 as defined in RFC5155.
+ static const uint8_t NSEC3_HASH_SHA1 = 1;
+
+public:
+ NSEC3HashRFC5155(const generic::NSEC3PARAM& param) :
algorithm_(param.getHashalg()),
iterations_(param.getIterations()),
salt_(param.getSalt()), digest_(SHA1_HASHSIZE), obuf_(Name::MAX_WIRE)
@@ -55,6 +66,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 +82,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 +92,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
diff --git a/src/lib/dns/nsec3hash.h b/src/lib/dns/nsec3hash.h
index eba508a..9f39172 100644
--- a/src/lib/dns/nsec3hash.h
+++ b/src/lib/dns/nsec3hash.h
@@ -17,8 +17,6 @@
#include <string>
-#include <boost/noncopyable.hpp>
-
#include <exceptions/exceptions.h>
namespace isc {
@@ -46,57 +44,72 @@ public:
/// \brief A calculator of NSEC3 hashes.
///
-/// This is a simple class that encapsulates the algorithm of calculating
-/// NSEC3 hash values as defined in RFC5155.
+/// This is an abstract base class that defines a simple interface to
+/// calculating NSEC3 hash values as defined in RFC5155.
///
-/// This class is designed to be "stateless" in that it basically doesn't
-/// hold mutable state once constructed, and hash calculation solely depends
-/// on the parameters given on construction and input to the \c calculate()
-/// method. In that sense this could be a single free function rather than
-/// a class, but we decided to provide the functionality as a class for
-/// two reasons: NSEC3 hash calculations would often take place more than one
-/// time in a single query or validation process, so it would be more
-/// efficient if we could hold some internal resources used for the
-/// calculation and reuse it over multiple calls to \c calculate() (this
-/// implementation actually does this); Second, for testing purposes we may
-/// want to use a fake calculator that returns pre-defined hash values
-/// (so a slight change to the test input wouldn't affect the test result).
-/// Using a class would make it possible by introducing a common base class
-/// and having the application depend on that base class (then the fake
-/// calculator will be defined as a separate subclass of the base).
+/// (Derived classes of) this class is designed to be "stateless" in that it
+/// basically doesn't hold mutable state once constructed, and hash
+/// calculation solely depends on the parameters given on construction and
+/// input to the \c calculate() method. In that sense this could be a
+/// single free function rather than a class, but we decided to provide the
+/// functionality as a class for two reasons: NSEC3 hash calculations would
+/// often take place more than one time in a single query or validation
+/// process, so it would be more efficient if we could hold some internal
+/// resources used for the calculation and reuse it over multiple calls to
+/// \c calculate() (a concrete implementation in this library actually does
+/// this); Second, we may want to customize the hash calculation logic for
+/// testing purposes or for other future extensions. For example, we may
+/// want to use a fake calculator for tests that returns pre-defined hash
+/// values (so a slight change to the test input wouldn't affect the test
+/// result). Using classes from this base would make it possible more
+/// transparently to the application.
///
-/// The initial implementation makes this class non copyable as it wouldn't
-/// used be passed from one place to another, especially if and when it's
-/// used via a base class abstraction. But there's no fundamental reason
-/// this cannot be copied, so if we see a specific need for it, this
-/// restriction can be revisited.
+/// A specific derived class instance must be created by the factory method,
+/// \c create().
///
/// There can be several ways to extend this class in future. Those include:
-/// - Introduce a base class and make it derived from it (mainly for testing
-/// purposes as noted above)
+/// - Allow customizing the factory method so the application change the
+/// behavior dynamically.
/// - Allow to construct the class from a tuple of parameters, that is,
/// integers for algorithm, iterations and flags, and opaque salt data.
/// For example, we might want to use that version for validators.
/// - 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:
+ /// \brief The default constructor.
+ ///
+ /// This is defined as protected to prevent this class from being directly
+ /// instantiated even if the class definition is modified (accidentally
+ /// or intentionally) to have no pure virtual methods.
+ NSEC3Hash() {}
+
public:
- /// \brief Constructor from NSEC3PARAM RDATA.
+ /// \brief Factory method of NSECHash from NSEC3PARAM RDATA.
///
/// The hash algorithm given via \c param must be known to the
/// implementation. Otherwise \c UnknownNSEC3HashAlgorithm exception
/// will be thrown.
///
+ /// This method creates an \c NSEC3Hash object using \c new. The caller
+ /// is responsible for releasing it with \c delete that is compatible to
+ /// the one used in this library. In practice, the application would
+ /// generally need to store the returned pointer in some form of smart
+ /// pointer; otherwise the resulting code will be quite fragile against
+ /// exceptions (and in this case the application doesn't have to worry
+ /// about explicit \c delete).
+ ///
/// \throw UnknownNSEC3HashAlgorithm The specified algorithm in \c param
/// is unknown.
/// \throw std::bad_alloc Internal resource allocation failure.
///
/// \param param NSEC3 parameters used for subsequent calculation.
- NSEC3Hash(const rdata::generic::NSEC3PARAM& param);
+ /// \return A pointer to a concrete derived object of \c NSEC3Hash.
+ static NSEC3Hash* create(const rdata::generic::NSEC3PARAM& param);
/// \brief The destructor.
- ~NSEC3Hash();
+ virtual ~NSEC3Hash() {}
/// \brief Calculate the NSEC3 hash.
///
@@ -109,12 +122,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
diff --git a/src/lib/dns/python/nsec3hash_python.cc b/src/lib/dns/python/nsec3hash_python.cc
index 52ce08f..cd97a80 100644
--- a/src/lib/dns/python/nsec3hash_python.cc
+++ b/src/lib/dns/python/nsec3hash_python.cc
@@ -59,7 +59,7 @@ NSEC3Hash_init(PyObject* po_self, PyObject* args, PyObject*) {
"not %.200s", po_rdata->ob_type->tp_name);
return (-1);
}
- self->cppobj = new NSEC3Hash(
+ self->cppobj = NSEC3Hash::create(
dynamic_cast<const generic::NSEC3PARAM&>(
PyRdata_ToRdata(po_rdata)));
return (0);
diff --git a/src/lib/dns/tests/nsec3hash_unittest.cc b/src/lib/dns/tests/nsec3hash_unittest.cc
index 44c04e6..c0fc9d9 100644
--- a/src/lib/dns/tests/nsec3hash_unittest.cc
+++ b/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
diff --git a/src/lib/util/python/wrapper_template.cc b/src/lib/util/python/wrapper_template.cc
index 1a67a88..780e695 100644
--- a/src/lib/util/python/wrapper_template.cc
+++ b/src/lib/util/python/wrapper_template.cc
@@ -33,14 +33,6 @@ using namespace isc::@MODULE@;
using namespace isc::@MODULE@::python;
//
-// Definition of the classes
-//
-
-// For each class, we need a struct, a helper functions (init, destroy,
-// and static wrappers around the methods we export), a list of methods,
-// and a type description
-
-//
// @CPPCLASS@
//
@@ -52,6 +44,7 @@ namespace {
// Shortcut type which would be convenient for adding class variables safely.
typedef CPPPyObjectContainer<s_ at CPPCLASS@, @CPPCLASS@> @CPPCLASS at Container;
+ at REMOVE_THIS_ON_RELEASE@
// This is a template of typical code logic of python class initialization
// with C++ backend. You'll need to adjust it according to details of the
// actual C++ class.
@@ -60,6 +53,7 @@ int
s_ at CPPCLASS@* self = static_cast<s_ at CPPCLASS@*>(po_self);
try {
if (PyArg_ParseTuple(args, "REPLACE ME")) {
+ @REMOVE_THIS_ON_RELEASE@
// YOU'LL NEED SOME VALIDATION, PREPARATION, ETC, HERE.
self->cppobj = new @CPPCLASS@(/*NECESSARY PARAMS*/);
return (0);
@@ -74,6 +68,7 @@ int
return (-1);
}
+ @REMOVE_THIS_ON_RELEASE@
// If we are here PyArg_ParseTuple() failed and TypeError should have
// been set. If the constructor is more complicated and the control
// could reach this point for other reasons, an appropriate Python
@@ -82,6 +77,7 @@ int
return (-1);
}
+ at REMOVE_THIS_ON_RELEASE@
// This is a template of typical code logic of python object destructor.
// In many cases you can use it without modification, but check that carefully.
void
@@ -92,6 +88,7 @@ void
Py_TYPE(self)->tp_free(self);
}
+ at REMOVE_THIS_ON_RELEASE@
// This should be able to be used without modification as long as the
// underlying C++ class has toText().
PyObject*
@@ -119,6 +116,7 @@ PyObject*
const_cast<char*>("")));
}
+ at REMOVE_THIS_ON_RELEASE@
// This is quite specific isc.dns. For other wrappers this should probably
// be removed.
PyObject* @CPPCLASS at _toWire(PyObject* self, PyObject* args) {
@@ -175,6 +173,8 @@ PyObject*
PyMethodDef @CPPCLASS at _methods[] = {
{ "to_text", @CPPCLASS at _toText, METH_NOARGS,
@CPPCLASS at _toText_doc },
+
+ @REMOVE_THIS_ON_RELEASE@
// This is quite specific isc.dns. For other wrappers this should probably
// be removed:
{ "to_wire", @CPPCLASS at _toWire, METH_VARARGS,
@@ -256,6 +256,7 @@ initModulePart_ at CPPCLASS@(PyObject* mod) {
}
Py_INCREF(&@cppclass at _type);
+ @REMOVE_THIS_ON_RELEASE@
// The following template is the typical procedure for installing class
// variables. If the class doesn't have a class variable, remove the
// entire try-catch clauses.
More information about the bind10-changes
mailing list