sync with trunk

also fixed a bug in recursor which caused a segfault (wrong iterator use in a foreach)


git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac327@3783 e5f2f494-b856-4b98-b285-d166d9295462

ChangeLog

@@ -12,6 +12,62 @@
 	It has "listen_on" and "forward_addresses" options.
 	(Trac #389, r3448)
 
+bind10-devel-20101201 released on December 01, 2010
+
+  125.  [func]		jelte
+	Added support for addressing individual list items in bindctl
+	configuration commands; If you have an element that is a list, you
+	can use foo[X] to address a specific item, where X is an integer
+	(starting at 0)
+	(Trac #405, svn r3739)
+
+  124.  [bug]		jreed
+	Fix some wrong version reporting. Now also show the version
+	for the component and BIND 10 suite. (Trac #302, svn r3696)
+
+  123.  [bug]		jelte
+	src/bin/bindctl printed values had the form of python literals
+	(e.g. 'True'), while the input requires valid JSON (e.g. 'true').
+	Output changed to JSON format for consistency. (svn r3694)
+
+  122.  [func]		stephen
+	src/bin/bind10: Added configuration options to Boss to determine
+	whether to start the authoritative server, recursive server (or
+	both). A dummy recursor has been provided for test purposes.
+	(Trac #412, svn r3676)
+
+  121.  [func]		jinmei
+	src/lib/dns: Added support for TSIG RDATA.  At this moment this is
+	not much of real use, however, because no protocol support was
+	added yet.  It will soon be added. (Trac #372, svn r3649)
+
+  120.  [func]		jinmei
+	src/lib/dns: introduced two new classes, TSIGKey and TSIGKeyRing,
+	to manage TSIG keys. (Trac #381, svn r3622)
+
+  119.	[bug]		jinmei
+	The master file parser of the python datasrc module incorrectly
+	regarded a domain name beginning with a decimal number as a TTL
+	specification.  This confused b10-loadzone and had it reject to
+	load a zone file that contains such a name.
+	Note: this fix is incomplete and the loadzone would still be
+	confused if the owner name is a syntactically indistinguishable
+	from a TTL specification.  This is part of a more general issue
+	and will be addressed in Trac #413. (Trac #411, svn r3599)
+
+  118.	[func]		jinmei
+	src/lib/dns: changed the interface of
+	AbstractRRset::getRdataIterator() so that the internal
+	cursor would point to the first RDATA automatically.  This
+	will be a more intuitive and less error prone behavior.
+	This is a backward compatible change. (Trac #410, r3595)
+
+  117.  [func]		jinmei
+	src/lib/datasrc: added new zone and zone table classes for the
+	support of in memory data source.  This is an intermediate step to
+	the bigger feature, and is not yet actually usable in practice.
+	(Trac #399, svn r3590)
+
   116.	[bug]		jerry
 	src/bin/xfrout: Xfrout and Auth will communicate by long tcp
 	connection, Auth needs to make a new connection only on the first
@@ -40,15 +96,13 @@
 	Add one mixin class to override the naive serve_forever() provided
 	in python library socketserver. Instead of polling for shutdwon
 	every poll_interval seconds, one socketpair is used to wake up
-	the waiting server.(Trac #352, svn r3366)
+	the waiting server. (Trac #352, svn r3366)
 
   111.	[bug]*   zhanglikun, Michal Vaner
-	Make sure process xfrin/xfrout/zonemgr/cmdctl can be stoped
+	Make sure process xfrin/xfrout/zonemgr/cmdctl can be stopped
 	properly when user enter "ctrl+c" or 'Boss shutdown' command
-	through	bindctl.
-
-	The ZonemgrRefresh.run_timer and NotifyOut.dispatcher spawn
-	a thread themselves.
+	through bindctl.  The ZonemgrRefresh.run_timer and
+	NotifyOut.dispatcher spawn a thread themselves.
 	(Trac #335, svn r3273)
 
   110.  [func]      Michal Vaner
@@ -60,7 +114,7 @@
   109.  [func]		naokikambe
 	Added the initial version of the stats module for the statistics
 	feature of BIND 10, which supports the restricted features and
-	items and reports via bindctl command (Trac #191, r3218)
+	items and reports via bindctl command. (Trac #191, r3218)
 	Added the document of the stats module, which is about how stats
 	module collects the data (Trac #170, [wiki:StatsModule])
 
@@ -87,11 +141,11 @@
   104.	[bug]		jerry
 	bin/zonemgr: zonemgr should be attempting to refresh expired zones.
 	(Trac #336, r3139)
-				   
+ 
   103.	[bug]		jerry
 	lib/python/isc/log: Fixed an issue with python logging,
-	python log shouldn't die with OSError.(Trac #267, r3137)
-				   
+	python log shouldn't die with OSError. (Trac #267, r3137)
+ 
   102.	[build]		jinmei
 	Disable threads in ASIO to minimize build time dependency.
 	(Trac #345, r3100)
@@ -144,7 +198,7 @@ bind10-devel-20100917 released on September 17, 2010
   93.	[bug]		jinmei
 	lib/datasrc: A DS query could crash the library (and therefore,
 	e.g. the authoritative server) if some RR of the same apex name
-	is stored in the hot spot cache.  (Trac #307, svn r2923)
+	is stored in the hot spot cache. (Trac #307, svn r2923)
 
   92.	[func]*		jelte
 	libdns_python (the python wrappers for libdns++) has been renamed
@@ -324,7 +378,7 @@ bind10-devel-20100701 released on July 1, 2010
   66.  [bug]		each
 	Check for duplicate RRsets before inserting data into a message
 	section; this, among other things, will prevent multiple copies
-	of the same CNAME from showing up when there's a loop.  (Trac #69,
+	of the same CNAME from showing up when there's a loop. (Trac #69,
 	svn r2350)
     
   65.  [func]		shentingting
@@ -446,7 +500,7 @@ bind10-devel-20100602 released on June 2, 2010
 	#205, svn r1957)
 
   44.   [build]         jreed
-	Install headers for libdns and libexception.  (Trac #68,
+	Install headers for libdns and libexception. (Trac #68,
 	svn r1941)
 
   43.   [func]          jelte
@@ -454,7 +508,7 @@ bind10-devel-20100602 released on June 2, 2010
 
   42.   [func]          jelte
 	lib/python/isc/config:      Make temporary file with python
-	tempfile module instead of manual with fixed name.  (Trac
+	tempfile module instead of manual with fixed name. (Trac
 	#184, svn r1859)
 
   41.   [func]          jelte
@@ -462,7 +516,7 @@ bind10-devel-20100602 released on June 2, 2010
 
   40.   [build]         jreed
 	Report detected features and configure settings at end of
-	configure output.  (svn r1836)
+	configure output. (svn r1836)
 
   39.   [func]*         each
 	Renamed libauth to libdatasrc.
@@ -475,7 +529,7 @@ bind10-devel-20100602 released on June 2, 2010
 	(Trac #135, #151, #134, svn r1797)
 
   37.   [build]         jinmei
-	Check for the availability of python-config.  (Trac #159,
+	Check for the availability of python-config. (Trac #159,
 	svn r1794)
 
   36.	[func]		shane
@@ -520,7 +574,7 @@ bind10-devel-20100421 released on April 21, 2010
 
   27.	[build]
 	Add missing copyright license statements to various source
-	files.  (svn r1750)
+	files. (svn r1750)
 
   26.	[func]
 	Use PACKAGE_STRING (name + version) from config.h instead

Makefile.am

@@ -37,7 +37,7 @@ report-coverage:
 			\*_unittest.cc \
 			\*_unittests.h \
 		--output report.info
-	$(GENHTML) -o coverage report.info 
+	$(GENHTML) --legend -o coverage report.info 
 
 coverage: clean-coverage perform-coverage report-coverage
 

README

@@ -17,8 +17,9 @@ This release includes the bind10 master process, b10-msgq message
 bus, b10-auth authoritative DNS server (with SQLite3 backend),
 b10-cmdctl remote control daemon, b10-cfgmgr configuration manager,
 b10-xfrin AXFR inbound service, b10-xfrout outgoing AXFR service,
-b10-zonemgr secondary manager, and a new libdns++ library for C++
-with a python wrapper.
+b10-zonemgr secondary manager, b10-stats statistics collection and
+reporting daemon, and a new libdns++ library for C++ with a python
+wrapper.
 
 Documentation is included and also available via the BIND 10
 website at http://bind10.isc.org/

configure.ac

@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.59])
-AC_INIT(bind10-devel, 20101013, bind10-dev@isc.org)
+AC_INIT(bind10-devel, 20101201, bind10-dev@isc.org)
 AC_CONFIG_SRCDIR(README)
 AM_INIT_AUTOMAKE
 AC_CONFIG_HEADERS([config.h])
@@ -496,6 +496,7 @@ AC_CONFIG_FILES([Makefile
                  src/lib/python/isc/util/Makefile
                  src/lib/python/isc/util/tests/Makefile
                  src/lib/python/isc/datasrc/Makefile
+                 src/lib/python/isc/datasrc/tests/Makefile
                  src/lib/python/isc/cc/Makefile
                  src/lib/python/isc/cc/tests/Makefile
                  src/lib/python/isc/config/Makefile
@@ -536,6 +537,8 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/xfrout/xfrout.spec.pre
            src/bin/xfrout/tests/xfrout_test
            src/bin/xfrout/run_b10-xfrout.sh
+           src/bin/recurse/recurse.spec.pre
+           src/bin/recurse/spec_config.h.pre
            src/bin/zonemgr/zonemgr.py
            src/bin/zonemgr/zonemgr.spec.pre
            src/bin/zonemgr/tests/zonemgr_test
@@ -563,8 +566,6 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/msgq/run_msgq.sh
            src/bin/auth/auth.spec.pre
            src/bin/auth/spec_config.h.pre
-           src/bin/recurse/recurse.spec.pre
-           src/bin/recurse/spec_config.h.pre
            src/bin/tests/process_rename_test.py
            src/lib/config/tests/data_def_unittests_config.h
            src/lib/python/isc/config/tests/config_test
@@ -580,6 +581,7 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            chmod +x src/bin/cmdctl/run_b10-cmdctl.sh
            chmod +x src/bin/xfrin/run_b10-xfrin.sh
            chmod +x src/bin/xfrout/run_b10-xfrout.sh
+           chmod +x src/bin/recurse/run_b10-recurse.sh
            chmod +x src/bin/zonemgr/run_b10-zonemgr.sh
            chmod +x src/bin/stats/tests/stats_test
            chmod +x src/bin/stats/run_b10-stats.sh

src/bin/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = bind10 bindctl cfgmgr loadzone msgq host cmdctl auth recurse xfrin \
-	xfrout usermgr zonemgr stats tests
+SUBDIRS = bind10 bindctl cfgmgr loadzone msgq host cmdctl auth xfrin xfrout \
+	usermgr zonemgr stats tests recurse
 
 check-recursive: all-recursive

src/bin/auth/Makefile.am

@@ -37,6 +37,7 @@ spec_config.h: spec_config.h.pre
 BUILT_SOURCES = spec_config.h 
 pkglibexec_PROGRAMS = b10-auth
 b10_auth_SOURCES = auth_srv.cc auth_srv.h
+b10_auth_SOURCES += query.cc query.h
 b10_auth_SOURCES += change_user.cc change_user.h
 b10_auth_SOURCES += common.h
 b10_auth_SOURCES += main.cc

src/bin/auth/auth_srv.h

@@ -147,7 +147,7 @@ public:
     /// containing the result of the update operation.
     isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr config);
 
-    /// \param Returns the command and configuration session for the
+    /// \brief Returns the command and configuration session for the
     /// \c AuthSrv.
     ///
     /// This method never throws an exception.
@@ -223,6 +223,7 @@ public:
     /// is shutdown.
     ///
     void setXfrinSession(isc::cc::AbstractSession* xfrin_session);
+
 private:
     AuthSrvImpl* impl_;
     asiolink::IOService* io_service_;

src/lib/dns/tests/tsig_unittest.cc

@@ -12,30 +12,31 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-// $Id: rrtype_unittest.cc 476 2010-01-19 00:29:28Z jinmei $
+#include <dns/message.h>
+#include <dns/rcode.h>
 
-#include <gtest/gtest.h>
+#include <datasrc/zonetable.h>
 
-#include <dns/tsig.h>
+#include <auth/query.h>
 
-#include <dns/tests/unittest_util.h>
-
-using isc::UnitTestUtil;
-using namespace std;
 using namespace isc::dns;
-
-namespace {
-class TsigTest : public ::testing::Test {
-protected:
-    TsigTest() {}
-};
-
-// simple creation test to get the testing ball rolling
-TEST_F(TsigTest, creates) {
-    Tsig tsig(Name("example.com"), Tsig::HMACMD5, "someRandomData");
-    EXPECT_TRUE(1);
+using namespace isc::datasrc;
+
+namespace isc {
+namespace auth {
+void
+Query::process() const {
+    const ZoneTable::FindResult result = zone_table_.find(qname_);
+
+    if (result.code != ZoneTable::SUCCESS &&
+        result.code != ZoneTable::PARTIALMATCH) {
+        response_.setRcode(Rcode::SERVFAIL());
+        return;
+    }
+
+    // Right now we have no code to search the zone, so we simply return
+    // NXDOMAIN for tests.
+    response_.setRcode(Rcode::NXDOMAIN());
+}
+}
 }
-
-} // end namespace
-
-

src/bin/auth/query.h

@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+ * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+ * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+ * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+ * PERFORMANCE OF THIS SOFTWARE.
+ */
+
+namespace isc {
+namespace dns {
+class Message;
+class Name;
+class RRType;
+}
+
+namespace datasrc {
+class ZoneTable;
+}
+
+namespace auth {
+
+/// The \c Query class represents a standard DNS query that encapsulates
+/// processing logic to answer the query.
+///
+/// Many of the design details for this class are still in flux.
+/// We'll revisit and update them as we add more functionality, for example:
+/// - zone_table parameter of the constructor.  This will eventually be
+///   replaced with a generic DataSrc object, or perhaps a notion of "view".
+/// - as a related point, we may have to pass the RR class of the query.
+///   in the initial implementation the RR class is an attribute of zone
+///   table and omitted.  It's not clear if this assumption holds with
+///   generic data sources.  On the other hand, it will help keep
+///   implementation simpler, and we might rather want to modify the design
+///   of the data source on this point.
+/// - return value of process().  rather than or in addition to setting the
+///   Rcode, we might use it as a return value of \c process().
+/// - we'll have to be able to specify whether DNSSEC is requested.
+///   It's an open question whether it should be in the constructor or via a
+///   separate attribute setter.
+/// - likewise, we'll eventually need to do per zone access control, for which
+///   we need querier's information such as its IP address.
+/// - zone_table (or DataSrc eventually) and response may better be parameters
+///   to process() instead of the constructor.
+///
+/// <b>Note:</b> The class name is intentionally the same as the one used in
+/// the datasrc library.  This is because the plan is to eventually merge
+/// the two classes.  We could give it a different name such as "AuthQuery"
+/// to avoid possible ambiguity, but it may sound redundant in that it's
+/// obvious that this class is for authoritative queries.
+/// Since the interfaces are very different for now and it's less
+/// likely to misuse one of the classes instead of the other
+/// accidentally, and since it's considered a temporary development state,
+/// we keep this name at the moment.
+class Query {
+public:
+    /// Constructor from query parameters.
+    ///
+    /// This constructor never throws an exception.
+    ///
+    /// \param zone_table The zone table wherein the answer to the query is
+    /// to be found.
+    /// \param qname The query name
+    /// \param qtype The RR type of the query
+    /// \param response The response message to store the answer to the query.
+    Query(const isc::datasrc::ZoneTable& zone_table,
+          const isc::dns::Name& qname, const isc::dns::RRType& qtype,
+          isc::dns::Message& response) :
+        zone_table_(zone_table), qname_(qname), qtype_(qtype),
+        response_(response)
+    {}
+
+    /// Process the query.
+    ///
+    /// This method first identifies the zone that best matches the query
+    /// name (and in some cases RR type when the search is dependent on the
+    /// type) and then searches the zone for an entry that best matches the
+    /// query name.
+    /// It then updates the response message accordingly; for example, a
+    /// successful search would result in adding a corresponding RRset to
+    /// the answer section of the response.
+    ///
+    /// If no matching zone is found in the zone table, the RCODE of
+    /// SERVFAIL will be set in the response.
+    /// <b>Note:</b> this is different from the error code that BIND 9 returns
+    /// by default when it's configured as an authoritative-only server (and
+    /// from the behavior of the BIND 10 datasrc library, which was implemented
+    /// to be compatible with BIND 9).
+    /// The difference comes from the fact that BIND 9 returns REFUSED as a
+    /// result of access control check on the use of its cache.
+    /// Since BIND 10's authoritative server doesn't have the notion of cache
+    /// by design, it doesn't make sense to return REFUSED.  On the other hand,
+    /// providing compatible behavior may have its own benefit, so this point
+    /// should be revisited later.
+    ///
+    /// Right now this method never throws an exception, but it may in a
+    /// future version.
+    void process() const;
+
+private:
+    const isc::datasrc::ZoneTable& zone_table_;
+    const isc::dns::Name& qname_;
+    const isc::dns::RRType& qtype_;
+    isc::dns::Message& response_;
+};
+
+}
+}
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/tests/Makefile.am

@@ -19,8 +19,10 @@ TESTS += run_unittests
 run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += ../auth_srv.h ../auth_srv.cc
+run_unittests_SOURCES += ../query.h ../query.cc
 run_unittests_SOURCES += ../change_user.h ../change_user.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
+run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += change_user_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)

src/bin/auth/tests/query_unittest.cc

@@ -0,0 +1,70 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/message.h>
+#include <dns/name.h>
+#include <dns/rcode.h>
+#include <dns/rrtype.h>
+
+#include <datasrc/zonetable.h>
+
+#include <auth/query.h>
+
+#include <gtest/gtest.h>
+
+using namespace isc::dns;
+using namespace isc::datasrc;
+using namespace isc::auth;
+
+namespace {
+class QueryTest : public ::testing::Test {
+protected:
+    QueryTest() :
+        qname(Name("www.example.com")), qclass(RRClass::IN()),
+        qtype(RRType::A()), response(Message::RENDER),
+        query(zone_table, qname, qtype, response)
+    {
+        response.setRcode(Rcode::NOERROR());
+    }
+    ZoneTable zone_table;
+    const Name qname;
+    const RRClass qclass;
+    const RRType qtype;
+    Message response;
+    Query query;
+};
+
+TEST_F(QueryTest, noZone) {
+    // There's no zone in the zone table.  So the response should have
+    // SERVFAIL.
+    query.process();
+    EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
+}
+
+TEST_F(QueryTest, matchZone) {
+    // add a matching zone.  since the zone is empty right now, the response
+    // should have NXDOMAIN.
+    zone_table.add(ZonePtr(new MemoryZone(qclass, Name("example.com"))));
+    query.process();
+    EXPECT_EQ(Rcode::NXDOMAIN(), response.getRcode());
+}
+
+TEST_F(QueryTest, noMatchZone) {
+    // there's a zone in the table but it doesn't match the qname.  should
+    // result in SERVFAIL.
+    zone_table.add(ZonePtr(new MemoryZone(qclass, Name("example.org"))));
+    query.process();
+    EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
+}
+}

src/bin/bind10/bind10.py.in

@@ -15,7 +15,7 @@
 # NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-"""\
+"""
 This file implements the Boss of Bind (BoB, or bob) program.
 
 Its purpose is to start up the BIND 10 system, and then manage the
@@ -72,7 +72,7 @@ isc.util.process.rename(sys.argv[0])
 # This is the version that gets displayed to the user.
 # The VERSION string consists of the module name, the module version
 # number, and the overall BIND 10 version number (set in configure.ac).
-VERSION = "bind10 20100916 (BIND 10 @PACKAGE_VERSION@)"
+VERSION = "bind10 20101129 (BIND 10 @PACKAGE_VERSION@)"
 
 # This is for bind10.boottime of stats module
 _BASETIME = time.gmtime()
@@ -189,26 +189,23 @@ class ProcessInfo:
     def respawn(self):
         self._spawn()
 
+class CChannelConnectError(Exception): pass
+
 class BoB:
     """Boss of BIND class."""
     
     def __init__(self, msgq_socket_file=None, dns_port=5300, address=None,
                  forward=None, nocache=False, verbose=False, setuid=None,
                  username=None):
-        """Initialize the Boss of BIND. This is a singleton (only one
-        can run).
+        """
+            Initialize the Boss of BIND. This is a singleton (only one can run).
         
-        The msgq_socket_file specifies the UNIX domain socket file
-        that the msgq process listens on.
-        If verbose is True, then the boss reports what it is doing.
+            The msgq_socket_file specifies the UNIX domain socket file that the
+            msgq process listens on.  If verbose is True, then the boss reports
+            what it is doing.
         """
-        self.verbose = verbose
-        self.msgq_socket_file = msgq_socket_file
+        self.address = address
         self.dns_port = dns_port
-        self.address = None
-        self.nocache = nocache
-        if address:
-            self.address = address
         self.forward = None
         self.recursive = False
         if forward:
@@ -217,137 +214,215 @@ class BoB:
             self.nocache = False
         self.cc_session = None
         self.ccs = None
-        self.processes = {}
+        self.cfg_start_auth = True
+        self.cfg_start_recurse = False
+        self.curproc = None
         self.dead_processes = {}
+        self.msgq_socket_file = msgq_socket_file
+        self.nocache = nocache
+        self.processes = {}
         self.runnable = False
         self.uid = setuid
         self.username = username
+        self.verbose = verbose
 
     def config_handler(self, new_config):
         if self.verbose:
-            sys.stdout.write("[bind10] handling new config:\n")
-            sys.stdout.write(new_config + "\n")
+            sys.stdout.write("[bind10] Handling new configuration: " +
+                str(new_config) + "\n")
         answer = isc.config.ccsession.create_answer(0)
         return answer
         # TODO
 
     def command_handler(self, command, args):
         if self.verbose:
-            sys.stdout.write("[bind10] Boss got command:\n")
-            sys.stdout.write(command + "\n")
+            sys.stdout.write("[bind10] Boss got command: " + command + "\n")
         answer = isc.config.ccsession.create_answer(1, "command not implemented")
         if type(command) != str:
             answer = isc.config.ccsession.create_answer(1, "bad command")
         else:
-            cmd = command
-            if cmd == "shutdown":
-                sys.stdout.write("[bind10] got shutdown command\n")
+            if command == "shutdown":
                 self.runnable = False
                 answer = isc.config.ccsession.create_answer(0)
             else:
                 answer = isc.config.ccsession.create_answer(1, 
                                                             "Unknown command")
         return answer
-    
-    def startup(self):
-        """Start the BoB instance.
- 
-        Returns None if successful, otherwise an string describing the
-        problem.
+
+    def kill_started_processes(self):
+        """
+            Called as part of the exception handling when a process fails to
+            start, this runs through the list of started processes, killing
+            each one.  It then clears that list.
         """
-        # try to connect to the c-channel daemon, 
-        # to see if it is already running
-        c_channel_env = {}
-        if self.msgq_socket_file is not None:
-             c_channel_env["BIND10_MSGQ_SOCKET_FILE"] = self.msgq_socket_file 
         if self.verbose:
-            sys.stdout.write("[bind10] Checking for already running b10-msgq\n")
-        # try to connect, and if we can't wait a short while
-        try:
-            self.cc_session = isc.cc.Session(self.msgq_socket_file)
-            return "b10-msgq already running, or socket file not cleaned , cannot start"
-        except isc.cc.session.SessionError:
-            # this is the case we want, where the msgq is not running
-            pass
+            sys.stdout.write("[bind10] killing started processes:\n")
+
+        for pid in self.processes:
+            if self.verbose:
+                sys.stdout.write("[bind10] - %s\n" % self.processes[pid].name)
+            self.processes[pid].process.kill()
+        self.processes = {}
 
-        # start the c-channel daemon
+    def read_bind10_config(self):
+        """
+            Reads the parameters associated with the BoB module itself.
+
+            At present these are the components to start although arguably this
+            information should be in the configuration for the appropriate
+            module itself. (However, this would cause difficulty in the case of
+            xfrin/xfrout and zone manager as we don't need to start those if we
+            are not running the authoritative server.)
+        """
         if self.verbose:
-            if self.msgq_socket_file:
-                sys.stdout.write("[bind10] Starting b10-msgq\n")
-        try:
-            c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], c_channel_env,
-                                    True, not self.verbose, uid=self.uid,
-                                    username=self.username)
-        except Exception as e:
-            return "Unable to start b10-msgq; " + str(e)
-        self.processes[c_channel.pid] = c_channel
+            sys.stdout.write("[bind10] Reading Boss configuration:\n")
+
+        config_data = self.ccs.get_full_config()
+        self.cfg_start_auth = config_data.get("start_auth")
+        self.cfg_start_recurse = config_data.get("start_recurse")
+
         if self.verbose:
-            sys.stdout.write("[bind10] Started b10-msgq (PID %d)\n" % 
-                             c_channel.pid)
+            sys.stdout.write("[bind10] - start_auth: %s\n" %
+                str(self.cfg_start_auth))
+            sys.stdout.write("[bind10] - start_recurse: %s\n" %
+                str(self.cfg_start_recurse))
+
+    def log_starting(self, process, port = None, address = None):
+        """
+            A convenience function to output a "Starting xxx" message if the
+            verbose option is set.  Putting this into a separate method ensures
+            that the output form is consistent across all processes.
+
+            The process name (passed as the first argument) is put into
+            self.curproc, and is used to indicate which process failed to
+            start if there is an error (and is used in the "Started" message
+            on success).  The optional port and address information are
+            appended to the message (if present).
+        """
+        self.curproc = process
+        if self.verbose:
+            sys.stdout.write("[bind10] Starting %s" % self.curproc)
+            if port is not None:
+                sys.stdout.write(" on port %d" % port)
+                if address is not None:
+                    sys.stdout.write(" (address %s)" % str(address))
+            sys.stdout.write("\n")
 
-        # now connect to the c-channel
+    def log_started(self, pid = None):
+        """
+            A convenience function to output a 'Started xxxx (PID yyyy)'
+            message.  As with starting_message(), this ensures a consistent
+            format.
+        """
+        if self.verbose:
+            sys.stdout.write("[bind10] Started %s" % self.curproc)
+            if pid is not None:
+                sys.stdout.write(" (PID %d)" % pid)
+            sys.stdout.write("\n")
+
+    # The next few methods start the individual processes of BIND-10.  They
+    # are called via start_all_process().  If any fail, an exception is raised
+    # which is caught by the caller of start_all_processes(); this kills
+    # processes started up to that point before terminating the program.
+
+    def start_msgq(self, c_channel_env):
+        """
+            Start the message queue and connect to the command channel.
+        """
+        self.log_starting("b10-msgq")
+        c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], c_channel_env,
+                                True, not self.verbose, uid=self.uid,
+                                username=self.username)
+        self.processes[c_channel.pid] = c_channel
+        self.log_started(c_channel.pid)
+
+        # Now connect to the c-channel
         cc_connect_start = time.time()
         while self.cc_session is None:
             # if we have been trying for "a while" give up
             if (time.time() - cc_connect_start) > 5:
-                c_channel.process.kill()
-                return "Unable to connect to c-channel after 5 seconds"
+                raise CChannelConnectError("Unable to connect to c-channel after 5 seconds")
+
             # try to connect, and if we can't wait a short while
             try:
                 self.cc_session = isc.cc.Session(self.msgq_socket_file)
             except isc.cc.session.SessionError:
                 time.sleep(0.1)
 
-        # start the configuration manager
-        if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-cfgmgr\n")
-        try:
-            bind_cfgd = ProcessInfo("b10-cfgmgr", ["b10-cfgmgr"],
-                                    c_channel_env, uid=self.uid,
-                                    username=self.username)
-        except Exception as e:
-            c_channel.process.kill()
-            return "Unable to start b10-cfgmgr; " + str(e)
+    def start_cfgmgr(self, c_channel_env):
+        """
+            Starts the configuration manager process
+        """
+        self.log_starting("b10-cfgmgr")
+        bind_cfgd = ProcessInfo("b10-cfgmgr", ["b10-cfgmgr"],
+                                c_channel_env, uid=self.uid,
+                                username=self.username)
         self.processes[bind_cfgd.pid] = bind_cfgd
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-cfgmgr (PID %d)\n" % 
-                             bind_cfgd.pid)
+        self.log_started(bind_cfgd.pid)
 
         # sleep until b10-cfgmgr is fully up and running, this is a good place
         # to have a (short) timeout on synchronized groupsend/receive
         # TODO: replace the sleep by a listen for ConfigManager started
         # message
         time.sleep(1)
-        if self.verbose:
-            sys.stdout.write("[bind10] starting ccsession\n")
+
+    def start_ccsession(self, c_channel_env):
+        """
+            Start the CC Session
+
+            The argument c_channel_env is unused but is supplied to keep the
+            argument list the same for all start_xxx methods.
+        """
+        self.log_starting("ccsession")
         self.ccs = isc.config.ModuleCCSession(SPECFILE_LOCATION, 
                                       self.config_handler, self.command_handler)
         self.ccs.start()
+        self.log_started()
+
+    # A couple of utility methods for starting processes...
+
+    def start_process(self, name, args, c_channel_env, port=None, address=None):
+        """
+            Given a set of command arguments, start the process and output
+            appropriate log messages.  If the start is successful, the process
+            is added to the list of started processes.
+
+            The port and address arguments are for log messages only.
+        """
+        self.log_starting(name, port, address)
+        newproc = ProcessInfo(name, args, c_channel_env)
+        self.processes[newproc.pid] = newproc
+        self.log_started(newproc.pid)
+
+    def start_simple(self, name, c_channel_env, port=None, address=None):
+        """
+            Most of the BIND-10 processes are started with the command:
+
+                <process-name> [-v]
+
+            ... where -v is appended if verbose is enabled.  This method
+            generates the arguments from the name and starts the process.
+
+            The port and address arguments are for log messages only.
+        """
+        # Set up the command arguments.
+        args = [name]
         if self.verbose:
-            sys.stdout.write("[bind10] ccsession started\n")
+            args += ['-v']
 
-        # if we're running a recursive-only server, we skip the xfrout
-        # modules. otherwise, start xfrout before the DNS server, to make
-        # sure every xfr-query can be processed properly.
-        xfrout=None
-        if not self.recursive:
-            xfrout_args = ['b10-xfrout']
-            if self.verbose:
-                sys.stdout.write("[bind10] Starting b10-xfrout\n")
-                xfrout_args += ['-v']
-            try:
-                xfrout = ProcessInfo("b10-xfrout", xfrout_args, 
-                                     c_channel_env )
-            except Exception as e:
-                c_channel.process.kill()
-                bind_cfgd.process.kill()
-                return "Unable to start b10-xfrout; " + str(e)
-            self.processes[xfrout.pid] = xfrout
-            if self.verbose:
-                sys.stdout.write("[bind10] Started b10-xfrout (PID %d)\n" % 
-                                 xfrout.pid)
+        # ... and start the process
+        self.start_process(name, args, c_channel_env, port, address)
 
-        # start DNS server
+    # The next few methods start up the rest of the BIND-10 processes.
+    # Although many of these methods are little more than a call to
+    # start_simple, they are retained (a) for testing reasons and (b) as a place
+    # where modifications can be made if the process start-up sequence changes
+    # for a given process.
+
+    def start_auth(self, c_channel_env):
+        """
+            Start the Authoritative server
+        """
         # XXX: this must be read from the configuration manager in the future
         if self.recursive:
             dns_prog = 'b10-recurse'
@@ -365,123 +440,115 @@ class BoB:
             dnsargs += ['-u', str(self.uid)]
         if self.verbose:
             dnsargs += ['-v']
-            sys.stdout.write("Starting %s using port %d" %
-                             (dns_prog, self.dns_port))
-            if self.address:
-                sys.stdout.write(" on %s" % str(self.address))
-            sys.stdout.write("\n")
-        try:
-            dns = ProcessInfo(dns_prog, dnsargs, c_channel_env)
-        except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            if xfrout:
-                xfrout.process.kill()
-            return "Unable to start " + dns_prog + ": " + str(e)
-        self.processes[dns.pid] = dns
+
+        # ... and start
+        self.start_process("b10-auth", dnsargs, c_channel_env,
+            self.dns_port, self.address)
+
+    def start_recurse(self, c_channel_env):
+        """
+            Start the Resolver.  At present, all these arguments and switches
+            are pure speculation.  As with the auth daemon, they should be
+            read from the configuration database.
+        """
+        self.curproc = "b10-recurse"
+        # XXX: this must be read from the configuration manager in the future
+        resargs = ['b10-recurse']
+        if self.uid:
+            resargs += ['-u', str(self.uid)]
         if self.verbose:
-            sys.stdout.write("[bind10] Started %s (PID %d)\n" %
-                             (dns_prog, dns.pid))
+            resargs += ['-v']
+
+        # ... and start
+        self.start_process("b10-recurse", resargs, c_channel_env)
+
+    def start_xfrout(self, c_channel_env):
+        self.start_simple("b10-xfrout", c_channel_env)
+
+    def start_xfrin(self, c_channel_env):
+        self.start_simple("b10-xfrin", c_channel_env)
+
+    def start_zonemgr(self, c_channel_env):
+        self.start_simple("b10-zonemgr", c_channel_env)
+
+    def start_stats(self, c_channel_env):
+        self.start_simple("b10-stats", c_channel_env)
+
+    def start_cmdctl(self, c_channel_env):
+        # XXX: we hardcode port 8080
+        self.start_simple("b10-cmdctl", c_channel_env, 8080)
 
-        # everything after the DNS server can run as non-root
+    def start_all_processes(self, c_channel_env):
+        """
+            Starts up all the processes.  Any exception generated during the
+            starting of the processes is handled by the caller.
+        """
+        self.start_msgq(c_channel_env)
+        self.start_cfgmgr(c_channel_env)
+        self.start_ccsession(c_channel_env)
+
+        # Extract the parameters associated with Bob.  This can only be
+        # done after the CC Session is started.
+        self.read_bind10_config()
+
+        # Continue starting the processes.  The authoritative server (if
+        # selected):
+        if self.cfg_start_auth:
+            self.start_auth(c_channel_env)
+
+        # ... and resolver (if selected):
+        if self.cfg_start_recurse:
+            self.start_recurse(c_channel_env)
+
+        # Everything after the main components can run as non-root.
+        # TODO: this is only temporary - once the privileged socket creator is
+        # fully working, nothing else will run as root.
         if self.uid is not None:
             posix.setuid(self.uid)
 
-        xfrind=None
-        if not self.recursive:
-            # If we're running an authoritative server, start b10-xfrin
-            xfrin_args = ['b10-xfrin']
-            if self.verbose:
-                sys.stdout.write("[bind10] Starting b10-xfrin\n")
-                xfrin_args += ['-v']
-            try:
-                xfrind = ProcessInfo("b10-xfrin", xfrin_args,
-                                     c_channel_env)
-            except Exception as e:
-                c_channel.process.kill()
-                bind_cfgd.process.kill()
-                if xfrout:
-                    xfrout.process.kill()
-                dns.process.kill()
-                return "Unable to start b10-xfrin; " + str(e)
-            self.processes[xfrind.pid] = xfrind
-            if self.verbose:
-                sys.stdout.write("[bind10] Started b10-xfrin (PID %d)\n" % 
-                                 xfrind.pid)
-
-        zonemgr=None
-        if not self.recursive:
-            # If we're running an authoritative server, start b10-zonemgr
-            zonemgr_args = ['b10-zonemgr']
-            if self.verbose:
-                sys.stdout.write("[bind10] Starting b10-zonemgr\n")
-                zonemgr_args += ['-v']
-            try:
-                zonemgr = ProcessInfo("b10-zonemgr", zonemgr_args,
-                                     c_channel_env)
-            except Exception as e:
-                c_channel.process.kill()
-                bind_cfgd.process.kill()
-                dns.process.kill()
-                if xfrout:
-                    xfrout.process.kill()
-                if xfrind:
-                    xfrind.process.kill()
-                return "Unable to start b10-zonemgr; " + str(e)
-            self.processes[zonemgr.pid] = zonemgr 
-            if self.verbose:
-                sys.stdout.write("[bind10] Started b10-zonemgr(PID %d)\n" % 
-                                 zonemgr.pid)
+        # xfrin/xfrout and the zone manager are only meaningful if the
+        # authoritative server has been started.
+        if self.cfg_start_auth:
+            self.start_xfrout(c_channel_env)
+            self.start_xfrin(c_channel_env)
+            self.start_zonemgr(c_channel_env)
 
-        # start b10-stats
-        stats_args = ['b10-stats']
+        # ... and finally start the remaining processes
+        self.start_stats(c_channel_env)
+        self.start_cmdctl(c_channel_env)
+    
+    def startup(self):
+        """
+            Start the BoB instance.
+ 
+            Returns None if successful, otherwise an string describing the
+            problem.
+        """
+        # Try to connect to the c-channel daemon, to see if it is already
+        # running
+        c_channel_env = {}
+        if self.msgq_socket_file is not None:
+             c_channel_env["BIND10_MSGQ_SOCKET_FILE"] = self.msgq_socket_file 
         if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-stats\n")
-            stats_args += ['-v']
+           sys.stdout.write("[bind10] Checking for already running b10-msgq\n")
+        # try to connect, and if we can't wait a short while
         try:
-            statsd = ProcessInfo("b10-stats", stats_args,
-                                 c_channel_env)
-        except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            xfrout.process.kill()
-            auth.process.kill()
-            xfrind.process.kill()
-            zonemgr.process.kill()
-            return "Unable to start b10-stats; " + str(e)
-
-        self.processes[statsd.pid] = statsd
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-stats (PID %d)\n" % statsd.pid)
+            self.cc_session = isc.cc.Session(self.msgq_socket_file)
+            return "b10-msgq already running, or socket file not cleaned , cannot start"
+        except isc.cc.session.SessionError:
+            # this is the case we want, where the msgq is not running
+            pass
 
-        # start the b10-cmdctl
-        # XXX: we hardcode port 8080
-        cmdctl_args = ['b10-cmdctl']
-        if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-cmdctl on port 8080\n")
-            cmdctl_args += ['-v']
+        # Start all processes.  If any one fails to start, kill all started
+        # processes and exit with an error indication.
         try:
-            cmd_ctrld = ProcessInfo("b10-cmdctl", cmdctl_args,
-                                    c_channel_env)
+            self.start_all_processes(c_channel_env)
         except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            dns.process.kill()
-            if xfrout:
-                xfrout.process.kill()
-            if xfrind:
-                xfrind.process.kill()
-            if zonemgr:
-                zonemgr.process.kill()
-            statsd.process.kill()
-            return "Unable to start b10-cmdctl; " + str(e)
-        self.processes[cmd_ctrld.pid] = cmd_ctrld
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-cmdctl (PID %d)\n" % 
-                             cmd_ctrld.pid)
+            self.kill_started_processes()
+            return "Unable to start " + self.curproc + ": " + str(e)
 
+        # Started successfully
         self.runnable = True
-
         return None
 
     def stop_all_processes(self):
@@ -645,7 +712,7 @@ def check_port(option, opt_str, value, parser):
     a valid port number. Used by OptionParser() on startup."""
     try:
         if opt_str in ['-p', '--port']:
-            parser.values.auth_port = isc.net.parse.port_parse(value)
+            parser.values.dns_port = isc.net.parse.port_parse(value)
         else:
             raise OptionValueError("Unknown option " + opt_str)
     except ValueError as e:
@@ -677,7 +744,7 @@ def main():
     # Parse any command-line options.
     parser = OptionParser(version=VERSION)
     parser.add_option("-a", "--address", dest="address", type="string",
-                      action="callback", callback=check_addr, default='',
+                      action="callback", callback=check_addr, default=None,
                       help="address the DNS server will use (default: listen on all addresses)")
     parser.add_option("-f", "--forward", dest="forward", type="string",
                       action="callback", callback=check_addr, default='',
@@ -810,6 +877,7 @@ def main():
     # shutdown
     signal.signal(signal.SIGCHLD, signal.SIG_DFL)
     boss_of_bind.shutdown()
+    sys.stdout.write("[bind10] BIND 10 exiting\n");
     sys.exit(0)
 
 if __name__ == "__main__":

src/bin/bind10/bob.spec

@@ -3,6 +3,18 @@
     "module_name": "Boss",
     "module_description": "Master process",
     "config_data": [
+      {
+        "item_name": "start_auth",
+        "item_type": "boolean",
+        "item_optional": false,
+        "item_default": true
+      },
+      {
+        "item_name": "start_recurse",
+        "item_type": "boolean",
+        "item_optional": false,
+        "item_default": false
+      }
     ],
     "commands": [
       {

src/bin/bind10/run_bind10.sh.in

@@ -23,7 +23,7 @@ BIND10_PATH=@abs_top_builddir@/src/bin/bind10
 PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/recurse:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:$PATH
 export PATH
 
-PYTHONPATH=@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs
+PYTHONPATH=@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs
 export PYTHONPATH
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

src/bin/bind10/tests/bind10_test.py

@@ -79,43 +79,237 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, None)
         self.assertEqual(bob.dns_port, 5300)
-        self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.address, None)
+        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
 
     def test_init_alternate_socket(self):
         bob = BoB("alt_socket_file")
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, "alt_socket_file")
+        self.assertEqual(bob.address, None)
+        self.assertEqual(bob.dns_port, 5300)
         self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
 
     def test_init_alternate_dns_port(self):
         bob = BoB(None, 9999)
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, None)
         self.assertEqual(bob.dns_port, 9999)
-        self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.address, None)
+        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
 
     def test_init_alternate_address(self):
-        bob = BoB(None, 5300, IPAddr('127.127.127.127'))
+        bob = BoB(None, 1234, IPAddr('127.127.127.127'))
         self.assertEqual(bob.verbose, False)
-        self.assertEqual(bob.dns_port, 5300)
         self.assertEqual(bob.msgq_socket_file, None)
-        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.dns_port, 1234)
         self.assertEqual(bob.address.addr, socket.inet_aton('127.127.127.127'))
+        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
-    # verbose testing...
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
+
+# Class for testing the Bob.start_all_processes() method call.
+#
+# Although testing that external processes start is outside the scope
+# of the unit test, by overriding the process start methods we can check
+# that the right processes are started depending on the configuration
+# options.
+class StartAllProcessesBob(BoB):
+    def __init__(self):
+        BoB.__init__(self)
+
+# Set flags as to which of the overridden methods has been run.
+        self.msgq = False
+        self.cfgmgr = False
+        self.ccsession = False
+        self.auth = False
+        self.recurse = False
+        self.xfrout = False
+        self.xfrin = False
+        self.zonemgr = False
+        self.stats = False
+        self.cmdctl = False
+
+    def read_bind10_config(self):
+        # Configuration options are set directly
+        pass
+
+    def start_msgq(self, c_channel_env):
+        self.msgq = True
+
+    def start_cfgmgr(self, c_channel_env):
+        self.cfgmgr = True
+
+    def start_ccsession(self, c_channel_env):
+        self.ccsession = True
+
+    def start_auth(self, c_channel_env):
+        self.auth = True
+
+    def start_recurse(self, c_channel_env):
+        self.recurse = True
+
+    def start_xfrout(self, c_channel_env):
+        self.xfrout = True
+
+    def start_xfrin(self, c_channel_env):
+        self.xfrin = True
+
+    def start_zonemgr(self, c_channel_env):
+        self.zonemgr = True
+
+    def start_stats(self, c_channel_env):
+        self.stats = True
+
+    def start_cmdctl(self, c_channel_env):
+        self.cmdctl = True
+
+# Check that the start_all_processes method starts the right combination
+# of processes.
+class TestStartAllProcessesBob(unittest.TestCase):
+    def check_preconditions(self, bob):
+        self.assertEqual(bob.msgq, False)
+        self.assertEqual(bob.cfgmgr, False)
+        self.assertEqual(bob.ccsession, False)
+        self.assertEqual(bob.auth, False)
+        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.xfrout, False)
+        self.assertEqual(bob.xfrin, False)
+        self.assertEqual(bob.zonemgr, False)
+        self.assertEqual(bob.stats, False)
+        self.assertEqual(bob.cmdctl, False)
+
+    # Checks the processes started when starting neither auth nor recurse
+    # is specified.
+    def test_start_none(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = False
+        bob.cfg_start_recurse = False
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, False)
+        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.xfrout, False)
+        self.assertEqual(bob.xfrin, False)
+        self.assertEqual(bob.zonemgr, False)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
+    # Checks the processes started when starting only the auth process
+    def test_start_auth(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = True
+        bob.cfg_start_recurse = False
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, True)
+        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.xfrout, True)
+        self.assertEqual(bob.xfrin, True)
+        self.assertEqual(bob.zonemgr, True)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
+    # Checks the processes started when starting only the recurse process
+    def test_start_recurse(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = False
+        bob.cfg_start_recurse = True
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, False)
+        self.assertEqual(bob.recurse, True)
+        self.assertEqual(bob.xfrout, False)
+        self.assertEqual(bob.xfrin, False)
+        self.assertEqual(bob.zonemgr, False)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
+    # Checks the processes started when starting both auth and recurse process
+    def test_start_both(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = True
+        bob.cfg_start_recurse = True
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, True)
+        self.assertEqual(bob.recurse, True)
+        self.assertEqual(bob.xfrout, True)
+        self.assertEqual(bob.xfrin, True)
+        self.assertEqual(bob.zonemgr, True)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
 
 if __name__ == '__main__':
     unittest.main()

src/bin/bindctl/bindcmd.py

@@ -558,7 +558,7 @@ class BindCmdInterpreter(Cmd):
                     if value_map['type'] in [ 'module', 'map', 'list' ]:
                         line += "/"
                     else:
-                        line += ":\t" + str(value_map['value'])
+                        line += ":\t" + json.dumps(value_map['value'])
                     line += "\t" + value_map['type']
                     line += "\t"
                     if value_map['default']:
@@ -569,7 +569,10 @@ class BindCmdInterpreter(Cmd):
             elif cmd.command == "add":
                 self.config_data.add_value(identifier, cmd.params['value'])
             elif cmd.command == "remove":
-                self.config_data.remove_value(identifier, cmd.params['value'])
+                if 'value' in cmd.params:
+                    self.config_data.remove_value(identifier, cmd.params['value'])
+                else:
+                    self.config_data.remove_value(identifier, None)
             elif cmd.command == "set":
                 if 'identifier' not in cmd.params:
                     print("Error: missing identifier or value")

src/bin/bindctl/bindctl-source.py.in

@@ -28,7 +28,10 @@ import isc.util.process
 
 isc.util.process.rename()
 
-__version__ = 'Bindctl'
+# This is the version that gets displayed to the user.
+# The VERSION string consists of the module name, the module version
+# number, and the overall BIND 10 version number (set in configure.ac).
+VERSION = "bindctl 20101201 (BIND 10 @PACKAGE_VERSION@)"
 
 def prepare_config_commands(tool):
     '''Prepare fixed commands for local configuration editing'''
@@ -48,7 +51,7 @@ def prepare_config_commands(tool):
     cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list")
     param = ParamInfo(name = "identifier", type = "string", optional=True)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=False)
+    param = ParamInfo(name = "value", type = "string", optional=True)
     cmd.add_param(param)
     module.add_command(cmd)
 
@@ -113,7 +116,7 @@ def set_bindctl_options(parser):
 
 if __name__ == '__main__':
     try:
-        parser = OptionParser(version = __version__)
+        parser = OptionParser(version = VERSION)
         set_bindctl_options(parser)
         (options, args) = parser.parse_args()
         server_addr = options.addr + ':' + str(options.port)

src/bin/host/host.cc

@@ -131,7 +131,7 @@ host_lookup(const char* const name, const char* const type) {
                       }
 
                       RdataIteratorPtr rit = (*it)->getRdataIterator();
-                      for (rit->first(); !rit->isLast(); rit->next()) {
+                      for (; !rit->isLast(); rit->next()) {
                           // instead of using my name, maybe use returned label?
                           cout << name << " has address " <<
                               (*rit).getCurrent().toText() << endl;

src/bin/msgq/msgq.py.in

@@ -38,7 +38,9 @@ import isc.cc
 isc.util.process.rename()
 
 # This is the version that gets displayed to the user.
-__version__ = "v20091030 (Paving the DNS Parking Lot)"
+# The VERSION string consists of the module name, the module version
+# number, and the overall BIND 10 version number (set in configure.ac).
+VERSION = "b10-msgq 20100818 (BIND 10 @PACKAGE_VERSION@)"
 
 class MsgQReceiveError(Exception): pass
 
@@ -421,7 +423,7 @@ if __name__ == "__main__":
         parser.values.msgq_port = intval
 
     # Parse any command-line options.
-    parser = OptionParser(version=__version__)
+    parser = OptionParser(version=VERSION)
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
                       help="display more about what is going on")
     parser.add_option("-s", "--socket-file", dest="msgq_socket_file",
@@ -433,7 +435,7 @@ if __name__ == "__main__":
 
     # Announce startup.
     if options.verbose:
-        sys.stdout.write("[b10-msgq] MsgQ %s\n" % __version__)
+        sys.stdout.write("[b10-msgq] %s\n" % VERSION)
 
     msgq = MsgQ(options.msgq_socket_file, options.verbose)
 

src/bin/recurse/Makefile.am

@@ -46,6 +46,7 @@ b10_recurse_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 b10_recurse_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_recurse_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_recurse_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+b10_recurse_LDADD += $(top_builddir)/src/lib/log/liblog.la
 b10_recurse_LDADD += $(top_builddir)/src/bin/auth/change_user.o
 b10_recurse_LDFLAGS = -pthread
 

src/bin/recurse/recursor.cc

@@ -152,8 +152,8 @@ public:
         message_(message), section_(sect)
     {}
     void operator()(const RRsetPtr rrset) {
-        dlog("Adding RRSet to message section " +
-            boost::lexical_cast<string>(section_));
+        //dlog("Adding RRSet to message section " +
+        //    boost::lexical_cast<string>(section_));
         message_->addRRset(section_, rrset, true);
     }
     MessagePtr message_;
@@ -263,17 +263,16 @@ public:
                 for_each(incoming.beginSection(Message::SECTION_ANSWER),
                          incoming.endSection(Message::SECTION_ANSWER),
                          SectionInserter(message, Message::SECTION_ANSWER));
+                for_each(incoming.beginSection(Message::SECTION_AUTHORITY),
+                         incoming.endSection(Message::SECTION_AUTHORITY),
+                         SectionInserter(message, Message::SECTION_AUTHORITY));
                 for_each(incoming.beginSection(Message::SECTION_ADDITIONAL),
                          incoming.endSection(Message::SECTION_ADDITIONAL),
                          SectionInserter(message, Message::SECTION_ADDITIONAL));
-                for_each(incoming.beginSection(Message::SECTION_AUTHORITY),
-                         incoming.endSection(Message::SECTION_ADDITIONAL),
-                         SectionInserter(message, Message::SECTION_AUTHORITY));
             } catch (const Exception& ex) {
                 // Incoming message couldn't be read, we just SERVFAIL
                 message->setRcode(Rcode::SERVFAIL());
             }
-
         }
 
         // Now we can clear the buffer and render the new message into it

src/bin/recurse/tests/Makefile.am

@@ -33,6 +33,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
 run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/bin/xfrin/b10-xfrin.xml

@@ -149,7 +149,6 @@
       the authoritative server to transfer from,
       and <varname>port</varname> to define the port number on the
       authoritative server (defaults to 53).
-<!-- TODO: note: not documenting db_file since that will be removed. -->
      </para>
 <!-- TODO: later hostname for master? -->
 

src/bin/xfrout/b10-xfrout.8

@@ -2,12 +2,12 @@
 .\"     Title: b10-xfrout
 .\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
 .\" Generator: DocBook XSL Stylesheets v1.75.2 <http://docbook.sf.net/>
-.\"      Date: September 8, 2010
+.\"      Date: December 1, 2010
 .\"    Manual: BIND10
 .\"    Source: BIND10
 .\"  Language: English
 .\"
-.TH "B10\-XFROUT" "8" "September 8, 2010" "BIND10" "BIND10"
+.TH "B10\-XFROUT" "8" "December 1, 2010" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
@@ -67,11 +67,6 @@ receives its configurations from
 The configurable settings are:
 .PP
 
-\fIdb_file\fR
-defines the path to the SQLite3 data store file\&. The default is
-/usr/local/var/bind10\-devel/zone\&.sqlite3\&.
-.PP
-
 \fItransfers_out\fR
 defines the maximum number of outgoing zone transfers that can run concurrently\&. The default is 10\&.
 .if n \{\

src/bin/xfrout/b10-xfrout.xml

@@ -21,7 +21,7 @@
 <refentry>
 
   <refentryinfo>
-    <date>September 8, 2010</date>
+    <date>December 1, 2010</date>
   </refentryinfo>
 
   <refmeta>
@@ -94,13 +94,6 @@
       The configurable settings are:
     </para>
     <para>
-      <varname>db_file</varname>
-      defines the path to the SQLite3 data store file.
-      The default is
-      <filename>/usr/local/var/bind10-devel/zone.sqlite3</filename>.
-<!-- TODO: db_file will be removed -->
-    </para>
-    <para>
       <varname>transfers_out</varname>
       defines the maximum number of outgoing zone transfers
       that can run concurrently. The default is 10.

src/bin/xfrout/xfrout.spec.pre.in

@@ -9,12 +9,6 @@
          "item_default": 10
        },
        {
-         "item_name": "db_file",
-         "item_type": "string",
-         "item_optional": false,
-         "item_default": "@@LOCALSTATEDIR@@/@PACKAGE@/zone.sqlite3"
-       },
-       {
          "item_name": "log_name",
          "item_type": "string",
          "item_optional": false,

src/lib/bench/benchmark.h

@@ -200,7 +200,7 @@ private:
     BenchMark(const BenchMark& source);
     BenchMark& operator=(const BenchMark& source);
 public:
-    /// \bench Constructor for immediate run.
+    /// \brief Constructor for immediate run.
     ///
     /// This is the constructor that is expected to be used normally.
     /// It runs the benchmark within the constructor and prints the result,
@@ -217,7 +217,7 @@ public:
         initialize(true);
     }
 
-    /// \bench Constructor for finer-grained control.
+    /// \brief Constructor for finer-grained control.
     ///
     /// This constructor takes the third parameter, \c immediate, to control
     /// whether to run the benchmark within the constructor.

src/lib/config/config_data.cc

@@ -128,11 +128,6 @@ spec_name_list(ElementPtr result, ConstElementPtr spec_part,
                 if (recurse && list_el->get("item_type")->stringValue() == "map") {
                     spec_name_list(result, list_el->get("map_item_spec"), new_prefix, recurse);
                 } else {
-                    if (list_el->get("item_type")->stringValue() == "map" ||
-                        list_el->get("item_type")->stringValue() == "list"
-                    ) {
-                        new_prefix += "/";
-                    }
                     result->add(Element::create(new_prefix));
                 }
             }

src/lib/config/tests/config_data_unittests.cc

@@ -120,8 +120,8 @@ TEST(ConfigData, getItemList) {
     ModuleSpec spec2 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec2.spec");
     ConfigData cd = ConfigData(spec2);
 
-    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5/\", \"item6/\" ]", cd.getItemList()->str());
-    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5/\", \"item6/value1\", \"item6/value2\" ]", cd.getItemList("", true)->str());
+    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5\", \"item6\" ]", cd.getItemList()->str());
+    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5\", \"item6/value1\", \"item6/value2\" ]", cd.getItemList("", true)->str());
     EXPECT_EQ("[ \"item6/value1\", \"item6/value2\" ]", cd.getItemList("item6")->str());
 }
 
@@ -129,12 +129,12 @@ TEST(ConfigData, getFullConfig) {
     ModuleSpec spec2 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec2.spec");
     ConfigData cd = ConfigData(spec2);
 
-    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5/\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
+    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
     ElementPtr my_config = Element::fromJSON("{ \"item1\": 2 }");
     cd.setLocalConfig(my_config);
-    EXPECT_EQ("{ \"item1\": 2, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5/\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
+    EXPECT_EQ("{ \"item1\": 2, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
     ElementPtr my_config2 = Element::fromJSON("{ \"item6\": { \"value1\": \"a\" } }");
     cd.setLocalConfig(my_config2);
-    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5/\": [ \"a\", \"b\" ], \"item6/value1\": \"a\", \"item6/value2\": None }", cd.getFullConfig()->str());
+    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5\": [ \"a\", \"b\" ], \"item6/value1\": \"a\", \"item6/value2\": None }", cd.getFullConfig()->str());
 }
 

src/lib/datasrc/Makefile.am

@@ -15,3 +15,4 @@ libdatasrc_la_SOURCES += static_datasrc.h static_datasrc.cc
 libdatasrc_la_SOURCES += sqlite3_datasrc.h sqlite3_datasrc.cc
 libdatasrc_la_SOURCES += query.h query.cc
 libdatasrc_la_SOURCES += cache.h cache.cc
+libdatasrc_la_SOURCES += zonetable.h zonetable.cc

src/lib/datasrc/cache.h

@@ -170,9 +170,9 @@ public:
     /// then promoted to the head of the LRU queue.  (NOTE: Because
     /// of this, "retrieve" cannot be implemented as a const method.)
     ///
-    /// \param name The query name
-    /// \param rrclass The query class
-    /// \param rrtype The query type
+    /// \param qname The query name
+    /// \param qclass The query class
+    /// \param qtype The query type
     /// \param rrset Returns the RRset found, if any, to the caller
     /// \param flags Returns the flags, if any, to the caller
     ///

src/lib/datasrc/data_source.cc

@@ -95,7 +95,7 @@ getAdditional(Query& q, ConstRRsetPtr rrset) {
     }
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    for (it->first(); !it->isLast(); it->next()) {
+    for (; !it->isLast(); it->next()) {
         const Rdata& rd(it->getCurrent());
         if (rrset->getType() == RRType::NS()) {
             const generic::NS& ns = dynamic_cast<const generic::NS&>(rd);
@@ -123,7 +123,6 @@ synthesizeCname(QueryTaskPtr task, RRsetPtr rrset, RRsetList& target) {
 
     // More than one DNAME RR in the RRset is illegal, so we only have
     // to process the first one.
-    it->first();
     if (it->isLast()) {
         return;
     }
@@ -152,7 +151,6 @@ chaseCname(Query& q, QueryTaskPtr task, RRsetPtr rrset) {
 
     // More than one CNAME RR in the RRset is illegal, so we only have
     // to process the first one.
-    it->first();
     if (it->isLast()) {
         return;
     }
@@ -660,7 +658,6 @@ getNsec3Param(Query& q, ZoneInfo& zoneinfo) {
     // XXX: currently only one NSEC3 chain per zone is supported;
     // we will need to revisit this.
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     if (it->isLast()) {
         return (ConstNsec3ParamPtr());
     }

src/lib/datasrc/static_datasrc.cc

@@ -79,6 +79,7 @@ StaticDataSrcImpl::StaticDataSrcImpl() :
     authors->addRdata(generic::TXT("JINMEI Tatuya"));
     authors->addRdata(generic::TXT("Kazunori Fujiwara"));
     authors->addRdata(generic::TXT("Michael Graff"));
+    authors->addRdata(generic::TXT("Michal Vaner"));
     authors->addRdata(generic::TXT("Naoki Kambe"));
     authors->addRdata(generic::TXT("Shane Kerr"));
     authors->addRdata(generic::TXT("Shen Tingting"));

src/lib/datasrc/tests/Makefile.am

@@ -24,6 +24,7 @@ run_unittests_SOURCES += static_unittest.cc
 run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += cache_unittest.cc
 run_unittests_SOURCES += test_datasrc.h test_datasrc.cc
+run_unittests_SOURCES += zonetable_unittest.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_LDADD = $(GTEST_LDADD)

src/lib/datasrc/tests/datasrc_unittest.cc

@@ -124,7 +124,6 @@ DataSrcTest::QueryCommon(const RRClass& qclass) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -138,7 +137,6 @@ DataSrcTest::QueryCommon(const RRClass& qclass) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -154,7 +152,6 @@ DataSrcTest::QueryCommon(const RRClass& qclass) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -193,7 +190,6 @@ TEST_F(DataSrcTest, NSQuery) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -216,7 +212,6 @@ TEST_F(DataSrcTest, DuplicateQuery) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -237,7 +232,6 @@ TEST_F(DataSrcTest, DuplicateQuery) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -334,7 +328,6 @@ TEST_F(DataSrcTest, Wildcard) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.2", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -353,7 +346,6 @@ TEST_F(DataSrcTest, Wildcard) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -369,7 +361,6 @@ TEST_F(DataSrcTest, Wildcard) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -406,7 +397,6 @@ TEST_F(DataSrcTest, WildcardCname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("www.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -419,7 +409,6 @@ TEST_F(DataSrcTest, WildcardCname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -438,7 +427,6 @@ TEST_F(DataSrcTest, WildcardCname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -454,7 +442,6 @@ TEST_F(DataSrcTest, WildcardCname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -474,7 +461,6 @@ TEST_F(DataSrcTest, WildcardCnameNodata) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("www.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -506,7 +492,6 @@ TEST_F(DataSrcTest, WildcardCnameNxdomain) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("spork.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -544,7 +529,6 @@ TEST_F(DataSrcTest, AuthDelegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.2", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -556,7 +540,6 @@ TEST_F(DataSrcTest, AuthDelegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -572,7 +555,6 @@ TEST_F(DataSrcTest, AuthDelegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -591,7 +573,6 @@ TEST_F(DataSrcTest, Dname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("sql1.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -605,7 +586,6 @@ TEST_F(DataSrcTest, Dname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -621,7 +601,6 @@ TEST_F(DataSrcTest, Dname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -649,7 +628,6 @@ TEST_F(DataSrcTest, Cname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("cnametest.flame.org.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -668,7 +646,6 @@ TEST_F(DataSrcTest, CnameInt) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("www.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -695,7 +672,6 @@ TEST_F(DataSrcTest, CnameExt) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("www.sql1.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -720,7 +696,6 @@ TEST_F(DataSrcTest, Delegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("ns1.subzone.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_FALSE(it->isLast());
@@ -732,7 +707,6 @@ TEST_F(DataSrcTest, Delegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -751,7 +725,6 @@ TEST_F(DataSrcTest, NSDelegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("ns1.subzone.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_FALSE(it->isLast());
@@ -763,7 +736,6 @@ TEST_F(DataSrcTest, NSDelegation) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -795,7 +767,6 @@ TEST_F(DataSrcTest, NSECZonecut) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -817,7 +788,6 @@ TEST_F(DataSrcTest, DNAMEZonecut) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("ns1.subzone.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_FALSE(it->isLast());
@@ -829,7 +799,6 @@ TEST_F(DataSrcTest, DNAMEZonecut) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
@@ -854,7 +823,6 @@ TEST_F(DataSrcTest, DS) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
@@ -890,7 +858,6 @@ TEST_F(DataSrcTest, NSECZonecutOfNonsecureZone) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ(createRdata(RRType::NS(), RRClass::IN(),
                           "ns.sub.example.org.")->toText(),
               it->getCurrent().toText());
@@ -904,7 +871,6 @@ TEST_F(DataSrcTest, NSECZonecutOfNonsecureZone) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ(createRdata(RRType::A(), RRClass::IN(), "192.0.2.101")->toText(),
               it->getCurrent().toText());
     it->next();

src/lib/datasrc/tests/sqlite3_unittest.cc

@@ -244,7 +244,6 @@ checkRRset(RRsetPtr rrset, const Name& expected_name,
     EXPECT_EQ(expected_rrttl, rrset->getTTL());
 
     RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
-    rdata_iterator->first();
     vector<string>::const_iterator data_it = expected_data.begin();
     for (; data_it != expected_data.end(); ++data_it) {
         EXPECT_FALSE(rdata_iterator->isLast());

src/lib/datasrc/tests/static_unittest.cc

@@ -63,6 +63,7 @@ protected:
         authors_data.push_back("JINMEI Tatuya");
         authors_data.push_back("Kazunori Fujiwara");
         authors_data.push_back("Michael Graff");
+        authors_data.push_back("Michal Vaner");
         authors_data.push_back("Naoki Kambe");
         authors_data.push_back("Shane Kerr");
         authors_data.push_back("Shen Tingting");
@@ -109,7 +110,6 @@ checkRRset(ConstRRsetPtr rrset, const Name& expected_name,
     EXPECT_EQ(rrttl, rrset->getTTL());
 
     RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
-    rdata_iterator->first();
     vector<string>::const_iterator data_it = expected_data.begin();
     for (; data_it != expected_data.end(); ++data_it) {
         EXPECT_FALSE(rdata_iterator->isLast());

src/lib/datasrc/tests/test_datasrc.cc

@@ -408,7 +408,7 @@ copyRRset(RRsetPtr const source) {
     RRsetPtr rrset = RRsetPtr(new RRset(source->getName(), source->getClass(),
                                         source->getType(), source->getTTL()));
     RdataIteratorPtr it = source->getRdataIterator();
-    for (it->first(); !it->isLast(); it->next()) {
+    for (; !it->isLast(); it->next()) {
         rrset->addRdata(it->getCurrent());
     }
     if (source->getRRsig()) {

src/lib/datasrc/tests/zonetable_unittest.cc

@@ -0,0 +1,113 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <exceptions/exceptions.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <datasrc/zonetable.h>
+
+#include <gtest/gtest.h>
+
+using namespace isc::dns;
+using namespace isc::datasrc;
+
+namespace {
+TEST(ZoneTest, init) {
+    MemoryZone zone(RRClass::IN(), Name("example.com"));
+    EXPECT_EQ(Name("example.com"), zone.getOrigin());
+    EXPECT_EQ(RRClass::IN(), zone.getClass());
+
+    MemoryZone ch_zone(RRClass::CH(), Name("example"));
+    EXPECT_EQ(Name("example"), ch_zone.getOrigin());
+    EXPECT_EQ(RRClass::CH(), ch_zone.getClass());
+}
+
+TEST(ZoneTest, find) {
+    MemoryZone zone(RRClass::IN(), Name("example.com"));
+    EXPECT_EQ(Zone::NXDOMAIN,
+              zone.find(Name("www.example.com"), RRType::A()).code);
+}
+
+class ZoneTableTest : public ::testing::Test {
+protected:
+    ZoneTableTest() : zone1(new MemoryZone(RRClass::IN(),
+                                           Name("example.com"))),
+                      zone2(new MemoryZone(RRClass::IN(),
+                                           Name("example.net"))),
+                      zone3(new MemoryZone(RRClass::IN(), Name("example")))
+    {}
+    ZoneTable zone_table;
+    ZonePtr zone1, zone2, zone3;
+};
+
+TEST_F(ZoneTableTest, add) {
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone1));
+    EXPECT_EQ(ZoneTable::EXIST, zone_table.add(zone1));
+    // names are compared in a case insensitive manner.
+    EXPECT_EQ(ZoneTable::EXIST, zone_table.add(
+                  ZonePtr(new MemoryZone(RRClass::IN(), Name("EXAMPLE.COM")))));
+
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone2));
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone3));
+
+    // Zone table is indexed only by name.  Duplicate origin name with
+    // different zone class isn't allowed.
+    EXPECT_EQ(ZoneTable::EXIST, zone_table.add(
+                  ZonePtr(new MemoryZone(RRClass::CH(),
+                                         Name("example.com")))));
+
+    /// Bogus zone (NULL)
+    EXPECT_THROW(zone_table.add(ZonePtr()), isc::InvalidParameter);
+}
+
+TEST_F(ZoneTableTest, remove) {
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone1));
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone2));
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone3));
+
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.remove(Name("example.net")));
+    EXPECT_EQ(ZoneTable::NOTFOUND, zone_table.remove(Name("example.net")));
+}
+
+TEST_F(ZoneTableTest, find) {
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone1));
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone2));
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone3));
+
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.find(Name("example.com")).code);
+    EXPECT_EQ(Name("example.com"),
+              zone_table.find(Name("example.com")).zone->getOrigin());
+
+    EXPECT_EQ(ZoneTable::NOTFOUND,
+              zone_table.find(Name("example.org")).code);
+    EXPECT_EQ(static_cast<const Zone*>(NULL),
+              zone_table.find(Name("example.org")).zone);
+
+    // there's no exact match.  the result should be the longest match,
+    // and the code should be PARTIALMATCH.
+    EXPECT_EQ(ZoneTable::PARTIALMATCH,
+              zone_table.find(Name("www.example.com")).code);
+    EXPECT_EQ(Name("example.com"),
+              zone_table.find(Name("www.example.com")).zone->getOrigin());
+
+    // make sure the partial match is indeed the longest match by adding
+    // a zone with a shorter origin and query again.
+    ZonePtr zone_com(new MemoryZone(RRClass::IN(), Name("com")));
+    EXPECT_EQ(ZoneTable::SUCCESS, zone_table.add(zone_com));
+    EXPECT_EQ(Name("example.com"),
+              zone_table.find(Name("www.example.com")).zone->getOrigin());
+}
+}

src/lib/datasrc/zonetable.cc

@@ -0,0 +1,117 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// Note: map and utility (for 'pair') are for temporary workaround.
+// we'll soon replace them with built-in intelligent backend structure. 
+#include <map>
+#include <utility>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <datasrc/zonetable.h>
+
+using namespace std;
+using namespace isc::dns;
+
+namespace isc {
+namespace datasrc {
+
+struct MemoryZone::MemoryZoneImpl {
+    MemoryZoneImpl(const RRClass& zone_class, const Name& origin) :
+        zone_class_(zone_class), origin_(origin)
+    {}
+    RRClass zone_class_;
+    Name origin_;
+};
+
+MemoryZone::MemoryZone(const RRClass& zone_class, const Name& origin) :
+    impl_(new MemoryZoneImpl(zone_class, origin))
+{
+}
+
+MemoryZone::~MemoryZone() {
+    delete impl_;
+}
+
+const Name&
+MemoryZone::getOrigin() const {
+    return (impl_->origin_);
+}
+
+const RRClass&
+MemoryZone::getClass() const {
+    return (impl_->zone_class_);
+}
+
+Zone::FindResult
+MemoryZone::find(const Name&, const RRType&) const {
+    // This is a tentative implementation that always returns NXDOMAIN.
+    return (FindResult(NXDOMAIN, RRsetPtr()));
+}
+
+// This is a temporary, inefficient implementation using std::map and handmade
+// iteration to realize longest match.
+
+struct ZoneTable::ZoneTableImpl {
+    typedef map<Name, ZonePtr> ZoneMap;
+    typedef pair<Name, ZonePtr> NameAndZone;
+    ZoneMap zones;
+};
+
+ZoneTable::ZoneTable() : impl_(new ZoneTableImpl)
+{}
+
+ZoneTable::~ZoneTable() {
+    delete impl_;
+}
+
+ZoneTable::Result
+ZoneTable::add(ZonePtr zone) {
+    if (!zone) {
+        isc_throw(InvalidParameter,
+                  "Null pointer is passed to ZoneTable::add()");
+    }
+
+    if (impl_->zones.insert(
+            ZoneTableImpl::NameAndZone(zone->getOrigin(), zone)).second
+        == true) {
+        return (SUCCESS);
+    } else {
+        return (EXIST);
+    }
+}
+
+ZoneTable::Result
+ZoneTable::remove(const Name& origin) {
+    return (impl_->zones.erase(origin) == 1 ? SUCCESS : NOTFOUND);
+}
+
+ZoneTable::FindResult
+ZoneTable::find(const Name& name) const {
+    // Inefficient internal loop to find a longest match.
+    // This will be replaced with a single call to more intelligent backend.
+    for (int i = 0; i < name.getLabelCount(); ++i) {
+        Name matchname(name.split(i));
+        ZoneTableImpl::ZoneMap::const_iterator found =
+            impl_->zones.find(matchname);
+        if (found != impl_->zones.end()) {
+            return (FindResult(i == 0 ? SUCCESS : PARTIALMATCH,
+                               (*found).second.get()));
+        }
+    }
+    return (FindResult(NOTFOUND, NULL));
+}
+} // end of namespace datasrc
+} // end of namespace isc

src/lib/datasrc/zonetable.h

@@ -0,0 +1,383 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __ZONETABLE_H
+#define __ZONETABLE_H 1
+
+#include <boost/shared_ptr.hpp>
+
+#include <dns/rrset.h>
+
+namespace isc {
+namespace dns {
+class Name;
+class RRClass;
+};
+
+namespace datasrc {
+
+/// \brief The base class for a single authoritative zone
+///
+/// The \c Zone class is an abstract base class for representing
+/// a DNS zone as part of data source.
+///
+/// At the moment this is provided mainly for making the \c ZoneTable class
+/// and the authoritative query logic  testable, and only provides a minimal
+/// set of features.
+/// This is why this class is defined in the same header file, but it may
+/// have to move to a separate header file when we understand what is
+/// necessary for this class for actual operation.
+///
+/// The idea is to provide a specific derived zone class for each data
+/// source, beginning with in memory one.  At that point the derived classes
+/// will have more specific features.  For example, they will maintain
+/// information about the location of a zone file, whether it's loaded in
+/// memory, etc.
+///
+/// It's not yet clear how the derived zone classes work with various other
+/// data sources when we integrate these components, but one possibility is
+/// something like this:
+/// - If the underlying database such as some variant of SQL doesn't have an
+///   explicit representation of zones (as part of public interface), we can
+///   probably use a "default" zone class that simply encapsulates the
+///   corresponding data source and calls a common "find" like method.
+/// - Some data source may want to specialize it by inheritance as an
+///   optimization.  For example, in the current schema design of the sqlite3
+///   data source, its (derived) zone class would contain the information of
+///   the "zone ID".
+///
+/// <b>Note:</b> Unlike some other abstract base classes we don't name the
+/// class beginning with "Abstract".  This is because we want to have
+/// commonly used definitions such as \c Result and \c ZonePtr, and we want
+/// to make them look more intuitive.
+class Zone {
+public:
+    /// Result codes of the \c find() method.
+    ///
+    /// Note: the codes are tentative.  We may need more, or we may find
+    /// some of them unnecessary as we implement more details.
+    enum Result {
+        SUCCESS,                ///< An exact match is found.
+        DELEGATION,             ///< The search encounters a zone cut.
+        NXDOMAIN, ///< There is no domain name that matches the search name
+        NXRRSET,  ///< There is a matching name but no RRset of the search type
+        CNAME,    ///< The search encounters and returns a CNAME RR
+        DNAME     ///< The search encounters and returns a DNAME RR
+    };
+
+    /// A helper structure to represent the search result of \c find().
+    ///
+    /// This is a straightforward tuple of the result code and a pointer
+    /// to the found RRset to represent the result of \c find()
+    /// (there will be more members in the future - see the class
+    /// description).
+    /// We use this in order to avoid overloading the return value for both
+    /// the result code ("success" or "not found") and the found object,
+    /// i.e., avoid using \c NULL to mean "not found", etc.
+    ///
+    /// This is a simple value class whose internal state never changes,
+    /// so for convenience we allow the applications to refer to the members
+    /// directly.
+    ///
+    /// Note: we should eventually include a notion of "zone node", which
+    /// corresponds to a particular domain name of the zone, so that we can
+    /// find RRsets of a different RR type for that name (e.g. for type ANY
+    /// query or to include DS RRs with delegation).
+    ///
+    /// Note: we may also want to include the closest enclosure "node" to
+    /// optimize including the NSEC for no-wildcard proof (FWIW NSD does that).
+    struct FindResult {
+        FindResult(Result param_code,
+                   const isc::dns::ConstRRsetPtr param_rrset) :
+            code(param_code), rrset(param_rrset)
+        {}
+        const Result code;
+        const isc::dns::ConstRRsetPtr rrset;
+    };
+
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    //@{
+protected:
+    /// The default constructor.
+    ///
+    /// This is intentionally defined as \c protected as this base class should
+    /// never be instantiated (except as part of a derived class).
+    Zone() {}
+public:
+    /// The destructor.
+    virtual ~Zone() {}
+    //@}
+
+    ///
+    /// \name Getter Methods
+    ///
+    /// These methods should never throw an exception.
+    //@{
+    /// Return the origin name of the zone.
+    virtual const isc::dns::Name& getOrigin() const = 0;
+
+    /// Return the RR class of the zone.
+    virtual const isc::dns::RRClass& getClass() const = 0;
+    //@}
+
+    ///
+    /// \name Search Method
+    ///
+    //@{
+    /// Search the zone for a given pair of domain name and RR type.
+    ///
+    /// Each derived version of this method searches the underlying backend
+    /// for the data that best matches the given name and type.
+    /// This method is expected to be "intelligent", and identifies the
+    /// best possible answer for the search key.  Specifically,
+    /// - If the search name belongs under a zone cut, it returns the code
+    ///   of \c DELEGATION and the NS RRset at the zone cut.
+    /// - If there is no matching name, it returns the code of \c NXDOMAIN,
+    ///   and, if DNSSEC is requested, the NSEC RRset that proves the
+    ///   non-existence.
+    /// - If there is a matching name but no RRset of the search type, it
+    ///   returns the code of \c NXRRSET, and, if DNSSEC is required,
+    ///   the NSEC RRset for that name.
+    /// - If there is a matching name with CNAME, it returns the code of
+    ///   \c CNAME and that CNAME RR.
+    /// - If the search name matches a delegation point of DNAME, it returns
+    ///   the code of \c DNAME and that DNAME RR.
+    ///
+    /// A derived version of this method may involve internal resource
+    /// allocation, especially for constructing the resulting RRset, and may
+    /// throw an exception if it fails.
+    /// It should not throw other types of exceptions.
+    ///
+    /// Note: It's quite likely that we'll need to specify search options.
+    /// For example, we should be able to specify whether to allow returning
+    /// glue records at or under a zone cut.  We leave this interface open
+    /// at this moment.
+    ///
+    /// \param name The domain name to be searched for.
+    /// \param type The RR type to be searched for.
+    /// \return A \c FindResult object enclosing the search result (see above).
+    virtual FindResult find(const isc::dns::Name& name,
+                            const isc::dns::RRType& type) const = 0;
+    //@}
+};
+
+/// \brief A pointer-like type pointing to a \c Zone object.
+typedef boost::shared_ptr<Zone> ZonePtr;
+
+/// \brief A pointer-like type pointing to a \c Zone object.
+typedef boost::shared_ptr<const Zone> ConstZonePtr;
+
+/// A derived zone class intended to be used with the memory data source.
+///
+/// Currently this is almost empty and is only used for testing the
+/// \c ZoneTable class.  It will be substantially expanded, and will probably
+/// moved to a separate header file.
+class MemoryZone : public Zone {
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    /// \b Note:
+    /// The copy constructor and the assignment operator are intentionally
+    /// defined as private, making this class non copyable.
+    //@{
+private:
+    MemoryZone(const MemoryZone& source);
+    MemoryZone& operator=(const MemoryZone& source);
+public:
+    /// \brief Constructor from zone parameters.
+    ///
+    /// This constructor internally involves resource allocation, and if
+    /// it fails, a corresponding standard exception will be thrown.
+    /// It never throws an exception otherwise.
+    ///
+    /// \param rrclass The RR class of the zone.
+    /// \param origin The origin name of the zone.
+    MemoryZone(const isc::dns::RRClass& rrclass, const isc::dns::Name& origin);
+
+    /// The destructor.
+    virtual ~MemoryZone();
+    //@}
+
+    virtual const isc::dns::Name& getOrigin() const;
+    virtual const isc::dns::RRClass& getClass() const;
+    virtual FindResult find(const isc::dns::Name& name,
+                            const isc::dns::RRType& type) const;
+
+private:
+    struct MemoryZoneImpl;
+    MemoryZoneImpl* impl_;
+};
+
+/// \brief A set of authoritative zones.
+///
+/// The \c ZoneTable class represents a set of zones of the same RR class
+/// and provides a basic interface to help DNS lookup processing.
+/// For a given domain name, its \c find() method searches the set for a zone
+/// that gives a longest match against that name.
+///
+/// The set of zones are assumed to be of the same RR class, but the
+/// \c ZoneTable class does not enforce the assumption through its interface.
+/// For example, the \c add() method does not check if the new zone
+/// is of the same RR class as that of the others already in the table.
+/// It is caller's responsibility to ensure this assumption.
+///
+/// <b>Notes to developer:</b>
+///
+/// The add() method takes a (Boost) shared pointer because it would be
+/// inconvenient to require the caller to maintain the ownership of zones,
+/// while it wouldn't be safe to delete unnecessary zones inside the zone
+/// table.
+///
+/// On the other hand, the find() method returns a bare pointer, rather than
+/// the shared pointer, in order to minimize the dependency on Boost
+/// definitions in our public interfaces.  This means the caller can only
+/// refer to the returned object (via the pointer) for a short period.
+///  It should be okay for simple lookup purposes, but if we see the need
+/// for keeping a \c Zone object for a longer period of context, we may
+/// have to revisit this decision.
+///
+/// Currently, \c FindResult::zone is immutable for safety.
+/// In future versions we may want to make it changeable.  For example,
+/// we may want to allow configuration update on an existing zone.
+///
+/// In BIND 9's "zt" module, the equivalent of \c find() has an "option"
+/// parameter.  The only defined option is the one to specify the "no exact"
+/// mode, and the only purpose of that mode is to prefer a second longest match
+/// even if there is an exact match in order to deal with type DS query.
+/// This trick may help enhance performance, but it also seems to make the
+/// implementation complicated for a very limited, minor case.  So, for now,
+/// we don't introduce the special mode, and, since it was the only reason to
+/// have search options in BIND 9, our initial implementation doesn't provide
+/// a switch for options.
+class ZoneTable {
+public:
+    /// Result codes of various public methods of \c ZoneTable.
+    ///
+    /// The detailed semantics may differ in different methods.
+    /// See the description of specific methods for more details.
+    enum Result {
+        SUCCESS,  ///< The operation is successful.
+        EXIST,    ///< A zone is already stored in \c ZoneTable.
+        NOTFOUND, ///< The specified zone is not found in \c ZoneTable.
+        PARTIALMATCH ///< \c Only a partial match is found in \c find(). 
+    };
+
+    /// \brief A helper structure to represent the search result of
+    /// <code>ZoneTable::find()</code>.
+    ///
+    /// This is a straightforward pair of the result code and a pointer
+    /// to the found zone to represent the result of \c find().
+    /// We use this in order to avoid overloading the return value for both
+    /// the result code ("success" or "not found") and the found object,
+    /// i.e., avoid using \c NULL to mean "not found", etc.
+    ///
+    /// This is a simple value class with no internal state, so for
+    /// convenience we allow the applications to refer to the members
+    /// directly.
+    ///
+    /// See the description of \c find() for the semantics of the member
+    /// variables.
+    struct FindResult {
+        FindResult(Result param_code, const Zone* param_zone) :
+            code(param_code), zone(param_zone)
+        {}
+        const Result code;
+        const Zone* const zone;
+    };
+
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    /// \b Note:
+    /// The copy constructor and the assignment operator are intentionally
+    /// defined as private, making this class non copyable.
+    //@{
+private:
+    ZoneTable(const ZoneTable& source);
+    ZoneTable& operator=(const ZoneTable& source);
+
+public:
+    /// Default constructor.
+    ///
+    /// This constructor internally involves resource allocation, and if
+    /// it fails, a corresponding standard exception will be thrown.
+    /// It never throws an exception otherwise.
+    ZoneTable();
+
+    /// The destructor.
+    ~ZoneTable();
+    //@}
+
+    /// Add a \c Zone to the \c ZoneTable.
+    ///
+    /// \c zone must not be associated with a NULL pointer; otherwise
+    /// an exception of class \c InvalidParameter will be thrown.
+    /// If internal resource allocation fails, a corresponding standard
+    /// exception will be thrown.
+    /// This method never throws an exception otherwise.
+    ///
+    /// \param zone A \c Zone object to be added.
+    /// \return \c SUCCESS If the zone is successfully added to the zone table.
+    /// \return \c EXIST The zone table already stores a zone that has the
+    /// same origin.
+    Result add(ZonePtr zone);
+
+    /// Remove a \c Zone of the given origin name from the \c ZoneTable.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// \param origin The origin name of the zone to be removed.
+    /// \return \c SUCCESS If the zone is successfully removed from the
+    /// zone table.
+    /// \return \c NOTFOUND The zone table does not store the zone that matches
+    /// \c origin.
+    Result remove(const isc::dns::Name& origin);
+
+    /// Find a \c Zone that best matches the given name in the \c ZoneTable.
+    ///
+    /// It searches the internal storage for a \c Zone that gives the
+    /// longest match against \c name, and returns the result in the
+    /// form of a \c FindResult object as follows:
+    /// - \c code: The result code of the operation.
+    ///   - \c SUCCESS: A zone that gives an exact match is found
+    ///   - \c PARTIALMATCH: A zone whose origin is a super domain of
+    ///     \c name is found (but there is no exact match)
+    ///   - \c NOTFOUND: For all other cases.
+    /// - \c zone: A pointer to the found \c Zone object if one is found;
+    /// otherwise \c NULL.
+    ///
+    /// The pointer returned in the \c FindResult object is only valid until
+    /// the corresponding zone is removed from the zone table.
+    /// The caller must ensure that the zone is held in the zone table while
+    /// it needs to refer to it.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// \param name A domain name for which the search is performed.
+    /// \return A \c FindResult object enclosing the search result (see above).
+    FindResult find(const isc::dns::Name& name) const;
+
+private:
+    struct ZoneTableImpl;
+    ZoneTableImpl* impl_;
+};
+}
+}
+#endif  // __ZONETABLE_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/Makefile.am

@@ -13,6 +13,8 @@ EXTRA_DIST += rrtype-placeholder.h
 
 # TODO: double-check that this is the only way
 # NOTE: when an rdata file is added, please also add to this list:
+EXTRA_DIST += rdata/any_255/tsig_250.cc
+EXTRA_DIST += rdata/any_255/tsig_250.h
 EXTRA_DIST += rdata/in_1/aaaa_28.cc
 EXTRA_DIST += rdata/in_1/aaaa_28.h
 EXTRA_DIST += rdata/in_1/a_1.cc
@@ -81,7 +83,7 @@ libdns___la_SOURCES += rrttl.h rrttl.cc
 libdns___la_SOURCES += rrtype.cc
 libdns___la_SOURCES += question.h question.cc
 libdns___la_SOURCES += util/sha1.h util/sha1.cc
-libdns___la_SOURCES += tsig.h tsig.cc
+libdns___la_SOURCES += tsigkey.h tsigkey.cc
 
 nodist_libdns___la_SOURCES = rdataclass.cc rrclass.h rrtype.h
 nodist_libdns___la_SOURCES += rrparamregistry.cc
@@ -96,11 +98,13 @@ libdns___includedir = $(includedir)/dns
 libdns___include_HEADERS = \
 	buffer.h \
 	dnssectime.h \
+	edns.h \
 	exceptions.h \
 	message.h \
 	messagerenderer.h \
 	name.h \
 	question.h \
+	rcode.h \
 	rdata.h \
 	rdataclass.h \
 	rrclass.h \
@@ -109,7 +113,7 @@ libdns___include_HEADERS = \
 	rrsetlist.h \
 	rrttl.h \
 	rrtype.h \
-	tsig.h
+	tsigkey.h
 # Purposely not installing these headers:
 # util/*.h: used only internally, and not actually DNS specific
 # rrclass-placeholder.h

src/lib/dns/python/Makefile.am

@@ -24,6 +24,7 @@ EXTRA_DIST += question_python.cc
 EXTRA_DIST += rrttl_python.cc
 EXTRA_DIST += rdata_python.cc
 EXTRA_DIST += rrtype_python.cc
+EXTRA_DIST += tsigkey_python.cc
 
 # Python prefers .so, while some OSes (specifically MacOS) use a different
 # suffix for dynamic objects.  -module is necessary to work this around.

src/lib/dns/python/pydnspp.cc

@@ -57,6 +57,7 @@ static PyObject* po_DNSMessageBADVERS;
 #include <dns/python/rrset_python.cc>          // needs Rdata, RRTTL
 #include <dns/python/question_python.cc>       // needs RRClass, RRType, RRTTL,
                                                // Name
+#include <dns/python/tsigkey_python.cc>        // needs Name
 #include <dns/python/opcode_python.cc>
 #include <dns/python/rcode_python.cc>
 #include <dns/python/edns_python.cc>           // needs Messagerenderer, Rcode
@@ -146,6 +147,14 @@ PyInit_pydnspp(void) {
         return (NULL);
     }
 
+    if (!initModulePart_TSIGKey(mod)) {
+        return (NULL);
+    }
+
+    if (!initModulePart_TSIGKeyRing(mod)) {
+        return (NULL);
+    }
+
     return (mod);
 }
 

src/lib/dns/python/rrset_python.cc

@@ -355,7 +355,7 @@ RRset_getRdata(s_RRset* self) {
 
     RdataIteratorPtr it = self->rrset->getRdataIterator();
 
-    for (it->first(); !it->isLast(); it->next()) {
+    for (; !it->isLast(); it->next()) {
         s_Rdata *rds = static_cast<s_Rdata*>(rdata_type.tp_alloc(&rdata_type, 0));
         if (rds != NULL) {
             // hmz them iterators/shared_ptrs and private constructors

src/lib/dns/python/tests/Makefile.am

@@ -10,6 +10,7 @@ PYTESTS += rrclass_python_test.py
 PYTESTS += rrset_python_test.py
 PYTESTS += rrttl_python_test.py
 PYTESTS += rrtype_python_test.py
+PYTESTS += tsigkey_python_test.py
 
 EXTRA_DIST = $(PYTESTS)
 EXTRA_DIST += testutil.py

src/lib/dns/python/tests/tsigkey_python_test.py

@@ -0,0 +1,174 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+# $Id$
+
+import unittest
+from pydnspp import *
+
+class TSIGKeyTest(unittest.TestCase):
+    key_name = Name('example.com')
+    secret = b'anotherRandomData'
+
+    def test_algorithm_names(self):
+        self.assertEqual(Name('hmac-md5.sig-alg.reg.int'),
+                         TSIGKey.HMACMD5_NAME)
+        self.assertEqual(Name('hmac-sha1'), TSIGKey.HMACSHA1_NAME)
+        self.assertEqual(Name('hmac-sha256'), TSIGKey.HMACSHA256_NAME)
+
+    def test_init(self):
+        key = TSIGKey(self.key_name, TSIGKey.HMACMD5_NAME, self.secret)
+        self.assertEqual(self.key_name, key.get_key_name())
+        self.assertEqual(Name('hmac-md5.sig-alg.reg.int'),
+                         key.get_algorithm_name())
+        self.assertEqual(self.secret, key.get_secret())
+
+        self.assertRaises(InvalidParameter, TSIGKey, self.key_name,
+                          Name('unknown-alg'), self.secret)
+
+        self.assertEqual('hmac-sha1.',
+                         TSIGKey(self.key_name, TSIGKey.HMACSHA1_NAME,
+                                 self.secret).get_algorithm_name().to_text())
+
+        self.assertRaises(TypeError, TSIGKey, self.key_name,
+                          TSIGKey.HMACMD5_NAME,
+                          'should be binary') # signature mismatch
+
+class TSIGKeyRingTest(unittest.TestCase):
+    key_name = Name('example.com')
+    secret = b'someRandomData'
+
+    def setUp(self):
+        self.keyring = TSIGKeyRing()
+
+    def test_init(self):
+        self.assertEqual(0, self.keyring.size())
+        self.assertRaises(TypeError, TSIGKeyRing, 1)
+        self.assertRaises(TypeError, TSIGKeyRing, 'there should not be arg')
+
+    def test_add(self):
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        self.assertEqual(1, self.keyring.size())
+        self.assertEqual(TSIGKeyRing.EXIST,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.EXIST,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA1_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.EXIST,
+                         self.keyring.add(TSIGKey(Name('EXAMPLE.COM'),
+                                                  TSIGKey.HMACSHA1_NAME,
+                                                  self.secret)))
+        self.assertEqual(1, self.keyring.size())
+
+    def test_add_more(self):
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(Name('another.example'),
+                                                  TSIGKey.HMACMD5_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(Name('more.example'),
+                                                  TSIGKey.HMACSHA1_NAME,
+                                                  self.secret)))
+        self.assertEqual(3, self.keyring.size())
+
+        self.assertRaises(TypeError, self.keyring.add, 1)
+        self.assertRaises(TypeError, self.keyring.add, 'invalid arg')
+
+    def test_remove(self):
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.remove(self.key_name))
+        self.assertEqual(TSIGKeyRing.NOTFOUND,
+                         self.keyring.remove(self.key_name))
+
+        self.assertRaises(TypeError, self.keyring.add, 1)
+        self.assertRaises(TypeError, self.keyring.add, 'invalid arg')
+        self.assertRaises(TypeError, self.keyring.add, self.key_name, 0)
+
+    def test_remove_from_some(self):
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(Name('another.example'),
+                                                  TSIGKey.HMACMD5_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(Name('more.example'),
+                                                  TSIGKey.HMACSHA1_NAME,
+                                                  self.secret)))
+
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.remove(Name('another.example')))
+        self.assertEqual(TSIGKeyRing.NOTFOUND,
+                         self.keyring.remove(Name('noexist.example')))
+        self.assertEqual(2, self.keyring.size())
+
+    def test_find(self):
+        self.assertEqual((TSIGKeyRing.NOTFOUND, None),
+                         self.keyring.find(self.key_name))
+
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        (code, key) = self.keyring.find(self.key_name)
+        self.assertEqual(TSIGKeyRing.SUCCESS, code)
+        self.assertEqual(self.key_name, key.get_key_name())
+        self.assertEqual(TSIGKey.HMACSHA256_NAME, key.get_algorithm_name())
+        self.assertEqual(self.secret, key.get_secret())
+
+        self.assertRaises(TypeError, self.keyring.find, 1)
+        self.assertRaises(TypeError, self.keyring.find, 'should be a name')
+        self.assertRaises(TypeError, self.keyring.find, self.key_name, 0)
+
+    def test_find_from_some(self):
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(self.key_name,
+                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(Name('another.example'),
+                                                  TSIGKey.HMACMD5_NAME,
+                                                  self.secret)))
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.keyring.add(TSIGKey(Name('more.example'),
+                                                  TSIGKey.HMACSHA1_NAME,
+                                                  self.secret)))
+
+        (code, key) = self.keyring.find(Name('another.example'))
+        self.assertEqual(TSIGKeyRing.SUCCESS, code)
+        self.assertEqual(Name('another.example'), key.get_key_name())
+        self.assertEqual(TSIGKey.HMACMD5_NAME, key.get_algorithm_name())
+
+        self.assertEqual((TSIGKeyRing.NOTFOUND, None),
+                         self.keyring.find(Name('noexist.example')))
+
+if __name__ == '__main__':
+    unittest.main()

src/lib/dns/python/tsigkey_python.cc

@@ -0,0 +1,455 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#include <new>
+
+#include <dns/tsigkey.h>
+
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+//
+// 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
+
+namespace {
+//
+// TSIGKey
+//
+
+// The s_* Class simply covers one instantiation of the object
+
+class s_TSIGKey : public PyObject {
+public:
+    s_TSIGKey() : tsigkey(NULL) {}
+    TSIGKey* tsigkey;
+};
+
+//
+// We declare the functions here, the definitions are below
+// the type definition of the object, since both can use the other
+//
+
+// General creation and destruction
+int TSIGKey_init(s_TSIGKey* self, PyObject* args);
+void TSIGKey_destroy(s_TSIGKey* self);
+
+// These are the functions we export
+// This is a second version of toText, we need one where the argument
+// is a PyObject*, for the str() function in python.
+PyObject* TSIGKey_getKeyName(const s_TSIGKey* self);
+PyObject* TSIGKey_getAlgorithmName(const s_TSIGKey* self);
+PyObject* TSIGKey_getSecret(const s_TSIGKey* self);
+
+// This list contains the actual set of functions we have in
+// python. Each entry has
+// 1. Python method name
+// 2. Our static function here
+// 3. Argument type
+// 4. Documentation
+PyMethodDef TSIGKey_methods[] = {
+    { "get_key_name",
+      reinterpret_cast<PyCFunction>(TSIGKey_getKeyName), METH_NOARGS,
+      "Return the key name." },
+    { "get_algorithm_name",
+      reinterpret_cast<PyCFunction>(TSIGKey_getAlgorithmName), METH_NOARGS,
+      "Return the algorithm name." },
+    { "get_secret",
+      reinterpret_cast<PyCFunction>(TSIGKey_getSecret), METH_NOARGS,
+      "Return the value of the TSIG secret." },
+    { NULL, NULL, 0, NULL }
+};
+
+// This defines the complete type for reflection in python and
+// parsing of PyObject* to s_EDNS
+// Most of the functions are not actually implemented and NULL here.
+PyTypeObject tsigkey_type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "libdns_python.TSIGKey",
+    sizeof(s_TSIGKey),                  // tp_basicsize
+    0,                                  // tp_itemsize
+    (destructor)TSIGKey_destroy,        // tp_dealloc
+    NULL,                               // tp_print
+    NULL,                               // tp_getattr
+    NULL,                               // tp_setattr
+    NULL,                               // tp_reserved
+    NULL,                               // tp_repr
+    NULL,                               // tp_as_number
+    NULL,                               // tp_as_sequence
+    NULL,                               // tp_as_mapping
+    NULL,                               // tp_hash 
+    NULL,                               // tp_call
+    NULL,                               // tp_str
+    NULL,                               // tp_getattro
+    NULL,                               // tp_setattro
+    NULL,                               // tp_as_buffer
+    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    "The TSIGKey class holds a TSIG key along with some related attributes as "
+    "defined in RFC2845.",
+    NULL,                               // tp_traverse
+    NULL,                               // tp_clear
+    NULL,                               // tp_richcompare
+    0,                                  // tp_weaklistoffset
+    NULL,                               // tp_iter
+    NULL,                               // tp_iternext
+    TSIGKey_methods,                    // tp_methods
+    NULL,                               // tp_members
+    NULL,                               // tp_getset
+    NULL,                               // tp_base
+    NULL,                               // tp_dict
+    NULL,                               // tp_descr_get
+    NULL,                               // tp_descr_set
+    0,                                  // tp_dictoffset
+    (initproc)TSIGKey_init,             // tp_init
+    NULL,                               // tp_alloc
+    PyType_GenericNew,                  // tp_new
+    NULL,                               // tp_free
+    NULL,                               // tp_is_gc
+    NULL,                               // tp_bases
+    NULL,                               // tp_mro
+    NULL,                               // tp_cache
+    NULL,                               // tp_subclasses
+    NULL,                               // tp_weaklist
+    NULL,                               // tp_del
+    0                                   // tp_version_tag
+};
+
+// A helper function to build a python "Name" object with error handling
+// encapsulated.
+s_Name*
+createNameObject(const Name& source) {
+    s_Name* name = PyObject_New(s_Name, &name_type);
+    if (name == NULL) {
+        return (NULL);
+    }
+    name->name = new(nothrow) Name(source);
+    if (name->name == NULL) {
+        Py_DECREF(name);
+        PyErr_SetString(po_IscException, "Allocating Name object failed");
+        return (NULL);
+    }
+    return (name);
+}
+
+int
+TSIGKey_init(s_TSIGKey* self, PyObject* args) {
+    const s_Name* key_name;
+    const s_Name* algorithm_name;
+    PyObject* bytes_obj;
+    const char* secret;
+    Py_ssize_t secret_len;
+
+    if (PyArg_ParseTuple(args, "O!O!O", &name_type, &key_name,
+                         &name_type, &algorithm_name, &bytes_obj) &&
+        PyObject_AsCharBuffer(bytes_obj, &secret, &secret_len) != -1) {
+        try {
+            self->tsigkey = new TSIGKey(*key_name->name,
+                                        *algorithm_name->name,
+                                        secret, secret_len);
+        } catch (const isc::InvalidParameter& ex) {
+            PyErr_SetString(po_InvalidParameter, ex.what());
+            return (-1);
+        } catch (...) {
+            PyErr_SetString(po_IscException, "Unexpected exception");
+            return (-1);
+        }
+        return (0);
+    }
+
+    PyErr_Clear();
+    PyErr_SetString(PyExc_TypeError,
+                    "Invalid arguments to TSIGKey constructor");
+
+    return (-1);
+}
+
+void
+TSIGKey_destroy(s_TSIGKey* const self) {
+    delete self->tsigkey;
+    self->tsigkey = NULL;
+    Py_TYPE(self)->tp_free(self);
+}
+
+PyObject*
+TSIGKey_getKeyName(const s_TSIGKey* const self) {
+    return (createNameObject(self->tsigkey->getKeyName()));
+}
+
+PyObject*
+TSIGKey_getAlgorithmName(const s_TSIGKey* const self) {
+    return (createNameObject(self->tsigkey->getAlgorithmName()));
+}
+
+PyObject*
+TSIGKey_getSecret(const s_TSIGKey* const self) {
+    return (Py_BuildValue("y#", self->tsigkey->getSecret(),
+                          self->tsigkey->getSecretLength()));
+}
+
+// Module Initialization, all statics are initialized here
+bool
+initModulePart_TSIGKey(PyObject* mod) {
+    // We initialize the static description object with PyType_Ready(),
+    // then add it to the module. This is not just a check! (leaving
+    // this out results in segmentation faults)
+    if (PyType_Ready(&tsigkey_type) < 0) {
+        return (false);
+    }
+    Py_INCREF(&tsigkey_type);
+    void* p = &tsigkey_type;
+    if (PyModule_AddObject(mod, "TSIGKey", static_cast<PyObject*>(p)) != 0) {
+        Py_DECREF(&tsigkey_type);
+        return (false);
+    }
+
+    s_Name* name;
+    if ((name = createNameObject(TSIGKey::HMACMD5_NAME())) == NULL) {
+        goto cleanup;
+    }
+    addClassVariable(tsigkey_type, "HMACMD5_NAME", name);
+    if ((name = createNameObject(TSIGKey::HMACSHA1_NAME())) == NULL) {
+        goto cleanup;
+    }
+    addClassVariable(tsigkey_type, "HMACSHA1_NAME", name);
+    if ((name = createNameObject(TSIGKey::HMACSHA256_NAME())) == NULL) {
+        goto cleanup;
+    }
+    addClassVariable(tsigkey_type, "HMACSHA256_NAME", name);
+
+    return (true);
+
+  cleanup:
+    Py_DECREF(&tsigkey_type);
+    return (false);
+}
+//
+// End of TSIGKey
+//
+
+//
+// TSIGKeyRing
+//
+
+// The s_* Class simply covers one instantiation of the object
+
+// The s_* Class simply covers one instantiation of the object
+
+class s_TSIGKeyRing : public PyObject {
+public:
+    s_TSIGKeyRing() : keyring(NULL) {}
+    TSIGKeyRing* keyring;
+};
+
+//
+// We declare the functions here, the definitions are below
+// the type definition of the object, since both can use the other
+//
+
+int TSIGKeyRing_init(s_TSIGKeyRing* self, PyObject* args);
+void TSIGKeyRing_destroy(s_TSIGKeyRing* self);
+
+PyObject* TSIGKeyRing_size(const s_TSIGKeyRing* self);
+PyObject* TSIGKeyRing_add(const s_TSIGKeyRing* self, PyObject* args);
+PyObject* TSIGKeyRing_remove(const s_TSIGKeyRing* self, PyObject* args);
+PyObject* TSIGKeyRing_find(const s_TSIGKeyRing* self, PyObject* args);
+
+PyMethodDef TSIGKeyRing_methods[] = {
+    { "size", reinterpret_cast<PyCFunction>(TSIGKeyRing_size), METH_NOARGS,
+      "Return the number of keys stored in the TSIGKeyRing." },
+    { "add", reinterpret_cast<PyCFunction>(TSIGKeyRing_add), METH_VARARGS,
+      "Add a TSIGKey to the TSIGKeyRing." },
+    { "remove", reinterpret_cast<PyCFunction>(TSIGKeyRing_remove),
+      METH_VARARGS,
+      "Remove a TSIGKey for the given name from the TSIGKeyRing." },
+    { "find", reinterpret_cast<PyCFunction>(TSIGKeyRing_find), METH_VARARGS,
+      "Find a TSIGKey for the given name in the TSIGKeyRing. "
+      "It returns a tuple of (result_code, key)." },
+    { NULL, NULL, 0, NULL }
+};
+
+PyTypeObject tsigkeyring_type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "libdns_python.TSIGKeyRing",
+    sizeof(s_TSIGKeyRing),              // tp_basicsize
+    0,                                  // tp_itemsize
+    (destructor)TSIGKeyRing_destroy,    // tp_dealloc
+    NULL,                               // tp_print
+    NULL,                               // tp_getattr
+    NULL,                               // tp_setattr
+    NULL,                               // tp_reserved
+    NULL,                               // tp_repr
+    NULL,                               // tp_as_number
+    NULL,                               // tp_as_sequence
+    NULL,                               // tp_as_mapping
+    NULL,                               // tp_hash 
+    NULL,                               // tp_call
+    NULL,                               // tp_str
+    NULL,                               // tp_getattro
+    NULL,                               // tp_setattro
+    NULL,                               // tp_as_buffer
+    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    "A simple repository of a set of TSIGKey objects.",
+    NULL,                               // tp_traverse
+    NULL,                               // tp_clear
+    NULL,                               // tp_richcompare
+    0,                                  // tp_weaklistoffset
+    NULL,                               // tp_iter
+    NULL,                               // tp_iternext
+    TSIGKeyRing_methods,                // tp_methods
+    NULL,                               // tp_members
+    NULL,                               // tp_getset
+    NULL,                               // tp_base
+    NULL,                               // tp_dict
+    NULL,                               // tp_descr_get
+    NULL,                               // tp_descr_set
+    0,                                  // tp_dictoffset
+    (initproc)TSIGKeyRing_init,         // tp_init
+    NULL,                               // tp_alloc
+    PyType_GenericNew,                  // tp_new
+    NULL,                               // tp_free
+    NULL,                               // tp_is_gc
+    NULL,                               // tp_bases
+    NULL,                               // tp_mro
+    NULL,                               // tp_cache
+    NULL,                               // tp_subclasses
+    NULL,                               // tp_weaklist
+    NULL,                               // tp_del
+    0                                   // tp_version_tag
+};
+
+int
+TSIGKeyRing_init(s_TSIGKeyRing* self, PyObject* args) {
+    if (!PyArg_ParseTuple(args, "")) {
+        PyErr_Clear();
+        PyErr_SetString(PyExc_TypeError,
+                        "Invalid arguments to TSIGKeyRing constructor");
+        return (-1);
+    }
+    
+    self->keyring = new(nothrow) TSIGKeyRing();
+    if (self->keyring == NULL) {
+        PyErr_SetString(po_IscException, "Allocating TSIGKeyRing failed");
+        return (-1);
+    }
+
+    return (0);
+}
+
+void
+TSIGKeyRing_destroy(s_TSIGKeyRing* self) {
+    delete self->keyring;
+    self->keyring = NULL;
+    Py_TYPE(self)->tp_free(self);
+}
+
+PyObject*
+TSIGKeyRing_size(const s_TSIGKeyRing* const self) {
+    return (Py_BuildValue("I", self->keyring->size()));
+}
+
+PyObject*
+TSIGKeyRing_add(const s_TSIGKeyRing* const self, PyObject* args) {
+    s_TSIGKey* tsigkey;
+    
+    if (PyArg_ParseTuple(args, "O!", &tsigkey_type, &tsigkey)) {
+        try {
+            const TSIGKeyRing::Result result =
+                self->keyring->add(*tsigkey->tsigkey);
+            return (Py_BuildValue("I", result));
+        } catch (...) {
+            PyErr_SetString(po_IscException, "Unexpected exception");
+            return (NULL);
+        }
+    }
+
+    PyErr_Clear();
+    PyErr_SetString(PyExc_TypeError, "Invalid arguments to TSIGKeyRing.add");
+
+    return (NULL);
+}
+
+PyObject*
+TSIGKeyRing_remove(const s_TSIGKeyRing* self, PyObject* args) {
+    s_Name* key_name;
+
+    if (PyArg_ParseTuple(args, "O!", &name_type, &key_name)) {
+        const TSIGKeyRing::Result result =
+            self->keyring->remove(*key_name->name);
+        return (Py_BuildValue("I", result));
+    }
+
+    PyErr_Clear();
+    PyErr_SetString(PyExc_TypeError, "Invalid arguments to TSIGKeyRing.add");
+
+    return (NULL);
+}
+
+PyObject*
+TSIGKeyRing_find(const s_TSIGKeyRing* self, PyObject* args) {
+    s_Name* key_name;
+
+    if (PyArg_ParseTuple(args, "O!", &name_type, &key_name)) {
+        const TSIGKeyRing::FindResult result =
+            self->keyring->find(*key_name->name);
+        if (result.key != NULL) {
+            s_TSIGKey* key = PyObject_New(s_TSIGKey, &tsigkey_type);
+            if (key == NULL) {
+                return (NULL);
+            }
+            key->tsigkey = new(nothrow) TSIGKey(*result.key);
+            if (key->tsigkey == NULL) {
+                Py_DECREF(key);
+                PyErr_SetString(po_IscException,
+                                "Allocating TSIGKey object failed");
+                return (NULL);
+            }
+            return (Py_BuildValue("IN", result.code, key));
+        } else {
+            return (Py_BuildValue("Is", result.code, NULL));
+        }
+    }
+
+    return (NULL);
+}
+
+bool
+initModulePart_TSIGKeyRing(PyObject* mod) {
+    if (PyType_Ready(&tsigkeyring_type) < 0) {
+        return (false);
+    }
+    Py_INCREF(&tsigkeyring_type);
+    void* p = &tsigkeyring_type;
+    if (PyModule_AddObject(mod, "TSIGKeyRing",
+                           static_cast<PyObject*>(p)) != 0) {
+        Py_DECREF(&tsigkeyring_type);
+        return (false);
+    }
+
+    addClassVariable(tsigkeyring_type, "SUCCESS",
+                     Py_BuildValue("I", TSIGKeyRing::SUCCESS));
+    addClassVariable(tsigkeyring_type, "EXIST",
+                     Py_BuildValue("I", TSIGKeyRing::EXIST));
+    addClassVariable(tsigkeyring_type, "NOTFOUND",
+                     Py_BuildValue("I", TSIGKeyRing::NOTFOUND));
+
+    return (true);
+}
+
+} // end of unnamed namespace

src/lib/dns/rdata/any_255/tsig_250.cc

@@ -0,0 +1,501 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#include <string>
+#include <sstream>
+#include <vector>
+
+#include <boost/lexical_cast.hpp>
+
+#include <dns/buffer.h>
+#include <dns/messagerenderer.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+
+#include <dns/util/base64.h>
+
+using namespace std;
+using namespace boost;
+
+// BEGIN_ISC_NAMESPACE
+// BEGIN_RDATA_NAMESPACE
+
+/// This is a straightforward representation of TSIG RDATA fields.
+struct TSIG::TSIGImpl {
+    TSIGImpl(const Name& algorithm, uint64_t time_signed, uint16_t fudge,
+             vector<uint8_t>& mac, uint16_t original_id, uint16_t error,
+             vector<uint8_t>& other_data) :
+        algorithm_(algorithm), time_signed_(time_signed), fudge_(fudge),
+        mac_(mac), original_id_(original_id), error_(error),
+        other_data_(other_data)
+    {}
+    TSIGImpl(const Name& algorithm, uint64_t time_signed, uint16_t fudge,
+             size_t macsize, const void* mac, uint16_t original_id,
+             uint16_t error, size_t other_len, const void* other_data) :
+        algorithm_(algorithm), time_signed_(time_signed), fudge_(fudge),
+        mac_(static_cast<const uint8_t*>(mac),
+             static_cast<const uint8_t*>(mac) + macsize),
+        original_id_(original_id), error_(error),
+        other_data_(static_cast<const uint8_t*>(other_data),
+                    static_cast<const uint8_t*>(other_data) + other_len)
+    {}
+    template <typename Output>
+    void toWireCommon(Output& output) const;
+
+    const Name algorithm_;
+    const uint64_t time_signed_;
+    const uint16_t fudge_;
+    const vector<uint8_t> mac_;
+    const uint16_t original_id_;
+    const uint16_t error_;
+    const vector<uint8_t> other_data_;
+};
+
+namespace {
+string
+getToken(istringstream& iss, const string& full_input) {
+    string token;
+    iss >> token;
+    if (iss.bad() || iss.fail()) {
+        isc_throw(InvalidRdataText, "Invalid TSIG text: parse error" <<
+                  full_input);
+    }
+    return (token);
+}
+
+// This helper function converts a string token to an *unsigned* integer.
+// NumType is a *signed* integral type (e.g. int32_t) that is sufficiently
+// wide to store resulting integers.
+// BitSize is the maximum number of bits that the resulting integer can take.
+// This function first checks whether the given token can be converted to
+// an integer of NumType type.  It then confirms the conversion result is
+// within the valid range, i.e., [0, 2^NumType - 1].  The second check is
+// necessary because lexical_cast<T> where T is an unsigned integer type
+// doesn't correctly reject negative numbers when compiled with SunStudio.
+template <typename NumType, int BitSize>
+NumType
+tokenToNum(const string& num_token) {
+    NumType num;
+    try {
+        num = lexical_cast<NumType>(num_token);
+    } catch (const boost::bad_lexical_cast& ex) {
+        isc_throw(InvalidRdataText, "Invalid TSIG numeric parameter: " <<
+                  num_token);
+    }
+    if (num < 0 || num >= (static_cast<NumType>(1) << BitSize)) {
+        isc_throw(InvalidRdataText, "Numeric TSIG parameter out of range: " <<
+                  num);
+    }
+    return (num);
+}
+}
+
+/// \brief Constructor from string.
+///
+/// \c tsig_str must be formatted as follows:
+/// \code <Alg> <Time> <Fudge> <MACsize> [<MAC>] <OrigID> <Error> <OtherLen> [<OtherData>]
+/// \endcode
+/// where
+/// - <Alg> is a valid textual representation of domain name.
+/// - <Time> is an unsigned 48-bit decimal integer.
+/// - <MACSize>, <OrigID>, and <OtherLen> are an unsigned 16-bit decimal
+///   integer.
+/// - <Error> is an unsigned 16-bit decimal integer or a valid mnemonic for
+///   the Error field specified in RFC2845.  Currently, "BADSIG", "BADKEY",
+///   and "BADTIME" are supported (case sensitive).  In future versions
+///   other representations that are compatible with the DNS RCODE will be
+///   supported.
+/// - <MAC> and <OtherData> is a BASE-64 encoded string that does not contain
+///   space characters.
+///   When <MACSize> and <OtherLen> is 0, <MAC> and <OtherData> must not
+///   appear in \c tsgi_str, respectively.
+/// - The decoded data of <MAC> is <MACSize> bytes of binary stream.
+/// - The decoded data of <OtherData> is <OtherLen> bytes of binary stream.
+///
+/// An example of valid string is:
+/// \code "hmac-sha256. 853804800 300 3 AAAA 2845 0 0" \endcode
+/// In this example <OtherData> is missing because <OtherLen> is 0.
+///
+/// Note that RFC2845 does not define the standard presentation format
+/// of %TSIG RR, so the above syntax is implementation specific.
+/// This is, however, compatible with the format acceptable to BIND 9's
+/// RDATA parser.
+///
+/// <b>Exceptions</b>
+///
+/// If <Alg> is not a valid domain name, a corresponding exception from
+/// the \c Name class will be thrown;
+/// if <MAC> or <OtherData> is not validly encoded in BASE-64, an exception
+/// of class \c isc::BadValue will be thrown;
+/// if %any of the other bullet points above is not met, an exception of
+/// class \c InvalidRdataText will be thrown.
+/// This constructor internally involves resource allocation, and if it fails
+/// a corresponding standard exception will be thrown.
+TSIG::TSIG(const std::string& tsig_str) : impl_(NULL) {
+    istringstream iss(tsig_str);
+
+    const Name algorithm(getToken(iss, tsig_str));
+    const int64_t time_signed = tokenToNum<int64_t, 48>(getToken(iss,
+                                                                 tsig_str));
+    const int32_t fudge = tokenToNum<int32_t, 16>(getToken(iss, tsig_str));
+    const int32_t macsize = tokenToNum<int32_t, 16>(getToken(iss, tsig_str));
+
+    const string mac_txt = (macsize > 0) ? getToken(iss, tsig_str) : "";
+    vector<uint8_t> mac;
+    decodeBase64(mac_txt, mac);
+    if (mac.size() != macsize) {
+        isc_throw(InvalidRdataText, "TSIG MAC size and data are inconsistent");
+    }
+
+    const int32_t orig_id = tokenToNum<int32_t, 16>(getToken(iss, tsig_str));
+
+    const string error_txt = getToken(iss, tsig_str);
+    int32_t error = 0;
+    // XXX: In the initial implementation we hardcode the mnemonics.
+    // We'll soon generalize this.
+    if (error_txt == "BADSIG") {
+        error = 16;
+    } else if (error_txt == "BADKEY") {
+        error = 17;
+    } else if (error_txt == "BADTIME") {
+        error = 18;
+    } else {
+        error = tokenToNum<int32_t, 16>(error_txt);
+    }
+
+    const int32_t otherlen = tokenToNum<int32_t, 16>(getToken(iss, tsig_str));
+    const string otherdata_txt = (otherlen > 0) ? getToken(iss, tsig_str) : "";
+    vector<uint8_t> other_data;
+    decodeBase64(otherdata_txt, other_data);
+
+    if (!iss.eof()) {
+        isc_throw(InvalidRdataText, "Unexpected input for TSIG RDATA: " <<
+                  tsig_str);
+    }
+
+    impl_ = new TSIGImpl(algorithm, time_signed, fudge, mac, orig_id,
+                         error, other_data);
+}
+
+/// \brief Constructor from wire-format data.
+///
+/// When a read operation on \c buffer fails (e.g., due to a corrupted
+/// message) a corresponding exception from the \c InputBuffer class will
+/// be thrown.
+/// If the wire-format data does not begin with a valid domain name,
+/// a corresponding exception from the \c Name class will be thrown.
+/// In addition, this constructor internally involves resource allocation,
+/// and if it fails a corresponding standard exception will be thrown.
+///
+/// According to RFC3597, the Algorithm field must be a non compressed form
+/// of domain name.  But this implementation accepts a %TSIG RR even if that
+/// field is compressed.
+///
+/// \param buffer A buffer storing the wire format data.
+/// \param rdata_len The length of the RDATA in bytes, normally expected
+/// to be the value of the RDLENGTH field of the corresponding RR.
+/// But this constructor does not use this parameter; if necessary, the caller
+/// must check consistency between the length parameter and the actual
+/// RDATA length.
+TSIG::TSIG(InputBuffer& buffer, size_t) : impl_(NULL) {
+    Name algorithm(buffer);
+
+    uint8_t time_signed_buf[6];
+    buffer.readData(time_signed_buf, sizeof(time_signed_buf));
+    const uint64_t time_signed =
+        (static_cast<uint64_t>(time_signed_buf[0]) << 40 |
+         static_cast<uint64_t>(time_signed_buf[1]) << 32 |
+         static_cast<uint64_t>(time_signed_buf[2]) << 24 |
+         static_cast<uint64_t>(time_signed_buf[3]) << 16 |
+         static_cast<uint64_t>(time_signed_buf[4]) << 8 |
+         static_cast<uint64_t>(time_signed_buf[5]));
+
+    const uint16_t fudge = buffer.readUint16();
+
+    const uint16_t mac_size = buffer.readUint16();
+    vector<uint8_t> mac(mac_size);
+    if (mac_size > 0) {
+        buffer.readData(&mac[0], mac_size);
+    }
+
+    const uint16_t original_id = buffer.readUint16();
+    const uint16_t error = buffer.readUint16();
+
+    const uint16_t other_len = buffer.readUint16();
+    vector<uint8_t> other_data(other_len);
+    if (other_len > 0) {
+        buffer.readData(&other_data[0], other_len);
+    }
+
+    impl_ = new TSIGImpl(algorithm, time_signed, fudge, mac, original_id,
+                         error, other_data);
+}
+
+TSIG::TSIG(const Name& algorithm, uint64_t time_signed, uint16_t fudge,
+           uint16_t mac_size, const void* mac, uint16_t original_id,
+           uint16_t error, uint16_t other_len, const void* other_data) :
+    impl_(NULL)
+{
+    // Time Signed is a 48-bit value.
+    if ((time_signed >> 48) != 0) {
+        isc_throw(OutOfRange, "TSIG Time Signed is too large: " <<
+                  time_signed);
+    }
+    if ((mac_size == 0 && mac != NULL) || (mac_size > 0 && mac == NULL)) {
+        isc_throw(InvalidParameter, "TSIG MAC size and data inconsistent");
+    }
+    if ((other_len == 0 && other_data != NULL) ||
+        (other_len > 0 && other_data == NULL)) {
+        isc_throw(InvalidParameter,
+                  "TSIG Other data length and data inconsistent");
+    }
+    impl_ = new TSIGImpl(algorithm, time_signed, fudge, mac_size, mac,
+                         original_id, error, other_len, other_data);
+}
+
+/// \brief The copy constructor.
+///
+/// It internally allocates a resource, and if it fails a corresponding
+/// standard exception will be thrown.
+/// This constructor never throws an exception otherwise.
+TSIG::TSIG(const TSIG& source) : Rdata(), impl_(new TSIGImpl(*source.impl_))
+{}
+
+TSIG&
+TSIG::operator=(const TSIG& source) {
+    if (impl_ == source.impl_) {
+        return (*this);
+    }
+
+    TSIGImpl* newimpl = new TSIGImpl(*source.impl_);
+    delete impl_;
+    impl_ = newimpl;
+
+    return (*this);
+}
+
+TSIG::~TSIG() {
+    delete impl_;
+}
+
+/// \brief Convert the \c TSIG to a string.
+///
+/// The output of this method is formatted as described in the "from string"
+/// constructor (\c TSIG(const std::string&))).
+///
+/// If internal resource allocation fails, a corresponding
+/// standard exception will be thrown.
+///
+/// \return A \c string object that represents the \c TSIG object.
+std::string
+TSIG::toText() const {
+    string result;
+
+    result += impl_->algorithm_.toText() + " " +
+        lexical_cast<string>(impl_->time_signed_) + " " +
+        lexical_cast<string>(impl_->fudge_) + " " +
+        lexical_cast<string>(impl_->mac_.size()) + " ";
+    if (impl_->mac_.size() > 0) {
+        result += encodeBase64(impl_->mac_) + " ";
+    }
+    result += lexical_cast<string>(impl_->original_id_) + " ";
+    if (impl_->error_ == 16) {  // XXX: we'll soon introduce generic converter.
+        result += "BADSIG ";
+    } else if (impl_->error_ == 17) {
+        result += "BADKEY ";
+    } else if (impl_->error_ == 18) {
+        result += "BADTIME ";
+    } else {
+        result += lexical_cast<string>(impl_->error_) + " ";
+    }
+    result += lexical_cast<string>(impl_->other_data_.size());
+    if (impl_->other_data_.size() > 0) {
+        result += " " + encodeBase64(impl_->other_data_);
+    }
+
+    return (result);
+}
+
+// Common sequence of toWire() operations used for the two versions of
+// toWire().
+template <typename Output>
+void
+TSIG::TSIGImpl::toWireCommon(Output& output) const {
+    output.writeUint16(time_signed_ >> 32);
+    output.writeUint32(time_signed_ & 0xffffffff);
+    output.writeUint16(fudge_);
+    const uint16_t mac_size = mac_.size();
+    output.writeUint16(mac_size);
+    if (mac_size > 0) {
+        output.writeData(&mac_[0], mac_size);
+    }
+    output.writeUint16(original_id_);
+    output.writeUint16(error_);
+    const uint16_t other_len = other_data_.size();
+    output.writeUint16(other_len);
+    if (other_len > 0) {
+        output.writeData(&other_data_[0], other_len);
+    }
+}
+
+/// \brief Render the \c TSIG in the wire format without name compression.
+///
+/// If internal resource allocation fails, a corresponding
+/// standard exception will be thrown.
+/// This method never throws an exception otherwise.
+///
+/// \param buffer An output buffer to store the wire data.
+void
+TSIG::toWire(OutputBuffer& buffer) const {
+    impl_->algorithm_.toWire(buffer);
+    impl_->toWireCommon<OutputBuffer>(buffer);
+}
+
+/// \brief Render the \c TSIG in the wire format with taking into account
+/// compression.
+///
+/// As specified in RFC3597, the Algorithm field (a domain name) will not
+/// be compressed.  However, the domain name could be a target of compression
+/// of other compressible names (though pretty unlikely), the offset
+/// information of the algorithm name may be recorded in \c renderer.
+///
+/// If internal resource allocation fails, a corresponding
+/// standard exception will be thrown.
+/// This method never throws an exception otherwise.
+///
+/// \param renderer DNS message rendering context that encapsulates the
+/// output buffer and name compression information.
+void
+TSIG::toWire(MessageRenderer& renderer) const {
+    renderer.writeName(impl_->algorithm_, false);
+    impl_->toWireCommon<MessageRenderer>(renderer);
+}
+
+// A helper function commonly used for TSIG::compare().
+int
+vectorComp(const vector<uint8_t>& v1, const vector<uint8_t>& v2) {
+    const size_t this_size = v1.size();
+    const size_t other_size = v2.size();
+    if (this_size != other_size) {
+        return (this_size < other_size ? -1 : 1);
+    }
+    if (this_size > 0) {
+        return (memcmp(&v1[0], &v2[0], this_size));
+    }
+    return (0);
+}
+
+/// \brief Compare two instances of \c TSIG RDATA.
+///
+/// This method compares \c this and the \c other \c TSIG objects
+/// in terms of the DNSSEC sorting order as defined in RFC4034, and returns
+/// the result as an integer.
+///
+/// This method is expected to be used in a polymorphic way, and the
+/// parameter to compare against is therefore of the abstract \c Rdata class.
+/// However, comparing two \c Rdata objects of different RR types
+/// is meaningless, and \c other must point to a \c TSIG object;
+/// otherwise, the standard \c bad_cast exception will be thrown.
+/// This method never throws an exception otherwise.
+///
+/// \param other the right-hand operand to compare against.
+/// \return < 0 if \c this would be sorted before \c other.
+/// \return 0 if \c this is identical to \c other in terms of sorting order.
+/// \return > 0 if \c this would be sorted after \c other.
+int
+TSIG::compare(const Rdata& other) const {
+    const TSIG& other_tsig = dynamic_cast<const TSIG&>(other);
+
+    const int ncmp = compareNames(impl_->algorithm_,
+                                  other_tsig.impl_->algorithm_);
+    if (ncmp != 0) {
+        return (ncmp);
+    }
+
+    if (impl_->time_signed_ != other_tsig.impl_->time_signed_) {
+        return (impl_->time_signed_ < other_tsig.impl_->time_signed_ ? -1 : 1);
+    }
+    if (impl_->fudge_ != other_tsig.impl_->fudge_) {
+        return (impl_->fudge_ < other_tsig.impl_->fudge_ ? -1 : 1);
+    }
+    const int vcmp = vectorComp(impl_->mac_, other_tsig.impl_->mac_);
+    if (vcmp != 0) {
+        return (vcmp);
+    }
+    if (impl_->original_id_ != other_tsig.impl_->original_id_) {
+        return (impl_->original_id_ < other_tsig.impl_->original_id_ ? -1 : 1);
+    }
+    if (impl_->error_ != other_tsig.impl_->error_) {
+        return (impl_->error_ < other_tsig.impl_->error_ ? -1 : 1);
+    }
+    return (vectorComp(impl_->other_data_, other_tsig.impl_->other_data_));
+}
+
+const Name&
+TSIG::getAlgorithm() const {
+    return (impl_->algorithm_);
+}
+
+uint64_t
+TSIG::getTimeSigned() const {
+    return (impl_->time_signed_);
+}
+
+uint16_t
+TSIG::getFudge() const {
+    return (impl_->fudge_);
+}
+
+uint16_t
+TSIG::getMACSize() const {
+    return (impl_->mac_.size());
+}
+
+const void*
+TSIG::getMAC() const {
+    if (impl_->mac_.size() > 0) {
+        return (&impl_->mac_[0]);
+    } else {
+        return (NULL);
+    }
+}
+
+uint16_t
+TSIG::getOriginalID() const {
+    return (impl_->original_id_);
+}
+
+uint16_t
+TSIG::getError() const {
+    return (impl_->error_);
+}
+
+uint16_t
+TSIG::getOtherLen() const {
+    return (impl_->other_data_.size());
+}
+
+const void*
+TSIG::getOtherData() const {
+    if (impl_->other_data_.size() > 0) {
+        return (&impl_->other_data_[0]);
+    } else {
+        return (NULL);
+    }
+}
+
+// END_RDATA_NAMESPACE
+// END_ISC_NAMESPACE

src/lib/dns/rdata/any_255/tsig_250.h

@@ -0,0 +1,160 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+// BEGIN_HEADER_GUARD
+
+#include <stdint.h>
+
+#include <string>
+
+#include <dns/rdata.h>
+
+namespace isc {
+namespace dns {
+class Name;
+}
+}
+
+// BEGIN_ISC_NAMESPACE
+
+// BEGIN_COMMON_DECLARATIONS
+// END_COMMON_DECLARATIONS
+
+// BEGIN_RDATA_NAMESPACE
+
+/// \brief \c rdata::TSIG class represents the TSIG RDATA as defined %in
+/// RFC2845.
+///
+/// This class implements the basic interfaces inherited from the abstract
+/// \c rdata::Rdata class, and provides trivial accessors specific to the
+/// TSIG RDATA.
+class TSIG : public Rdata {
+public:
+    // BEGIN_COMMON_MEMBERS
+    // END_COMMON_MEMBERS
+
+    /// \brief Constructor from RDATA field parameters.
+    ///
+    /// The parameters are a straightforward mapping of %TSIG RDATA
+    /// fields as defined %in RFC2845, but there are some implementation
+    /// specific notes as follows.
+    ///
+    /// \c algorithm is a \c Name object that specifies the algorithm.
+    /// For example, if the algorithm is HMAC-SHA256, \c algorithm would be
+    /// \c Name("hmac-sha256").
+    ///
+    /// \c time_signed corresponds to the Time Signed field, which is of
+    /// 48-bit unsigned integer type, and therefore cannot exceed 2^48-1;
+    /// otherwise, an exception of type \c OutOfRange will be thrown.
+    ///
+    /// \c mac_size and \c mac correspond to the MAC Size and MAC fields,
+    /// respectively.  When the MAC field is empty, \c mac must be NULL.
+    /// \c mac_size and \c mac must be consistent %in that \c mac_size is 0 if
+    /// and only if \c mac is NULL; otherwise an exception of type
+    /// InvalidParameter will be thrown.
+    ///
+    /// The same restriction applies to \c other_len and \c other_data,
+    /// which correspond to the Other Len and Other Data fields, respectively.
+    ///
+    /// This constructor internally involves resource allocation, and if
+    /// it fails, a corresponding standard exception will be thrown.
+    TSIG(const Name& algorithm, uint64_t time_signed, uint16_t fudge,
+         uint16_t mac_size, const void* mac, uint16_t original_id,
+         uint16_t error, uint16_t other_len, const void* other_data);
+
+    /// \brief Assignment operator.
+    ///
+    /// It internally allocates a resource, and if it fails a corresponding
+    /// standard exception will be thrown.
+    /// This operator never throws an exception otherwise.
+    ///
+    /// This operator provides the strong exception guarantee: When an
+    /// exception is thrown the content of the assignment target will be
+    /// intact.
+    TSIG& operator=(const TSIG& source);
+
+    /// \brief The destructor.
+    ~TSIG();
+
+    /// \brief Return the algorithm name.
+    ///
+    /// This method never throws an exception.
+    const Name& getAlgorithm() const;
+
+    /// \brief Return the value of the Time Signed field.
+    ///
+    /// The returned value does not exceed 2^48-1.
+    ///
+    /// This method never throws an exception.
+    uint64_t getTimeSigned() const;
+
+    /// \brief Return the value of the Fudge field.
+    ///
+    /// This method never throws an exception.
+    uint16_t getFudge() const;
+
+    /// \brief Return the value of the MAC Size field.
+    ///
+    /// This method never throws an exception.
+    uint16_t getMACSize() const;
+
+    /// \brief Return the value of the MAC field.
+    ///
+    /// If the MAC field is empty, it returns NULL.
+    /// Otherwise, the memory region beginning at the address returned by
+    /// this method is valid up to the bytes specified by the return value
+    /// of \c getMACSize().
+    /// The memory region is only valid while the corresponding \c TSIG
+    /// object is valid.  The caller must hold the \c TSIG object while
+    /// it needs to refer to the region or it must make a local copy of the
+    /// region.
+    ///
+    /// This method never throws an exception.
+    const void* getMAC() const;
+
+    /// \brief Return the value of the Original ID field.
+    ///
+    /// This method never throws an exception.
+    uint16_t getOriginalID() const;
+
+    /// \brief Return the value of the Error field.
+    ///
+    /// This method never throws an exception.
+    uint16_t getError() const;
+
+    /// \brief Return the value of the Other Len field.
+    ///
+    /// This method never throws an exception.
+    uint16_t getOtherLen() const;
+
+    /// \brief Return the value of the Other Data field.
+    ///
+    /// The same note as \c getMAC() applies.
+    ///
+    /// This method never throws an exception.
+    const void* getOtherData() const;
+private:
+    struct TSIGImpl;
+    TSIGImpl* impl_;
+};
+
+// END_RDATA_NAMESPACE
+// END_ISC_NAMESPACE
+// END_HEADER_GUARD
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/rrclass-placeholder.h

@@ -244,14 +244,12 @@ public:
     // END_WELL_KNOWN_CLASS_DECLARATIONS
     
     static const RRClass& NONE();
-    static const RRClass& ANY();
 
 private:
     // \brief Meta-classes
     enum {
         RRCLASS_RESERVED0 = 0,
-        RRCLASS_NONE = 254,
-        RRCLASS_ANY = 255
+        RRCLASS_NONE = 254
     };
     uint16_t classcode_;
 };
@@ -266,13 +264,6 @@ RRClass::NONE() {
     return (rrclass);
 }
 
-inline const RRClass&
-RRClass::ANY() {
-    static RRClass rrclass(RRCLASS_ANY);
-
-    return (rrclass);
-}
-
 ///
 /// \brief Insert the \c RRClass as a string into stream.
 ///

src/lib/dns/rrset.cc

@@ -44,7 +44,6 @@ AbstractRRset::toText() const {
     string s;
     RdataIteratorPtr it = getRdataIterator();
 
-    it->first();
     if (it->isLast()) {
         isc_throw(EmptyRRset, "ToText() is attempted for an empty RRset");
     }
@@ -66,7 +65,6 @@ rrsetToWire(const AbstractRRset& rrset, T& output, const size_t limit) {
     unsigned int n = 0;
     RdataIteratorPtr it = rrset.getRdataIterator();
 
-    it->first();
     if (it->isLast()) {
         isc_throw(EmptyRRset, "ToWire() is attempted for an empty RRset");
     }
@@ -224,7 +222,8 @@ private:
     BasicRdataIterator() {}
 public:
     BasicRdataIterator(const std::vector<rdata::ConstRdataPtr>& datavector) :
-        datavector_(&datavector) {}
+        datavector_(&datavector), it_(datavector_->begin())
+    {}
     ~BasicRdataIterator() {}
     virtual void first() { it_ = datavector_->begin(); }
     virtual void next() { ++it_; }

src/lib/dns/rrset.h

@@ -148,7 +148,7 @@ typedef boost::shared_ptr<RdataIterator> RdataIteratorPtr;
 ///      "sort" and "search(find)" method?
 ///   - what about comparing two RRsets of the same type?  If we need this,
 ///     should it compare rdata's as a set or as a list (i.e. compare
-///     each %rdata one by one or as a whole)?  c.f. NLnet Labs' ldns
+///     each rdata one by one or as a whole)?  c.f. NLnet Labs' ldns
 ///     (http://www.nlnetlabs.nl/projects/ldns/doc/index.html)
 ///     has \c ldns_rr_list_compare(), which takes the latter approach
 ///     (seemingly assuming the caller sorts the lists beforehand).
@@ -357,7 +357,7 @@ public:
     /// \endcode
     ///
     /// This method is more strictly typed than the pointer version:
-    /// If \c %rdata does not refer to the appropriate derived
+    /// If \c rdata does not refer to the appropriate derived
     /// \c Rdata class
     /// for the \c RRType for this \c RRset, it throws an exception of class
     /// \c std::bad_cast.
@@ -385,6 +385,10 @@ public:
     /// \brief Return an iterator to go through all RDATA stored in the
     /// \c RRset.
     ///
+    /// The rdata cursor of the returned iterator will point to the first
+    /// RDATA, that is, it effectively calls \c RdataIterator::first()
+    /// internally.
+    ///
     /// Using the design pattern terminology, \c getRdataIterator()
     /// is an example of a <em>factory method</em>.
     ///
@@ -417,10 +421,10 @@ public:
 /// The RDATA objects stored in the \c RRset are considered to form
 /// a unidirectional list from the \c RdataIterator point of view (while
 /// the actual implementation in the derived \c RRset may not use a list).
-/// We call this unidirectional list the <em>%rdata list</em>.
+/// We call this unidirectional list the <em>rdata list</em>.
 ///
 /// An \c RdataIterator object internally (and conceptually) holds a
-/// <em>%rdata cursor</em>, which points to a specific item of the %rdata list.
+/// <em>rdata cursor</em>, which points to a specific item of the rdata list.
 ///
 /// Note about design choice: as is clear from the interface, \c RdataIterator
 /// is not compatible with the standard iterator classes.
@@ -458,29 +462,29 @@ private:
     //@}
 
 public:
-    /// \brief Move the %rdata cursor to the first RDATA in the %rdata list
+    /// \brief Move the rdata cursor to the first RDATA in the rdata list
     /// (if any).
     ///
     /// This method can safely be called multiple times, even after moving
-    /// the %rdata cursor forward by the \c next() method.
+    /// the rdata cursor forward by the \c next() method.
     ///
     /// This method should never throw an exception.
     virtual void first() = 0;
 
-    /// \brief Move the %rdata cursor to the next RDATA in the %rdata list
+    /// \brief Move the rdata cursor to the next RDATA in the rdata list
     /// (if any).
     ///
     /// This method should never throw an exception.
     virtual void next() = 0;
 
-    /// \brief Return the current \c Rdata corresponding to the %rdata cursor.
+    /// \brief Return the current \c Rdata corresponding to the rdata cursor.
     ///
     /// \return A reference to an \c rdata::::Rdata object corresponding
-    /// to the %rdata cursor.
+    /// to the rdata cursor.
     virtual const rdata::Rdata& getCurrent() const = 0;
 
-    /// \brief Return true iff the %rdata cursor has reached the end of the
-    /// %rdata list.
+    /// \brief Return true iff the rdata cursor has reached the end of the
+    /// rdata list.
     ///
     /// Once this method returns \c true, the behavior of any subsequent
     /// call to \c next() or \c getCurrent() is undefined.
@@ -489,8 +493,8 @@ public:
     ///
     /// This method should never throw an exception.
     ///
-    /// \return \c true if the %rdata cursor has reached the end of the
-    /// %rdata list; otherwise \c false.
+    /// \return \c true if the rdata cursor has reached the end of the
+    /// rdata list; otherwise \c false.
     virtual bool isLast() const = 0;
 };
 

src/lib/dns/tests/Makefile.am

@@ -38,6 +38,7 @@ run_unittests_SOURCES += rdata_nsec_unittest.cc
 run_unittests_SOURCES += rdata_nsec3_unittest.cc
 run_unittests_SOURCES += rdata_nsec3param_unittest.cc
 run_unittests_SOURCES += rdata_rrsig_unittest.cc
+run_unittests_SOURCES += rdata_tsig_unittest.cc
 run_unittests_SOURCES += rrset_unittest.cc rrsetlist_unittest.cc
 run_unittests_SOURCES += question_unittest.cc
 run_unittests_SOURCES += rrparamregistry_unittest.cc
@@ -46,7 +47,7 @@ run_unittests_SOURCES += base32hex_unittest.cc
 run_unittests_SOURCES += base64_unittest.cc
 run_unittests_SOURCES += hex_unittest.cc
 run_unittests_SOURCES += sha1_unittest.cc
-run_unittests_SOURCES += tsig_unittest.cc
+run_unittests_SOURCES += tsigkey_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

src/lib/dns/tests/message_unittest.cc

@@ -293,7 +293,6 @@ TEST_F(MessageTest, fromWire) {
     // TTL should be 3600, even though that of the 2nd RR is 7200
     EXPECT_EQ(RRTTL(3600), rrset->getTTL());
     RdataIteratorPtr it = rrset->getRdataIterator();
-    it->first();
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
     it->next();
     EXPECT_EQ("192.0.2.2", it->getCurrent().toText());

src/lib/dns/tests/rdata_tsig_unittest.cc

@@ -0,0 +1,368 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#include <string>
+
+#include <exceptions/exceptions.h>
+
+#include <dns/buffer.h>
+#include <dns/exceptions.h>
+#include <dns/messagerenderer.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+
+#include <gtest/gtest.h>
+
+#include <dns/tests/unittest_util.h>
+#include <dns/tests/rdata_unittest.h>
+
+using isc::UnitTestUtil;
+using namespace std;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+namespace {
+class Rdata_TSIG_Test : public RdataTest {
+protected:
+    vector<uint8_t> expect_data;
+};
+
+const char* const valid_text1 = "hmac-md5.sig-alg.reg.int. 1286779327 300 "
+    "0 16020 BADKEY 0";
+const char* const valid_text2 = "hmac-sha256. 1286779327 300 12 "
+    "FAKEFAKEFAKEFAKE 16020 BADSIG 0";
+
+const char* const valid_text3 = "hmac-sha1. 1286779327 300 12 "
+    "FAKEFAKEFAKEFAKE 16020 BADTIME 6 FAKEFAKE";
+const char* const valid_text4 = "hmac-sha1. 1286779327 300 12 "
+    "FAKEFAKEFAKEFAKE 16020 BADSIG 6 FAKEFAKE";
+const char* const valid_text5 = "hmac-sha256. 1286779327 300 12 "
+    "FAKEFAKEFAKEFAKE 16020 2845 0"; // using numeric error code
+const char* const too_long_label = "012345678901234567890123456789"
+    "0123456789012345678901234567890123";
+
+// commonly used test RDATA
+const any::TSIG rdata_tsig((string(valid_text1)));
+
+TEST_F(Rdata_TSIG_Test, createFromText) {
+    // normal case.  it also tests getter methods.
+    EXPECT_EQ(Name("hmac-md5.sig-alg.reg.int"), rdata_tsig.getAlgorithm());
+    EXPECT_EQ(1286779327, rdata_tsig.getTimeSigned());
+    EXPECT_EQ(300, rdata_tsig.getFudge());
+    EXPECT_EQ(0, rdata_tsig.getMACSize());
+    EXPECT_EQ(static_cast<void*>(NULL), rdata_tsig.getMAC());
+    EXPECT_EQ(16020, rdata_tsig.getOriginalID());
+    EXPECT_EQ(17, rdata_tsig.getError()); // TODO: use constant
+    EXPECT_EQ(0, rdata_tsig.getOtherLen());
+    EXPECT_EQ(static_cast<void*>(NULL), rdata_tsig.getOtherData());
+
+    any::TSIG tsig2((string(valid_text2)));
+    EXPECT_EQ(12, tsig2.getMACSize());
+    EXPECT_EQ(16, tsig2.getError()); // TODO: use constant
+
+    any::TSIG tsig3((string(valid_text3)));
+    EXPECT_EQ(6, tsig3.getOtherLen());
+
+    // The other data is unusual, but we don't reject it.
+    EXPECT_NO_THROW(any::TSIG(string(valid_text4)));
+
+    // numeric representation of TSIG error
+    any::TSIG tsig5((string(valid_text5)));
+    EXPECT_EQ(2845, tsig5.getError());
+
+    //
+    // invalid cases
+    //
+    // there's a garbage parameter at the end
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 BADKEY 0 0"), InvalidRdataText);
+    // input is too short
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 BADKEY"), InvalidRdataText);
+    // bad domain name
+    EXPECT_THROW(any::TSIG(string(too_long_label) + "0 0 0 0 BADKEY 0"),
+                 TooLongLabel);
+    // time is too large (2814...6 is 2^48)
+    EXPECT_THROW(any::TSIG("foo 281474976710656 0 0 0 BADKEY 0"),
+                 InvalidRdataText);
+    // invalid time (negative)
+    EXPECT_THROW(any::TSIG("foo -1 0 0 0 BADKEY 0"), InvalidRdataText);
+    // fudge is too large
+    EXPECT_THROW(any::TSIG("foo 0 65536 0 0 BADKEY 0"), InvalidRdataText);
+    // invalid fudge (negative)
+    EXPECT_THROW(any::TSIG("foo 0 -1 0 0 BADKEY 0"), InvalidRdataText);
+    // MAC size is too large
+    EXPECT_THROW(any::TSIG("foo 0 0 65536 0 BADKEY 0"), InvalidRdataText);
+    // MAC size and MAC mismatch
+    EXPECT_THROW(any::TSIG("foo 0 0 9 FAKE 0 BADKEY 0"), InvalidRdataText);
+    EXPECT_THROW(any::TSIG("foo 0 0 0 FAKE 0 BADKEY 0"), InvalidRdataText);
+    // MAC is bad base64
+    EXPECT_THROW(any::TSIG("foo 0 0 3 FAK= 0 BADKEY 0"), isc::BadValue);
+    // Unknown error code
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 TEST 0"), InvalidRdataText);
+    // Numeric error code is too large
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 65536 0"), InvalidRdataText);
+    // Other len is too large
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 NOERROR 65536 FAKE"), InvalidRdataText);
+    // Other len and data mismatch
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 NOERROR 9 FAKE"), InvalidRdataText);
+    EXPECT_THROW(any::TSIG("foo 0 0 0 0 NOERROR 0 FAKE"), InvalidRdataText);
+}
+
+void
+fromWireCommonChecks(const any::TSIG& tsig) {
+    EXPECT_EQ(Name("hmac-sha256"), tsig.getAlgorithm());
+    EXPECT_EQ(1286978795, tsig.getTimeSigned());
+    EXPECT_EQ(300, tsig.getFudge());
+
+    vector<uint8_t> expect_mac(32, 'x');
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_mac[0], expect_mac.size(),
+                        tsig.getMAC(), tsig.getMACSize());
+
+    EXPECT_EQ(2845, tsig.getOriginalID());
+
+    EXPECT_EQ(0, tsig.getOtherLen());
+    EXPECT_EQ(static_cast<const void*>(NULL), tsig.getOtherData());
+}
+
+TEST_F(Rdata_TSIG_Test, createFromWire) {
+    RdataPtr rdata(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                        "rdata_tsig_fromWire1.wire"));
+    fromWireCommonChecks(dynamic_cast<any::TSIG&>(*rdata));
+}
+
+TEST_F(Rdata_TSIG_Test, createFromWireWithOtherData) {
+    RdataPtr rdata(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                        "rdata_tsig_fromWire2.wire"));
+    const any::TSIG& tsig(dynamic_cast<any::TSIG&>(*rdata));
+
+    EXPECT_EQ(18, tsig.getError());
+    const uint64_t otherdata = 1286978795 + 300 + 1; // time-signed + fudge + 1
+    expect_data.resize(6);
+    expect_data[0] = (otherdata >> 40);
+    expect_data[1] = ((otherdata >> 32) & 0xff);
+    expect_data[2] = ((otherdata >> 24) & 0xff);
+    expect_data[3] = ((otherdata >> 16) & 0xff);
+    expect_data[4] = ((otherdata >> 8) & 0xff);
+    expect_data[5] = (otherdata & 0xff);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_data[0], expect_data.size(),
+                        tsig.getOtherData(), tsig.getOtherLen());
+}
+
+TEST_F(Rdata_TSIG_Test, createFromWireWithoutMAC) {
+    RdataPtr rdata(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                        "rdata_tsig_fromWire3.wire"));
+    const any::TSIG& tsig(dynamic_cast<any::TSIG&>(*rdata));
+    EXPECT_EQ(16, tsig.getError());
+    EXPECT_EQ(0, tsig.getMACSize());
+    EXPECT_EQ(static_cast<const void*>(NULL), tsig.getMAC());
+}
+
+TEST_F(Rdata_TSIG_Test, createFromWireWithCompression) {
+    RdataPtr rdata(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                        "rdata_tsig_fromWire4.wire",
+                                        // we need to skip the dummy name:
+                                        Name("hmac-sha256").getLength()));
+    fromWireCommonChecks(dynamic_cast<any::TSIG&>(*rdata));
+}
+
+TEST_F(Rdata_TSIG_Test, badFromWire) {
+    // RDLENGTH is too short:
+    EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                      "rdata_tsig_fromWire5.wire"),
+                 InvalidRdataLength);
+    // RDLENGTH is too long:
+    EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                      "rdata_tsig_fromWire6.wire"),
+                 InvalidRdataLength);
+    // Algorithm name is broken:
+    EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                      "rdata_tsig_fromWire7.wire"),
+                 DNSMessageFORMERR);
+    // MAC size is bogus:
+    EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                      "rdata_tsig_fromWire8.wire"),
+                 InvalidBufferPosition);
+    // Other-data length is bogus:
+    EXPECT_THROW(rdataFactoryFromFile(RRType::TSIG(), RRClass::ANY(),
+                                      "rdata_tsig_fromWire9.wire"),
+                 InvalidBufferPosition);
+}
+
+TEST_F(Rdata_TSIG_Test, copyConstruct) {
+    const any::TSIG copy(rdata_tsig);
+    EXPECT_EQ(0, copy.compare(rdata_tsig));
+
+    // Check the copied data is valid even after the original is deleted
+    any::TSIG* copy2 = new any::TSIG(rdata_tsig);
+    any::TSIG copy3(*copy2);
+    delete copy2;
+    EXPECT_EQ(0, copy3.compare(rdata_tsig));
+}
+
+TEST_F(Rdata_TSIG_Test, createFromParams) {
+    EXPECT_EQ(0, rdata_tsig.compare(any::TSIG(Name("hmac-md5.sig-alg.reg.int"),
+                                              1286779327, 300, 0, NULL, 16020,
+                                              17, 0, NULL)));
+
+    const uint8_t fake_data[] = { 0x14, 0x02, 0x84, 0x14, 0x02, 0x84,
+                                  0x14, 0x02, 0x84, 0x14, 0x02, 0x84 }; 
+    EXPECT_EQ(0, any::TSIG((string(valid_text2))).compare(
+                  any::TSIG(Name("hmac-sha256"), 1286779327, 300, 12,
+                            fake_data, 16020, 16, 0, NULL)));
+
+    const uint8_t fake_data2[] = { 0x14, 0x02, 0x84, 0x14, 0x02, 0x84 };
+    EXPECT_EQ(0, any::TSIG((string(valid_text3))).compare(
+                  any::TSIG(Name("hmac-sha1"), 1286779327, 300, 12,
+                            fake_data, 16020, 18, 6, fake_data2)));
+
+    EXPECT_THROW(any::TSIG(Name("hmac-sha256"), 1LLU << 48, 300, 12,
+                           fake_data, 16020, 18, 6, fake_data2),
+                 isc::OutOfRange);
+    EXPECT_THROW(any::TSIG(Name("hmac-sha256"), 0, 300, 0, fake_data, 16020,
+                           18, 0, NULL),
+                 isc::InvalidParameter);
+    EXPECT_THROW(any::TSIG(Name("hmac-sha256"), 0, 300, 12, NULL, 16020,
+                           18, 0, NULL),
+                 isc::InvalidParameter);
+    EXPECT_THROW(any::TSIG(Name("hmac-sha256"), 0, 300, 0, NULL, 16020,
+                           18, 0, fake_data),
+                 isc::InvalidParameter);
+    EXPECT_THROW(any::TSIG(Name("hmac-sha256"), 0, 300, 0, NULL, 16020,
+                           18, 6, NULL),
+                 isc::InvalidParameter);
+}
+
+TEST_F(Rdata_TSIG_Test, assignment) {
+    any::TSIG copy((string(valid_text2)));
+    copy = rdata_tsig;
+    EXPECT_EQ(0, copy.compare(rdata_tsig));
+
+    // Check if the copied data is valid even after the original is deleted
+    any::TSIG* copy2 = new any::TSIG(rdata_tsig);
+    any::TSIG copy3((string(valid_text2)));
+    copy3 = *copy2;
+    delete copy2;
+    EXPECT_EQ(0, copy3.compare(rdata_tsig));
+
+    // Self assignment
+    copy = copy;
+    EXPECT_EQ(0, copy.compare(rdata_tsig));
+}
+
+template <typename Output>
+void
+toWireCommonChecks(Output& output) {
+    vector<uint8_t> expect_data;
+
+    output.clear();
+    expect_data.clear();
+    rdata_tsig.toWire(output);
+    // read the expected wire format data and trim the RDLEN part.
+    UnitTestUtil::readWireData("rdata_tsig_toWire1.wire", expect_data);
+    expect_data.erase(expect_data.begin(), expect_data.begin() + 2);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_data[0], expect_data.size(),
+                        output.getData(), output.getLength());
+
+    expect_data.clear();
+    output.clear();
+    any::TSIG(string(valid_text2)).toWire(output);
+    UnitTestUtil::readWireData("rdata_tsig_toWire2.wire", expect_data);
+    expect_data.erase(expect_data.begin(), expect_data.begin() + 2);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_data[0], expect_data.size(),
+                        output.getData(), output.getLength());
+
+    expect_data.clear();
+    output.clear();
+    any::TSIG(string(valid_text3)).toWire(output);
+    UnitTestUtil::readWireData("rdata_tsig_toWire3.wire", expect_data);
+    expect_data.erase(expect_data.begin(), expect_data.begin() + 2);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_data[0], expect_data.size(),
+                        output.getData(), output.getLength());
+}
+
+TEST_F(Rdata_TSIG_Test, toWireBuffer) {
+    toWireCommonChecks<OutputBuffer>(obuffer);
+}
+
+TEST_F(Rdata_TSIG_Test, toWireRenderer) {
+    toWireCommonChecks<MessageRenderer>(renderer);
+
+    // check algorithm name won't compressed when it would otherwise.
+    expect_data.clear();
+    renderer.clear();
+    renderer.writeName(Name("hmac-md5.sig-alg.reg.int"));
+    renderer.writeUint16(42); // RDLEN
+    rdata_tsig.toWire(renderer);
+    UnitTestUtil::readWireData("rdata_tsig_toWire4.wire", expect_data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_data[0], expect_data.size(),
+                        renderer.getData(), renderer.getLength());
+
+    // check algorithm can be used as a compression target.
+    expect_data.clear();
+    renderer.clear();
+    renderer.writeUint16(42);
+    rdata_tsig.toWire(renderer);
+    renderer.writeName(Name("hmac-md5.sig-alg.reg.int"));
+    UnitTestUtil::readWireData("rdata_tsig_toWire5.wire", expect_data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        &expect_data[0], expect_data.size(),
+                        renderer.getData(), renderer.getLength());
+}
+
+TEST_F(Rdata_TSIG_Test, toText) {
+    EXPECT_EQ(string(valid_text1), rdata_tsig.toText());
+    EXPECT_EQ(string(valid_text2), any::TSIG(string(valid_text2)).toText());
+    EXPECT_EQ(string(valid_text3), any::TSIG(string(valid_text3)).toText());
+    EXPECT_EQ(string(valid_text5), any::TSIG(string(valid_text5)).toText());
+}
+
+TEST_F(Rdata_TSIG_Test, compare) {
+    // test RDATAs, sorted in the ascendent order.
+    // "AAAA" encoded in BASE64 corresponds to 0x000000, so it should be the
+    // smallest data of the same length.
+    vector<any::TSIG> compare_set;
+    compare_set.push_back(any::TSIG("a.example 0 300 0 16020 0 0"));
+    compare_set.push_back(any::TSIG("example 0 300 0 16020 0 0"));
+    compare_set.push_back(any::TSIG("example 1 300 0 16020 0 0"));
+    compare_set.push_back(any::TSIG("example 1 600 0 16020 0 0"));
+    compare_set.push_back(any::TSIG("example 1 600 3 AAAA 16020 0 0"));
+    compare_set.push_back(any::TSIG("example 1 600 3 FAKE 16020 0 0"));
+    compare_set.push_back(any::TSIG("example 1 600 3 FAKE 16021 0 0"));
+    compare_set.push_back(any::TSIG("example 1 600 3 FAKE 16021 1 0"));
+    compare_set.push_back(any::TSIG("example 1 600 3 FAKE 16021 1 3 AAAA"));
+    compare_set.push_back(any::TSIG("example 1 600 3 FAKE 16021 1 3 FAKE"));
+
+    EXPECT_EQ(0, compare_set[0].compare(
+                  any::TSIG("A.EXAMPLE 0 300 0 16020 0 0")));
+
+    vector<any::TSIG>::const_iterator it;
+    vector<any::TSIG>::const_iterator it_end = compare_set.end();
+    for (it = compare_set.begin(); it != it_end - 1; ++it) {
+        EXPECT_GT(0, (*it).compare(*(it + 1)));
+        EXPECT_LT(0, (*(it + 1)).compare(*it));
+    }
+
+    // comparison attempt between incompatible RR types should be rejected
+    EXPECT_THROW(rdata_tsig.compare(*RdataTest::rdata_nomatch), bad_cast);
+}
+}

src/lib/dns/tests/rrset_unittest.cc

@@ -117,8 +117,7 @@ void
 addRdataTestCommon(const RRset& rrset) {
     EXPECT_EQ(2, rrset.getRdataCount());
 
-    RdataIteratorPtr it = rrset.getRdataIterator();
-    it->first();
+    RdataIteratorPtr it = rrset.getRdataIterator(); // cursor is set to the 1st
     EXPECT_FALSE(it->isLast());
     EXPECT_EQ(0, it->getCurrent().compare(in::A("192.0.2.1")));
     it->next();
@@ -156,7 +155,6 @@ TEST_F(RRsetTest, addRdataPtr) {
 TEST_F(RRsetTest, iterator) {
     // Iterator for an empty RRset.
     RdataIteratorPtr it = rrset_a_empty.getRdataIterator();
-    it->first();
     EXPECT_TRUE(it->isLast());
 
     // Normal case (already tested, but do it again just in case)

src/lib/dns/tests/rrsetlist_unittest.cc

@@ -150,7 +150,6 @@ TEST_F(RRsetListTest, checkData) {
 
     RdataIteratorPtr it =
         list.findRRset(RRType::A(), RRClass::IN())->getRdataIterator();
-    it->first();
     EXPECT_FALSE(it->isLast());
     EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
 }

src/lib/dns/tests/testdata/Makefile.am

@@ -12,6 +12,14 @@ BUILT_SOURCES += rdata_rrsig_fromWire2.wire
 BUILT_SOURCES += rdata_soa_toWireUncompressed.wire
 BUILT_SOURCES +=  rdata_txt_fromWire2.wire rdata_txt_fromWire3.wire
 BUILT_SOURCES += rdata_txt_fromWire4.wire rdata_txt_fromWire5.wire
+BUILT_SOURCES += rdata_tsig_fromWire1.wire rdata_tsig_fromWire2.wire
+BUILT_SOURCES += rdata_tsig_fromWire3.wire rdata_tsig_fromWire4.wire
+BUILT_SOURCES += rdata_tsig_fromWire5.wire rdata_tsig_fromWire6.wire
+BUILT_SOURCES += rdata_tsig_fromWire7.wire rdata_tsig_fromWire8.wire
+BUILT_SOURCES += rdata_tsig_fromWire9.wire
+BUILT_SOURCES += rdata_tsig_toWire1.wire rdata_tsig_toWire2.wire
+BUILT_SOURCES += rdata_tsig_toWire3.wire rdata_tsig_toWire4.wire
+BUILT_SOURCES += rdata_tsig_toWire5.wire
 
 # NOTE: keep this in sync with real file listing
 # so is included in tarball
@@ -51,6 +59,14 @@ EXTRA_DIST += rdata_txt_fromWire5.spec rdata_unknown_fromWire
 EXTRA_DIST += rrcode16_fromWire1 rrcode16_fromWire2
 EXTRA_DIST += rrcode32_fromWire1 rrcode32_fromWire2
 EXTRA_DIST += rrset_toWire1 rrset_toWire2
+EXTRA_DIST += rdata_tsig_fromWire1.spec rdata_tsig_fromWire2.spec
+EXTRA_DIST += rdata_tsig_fromWire3.spec rdata_tsig_fromWire4.spec
+EXTRA_DIST += rdata_tsig_fromWire5.spec rdata_tsig_fromWire6.spec
+EXTRA_DIST += rdata_tsig_fromWire7.spec rdata_tsig_fromWire8.spec
+EXTRA_DIST += rdata_tsig_fromWire9.spec
+EXTRA_DIST += rdata_tsig_toWire1.spec rdata_tsig_toWire2.spec
+EXTRA_DIST += rdata_tsig_toWire3.spec rdata_tsig_toWire4.spec
+EXTRA_DIST += rdata_tsig_toWire5.spec
 
 .spec.wire:
 	./gen-wiredata.py -o $@ $<

src/lib/dns/tests/testdata/edns_toWire4.spec

@@ -1,5 +1,6 @@
 #
-# Same as edns_toWire1 but setting the DO bit
+# Same as edns_toWire1 but setting the DO bit, and using an unusual
+# UDP payload size
 #
 [edns]
 do: 1

src/lib/dns/tests/testdata/gen-wiredata.py.in

@@ -19,8 +19,8 @@ import configparser, re, time, sys
 from datetime import datetime
 from optparse import OptionParser
 
-re_hex = re.compile(r'0x[0-9a-fA-F]+')
-re_decimal = re.compile(r'\d+$')
+re_hex = re.compile(r'^0x[0-9a-fA-F]+')
+re_decimal = re.compile(r'^\d+$')
 re_string = re.compile(r"\'(.*)\'$")
 
 dnssec_timefmt = '%Y%m%d%H%M%S'
@@ -48,9 +48,12 @@ dict_rrtype = { 'none' : 0, 'a' : 1, 'ns' : 2, 'md' : 3, 'mf' : 4, 'cname' : 5,
                 'maila' : 254, 'any' : 255 }
 rdict_rrtype = dict([(dict_rrtype[k], k.upper()) for k in dict_rrtype.keys()])
 dict_rrclass = { 'in' : 1, 'ch' : 3, 'hs' : 4, 'any' : 255 }
-rdict_rrclass = dict([(dict_rrclass[k], k.upper()) for k in dict_rrclass.keys()])
-dict_algorithm = { 'rsamd5' : 1, 'dh' : 2, 'dsa' : 3, 'ecc' : 4, 'rsasha1' : 5 }
-rdict_algorithm = dict([(dict_algorithm[k], k.upper()) for k in dict_algorithm.keys()])
+rdict_rrclass = dict([(dict_rrclass[k], k.upper()) for k in \
+                          dict_rrclass.keys()])
+dict_algorithm = { 'rsamd5' : 1, 'dh' : 2, 'dsa' : 3, 'ecc' : 4,
+                   'rsasha1' : 5 }
+rdict_algorithm = dict([(dict_algorithm[k], k.upper()) for k in \
+                            dict_algorithm.keys()])
 
 header_xtables = { 'qr' : dict_qr, 'opcode' : dict_opcode,
                    'rcode' : dict_rcode }
@@ -75,13 +78,17 @@ def code_totext(code, dict):
         return dict[code] + '(' + str(code) + ')'
     return str(code)
 
-def encode_name(name, absolute = True):
+def encode_name(name, absolute=True):
     # make sure the name is dot-terminated.  duplicate dots will be ignored
     # below.
     name += '.'
     labels = name.split('.')
     wire = ''
     for l in labels:
+        if len(l) > 4 and l[0:4] == 'ptr=':
+            # special meta-syntax for compression pointer
+            wire += ' %04x' % (0xc000 | int(l[4:]))
+            break
         if absolute or len(l) > 0:
             wire += '%02x' % len(l)
             wire += ''.join(['%02x' % ord(ch) for ch in l])
@@ -89,7 +96,9 @@ def encode_name(name, absolute = True):
             break
     return wire
 
-def encode_string(name):
+def encode_string(name, len=None):
+    if type(name) is int and len is not None:
+        return '%0.*x' % (len * 2, name)
     return ''.join(['%02x' % ord(ch) for ch in name])
 
 def count_namelabels(name):
@@ -121,17 +130,19 @@ def print_header(f, input_file):
 
 class Name:
     name = 'example.com'
-    pointer = -1                # no compression by default
+    pointer = None                # no compression by default
     def dump(self, f):
-        name_wire = encode_name(self.name,
-                                True if self.pointer == -1 else False)
+        name = self.name
+        if self.pointer is not None:
+            if len(name) > 0 and name[-1] != '.':
+                name += '.'
+            name += 'ptr=%d' % self.pointer
+        name_wire = encode_name(name)
         f.write('\n# DNS Name: %s' % self.name)
-        if self.pointer >= 0:
+        if self.pointer is not None:
             f.write(' + compression pointer: %d' % self.pointer)
         f.write('\n')
         f.write('%s' % name_wire)
-        if self.pointer >= 0:
-            f.write(' %04x' % (0xc000 | self.pointer))
         f.write('\n')
 
 class DNSHeader:
@@ -338,20 +349,73 @@ class RRSIG:
                 (code_totext(self.covered, rdict_rrtype),
                  code_totext(self.algorithm, rdict_algorithm), labels,
                  self.originalttl))
-        f.write('%04x %02x %02x %08x\n' % (self.covered, self.algorithm, labels,
-                                           self.originalttl))
+        f.write('%04x %02x %02x %08x\n' % (self.covered, self.algorithm,
+                                           labels, self.originalttl))
         f.write('# Expiration=%s, Inception=%s\n' %
                 (str(self.expiration), str(self.inception)))
         f.write('%08x %08x\n' % (self.expiration, self.inception))
         f.write('# Tag=%d Signer=%s and Signature\n' % (self.tag, self.signer))
         f.write('%04x %s %s\n' % (self.tag, name_wire, sig_wire))
 
+class TSIG:
+    rdlen = None                # auto-calculate
+    algorithm = 'hmac-sha256'
+    time_signed = 1286978795    # arbitrarily chosen default
+    fudge = 300
+    mac_size = None             # use a common value for the algorithm
+    mac = None                  # use 'x' * mac_size
+    original_id = 2845          # arbitrarily chosen default
+    error = 0
+    other_len = None         # 6 if error is BADTIME; otherwise 0
+    other_data = None        # use time_signed + fudge + 1 for BADTIME
+    dict_macsize = { 'hmac-md5' : 16, 'hmac-sha1' : 20, 'hmac-sha256' : 32 }
+    def dump(self, f):
+        if str(self.algorithm) == 'hmac-md5':
+            name_wire = encode_name('hmac-md5.sig-alg.reg.int')
+        else:
+            name_wire = encode_name(self.algorithm)
+        rdlen = self.rdlen
+        mac_size = self.mac_size
+        if mac_size is None:
+            if self.algorithm in self.dict_macsize.keys():
+                mac_size = self.dict_macsize[self.algorithm]
+            else:
+                raise RuntimeError('TSIG Mac Size cannot be determined')
+        mac = encode_string('x' * mac_size) if self.mac is None else \
+            encode_string(self.mac, mac_size)
+        other_len = self.other_len
+        if other_len is None:
+            # 18 = BADTIME
+            other_len = 6 if self.error == 18 else 0
+        other_data = self.other_data
+        if other_data is None:
+            other_data = '%012x' % (self.time_signed + self.fudge + 1) \
+                if self.error == 18 else ''
+        else:
+            other_data = encode_string(self.other_data, other_len)
+        if rdlen is None:
+            rdlen = int(len(name_wire) / 2 + 16 + len(mac) / 2 + \
+                            len(other_data) / 2)
+        f.write('\n# TSIG RDATA (RDLEN=%d)\n' % rdlen)
+        f.write('%04x\n' % rdlen);
+        f.write('# Algorithm=%s Time-Signed=%d Fudge=%d\n' %
+                (self.algorithm, self.time_signed, self.fudge))
+        f.write('%s %012x %04x\n' % (name_wire, self.time_signed, self.fudge))
+        f.write('# MAC Size=%d MAC=(see hex)\n' % mac_size)
+        f.write('%04x%s\n' % (mac_size, ' ' + mac if len(mac) > 0 else ''))
+        f.write('# Original-ID=%d Error=%d\n' % (self.original_id, self.error))
+        f.write('%04x %04x\n' %  (self.original_id, self.error))
+        f.write('# Other-Len=%d Other-Data=(see hex)\n' % other_len)
+        f.write('%04x%s\n' % (other_len,
+                              ' ' + other_data if len(other_data) > 0 else ''))
+
 def get_config_param(section):
     config_param = {'name' : (Name, {}),
                     'header' : (DNSHeader, header_xtables),
                     'question' : (DNSQuestion, question_xtables),
                     'edns' : (EDNS, {}), 'soa' : (SOA, {}), 'txt' : (TXT, {}),
-                    'rrsig' : (RRSIG, {}), 'nsec' : (NSEC, {})}
+                    'rrsig' : (RRSIG, {}), 'nsec' : (NSEC, {}),
+                    'tsig' : (TSIG, {}) }
     s = section
     m = re.match('^([^:]+)/\d+$', section)
     if m:

src/lib/dns/tests/testdata/rdata_tsig_fromWire1.spec

@@ -0,0 +1,6 @@
+#
+# A simplest form of TSIG: all default parameters
+#
+[custom]
+sections: tsig
+[tsig]

src/lib/dns/tests/testdata/rdata_tsig_fromWire2.spec

@@ -0,0 +1,8 @@
+#
+# TSIG with other data (error = BADTIME(18))
+#
+[custom]
+sections: tsig
+[tsig]
+mac_size: 0
+error: 18

src/lib/dns/tests/testdata/rdata_tsig_fromWire3.spec

@@ -0,0 +1,8 @@
+#
+# TSIG without MAC (error = BADSIG(16))
+#
+[custom]
+sections: tsig
+[tsig]
+mac_size: 0
+error: 16

src/lib/dns/tests/testdata/rdata_tsig_fromWire4.spec

@@ -0,0 +1,11 @@
+#
+# A simplest form of TSIG, but the algorithm name is compressed (quite
+# pathological, but we accept it)
+#
+[custom]
+sections: name:tsig
+[name]
+name: hmac-sha256
+[tsig]
+algorithm: ptr=0
+mac_size: 32

src/lib/dns/tests/testdata/rdata_tsig_fromWire5.spec

@@ -0,0 +1,7 @@
+#
+# TSIG-like RDATA but RDLEN is too short.
+#
+[custom]
+sections: tsig
+[tsig]
+rdlen: 60

src/lib/dns/tests/testdata/rdata_tsig_fromWire6.spec

@@ -0,0 +1,7 @@
+#
+# TSIG-like RDATA but RDLEN is too long.
+#
+[custom]
+sections: tsig
+[tsig]
+rdlen: 63

src/lib/dns/tests/testdata/rdata_tsig_fromWire7.spec

@@ -0,0 +1,8 @@
+#
+# TSIG-like RDATA but algorithm name is broken.
+#
+[custom]
+sections: tsig
+[tsig]
+algorithm: "01234567890123456789012345678901234567890123456789012345678901234"
+mac_size: 32

src/lib/dns/tests/testdata/rdata_tsig_fromWire8.spec

@@ -0,0 +1,8 @@
+#
+# TSIG-like RDATA but MAC size is bogus
+#
+[custom]
+sections: tsig
+[tsig]
+mac_size: 65535
+mac: "dummy data"

src/lib/dns/tests/testdata/rdata_tsig_fromWire9.spec

@@ -0,0 +1,8 @@
+#
+# TSIG-like RDATA but Other-Data length is bogus
+#
+[custom]
+sections: tsig
+[tsig]
+other_len: 65535
+otherdata: "dummy data"

src/lib/dns/tests/testdata/rdata_tsig_toWire1.spec

@@ -0,0 +1,11 @@
+#
+# An artificial TSIG RDATA for toWire test.
+#
+[custom]
+sections: tsig
+[tsig]
+algorithm: hmac-md5
+time_signed: 1286779327
+mac_size: 0
+original_id: 16020
+error: 17

src/lib/dns/tests/testdata/rdata_tsig_toWire2.spec

@@ -0,0 +1,13 @@
+#
+# An artificial TSIG RDATA for toWire test.
+#
+[custom]
+sections: tsig
+[tsig]
+algorithm: hmac-sha256
+time_signed: 1286779327
+mac_size: 12
+# 0x1402... would be FAKEFAKE... if encoded in BASE64
+mac: 0x140284140284140284140284
+original_id: 16020
+error: 16

src/lib/dns/tests/testdata/rdata_tsig_toWire3.spec

@@ -0,0 +1,15 @@
+#
+# An artificial TSIG RDATA for toWire test.
+#
+[custom]
+sections: tsig
+[tsig]
+algorithm: hmac-sha1
+time_signed: 1286779327
+mac_size: 12
+# 0x1402... would be FAKEFAKE... if encoded in BASE64
+mac: 0x140284140284140284140284
+original_id: 16020
+error: 18
+other_len: 6
+other_data: 0x140284140284

src/lib/dns/tests/testdata/rdata_tsig_toWire4.spec

@@ -0,0 +1,13 @@
+#
+# An artificial TSIG RDATA for toWire test.
+#
+[custom]
+sections: name:tsig
+[name]
+name: hmac-md5.sig-alg.reg.int.
+[tsig]
+algorithm: hmac-md5
+time_signed: 1286779327
+mac_size: 0
+original_id: 16020
+error: 17

src/lib/dns/tests/testdata/rdata_tsig_toWire5.spec

@@ -0,0 +1,13 @@
+#
+# An artificial TSIG RDATA for toWire test.
+#
+[custom]
+sections: tsig:name
+[tsig]
+algorithm: hmac-md5
+time_signed: 1286779327
+mac_size: 0
+original_id: 16020
+error: 17
+[name]
+name: ptr=2

src/lib/dns/tests/tsigkey_unittest.cc

@@ -0,0 +1,230 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <string>
+
+#include <gtest/gtest.h>
+
+#include <exceptions/exceptions.h>
+
+#include <dns/tsigkey.h>
+
+#include <dns/tests/unittest_util.h>
+
+using namespace std;
+using namespace isc::dns;
+using isc::UnitTestUtil;
+
+namespace {
+class TSIGKeyTest : public ::testing::Test {
+protected:
+    TSIGKeyTest() : secret("someRandomData"), key_name("example.com") {}
+    string secret;
+    Name key_name;
+};
+
+TEST_F(TSIGKeyTest, algorithmNames) {
+    EXPECT_EQ(Name("hmac-md5.sig-alg.reg.int"), TSIGKey::HMACMD5_NAME());
+    EXPECT_EQ(Name("hmac-sha1"), TSIGKey::HMACSHA1_NAME());
+    EXPECT_EQ(Name("hmac-sha256"), TSIGKey::HMACSHA256_NAME());
+}
+
+TEST_F(TSIGKeyTest, construct) {
+    TSIGKey key(key_name, TSIGKey::HMACMD5_NAME(),
+                secret.c_str(), secret.size());
+    EXPECT_EQ(key_name, key.getKeyName());
+    EXPECT_EQ(Name("hmac-md5.sig-alg.reg.int"), key.getAlgorithmName());
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret.c_str(),
+                        secret.size(), key.getSecret(), key.getSecretLength());
+
+    EXPECT_THROW(TSIGKey(key_name, Name("unknown-alg"),
+                         secret.c_str(), secret.size()),
+                 isc::InvalidParameter);
+
+    // The algorithm name should be converted to the canonical form.
+    EXPECT_EQ("hmac-sha1.",
+              TSIGKey(key_name, Name("HMAC-sha1"),
+                      secret.c_str(),
+                      secret.size()).getAlgorithmName().toText());
+
+    // Invalid combinations of secret and secret_len:
+    EXPECT_THROW(TSIGKey(key_name, TSIGKey::HMACSHA1_NAME(), secret.c_str(), 0),
+                 isc::InvalidParameter);
+    EXPECT_THROW(TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(), NULL, 16),
+                 isc::InvalidParameter);
+}
+
+void
+compareTSIGKeys(const TSIGKey& expect, const TSIGKey& actual) {
+    EXPECT_EQ(expect.getKeyName(), actual.getKeyName());
+    EXPECT_EQ(expect.getAlgorithmName(), actual.getAlgorithmName());
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        expect.getSecret(), expect.getSecretLength(),
+                        actual.getSecret(), actual.getSecretLength());
+}
+
+TEST_F(TSIGKeyTest, copyConstruct) {
+    const TSIGKey original(key_name, TSIGKey::HMACSHA256_NAME(),
+                           secret.c_str(), secret.size());
+    const TSIGKey copy(original);
+    compareTSIGKeys(original, copy);
+
+    // Check the copied data is valid even after the original is deleted
+    TSIGKey* copy2 = new TSIGKey(original);
+    TSIGKey copy3(*copy2);
+    delete copy2;
+    compareTSIGKeys(original, copy3);
+}
+
+TEST_F(TSIGKeyTest, assignment) {
+    const TSIGKey original(key_name, TSIGKey::HMACSHA256_NAME(),
+                           secret.c_str(), secret.size());
+    TSIGKey copy = original;
+    compareTSIGKeys(original, copy);
+
+    // Check if the copied data is valid even after the original is deleted
+    TSIGKey* copy2 = new TSIGKey(original);
+    TSIGKey copy3(original);
+    copy3 = *copy2;
+    delete copy2;
+    compareTSIGKeys(original, copy3);
+
+    // self assignment
+    copy = copy;
+    compareTSIGKeys(original, copy);
+}
+
+class TSIGKeyRingTest : public ::testing::Test {
+protected:
+    TSIGKeyRingTest() :
+        key_name("example.com"),
+        secretstring("anotherRandomData"),
+        secret(secretstring.c_str()),
+        secret_len(secretstring.size())
+    {}
+    TSIGKeyRing keyring;
+    Name key_name;
+private:
+    const string secretstring;
+protected:
+    const char* secret;
+    size_t secret_len;
+};
+
+TEST_F(TSIGKeyRingTest, init) {
+    EXPECT_EQ(0, keyring.size());
+}
+
+TEST_F(TSIGKeyRingTest, add) {
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(1, keyring.size());
+    EXPECT_EQ(TSIGKeyRing::EXIST, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    // keys are identified their names, the same name of key with a different
+    // algorithm would be considered a duplicate.
+    EXPECT_EQ(TSIGKeyRing::EXIST, keyring.add(
+                  TSIGKey(Name("example.com"), TSIGKey::HMACSHA1_NAME(),
+                          secret, secret_len)));
+    // names are compared in a case insensitive manner.
+    EXPECT_EQ(TSIGKeyRing::EXIST, keyring.add(
+                  TSIGKey(Name("EXAMPLE.COM"), TSIGKey::HMACSHA1_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(1, keyring.size());
+}
+
+TEST_F(TSIGKeyRingTest, addMore) {
+    // essentially the same test, but try adding more than 1
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(Name("another.example"), TSIGKey::HMACMD5_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(Name("more.example"), TSIGKey::HMACSHA1_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(3, keyring.size());
+}
+
+TEST_F(TSIGKeyRingTest, remove) {
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.remove(key_name));
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND, keyring.remove(key_name));
+}
+
+TEST_F(TSIGKeyRingTest, removeFromSome) {
+    // essentially the same test, but try removing from a larger set
+
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(Name("another.example"), TSIGKey::HMACMD5_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(Name("more.example"), TSIGKey::HMACSHA1_NAME(),
+                          secret, secret_len)));
+
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.remove(Name("another.example")));
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND, keyring.remove(Name("noexist.example")));
+    EXPECT_EQ(2, keyring.size());
+}
+
+TEST_F(TSIGKeyRingTest, find) {
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND, keyring.find(key_name).code);
+    EXPECT_EQ(static_cast<const TSIGKey*>(NULL), keyring.find(key_name).key);
+
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    const TSIGKeyRing::FindResult result(keyring.find(key_name));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, result.code);
+    EXPECT_EQ(key_name, result.key->getKeyName());
+    EXPECT_EQ(TSIGKey::HMACSHA256_NAME(), result.key->getAlgorithmName());
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret, secret_len,
+                        result.key->getSecret(),
+                        result.key->getSecretLength());
+}
+
+TEST_F(TSIGKeyRingTest, findFromSome) {
+    // essentially the same test, but search a larger set
+
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(Name("another.example"), TSIGKey::HMACMD5_NAME(),
+                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
+                  TSIGKey(Name("more.example"), TSIGKey::HMACSHA1_NAME(),
+                          secret, secret_len)));
+
+    const TSIGKeyRing::FindResult result(
+        keyring.find(Name("another.example")));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, result.code);
+    EXPECT_EQ(Name("another.example"), result.key->getKeyName());
+    EXPECT_EQ(TSIGKey::HMACMD5_NAME(), result.key->getAlgorithmName());
+
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND,
+              keyring.find(Name("noexist.example")).code);
+    EXPECT_EQ(static_cast<const TSIGKey*>(NULL),
+              keyring.find(Name("noexist.example")).key);
+}
+
+} // end namespace

src/lib/dns/tsig.cc

@@ -1,33 +0,0 @@
-// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-// $Id$
-
-#include <cctype>
-#include <cassert>
-#include <iterator>
-#include <functional>
-
-#include <algorithm>
-
-#include <dns/tsig.h>
-
-using namespace std;
-using isc::dns::MessageRenderer;
-
-namespace isc {
-namespace dns {
-
-} // namespace dns
-} // namespace isc

src/lib/dns/tsig.h

@@ -1,72 +0,0 @@
-// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-// $Id$
-
-#ifndef __TSIG_H
-#define __TSIG_H 1
-
-#include <string>
-#include <vector>
-
-#include <exceptions/exceptions.h>
-
-#include <dns/name.h>
-#include <dns/message.h>
-
-namespace isc {
-namespace dns {
-
-class BadTsigKey : public Exception {
-public:
-    BadTsigKey(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
-};
-
-//
-// This class holds a Tsig key, including all its attributes.
-//
-class Tsig {
-public:
-    enum TsigAlgorithm {
-        HMACMD5 = 0,
-        GSS = 1,
-        HMACSHA1 = 2,
-        HMACSHA224 = 3,
-        HMACSHA265 = 4,
-        HMACSHA384 = 5,
-        HMACSHA512 = 6,
-    };
-
-    Tsig(const Name& name, TsigAlgorithm algorithm,
-         const std::string& algorithm_data) :
-         name_(name), algorithm_(algorithm), algorithm_data_(algorithm_data) {};
-
-    bool signMessage(const Message& message);
-    bool verifyMessage(const Message &message);
-
-private:
-    Name name_;
-    TsigAlgorithm algorithm_;
-    std::string algorithm_data_;
-};
-
-}
-}
-
-#endif  // __TSIG_H
-
-// Local Variables: 
-// mode: c++
-// End: 

src/lib/dns/tsigkey.cc

@@ -0,0 +1,167 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#include <map>
+#include <utility>
+#include <vector>
+
+#include <exceptions/exceptions.h>
+
+#include <dns/name.h>
+#include <dns/tsigkey.h>
+
+using namespace std;
+
+namespace isc {
+namespace dns {
+struct
+TSIGKey::TSIGKeyImpl {
+    TSIGKeyImpl(const Name& key_name, const Name& algorithm_name,
+                const void* secret, size_t secret_len) :
+        key_name_(key_name), algorithm_name_(algorithm_name),
+        secret_(static_cast<const uint8_t*>(secret),
+                static_cast<const uint8_t*>(secret) + secret_len)
+    {
+        // Convert the name to the canonical form.
+        algorithm_name_.downcase();
+    }
+    const Name key_name_;
+    Name algorithm_name_;
+    const vector<uint8_t> secret_;
+};
+
+TSIGKey::TSIGKey(const Name& key_name, const Name& algorithm_name,
+                 const void* secret, size_t secret_len) : impl_(NULL)
+{
+    if (algorithm_name != HMACMD5_NAME() &&
+        algorithm_name != HMACSHA1_NAME() &&
+        algorithm_name != HMACSHA256_NAME()) {
+        isc_throw(InvalidParameter, "Unknown TSIG algorithm is specified: " <<
+                  algorithm_name);
+    }
+    if ((secret != NULL && secret_len == 0) ||
+        (secret == NULL && secret_len != 0)) {
+        isc_throw(InvalidParameter,
+                  "TSIGKey secret and its length are inconsistent");
+    }
+
+    impl_ = new TSIGKeyImpl(key_name, algorithm_name, secret, secret_len);
+}
+
+TSIGKey::TSIGKey(const TSIGKey& source) : impl_(new TSIGKeyImpl(*source.impl_))
+{}
+
+TSIGKey&
+TSIGKey::operator=(const TSIGKey& source) {
+    if (impl_ == source.impl_) {
+        return (*this);
+    }
+
+    TSIGKeyImpl* newimpl = new TSIGKeyImpl(*source.impl_);
+    delete impl_;
+    impl_ = newimpl;
+
+    return (*this);
+}
+
+TSIGKey::~TSIGKey() {
+    delete impl_;
+}
+
+const Name&
+TSIGKey::getKeyName() const {
+    return (impl_->key_name_);
+}
+
+const Name&
+TSIGKey::getAlgorithmName() const {
+    return (impl_->algorithm_name_);
+}
+
+const void*
+TSIGKey::getSecret() const {
+    return ((impl_->secret_.size() > 0) ? &impl_->secret_[0] : NULL);
+}
+
+size_t
+TSIGKey::getSecretLength() const {
+    return (impl_->secret_.size());
+}
+
+const
+Name& TSIGKey::HMACMD5_NAME() {
+    static Name alg_name("hmac-md5.sig-alg.reg.int");
+    return (alg_name);
+}
+
+const
+Name& TSIGKey::HMACSHA1_NAME() {
+    static Name alg_name("hmac-sha1");
+    return (alg_name);
+}
+
+const
+Name& TSIGKey::HMACSHA256_NAME() {
+    static Name alg_name("hmac-sha256");
+    return (alg_name);
+}
+
+struct TSIGKeyRing::TSIGKeyRingImpl {
+    typedef map<Name, TSIGKey> TSIGKeyMap;
+    typedef pair<Name, TSIGKey> NameAndKey;
+    TSIGKeyMap keys;
+};
+
+TSIGKeyRing::TSIGKeyRing() : impl_(new TSIGKeyRingImpl) {
+}
+
+TSIGKeyRing::~TSIGKeyRing() {
+    delete impl_;
+}
+
+unsigned int
+TSIGKeyRing::size() const {
+    return (impl_->keys.size());
+}
+
+TSIGKeyRing::Result
+TSIGKeyRing::add(const TSIGKey& key) {
+    if (impl_->keys.insert(
+                TSIGKeyRingImpl::NameAndKey(key.getKeyName(), key)).second
+        == true) {
+        return (SUCCESS);
+    } else {
+        return (EXIST);
+    }
+}
+
+TSIGKeyRing::Result
+TSIGKeyRing::remove(const Name& key_name) {
+    return (impl_->keys.erase(key_name) == 1 ? SUCCESS : NOTFOUND);
+}
+
+TSIGKeyRing::FindResult
+TSIGKeyRing::find(const Name& key_name) {
+    TSIGKeyRingImpl::TSIGKeyMap::const_iterator found =
+        impl_->keys.find(key_name);
+    if (found == impl_->keys.end()) {
+        return (FindResult(NOTFOUND, NULL));
+    }
+    return (FindResult(SUCCESS, &((*found).second)));
+}
+
+} // namespace dns
+} // namespace isc

src/lib/dns/tsigkey.h

@@ -0,0 +1,300 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#ifndef __TSIGKEY_H
+#define __TSIGKEY_H 1
+
+namespace isc {
+namespace dns {
+
+class Name;
+
+/// \brief TSIG key.
+///
+/// This class holds a TSIG key along with some related attributes as
+/// defined in RFC2845.
+///
+/// A TSIG key consists of the following attributes:
+/// - Key name
+/// - Hash algorithm
+/// - Shared secret
+///
+/// <b>Implementation Notes</b>
+///
+/// We may add more attributes in future versions.  For example, if and when
+/// we support the TKEY protocol (RFC2930), we may need to introduce the
+/// notion of inception and expiration times.
+/// At that point we may also have to introduce a class hierarchy to handle
+/// different types of keys in a polymorphic way.
+/// At the moment we use the straightforward value-type class with minimal
+/// attributes.
+///
+/// In the TSIG protocol, hash algorithms are represented in the form of
+/// domain name.
+/// Our interfaces provide direct translation of this concept; for example,
+/// the constructor from parameters take a \c Name object to specify the
+/// algorithm.
+/// On one hand, this may be counter intuitive.
+/// An API user would rather specify "hmac-md5" instead of
+/// <code>Name("hmac-md5.sig-alg.reg.int")</code>.
+/// On the other hand, it may be more convenient for some kind of applications
+/// if we maintain the algorithm as the expected representation for
+/// protocol operations (such as sign and very a message).
+/// Considering these points, we adopt the interface closer to the protocol
+/// specification for now.
+/// To minimize the burden for API users, we also define a set of constants
+/// for commonly used algorithm names so that the users don't have to
+/// remember the actual domain names defined in the protocol specification.
+/// We may also have to add conversion routines between domain names
+/// and more intuitive representations (e.g. strings) for algorithms.
+class TSIGKey {
+public:
+    ///
+    /// \name Constructors, Assignment Operator and Destructor.
+    ///
+    //@{
+    /// \brief Constructor from key parameters
+    ///
+    /// In the current implementation, \c algorithm_name must be a known
+    /// algorithm to this implementation, which are defined via the
+    /// <code>static const</code> member functions.  For other names
+    /// an exception of class \c InvalidParameter will be thrown.
+    /// Note: This restriction may be too strict, and we may revisit it
+    /// later.
+    ///
+    /// \c secret and \c secret_len must be consistent in that the latter
+    /// is 0 if and only if the former is \c NULL;
+    /// otherwise an exception of type \c InvalidParameter will be thrown.
+    ///
+    /// This constructor internally involves resource allocation, and if
+    /// it fails, a corresponding standard exception will be thrown.
+    ///
+    /// \param key_name The name of the key as a domain name.
+    /// \param algorithm_name The hash algorithm used for this key in the
+    /// form of domain name.  For example, it can be
+    /// \c TSIGKey::HMACSHA256_NAME() for HMAC-SHA256.
+    /// \param secret Point to a binary sequence of the shared secret to be
+    /// used for this key, or \c NULL if the secret is empty.
+    /// \param secret_len The size of the binary %data (\c secret) in bytes.
+    TSIGKey(const Name& key_name, const Name& algorithm_name,
+            const void* secret, size_t secret_len);
+
+    /// \brief The copy constructor.
+    ///
+    /// It internally allocates a resource, and if it fails a corresponding
+    /// standard exception will be thrown.
+    /// This constructor never throws an exception otherwise.
+    TSIGKey(const TSIGKey& source);
+
+    /// \brief Assignment operator.
+    ///
+    /// It internally allocates a resource, and if it fails a corresponding
+    /// standard exception will be thrown.
+    /// This operator never throws an exception otherwise.
+    ///
+    /// This operator provides the strong exception guarantee: When an
+    /// exception is thrown the content of the assignment target will be
+    /// intact.
+    TSIGKey& operator=(const TSIGKey& source);
+
+    /// The destructor.
+    ~TSIGKey();
+    //@}
+
+    ///
+    /// \name Getter Methods
+    ///
+    /// These methods never throw an exception.
+    //@{
+    /// Return the key name.
+    const Name& getKeyName() const;
+
+    /// Return the algorithm name.
+    const Name& getAlgorithmName() const;
+
+    /// Return the length of the TSIG secret in bytes.
+    size_t getSecretLength() const;
+
+    /// Return the value of the TSIG secret.
+    ///
+    /// If it returns a non NULL pointer, the memory region beginning at the
+    /// address returned by this method is valid up to the bytes specified
+    /// by the return value of \c getSecretLength().
+    ///
+    /// The memory region is only valid while the corresponding \c TSIGKey
+    /// object is valid.  The caller must hold the \c TSIGKey object while
+    /// it needs to refer to the region or it must make a local copy of the
+    /// region.
+    const void* getSecret() const;
+    //@}
+
+    ///
+    /// \name Well known algorithm names as defined in RFC2845 and RFC4635.
+    ///
+    /// Note: we begin with the "mandatory" algorithms defined in RFC4635
+    /// as a minimal initial set.
+    /// We'll add others as we see the need for them.
+    //@{
+    static const Name& HMACMD5_NAME();    ///< HMAC-MD5 (RFC2845)
+    static const Name& HMACSHA1_NAME();   ///< HMAC-SHA1 (RFC4635)
+    static const Name& HMACSHA256_NAME(); ///< HMAC-SHA256 (RFC4635)
+    //@}
+
+private:
+    struct TSIGKeyImpl;
+    const TSIGKeyImpl* impl_;
+};
+
+/// \brief A simple repository of a set of \c TSIGKey objects.
+///
+/// This is a "key ring" to maintain TSIG keys (\c TSIGKey objects) and
+/// provides trivial operations such as add, remove, and find.
+///
+/// The keys are identified by their key names.
+/// So, for example, two or more keys of the same key name but of different
+/// algorithms are considered to be the same, and cannot be stored in the
+/// key ring at the same time.
+///
+/// <b>Implementation Note:</b>
+/// For simplicity the initial implementation requests the application make
+/// a copy of keys stored in the key ring if it needs to use the keys for
+/// a long period (during which some of the keys may be removed).
+/// This is based on the observations that a single server will not hold
+/// a huge number of keys nor use keys in many different contexts (such as
+/// in different DNS transactions).
+/// If this assumption does not hold and memory consumption becomes an issue
+/// we may have to revisit the design.
+class TSIGKeyRing {
+public:
+    /// Result codes of various public methods of \c TSIGKeyRing
+    enum Result {
+        SUCCESS = 0,    ///< The operation is successful.
+        EXIST = 1,      ///< A key is already stored in \c TSIGKeyRing.
+        NOTFOUND = 2    ///< The specified key is not found in \c TSIGKeyRing.
+    };
+
+    /// \brief A helper structure to represent the search result of
+    /// <code>TSIGKeyRing::find()</code>.
+    ///
+    /// This is a straightforward pair of the result code and a pointer
+    /// to the found key to represent the result of \c find().
+    /// We use this in order to avoid overloading the return value for both
+    /// the result code ("success" or "not found") and the found object,
+    /// i.e., avoid using \c NULL to mean "not found", etc.
+    ///
+    /// This is a simple value class with no internal state, so for
+    /// convenience we allow the applications to refer to the members
+    /// directly.
+    ///
+    /// See the description of \c find() for the semantics of the member
+    /// variables.
+    struct FindResult {
+        FindResult(Result param_code, const TSIGKey* param_key) :
+            code(param_code), key(param_key)
+        {}
+        const Result code;
+        const TSIGKey* const key;
+    };
+
+    ///
+    /// \name Constructors and Destructor.
+    ///
+    /// \b Note:
+    /// The copy constructor and the assignment operator are
+    /// intentionally defined as private, making this class non copyable.
+    /// There is no technical reason why this class cannot be copied,
+    /// but since the key ring can potentially have a large number of keys,
+    /// a naive copy operation may cause unexpected overhead.
+    /// It's generally expected for an application to share the same
+    /// instance of key ring and share it throughout the program via
+    /// references, so we prevent the copy operation explicitly to avoid
+    /// unexpected copy operations.
+    //@{
+private:
+    TSIGKeyRing(const TSIGKeyRing& source);
+    TSIGKeyRing& operator=(const TSIGKeyRing& source);
+public:
+    /// \brief The default constructor.
+    ///
+    /// This constructor never throws an exception.
+    TSIGKeyRing();
+
+    /// The destructor.
+    ~TSIGKeyRing();
+    //@}
+
+    /// Return the number of keys stored in the \c TSIGKeyRing.
+    ///
+    /// This method never throws an exception.
+    unsigned int size() const;
+
+    /// Add a \c TSIGKey to the \c TSIGKeyRing.
+    ///
+    /// This method will create a local copy of the given key, so the caller
+    /// does not have to keep owning it.
+    ///
+    /// If internal resource allocation fails, a corresponding standard
+    /// exception will be thrown.
+    /// This method never throws an exception otherwise.
+    ///
+    /// \param key A \c TSIGKey to be added.
+    /// \return \c SUCCESS If the key is successfully added to the key ring.
+    /// \return \c EXIST The key ring already stores a key whose name is
+    /// identical to that of \c key.
+    Result add(const TSIGKey& key);
+
+    /// Remove a \c TSIGKey for the given name from the \c TSIGKeyRing.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// \param key_name The name of the key to be removed.
+    /// \return \c SUCCESS If the key is successfully removed from the key
+    /// ring.
+    /// \return \c NOTFOUND The key ring does not store the key that matches
+    /// \c key_name.
+    Result remove(const Name& key_name);
+
+    /// Find a \c TSIGKey for the given name in the \c TSIGKeyRing.
+    ///
+    /// It searches the internal storage for a \c TSIGKey whose name is
+    /// \c key_name, and returns the result in the form of a \c FindResult
+    /// object as follows:
+    /// - \c code: \c SUCCESS if a key is found; otherwise \c NOTFOUND.
+    /// - \c key: A pointer to the found \c TSIGKey object if one is found;
+    /// otherwise \c NULL.
+    ///
+    /// The pointer returned in the \c FindResult object is only valid until
+    /// the corresponding key is removed from the key ring.
+    /// The caller must ensure that the key is held in the key ring while
+    /// it needs to refer to it, or it must make a local copy of the key.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// \param key_name The name of the key to be found.
+    /// \return A \c FindResult object enclosing the search result (see above).
+    FindResult find(const Name& key_name);
+private:
+    struct TSIGKeyRingImpl;
+    TSIGKeyRingImpl* impl_;
+};
+}
+}
+
+#endif  // __TSIGKEY_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/exceptions/exceptions.h

@@ -40,7 +40,7 @@ public:
     /// file line number.
     ///
     /// @param file the file name where the exception was thrown.
-    /// @param line the line in @ref file where the exception was thrown.
+    /// @param line the line in \a file where the exception was thrown.
     /// @param what a description (type) of the exception.
     Exception(const char* file, size_t line, const char* what) :
         file_(file), line_(line), what_(what) {}
@@ -49,7 +49,7 @@ public:
     /// file line number.
     ///
     /// @param file the file name where the exception was thrown.
-    /// @param line the line in @ref file where the exception was thrown.
+    /// @param line the line in \a file where the exception was thrown.
     /// @param what a description (type) of the exception.
     Exception(const char* file, size_t line, const std::string& what) :
         file_(file), line_(line), what_(what) {}

src/lib/python/isc/cc/data.py

@@ -56,20 +56,116 @@ def remove_null_items(d):
     for k in null_keys:
         del d[k]
 
-def find(element, identifier):
-    """Returns the subelement in the given data element, raises DataNotFoundError if not found"""
-    if type(identifier) != str or (type(element) != dict and identifier != ""):
-        raise DataTypeError("identifier in merge() is not a string")
-    if type(identifier) != str or (type(element) != dict and identifier != ""):
-        raise DataTypeError("element in merge() is not a dict")
-    id_parts = identifier.split("/")
+def _concat_identifier(id_parts):
+    """Concatenates the given identifier parts into a string,
+       delimited with the '/' character.
+    """
+    return '/'.join(id_parts)
+
+def split_identifier(identifier):
+    """Splits the given identifier into a list of identifier parts,
+       as delimited by the '/' character.
+       Raises a DataTypeError if identifier is not a string."""
+    if type(identifier) != str:
+        raise DataTypeError("identifier is not a string")
+    id_parts = identifier.split('/')
     id_parts[:] = (value for value in id_parts if value != "")
+    return id_parts
+
+def split_identifier_list_indices(identifier):
+    """Finds list indexes in the given identifier, which are of the
+       format [integer].
+       Identifier must be a string.
+       This will only give the list index for the last 'part' of the
+       given identifier (as delimited by the '/' sign).
+       Raises a DataTypeError if the identifier is not a string,
+       or if the format is bad.
+       Returns a tuple, where the first element is the string part of
+       the identifier, and the second element is a list of (nested) list
+       indices.
+       Examples:
+       'a/b/c' will return ('a/b/c', None)
+       'a/b/c[1]' will return ('a/b/c', [1])
+       'a/b/c[1][2][3]' will return ('a/b/c', [1, 2, 3])
+       'a[0]/b[1]/c[2]' will return ('a[0]/b[1]/c', [2])
+    """
+    if type(identifier) != str:
+        raise DataTypeError("identifier in "
+                            "split_identifier_list_indices() "
+                            "not a string: " + str(identifier))
+
+    # We only work on the final 'part' of the identifier
+    id_parts = split_identifier(identifier)
+    id_str = id_parts[-1]
+
+    i = id_str.find('[')
+    if i < 0:
+        if identifier.find(']') >= 0:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+        return identifier, None
+
+    # keep the non-index part of that to replace later
+    id = id_str[:i]
+    indices = []
+    while i >= 0:
+        e = id_str.find(']')
+        if e < i + 1:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+        try:
+            indices.append(int(id_str[i+1:e]))
+        except ValueError:
+            raise DataTypeError("List index in " + identifier + " not an integer")
+        id_str = id_str[e + 1:]
+        i = id_str.find('[')
+        if i > 0:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+    if id.find(']') >= 0 or len(id_str) > 0:
+        raise DataTypeError("Bad format in identifier: " + str(identifier))
+
+    # we replace the final part of the original identifier with
+    # the stripped string
+    id_parts[-1] = id
+    id = _concat_identifier(id_parts)
+    return id, indices
+
+def _find_child_el(element, id):
+    """Finds the child of element with the given id. If the id contains
+       [i], where i is a number, and the child element is a list, the
+       i-th element of that list is returned instead of the list itself.
+       Raises a DataTypeError if the element is of wrong type, if id
+       is not a string, or if the id string contains a bad value.
+       Raises a DataNotFoundError if the element at id could not be
+       found.
+    """
+    id, list_indices = split_identifier_list_indices(id)
+    if type(element) == dict and id in element.keys():
+        result = element[id]
+    else:
+        raise DataNotFoundError(id + " in " + str(element))
+    if type(result) == list and list_indices is not None:
+        for list_index in list_indices:
+            if list_index >= len(result):
+                raise DataNotFoundError("Element " + str(list_index) + " in " + str(result))
+            result = result[list_index]
+    return result
+
+def find(element, identifier):
+    """Returns the subelement in the given data element, raises
+       DataNotFoundError if not found.
+       Returns the given element if the identifier is an empty string.
+       Raises a DataTypeError if identifier is not a string, or if
+       identifier is not empty, and element is not a dict.
+    """
+    if type(identifier) != str:
+        raise DataTypeError("identifier in find() is not a str")
+    if identifier == "":
+        return element
+    if type(element) != dict:
+        raise DataTypeError("element in find() is not a dict")
+    id_parts = split_identifier(identifier)
     cur_el = element
     for id in id_parts:
-        if type(cur_el) == dict and id in cur_el.keys():
-            cur_el = cur_el[id]
-        else:
-            raise DataNotFoundError(identifier + " in " + str(element))
+        cur_el = _find_child_el(cur_el, id)
     return cur_el
 
 def set(element, identifier, value):
@@ -83,25 +179,46 @@ def set(element, identifier, value):
     if type(element) != dict:
         raise DataTypeError("element in set() is not a dict")
     if type(identifier) != str:
-        raise DataTypeError("identifier in set() is not a string")
-    id_parts = identifier.split("/")
-    id_parts[:] = (value for value in id_parts if value != "")
+        raise DataTypeError("identifier in set() is not a str")
+    id_parts = split_identifier(identifier)
     cur_el = element
     for id in id_parts[:-1]:
-        if id in cur_el.keys():
-            cur_el = cur_el[id]
-        else:
-            if value == None:
+        try:
+            cur_el = _find_child_el(cur_el, id)
+        except DataNotFoundError:
+            if value is None:
                 # ok we are unsetting a value that wasn't set in
                 # the first place. Simply stop.
                 return
             cur_el[id] = {}
             cur_el = cur_el[id]
-    # value can be an empty list or dict, so check for None eplicitely
-    if value != None:
-        cur_el[id_parts[-1]] = value
-    elif id_parts[-1] in cur_el:
-        del cur_el[id_parts[-1]]
+
+    id, list_indices = split_identifier_list_indices(id_parts[-1])
+    if list_indices is None:
+        # value can be an empty list or dict, so check for None eplicitely
+        if value is not None:
+            cur_el[id] = value
+        else:
+            del cur_el[id]
+    else:
+        cur_el = cur_el[id]
+        # in case of nested lists, we need to get to the next to last
+        for list_index in list_indices[:-1]:
+            if type(cur_el) != list:
+                raise DataTypeError("Element at " + identifier + " is not a list")
+            if len(cur_el) <= list_index:
+                raise DataNotFoundError("List index at " + identifier + " out of range")
+            cur_el = cur_el[list_index]
+        # value can be an empty list or dict, so check for None eplicitely
+        list_index = list_indices[-1]
+        if type(cur_el) != list:
+            raise DataTypeError("Element at " + identifier + " is not a list")
+        if len(cur_el) <= list_index:
+            raise DataNotFoundError("List index at " + identifier + " out of range")
+        if value is not None:
+            cur_el[list_index] = value
+        else:
+            del cur_el[list_index]
     return element
 
 def unset(element, identifier):
@@ -116,17 +233,12 @@ def find_no_exc(element, identifier):
     """Returns the subelement in the given data element, returns None
        if not found, or if an error occurred (i.e. this function should
        never raise an exception)"""
-    if type(identifier) != str:
+    try:
+        return find(element, identifier)
+    except DataNotFoundError:
+        return None
+    except DataTypeError:
         return None
-    id_parts = identifier.split("/")
-    id_parts[:] = (value for value in id_parts if value != "")
-    cur_el = element
-    for id in id_parts:
-        if (type(cur_el) == dict and id in cur_el.keys()) or id=="":
-            cur_el = cur_el[id]
-        else:
-            return None
-    return cur_el
 
 def parse_value_str(value_str):
     """Parses the given string to a native python object. If the
@@ -139,7 +251,4 @@ def parse_value_str(value_str):
     except ValueError as ve:
         # simply return the string itself
         return value_str
-    except SyntaxError as ve:
-        # simply return the string itself
-        return value_str
 

src/lib/python/isc/cc/tests/data_test.py

@@ -70,6 +70,11 @@ class TestData(unittest.TestCase):
         c = { "a": { "b": "c" } }
         data.remove_identical(a, b)
         self.assertEqual(a, c)
+
+        self.assertRaises(data.DataTypeError, data.remove_identical,
+                          a, 1)
+        self.assertRaises(data.DataTypeError, data.remove_identical,
+                          1, b)
         
     def test_merge(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
@@ -82,6 +87,45 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataTypeError, data.merge, 1, d2)
         self.assertRaises(data.DataTypeError, data.merge, None, None)
 
+
+    def test_split_identifier_list_indices(self):
+        id, indices = data.split_identifier_list_indices('a')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, None)
+        id, indices = data.split_identifier_list_indices('a[0]')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, [0])
+        id, indices = data.split_identifier_list_indices('a[0][1]')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, [0, 1])
+
+        # examples from the docstring
+        id, indices = data.split_identifier_list_indices('a/b/c')
+        self.assertEqual(id, 'a/b/c')
+        self.assertEqual(indices, None)
+        
+        id, indices = data.split_identifier_list_indices('a/b/c[1]')
+        self.assertEqual(id, 'a/b/c')
+        self.assertEqual(indices, [1])
+       
+        id, indices = data.split_identifier_list_indices('a/b/c[1][2][3]')
+        self.assertEqual(id, 'a/b/c')
+        self.assertEqual(indices, [1, 2, 3])
+        
+        id, indices = data.split_identifier_list_indices('a[0]/b[1]/c[2]')
+        self.assertEqual(id, 'a[0]/b[1]/c')
+        self.assertEqual(indices, [2])
+
+        # bad formats
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a]')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[[0]]')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[0]a')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[0]a[1]')
+
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 1)
+        
+
     def test_find(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
         self.assertEqual(data.find(d1, ''), d1)
@@ -93,19 +137,47 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataNotFoundError, data.find, d1, 'f')
         self.assertRaises(data.DataTypeError, data.find, d1, 1)
         self.assertRaises(data.DataTypeError, data.find, None, 1)
+        self.assertRaises(data.DataTypeError, data.find, None, "foo")
         self.assertRaises(data.DataTypeError, data.find, "123", "123")
         self.assertEqual(data.find("123", ""), "123")
+
+        d2 = { 'a': [ 1, 2, 3 ] }
+        self.assertEqual(data.find(d2, 'a[0]'), 1)
+        self.assertEqual(data.find(d2, 'a[1]'), 2)
+        self.assertEqual(data.find(d2, 'a[2]'), 3)
+        self.assertRaises(data.DataNotFoundError, data.find, d2, 'a[3]')
+        self.assertRaises(data.DataTypeError, data.find, d2, 'a[a]')
+
+        d3 = { 'a': [ { 'b': [ {}, { 'c': 'd' } ] } ] }
+        self.assertEqual(data.find(d3, 'a[0]/b[1]/c'), 'd')
+        self.assertRaises(data.DataNotFoundError, data.find, d3, 'a[1]/b[1]/c')
         
     def test_set(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
         d12 = { 'b': 1, 'c': { 'e': 3, 'f': [ 1 ] } }
+        d13 = { 'b': 1, 'c': { 'e': 3, 'f': [ 2 ] } }
+        d14 = { 'b': 1, 'c': { 'e': 3, 'f': [ { 'g': [ 1, 2 ] } ] } }
+        d15 = { 'b': 1, 'c': { 'e': 3, 'f': [ { 'g': [ 1, 3 ] } ] } }
         data.set(d1, 'a', None)
         data.set(d1, 'c/d', None)
         data.set(d1, 'c/e/', 3)
         data.set(d1, 'c/f', [ 1 ] )
         self.assertEqual(d1, d12)
+        data.set(d1, 'c/f[0]', 2 )
+        self.assertEqual(d1, d13)
+
+        data.set(d1, 'c/f[0]', { 'g': [ 1, 2] } )
+        self.assertEqual(d1, d14)
+        data.set(d1, 'c/f[0]/g[1]', 3)
+        self.assertEqual(d1, d15)
+        
         self.assertRaises(data.DataTypeError, data.set, d1, 1, 2)
         self.assertRaises(data.DataTypeError, data.set, 1, "", 2)
+        self.assertRaises(data.DataTypeError, data.set, d1, 'c[1]', 2)
+        self.assertRaises(data.DataTypeError, data.set, d1, 'c[1][2]', 2)
+        self.assertRaises(data.DataNotFoundError, data.set, d1, 'c/f[5]', 2)
+        self.assertRaises(data.DataNotFoundError, data.set, d1, 'c/f[5][2]', 2)
+
         d3 = {}
         e3 = data.set(d3, "does/not/exist", 123)
         self.assertEqual(d3,
@@ -114,11 +186,25 @@ class TestData(unittest.TestCase):
                          { 'does': { 'not': { 'exist': 123 } } })
 
     def test_unset(self):
-        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': [ 1, 2, 3 ] } }
         data.unset(d1, 'a')
         data.unset(d1, 'c/d')
         data.unset(d1, 'does/not/exist')
-        self.assertEqual(d1, { 'b': 1, 'c': { 'e': 2 } })
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 1, 2, 3 ] } })
+        data.unset(d1, 'c/e[0]')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 2, 3 ] } })
+        data.unset(d1, 'c/e[1]')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 2 ] } })
+        # index 1 should now be out of range
+        self.assertRaises(data.DataNotFoundError, data.unset, d1, 'c/e[1]')
+        d2 = { 'a': [ { 'b': [ 1, 2 ] } ] }
+        data.unset(d2, 'a[0]/b[1]')
+        self.assertEqual(d2, { 'a': [ { 'b': [ 1 ] } ] })
+        d3 = { 'a': [ [ 1, 2 ] ] }
+        data.set(d3, "a[0][1]", 3)
+        self.assertEqual(d3, { 'a': [ [ 1, 3 ] ] })
+        data.unset(d3, 'a[0][1]')
+        self.assertEqual(d3, { 'a': [ [ 1 ] ] })
         
     def test_find_no_exc(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
@@ -146,6 +232,9 @@ class TestData(unittest.TestCase):
         self.assertEqual(data.parse_value_str("{ \"a\": \"b\", \"c\": 1 }"), { 'a': 'b', 'c': 1 })
         self.assertEqual(data.parse_value_str("[ a c"), "[ a c")
 
+        self.assertEqual(data.parse_value_str(1), None)
+
+
 if __name__ == '__main__':
     #if not 'CONFIG_TESTDATA_PATH' in os.environ:
     #    print("You need to set the environment variable CONFIG_TESTDATA_PATH to point to the directory containing the test data files")

src/lib/python/isc/config/ccsession.py

@@ -398,16 +398,25 @@ class UIModuleCCSession(MultiConfigData):
         module_spec = self.find_spec_part(identifier)
         if (type(module_spec) != dict or "list_item_spec" not in module_spec):
             raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
-        value = isc.cc.data.parse_value_str(value_str)
-        isc.config.config_data.check_type(module_spec, [value])
-        cur_list, status = self.get_value(identifier)
-        #if not cur_list:
-        #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
-        if not cur_list:
-            cur_list = []
-        if value in cur_list:
-            cur_list.remove(value)
-        self.set_value(identifier, cur_list)
+
+        if value_str is None:
+            # we are directly removing an list index
+            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
+            if list_indices is None:
+                raise DataTypeError("identifier in remove_value() does not contain a list index, and no value to remove")
+            else:
+                self.set_value(identifier, None)
+        else:
+            value = isc.cc.data.parse_value_str(value_str)
+            isc.config.config_data.check_type(module_spec, [value])
+            cur_list, status = self.get_value(identifier)
+            #if not cur_list:
+            #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
+            if not cur_list:
+                cur_list = []
+            if value in cur_list:
+                cur_list.remove(value)
+            self.set_value(identifier, cur_list)
 
     def commit(self):
         """Commit all local changes, send them through b10-cmdctl to

src/lib/python/isc/config/config_data.py

@@ -22,6 +22,7 @@ two through the classes in ccsession)
 
 import isc.cc.data
 import isc.config.module_spec
+import ast
 
 class ConfigDataError(Exception): pass
 
@@ -56,14 +57,14 @@ def check_type(spec_part, value):
         raise isc.cc.data.DataTypeError(str(value) + " is not a map")
 
 def convert_type(spec_part, value):
-    """Convert the give value(type is string) according specification 
+    """Convert the given value(type is string) according specification 
     part relevant for the value. Raises an isc.cc.data.DataTypeError 
     exception if conversion failed.
     """
     if type(spec_part) == dict and 'item_type' in spec_part:
         data_type = spec_part['item_type']
     else:
-        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type convering"))
+        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type conversion"))
    
     try:
         if data_type == "integer":
@@ -81,18 +82,25 @@ def convert_type(spec_part, value):
                     ret.append(convert_type(spec_part['list_item_spec'], item))
             elif type(value) == str:    
                 value = value.split(',')
-                for item in value:    
+                for item in value:
                     sub_value = item.split()
                     for sub_item in sub_value:
-                        ret.append(convert_type(spec_part['list_item_spec'], sub_item))
+                        ret.append(convert_type(spec_part['list_item_spec'],
+                                                sub_item))
 
             if ret == []:
                 raise isc.cc.data.DataTypeError(str(value) + " is not a list")
 
             return ret
         elif data_type == "map":
-            return dict(value)
-            # todo: check types of map contents too
+            map = ast.literal_eval(value)
+            if type(map) == dict:
+                # todo: check types of map contents too
+                return map
+            else:
+                raise isc.cc.data.DataTypeError(
+                           "Value in convert_type not a string "
+                           "specifying a dict")
         else:
             return value
     except ValueError as err:
@@ -108,7 +116,11 @@ def find_spec_part(element, identifier):
     id_parts = identifier.split("/")
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
-    for id in id_parts:
+
+    for id_part in id_parts:
+        # strip list selector part
+        # don't need it for the spec part, so just drop it
+        id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
         if type(cur_el) == dict and 'map_item_spec' in cur_el.keys():
             found = False
             for cur_el_item in cur_el['map_item_spec']:
@@ -158,11 +170,9 @@ def spec_name_list(spec, prefix="", recurse=False):
                     result.extend(spec_name_list(list_el['map_item_spec'], prefix + list_el['item_name'], recurse))
                 else:
                     name = list_el['item_name']
-                    if list_el['item_type'] in ["list", "map"]:
-                        name += "/"
                     result.append(prefix + name)
             else:
-                raise ConfigDataError("Bad specication")
+                raise ConfigDataError("Bad specification")
     else:
         raise ConfigDataError("Bad specication")
     return result
@@ -228,6 +238,20 @@ class ConfigData:
             result[item] = value
         return result
 
+# should we just make a class for these?
+def _create_value_map_entry(name, type, value, status = None):
+    entry = {}
+    entry['name'] = name
+    entry['type'] = type
+    entry['value'] = value
+    entry['modified'] = False
+    entry['default'] = False
+    if status == MultiConfigData.LOCAL:
+        entry['modified'] = True
+    if status == MultiConfigData.DEFAULT:
+        entry['default'] = True
+    return entry
+
 class MultiConfigData:
     """This class stores the module specs, current non-default
        configuration values and 'local' (uncommitted) changes for
@@ -272,7 +296,7 @@ class MultiConfigData:
            identifier (up to the first /) is interpreted as the module
            name. Returns None if not found, or if identifier is not a
            string."""
-        if type(identifier) != str:
+        if type(identifier) != str or identifier == "":
             return None
         if identifier[0] == '/':
             identifier = identifier[1:]
@@ -336,28 +360,42 @@ class MultiConfigData:
         try:
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
-                return spec['item_default']
+                id, list_indices = isc.cc.data.split_identifier_list_indices(id)
+                if list_indices is not None and \
+                   type(spec['item_default']) == list:
+                    if len(list_indices) == 1:
+                        default_list = spec['item_default']
+                        index = list_indices[0]
+                        if index < len(default_list):
+                            return default_list[index]
+                        else:
+                            return None
+                else:
+                    return spec['item_default']
             else:
                 return None
         except isc.cc.data.DataNotFoundError as dnfe:
             return None
 
-    def get_value(self, identifier):
+    def get_value(self, identifier, default = True):
         """Returns a tuple containing value,status.
            The value contains the configuration value for the given
            identifier. The status reports where this value came from;
            it is one of: LOCAL, CURRENT, DEFAULT or NONE, corresponding
            (local change, current setting, default as specified by the
-           specification, or not found at all)."""
+           specification, or not found at all). Does not check and
+           set DEFAULT if the argument 'default' is False (default
+           defaults to True)"""
         value = self.get_local_value(identifier)
         if value != None:
             return value, self.LOCAL
         value = self.get_current_value(identifier)
         if value != None:
             return value, self.CURRENT
-        value = self.get_default_value(identifier)
-        if value != None:
-            return value, self.DEFAULT
+        if default:
+            value = self.get_default_value(identifier)
+            if value != None:
+                return value, self.DEFAULT
         return None, self.NONE
 
     def get_value_maps(self, identifier = None):
@@ -374,12 +412,7 @@ class MultiConfigData:
         if not identifier:
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
-                entry = {}
-                entry['name'] = module
-                entry['type'] = 'module'
-                entry['value'] = None
-                entry['modified'] = False
-                entry['default'] = False
+                entry = _create_value_map_entry(module, 'module', None)
                 result.append(entry)
         else:
             if identifier[0] == '/':
@@ -389,51 +422,41 @@ class MultiConfigData:
             if spec:
                 spec_part = find_spec_part(spec.get_config_spec(), id)
                 if type(spec_part) == list:
+                    # list of items to show
                     for item in spec_part:
-                        entry = {}
-                        entry['name'] = item['item_name']
-                        entry['type'] = item['item_type']
-                        value, status = self.get_value("/" + identifier + "/" + item['item_name'])
-                        entry['value'] = value
-                        if status == self.LOCAL:
-                            entry['modified'] = True
-                        else:
-                            entry['modified'] = False
-                        if status == self.DEFAULT:
-                            entry['default'] = False
-                        else:
-                            entry['default'] = False
+                        value, status = self.get_value("/" + identifier\
+                                              + "/" + item['item_name'])
+                        entry = _create_value_map_entry(item['item_name'],
+                                                        item['item_type'],
+                                                        value, status)
                         result.append(entry)
                 elif type(spec_part) == dict:
+                    # Sub-specification
                     item = spec_part
                     if item['item_type'] == 'list':
                         li_spec = item['list_item_spec']
-                        item_list, status =  self.get_value("/" + identifier)
-                        if item_list != None:
-                            for value in item_list:
-                                result_part2 = {}
-                                result_part2['name'] = li_spec['item_name']
-                                result_part2['value'] = value
-                                result_part2['type'] = li_spec['item_type']
-                                result_part2['default'] = False
-                                result_part2['modified'] = False
+                        value, status =  self.get_value("/" + identifier)
+                        if type(value) == list:
+                            for list_value in value:
+                                result_part2 = _create_value_map_entry(
+                                                   li_spec['item_name'],
+                                                   li_spec['item_type'],
+                                                   list_value)
                                 result.append(result_part2)
+                        elif value is not None:
+                            entry = _create_value_map_entry(
+                                        li_spec['item_name'],
+                                        li_spec['item_type'],
+                                        value, status)
+                            result.append(entry)
                     else:
-                        entry = {}
-                        entry['name'] = item['item_name']
-                        entry['type'] = item['item_type']
-                        #value, status = self.get_value("/" + identifier + "/" + item['item_name'])
                         value, status = self.get_value("/" + identifier)
-                        entry['value'] = value
-                        if status == self.LOCAL:
-                            entry['modified'] = True
-                        else:
-                            entry['modified'] = False
-                        if status == self.DEFAULT:
-                            entry['default'] = False
-                        else:
-                            entry['default'] = False
-                        result.append(entry)
+                        if value is not None:
+                            entry = _create_value_map_entry(
+                                        item['item_name'],
+                                        item['item_type'],
+                                        value, status)
+                            result.append(entry)
         return result
 
     def set_value(self, identifier, value):
@@ -441,8 +464,28 @@ class MultiConfigData:
            there is a specification for the given identifier, the type
            is checked."""
         spec_part = self.find_spec_part(identifier)
-        if spec_part != None:
+        if spec_part is not None and value is not None:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
+            if list_indices is not None \
+               and spec_part['item_type'] == 'list':
+                spec_part = spec_part['list_item_spec']
             check_type(spec_part, value)
+
+        # Since we do not support list diffs (yet?), we need to
+        # copy the currently set list of items to _local_changes
+        # if we want to modify an element in there
+        # (for any list indices specified in the full identifier)
+        id_parts = isc.cc.data.split_identifier(identifier)
+        cur_id_part = '/'
+        for id_part in id_parts:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+            if list_indices is not None:
+                cur_list, status = self.get_value(cur_id_part + id)
+                if status != MultiConfigData.LOCAL:
+                    isc.cc.data.set(self._local_changes,
+                                    cur_id_part + id,
+                                    cur_list)
+            cur_id_part = cur_id_part + id_part + "/"
         isc.cc.data.set(self._local_changes, identifier, value)
  
     def get_config_item_list(self, identifier = None, recurse = False):

src/lib/python/isc/config/tests/ccsession_test.py

@@ -637,6 +637,8 @@ class TestUIModuleCCSession(unittest.TestCase):
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
+        uccs.remove_value("Spec2/item5[0]", None)
+        self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 
     def test_commit(self):
         fake_conn = fakeUIConn()

src/lib/python/isc/config/tests/config_data_test.py

@@ -107,6 +107,8 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ 1, 2 ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, 1, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, { 'somedict': 'somevalue' }, "a")
         
         spec_part = find_spec_part(config_spec, "value2")
         self.assertEqual(1.1, convert_type(spec_part, '1.1'))
@@ -146,6 +148,18 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
 
+        spec_part = find_spec_part(config_spec, "value6")
+        self.assertEqual({}, convert_type(spec_part, '{}'))
+        self.assertEqual({ 'v61': 'a' }, convert_type(spec_part, '{ \'v61\': \'a\' }'))
+
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, 1.1)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, True)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "1")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "a", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+
         spec_part = find_spec_part(config_spec, "value7")
         self.assertEqual(['1', '2'], convert_type(spec_part, '1, 2'))
         self.assertEqual(['1', '2', '3'], convert_type(spec_part, '1 2  3'))
@@ -175,9 +189,9 @@ class TestConfigData(unittest.TestCase):
 
     def test_spec_name_list(self):
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec())
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec(), "", True)
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/value1', 'item6/value2'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6/value1', 'item6/value2'], name_list)
         spec_part = find_spec_part(self.cd.get_module_spec().get_config_spec(), "item6")
         name_list = spec_name_list(spec_part, "item6", True)
         self.assertEqual(['item6/value1', 'item6/value2'], name_list)
@@ -193,7 +207,7 @@ class TestConfigData(unittest.TestCase):
         name_list = spec_name_list({ "myModule": config_spec }, "", False)
         self.assertEqual(['myModule/'], name_list)
         name_list = spec_name_list({ "myModule": config_spec }, "", True)
-        self.assertEqual(['myModule/', 'myModule/value1', 'myModule/value2', 'myModule/value3', 'myModule/value4', 'myModule/value5/', 'myModule/value6/v61', 'myModule/value6/v62', 'myModule/value7/', 'myModule/value8/', 'myModule/value9/v91', 'myModule/value9/v92/v92a', 'myModule/value9/v92/v92b'], name_list)
+        self.assertEqual(['myModule/', 'myModule/value1', 'myModule/value2', 'myModule/value3', 'myModule/value4', 'myModule/value5', 'myModule/value6/v61', 'myModule/value6/v62', 'myModule/value7', 'myModule/value8', 'myModule/value9/v91', 'myModule/value9/v92/v92a', 'myModule/value9/v92/v92b'], name_list)
 
         self.assertRaises(ConfigDataError, spec_name_list, 1)
         self.assertRaises(ConfigDataError, spec_name_list, [ 'a' ])
@@ -240,19 +254,19 @@ class TestConfigData(unittest.TestCase):
 
     def test_get_item_list(self):
         name_list = self.cd.get_item_list()
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
         name_list = self.cd.get_item_list("", True)
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/value1', 'item6/value2'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6/value1', 'item6/value2'], name_list)
         name_list = self.cd.get_item_list("item6", False)
         self.assertEqual(['item6/value1', 'item6/value2'], name_list)
 
     def test_get_full_config(self):
         full_config = self.cd.get_full_config()
-        self.assertEqual({ "item1": 1, "item2": 1.1, "item3": True, "item4": "test", "item5/": ['a', 'b'], "item6/value1": 'default', 'item6/value2': None}, full_config)
+        self.assertEqual({ "item1": 1, "item2": 1.1, "item3": True, "item4": "test", "item5": ['a', 'b'], "item6/value1": 'default', 'item6/value2': None}, full_config)
         my_config = { "item1": 2, "item2": 2.2, "item3": False, "item4": "asdf", "item5": [ "c", "d" ] }
         self.cd.set_local_config(my_config)
         full_config = self.cd.get_full_config()
-        self.assertEqual({ "item1": 2, "item2": 2.2, "item3": False, "item4": "asdf", "item5/": [ "c", "d" ], "item6/value1": 'default', 'item6/value2': None}, full_config)
+        self.assertEqual({ "item1": 2, "item2": 2.2, "item3": False, "item4": "asdf", "item5": [ "c", "d" ], "item6/value1": 'default', 'item6/value2': None}, full_config)
 
 class TestMultiConfigData(unittest.TestCase):
     def setUp(self):
@@ -342,6 +356,12 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(1, value)
         value = self.mcd.get_default_value("/Spec2/item1")
         self.assertEqual(1, value)
+        value = self.mcd.get_default_value("Spec2/item5[0]")
+        self.assertEqual('a', value)
+        value = self.mcd.get_default_value("Spec2/item5[5]")
+        self.assertEqual(None, value)
+        value = self.mcd.get_default_value("Spec2/item5[0][1]")
+        self.assertEqual(None, value)
         value = self.mcd.get_default_value("Spec2/item6/value1")
         self.assertEqual('default', value)
         value = self.mcd.get_default_value("Spec2/item6/value2")
@@ -353,20 +373,34 @@ class TestMultiConfigData(unittest.TestCase):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
-        value,status = self.mcd.get_value("Spec2/item1")
+
+        value, status = self.mcd.get_value("Spec2/item1")
         self.assertEqual(2, value)
         self.assertEqual(MultiConfigData.LOCAL, status)
-        value,status = self.mcd.get_value("Spec2/item2")
+
+        value, status = self.mcd.get_value("Spec2/item2")
         self.assertEqual(1.1, value)
         self.assertEqual(MultiConfigData.DEFAULT, status)
+
         self.mcd._current_config = { "Spec2": { "item3": False } }
-        value,status = self.mcd.get_value("Spec2/item3")
+
+        value, status = self.mcd.get_value("Spec2/item3")
         self.assertEqual(False, value)
         self.assertEqual(MultiConfigData.CURRENT, status)
-        value,status = self.mcd.get_value("Spec2/no_such_item")
+
+        value, status = self.mcd.get_value("Spec2/no_such_item")
+        self.assertEqual(None, value)
+        self.assertEqual(MultiConfigData.NONE, status)
+
+        value, status = self.mcd.get_value("Spec2/item5[0]")
+        self.assertEqual("a", value)
+        self.assertEqual(MultiConfigData.DEFAULT, status)
+
+        value, status = self.mcd.get_value("Spec2/item5[0]", False)
         self.assertEqual(None, value)
         self.assertEqual(MultiConfigData.NONE, status)
 
+
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         self.assertEqual([], maps)
@@ -390,29 +424,31 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd.set_value("Spec2/item3", False)
         maps = self.mcd.get_value_maps("/Spec2")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
-                          {'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
                           {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
-                          {'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
-                          {'default': False, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': False, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("Spec2")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
-                          {'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
                           {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
-                          {'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
-                          {'default': False, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': False, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item5")
         self.assertEqual([{'default': False, 'type': 'string', 'name': 'list_element', 'value': 'a', 'modified': False},
                           {'default': False, 'type': 'string', 'name': 'list_element', 'value': 'b', 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec2/item5[0]")
+        self.assertEqual([{'default': True, 'modified': False, 'name': 'list_element', 'type': 'string', 'value': 'a'}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item1")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item2")
-        self.assertEqual([{'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item3")
         self.assertEqual([{'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item4")
-        self.assertEqual([{'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
 
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec24.spec")
         self.mcd.set_specification(module_spec)
@@ -429,7 +465,19 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
         self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item1", "asdf")
+
         self.mcd.set_value("Spec2/no_such_item", 4)
+        value, status = self.mcd.get_value("Spec2/no_such_item")
+        self.assertEqual(value, 4)
+        self.assertEqual(MultiConfigData.LOCAL, status)
+
+        self.mcd.set_value("Spec2/item5[0]", "c")
+        value, status = self.mcd.get_value("Spec2/item5[0]")
+        self.assertEqual(value, "c")
+        self.assertEqual(MultiConfigData.LOCAL, status)
+
+        self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item5[a]", "asdf")
+        
 
     def test_get_config_item_list(self):
         config_items = self.mcd.get_config_item_list()
@@ -441,13 +489,15 @@ class TestMultiConfigData(unittest.TestCase):
         config_items = self.mcd.get_config_item_list(None, False)
         self.assertEqual(['Spec2'], config_items)
         config_items = self.mcd.get_config_item_list(None, True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2", True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2")
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
+        config_items = self.mcd.get_config_item_list("/Spec2")
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2", True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
 
 if __name__ == '__main__':
     unittest.main()

src/lib/python/isc/datasrc/Makefile.am

@@ -1,3 +1,5 @@
+SUBDIRS = . tests
+
 python_PYTHON = __init__.py master.py sqlite3_ds.py
 
 pythondir = $(pyexecdir)/isc/datasrc

src/lib/python/isc/datasrc/master.py

@@ -103,7 +103,7 @@ def isname(s):
 # isttl: check whether a string is a valid TTL specifier.
 # returns: boolean
 #########################################################################
-ttl_regex = re.compile('([0-9]+[wdhms]?)+', re.I)
+ttl_regex = re.compile('([0-9]+[wdhms]?)+$', re.I)
 def isttl(s):
     global ttl_regex
     if ttl_regex.match(s):

src/lib/python/isc/datasrc/tests/Makefile.am

@@ -0,0 +1,12 @@
+PYTESTS = master_test.py
+EXTRA_DIST = $(PYTESTS)
+
+# later will have configure option to choose this, like: coverage run --branch
+PYCOVERAGE = $(PYTHON)
+# test using command-line arguments, so use check-local target instead of TESTS
+check-local:
+	for pytest in $(PYTESTS) ; do \
+	echo Running test: $$pytest ; \
+	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log \
+	$(PYCOVERAGE) $(abs_srcdir)/$$pytest || exit ; \
+	done

src/lib/python/isc/datasrc/tests/master_test.py

@@ -0,0 +1,35 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+from isc.datasrc.master import *
+import unittest
+
+class TestTTL(unittest.TestCase):
+    def test_ttl(self):
+        self.assertTrue(isttl('3600'))
+        self.assertTrue(isttl('1W'))
+        self.assertTrue(isttl('1w'))
+        self.assertTrue(isttl('2D'))
+        self.assertTrue(isttl('2d'))
+        self.assertTrue(isttl('30M'))
+        self.assertTrue(isttl('30m'))
+        self.assertTrue(isttl('10S'))
+        self.assertTrue(isttl('10s'))
+        self.assertTrue(isttl('2W1D'))
+        self.assertFalse(isttl('not a ttl'))
+        self.assertFalse(isttl('1X'))
+
+if __name__ == '__main__':
+    unittest.main()

src/lib/python/isc/notify/notify_out.py

@@ -367,6 +367,7 @@ class NotifyOut:
                     zone_id = self._waiting_zones.pop(0) 
                     self._notify_infos[zone_id].prepare_notify_out()
                     self.notify_num += 1 
+                    self._notifying_zones.append(zone_id)
 
     def _send_notify_message_udp(self, zone_notify_info, addrinfo):
         msg, qid = self._create_notify_message(zone_notify_info.zone_name, 

src/lib/python/isc/notify/tests/notify_out_test.py

@@ -156,7 +156,7 @@ class TestNotifyOut(unittest.TestCase):
         com_info = self._notify._notify_infos[('com.', 'IN')]
         self._notify._notify_next_target(com_info)
         self.assertEqual(2, self._notify.notify_num)
-        self.assertEqual(0, len(self._notify._notifying_zones))
+        self.assertEqual(2, len(self._notify._notifying_zones))
     
     def test_handle_notify_reply(self):
         self.assertEqual(notify_out._BAD_REPLY_PACKET, self._notify._handle_notify_reply(None, b'badmsg'))

src/lib/python/isc/utils/Makefile.am

@@ -0,0 +1,5 @@
+SUBDIRS = tests
+
+python_PYTHON = __init__.py process.py
+
+pythondir = $(pyexecdir)/isc/utils

src/lib/python/isc/utils/process.py

@@ -0,0 +1,37 @@
+# Copyright (C) 2010  CZ NIC
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+"""
+Module to manipulate the python processes.
+
+It contains only function to rename the process, which is currently
+wrapper around setproctitle library. Does not fail if the setproctitle
+module is missing, but does nothing in that case.
+"""
+try:
+    from setproctitle import setproctitle
+except ImportError:
+    def setproctitle(_): pass
+import sys
+import os.path
+
+"""
+Rename the current process to given name (so it can be found in ps).
+If name is None, use zero'th command line argument.
+"""
+def rename(name=None):
+    if name is None:
+        name = os.path.basename(sys.argv[0])
+    setproctitle(name)

src/lib/python/isc/utils/tests/Makefile.am

@@ -0,0 +1,12 @@
+PYTESTS = process_test.py
+EXTRA_DIST = $(PYTESTS)
+
+# later will have configure option to choose this, like: coverage run --branch
+PYCOVERAGE = $(PYTHON)
+# test using command-line arguments, so use check-local target instead of TESTS
+check-local:
+	for pytest in $(PYTESTS) ; do \
+	echo Running test: $$pytest ; \
+	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/python/.libs \
+	$(PYCOVERAGE) $(abs_srcdir)/$$pytest || exit ; \
+	done

src/lib/python/isc/utils/tests/process_test.py

@@ -0,0 +1,39 @@
+# Copyright (C) 2010  CZ NIC
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+"""Tests for isc.utils.process."""
+import unittest
+import isc.utils.process
+run_tests = True
+try:
+    import setproctitle
+except ImportError:
+    run_tests = False
+
+class TestRename(unittest.TestCase):
+    """Testcase for isc.process.rename."""
+    def __get_self_name(self):
+        return setproctitle.getproctitle()
+
+    @unittest.skipIf(not run_tests, "Setproctitle not installed, not testing")
+    def test_rename(self):
+        """Test if the renaming function works."""
+        isc.utils.process.rename("rename-test")
+        self.assertEqual("rename-test", self.__get_self_name())
+        isc.utils.process.rename()
+        self.assertEqual("process_test.py", self.__get_self_name())
+
+if __name__ == "__main__":
+    unittest.main()