Parcourir la source

Merge branch 'master' into trac1756_2

Mukund Sivaraman il y a 12 ans
Parent
commit
9b3c959af1
73 fichiers modifiés avec 2700 ajouts et 1614 suppressions
  1. 51 13
      ChangeLog
  2. 395 283
      doc/guide/bind10-guide.xml
  3. 7 6
      src/bin/auth/query.cc
  4. 50 32
      src/bin/auth/tests/query_unittest.cc
  5. 107 68
      src/bin/bindctl/bindcmd.py
  6. 114 11
      src/bin/bindctl/tests/bindctl_test.py
  7. 33 20
      src/bin/cmdctl/cmdctl.py.in
  8. 7 0
      src/bin/cmdctl/cmdctl_messages.mes
  9. 1 1
      src/bin/cmdctl/tests/Makefile.am
  10. 115 45
      src/bin/cmdctl/tests/cmdctl_test.py
  11. 16 15
      src/bin/dhcp4/config_parser.cc
  12. 60 10
      src/bin/dhcp4/ctrl_dhcp4_srv.cc
  13. 21 0
      src/bin/dhcp4/ctrl_dhcp4_srv.h
  14. 1 1
      src/bin/dhcp4/dhcp4.spec
  15. 86 19
      src/bin/dhcp4/dhcp4_srv.cc
  16. 16 2
      src/bin/dhcp4/dhcp4_srv.h
  17. 0 4
      src/bin/dhcp4/tests/config_parser_unittest.cc
  18. 180 10
      src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
  19. 13 12
      src/bin/dhcp6/config_parser.cc
  20. 65 14
      src/bin/dhcp6/ctrl_dhcp6_srv.cc
  21. 21 0
      src/bin/dhcp6/ctrl_dhcp6_srv.h
  22. 1 1
      src/bin/dhcp6/dhcp6.spec
  23. 1 1
      src/bin/dhcp6/dhcp6_srv.cc
  24. 1 4
      src/bin/dhcp6/tests/config_parser_unittest.cc
  25. 4 0
      src/bin/xfrin/xfrin_messages.mes
  26. 4 2
      src/lib/datasrc/Makefile.am
  27. 2 1
      src/lib/datasrc/client.h
  28. 1 1
      src/lib/datasrc/database.cc
  29. 1 1
      src/lib/datasrc/memory/memory_client.h
  30. 1 1
      src/lib/datasrc/memory/zone_data_loader.h
  31. 1 1
      src/lib/datasrc/memory/zone_finder.cc
  32. 1 1
      src/lib/datasrc/memory/zone_finder.h
  33. 2 1
      src/lib/datasrc/memory_datasrc.cc
  34. 1 0
      src/lib/datasrc/rrset_collection_base.cc
  35. 1 1
      src/lib/datasrc/tests/client_list_unittest.cc
  36. 234 31
      src/lib/datasrc/tests/database_unittest.cc
  37. 1 1
      src/lib/datasrc/tests/faked_nsec3.h
  38. 1 1
      src/lib/datasrc/tests/memory_datasrc_unittest.cc
  39. 1 1
      src/lib/datasrc/tests/zone_finder_context_unittest.cc
  40. 2 713
      src/lib/datasrc/zone.h
  41. 93 2
      src/lib/datasrc/zone_finder.cc
  42. 809 0
      src/lib/datasrc/zone_finder.h
  43. 1 1
      src/lib/datasrc/zone_finder_context.cc
  44. 0 0
      src/lib/datasrc/zone_iterator.h
  45. 1 1
      src/lib/datasrc/zone_loader.cc
  46. 1 1
      src/lib/datasrc/zonetable.h
  47. 1 9
      src/lib/dhcp/libdhcp++.cc
  48. 0 10
      src/lib/dhcp/libdhcp++.h
  49. 8 54
      src/lib/dhcp/option.cc
  50. 1 23
      src/lib/dhcp/option.h
  51. 1 1
      src/lib/dhcp/option4_addrlst.cc
  52. 6 4
      src/lib/dhcp/option4_addrlst.h
  53. 3 22
      src/lib/dhcp/option_custom.cc
  54. 5 12
      src/lib/dhcp/option_custom.h
  55. 4 10
      src/lib/dhcp/option_definition.cc
  56. 24 0
      src/lib/dhcp/option_int_array.h
  57. 5 6
      src/lib/dhcp/pkt4.cc
  58. 3 3
      src/lib/dhcp/pkt6.cc
  59. 1 1
      src/lib/dhcp/tests/libdhcp++_unittest.cc
  60. 2 2
      src/lib/dhcp/tests/option4_addrlst_unittest.cc
  61. 74 95
      src/lib/dhcp/tests/option_int_array_unittest.cc
  62. 7 7
      src/lib/dhcp/tests/option_unittest.cc
  63. 2 2
      src/lib/dhcpsrv/alloc_engine.h
  64. 1 1
      src/lib/dhcpsrv/dbaccess_parser.cc
  65. 12 2
      src/lib/dhcpsrv/lease_mgr.h
  66. 1 1
      src/lib/dhcpsrv/option_space_container.h
  67. 6 7
      src/lib/dhcpsrv/subnet.cc
  68. 2 2
      src/lib/dhcpsrv/subnet.h
  69. 2 2
      src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc
  70. 1 1
      src/lib/python/isc/datasrc/client_python.cc
  71. 1 1
      src/lib/python/isc/datasrc/finder_python.cc
  72. 1 1
      src/lib/python/isc/datasrc/iterator_python.cc
  73. 1 1
      src/lib/python/isc/datasrc/updater_python.cc

+ 51 - 13
ChangeLog

@@ -1,3 +1,41 @@
+560.	[bug]		jinmei
+	b10-auth now sets the TTL of SOA RR for negative responses to
+	the minimum of the RR TTL and the minimum TTL of the SOA RDATA
+	as specified in RFC2308; previously the RR TTL was always used.
+	The ZoneFinder class was extended partly for implementing this
+	and partly for allowing further optimization.
+	(Trac #2309 and #2635, git ee17e979fcde48b59d91c74ac368244169065f3b)
+
+559.	[bug]		jelte
+	b10-cmdctl no longer aborts on basic file issues with its https
+	certificate or private key file. It performs additional checks, and
+	provides better error logs if these fail. Additionally, bindctl
+	provides a better error report if it is unable to connect over
+	https connection. This issue could occur if BIND 10 was installed
+	with root privileges but then started as a normal user.
+	(Trac #2595, git 09b1a2f927483b407d70e98f5982f424cc872149)
+
+558.	[func]		marcin
+	b10-dhcp4: server now adds configured options to its
+	responses to a client when client requests them.
+	A few basic options: Routers, Domain Name, Domain
+	Name Servers and Subnet Mask are added regardless
+	if client requested them or not.
+	(Trac #2591, git aeec2dc1b9c511d17971ac63138576c37e7c5164)
+
+557.	[doc]		stephen
+	Update DHCP sections of the BIND 10 guide.
+	(Trac #2642, git e5faeb5fa84b7218fde486347359504cf692510e)
+
+556.	[bug]		marcin
+	Fixed DHCP servers configuration whereby the servers did not
+	receive a configuration stored in the database on their startup.
+	Also, the configuration handler function now uses full configuration
+	instead of partial to configure the server. This guarantees that
+	dependencies between various configuration parameters are
+	fulfilled.
+	(Trac #2637, git 91aa998226f1f91a232f2be59a53c9568c4ece77)
+
 555.	[func]		marcin
 	The encapsulated option space name can be specified for
 	a DHCP option. It comprises sub-options being sent within
@@ -12,20 +50,20 @@
 	to complete.
 	(Trac #2574, git 5b8a824054313bdecb8988b46e55cb2e94cb2d6c)
 
-553.    [func]      stephen
+553.	[func]		stephen
 	Values of the parameters to access the DHCP server lease database
 	can now be set through the BIND 10 configuration mechanism.
 	(Trac #2559, git 6c6f405188cc02d2358e114c33daff58edabd52a)
 
-552.    [bug]       shane
-    Build on Raspberry PI.
+552.	[bug]		shane
+	Build on Raspberry PI.
 	The main issue was use of char for reading from input streams,
 	which is incorrect, as EOF is returned as an int -1, which would
 	then get cast into a char -1.
 	A number of other minor issues were also fixed.
 	(Trac #2571, git 525333e187cc4bbbbde288105c9582c1024caa4a)
 
-551.    [bug]       shane
+551.	[bug]		shane
 	Kill msgq if we cannot connect to it on startup.
 	When the boss process was unable to connect to the msgq, it would
 	exit. However, it would leave the msgq process running. This has
@@ -89,8 +127,8 @@
 543.	[func]*		jelte
 	When calling getFullConfig() as a module, , the configuration is now
 	returned as properly-structured JSON.  Previously, the structure had
-	been flattened, with all data being labelled by fully-qualified element
-	names.
+	been flattened, with all data being labelled by fully-qualified
+	element names.
 	(Trac #2619, git bed3c88c25ea8f7e951317775e99ebce3340ca22)
 
 542.	[func]		marcin
@@ -154,7 +192,7 @@
 	compile-time option --enable-debug.
 	(Trac #1081, git db55f102b30e76b72b134cbd77bd183cd01f95c0)
 
-534.	[func]*			vorner
+534.	[func]*		vorner
 	The b10-msgq now uses the same logging format as the rest
 	of the system. However, it still doesn't obey the common
 	configuration, as due to technical issues it is not able
@@ -2751,7 +2789,7 @@ bind10-devel-20110224 released on February 24, 2011
 	(Trac #496, git b9296ca023cc9e76cda48a7eeebb0119166592c5)
 
 160.	[func]		jelte
-  	Updated the resolver to take 3 different timeout values;
+	Updated the resolver to take 3 different timeout values;
 	timeout_query for outstanding queries we sent while resolving
 	timeout_client for sending an answer back to the client
 	timeout_lookup for stopping the resolving
@@ -2930,7 +2968,7 @@ bind10-devel-20110120 released on January 20, 2011
 	(Trac #226, svn r3989)
 
 136.	[bug]		jelte
-  	bindctl (and the configuration manager in general) now no longer
+	bindctl (and the configuration manager in general) now no longer
 	accepts 'unknown' data; i.e. data for modules that it does not know
 	about, or configuration items that are not specified in the .spec
 	files.
@@ -3172,7 +3210,7 @@ bind10-devel-20100917 released on September 17, 2010
 	(Trac #342, svn r2949)
 
 94.	[bug]		jelte
-  	bin/xfrout:  Fixed a problem in xfrout where only 2 or 3 RRs
+	bin/xfrout:  Fixed a problem in xfrout where only 2 or 3 RRs
 	were used per DNS message in the xfrout stream.
 	(Trac #334, r2931)
 
@@ -3306,7 +3344,7 @@ bind10-devel-20100812 released on August 12, 2010
 	module. (Trac #275, r2459)
 
 73.	[bug]		jelte
-  	Fixed a bug where in bindctl, locally changed settings were
+	Fixed a bug where in bindctl, locally changed settings were
 	reset when the list of running modules is updated. (Trac #285,
 	r2452)
 
@@ -3317,11 +3355,11 @@ bind10-devel-20100812 released on August 12, 2010
 	known such platform. (Trac #148, r2427)
 
 71.	[func]		each
-  	Add "-a" (address) option to bind10 to specify an address for
+	Add "-a" (address) option to bind10 to specify an address for
 	the auth server to listen on.
 
 70.	[func]		each
-  	Added a hot-spot cache to libdatasrc to speed up access to
+	Added a hot-spot cache to libdatasrc to speed up access to
 	repeatedly-queried data and reduce the number of queries to
 	the underlying database; this should substantially improve
 	performance.  Also added a "-n" ("no cache") option to

+ 395 - 283
doc/guide/bind10-guide.xml

@@ -740,6 +740,15 @@ as a dependency earlier -->
             </listitem>
           </varlistentry>
 
+          <varlistentry>
+            <term>--with-dhcp-mysql</term>
+            <listitem>
+              <simpara>Enable MySQL support for BIND 10 DHCP. For notes on configuring
+              and building DHCP with MySQL see <xref linkend="dhcp-install-configure">.</xref>
+              </simpara>
+            </listitem>
+          </varlistentry>
+
           </variablelist>
 
         </para>
@@ -761,9 +770,7 @@ as a dependency earlier -->
           dependencies.
         </para>
 
-        <note>
-          <para>For notes on configuring and building DHCPv6 with MySQL see <xref linkend="dhcp6-install">.</xref></para>
-        </note>
+
       </section>
 
       <section>
@@ -3317,9 +3324,9 @@ then change those defaults with config set Resolver/forward_addresses[0]/address
 
   </chapter>
 
-  <chapter id="dhcp4">
-    <title>DHCPv4 Server</title>
-    <para>Dynamic Host Configuration Protocol for IPv4 (DHCP or
+  <chapter id="dhcp">
+  <title>DHCP</title>
+     <para>The Dynamic Host Configuration Protocol for IPv4 (DHCP or
     DHCPv4) and Dynamic Host Configuration Protocol for IPv6 (DHCPv6)
     are protocols that allow one node (server) to provision
     configuration parameters to many hosts and devices (clients). To
@@ -3327,57 +3334,113 @@ then change those defaults with config set Resolver/forward_addresses[0]/address
     be deployed that facilitate communication between servers and
     clients. Even though principles of both DHCPv4 and DHCPv6 are
     somewhat similar, these are two radically different
-    protocols. BIND 10 offers server implementations for both DHCPv4
-    and DHCPv6. This chapter is about DHCP for IPv4. For a description
-    of the DHCPv6 server, see <xref linkend="dhcp6"/>.</para>
+    protocols. BIND 10 offers two server implementations, one for DHCPv4
+    and one for DHCPv6.</para>
+    <para>This chapter covers those parts of BIND 10 that are common to
+    both servers.  DHCPv4-specific details are covered in <xref linkend="dhcp4"/>,
+    while those details specific to DHCPv6 are described in <xref linkend="dhcp6"/>
+    </para>
+    
+    <section id="dhcp-install-configure">
+      <title>DHCP Database Installation and Configuration</title>
+      <para>
+      BIND 10 DHCP stores its leases in a lease database.  The software has been written in
+      a way that makes it possible to choose which database product should be used to
+      store the lease information.  At present, only support for MySQL is provided, and that support must
+      be explicitly included when BIND 10 is built.  This section covers the building of
+      BIND 10 with MySQL and the creation of the lease database.
+      </para>
+      <section>
+        <title>Install MySQL</title>
+        <para>
+          Install MySQL according to the instructions for your system.  The client development
+          libraries must be installed.
+        </para>
+      </section>
+      <section>
+        <title>Build and Install BIND 10</title>
+        <para>
+          Build and install BIND 10 as described in <xref linkend="installation"/>, with
+          the following modification: to enable the MySQL database code, at the
+          "configure" step (see <xref linkend="configure"/>), specify the location of the
+          MySQL configuration program "mysql_config" with the "--with-mysql-config" switch,
+          i.e.
+          <screen><userinput>./configure [other-options] --with-dhcp-mysql</userinput></screen>
+          ...if MySQL was installed in the default location, or:
+          <screen><userinput>./configure [other-options] --with-dhcp-mysql=<replaceable>path-to-mysql_config</replaceable></userinput></screen>
+          ...if not.
+        </para>
+      </section>
+      <section id="dhcp-database-create">
+        <title>Create MySQL Database and BIND 10 User</title>
+        <para>
+          The next task is to create both the lease database and the user under which the servers will
+          access it. A number of steps are required:
+        </para>
+        <para>
+          1. Log into MySQL as "root":
+          <screen>$ <userinput>mysql -u root -p</userinput>
+Enter password:<userinput/>
+   :<userinput/>
+mysql></screen>
+        </para>
+        <para>
+          2. Create the database:
+          <screen>mysql> <userinput>CREATE DATABASE <replaceable>database-name</replaceable>;</userinput></screen>
+          ... <replaceable>database-name</replaceable> is the name you have chosen for the database.
+        </para>
+         <para>
+          3. Create the database tables:
+          <screen>mysql> <userinput>CONNECT <replaceable>database-name</replaceable>;</userinput>
+mysql> <userinput>SOURCE <replaceable>path-to-bind10</replaceable>/share/bind10/dhcpdb_create.mysql</userinput></screen>
+        </para>
+         <para>
+          4. Create the user under which BIND 10 will access the database (and give it a password), then grant it access to the database tables:
+          <screen>mysql> <userinput>CREATE USER '<replaceable>user-name</replaceable>'@'localhost' IDENTIFIED BY '<replaceable>password</replaceable>';</userinput>
+mysql> <userinput>GRANT ALL ON <replaceable>database-name</replaceable>.* TO '<replaceable>user-name</replaceable>'@'localhost';</userinput></screen>
+        </para>
+        <para>
+          5. Exit MySQL:
+          <screen>mysql> <userinput>quit</userinput>
+Bye<userinput/>
+$</screen>
+       </para>
+     </section>
+   </section>
 
-    <para>The DHCPv4 server component is currently under intense
-    development. You may want to check out <ulink
-    url="http://bind10.isc.org/wiki/Kea">BIND 10 DHCP (Kea) wiki</ulink>
-    and recent posts on <ulink
-    url="https://lists.isc.org/mailman/listinfo/bind10-dev">BIND 10
-    developers mailing list</ulink>.</para>
+  </chapter>
+  
+  <chapter id="dhcp4">
+    <title>The DHCPv4 Server</title>
 
-    <para>The DHCPv4 and DHCPv6 components in BIND 10 architecture are
-    internally code named <quote>Kea</quote>.</para>
+    <section id="dhcp4-start-stop">
+      <title>Starting and Stopping the DHCPv4 Server</title>
 
-    <note>
       <para>
-        As of January 2013, the DHCPv4 component is a work in progress.
-        That means that while it is capable of performing DHCP configuration,
-        it is not fully functional.  The server is able to offer,
-        assign, renew, release and reuse expired leases, but some of the
-        options are not configurable yet. In particular Router option is hardcoded.
-        This means that the server is not really usable in actual deployments
-        yet. See <xref linkend="dhcp4-limit"/> for a detailed description.
+        <command>b10-dhcp4</command> is the BIND 10 DHCPv4 server and, like other
+        parts of BIND 10, is configured through the <command>bindctl</command>
+        program.
       </para>
-    </note>
-
-    <section id="dhcp4-usage">
-      <title>DHCPv4 Server Usage</title>
-      <para>BIND 10 has provided the DHCPv4 server component since December
-      2011. It is current experimental implementation and is not fully functional
-      yet. It is mature enough to conduct tests in lab environment, but it has
-      significant limitations. See <xref linkend="dhcp4-limit"/> for
-      details.
-      </para>
-
       <para>
-        <command>b10-dhcp4</command> is a BIND 10 component and is being
-        run under BIND 10 framework. To add a DHCPv4 process to the set of running
-        BIND 10 services, you can use following commands in <command>bindctl</command>:
-        <screen>&gt; <userinput>config add Boss/components b10-dhcp4</userinput>
+        After starting BIND 10 and entering bindctl, the first step
+        in configuring the server is to add it to the list of running BIND 10 services.
+<screen>
+&gt; <userinput>config add Boss/components b10-dhcp4</userinput>
 &gt; <userinput>config set Boss/components/b10-dhcp4/kind dispensable</userinput>
-&gt; <userinput>config commit</userinput></screen></para>
-
-       <para>
-         To stop running <command>b10-dhcp4</command>, please use the
-         following command:
-         <screen>&gt; <userinput>config remove Boss/components b10-dhcp4</userinput>
-&gt; <userinput>config commit</userinput></screen></para>
+&gt; <userinput>config commit</userinput>
+</screen>
+      </para>
+      <para>
+         To remove <command>b10-dhcp4</command> from the set of running services,
+         the <command>b10-dhcp4</command> is removed from list of Boss components:
+<screen>
+&gt; <userinput>config remove Boss/components b10-dhcp4</userinput>
+&gt; <userinput>config commit</userinput>
+</screen>
+      </para>
 
       <para>
-        During start-up the server will detect available network interfaces
+        On start-up, the server will detect available network interfaces
         and will attempt to open UDP sockets on all interfaces that
         are up, running, are not loopback, and have IPv4 address
         assigned.
@@ -3392,23 +3455,29 @@ then change those defaults with config set Resolver/forward_addresses[0]/address
 
     </section>
 
-    <section id="dhcp4-config">
-      <title>DHCPv4 Server Configuration</title>
+    <section id="dhcp4-configuration">
+      <title>Configuring the DHCPv4 Server</title>
       <para>
         Once the server is started, it can be configured. To view the
         current configuration, use the following command in <command>bindctl</command>:
         <screen>
 &gt; <userinput>config show Dhcp4</userinput></screen>
-        When starting Dhcp4 daemon for the first time, the default configuration
+        When starting the DHCPv4 daemon for the first time, the default configuration
         will be available. It will look similar to this:
-        <screen>
+<screen>
 &gt; <userinput>config show Dhcp4</userinput>
-Dhcp4/interface/                list    (default)
-Dhcp4/renew-timer        1000   integer	(default)
-Dhcp4/rebind-timer       2000   integer	(default)
-Dhcp4/preferred-lifetime 3000   integer	(default)
-Dhcp4/valid-lifetime	 4000   integer	(default)
-Dhcp4/subnet4	         []     list    (default)</screen>
+Dhcp4/interface/	list	(default)
+Dhcp4/renew-timer	1000	integer	(default)
+Dhcp4/rebind-timer	2000	integer	(default)
+Dhcp4/valid-lifetime	4000	integer	(default)
+Dhcp4/option-data	[]	list	(default)
+Dhcp4/lease-database/type	"memfile"	string	(default)
+Dhcp4/lease-database/name	""	string	(default)
+Dhcp4/lease-database/user	""	string	(default)
+Dhcp4/lease-database/host	""	string	(default)
+Dhcp4/lease-database/password	""	string	(default)
+Dhcp4/subnet4	[]	list	(default)
+</screen>
       </para>
 
       <para>
@@ -3423,6 +3492,69 @@ Dhcp4/subnet4	         []     list    (default)</screen>
         per-subnet basis.
       </para>
 
+      <section>
+      <title>Database Configuration</title>
+      <para>
+      All leases issued by the server are stored in the lease database.  Currently,
+      the only supported database is MySQL
+      <footnote>
+      <para>
+      The server comes with an in-memory database ("memfile") configured as the default
+      database. This is used for internal testing and is not supported.  In addition,
+      it does not store lease information on disk: lease information will be lost if the
+      server is restarted. 
+      </para>
+      </footnote>, and so the server must be configured to
+      access the correct database with the appropriate credentials.
+      </para>
+        <note>
+            <para>
+            Database access information must be configured for the DHCPv4 server, even if
+            it has already been configured for the DHCPv6 server.  The servers store their
+            information independently, so each server can use a separate
+            database or both servers can use the same database.
+            </para>
+        </note>
+      <para>
+      Database configuration is controlled through the Dhcp4/lease-database parameters.
+      The type of the database must be set to MySQL (although the string entered is "mysql"):
+<screen>
+&gt; <userinput>config set Dhcp4/lease-database/type "mysql"</userinput>
+</screen>
+      Next, the name of the database is to hold the leases must be set: this is the
+      name used when the lease database was created (see <xref linkend="dhcp-database-create"/>).
+<screen>
+&gt; <userinput>config set Dhcp4/lease-database/name "<replaceable>database-name</replaceable>"</userinput>
+</screen>
+      If the database is located on a different system to the DHCPv4 server, the
+      database host name must also be specified (although note that this configuration
+      may have a severe impact on server performance):
+<screen>
+&gt; <userinput>config set Dhcp4/lease-database/host "<replaceable>remote-host-name</replaceable>"</userinput>
+</screen>
+      The usual state of affairs will be to have the database on the same machine as the
+      DHCPv4 server.  In this case, set the value to the empty string (this is the default):
+<screen>
+&gt; <userinput>config set Dhcp4/lease-database/host ""</userinput>
+</screen>
+      </para>
+      <para>
+      Finally, the credentials of the account under which the server will access the database
+      should be set:
+<screen>
+&gt; <userinput>config set Dhcp4/lease-database/user "<replaceable>user-name</replaceable>"</userinput>
+&gt; <userinput>config set Dhcp4/lease-database/password "<replaceable>password</replaceable>"</userinput>
+</screen>
+      If there is no password to the account, set the password to the empty string "". (This is also the default.)
+      </para>
+      <note>
+      <para>The password is echoed when entered and is stored in clear text in the BIND 10 configuration
+      database.  Improved password security will be added in a future version of BIND 10 DHCP</para>
+      </note>
+      </section>
+      
+      <section id="dhcp4-address-config">
+      <title>Configuration of Address Pools</title>
       <para>
         The essential role of DHCPv4 server is address assignment. The server
         has to be configured with at least one subnet and one pool of dynamic
@@ -3462,7 +3594,7 @@ Dhcp4/subnet4	         []     list    (default)</screen>
 &gt; <userinput>config set Dhcp4/subnet4[1]/pool [ "192.0.3.0/24" ]</userinput>
 &gt; <userinput>config commit</userinput></screen>
         Arrays are counted from 0. subnet[0] refers to the subnet defined in the
-        previous example.  The <command>config add Dhcp4/subnet4</command> adds
+        previous example.  The <command>config add Dhcp4/subnet4</command> command adds
         another (second) subnet. It can be referred to as
         <command>Dhcp4/subnet4[1]</command>. In this example, we allow server to
         dynamically assign all addresses available in the whole subnet.
@@ -3474,23 +3606,9 @@ Dhcp4/subnet4	         []     list    (default)</screen>
         address) and the last (typically broadcast address) address from that pool.
         In the aforementioned example of pool 192.0.3.0/24, both 192.0.3.0 and
         192.0.3.255 addresses may be assigned as well. This may be invalid in some
-        network configurations. If you want to avoid this, please use min-max notation.
-      </para>
-
-      <para>
-        Note: Although configuration is now accepted, some parts of it is not internally used
-        by they server yet. Address pools are used, but option definitons are not.
-        The only way to alter some options (e.g. Router Option or DNS servers and Domain name)
-        is to modify source code. To do so, please edit
-        src/bin/dhcp6/dhcp4_srv.cc file, modify the following parameters and
-        recompile:
-        <screen>
-const std::string HARDCODED_GATEWAY = "192.0.2.1";
-const std::string HARDCODED_DNS_SERVER = "192.0.2.2";
-const std::string HARDCODED_DOMAIN_NAME = "isc.example.com";</screen>
-
-        Lease database and configuration support is planned for end of 2012.
+        network configurations. If you want to avoid this, please use the "min-max" notation.
       </para>
+      </section>
     </section>
 
     <section id="dhcp4-serverid">
@@ -3514,36 +3632,44 @@ const std::string HARDCODED_DOMAIN_NAME = "isc.example.com";</screen>
     </section>
 
     <section id="dhcp4-std">
-      <title>Supported standards</title>
+      <title>Supported Standards</title>
       <para>The following standards and draft standards are currently
       supported:</para>
       <itemizedlist>
           <listitem>
-            <simpara>RFC2131: Supported messages are DISCOVER, OFFER,
-            REQUEST, ACK, NAK, RELEASE.</simpara>
+            <simpara><ulink url="http://tools.ietf.org/html/rfc2131">RFC 2131</ulink>: Supported messages are DISCOVER, OFFER,
+            REQUEST, RELEASE, ACK, and NAK.</simpara>
           </listitem>
           <listitem>
-            <simpara>RFC2132: Supported options are: PAD (0),
+            <simpara><ulink url="http://tools.ietf.org/html/rfc2132">RFC 2132</ulink>: Supported options are: PAD (0),
             END(255), Message Type(53), DHCP Server Identifier (54),
             Domain Name (15), DNS Servers (6), IP Address Lease Time
             (51), Subnet mask (1), and Routers (3).</simpara>
           </listitem>
-          <listitem>
-            <simpara>RFC6842: Server responses include client-id option
-            if client sent it in its message.</simpara>
-          </listitem>
       </itemizedlist>
     </section>
 
     <section id="dhcp4-limit">
       <title>DHCPv4 Server Limitations</title>
       <para>These are the current limitations of the DHCPv4 server
-      software. Most of them are reflections of the early stage of
+      software. Most of them are reflections of the current stage of
       development and should be treated as <quote>not implemented
       yet</quote>, rather than actual limitations.</para>
       <itemizedlist>
           <listitem>
-            <simpara>During initial IPv4 node configuration, the
+          <para>
+            On startup, the DHCPv4 server does not get the full configuration from
+            BIND 10.  To remedy this, after starting BIND 10, modify any parameter
+            and commit the changes, e.g.
+            <screen>
+&gt; <userinput>config show Dhcp4/renew-timer</userinput>
+Dhcp4/renew-timer	1000	integer	(default)
+&gt; <userinput>config set Dhcp4/renew-timer 1001</userinput>
+&gt; <userinput>config commit</userinput></screen>
+          </para>
+        </listitem>
+          <listitem>
+            <simpara>During the initial IPv4 node configuration, the
             server is expected to send packets to a node that does not
             have IPv4 address assigned yet. The server requires
             certain tricks (or hacks) to transmit such packets. This
@@ -3551,184 +3677,91 @@ const std::string HARDCODED_DOMAIN_NAME = "isc.example.com";</screen>
             relayed traffic only (that is, normal point to point
             communication).</simpara>
           </listitem>
+
           <listitem>
             <simpara>Upon start, the server will open sockets on all
             interfaces that are not loopback, are up and running and
             have IPv4 address.</simpara>
           </listitem>
+
           <listitem>
-            <simpara>PRL (Parameter Request List, a list of options
-            requested by a client) is currently ignored and server
-            assigns DNS SERVER and DOMAIN NAME options.</simpara>
-          </listitem>
-          <listitem>
-            <simpara><command>b10-dhcp4</command> does not support
-            BOOTP. That is a design choice. This limitation is
+            <simpara>The DHCPv4 server does not support
+            BOOTP. That is a design choice and the limitation is
             permanent. If you have legacy nodes that can't use DHCP and
-            require BOOTP support, please use the latest version of ISC DHCP
-            via <ulink url="http://www.isc.org/software/dhcp"/>.</simpara>
+            require BOOTP support, please use the latest version of ISC DHCP,
+            available from <ulink url="http://www.isc.org/software/dhcp"/>.</simpara>
           </listitem>
           <listitem>
             <simpara>Interface detection is currently working on Linux
             only. See <xref linkend="iface-detect"/> for details.</simpara>
           </listitem>
           <listitem>
-            <simpara><command>b10-dhcp4</command> does not verify that
-            assigned address is unused. According to RFC2131, the
-            allocating server should verify that address is no used by
+            <simpara>The DHCPv4 server does not  verify that
+            assigned address is unused. According to <ulink url="http://tools.ietf.org/html/rfc2131">RFC 2131</ulink>, the
+            allocating server should verify that address is not used by
             sending ICMP echo request.</simpara>
           </listitem>
           <listitem>
-            <simpara>Address rebinding (REQUEST/Rebinding), confirmation
-            (CONFIRM) and duplication report (DECLINE) are not supported
-            yet.</simpara>
-          </listitem>
-          <listitem>
-            <simpara>DNS Update is not supported yet.</simpara>
-          </listitem>
-          <listitem>
-            <simpara>-v (verbose) command line option is currently
-            the default, and cannot be disabled.</simpara>
+            <simpara>Address rebinding (REBIND) and duplication report (DECLINE)
+            are not supported yet.</simpara>
           </listitem>
+
       </itemizedlist>
     </section>
 
   </chapter>
 
   <chapter id="dhcp6">
-    <title>DHCPv6 Server</title>
-    <para>The Dynamic Host Configuration Protocol for IPv6 (DHCPv6) is
-    specified in RFC3315. BIND 10 provides a DHCPv6 server implementation
-    that is described in this chapter. For a description of the DHCPv4
-    server implementation, see <xref linkend="dhcp4"/>.
-    </para>
+    <title>The DHCPv6 Server</title>
 
-    <para>The DHCPv6 server component is currently under intense
-    development. You may want to check out <ulink
-    url="http://bind10.isc.org/wiki/Kea">BIND 10 DHCP (Kea) wiki</ulink>
-    and recent posts on <ulink
-    url="https://lists.isc.org/mailman/listinfo/bind10-dev">BIND 10
-    developers mailing list</ulink>.</para>
+    <section id="dhcp6-start-stop">
+      <title>Starting and Stopping the DHCPv6 Server</title>
 
-    <note>
       <para>
-        As of November 2012, the DHCPv6 component is partially functioning,
-        having the following capabilities:
+        <command>b10-dhcp6</command> is the BIND 10 DHCPv6 server and, like other
+        parts of BIND 10, is configured through the <command>bindctl</command>
+        program.
+      </para>
+      <para>
+        After starting BIND 10 and starting <command>bindctl</command>, the first step
+        in configuring the server is to add <command>b10-dhcp6</command> to the list of running BIND 10 services.
+<screen>
+&gt; <userinput>config add Boss/components b10-dhcp6</userinput>
+&gt; <userinput>config set Boss/components/b10-dhcp6/kind dispensable</userinput>
+&gt; <userinput>config commit</userinput>
+</screen>
       </para>
-      <itemizedlist>
-          <listitem>
-            <simpara>DHCPv6 server able to allocate leases (but not renew them).</simpara>
-          </listitem>
-          <listitem>
-            <simpara>Some configuration available through the BIND 10 configuration mechanism.</simpara>
-          </listitem>
-          <listitem>
-            <simpara>Lease storage in a MySQL database.</simpara>
-          </listitem>
-      </itemizedlist>
-    </note>
-
-    <section id="dhcp6-install">
-      <title>DHCPv6 Server Build and Installation</title>
       <para>
-      DHCPv6 is part of the BIND 10 suite of programs and is built as part of
-      the build of BIND 10.  With the use of MySQL, some additional
-      installation steps are needed:
+         To remove <command>b10-dhcp6</command> from the set of running services,
+         the <command>b10-dhcp4</command> is removed from list of Boss components:
+<screen>
+&gt; <userinput>config remove Boss/components b10-dhcp6</userinput>
+&gt; <userinput>config commit</userinput>
+</screen>
       </para>
-      <section>
-        <title>Install MySQL</title>
-        <para>
-          Install MySQL according to the instructions for your system.  The client development
-          libraries must be installed.
-        </para>
-      </section>
-      <section>
-        <title>Build and Install BIND 10</title>
-        <para>
-          Build and install BIND 10 as described in <xref linkend="installation"/>, with
-          the following modification: to enable the MySQL database code, the
-          "configure" step (see <xref linkend="configure"/>), specify the location of the
-          MySQL configuration program "mysql_config" with the "--with-mysql-config" switch,
-          i.e.
-          <screen><userinput>./configure [other-options] --with-dhcp-mysql</userinput></screen>
-          ...if MySQL was installed in the default location, or:
-          <screen><userinput>./configure [other-options] --with-dhcp-mysql=<replaceable>&lt;path-to-mysql_config&gt;</replaceable></userinput></screen>
-          ...if not.
-        </para>
-      </section>
-      <section>
-        <title>Create MySQL Database and BIND 10 User</title>
-        <para>
-          The next task is to create both the DHCPv6 lease database and the user under which the DHCPv6 server will
-          access it.  Although the intention is to have the name of the database and the user configurable,
-          at the moment they are hard-coded as "kea", as is the associated password.  ("kea" is an internal
-          code name for BIND 10 DHCP.) There are a number of steps required:
-        </para>
-        <para>
-          1. Log into MySQL as "root":
-          <screen>$ <userinput>mysql -u root -p</userinput>
-Enter password:<userinput/>
-   :<userinput/>
-mysql></screen>
-        </para>
-        <para>
-          2. Create the database:
-          <screen>mysql> <userinput>CREATE DATABASE kea;</userinput></screen>
-        </para>
-         <para>
-          3. Create the database tables:
-          <screen>mysql> <userinput>CONNECT kea;</userinput>
-mysql> <userinput>SOURCE <replaceable>&lt;path-to-bind10&gt;</replaceable>/share/bind10/dhcpdb_create.mysql</userinput></screen>
-        </para>
-         <para>
-          4. Create the user under which BIND 10 will access the database and grant it access to the database tables:
-          <screen>mysql> <userinput>CREATE USER 'kea'@'localhost' IDENTIFIED BY 'kea';</userinput>
-mysql> <userinput>GRANT ALL ON kea.* TO 'kea'@'localhost';</userinput></screen>
-        </para>
-        <para>
-          5. Exit MySQL:
-          <screen>mysql> <userinput>quit</userinput>
-Bye<userinput/>
-$</screen>
-       </para>
-     </section>
-   </section>
-
-    <section id="dhcp6-usage">
-      <title>DHCPv6 Server Usage</title>
-
       <para>
-        <command>b10-dhcp6</command> is a BIND 10 component and is being
-        run under BIND 10 framework. To add a DHCPv6 process to the set of running
-        BIND 10 services, you can use following commands in <command>bindctl</command>:
-        <screen>&gt; <userinput>config add Boss/components b10-dhcp6</userinput>
-&gt; <userinput>config set Boss/components/b10-dhcp6/kind dispensable</userinput>
+        To change one of the parameters, simply follow
+        the usual <command>bindctl</command> procedure. For example, to make the
+        leases longer, change their valid-lifetime parameter:
+        <screen>
+&gt; <userinput>config set Dhcp6/valid-lifetime 7200</userinput>
 &gt; <userinput>config commit</userinput></screen>
+        Please note that most Dhcp6 parameters are of global scope
+        and apply to all defined subnets, unless they are overridden on a
+        per-subnet basis.
       </para>
 
-       <para>
-         To stop running <command>b10-dhcp6</command>, use the
-         following command:
-         <screen>&gt; <userinput>config remove Boss/components b10-dhcp6</userinput>
-&gt; <userinput>config commit</userinput></screen>
-       </para>
-
       <para>
         During start-up the server will detect available network interfaces
         and will attempt to open UDP sockets on all interfaces that
         are up, running, are not loopback, are multicast-capable, and
-        have IPv6 address assigned. It will then listen to incoming traffic. The
-        currently supported client messages are SOLICIT and REQUEST. The server
-        will respond to them with ADVERTISE and REPLY, respectively.
-      </para>
-      <para>
-        Since the DHCPv6 server opens privileged ports, it requires root
-        access. Make sure you run this daemon as root.
+        have IPv6 address assigned. It will then listen to incoming traffic.
       </para>
 
+
     </section>
 
-    <section id="dhcp6-config">
+    <section id="dhcp6-configuration">
       <title>DHCPv6 Server Configuration</title>
       <para>
         Once the server has been started, it can be configured. To view the
@@ -3736,16 +3769,22 @@ $</screen>
         <screen>&gt; <userinput>config show Dhcp6</userinput></screen>
         When starting the Dhcp6 daemon for the first time, the default configuration
         will be available. It will look similar to this:
-        <screen>
+<screen>
 &gt; <userinput>config show Dhcp6</userinput>
-Dhcp6/interface	         "eth0" string	(default)
-Dhcp6/renew-timer        1000   integer	(default)
-Dhcp6/rebind-timer       2000   integer	(default)
-Dhcp6/preferred-lifetime 3000   integer	(default)
-Dhcp6/valid-lifetime	 4000   integer	(default)
-Dhcp6/subnet6	         []     list    (default)</screen>
+Dhcp6/interface/	list	(default)
+Dhcp6/renew-timer	1000	integer	(default)
+Dhcp6/rebind-timer	2000	integer	(default)
+Dhcp6/preferred-lifetime	3000	integer	(default)
+Dhcp6/valid-lifetime	4000	integer	(default)
+Dhcp6/option-data	[]	list	(default)
+Dhcp6/lease-database/type	"memfile"	string	(default)
+Dhcp6/lease-database/name	""	string	(default)
+Dhcp6/lease-database/user	""	string	(default)
+Dhcp6/lease-database/host	""	string	(default)
+Dhcp6/lease-database/password	""	string	(default)
+Dhcp6/subnet6/	list	
+</screen>
       </para>
-
       <para>
         To change one of the parameters, simply follow
         the usual <command>bindctl</command> procedure. For example, to make the
@@ -3757,7 +3796,77 @@ Dhcp6/subnet6	         []     list    (default)</screen>
         and apply to all defined subnets, unless they are overridden on a
         per-subnet basis.
       </para>
+      <note>
+        <para>
+          With this version of BIND 10, there are a number of known limitations
+          and problems in the DHCPv6 server. See <xref linkend="dhcp6-limit"/>.
+        </para>
+      </note>
 
+      <section>
+      <title>Database Configuration</title>
+      <para>
+      All leases issued by the server are stored in the lease database.  Currently,
+      the only supported database is MySQL
+      <footnote>
+      <para>
+      The server comes with an in-memory database ("memfile") configured as the default
+      database. This is used for internal testing and is not supported.  In addition,
+      it does not store lease information on disk: lease information will be lost if the
+      server is restarted. 
+      </para>
+      </footnote>, and so the server must be configured to
+      access the correct database with the appropriate credentials.
+      </para>
+      <note>
+        <para>
+            Database access information must be configured for the DHCPv6 server, even if
+            it has already been configured for the DHCPv4 server.  The servers store their
+            information independently, so each server can use a separate
+            database or both servers can use the same database.
+          </para>
+        </note>
+      <para>
+      Database configuration is controlled through the Dhcp6/lease-database parameters.
+      The type of the database must be set to MySQL (although the string entered is "mysql"):
+<screen>
+&gt; <userinput>config set Dhcp6/lease-database/type "mysql"</userinput>
+</screen>
+      Next, the name of the database is to hold the leases must be set: this is the
+      name used when the lease database was created (see <xref linkend="dhcp-database-create"/>).
+<screen>
+&gt; <userinput>config set Dhcp6/lease-database/name "<replaceable>database-name</replaceable>"</userinput>
+</screen>
+      If the database is located on a different system to the DHCPv6 server, the
+      database host name must also be specified (although note that this configuration
+      may have a severe impact on server performance):
+<screen>
+&gt; <userinput>config set Dhcp6/lease-database/host "<replaceable>remote-host-name</replaceable>"</userinput>
+</screen>
+      The usual state of affairs will be to have the database on the same machine as the
+      DHCPv6 server.  In this case, set the value to the empty string (this is the default):
+<screen>
+&gt; <userinput>config set Dhcp6/lease-database/host ""</userinput>
+</screen>
+      </para>
+      <para>
+      Finally, the credentials of the account under which the server will access the database
+      should be set:
+<screen>
+&gt; <userinput>config set Dhcp6/lease-database/user "<replaceable>user-name</replaceable>"</userinput>
+&gt; <userinput>config set Dhcp6/lease-database/password "<replaceable>password</replaceable>"</userinput>
+</screen>
+      If there is no password to the account, set the password to the empty string "". (This is also the default.)
+      </para>
+      <note>
+      <para>The password is echoed when entered and is stored in clear text in the BIND 10 configuration
+      database.  Improved password security will be added in a future version of BIND 10 DHCP</para>
+      </note>
+      </section>
+
+
+    <section>
+      <title>Subnet and Address Pool</title>
       <para>
         The essential role of a DHCPv6 server is address assignment. For this,
         the server has to be configured with at least one subnet and one pool of dynamic
@@ -3797,7 +3906,7 @@ Dhcp6/subnet6	         []     list    (default)</screen>
 &gt; <userinput>config set Dhcp6/subnet6[1]/pool [ "2001:db8:beef::/48" ]</userinput>
 &gt; <userinput>config commit</userinput></screen>
         Arrays are counted from 0. subnet[0] refers to the subnet defined in the
-        previous example.  The <command>config add Dhcp6/subnet6</command> adds
+        previous example.  The <command>config add Dhcp6/subnet6</command> command adds
         another (second) subnet. It can be referred to as
         <command>Dhcp6/subnet6[1]</command>. In this example, we allow server to
         dynamically assign all addresses available in the whole subnet. Although
@@ -3810,7 +3919,7 @@ Dhcp6/subnet6	         []     list    (default)</screen>
         a given pool, it will be able to allocate also first (typically network
         address) address from that pool. For example for pool 2001:db8::/64 the
         2001:db8:: address may be assigned as well. If you want to avoid this,
-        please use min-max notation.
+        please use the "min-max" notation.
       </para>
       <para>
         Options can also be configured: the following commands configure
@@ -3825,7 +3934,7 @@ Dhcp6/subnet6	         []     list    (default)</screen>
 &gt; <userinput>config commit</userinput>
         </screen>
        (The value for the setting of the "data" element is split across two
-        lines in this document for clarity: when entering the command, all the
+        lines in this document for clarity: when entering the command, the whole
         string should be entered on the same line.)
       </para>
       <para>
@@ -3849,13 +3958,45 @@ Dhcp6/subnet6	         []     list    (default)</screen>
         (As before, the setting of the "data" element has been split across two
         lines for clarity.)
       </para>
-      <note>
-        <para>
-          With this version of BIND 10, there are a number of known limitations
-          and problems in the DHCPv6 server. See <xref linkend="dhcp6-limit"/>.
-        </para>
-      </note>
     </section>
+       
+      <section id="dhcp6-config-subnets">
+        <title>Subnet Selection</title>
+          <para>
+          The DHCPv6 server may receive requests from local (connected to the same
+          subnet as the server) and remote (connecting via relays)
+          clients.
+          <note>
+          <para>
+          Currently relayed DHCPv6 traffic is not supported.  The server will
+          only respond to local DHCPv6 requests - see <xref linkend="dhcp6-limit"/>
+          </para>
+          </note>
+          As it may have many subnet configurations defined, it
+          must select appropriate subnet for a given request. To do this, the server first
+          checks if there is only one subnet defined and source of the packet is
+          link-local. If this is the case, the server assumes that the only subnet
+          defined is local and client is indeed connected to it. This check
+          simplifies small deployments.
+          </para>
+          <para>
+          If there are two or more subnets defined, the server can not assume
+          which of those (if any) subnets are local. Therefore an optional
+          "interface" parameter is available within a subnet definition to designate that a given subnet
+          is local, i.e. reachable directly over specified interface. For example
+          the server that is intended to serve a local subnet over eth0 may be configured
+          as follows:
+<screen>
+&gt; <userinput>config add Dhcp6/subnet6</userinput>
+&gt; <userinput>config set Dhcp6/subnet6[1]/subnet "2001:db8:beef::/48"</userinput>
+&gt; <userinput>config set Dhcp6/subnet6[1]/pool [ "2001:db8:beef::/48" ]</userinput>
+&gt; <userinput>config set Dhcp6/subnet6[1]/interface "eth0"</userinput>
+&gt; <userinput>config commit</userinput>
+</screen>
+        </para>
+      </section>    
+    
+   </section>
 
     <section id="dhcp6-serverid">
       <title>Server Identifier in DHCPv6</title>
@@ -3888,12 +4029,11 @@ Dhcp6/subnet6	         []     list    (default)</screen>
       supported:</para>
       <itemizedlist>
           <listitem>
-            <simpara>RFC3315: Supported messages are SOLICIT,
-            ADVERTISE, REQUEST, and REPLY. Supported options are
-            SERVER_ID, CLIENT_ID, IA_NA, and IAADDRESS.</simpara>
+            <simpara><ulink url="http://tools.ietf.org/html/rfc3315">RFC 3315</ulink>: Supported messages are SOLICIT,
+            ADVERTISE, REQUEST, RELEASE, RENEW, and REPLY.</simpara>
           </listitem>
           <listitem>
-            <simpara>RFC3646: Supported option is DNS_SERVERS.</simpara>
+            <simpara><ulink url="http://tools.ietf.org/html/rfc3646">RFC 3646</ulink>: Supported option is DNS_SERVERS.</simpara>
           </listitem>
       </itemizedlist>
     </section>
@@ -3905,20 +4045,8 @@ Dhcp6/subnet6	         []     list    (default)</screen>
       software. Most of them are reflections of the early stage of
       development and should be treated as <quote>not implemented
       yet</quote>, rather than actual limitations.</para>
-      <para>
       <itemizedlist>
-        <listitem>
-          <para>The DHCPv6 server has only been tested on Debian
-          operating systems.  There are known problems with the
-          handling of packets in CentOS and RHEL.</para>
-        </listitem>
-        <listitem>
-          <para>Relayed traffic is not supported.</para>
-        </listitem>
-       <listitem>
-          <para><command>b10-dhcp6</command> only supports
-          a limited number of configuration options.</para>
-        </listitem>
+
         <listitem>
           <para>
             On startup, the DHCPv6 server does not get the full configuration from
@@ -3932,40 +4060,26 @@ Dhcp6/renew-timer	1000	integer	(default)
           </para>
         </listitem>
         <listitem>
-          <para>Upon start, the server will open sockets on all
-          interfaces that are not loopback, are up, running and are
-          multicast capable and have IPv6 address.  Support for
-          multiple interfaces is not coded in reception routines yet,
-          so if you are running this code on a machine that has many
-          interfaces and <command>b10-dhcp6</command> happens to
-          listen on wrong interface, the easiest way to work around
-          this problem is to turn down other interfaces. This
-          limitation will be fixed shortly.</para>
-        </listitem>
-        <listitem>
-          <para>ORO (Option Request Option, a list of options
-          requested by a client) is currently unsupported.</para>
+          <simpara>Relayed traffic is not supported.</simpara>
         </listitem>
         <listitem>
-          <para>Temporary addresses are not supported.</para>
+          <simpara>Temporary addresses are not supported.</simpara>
         </listitem>
         <listitem>
-          <para>Prefix delegation is not supported.</para>
+          <simpara>Prefix delegation is not supported.</simpara>
         </listitem>
         <listitem>
-          <para>Address renewal (RENEW), rebinding (REBIND),
-          confirmation (CONFIRM), duplication report (DECLINE) and
-          release (RELEASE) are not supported.</para>
+          <simpara>Rebinding (REBIND), confirmation (CONFIRM),
+          and duplication report (DECLINE) are not yet supported.</simpara>
         </listitem>
         <listitem>
-          <para>DNS Update is not supported.</para>
+          <simpara>DNS Update is not supported.</simpara>
         </listitem>
         <listitem>
-          <para>Interface detection is currently working on Linux
-          only. See <xref linkend="iface-detect"/> for details.</para>
+          <simpara>Interface detection is currently working on Linux
+          only. See <xref linkend="iface-detect"/> for details.</simpara>
         </listitem>
       </itemizedlist>
-      </para>
     </section>
 
   </chapter>
@@ -3974,7 +4088,7 @@ Dhcp6/renew-timer	1000	integer	(default)
     <title>libdhcp++ library</title>
     <para>
       libdhcp++ is a common library written in C++ that handles
-      many DHCP-related tasks, including
+      many DHCP-related tasks, including:
       <itemizedlist>
         <listitem>
           <simpara>DHCPv4 and DHCPv6 packets parsing, manipulation and assembly</simpara>
@@ -3992,10 +4106,8 @@ Dhcp6/renew-timer	1000	integer	(default)
     </para>
 
     <para>
-    While this library is currently used by
-    <command>b10-dhcp4</command> and <command>b10-dhcp6</command>
-    only, it is designed to be a portable, universal library, useful for
-    any kind of DHCP-related software.
+    While this library is currently used by BIND 10 DHCP, it is designed to
+    be a portable, universal library, useful for any kind of DHCP-related software.
     </para>
 
 <!-- TODO: point to doxygen docs -->

+ 7 - 6
src/bin/auth/query.cc

@@ -101,8 +101,11 @@ Query::ResponseCreator::create(Message& response,
 
 void
 Query::addSOA(ZoneFinder& finder) {
-    ZoneFinderContextPtr soa_ctx = finder.find(finder.getOrigin(),
-                                               RRType::SOA(), dnssec_opt_);
+    // This method is always called in finding SOA for a negative response,
+    // so we specify the use of min(RRTTL, SOA MINTTL) as specified in
+    // Section 3 of RFC2308.
+    ZoneFinderContextPtr soa_ctx = finder.findAtOrigin(RRType::SOA(), true,
+                                                       dnssec_opt_);
     if (soa_ctx->code != ZoneFinder::SUCCESS) {
         isc_throw(NoSOA, "There's no SOA record in zone " <<
             finder.getOrigin().toText());
@@ -318,11 +321,9 @@ void
 Query::addAuthAdditional(ZoneFinder& finder,
                          vector<ConstRRsetPtr>& additionals)
 {
-    const Name& origin = finder.getOrigin();
-
     // Fill in authority and addtional sections.
-    ConstZoneFinderContextPtr ns_context = finder.find(origin, RRType::NS(),
-                                                       dnssec_opt_);
+    ConstZoneFinderContextPtr ns_context =
+        finder.findAtOrigin(RRType::NS(), false, dnssec_opt_);
 
     // zone origin name should have NS records
     if (ns_context->code != ZoneFinder::SUCCESS) {

+ 50 - 32
src/bin/auth/tests/query_unittest.cc

@@ -90,6 +90,10 @@ private:
 #include <auth/tests/example_base_inc.cc>
 #include <auth/tests/example_nsec3_inc.cc>
 
+// This SOA is used in negative responses; its RRTTL is set to SOA's MINTTL
+const char* const soa_minttl_txt =
+    "example.com. 0 IN SOA . . 1 0 0 0 0\n";
+
 // This is used only in one pathological test case.
 const char* const zone_ds_txt =
     "example.com. 3600 IN DS 57855 5 1 "
@@ -1207,7 +1211,7 @@ TEST_P(QueryTest, nodomainANY) {
     EXPECT_NO_THROW(query.process(*list_, Name("nxdomain.example.com"),
                                   RRType::ANY(), response));
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 1, 0,
-                  NULL, soa_txt, NULL, mock_finder->getOrigin());
+                  NULL, soa_minttl_txt, NULL, mock_finder->getOrigin());
 }
 
 // This tests that when we need to look up Zone's apex NS records for
@@ -1345,7 +1349,7 @@ TEST_P(QueryTest, nxdomain) {
                                   Name("nxdomain.example.com"), qtype,
                                   response));
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 1, 0,
-                  NULL, soa_txt, NULL, mock_finder->getOrigin());
+                  NULL, soa_minttl_txt, NULL, mock_finder->getOrigin());
 }
 
 TEST_P(QueryTest, nxdomainWithNSEC) {
@@ -1356,8 +1360,8 @@ TEST_P(QueryTest, nxdomainWithNSEC) {
                                   Name("nxdomain.example.com"), qtype,
                                   response, true));
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
-                  NULL, (string(soa_txt) +
-                         string("example.com. 3600 IN RRSIG ") +
+                  NULL, (string(soa_minttl_txt) +
+                         string("example.com. 0 IN RRSIG ") +
                          getCommonRRSIGText("SOA") + "\n" +
                          string(nsec_nxdomain_txt) + "\n" +
                          string("noglue.example.com. 3600 IN RRSIG ") +
@@ -1382,8 +1386,8 @@ TEST_P(QueryTest, nxdomainWithNSEC2) {
     query.process(*list_, Name("(.no.example.com"), qtype, response,
                   true);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
-                  NULL, (string(soa_txt) +
-                         string("example.com. 3600 IN RRSIG ") +
+                  NULL, (string(soa_minttl_txt) +
+                         string("example.com. 0 IN RRSIG ") +
                          getCommonRRSIGText("SOA") + "\n" +
                          string(nsec_mx_txt) + "\n" +
                          string("mx.example.com. 3600 IN RRSIG ") +
@@ -1407,8 +1411,8 @@ TEST_P(QueryTest, nxdomainWithNSECDuplicate) {
     query.process(*list_, Name("nx.no.example.com"), qtype, response,
                   true);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 4, 0,
-                  NULL, (string(soa_txt) +
-                         string("example.com. 3600 IN RRSIG ") +
+                  NULL, (string(soa_minttl_txt) +
+                         string("example.com. 0 IN RRSIG ") +
                          getCommonRRSIGText("SOA") + "\n" +
                          string(nsec_no_txt) + "\n" +
                          string(").no.example.com. 3600 IN RRSIG ") +
@@ -1474,8 +1478,8 @@ TEST_F(QueryTestForMockOnly, nxdomainBadNSEC5) {
     query.process(*list_, Name("nxdomain.example.com"), qtype,
                   response, true);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 6, 0,
-                  NULL, (string(soa_txt) +
-                         string("example.com. 3600 IN RRSIG ") +
+                  NULL, (string(soa_minttl_txt) +
+                         string("example.com. 0 IN RRSIG ") +
                          getCommonRRSIGText("SOA") + "\n" +
                          string(nsec_nxdomain_txt) + "\n" +
                          string("noglue.example.com. 3600 IN RRSIG ") +
@@ -1503,7 +1507,7 @@ TEST_P(QueryTest, nxrrset) {
                                   RRType::TXT(), response));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 1, 0,
-                  NULL, soa_txt, NULL, mock_finder->getOrigin());
+                  NULL, soa_minttl_txt, NULL, mock_finder->getOrigin());
 }
 
 TEST_P(QueryTest, nxrrsetWithNSEC) {
@@ -1513,7 +1517,8 @@ TEST_P(QueryTest, nxrrsetWithNSEC) {
                   response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec_www_txt) + "\n" +
                    string("www.example.com. 3600 IN RRSIG ") +
@@ -1534,7 +1539,8 @@ TEST_P(QueryTest, emptyNameWithNSEC) {
                   response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec_mx_txt) + "\n" +
                    string("mx.example.com. 3600 IN RRSIG ") +
@@ -1550,7 +1556,8 @@ TEST_P(QueryTest, nxrrsetWithoutNSEC) {
                   response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 2, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n").c_str(),
                   NULL, mock_finder->getOrigin());
 }
@@ -1706,7 +1713,8 @@ TEST_P(QueryTest, wildcardNxrrsetWithDuplicateNSEC) {
                   response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec_wild_txt) +
                    string("*.wild.example.com. 3600 IN RRSIG ") +
@@ -1729,7 +1737,8 @@ TEST_P(QueryTest, wildcardNxrrsetWithNSEC) {
                   RRType::TXT(), response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec_wild_txt_nxrrset) +
                    string("*.uwild.example.com. 3600 IN RRSIG ") +
@@ -1753,7 +1762,8 @@ TEST_P(QueryTest, wildcardNxrrsetWithNSEC3) {
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 8, 0, NULL,
                   // SOA + its RRSIG
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    // NSEC3 for the closest encloser + its RRSIG
                    string(nsec3_uwild_txt) +
@@ -1816,7 +1826,8 @@ TEST_P(QueryTest, wildcardEmptyWithNSEC) {
                   response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec_empty_prev_txt) +
                    string("t.example.com. 3600 IN RRSIG ") +
@@ -2043,7 +2054,7 @@ TEST_P(QueryTest, DNAME_NX_RRSET) {
                     RRType::TXT(), response));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 1, 0,
-        NULL, soa_txt, NULL, mock_finder->getOrigin());
+        NULL, soa_minttl_txt, NULL, mock_finder->getOrigin());
 }
 
 /*
@@ -2307,8 +2318,8 @@ TEST_P(QueryTest, dsAboveDelegationNoData) {
                                   RRType::DS(), response, true));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) +
-                   string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(unsigned_delegation_nsec_txt) +
                    "unsigned-delegation.example.com. 3600 IN RRSIG " +
@@ -2324,7 +2335,8 @@ TEST_P(QueryTest, dsBelowDelegation) {
                                   RRType::DS(), response, true));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec_apex_txt) + "\n" +
                    string("example.com. 3600 IN RRSIG ") +
@@ -2342,7 +2354,8 @@ TEST_P(QueryTest, dsBelowDelegationWithDS) {
                                   RRType::DS(), response, true));
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 2, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA")).c_str(), NULL,
                   mock_finder->getOrigin());
 }
@@ -2382,9 +2395,10 @@ TEST_F(QueryTestForMockOnly, dsAtGrandParentAndChild) {
     memory_client.addZone(ZoneFinderPtr(
                               new AlternateZoneFinder(childname)));
     query.process(*list_, childname, RRType::DS(), response, true);
+    // Note that RR TTL of SOA and its RRSIG are set to SOA MINTTL, 0
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (childname.toText() + " 3600 IN SOA . . 0 0 0 0 0\n" +
-                   childname.toText() + " 3600 IN RRSIG " +
+                  (childname.toText() + " 0 IN SOA . . 0 0 0 0 0\n" +
+                   childname.toText() + " 0 IN RRSIG " +
                    getCommonRRSIGText("SOA") + "\n" +
                    childname.toText() + " 3600 IN NSEC " +
                    childname.toText() + " SOA NSEC RRSIG\n" +
@@ -2404,9 +2418,10 @@ TEST_F(QueryTestForMockOnly, dsAtRoot) {
                               new AlternateZoneFinder(Name::ROOT_NAME())));
     query.process(*list_, Name::ROOT_NAME(), RRType::DS(), response,
                   true);
+    // Note that RR TTL of SOA and its RRSIG are set to SOA MINTTL, 0
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(". 3600 IN SOA . . 0 0 0 0 0\n") +
-                   ". 3600 IN RRSIG " + getCommonRRSIGText("SOA") + "\n" +
+                  (string(". 0 IN SOA . . 0 0 0 0 0\n") +
+                   ". 0 IN RRSIG " + getCommonRRSIGText("SOA") + "\n" +
                    ". 3600 IN NSEC " + ". SOA NSEC RRSIG\n" +
                    ". 3600 IN RRSIG " +
                    getCommonRRSIGText("NSEC")).c_str(), NULL);
@@ -2443,7 +2458,8 @@ TEST_P(QueryTest, nxrrsetWithNSEC3) {
                   response, true);
 
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec3_www_txt) + "\n" +
                    nsec3_hash_.calculate(Name("www.example.com.")) +
@@ -2478,7 +2494,8 @@ TEST_P(QueryTest, nxrrsetWithNSEC3_ds_exact) {
     query.process(*list_, Name("unsigned-delegation.example.com."),
                   RRType::DS(), response, true);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 4, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(unsigned_delegation_nsec3_txt) + "\n" +
                    nsec3_hash_.calculate(
@@ -2500,7 +2517,8 @@ TEST_P(QueryTest, nxrrsetWithNSEC3_ds_no_exact) {
     query.process(*list_, Name("unsigned-delegation-optout.example.com."),
                   RRType::DS(), response, true);
     responseCheck(response, Rcode::NOERROR(), AA_FLAG, 0, 6, 0, NULL,
-                  (string(soa_txt) + string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    string(nsec3_apex_txt) + "\n" +
                    nsec3_hash_.calculate(Name("example.com.")) +
@@ -2528,8 +2546,8 @@ TEST_P(QueryTest, nxdomainWithNSEC3Proof) {
                   response, true);
     responseCheck(response, Rcode::NXDOMAIN(), AA_FLAG, 0, 8, 0, NULL,
                   // SOA + its RRSIG
-                  (string(soa_txt) +
-                   string("example.com. 3600 IN RRSIG ") +
+                  (string(soa_minttl_txt) +
+                   string("example.com. 0 IN RRSIG ") +
                    getCommonRRSIGText("SOA") + "\n" +
                    // NSEC3 for the closest encloser + its RRSIG
                    string(nsec3_apex_txt) + "\n" +

+ 107 - 68
src/bin/bindctl/bindcmd.py

@@ -39,6 +39,7 @@ import csv
 import pwd
 import getpass
 import copy
+import errno
 
 try:
     from collections import OrderedDict
@@ -123,6 +124,11 @@ class BindCmdInterpreter(Cmd):
             self.csv_file_dir = pwd.getpwnam(getpass.getuser()).pw_dir + \
                 os.sep + '.bind10' + os.sep
 
+    def _print(self, *args):
+        '''Simple wrapper around calls to print that can be overridden in
+           unit tests.'''
+        print(*args)
+
     def _get_session_id(self):
         '''Generate one session id for the connection. '''
         rand = os.urandom(16)
@@ -150,19 +156,19 @@ WARNING: Python readline module isn't available, so the command line editor
                 return 1
 
             self.cmdloop()
-            print('\nExit from bindctl')
+            self._print('\nExit from bindctl')
             return 0
         except FailToLogin as err:
             # error already printed when this was raised, ignoring
             return 1
         except KeyboardInterrupt:
-            print('\nExit from bindctl')
+            self._print('\nExit from bindctl')
             return 0
         except socket.error as err:
-            print('Failed to send request, the connection is closed')
+            self._print('Failed to send request, the connection is closed')
             return 1
         except http.client.CannotSendRequest:
-            print('Can not send request, the connection is busy')
+            self._print('Can not send request, the connection is busy')
             return 1
 
     def _get_saved_user_info(self, dir, file_name):
@@ -181,7 +187,8 @@ WARNING: Python readline module isn't available, so the command line editor
             for row in users_info:
                 users.append([row[0], row[1]])
         except (IOError, IndexError) as err:
-            print("Error reading saved username and password from %s%s: %s" % (dir, file_name, err))
+            self._print("Error reading saved username and password "
+                        "from %s%s: %s" % (dir, file_name, err))
         finally:
             if csvfile:
                 csvfile.close()
@@ -201,12 +208,48 @@ WARNING: Python readline module isn't available, so the command line editor
             writer.writerow([username, passwd])
             csvfile.close()
         except IOError as err:
-            print("Error saving user information:", err)
-            print("user info file name: %s%s" % (dir, file_name))
+            self._print("Error saving user information:", err)
+            self._print("user info file name: %s%s" % (dir, file_name))
             return False
 
         return True
 
+    def __print_check_ssl_msg(self):
+        self._print("Please check the logs of b10-cmdctl, there may "
+                    "be a problem accepting SSL connections, such "
+                    "as a permission problem on the server "
+                    "certificate file.")
+
+    def _try_login(self, username, password):
+        '''
+        Attempts to log in to cmdctl by sending a POST with
+        the given username and password.
+        On success of the POST (mind, not the login, only the network
+        operation), returns a tuple (response, data).
+        On failure, raises a FailToLogin exception, and prints some
+        information on the failure.
+        This call is essentially 'private', but made 'protected' for
+        easier testing.
+        '''
+        param = {'username': username, 'password' : password}
+        try:
+            response = self.send_POST('/login', param)
+            data = response.read().decode()
+            # return here (will raise error after try block)
+            return (response, data)
+        except ssl.SSLError as err:
+            self._print("SSL error while sending login information: ", err)
+            if err.errno == ssl.SSL_ERROR_EOF:
+                self.__print_check_ssl_msg()
+        except socket.error as err:
+            self._print("Socket error while sending login information: ", err)
+            # An SSL setup error can also bubble up as a plain CONNRESET...
+            # (on some systems it usually does)
+            if err.errno == errno.ECONNRESET:
+                self.__print_check_ssl_msg()
+            pass
+        raise FailToLogin()
+
     def login_to_cmdctl(self):
         '''Login to cmdctl with the username and password given by
         the user. After the login is sucessful, the username and
@@ -217,41 +260,30 @@ WARNING: Python readline module isn't available, so the command line editor
         # Look at existing username/password combinations and try to log in
         users = self._get_saved_user_info(self.csv_file_dir, CSV_FILE_NAME)
         for row in users:
-            param = {'username': row[0], 'password' : row[1]}
-            try:
-                response = self.send_POST('/login', param)
-                data = response.read().decode()
-            except socket.error as err:
-                print("Socket error while sending login information:", err)
-                raise FailToLogin()
+            response, data = self._try_login(row[0], row[1])
 
             if response.status == http.client.OK:
                 # Is interactive?
                 if sys.stdin.isatty():
-                    print(data + ' login as ' + row[0])
+                    self._print(data + ' login as ' + row[0])
                 return True
 
         # No valid logins were found, prompt the user for a username/password
         count = 0
-        print('No stored password file found, please see sections '
+        self._print('No stored password file found, please see sections '
               '"Configuration specification for b10-cmdctl" and "bindctl '
               'command-line options" of the BIND 10 guide.')
         while True:
             count = count + 1
             if count > 3:
-                print("Too many authentication failures")
+                self._print("Too many authentication failures")
                 return False
 
             username = input("Username: ")
             passwd = getpass.getpass()
-            param = {'username': username, 'password' : passwd}
-            try:
-                response = self.send_POST('/login', param)
-                data = response.read().decode()
-                print(data)
-            except socket.error as err:
-                print("Socket error while sending login information:", err)
-                raise FailToLogin()
+
+            response, data = self._try_login(username, passwd)
+            self._print(data)
 
             if response.status == http.client.OK:
                 self._save_user_info(username, passwd, self.csv_file_dir,
@@ -449,25 +481,26 @@ WARNING: Python readline module isn't available, so the command line editor
         pass
 
     def do_help(self, name):
-        print(CONST_BINDCTL_HELP)
+        self._print(CONST_BINDCTL_HELP)
         for k in self.modules.values():
             n = k.get_name()
             if len(n) >= CONST_BINDCTL_HELP_INDENT_WIDTH:
-                print("    %s" % n)
-                print(textwrap.fill(k.get_desc(),
-                      initial_indent="            ",
-                      subsequent_indent="    " +
-                      " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
-                      width=70))
+                self._print("    %s" % n)
+                self._print(textwrap.fill(k.get_desc(),
+                            initial_indent="            ",
+                            subsequent_indent="    " +
+                            " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                            width=70))
             else:
-                print(textwrap.fill("%s%s%s" %
-                    (k.get_name(),
-                     " "*(CONST_BINDCTL_HELP_INDENT_WIDTH - len(k.get_name())),
-                     k.get_desc()),
-                    initial_indent="    ",
-                    subsequent_indent="    " +
-                    " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
-                    width=70))
+                self._print(textwrap.fill("%s%s%s" %
+                            (k.get_name(),
+                            " "*(CONST_BINDCTL_HELP_INDENT_WIDTH -
+                                 len(k.get_name())),
+                            k.get_desc()),
+                            initial_indent="    ",
+                            subsequent_indent="    " +
+                            " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                            width=70))
 
     def onecmd(self, line):
         if line == 'EOF' or line.lower() == "quit":
@@ -642,20 +675,20 @@ WARNING: Python readline module isn't available, so the command line editor
             self._validate_cmd(cmd)
             self._handle_cmd(cmd)
         except (IOError, http.client.HTTPException) as err:
-            print('Error: ', err)
+            self._print('Error: ', err)
         except BindCtlException as err:
-            print("Error! ", err)
+            self._print("Error! ", err)
             self._print_correct_usage(err)
         except isc.cc.data.DataTypeError as err:
-            print("Error! ", err)
+            self._print("Error! ", err)
         except isc.cc.data.DataTypeError as dte:
-            print("Error: " + str(dte))
+            self._print("Error: " + str(dte))
         except isc.cc.data.DataNotFoundError as dnfe:
-            print("Error: " + str(dnfe))
+            self._print("Error: " + str(dnfe))
         except isc.cc.data.DataAlreadyPresentError as dape:
-            print("Error: " + str(dape))
+            self._print("Error: " + str(dape))
         except KeyError as ke:
-            print("Error: missing " + str(ke))
+            self._print("Error: missing " + str(ke))
 
     def _print_correct_usage(self, ept):
         if isinstance(ept, CmdUnknownModuleSyntaxError):
@@ -704,7 +737,8 @@ WARNING: Python readline module isn't available, so the command line editor
             module_name = identifier.split('/')[1]
             if module_name != "" and (self.config_data is None or \
                not self.config_data.have_specification(module_name)):
-                print("Error: Module '" + module_name + "' unknown or not running")
+                self._print("Error: Module '" + module_name +
+                            "' unknown or not running")
                 return
 
         if cmd.command == "show":
@@ -718,7 +752,9 @@ WARNING: Python readline module isn't available, so the command line editor
                     #identifier
                     identifier += cmd.params['argument']
                 else:
-                    print("Error: unknown argument " + cmd.params['argument'] + ", or multiple identifiers given")
+                    self._print("Error: unknown argument " +
+                                cmd.params['argument'] +
+                                ", or multiple identifiers given")
                     return
             values = self.config_data.get_value_maps(identifier, show_all)
             for value_map in values:
@@ -746,13 +782,14 @@ WARNING: Python readline module isn't available, so the command line editor
                     line += "(default)"
                 if value_map['modified']:
                     line += "(modified)"
-                print(line)
+                self._print(line)
         elif cmd.command == "show_json":
             if identifier == "":
-                print("Need at least the module to show the configuration in JSON format")
+                self._print("Need at least the module to show the "
+                            "configuration in JSON format")
             else:
                 data, default = self.config_data.get_value(identifier)
-                print(json.dumps(data))
+                self._print(json.dumps(data))
         elif cmd.command == "add":
             self.config_data.add_value(identifier,
                                        cmd.params.get('value_or_name'),
@@ -764,7 +801,7 @@ WARNING: Python readline module isn't available, so the command line editor
                 self.config_data.remove_value(identifier, None)
         elif cmd.command == "set":
             if 'identifier' not in cmd.params:
-                print("Error: missing identifier or value")
+                self._print("Error: missing identifier or value")
             else:
                 parsed_value = None
                 try:
@@ -781,9 +818,9 @@ WARNING: Python readline module isn't available, so the command line editor
             try:
                 self.config_data.commit()
             except isc.config.ModuleCCSessionError as mcse:
-                print(str(mcse))
+                self._print(str(mcse))
         elif cmd.command == "diff":
-            print(self.config_data.get_local_changes())
+            self._print(self.config_data.get_local_changes())
         elif cmd.command == "go":
             self.go(identifier)
 
@@ -803,7 +840,7 @@ WARNING: Python readline module isn't available, so the command line editor
         # check if exists, if not, revert and error
         v,d = self.config_data.get_value(new_location)
         if v is None:
-            print("Error: " + identifier + " not found")
+            self._print("Error: " + identifier + " not found")
             return
 
         self.location = new_location
@@ -818,7 +855,7 @@ WARNING: Python readline module isn't available, so the command line editor
                 with open(command.params['filename']) as command_file:
                     commands = command_file.readlines()
             except IOError as ioe:
-                print("Error: " + str(ioe))
+                self._print("Error: " + str(ioe))
                 return
         elif command_sets.has_command_set(command.command):
             commands = command_sets.get_commands(command.command)
@@ -836,7 +873,7 @@ WARNING: Python readline module isn't available, so the command line editor
     def __show_execute_commands(self, commands):
         '''Prints the command list without executing them'''
         for line in commands:
-            print(line.strip())
+            self._print(line.strip())
 
     def __apply_execute_commands(self, commands):
         '''Applies the configuration commands from the given iterator.
@@ -857,18 +894,19 @@ WARNING: Python readline module isn't available, so the command line editor
             for line in commands:
                 line = line.strip()
                 if verbose:
-                    print(line)
+                    self._print(line)
                 if line.startswith('#') or len(line) == 0:
                     continue
                 elif line.startswith('!'):
                     if re.match('^!echo ', line, re.I) and len(line) > 6:
-                        print(line[6:])
+                        self._print(line[6:])
                     elif re.match('^!verbose\s+on\s*$', line, re.I):
                         verbose = True
                     elif re.match('^!verbose\s+off$', line, re.I):
                         verbose = False
                     else:
-                        print("Warning: ignoring unknown directive: " + line)
+                        self._print("Warning: ignoring unknown directive: " +
+                                    line)
                 else:
                     cmd = BindCmdParser(line)
                     self._validate_cmd(cmd)
@@ -879,12 +917,12 @@ WARNING: Python readline module isn't available, so the command line editor
                 isc.cc.data.DataNotFoundError,
                 isc.cc.data.DataAlreadyPresentError,
                 KeyError) as err:
-            print('Error: ', err)
-            print()
-            print('Depending on the contents of the script, and which')
-            print('commands it has called, there can be committed and')
-            print('local changes. It is advised to check your settings,')
-            print('and revert local changes with "config revert".')
+            self._print('Error: ', err)
+            self._print()
+            self._print('Depending on the contents of the script, and which')
+            self._print('commands it has called, there can be committed and')
+            self._print('local changes. It is advised to check your settings')
+            self._print(', and revert local changes with "config revert".')
 
     def apply_cmd(self, cmd):
         '''Handles a general module command'''
@@ -898,6 +936,7 @@ WARNING: Python readline module isn't available, so the command line editor
         # The reply is a string containing JSON data,
         # parse it, then prettyprint
         if data != "" and data != "{}":
-            print(json.dumps(json.loads(data), sort_keys=True, indent=4))
+            self._print(json.dumps(json.loads(data), sort_keys=True,
+                                   indent=4))
 
 

+ 114 - 11
src/bin/bindctl/tests/bindctl_test.py

@@ -18,11 +18,14 @@ import unittest
 import isc.cc.data
 import os
 import io
+import errno
 import sys
 import socket
+import ssl
 import http.client
 import pwd
 import getpass
+import re
 from optparse import OptionParser
 from isc.config.config_data import ConfigData, MultiConfigData
 from isc.config.module_spec import ModuleSpec
@@ -335,6 +338,8 @@ class TestConfigCommands(unittest.TestCase):
         self.tool.add_module_info(mod_info)
         self.tool.config_data = FakeCCSession()
         self.stdout_backup = sys.stdout
+        self.printed_messages = []
+        self.tool._print = self.store_print
 
     def test_precmd(self):
         def update_all_modules_info():
@@ -347,6 +352,111 @@ class TestConfigCommands(unittest.TestCase):
         precmd('EOF')
         self.assertRaises(socket.error, precmd, 'continue')
 
+    def store_print(self, *args):
+        '''Method to override _print in BindCmdInterpreter.
+           Instead of printing the values, appends the argument tuple
+           to the list in self.printed_messages'''
+        self.printed_messages.append(" ".join(map(str, args)))
+
+    def __check_printed_message(self, expected_message, printed_message):
+        self.assertIsNotNone(re.match(expected_message, printed_message),
+                             "Printed message '" + printed_message +
+                             "' does not match '" + expected_message + "'")
+
+    def __check_printed_messages(self, expected_messages):
+        '''Helper test function to check the printed messages against a list
+           of regexps'''
+        self.assertEqual(len(expected_messages), len(self.printed_messages))
+        for _ in map(self.__check_printed_message,
+                     expected_messages,
+                     self.printed_messages):
+            pass
+
+    def test_try_login(self):
+        # Make sure __try_login raises the correct exception
+        # upon failure of either send_POST or the read() on the
+        # response
+
+        orig_send_POST = self.tool.send_POST
+        expected_printed_messages = []
+        try:
+            def send_POST_raiseImmediately(self, params):
+                raise socket.error("test error")
+
+            self.tool.send_POST = send_POST_raiseImmediately
+            self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+            expected_printed_messages.append(
+                'Socket error while sending login information:  test error')
+            self.__check_printed_messages(expected_printed_messages)
+
+            def create_send_POST_raiseOnRead(exception):
+                '''Create a replacement send_POST() method that raises
+                   the given exception when read() is called on the value
+                   returned from send_POST()'''
+                def send_POST_raiseOnRead(self, params):
+                    class MyResponse:
+                        def read(self):
+                            raise exception
+                    return MyResponse()
+                return send_POST_raiseOnRead
+
+            # basic socket error
+            self.tool.send_POST =\
+                create_send_POST_raiseOnRead(socket.error("read error"))
+            self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+            expected_printed_messages.append(
+                'Socket error while sending login information:  read error')
+            self.__check_printed_messages(expected_printed_messages)
+
+            # connection reset
+            exc = socket.error("connection reset")
+            exc.errno = errno.ECONNRESET
+            self.tool.send_POST =\
+                create_send_POST_raiseOnRead(exc)
+            self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+            expected_printed_messages.append(
+                'Socket error while sending login information:  '
+                'connection reset')
+            expected_printed_messages.append(
+                'Please check the logs of b10-cmdctl, there may be a '
+                'problem accepting SSL connections, such as a permission '
+                'problem on the server certificate file.'
+            )
+            self.__check_printed_messages(expected_printed_messages)
+
+            # 'normal' SSL error
+            exc = ssl.SSLError()
+            self.tool.send_POST =\
+                create_send_POST_raiseOnRead(exc)
+            self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+            expected_printed_messages.append(
+                'SSL error while sending login information:  .*')
+            self.__check_printed_messages(expected_printed_messages)
+
+            # 'EOF' SSL error
+            exc = ssl.SSLError()
+            exc.errno = ssl.SSL_ERROR_EOF
+            self.tool.send_POST =\
+                create_send_POST_raiseOnRead(exc)
+            self.assertRaises(FailToLogin, self.tool._try_login, "foo", "bar")
+            expected_printed_messages.append(
+                'SSL error while sending login information: .*')
+            expected_printed_messages.append(
+                'Please check the logs of b10-cmdctl, there may be a '
+                'problem accepting SSL connections, such as a permission '
+                'problem on the server certificate file.'
+            )
+            self.__check_printed_messages(expected_printed_messages)
+
+            # any other exception should be passed through
+            self.tool.send_POST =\
+                create_send_POST_raiseOnRead(ImportError())
+            self.assertRaises(ImportError, self.tool._try_login, "foo", "bar")
+            self.__check_printed_messages(expected_printed_messages)
+
+        finally:
+            self.tool.send_POST = orig_send_POST
+
     def test_run(self):
         def login_to_cmdctl():
             return True
@@ -360,29 +470,22 @@ class TestConfigCommands(unittest.TestCase):
         self.tool.conn.sock = FakeSocket()
         self.tool.conn.sock.close()
 
-        # validate log message for socket.err
-        socket_err_output = io.StringIO()
-        sys.stdout = socket_err_output
         self.assertEqual(1, self.tool.run())
 
         # First few lines may be some kind of heading, or a warning that
         # Python readline is unavailable, so we do a sub-string check.
         self.assertIn("Failed to send request, the connection is closed",
-                      socket_err_output.getvalue())
-
-        socket_err_output.close()
+                      self.printed_messages)
+        self.assertEqual(1, len(self.printed_messages))
 
         # validate log message for http.client.CannotSendRequest
-        cannot_send_output = io.StringIO()
-        sys.stdout = cannot_send_output
         self.assertEqual(1, self.tool.run())
 
         # First few lines may be some kind of heading, or a warning that
         # Python readline is unavailable, so we do a sub-string check.
         self.assertIn("Can not send request, the connection is busy",
-                      cannot_send_output.getvalue())
-
-        cannot_send_output.close()
+                      self.printed_messages)
+        self.assertEqual(2, len(self.printed_messages))
 
     def test_apply_cfg_command_int(self):
         self.tool.location = '/'

+ 33 - 20
src/bin/cmdctl/cmdctl.py.in

@@ -82,6 +82,18 @@ SPECFILE_LOCATION = SPECFILE_PATH + os.sep + "cmdctl.spec"
 class CmdctlException(Exception):
     pass
 
+def check_file(file_name):
+    # TODO: Check contents of certificate file
+    if not os.path.exists(file_name):
+        raise CmdctlException("'%s' does not exist" % file_name)
+
+    if not os.path.isfile(file_name):
+        raise CmdctlException("'%s' is not a file" % file_name)
+
+    if not os.access(file_name, os.R_OK):
+        raise CmdctlException("'%s' is not readable" % file_name)
+
+
 class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
     '''https connection request handler.
     Currently only GET and POST are supported.  '''
@@ -153,7 +165,6 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
         self.end_headers()
         self.wfile.write(json.dumps(reply).encode())
 
-
     def _handle_login(self):
         if self._is_user_logged_in():
             return http.client.OK, ["user has already login"]
@@ -278,12 +289,14 @@ class CommandControl():
             if key == 'version':
                 continue
             elif key in ['key_file', 'cert_file']:
-                #TODO, only check whether the file exist,
-                # further check need to be done: eg. whether
-                # the private/certificate is valid.
+                # TODO: we only check whether the file exist, is a
+                # file, and is readable; but further check need to be done:
+                # eg. whether the private/certificate is valid.
                 path = new_config[key]
-                if not os.path.exists(path):
-                    errstr = "the file doesn't exist: " + path
+                try:
+                    check_file(path)
+                except CmdctlException as cce:
+                    errstr = str(cce)
             elif key == 'accounts_file':
                 errstr = self._accounts_file_check(new_config[key])
             else:
@@ -524,27 +537,27 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
         self.user_sessions[session_id] = time.time()
 
     def _check_key_and_cert(self, key, cert):
-        # TODO, check the content of key/certificate file
-        if not os.path.exists(key):
-            raise CmdctlException("key file '%s' doesn't exist " % key)
-
-        if not os.path.exists(cert):
-            raise CmdctlException("certificate file '%s' doesn't exist " % cert)
+        check_file(key)
+        check_file(cert);
 
     def _wrap_socket_in_ssl_context(self, sock, key, cert):
         try:
             self._check_key_and_cert(key, cert)
             ssl_sock = ssl.wrap_socket(sock,
-                                      server_side = True,
-                                      certfile = cert,
-                                      keyfile = key,
-                                      ssl_version = ssl.PROTOCOL_SSLv23)
+                                       server_side=True,
+                                       certfile=cert,
+                                       keyfile=key,
+                                       ssl_version=ssl.PROTOCOL_SSLv23)
+            # Return here (if control leaves this blocks it will raise an
+            # error)
             return ssl_sock
-        except (ssl.SSLError, CmdctlException) as err :
+        except ssl.SSLError as err:
             logger.error(CMDCTL_SSL_SETUP_FAILURE_USER_DENIED, err)
-            self.close_request(sock)
-            # raise socket error to finish the request
-            raise socket.error
+        except (CmdctlException, IOError) as cce:
+            logger.error(CMDCTL_SSL_SETUP_FAILURE_READING_CERT, cce)
+        self.close_request(sock)
+        # raise socket error to finish the request
+        raise socket.error
 
     def get_request(self):
         '''Get client request socket and wrap it in SSL context. '''

+ 7 - 0
src/bin/cmdctl/cmdctl_messages.mes

@@ -58,6 +58,13 @@ with the tool b10-cmdctl-usermgr.
 This debug message indicates that the given command is being sent to
 the given module.
 
+% CMDCTL_SSL_SETUP_FAILURE_READING_CERT failed to read certificate or key: %1
+The b10-cmdctl daemon is unable to read either the certificate file or
+the private key file, and is therefore unable to accept any SSL connections.
+The specific error is printed in the message.
+The administrator should solve the issue with the files, or recreate them
+with the b10-certgen tool.
+
 % CMDCTL_SSL_SETUP_FAILURE_USER_DENIED failed to create an SSL connection (user denied): %1
 The user was denied because the SSL connection could not successfully
 be set up. The specific error is given in the log message. Possible

+ 1 - 1
src/bin/cmdctl/tests/Makefile.am

@@ -25,7 +25,7 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/cmdctl \
-	CMDCTL_SPEC_PATH=$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_BUILD_PATH=$(abs_top_builddir)/src/bin/cmdctl \
 	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
 	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \

+ 115 - 45
src/bin/cmdctl/tests/cmdctl_test.py

@@ -17,17 +17,18 @@
 import unittest
 import socket
 import tempfile
+import stat
 import sys
 from cmdctl import *
 import isc.log
 
-SPEC_FILE_PATH = '..' + os.sep
-if 'CMDCTL_SPEC_PATH' in os.environ:
-    SPEC_FILE_PATH = os.environ['CMDCTL_SPEC_PATH'] + os.sep
+assert 'CMDCTL_SRC_PATH' in os.environ,\
+       "Please run this test with 'make check'"
+SRC_FILE_PATH = os.environ['CMDCTL_SRC_PATH'] + os.sep
 
-SRC_FILE_PATH = '..' + os.sep
-if 'CMDCTL_SRC_PATH' in os.environ:
-    SRC_FILE_PATH = os.environ['CMDCTL_SRC_PATH'] + os.sep
+assert 'CMDCTL_BUILD_PATH' in os.environ,\
+       "Please run this test with 'make check'"
+BUILD_FILE_PATH = os.environ['CMDCTL_BUILD_PATH'] + os.sep
 
 # Rewrite the class for unittest.
 class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
@@ -36,7 +37,7 @@ class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
 
     def send_response(self, rcode):
         self.rcode = rcode
-    
+
     def end_headers(self):
         pass
 
@@ -51,13 +52,13 @@ class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
         super().do_POST()
         self.wfile.close()
         os.remove('tmp.file')
-    
+
 
 class FakeSecureHTTPServer(SecureHTTPServer):
     def __init__(self):
         self.user_sessions = {}
         self.cmdctl = FakeCommandControlForTestRequestHandler()
-        self._verbose = True 
+        self._verbose = True
         self._user_infos = {}
         self.idle_timeout = 1200
         self._lock = threading.Lock()
@@ -71,6 +72,17 @@ class FakeCommandControlForTestRequestHandler(CommandControl):
     def send_command(self, mod, cmd, param):
         return 0, {}
 
+# context to temporarily make a file unreadable
+class UnreadableFile:
+    def __init__(self, file_name):
+        self.file_name = file_name
+        self.orig_mode = os.stat(file_name).st_mode
+
+    def __enter__(self):
+        os.chmod(self.file_name, self.orig_mode & ~stat.S_IRUSR)
+
+    def __exit__(self, type, value, traceback):
+        os.chmod(self.file_name, self.orig_mode)
 
 class TestSecureHTTPRequestHandler(unittest.TestCase):
     def setUp(self):
@@ -97,7 +109,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.path = '/abc'
         mod, cmd = self.handler._parse_request_path()
         self.assertTrue((mod == 'abc') and (cmd == None))
-        
+
         self.handler.path = '/abc/edf'
         mod, cmd = self.handler._parse_request_path()
         self.assertTrue((mod == 'abc') and (cmd == 'edf'))
@@ -125,20 +137,20 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
 
     def test_do_GET(self):
         self.handler.do_GET()
-        self.assertEqual(self.handler.rcode, http.client.BAD_REQUEST)    
-        
+        self.assertEqual(self.handler.rcode, http.client.BAD_REQUEST)
+
     def test_do_GET_1(self):
         self.handler.headers['cookie'] = 12345
         self.handler.do_GET()
-        self.assertEqual(self.handler.rcode, http.client.UNAUTHORIZED)    
+        self.assertEqual(self.handler.rcode, http.client.UNAUTHORIZED)
 
     def test_do_GET_2(self):
         self.handler.headers['cookie'] = 12345
         self.handler.server.user_sessions[12345] = time.time() + 1000000
         self.handler.path = '/how/are'
         self.handler.do_GET()
-        self.assertEqual(self.handler.rcode, http.client.NO_CONTENT)    
-    
+        self.assertEqual(self.handler.rcode, http.client.NO_CONTENT)
+
     def test_do_GET_3(self):
         self.handler.headers['cookie'] = 12346
         self.handler.server.user_sessions[12346] = time.time() + 1000000
@@ -146,8 +158,8 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         for path in path_vec:
             self.handler.path = '/' + path
             self.handler.do_GET()
-            self.assertEqual(self.handler.rcode, http.client.OK)    
-    
+            self.assertEqual(self.handler.rcode, http.client.OK)
+
     def test_user_logged_in(self):
         self.handler.server.user_sessions = {}
         self.handler.session_id = 12345
@@ -243,8 +255,8 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.assertEqual(http.client.BAD_REQUEST, rcode)
 
     def _gen_module_spec(self):
-        spec = { 'commands': [ 
-                  { 'command_name' :'command', 
+        spec = { 'commands': [
+                  { 'command_name' :'command',
                     'command_args': [ {
                             'item_name' : 'param1',
                             'item_type' : 'integer',
@@ -253,9 +265,9 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
                            } ],
                     'command_description' : 'cmd description'
                   }
-                ] 
+                ]
                }
-        
+
         return spec
 
     def test_handle_post_request_2(self):
@@ -290,13 +302,13 @@ class MyCommandControl(CommandControl):
         return {}
 
     def _setup_session(self):
-        spec_file = SPEC_FILE_PATH + 'cmdctl.spec'
+        spec_file = BUILD_FILE_PATH + 'cmdctl.spec'
         module_spec = isc.config.module_spec_from_file(spec_file)
         config = isc.config.config_data.ConfigData(module_spec)
         self._module_name = 'Cmdctl'
         self._cmdctl_config_data = config.get_full_config()
 
-    def _handle_msg_from_msgq(self): 
+    def _handle_msg_from_msgq(self):
         pass
 
 class TestCommandControl(unittest.TestCase):
@@ -305,7 +317,7 @@ class TestCommandControl(unittest.TestCase):
         self.old_stdout = sys.stdout
         sys.stdout = open(os.devnull, 'w')
         self.cmdctl = MyCommandControl(None, True)
-   
+
     def tearDown(self):
         sys.stdout.close()
         sys.stdout = self.old_stdout
@@ -320,7 +332,7 @@ class TestCommandControl(unittest.TestCase):
         old_env = os.environ
         if 'B10_FROM_SOURCE' in os.environ:
             del os.environ['B10_FROM_SOURCE']
-        self.cmdctl.get_cmdctl_config_data() 
+        self.cmdctl.get_cmdctl_config_data()
         self._check_config(self.cmdctl)
         os.environ = old_env
 
@@ -328,7 +340,7 @@ class TestCommandControl(unittest.TestCase):
         os.environ['B10_FROM_SOURCE'] = '../'
         self._check_config(self.cmdctl)
         os.environ = old_env
-    
+
     def test_parse_command_result(self):
         self.assertEqual({}, self.cmdctl._parse_command_result(1, {'error' : 1}))
         self.assertEqual({'a': 1}, self.cmdctl._parse_command_result(0, {'a' : 1}))
@@ -391,13 +403,13 @@ class TestCommandControl(unittest.TestCase):
         os.environ = old_env
 
         answer = self.cmdctl.config_handler({'key_file': '/user/non-exist_folder'})
-        self._check_answer(answer, 1, "the file doesn't exist: /user/non-exist_folder")
+        self._check_answer(answer, 1, "'/user/non-exist_folder' does not exist")
 
         answer = self.cmdctl.config_handler({'cert_file': '/user/non-exist_folder'})
-        self._check_answer(answer, 1, "the file doesn't exist: /user/non-exist_folder")
+        self._check_answer(answer, 1, "'/user/non-exist_folder' does not exist")
 
         answer = self.cmdctl.config_handler({'accounts_file': '/user/non-exist_folder'})
-        self._check_answer(answer, 1, 
+        self._check_answer(answer, 1,
                 "Invalid accounts file: [Errno 2] No such file or directory: '/user/non-exist_folder'")
 
         # Test with invalid accounts file
@@ -409,7 +421,7 @@ class TestCommandControl(unittest.TestCase):
         answer = self.cmdctl.config_handler({'accounts_file': file_name})
         self._check_answer(answer, 1, "Invalid accounts file: list index out of range")
         os.remove(file_name)
-    
+
     def test_send_command(self):
         rcode, value = self.cmdctl.send_command('Cmdctl', 'print_settings', None)
         self.assertEqual(rcode, 0)
@@ -424,7 +436,7 @@ class TestSecureHTTPServer(unittest.TestCase):
         self.old_stderr = sys.stderr
         sys.stdout = open(os.devnull, 'w')
         sys.stderr = sys.stdout
-        self.server = MySecureHTTPServer(('localhost', 8080), 
+        self.server = MySecureHTTPServer(('localhost', 8080),
                                          MySecureHTTPRequestHandler,
                                          MyCommandControl, verbose=True)
 
@@ -458,32 +470,90 @@ class TestSecureHTTPServer(unittest.TestCase):
         self.assertEqual(1, len(self.server._user_infos))
         self.assertTrue('root' in self.server._user_infos)
 
+    def test_check_file(self):
+        # Just some file that we know exists
+        file_name = BUILD_FILE_PATH + 'cmdctl-keyfile.pem'
+        check_file(file_name)
+        with UnreadableFile(file_name):
+            self.assertRaises(CmdctlException, check_file, file_name)
+        self.assertRaises(CmdctlException, check_file, '/local/not-exist')
+        self.assertRaises(CmdctlException, check_file, '/')
+
+
     def test_check_key_and_cert(self):
+        keyfile = BUILD_FILE_PATH + 'cmdctl-keyfile.pem'
+        certfile = BUILD_FILE_PATH + 'cmdctl-certfile.pem'
+
+        # no exists
+        self.assertRaises(CmdctlException, self.server._check_key_and_cert,
+                          keyfile, '/local/not-exist')
+        self.assertRaises(CmdctlException, self.server._check_key_and_cert,
+                         '/local/not-exist', certfile)
+
+        # not a file
+        self.assertRaises(CmdctlException, self.server._check_key_and_cert,
+                          keyfile, '/')
         self.assertRaises(CmdctlException, self.server._check_key_and_cert,
-                         '/local/not-exist', 'cmdctl-keyfile.pem')
+                         '/', certfile)
 
-        self.server._check_key_and_cert(SRC_FILE_PATH + 'cmdctl-keyfile.pem',
-                                        SRC_FILE_PATH + 'cmdctl-certfile.pem')
+        # no read permission
+        with UnreadableFile(certfile):
+            self.assertRaises(CmdctlException,
+                              self.server._check_key_and_cert,
+                              keyfile, certfile)
+
+        with UnreadableFile(keyfile):
+            self.assertRaises(CmdctlException,
+                              self.server._check_key_and_cert,
+                              keyfile, certfile)
+
+        # All OK (also happens to check the context code above works)
+        self.server._check_key_and_cert(keyfile, certfile)
 
     def test_wrap_sock_in_ssl_context(self):
         sock = socket.socket()
-        self.assertRaises(socket.error, 
+
+        # Bad files should result in a socket.error raised by our own
+        # code in the basic file checks
+        self.assertRaises(socket.error,
                           self.server._wrap_socket_in_ssl_context,
-                          sock, 
-                          '../cmdctl-keyfile',
-                          '../cmdctl-certfile')
+                          sock,
+                          'no_such_file', 'no_such_file')
 
+        # Using a non-certificate file would cause an SSLError, which
+        # is caught by our code which then raises a basic socket.error
+        self.assertRaises(socket.error,
+                          self.server._wrap_socket_in_ssl_context,
+                          sock,
+                          BUILD_FILE_PATH + 'cmdctl.py',
+                          BUILD_FILE_PATH + 'cmdctl-certfile.pem')
+
+        # Should succeed
         sock1 = socket.socket()
-        self.server._wrap_socket_in_ssl_context(sock1, 
-                          SRC_FILE_PATH + 'cmdctl-keyfile.pem',
-                          SRC_FILE_PATH + 'cmdctl-certfile.pem')
+        ssl_sock = self.server._wrap_socket_in_ssl_context(sock1,
+                                   BUILD_FILE_PATH + 'cmdctl-keyfile.pem',
+                                   BUILD_FILE_PATH + 'cmdctl-certfile.pem')
+        self.assertTrue(isinstance(ssl_sock, ssl.SSLSocket))
+
+        # wrap_socket can also raise IOError, which should be caught and
+        # handled like the other errors.
+        # Force this by temporarily disabling our own file checks
+        orig_check_func = self.server._check_key_and_cert
+        try:
+            self.server._check_key_and_cert = lambda x,y: None
+            self.assertRaises(socket.error,
+                              self.server._wrap_socket_in_ssl_context,
+                              sock,
+                              'no_such_file', 'no_such_file')
+        finally:
+            self.server._check_key_and_cert = orig_check_func
 
 class TestFuncNotInClass(unittest.TestCase):
     def test_check_port(self):
-        self.assertRaises(OptionValueError, check_port, None, 'port', -1, None)        
-        self.assertRaises(OptionValueError, check_port, None, 'port', 65536, None)        
-        self.assertRaises(OptionValueError, check_addr, None, 'ipstr', 'a.b.d', None)        
-        self.assertRaises(OptionValueError, check_addr, None, 'ipstr', '1::0:a.b', None)        
+        self.assertRaises(OptionValueError, check_port, None, 'port', -1, None)
+        self.assertRaises(OptionValueError, check_port, None, 'port', 65536, None)
+        self.assertRaises(OptionValueError, check_addr, None, 'ipstr', 'a.b.d', None)
+        self.assertRaises(OptionValueError, check_addr, None, 'ipstr', '1::0:a.b', None)
 
 
 if __name__== "__main__":

+ 16 - 15
src/bin/dhcp4/config_parser.cc

@@ -743,32 +743,33 @@ private:
     void createOption() {
         // Option code is held in the uint32_t storage but is supposed to
         // be uint16_t value. We need to check that value in the configuration
-        // does not exceed range of uint16_t and is not zero.
+        // does not exceed range of uint8_t and is not zero.
         uint32_t option_code = getParam<uint32_t>("code", uint32_values_);
         if (option_code == 0) {
-            isc_throw(DhcpConfigError, "Parser error: value of 'code' must not"
-                      << " be equal to zero. Option code '0' is reserved in"
-                      << " DHCPv4.");
-        } else if (option_code > std::numeric_limits<uint16_t>::max()) {
-            isc_throw(DhcpConfigError, "Parser error: value of 'code' must not"
-                      << " exceed " << std::numeric_limits<uint16_t>::max());
+            isc_throw(DhcpConfigError, "option code must not be zero."
+                      << " Option code '0' is reserved in DHCPv4.");
+        } else if (option_code > std::numeric_limits<uint8_t>::max()) {
+            isc_throw(DhcpConfigError, "invalid option code '" << option_code
+                      << "', it must not exceed '"
+                      << std::numeric_limits<uint8_t>::max() << "'");
         }
         // Check that the option name has been specified, is non-empty and does not
         // contain spaces.
-        // @todo possibly some more restrictions apply here?
         std::string option_name = getParam<std::string>("name", string_values_);
         if (option_name.empty()) {
-            isc_throw(DhcpConfigError, "Parser error: option name must not be"
-                      << " empty");
+            isc_throw(DhcpConfigError, "name of the option with code '"
+                      << option_code << "' is empty");
         } else if (option_name.find(" ") != std::string::npos) {
-            isc_throw(DhcpConfigError, "Parser error: option name must not contain"
-                      << " spaces");
+            isc_throw(DhcpConfigError, "invalid option name '" << option_name
+                      << "', space character is not allowed");
         }
 
         std::string option_space = getParam<std::string>("space", string_values_);
         if (!OptionSpace::validateName(option_space)) {
-            isc_throw(DhcpConfigError, "invalid option space name'"
-                      << option_space << "'");
+            isc_throw(DhcpConfigError, "invalid option space name '"
+                      << option_space << "' specified for option '"
+                      << option_name << "' (code '" << option_code
+                      << "')");
         }
 
         OptionDefinitionPtr def;
@@ -823,7 +824,7 @@ private:
             try {
                 util::encode::decodeHex(option_data, binary);
             } catch (...) {
-                isc_throw(DhcpConfigError, "Parser error: option data is not a valid"
+                isc_throw(DhcpConfigError, "option data is not a valid"
                           << " string of hexadecimal digits: " << option_data);
             }
         }

+ 60 - 10
src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -47,18 +47,61 @@ namespace dhcp {
 ControlledDhcpv4Srv* ControlledDhcpv4Srv::server_ = NULL;
 
 ConstElementPtr
+ControlledDhcpv4Srv::dhcp4StubConfigHandler(ConstElementPtr) {
+    // This configuration handler is intended to be used only
+    // when the initial configuration comes in. To receive this
+    // configuration a pointer to this handler must be passed
+    // using ModuleCCSession constructor. This constructor will
+    // invoke the handler and will store the configuration for
+    // the configuration session when the handler returns success.
+    // Since this configuration is partial we just pretend to
+    // parse it and always return success. The function that
+    // initiates the session must get the configuration on its
+    // own using getFullConfig.
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+ConstElementPtr
 ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr new_config) {
-    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_UPDATE)
-              .arg(new_config->str());
-    if (server_) {
-        return (configureDhcp4Server(*server_, new_config));
+    if (!server_ || !server_->config_session_) {
+        // That should never happen as we install config_handler
+        // after we instantiate the server.
+        ConstElementPtr answer =
+            isc::config::createAnswer(1, "Configuration rejected,"
+                                      " server is during startup/shutdown phase.");
+        return (answer);
     }
 
-    // That should never happen as we install config_handler after we instantiate
-    // the server.
-    ConstElementPtr answer = isc::config::createAnswer(1,
-           "Configuration rejected, server is during startup/shutdown phase.");
-    return (answer);
+    // The configuration passed to this handler function is partial.
+    // In other words, it just includes the values being modified.
+    // In the same time, there are dependencies between various
+    // DHCP configuration parsers. For example: the option value can
+    // be set if the definition of this option is set. If someone removes
+    // an existing option definition then the partial configuration that
+    // removes that definition is triggered while a relevant option value
+    // may remain configured. This eventually results in the DHCP server
+    // configuration being in the inconsistent state.
+    // In order to work around this problem we need to merge the new
+    // configuration with the existing (full) configuration.
+
+    // Let's create a new object that will hold the merged configuration.
+    boost::shared_ptr<MapElement> merged_config(new MapElement());
+    // Let's get the existing configuration.
+    ConstElementPtr full_config = server_->config_session_->getFullConfig();
+    // The full_config and merged_config should be always non-NULL
+    // but to provide some level of exception safety we check that they
+    // really are (in case we go out of memory).
+    if (full_config && merged_config) {
+        merged_config->setValue(full_config->mapValue());
+
+        // Merge an existing and new configuration.
+        isc::data::merge(merged_config, new_config);
+        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_UPDATE)
+            .arg(full_config->str());
+    }
+
+    // Configure the server.
+    return (configureDhcp4Server(*server_, merged_config));
 }
 
 ConstElementPtr
@@ -109,8 +152,15 @@ void ControlledDhcpv4Srv::establishSession() {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_CCSESSION_STARTING)
               .arg(specfile);
     cc_session_ = new Session(io_service_.get_io_service());
+    // Create a session with the dummy configuration handler.
+    // Dumy configuration handler is internally invoked by the
+    // constructor and on success the constructor updates
+    // the current session with the configuration that had been
+    // commited in the previous session. If we did not install
+    // the dummy handler, the previous configuration would have
+    // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,
-                                          NULL,
+                                          dhcp4StubConfigHandler,
                                           dhcp4CommandHandler, false);
     config_session_->start();
 

+ 21 - 0
src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -94,6 +94,27 @@ protected:
     static isc::data::ConstElementPtr
     dhcp4ConfigHandler(isc::data::ConstElementPtr new_config);
 
+    /// @brief A dummy configuration handler that always returns success.
+    ///
+    /// This configuration handler does not perform configuration
+    /// parsing and always returns success. A dummy hanlder should
+    /// be installed using \ref isc::config::ModuleCCSession ctor
+    /// to get the initial configuration. This initial configuration
+    /// comprises values for only those elements that were modified
+    /// the previous session. The \ref dhcp4ConfigHandler can't be
+    /// used to parse the initial configuration because it needs the
+    /// full configuration to satisfy dependencies between the
+    /// various configuration values. Installing the dummy handler
+    /// that guarantees to return success causes initial configuration
+    /// to be stored for the session being created and that it can
+    /// be later accessed with \ref isc::ConfigData::getFullConfig.
+    ///
+    /// @param new_config new configuration.
+    ///
+    /// @return success configuration status.
+    static isc::data::ConstElementPtr
+    dhcp4StubConfigHandler(isc::data::ConstElementPtr new_config);
+
     /// @brief A callback for handling incoming commands.
     ///
     /// @param command textual representation of the command

+ 1 - 1
src/bin/dhcp4/dhcp4.spec

@@ -70,7 +70,7 @@
             "item_default": False
           },
 
-          { "item_name": "record_types",
+          { "item_name": "record-types",
             "item_type": "string",
             "item_optional": false,
             "item_default": ""

+ 86 - 19
src/bin/dhcp4/dhcp4_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 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
@@ -17,6 +17,7 @@
 #include <dhcp/iface_mgr.h>
 #include <dhcp/option4_addrlst.h>
 #include <dhcp/option_int.h>
+#include <dhcp/option_int_array.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/duid.h>
 #include <dhcp/hwaddr.h>
@@ -43,9 +44,6 @@ using namespace std;
 
 // These are hardcoded parameters. Currently this is a skeleton server that only
 // grants those options and a single, fixed, hardcoded lease.
-const std::string HARDCODED_GATEWAY = "192.0.2.1";
-const std::string HARDCODED_DNS_SERVER = "192.0.2.2";
-const std::string HARDCODED_DOMAIN_NAME = "isc.example.com";
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
@@ -341,18 +339,75 @@ void Dhcpv4Srv::appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type) {
 }
 
 
-void Dhcpv4Srv::appendRequestedOptions(Pkt4Ptr& msg) {
-    OptionPtr opt;
+void Dhcpv4Srv::appendRequestedOptions(const Pkt4Ptr& question, Pkt4Ptr& msg) {
+
+    // Get the subnet relevant for the client. We will need it
+    // to get the options associated with it.
+    Subnet4Ptr subnet = selectSubnet(question);
+    // If we can't find the subnet for the client there is no way
+    // to get the options to be sent to a client. We don't log an
+    // error because it will be logged by the assignLease method
+    // anyway.
+    if (!subnet) {
+        return;
+    }
+
+    // try to get the 'Parameter Request List' option which holds the
+    // codes of requested options.
+    OptionUint8ArrayPtr option_prl = boost::dynamic_pointer_cast<
+        OptionUint8Array>(question->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+    // If there is no PRL option in the message from the client then
+    // there is nothing to do.
+    if (!option_prl) {
+        return;
+    }
+
+    // Get the codes of requested options.
+    const std::vector<uint8_t>& requested_opts = option_prl->getValues();
+    // For each requested option code get the instance of the option
+    // to be returned to the client.
+    for (std::vector<uint8_t>::const_iterator opt = requested_opts.begin();
+         opt != requested_opts.end(); ++opt) {
+        Subnet::OptionDescriptor desc =
+            subnet->getOptionDescriptor("dhcp4", *opt);
+        if (desc.option) {
+            msg->addOption(desc.option);
+        }
+    }
+}
 
-    // Domain name (type 15)
-    vector<uint8_t> domain(HARDCODED_DOMAIN_NAME.begin(), HARDCODED_DOMAIN_NAME.end());
-    opt = OptionPtr(new Option(Option::V4, DHO_DOMAIN_NAME, domain));
-    msg->addOption(opt);
-    // TODO: Add Option_String class
+void
+Dhcpv4Srv::appendBasicOptions(const Pkt4Ptr& question, Pkt4Ptr& msg) {
+    // Identify options that we always want to send to the
+    // client (if they are configured).
+    static const uint16_t required_options[] = {
+        DHO_SUBNET_MASK,
+        DHO_ROUTERS,
+        DHO_DOMAIN_NAME_SERVERS,
+        DHO_DOMAIN_NAME };
 
-    // DNS servers (type 6)
-    opt = OptionPtr(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS, IOAddress(HARDCODED_DNS_SERVER)));
-    msg->addOption(opt);
+    static size_t required_options_size =
+        sizeof(required_options) / sizeof(required_options[0]);
+
+    // Get the subnet.
+    Subnet4Ptr subnet = selectSubnet(question);
+    if (!subnet) {
+        return;
+    }
+
+    // Try to find all 'required' options in the outgoing
+    // message. Those that are not present will be added.
+    for (int i = 0; i < required_options_size; ++i) {
+        OptionPtr opt = msg->getOption(required_options[i]);
+        if (!opt) {
+            // Check whether option has been configured.
+            Subnet::OptionDescriptor desc =
+                subnet->getOptionDescriptor("dhcp4", required_options[i]);
+            if (desc.option) {
+                msg->addOption(desc.option);
+            }
+        }
+    }
 }
 
 void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
@@ -424,10 +479,12 @@ void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         opt->setUint32(lease->valid_lft_);
         answer->addOption(opt);
 
-        // @todo: include real router information here
         // Router (type 3)
-        opt = OptionPtr(new Option4AddrLst(DHO_ROUTERS, IOAddress(HARDCODED_GATEWAY)));
-        answer->addOption(opt);
+        Subnet::OptionDescriptor opt_routers =
+            subnet->getOptionDescriptor("dhcp4", DHO_ROUTERS);
+        if (opt_routers.option) {
+            answer->addOption(opt_routers.option);
+        }
 
         // Subnet mask (type 1)
         answer->addOption(getNetmaskOption(subnet));
@@ -466,10 +523,15 @@ Pkt4Ptr Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
 
     copyDefaultFields(discover, offer);
     appendDefaultOptions(offer, DHCPOFFER);
-    appendRequestedOptions(offer);
+    appendRequestedOptions(discover, offer);
 
     assignLease(discover, offer);
 
+    // There are a few basic options that we always want to
+    // include in the response. If client did not request
+    // them we append them for him.
+    appendBasicOptions(discover, offer);
+
     return (offer);
 }
 
@@ -479,10 +541,15 @@ Pkt4Ptr Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
 
     copyDefaultFields(request, ack);
     appendDefaultOptions(ack, DHCPACK);
-    appendRequestedOptions(ack);
+    appendRequestedOptions(request, ack);
 
     assignLease(request, ack);
 
+    // There are a few basic options that we always want to
+    // include in the response. If client did not request
+    // them we append them for him.
+    appendBasicOptions(request, ack);
+
     return (ack);
 }
 

+ 16 - 2
src/bin/dhcp4/dhcp4_srv.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 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
@@ -182,8 +182,9 @@ protected:
     /// This method assigns options that were requested by client
     /// (sent in PRL) or are enforced by server.
     ///
+    /// @param question DISCOVER or REQUEST message from a client.
     /// @param msg outgoing message (options will be added here)
-    void appendRequestedOptions(Pkt4Ptr& msg);
+    void appendRequestedOptions(const Pkt4Ptr& question, Pkt4Ptr& msg);
 
     /// @brief Assigns a lease and appends corresponding options
     ///
@@ -195,6 +196,19 @@ protected:
     /// @param answer OFFER or ACK/NAK message (lease options will be added here)
     void assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer);
 
+    /// @brief Append basic options if they are not present.
+    ///
+    /// This function adds the following basic options if they
+    /// are not yet added to the message:
+    /// - Subnet Mask,
+    /// - Router,
+    /// - Name Server,
+    /// - Domain Name.
+    ///
+    /// @param question DISCOVER or REQUEST message from a client.
+    /// @param msg the message to add options to.
+    void appendBasicOptions(const Pkt4Ptr& question, Pkt4Ptr& msg);
+
     /// @brief Attempts to renew received addresses
     ///
     /// Attempts to renew existing lease. This typically includes finding a lease that

+ 0 - 4
src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -347,7 +347,6 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
         "    \"subnet\": \"192.0.2.0/24\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 
@@ -381,7 +380,6 @@ TEST_F(Dhcp4ParserTest, subnetLocal) {
         "    \"valid-lifetime\": 4,"
         "    \"subnet\": \"192.0.2.0/24\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 
@@ -410,7 +408,6 @@ TEST_F(Dhcp4ParserTest, poolOutOfSubnet) {
         "    \"pool\": [ \"192.0.4.0/28\" ],"
         "    \"subnet\": \"192.0.2.0/24\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 
@@ -435,7 +432,6 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) {
         "    \"pool\": [ \"192.0.2.128/28\" ],"
         "    \"subnet\": \"192.0.2.0/24\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 

+ 180 - 10
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -18,6 +18,9 @@
 #include <asiolink/io_address.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/option.h>
+#include <dhcp/option4_addrlst.h>
+#include <dhcp/option_custom.h>
+#include <dhcp/option_int_array.h>
 #include <dhcp4/dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcpsrv/cfgmgr.h>
@@ -74,14 +77,82 @@ public:
         CfgMgr::instance().deleteSubnets4();
         CfgMgr::instance().addSubnet4(subnet_);
 
+        // Add Router option.
+        Option4AddrLstPtr opt_routers(new Option4AddrLst(DHO_ROUTERS));
+        opt_routers->setAddress(IOAddress("192.0.2.2"));
+        subnet_->addOption(opt_routers, false, "dhcp4");
+
         // it's ok if that fails. There should not be such a file anyway
         unlink(SRVID_FILE);
     }
 
+    /// @brief Add 'Parameter Request List' option to the packet.
+    ///
+    /// This function PRL option comprising the following option codes:
+    /// - 5 - Name Server
+    /// - 15 - Domain Name
+    /// - 7 - Log Server
+    /// - 8 - Quotes Server
+    /// - 9 - LPR Server
+    ///
+    /// @param pkt packet to add PRL option to.
+    void addPrlOption(Pkt4Ptr& pkt) {
+
+        OptionUint8ArrayPtr option_prl =
+            OptionUint8ArrayPtr(new OptionUint8Array(Option::V4,
+                                                     DHO_DHCP_PARAMETER_REQUEST_LIST));
+
+        // Let's request options that have been configured for the subnet.
+        option_prl->addValue(DHO_DOMAIN_NAME_SERVERS);
+        option_prl->addValue(DHO_DOMAIN_NAME);
+        option_prl->addValue(DHO_LOG_SERVERS);
+        option_prl->addValue(DHO_COOKIE_SERVERS);
+        // Let's also request the option that hasn't been configured. In such
+        // case server should ignore request for this particular option.
+        option_prl->addValue(DHO_LPR_SERVERS);
+        // And add 'Parameter Request List' option into the DISCOVER packet.
+        pkt->addOption(option_prl);
+    }
+
+    /// @brief Configures options being requested in the PRL option.
+    ///
+    /// The lpr-servers option is NOT configured here altough it is
+    /// added to the 'Parameter Request List' option in the
+    /// \ref addPrlOption. When requested option is not configured
+    /// the server should not return it in its rensponse. The goal
+    /// of not configuring the requested option is to verify that
+    /// the server will not return it.
+    void configureRequestedOptions() {
+        // dns-servers
+        Option4AddrLstPtr
+            option_dns_servers(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS));
+        option_dns_servers->addAddress(IOAddress("192.0.2.1"));
+        option_dns_servers->addAddress(IOAddress("192.0.2.100"));
+        ASSERT_NO_THROW(subnet_->addOption(option_dns_servers, false, "dhcp4"));
+
+        // domain-name
+        OptionDefinition def("domain-name", DHO_DOMAIN_NAME, OPT_FQDN_TYPE);
+        boost::shared_ptr<OptionCustom>
+            option_domain_name(new OptionCustom(def, Option::V4));
+        option_domain_name->writeFqdn("example.com");
+        subnet_->addOption(option_domain_name, false, "dhcp4");
+
+        // log-servers
+        Option4AddrLstPtr option_log_servers(new Option4AddrLst(DHO_LOG_SERVERS));
+        option_log_servers->addAddress(IOAddress("192.0.2.2"));
+        option_log_servers->addAddress(IOAddress("192.0.2.10"));
+        ASSERT_NO_THROW(subnet_->addOption(option_log_servers, false, "dhcp4"));
+
+        // cookie-servers
+        Option4AddrLstPtr option_cookie_servers(new Option4AddrLst(DHO_COOKIE_SERVERS));
+        option_cookie_servers->addAddress(IOAddress("192.0.2.1"));
+        ASSERT_NO_THROW(subnet_->addOption(option_cookie_servers, false, "dhcp4"));
+    }
+
     /// @brief checks that the response matches request
     /// @param q query (client's message)
     /// @param a answer (server's message)
-    void MessageCheck(const boost::shared_ptr<Pkt4>& q,
+    void messageCheck(const boost::shared_ptr<Pkt4>& q,
                       const boost::shared_ptr<Pkt4>& a) {
         ASSERT_TRUE(q);
         ASSERT_TRUE(a);
@@ -91,20 +162,40 @@ public:
         EXPECT_EQ(q->getIndex(),  a->getIndex());
         EXPECT_EQ(q->getGiaddr(), a->getGiaddr());
 
-        // Check that bare minimum of required options are there
+        // Check that bare minimum of required options are there.
+        // We don't check options requested by a client. Those
+        // are checked elsewhere.
         EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
         EXPECT_TRUE(a->getOption(DHO_ROUTERS));
         EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
         EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
         EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
-        EXPECT_TRUE(a->getOption(DHO_ROUTERS));
-        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
-        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
 
         // Check that something is offered
         EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
     }
 
+    /// @brief Check that requested options are present.
+    ///
+    /// @param pkt packet to be checked.
+    void optionsCheck(const Pkt4Ptr& pkt) {
+        // Check that the requested and configured options are returned
+        // in the ACK message.
+        EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME))
+            << "domain-name not present in the response";
+        EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME_SERVERS))
+            << "dns-servers not present in the response";
+        EXPECT_TRUE(pkt->getOption(DHO_LOG_SERVERS))
+            << "log-servers not present in the response";
+        EXPECT_TRUE(pkt->getOption(DHO_COOKIE_SERVERS))
+            << "cookie-servers not present in the response";
+        // Check that the requested but not configured options are not
+        // returned in the ACK message.
+        EXPECT_FALSE(pkt->getOption(DHO_LPR_SERVERS))
+            << "domain-name present in the response but it is"
+            << " expected not to be present";
+    }
+
     /// @brief generates client-id option
     ///
     /// Generate client-id option of specified length
@@ -324,6 +415,12 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
     pkt->setHops(3);
     pkt->setRemotePort(DHCP4_SERVER_PORT);
 
+    // We are going to test that certain options are returned
+    // (or not returned) in the OFFER message when requested
+    // using 'Parameter Request List' option. Let's configure
+    // those options that are returned when requested.
+    configureRequestedOptions();
+
     // Should not throw
     EXPECT_NO_THROW(
         offer = srv->processDiscover(pkt);
@@ -337,7 +434,39 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
     // This is relayed message. It should be sent back to relay address.
     EXPECT_EQ(pkt->getGiaddr(), offer->getRemoteAddr());
 
-    MessageCheck(pkt, offer);
+    messageCheck(pkt, offer);
+
+    // There are some options that are always present in the
+    // message, even if not requested.
+    EXPECT_TRUE(offer->getOption(DHO_DOMAIN_NAME));
+    EXPECT_TRUE(offer->getOption(DHO_DOMAIN_NAME_SERVERS));
+
+    // We did not request any options so they should not be present
+    // in the OFFER.
+    EXPECT_FALSE(offer->getOption(DHO_LOG_SERVERS));
+    EXPECT_FALSE(offer->getOption(DHO_COOKIE_SERVERS));
+    EXPECT_FALSE(offer->getOption(DHO_LPR_SERVERS));
+
+    // Add 'Parameter Request List' option.
+    addPrlOption(pkt);
+
+    // Now repeat the test but request some options.
+    EXPECT_NO_THROW(
+        offer = srv->processDiscover(pkt);
+    );
+
+    // Should return something
+    ASSERT_TRUE(offer);
+
+    EXPECT_EQ(DHCPOFFER, offer->getType());
+
+    // This is relayed message. It should be sent back to relay address.
+    EXPECT_EQ(pkt->getGiaddr(), offer->getRemoteAddr());
+
+    messageCheck(pkt, offer);
+
+    // Check that the requested options are returned.
+    optionsCheck(offer);
 
     // Now repeat the test for directly sent message
     pkt->setHops(0);
@@ -357,7 +486,10 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
     // to relay.
     EXPECT_EQ(pkt->getRemoteAddr(), offer->getRemoteAddr());
 
-    MessageCheck(pkt, offer);
+    messageCheck(pkt, offer);
+
+    // Check that the requested options are returned.
+    optionsCheck(offer);
 
     delete srv;
 }
@@ -385,6 +517,12 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
     req->setRemoteAddr(IOAddress("192.0.2.56"));
     req->setGiaddr(IOAddress("192.0.2.67"));
 
+    // We are going to test that certain options are returned
+    // in the ACK message when requested using 'Parameter
+    // Request List' option. Let's configure those options that
+    // are returned when requested.
+    configureRequestedOptions();
+
     // Should not throw
     ASSERT_NO_THROW(
         ack = srv->processRequest(req);
@@ -398,7 +536,37 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
     // This is relayed message. It should be sent back to relay address.
     EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
 
-    MessageCheck(req, ack);
+    messageCheck(req, ack);
+
+    // There are some options that are always present in the
+    // message, even if not requested.
+    EXPECT_TRUE(ack->getOption(DHO_DOMAIN_NAME));
+    EXPECT_TRUE(ack->getOption(DHO_DOMAIN_NAME_SERVERS));
+
+    // We did not request any options so these should not be present
+    // in the ACK.
+    EXPECT_FALSE(ack->getOption(DHO_LOG_SERVERS));
+    EXPECT_FALSE(ack->getOption(DHO_COOKIE_SERVERS));
+    EXPECT_FALSE(ack->getOption(DHO_LPR_SERVERS));
+
+    // Add 'Parameter Request List' option.
+    addPrlOption(req);
+
+    // Repeat the test but request some options.
+    ASSERT_NO_THROW(
+        ack = srv->processRequest(req);
+    );
+
+    // Should return something
+    ASSERT_TRUE(ack);
+
+    EXPECT_EQ(DHCPACK, ack->getType());
+
+    // This is relayed message. It should be sent back to relay address.
+    EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
+
+    // Check that the requested options are returned.
+    optionsCheck(ack);
 
     // Now repeat the test for directly sent message
     req->setHops(0);
@@ -418,7 +586,10 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
     // to relay.
     EXPECT_EQ(ack->getRemoteAddr(), req->getRemoteAddr());
 
-    MessageCheck(req, ack);
+    messageCheck(req, ack);
+
+    // Check that the requested options are returned.
+    optionsCheck(ack);
 
     delete srv;
 }
@@ -890,7 +1061,6 @@ TEST_F(Dhcpv4SrvTest, RenewBasic) {
 
     // let's create a lease and put it in the LeaseMgr
     uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
     Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,

+ 13 - 12
src/bin/dhcp6/config_parser.cc

@@ -776,29 +776,30 @@ private:
         // does not exceed range of uint16_t and is not zero.
         uint32_t option_code = getParam<uint32_t>("code", uint32_values_);
         if (option_code == 0) {
-            isc_throw(DhcpConfigError, "Parser error: value of 'code' must not"
-                      << " be equal to zero. Option code '0' is reserved in"
-                      << " DHCPv6.");
+            isc_throw(DhcpConfigError, "option code must not be zero."
+                      << " Option code '0' is reserved in DHCPv6.");
         } else if (option_code > std::numeric_limits<uint16_t>::max()) {
-            isc_throw(DhcpConfigError, "Parser error: value of 'code' must not"
-                      << " exceed " << std::numeric_limits<uint16_t>::max());
+            isc_throw(DhcpConfigError, "invalid option code '" << option_code
+                      << "', it must not exceed '"
+                      << std::numeric_limits<uint16_t>::max() << "'");
         }
         // Check that the option name has been specified, is non-empty and does not
         // contain spaces.
-        // @todo possibly some more restrictions apply here?
         std::string option_name = getParam<std::string>("name", string_values_);
         if (option_name.empty()) {
-            isc_throw(DhcpConfigError, "Parser error: option name must not be"
-                      << " empty");
+            isc_throw(DhcpConfigError, "name of the option with code '"
+                      << option_code << "' is empty");
         } else if (option_name.find(" ") != std::string::npos) {
-            isc_throw(DhcpConfigError, "Parser error: option name must not contain"
-                      << " spaces");
+            isc_throw(DhcpConfigError, "invalid option name '" << option_name
+                      << "', space character is not allowed");
         }
 
         std::string option_space = getParam<std::string>("space", string_values_);
         if (!OptionSpace::validateName(option_space)) {
-            isc_throw(DhcpConfigError, "invalid option space name'"
-                      << option_space << "'");
+            isc_throw(DhcpConfigError, "invalid option space name '"
+                      << option_space << "' specified for option '"
+                      << option_name << "' (code '" << option_code
+                      << "')");
         }
 
         OptionDefinitionPtr def;

+ 65 - 14
src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -45,19 +45,62 @@ namespace dhcp {
 ControlledDhcpv6Srv* ControlledDhcpv6Srv::server_ = NULL;
 
 ConstElementPtr
+ControlledDhcpv6Srv::dhcp6StubConfigHandler(ConstElementPtr) {
+    // This configuration handler is intended to be used only
+    // when the initial configuration comes in. To receive this
+    // configuration a pointer to this handler must be passed
+    // using ModuleCCSession constructor. This constructor will
+    // invoke the handler and will store the configuration for
+    // the configuration session when the handler returns success.
+    // Since this configuration is partial we just pretend to
+    // parse it and always return success. The function that
+    // initiates the session must get the configuration on its
+    // own using getFullConfig.
+    return (isc::config::createAnswer(0, "Configuration accepted."));
+}
+
+ConstElementPtr
 ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr new_config) {
-    LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_UPDATE)
-              .arg(new_config->str());
 
-    if (server_) {
-        return (configureDhcp6Server(*server_, new_config));
+    if (!server_ || !server_->config_session_) {
+        // That should never happen as we install config_handler
+        // after we instantiate the server.
+        ConstElementPtr answer =
+            isc::config::createAnswer(1, "Configuration rejected,"
+                                      " server is during startup/shutdown phase.");
+        return (answer);
     }
 
-    // That should never happen as we install config_handler after we instantiate
-    // the server.
-    ConstElementPtr answer = isc::config::createAnswer(1,
-           "Configuration rejected, server is during startup/shutdown phase.");
-    return (answer);
+    // The configuration passed to this handler function is partial.
+    // In other words, it just includes the values being modified.
+    // In the same time, there are dependencies between various
+    // DHCP configuration parsers. For example: the option value can
+    // be set if the definition of this option is set. If someone removes
+    // an existing option definition then the partial configuration that
+    // removes that definition is triggered while a relevant option value
+    // may remain configured. This eventually results in the DHCP server
+    // configuration being in the inconsistent state.
+    // In order to work around this problem we need to merge the new
+    // configuration with the existing (full) configuration.
+
+    // Let's create a new object that will hold the merged configuration.
+    boost::shared_ptr<MapElement> merged_config(new MapElement());
+    // Let's get the existing configuration.
+    ConstElementPtr full_config = server_->config_session_->getFullConfig();
+    // The full_config and merged_config should be always non-NULL
+    // but to provide some level of exception safety we check that they
+    // really are (in case we go out of memory).
+    if (full_config && merged_config) {
+        merged_config->setValue(full_config->mapValue());
+
+        // Merge an existing and new configuration.
+        isc::data::merge(merged_config, new_config);
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_UPDATE)
+            .arg(merged_config->str());
+    }
+
+    // Configure the server.
+    return (configureDhcp6Server(*server_, merged_config));
 }
 
 ConstElementPtr
@@ -108,18 +151,26 @@ void ControlledDhcpv6Srv::establishSession() {
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_CCSESSION_STARTING)
               .arg(specfile);
     cc_session_ = new Session(io_service_.get_io_service());
+    // Create a session with the dummy configuration handler.
+    // Dumy configuration handler is internally invoked by the
+    // constructor and on success the constructor updates
+    // the current session with the configuration that had been
+    // commited in the previous session. If we did not install
+    // the dummy handler, the previous configuration would have
+    // been lost.
     config_session_ = new ModuleCCSession(specfile, *cc_session_,
-                                          NULL,
+                                          dhcp6StubConfigHandler,
                                           dhcp6CommandHandler, false);
     config_session_->start();
 
-    // We initially create ModuleCCSession() without configHandler, as
-    // the session module is too eager to send partial configuration.
-    // We want to get the full configuration, so we explicitly call
-    // getFullConfig() and then pass it to our configHandler.
+    // The constructor already pulled the configuration that had
+    // been created in the previous session thanks to the dummy
+    // handler. We can switch to the handler that will be
+    // parsing future changes to the configuration.
     config_session_->setConfigHandler(dhcp6ConfigHandler);
 
     try {
+        // Pull the full configuration out from the session.
         configureDhcp6Server(*this, config_session_->getFullConfig());
     } catch (const DhcpConfigError& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(ex.what());

+ 21 - 0
src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -92,6 +92,27 @@ protected:
     static isc::data::ConstElementPtr
     dhcp6ConfigHandler(isc::data::ConstElementPtr new_config);
 
+    /// @brief A dummy configuration handler that always returns success.
+    ///
+    /// This configuration handler does not perform configuration
+    /// parsing and always returns success. A dummy hanlder should
+    /// be installed using \ref isc::config::ModuleCCSession ctor
+    /// to get the initial configuration. This initial configuration
+    /// comprises values for only those elements that were modified
+    /// the previous session. The \ref dhcp6ConfigHandler can't be
+    /// used to parse the initial configuration because it needs the
+    /// full configuration to satisfy dependencies between the
+    /// various configuration values. Installing the dummy handler
+    /// that guarantees to return success causes initial configuration
+    /// to be stored for the session being created and that it can
+    /// be later accessed with \ref isc::ConfigData::getFullConfig.
+    ///
+    /// @param new_config new configuration.
+    ///
+    /// @return success configuration status.
+    static isc::data::ConstElementPtr
+    dhcp6StubConfigHandler(isc::data::ConstElementPtr new_config);
+
     /// @brief A callback for handling incoming commands.
     ///
     /// @param command textual representation of the command

+ 1 - 1
src/bin/dhcp6/dhcp6.spec

@@ -76,7 +76,7 @@
             "item_default": False
           },
 
-          { "item_name": "record_types",
+          { "item_name": "record-types",
             "item_type": "string",
             "item_optional": false,
             "item_default": ""

+ 1 - 1
src/bin/dhcp6/dhcp6_srv.cc

@@ -205,7 +205,7 @@ bool Dhcpv6Srv::run() {
 
                 LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL_DATA,
                           DHCP6_RESPONSE_DATA)
-                          .arg(rsp->getType()).arg(rsp->toText());
+                    .arg(static_cast<int>(rsp->getType())).arg(rsp->toText());
 
                 if (rsp->pack()) {
                     try {

+ 1 - 4
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -351,7 +351,6 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) {
         "    \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ],"
         "    \"subnet\": \"2001:db8:1::/64\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 
@@ -390,7 +389,6 @@ TEST_F(Dhcp6ParserTest, subnetLocal) {
         "    \"valid-lifetime\": 4,"
         "    \"subnet\": \"2001:db8:1::/64\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 
@@ -516,7 +514,7 @@ TEST_F(Dhcp6ParserTest, poolOutOfSubnet) {
         "    \"pool\": [ \"4001:db8:1::/80\" ],"
         "    \"subnet\": \"2001:db8:1::/64\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
+
 
     ElementPtr json = Element::fromJSON(config);
 
@@ -544,7 +542,6 @@ TEST_F(Dhcp6ParserTest, poolPrefixLen) {
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
         "    \"subnet\": \"2001:db8:1::/64\" } ],"
         "\"valid-lifetime\": 4000 }";
-    cout << config << endl;
 
     ElementPtr json = Element::fromJSON(config);
 

+ 4 - 0
src/bin/xfrin/xfrin_messages.mes

@@ -175,6 +175,10 @@ exception message is printed in the log message.
 The XFR transfer for the given zone has failed due to a problem outside
 of the xfrin module.  Possible reasons are a broken DNS message or failure
 in database connection.  The error is shown in the log message.
+One common cause of this error could be a locked database; especially when
+using sqlite3 where a single transaction involving write operations blocks
+any other read or write transactions. This is not a critical error, and
+the transfer will be attempted again at the next retry time.
 
 % XFRIN_XFR_PROCESS_FAILURE %1 transfer of zone %2/%3 failed: %4
 An XFR session failed outside the main protocol handling.  This

+ 4 - 2
src/lib/datasrc/Makefile.am

@@ -28,10 +28,12 @@ libb10_datasrc_la_SOURCES += rbnode_rrset.h
 libb10_datasrc_la_SOURCES += rbtree.h
 libb10_datasrc_la_SOURCES += exceptions.h
 libb10_datasrc_la_SOURCES += zonetable.h zonetable.cc
-libb10_datasrc_la_SOURCES += zone.h zone_finder.cc zone_finder_context.cc
+libb10_datasrc_la_SOURCES += zone.h zone_finder.h zone_finder.cc
+libb10_datasrc_la_SOURCES += zone_finder_context.cc
+libb10_datasrc_la_SOURCES += zone_iterator.h
 libb10_datasrc_la_SOURCES += result.h
 libb10_datasrc_la_SOURCES += logger.h logger.cc
-libb10_datasrc_la_SOURCES += client.h client.cc iterator.h
+libb10_datasrc_la_SOURCES += client.h client.cc
 libb10_datasrc_la_SOURCES += database.h database.cc
 libb10_datasrc_la_SOURCES += factory.h factory.cc
 libb10_datasrc_la_SOURCES += client_list.h client_list.cc

+ 2 - 1
src/lib/datasrc/client.h

@@ -21,6 +21,7 @@
 #include <boost/shared_ptr.hpp>
 
 #include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 
 /// \file
 /// Datasource clients
@@ -66,7 +67,7 @@
 namespace isc {
 namespace datasrc {
 
-// The iterator.h is not included on purpose, most application won't need it
+// zone_iterator.h is not included on purpose, most application won't need it
 class ZoneIterator;
 typedef boost::shared_ptr<ZoneIterator> ZoneIteratorPtr;
 

+ 1 - 1
src/lib/datasrc/database.cc

@@ -18,7 +18,7 @@
 
 #include <datasrc/database.h>
 #include <datasrc/data_source.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/rrset_collection_base.h>
 
 #include <exceptions/exceptions.h>

+ 1 - 1
src/lib/datasrc/memory/memory_client.h

@@ -17,7 +17,7 @@
 
 #include <util/memory_segment.h>
 
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/client.h>
 #include <datasrc/memory/zone_table.h>
 #include <datasrc/memory/zone_data.h>

+ 1 - 1
src/lib/datasrc/memory/zone_data_loader.h

@@ -17,7 +17,7 @@
 
 #include <datasrc/exceptions.h>
 #include <datasrc/memory/zone_data.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <util/memory_segment.h>

+ 1 - 1
src/lib/datasrc/memory/zone_finder.cc

@@ -17,7 +17,7 @@
 #include <datasrc/memory/treenode_rrset.h>
 #include <datasrc/memory/rdata_serialization.h>
 
-#include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 #include <datasrc/data_source.h>
 #include <dns/labelsequence.h>
 #include <dns/name.h>

+ 1 - 1
src/lib/datasrc/memory/zone_finder.h

@@ -18,7 +18,7 @@
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/treenode_rrset.h>
 
-#include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 #include <dns/name.h>
 #include <dns/rrset.h>
 #include <dns/rrtype.h>

+ 2 - 1
src/lib/datasrc/memory_datasrc.cc

@@ -26,9 +26,10 @@
 #include <datasrc/rbtree.h>
 #include <datasrc/rbnode_rrset.h>
 #include <datasrc/logger.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/data_source.h>
 #include <datasrc/factory.h>
+#include <datasrc/zone_finder.h>
 
 #include <boost/function.hpp>
 #include <boost/shared_ptr.hpp>

+ 1 - 0
src/lib/datasrc/rrset_collection_base.cc

@@ -14,6 +14,7 @@
 
 #include <datasrc/rrset_collection_base.h>
 #include <datasrc/zone_loader.h>
+#include <datasrc/zone_finder.h>
 #include <exceptions/exceptions.h>
 
 using namespace isc;

+ 1 - 1
src/lib/datasrc/tests/client_list_unittest.cc

@@ -14,7 +14,7 @@
 
 #include <datasrc/client_list.h>
 #include <datasrc/client.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/data_source.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_table_segment.h>

+ 234 - 31
src/lib/datasrc/tests/database_unittest.cc

@@ -25,8 +25,9 @@
 
 #include <datasrc/database.h>
 #include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 #include <datasrc/data_source.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/sqlite3_accessor.h>
 
 #include <testutils/dnsmessage_test.h>
@@ -1728,58 +1729,107 @@ TYPED_TEST(DatabaseClientTest, updateAfterDeleteIterator) {
 }
 
 void
-doFindTest(ZoneFinder& finder,
-           const isc::dns::Name& name,
-           const isc::dns::RRType& type,
-           const isc::dns::RRType& expected_type,
-           const isc::dns::RRTTL expected_ttl,
-           ZoneFinder::Result expected_result,
-           const std::vector<std::string>& expected_rdatas,
-           const std::vector<std::string>& expected_sig_rdatas,
-           ZoneFinder::FindResultFlags expected_flags =
-           ZoneFinder::RESULT_DEFAULT,
-           const isc::dns::Name& expected_name = isc::dns::Name::ROOT_NAME(),
-           const ZoneFinder::FindOptions options = ZoneFinder::FIND_DEFAULT)
+findTestCommon(ZoneFinder& finder, const isc::dns::Name& name,
+               const isc::dns::RRType& type,
+               ConstZoneFinderContextPtr actual_result,
+               const isc::dns::RRType& expected_type,
+               const isc::dns::RRTTL expected_ttl,
+               ZoneFinder::Result expected_result,
+               const std::vector<string>& expected_rdatas,
+               const std::vector<string>& expected_sig_rdatas,
+               ZoneFinder::FindResultFlags expected_flags,
+               const isc::dns::Name& expected_name,
+               const ZoneFinder::FindOptions options)
 {
-    SCOPED_TRACE("doFindTest " + name.toText() + " " + type.toText());
-    ConstZoneFinderContextPtr result = finder.find(name, type, options);
-    ASSERT_EQ(expected_result, result->code) << name << " " << type;
+    ASSERT_EQ(expected_result, actual_result->code) << name << " " << type;
     EXPECT_EQ((expected_flags & ZoneFinder::RESULT_WILDCARD) != 0,
-              result->isWildcard());
+              actual_result->isWildcard());
     EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0,
-              result->isNSECSigned());
+              actual_result->isNSECSigned());
     EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0,
-              result->isNSEC3Signed());
-    if (!expected_rdatas.empty() && result->rrset) {
-        checkRRset(result->rrset, expected_name != Name(".") ? expected_name :
+              actual_result->isNSEC3Signed());
+    if (!expected_rdatas.empty() && actual_result->rrset) {
+        checkRRset(actual_result->rrset,
+                   expected_name != Name::ROOT_NAME() ? expected_name :
                    name, finder.getClass(), expected_type, expected_ttl,
                    expected_rdatas);
         if ((options & ZoneFinder::FIND_DNSSEC) == ZoneFinder::FIND_DNSSEC) {
-            if (!expected_sig_rdatas.empty() && result->rrset->getRRsig()) {
-                checkRRset(result->rrset->getRRsig(),
-                           expected_name != Name(".") ? expected_name : name,
+            if (!expected_sig_rdatas.empty() &&
+                actual_result->rrset->getRRsig()) {
+                checkRRset(actual_result->rrset->getRRsig(),
+                           expected_name != Name::ROOT_NAME() ?
+                           expected_name : name,
                            finder.getClass(),
                            isc::dns::RRType::RRSIG(), expected_ttl,
                            expected_sig_rdatas);
             } else if (expected_sig_rdatas.empty()) {
-                EXPECT_EQ(isc::dns::RRsetPtr(), result->rrset->getRRsig()) <<
-                    "Unexpected RRSIG: " << result->rrset->getRRsig()->toText();
+                EXPECT_EQ(isc::dns::RRsetPtr(),
+                          actual_result->rrset->getRRsig()) <<
+                    "Unexpected RRSIG: " <<
+                    actual_result->rrset->getRRsig()->toText();
             } else {
                 ADD_FAILURE() << "Missing RRSIG";
             }
-        } else if (result->rrset->getRRsig()) {
-            EXPECT_EQ(isc::dns::RRsetPtr(), result->rrset->getRRsig()) <<
-                "Unexpected RRSIG: " << result->rrset->getRRsig()->toText();
+        } else if (actual_result->rrset->getRRsig()) {
+            EXPECT_EQ(isc::dns::RRsetPtr(), actual_result->rrset->getRRsig())
+                << "Unexpected RRSIG: "
+                << actual_result->rrset->getRRsig()->toText();
         }
     } else if (expected_rdatas.empty()) {
-        EXPECT_EQ(isc::dns::RRsetPtr(), result->rrset) <<
-            "Unexpected RRset: " << result->rrset->toText();
+        EXPECT_EQ(isc::dns::RRsetPtr(), actual_result->rrset) <<
+            "Unexpected RRset: " << actual_result->rrset->toText();
     } else {
         ADD_FAILURE() << "Missing result";
     }
 }
 
 void
+doFindTest(ZoneFinder& finder,
+           const isc::dns::Name& name,
+           const isc::dns::RRType& type,
+           const isc::dns::RRType& expected_type,
+           const isc::dns::RRTTL expected_ttl,
+           ZoneFinder::Result expected_result,
+           const std::vector<std::string>& expected_rdatas,
+           const std::vector<std::string>& expected_sig_rdatas,
+           ZoneFinder::FindResultFlags expected_flags =
+           ZoneFinder::RESULT_DEFAULT,
+           const isc::dns::Name& expected_name = isc::dns::Name::ROOT_NAME(),
+           const ZoneFinder::FindOptions options = ZoneFinder::FIND_DEFAULT)
+{
+    SCOPED_TRACE("doFindTest " + name.toText() + " " + type.toText());
+    ConstZoneFinderContextPtr result = finder.find(name, type, options);
+    findTestCommon(finder, name, type, result, expected_type, expected_ttl,
+                   expected_result, expected_rdatas, expected_sig_rdatas,
+                   expected_flags, expected_name, options);
+}
+
+void
+doFindAtOriginTest(ZoneFinder& finder,
+                   const isc::dns::Name& origin,
+                   const isc::dns::RRType& type,
+                   const isc::dns::RRType& expected_type,
+                   const isc::dns::RRTTL expected_ttl,
+                   ZoneFinder::Result expected_result,
+                   const std::vector<std::string>& expected_rdatas,
+                   const std::vector<std::string>& expected_sig_rdatas,
+                   bool use_minttl = false,
+                   ZoneFinder::FindResultFlags expected_flags =
+                   ZoneFinder::RESULT_DEFAULT,
+                   const isc::dns::Name& expected_name =
+                   isc::dns::Name::ROOT_NAME(),
+                   const ZoneFinder::FindOptions options =
+                   ZoneFinder::FIND_DEFAULT)
+{
+    SCOPED_TRACE("doFindOriginTest " + origin.toText() + " " + type.toText());
+    ConstZoneFinderContextPtr result =
+        finder.findAtOrigin(type, use_minttl, options);
+    findTestCommon(finder, origin, type, result, expected_type, expected_ttl,
+                   expected_result, expected_rdatas, expected_sig_rdatas,
+                   expected_flags, expected_name, options);
+}
+
+void
 doFindAllTestResult(ZoneFinder& finder, const isc::dns::Name& name,
                     ZoneFinder::Result expected_result,
                     const isc::dns::RRType expected_type,
@@ -2116,6 +2166,159 @@ TYPED_TEST(DatabaseClientTest, find) {
                this->expected_rdatas_, this->expected_sig_rdatas_);
 }
 
+TYPED_TEST(DatabaseClientTest, findAtOrigin) {
+    ZoneFinderPtr finder(this->getFinder());
+
+    // Specified type of RR exists, no DNSSEC
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("ns.example.com.");
+    doFindAtOriginTest(*finder, this->zname_, RRType::NS(), RRType::NS(),
+                       this->rrttl_, ZoneFinder::SUCCESS,
+                       this->expected_rdatas_, this->expected_sig_rdatas_);
+
+    // Specified type of RR exists, with DNSSEC
+    this->expected_sig_rdatas_.push_back("NS 5 3 3600 20000101000000 "
+                                         "20000201000000 12345 example.org. "
+                                         "FAKEFAKEFAKE");
+    doFindAtOriginTest(*finder, this->zname_, RRType::NS(), RRType::NS(),
+                       this->rrttl_, ZoneFinder::SUCCESS,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       false, ZoneFinder::RESULT_DEFAULT, this->zname_,
+                       ZoneFinder::FIND_DNSSEC);
+
+    // Specified type of RR doesn't exist, no DNSSEC
+    this->expected_rdatas_.clear();
+    this->expected_sig_rdatas_.clear();
+    doFindAtOriginTest(*finder, this->zname_, RRType::TXT(), this->qtype_,
+                       this->rrttl_, ZoneFinder::NXRRSET,
+                       this->expected_rdatas_, this->expected_sig_rdatas_);
+
+    // Specified type of RR doesn't exist, with DNSSEC
+    this->expected_rdatas_.clear();
+    this->expected_sig_rdatas_.clear();
+    this->expected_rdatas_.push_back(
+        "acnamesig1.example.org. A NS RRSIG NSEC");
+    this->expected_sig_rdatas_.push_back("NSEC 5 3 3600 20000101000000 "
+                                         "20000201000000 12345 example.org. "
+                                         "FAKEFAKEFAKE");
+    doFindAtOriginTest(*finder, this->zname_, RRType::TXT(), RRType::NSEC(),
+                       this->rrttl_, ZoneFinder::NXRRSET,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       false, ZoneFinder::RESULT_NSEC_SIGNED,
+                       this->zname_, ZoneFinder::FIND_DNSSEC);
+
+    // Specified type of RR doesn't exist, with DNSSEC, enabling NSEC3
+    this->current_accessor_->enableNSEC3();
+    this->expected_rdatas_.clear();
+    this->expected_sig_rdatas_.clear();
+    doFindAtOriginTest(*finder, this->zname_, RRType::TXT(), RRType::TXT(),
+                       this->rrttl_, ZoneFinder::NXRRSET,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       false, ZoneFinder::RESULT_NSEC3_SIGNED,
+                       this->zname_, ZoneFinder::FIND_DNSSEC);
+}
+
+TYPED_TEST(DatabaseClientTest, findAtOriginWithMinTTL) {
+    // First, replace the SOA of the test zone so that its RR TTL is larger
+    // than MINTTL (the original data are used in many places, so replacing
+    // them just for this doesn't make sense).
+    RRsetPtr old_soa(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                               this->rrttl_));
+    old_soa->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                         "ns1.example.org. admin.example.org. "
+                                         "1234 3600 1800 2419200 7200"));
+
+    const string new_soa_rdata = "ns1.example.org. admin.example.org. "
+        "1234 3600 1800 2419200 1200";
+    RRsetPtr new_soa(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                               this->rrttl_));
+    new_soa->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                         new_soa_rdata));
+
+    this->updater_ = this->client_->getUpdater(this->zname_, false);
+    this->updater_->deleteRRset(*old_soa);
+    this->updater_->addRRset(*new_soa);
+    this->updater_->commit();
+
+    ZoneFinderPtr finder = this->getFinder();
+
+    // Specify the use of min TTL, then the resulting TTL should be derived
+    // from the SOA MINTTL (which is smaller).
+    this->expected_rdatas_.push_back(new_soa_rdata);
+    doFindAtOriginTest(*finder, this->zname_, RRType::SOA(), RRType::SOA(),
+                       RRTTL(1200), ZoneFinder::SUCCESS,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       true);
+
+    // If DNSSEC is requested, TTL of the RRSIG should also be the min.
+    this->expected_sig_rdatas_.push_back(
+        "SOA 5 3 3600 20000101000000 "
+        "20000201000000 12345 example.org. FAKEFAKEFAKE");
+    doFindAtOriginTest(*finder, this->zname_, RRType::SOA(), RRType::SOA(),
+                       RRTTL(1200), ZoneFinder::SUCCESS,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       true, ZoneFinder::RESULT_DEFAULT, this->zname_,
+                       ZoneFinder::FIND_DNSSEC);
+
+    // Not really intended usage, but specify the use of min TTL for non SOA.
+    // It should still work as specified.
+    this->expected_rdatas_.clear();
+    this->expected_sig_rdatas_.clear();
+    this->expected_rdatas_.push_back("ns.example.com.");
+    doFindAtOriginTest(*finder, this->zname_, RRType::NS(), RRType::NS(),
+                       RRTTL(1200), ZoneFinder::SUCCESS,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       true);
+
+    // If we don't request the use of min TTL, the original TTL will be used.
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back(new_soa_rdata);
+    doFindAtOriginTest(*finder, this->zname_, RRType::SOA(), RRType::SOA(),
+                       this->rrttl_, ZoneFinder::SUCCESS,
+                       this->expected_rdatas_, this->expected_sig_rdatas_);
+
+    // If no RRset is returned, use_minttl doesn't matter (it shouldn't cause
+    // disruption)
+    this->expected_rdatas_.clear();
+    doFindAtOriginTest(*finder, this->zname_, RRType::TXT(), this->qtype_,
+                       this->rrttl_, ZoneFinder::NXRRSET,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       true);
+
+    // If it results in NXRRSET with NSEC, and if we specify the use of min
+    // TTL, the NSEC and RRSIG should have the min TTL (again, though, this
+    // use case is not really the intended one)
+    this->expected_rdatas_.push_back(
+        "acnamesig1.example.org. A NS RRSIG NSEC");
+    this->expected_sig_rdatas_.push_back("NSEC 5 3 3600 20000101000000 "
+                                         "20000201000000 12345 example.org. "
+                                         "FAKEFAKEFAKE");
+    doFindAtOriginTest(*finder, this->zname_, RRType::TXT(), RRType::NSEC(),
+                       RRTTL(1200), ZoneFinder::NXRRSET,
+                       this->expected_rdatas_, this->expected_sig_rdatas_,
+                       true, ZoneFinder::RESULT_NSEC_SIGNED,
+                       this->zname_, ZoneFinder::FIND_DNSSEC);
+}
+
+TYPED_TEST(DatabaseClientTest, findAtOriginWithMinTTLBroken) {
+    // Similar to the previous case, but we intentionally remove the SOA
+    // (assuming the underlying data source doesn't complain about it).
+    // This will cause exception in subsequent findAtOrigin() with use_minttl
+    // being true.
+    RRsetPtr old_soa(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                               this->rrttl_));
+    old_soa->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                         "ns1.example.org. admin.example.org. "
+                                         "1234 3600 1800 2419200 7200"));
+    this->updater_ = this->client_->getUpdater(this->zname_, false);
+    this->updater_->deleteRRset(*old_soa);
+    this->updater_->commit();
+
+    EXPECT_THROW(this->getFinder()->findAtOrigin(RRType::NS(), true,
+                                                 ZoneFinder::FIND_DEFAULT),
+                 DataSourceError);
+}
+
 TYPED_TEST(DatabaseClientTest, findOutOfZone) {
     // If the query name is out-of-zone it should result in an exception
     boost::shared_ptr<DatabaseClient::Finder> finder(this->getFinder());

+ 1 - 1
src/lib/datasrc/tests/faked_nsec3.h

@@ -15,7 +15,7 @@
 #ifndef FAKED_NSEC3_H
 #define FAKED_NSEC3_H
 
-#include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 
 #include <dns/nsec3hash.h>
 

+ 1 - 1
src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -28,7 +28,7 @@
 #include <datasrc/client.h>
 #include <datasrc/memory_datasrc.h>
 #include <datasrc/data_source.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 
 #include "test_client.h"
 

+ 1 - 1
src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -18,7 +18,7 @@
 #include <dns/name.h>
 #include <dns/rrclass.h>
 
-#include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 #include <datasrc/memory/memory_client.h>
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/database.h>

+ 2 - 713
src/lib/datasrc/zone.h

@@ -24,726 +24,15 @@
 #include <datasrc/rrset_collection_base.h>
 
 #include <utility>
-#include <vector>
 
 namespace isc {
 namespace datasrc {
 
-/// \brief Out of zone exception
-///
-/// This is thrown when a method is called for a name or RRset which
-/// is not in or below the zone.
-class OutOfZone : public ZoneException {
-public:
-    OutOfZone(const char* file, size_t line, const char* what) :
-        ZoneException(file, line, what) {}
-};
-
-/// \brief The base class to search a zone for RRsets
-///
-/// The \c ZoneFinder class is an abstract base class for representing
-/// an object that performs DNS lookups in a specific zone accessible via
-/// a data source.  In general, different types of data sources (in-memory,
-/// database-based, etc) define their own derived classes of \c ZoneFinder,
-/// implementing ways to retrieve the required data through the common
-/// interfaces declared in the base class.  Each concrete \c ZoneFinder
-/// object is therefore (conceptually) associated with a specific zone
-/// of one specific data source instance.
-///
-/// The origin name and the RR class of the associated zone are available
-/// via the \c getOrigin() and \c getClass() methods, respectively.
-///
-/// The most important method of this class is \c find(), which performs
-/// the lookup for a given domain and type.  See the description of the
-/// method for details.
-///
-/// \note It's not clear whether we should request that a zone finder form a
-/// "transaction", that is, whether to ensure the finder is not susceptible
-/// to changes made by someone else than the creator of the finder.  If we
-/// don't request that, for example, two different lookup results for the
-/// same name and type can be different if other threads or programs make
-/// updates to the zone between the lookups.  We should revisit this point
-/// as we gain more experiences.
-class ZoneFinder {
-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.
-    ///
-    /// See the description of \c find() for further details of how
-    /// these results should be interpreted.
-    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
-    };
-
-    /// Special attribute flags on the result of the \c find() method
-    ///
-    /// The flag values defined here are intended to signal to the caller
-    /// that it may need special handling on the result.  This is particularly
-    /// of concern when DNSSEC is requested.  For example, for negative
-    /// responses the caller would want to know whether the zone is signed
-    /// with NSEC or NSEC3 so that it can subsequently provide necessary
-    /// proof of the result.
-    ///
-    /// The caller is generally expected to get access to the information
-    /// via read-only getter methods of \c FindContext so that it won't rely
-    /// on specific details of the representation of the flags.  So these
-    /// definitions are basically only meaningful for data source
-    /// implementations.
-    enum FindResultFlags {
-        RESULT_DEFAULT = 0,       ///< The default flags
-        RESULT_WILDCARD = 1,      ///< find() resulted in a wildcard match
-        RESULT_NSEC_SIGNED = 2,   ///< The zone is signed with NSEC RRs
-        RESULT_NSEC3_SIGNED = 4   ///< The zone is signed with NSEC3 RRs
-    };
-
-    /// Find options.
-    ///
-    /// The option values are used as a parameter for \c find().
-    /// These are values of a bitmask type.  Bitwise operations can be
-    /// performed on these values to express compound options.
-    enum FindOptions {
-        FIND_DEFAULT = 0,       ///< The default options
-        FIND_GLUE_OK = 1,       ///< Allow search under a zone cut
-        FIND_DNSSEC = 2,        ///< Require DNSSEC data in the answer
-                                ///< (RRSIG, NSEC, etc.). The implementation
-                                ///< is allowed to include it even if it is
-                                ///< not set.
-        NO_WILDCARD = 4         ///< Do not try wildcard matching.
-    };
-
-protected:
-    /// \brief A convenient tuple representing a set of find() results.
-    ///
-    /// This helper structure is specifically expected to be used as an input
-    /// for the construct of the \c Context class object used by derived
-    /// ZoneFinder implementations.  This is therefore defined as protected.
-    struct ResultContext {
-        ResultContext(Result code_param,
-                      isc::dns::ConstRRsetPtr rrset_param,
-                      FindResultFlags flags_param = RESULT_DEFAULT) :
-            code(code_param), rrset(rrset_param), flags(flags_param)
-        {}
-        const Result code;
-        const isc::dns::ConstRRsetPtr rrset;
-        const FindResultFlags flags;
-    };
-
-public:
-    /// \brief A helper function to strip RRSIGs when FIND_DNSSEC is not
-    /// requested.
-    static isc::dns::ConstRRsetPtr
-    stripRRsigs(isc::dns::ConstRRsetPtr rp, const FindOptions options);
-
-    /// \brief Context of the result of a find() call.
-    ///
-    /// This class encapsulates results and (possibly) associated context
-    /// of a call to the \c find() method.   The public member variables of
-    /// this class represent the result of the call.  They are a
-    /// straightforward tuple of the result code and a pointer (and
-    /// optionally special flags) to the found RRset.
-    ///
-    /// These member variables will be initialized on construction and never
-    /// change, so for convenience we allow the applications to refer to some
-    /// of the members directly.  For some others we provide read-only accessor
-    /// methods to hide specific representation.
-    ///
-    /// Another role of this class is to provide the interface to some common
-    /// processing logic that may be necessary using the result of \c find().
-    /// Specifically, it's expected to be used in the context of DNS query
-    /// handling, where the caller would need to look into the data source
-    /// again based on the \c find() result.  For example, it would need to
-    /// get A and/or AAAA records for some of the answer or authority RRs.
-    ///
-    /// This class defines (a set of) method(s) that can be commonly used
-    /// for such purposes for any type of data source (as long as it conforms
-    /// to the public \c find() interface).  In some cases, a specific data
-    /// source implementation may want to (and can) optimize the processing
-    /// exploiting its internal data structure and the knowledge of the context
-    /// of the precedent \c find() call.  Such a data source implementation
-    /// can define a derived class of the base Context and override the
-    /// specific virtual method.
-    ///
-    /// This base class defines these common protected methods along with
-    /// some helper pure virtual methods that would be necessary for the
-    /// common methods.  If a derived class wants to use the common version
-    /// of the protected method, it needs to provide expected result through
-    /// their implementation of the pure virtual methods.
-    ///
-    /// This class object is generally expected to be associated with the
-    /// ZoneFinder that originally performed the \c find() call, and expects
-    /// the finder is valid throughout the lifetime of this object.  It's
-    /// caller's responsibility to ensure that assumption.
-    class Context {
-    public:
-        /// \brief The constructor.
-        ///
-        /// \param options The find options specified for the find() call.
-        /// \param result The result of the find() call.
-        Context(FindOptions options, const ResultContext& result) :
-            code(result.code), rrset(result.rrset),
-            flags_(result.flags), options_(options)
-        {}
-
-        /// \brief The destructor.
-        virtual ~Context() {}
-
-        const Result code;
-        const isc::dns::ConstRRsetPtr rrset;
-
-        /// Return true iff find() results in a wildcard match.
-        bool isWildcard() const { return ((flags_ & RESULT_WILDCARD) != 0); }
-
-        /// Return true when the underlying zone is signed with NSEC.
-        ///
-        /// The \c find() implementation allows this to return false if
-        /// \c FIND_DNSSEC isn't specified regardless of whether the zone
-        /// is signed or which of NSEC/NSEC3 is used.
-        ///
-        /// When this is returned, the implementation of find() must ensure
-        /// that \c rrset be a valid NSEC RRset as described in \c find()
-        /// documentation.
-        bool isNSECSigned() const {
-            return ((flags_ & RESULT_NSEC_SIGNED) != 0);
-        }
-
-        /// Return true when the underlying zone is signed with NSEC3.
-        ///
-        /// The \c find() implementation allows this to return false if
-        /// \c FIND_DNSSEC isn't specified regardless of whether the zone
-        /// is signed or which of NSEC/NSEC3 is used.
-        bool isNSEC3Signed() const {
-            return ((flags_ & RESULT_NSEC3_SIGNED) != 0);
-        }
-
-        /// \brief Find and return additional RRsets corresponding to the
-        ///        result of \c find().
-        ///
-        /// If this context is based on a normal find() call that resulted
-        /// in SUCCESS or DELEGATION, it examines the returned RRset (in many
-        /// cases NS, sometimes MX or others), searches the data source for
-        /// specified type of additional RRs for each RDATA of the RRset
-        /// (e.g., A or AAAA for the name server addresses), and stores the
-        /// result in the given vector.  The vector may not be empty; this
-        /// method appends any found RRsets to it, without touching existing
-        /// elements.
-        ///
-        /// If this context is based on a findAll() call that resulted in
-        /// SUCCESS, it performs the same process for each RRset returned in
-        /// the \c findAll() call.
-        ///
-        /// The caller specifies desired RR types of the additional RRsets
-        /// in \c requested_types.  Normally it consists of A and/or AAAA
-        /// types, but other types can be specified.
-        ///
-        /// This method is meaningful only when the precedent find()/findAll()
-        /// call resulted in SUCCESS or DELEGATION.  Otherwise this method
-        /// does nothing.
-        ///
-        /// \note The additional RRsets returned via method are limited to
-        /// ones contained in the zone which the corresponding find/findAll
-        /// call searched (possibly including glues under a zone cut where
-        /// they are applicable).  If the caller needs to get out-of-zone
-        /// additional RRsets, it needs to explicitly finds them by
-        /// identifying the corresponding zone and calls \c find() for it.
-        ///
-        /// \param requested_types A vector of RR types for desired additional
-        ///  RRsets.
-        /// \param result A vector to which any found additional RRsets are
-        /// to be inserted.
-        void getAdditional(
-            const std::vector<isc::dns::RRType>& requested_types,
-            std::vector<isc::dns::ConstRRsetPtr>& result)
-        {
-            // Perform common checks, and delegate the process to the default
-            // or specialized implementation.
-            if (code != SUCCESS && code != DELEGATION) {
-                return;
-            }
-
-            getAdditionalImpl(requested_types, result);
-        }
-
-    protected:
-        /// \brief Return the \c ZoneFinder that created this \c Context.
-        ///
-        /// A derived class implementation can return NULL if it defines
-        /// other protected methods that require a non NULL result from
-        /// this method.  Otherwise it must return a valid, non NULL pointer
-        /// to the \c ZoneFinder object.
-        ///
-        /// When returning non NULL, the ownership of the pointed object
-        /// was not transferred to the caller; it cannot be assumed to be
-        /// valid after the originating \c Context object is destroyed.
-        /// Also, the caller must not try to delete the returned object.
-        virtual ZoneFinder* getFinder() = 0;
-
-        /// \brief Return a vector of RRsets corresponding to findAll() result.
-        ///
-        /// This method returns a set of RRsets that correspond to the
-        /// returned RRsets to a prior \c findAll() call.
-        ///
-        /// A derived class implementation can return NULL if it defines
-        /// other protected methods that require a non NULL result from
-        /// this method.  Otherwise it must return a valid, non NULL pointer
-        /// to a vector that correspond to the expected set of RRsets.
-        ///
-        /// When returning non NULL, the ownership of the pointed object
-        /// was not transferred to the caller; it cannot be assumed to be
-        /// valid after the originating \c Context object is destroyed.
-        /// Also, the caller must not try to delete the returned object.
-        virtual const std::vector<isc::dns::ConstRRsetPtr>*
-        getAllRRsets() const = 0;
-
-        /// \brief Actual implementation of getAdditional().
-        ///
-        /// This base class defines a default implementation that can be
-        /// used for any type of data sources.  A data source implementation
-        /// can override it.
-        ///
-        /// The default version of this implementation requires both
-        /// \c getFinder() and \c getAllRRsets() return valid results.
-        virtual void getAdditionalImpl(
-            const std::vector<isc::dns::RRType>& requested_types,
-            std::vector<isc::dns::ConstRRsetPtr>& result);
-
-    private:
-        const FindResultFlags flags_;
-    protected:
-        const FindOptions options_;
-    };
-
-    /// \brief Generic ZoneFinder context that works for all implementations.
-    ///
-    /// This is a concrete derived class of \c ZoneFinder::Context that
-    /// only use the generic (default) versions of the protected methods
-    /// and therefore work for any data source implementation.
-    ///
-    /// A data source implementation can use this class to create a
-    /// \c Context object as a return value of \c find() or \c findAll()
-    /// method if it doesn't have to optimize specific protected methods.
-    class GenericContext : public Context {
-    public:
-        /// \brief The constructor for the normal find call.
-        ///
-        /// This constructor is expected to be called from the \c find()
-        /// method when it constructs the return value.
-        ///
-        /// \param finder The ZoneFinder on which find() is called.
-        /// \param options See the \c Context class.
-        /// \param result See the \c Context class.
-        GenericContext(ZoneFinder& finder, FindOptions options,
-                       const ResultContext& result) :
-            Context(options, result), finder_(finder)
-        {}
-
-        /// \brief The constructor for the normal findAll call.
-        ///
-        /// This constructor is expected to be called from the \c findAll()
-        /// method when it constructs the return value.
-        ///
-        /// It copies the vector that is to be returned to the caller of
-        /// \c findAll() for possible subsequent use.  Note that it cannot
-        /// simply hold a reference to the vector because the caller may
-        /// alter it after the \c findAll() call.
-        ///
-        /// \param finder The ZoneFinder on which findAll() is called.
-        /// \param options See the \c Context class.
-        /// \param result See the \c Context class.
-        /// \param all_set Reference to the vector given by the caller of
-        ///       \c findAll(), storing the RRsets to be returned.
-        GenericContext(ZoneFinder& finder, FindOptions options,
-                       const ResultContext& result,
-                       const std::vector<isc::dns::ConstRRsetPtr>& all_set) :
-            Context(options, result), finder_(finder), all_set_(all_set)
-        {}
-
-    protected:
-        virtual ZoneFinder* getFinder() { return (&finder_); }
-        virtual const std::vector<isc::dns::ConstRRsetPtr>*
-        getAllRRsets() const {
-            return (&all_set_);
-        }
-
-    private:
-        ZoneFinder& finder_;
-        std::vector<isc::dns::ConstRRsetPtr> all_set_;
-    };
-
-    ///
-    /// \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).
-    ZoneFinder() {}
-public:
-    /// The destructor.
-    virtual ~ZoneFinder() {}
-    //@}
-
-    ///
-    /// \name Getter Methods
-    ///
-    /// These methods should never throw an exception.
-    //@{
-    /// Return the origin name of the zone.
-    virtual isc::dns::Name getOrigin() const = 0;
-
-    /// Return the RR class of the zone.
-    virtual isc::dns::RRClass getClass() const = 0;
-    //@}
-
-    ///
-    /// \name Search Methods
-    ///
-    //@{
-    /// 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.
-    /// - If there is a matching name but no RRset of the search type, it
-    ///   returns the code of \c NXRRSET.  This case includes the search name
-    ///   matches an empty node of the zone.
-    /// - If there is a CNAME RR of the searched name but there is no
-    ///   RR of the searched type of the name (so this type is different from
-    ///   CNAME), it returns the code of \c CNAME and that CNAME RR.
-    ///   Note that if the searched RR type is CNAME, it is considered
-    ///   a successful match, and the code of \c SUCCESS will be returned.
-    /// - If the search name matches a delegation point of DNAME, it returns
-    ///   the code of \c DNAME and that DNAME RR.
-    ///
-    /// No RRset will be returned in the \c NXDOMAIN and \c NXRRSET cases
-    /// (\c rrset member of \c FindContext will be NULL), unless DNSSEC data
-    /// are required.  See below for the cases with DNSSEC.
-    ///
-    /// The returned \c FindContext object can also provide supplemental
-    /// information about the search result via its methods returning a
-    /// boolean value.  Such information may be useful for the caller if
-    /// the caller wants to collect additional DNSSEC proofs based on the
-    /// search result.
-    ///
-    /// The \c options parameter specifies customized behavior of the search.
-    /// Their semantics is as follows (they are or bit-field):
-    ///
-    /// - \c FIND_GLUE_OK Allow search under a zone cut.  By default the search
-    ///   will stop once it encounters a zone cut.  If this option is specified
-    ///   it remembers information about the highest zone cut and continues
-    ///   the search until it finds an exact match for the given name or it
-    ///   detects there is no exact match.  If an exact match is found,
-    ///   RRsets for that name are searched just like the normal case;
-    ///   otherwise, if the search has encountered a zone cut, \c DELEGATION
-    ///   with the information of the highest zone cut will be returned.
-    ///   Note: the term "glue" in the DNS protocol standard may sometimes
-    ///   cause confusion: some people use this term strictly for an address
-    ///   record (type AAAA or A) for the name used in the RDATA of an NS RR;
-    ///   some others seem to give it broader flexibility.  Nevertheless,
-    ///   in this API the "GLUE OK" simply means the search by find() can
-    ///   continue beyond a zone cut; the derived class implementation does
-    ///   not have to, and should not, check whether the type is an address
-    ///   record or whether the query name is pointed by some NS RR.
-    ///   It's up to the caller with which definition of "glue" the search
-    ///   result with this option should be used.
-    /// - \c FIND_DNSSEC Request that DNSSEC data (like NSEC, RRSIGs) are
-    ///   returned with the answer. It is allowed for the data source to
-    ///   include them even when not requested.
-    /// - \c NO_WILDCARD Do not try wildcard matching.  This option is of no
-    ///   use for normal lookups; it's intended to be used to get a DNSSEC
-    ///   proof of the non existence of any matching wildcard or non existence
-    ///   of an exact match when a wildcard match is found.
-    ///
-    /// In general, \c name is expected to be included in the zone, that is,
-    /// it should be equal to or a subdomain of the zone origin.  Otherwise
-    /// this method will return \c NXDOMAIN with an empty RRset.  But such a
-    /// case should rather be considered a caller's bug.
-    ///
-    /// \note For this reason it's probably better to throw an exception
-    /// than returning \c NXDOMAIN.  This point should be revisited in a near
-    /// future version.  In any case applications shouldn't call this method
-    /// for an out-of-zone name.
-    ///
-    /// <b>DNSSEC considerations:</b>
-    /// The result when DNSSEC data are required can be very complicated,
-    /// especially if it involves negative result or wildcard match.
-    /// Specifically, if an application calls this method for DNS query
-    /// processing with DNSSEC data, and if the search result code is
-    /// either \c NXDOMAIN or \c NXRRRSET, and/or \c isWildcard() returns
-    /// true, then the application will need to find additional NSEC or
-    /// NSEC3 records for supplemental proofs.  This method helps the
-    /// application for such post search processing.
-    ///
-    /// First, it tells the application whether the zone is signed with
-    /// NSEC or NSEC3 via the \c isNSEC(3)Signed() method.  Any sanely signed
-    /// zone should be signed with either (and only one) of these two types
-    /// of RRs; however, the application should expect that the zone could
-    /// be broken and these methods could both return false.  But this method
-    /// should ensure that not both of these methods return true.
-    ///
-    /// In case it's signed with NSEC3, there is no further information
-    /// returned from this method.
-    ///
-    /// In case it's signed with NSEC, this method will possibly return
-    /// a related NSEC RRset in the \c rrset member of \c FindContext.
-    /// What kind of NSEC is returned depends on the result code
-    /// (\c NXDOMAIN or \c NXRRSET) and on whether it's a wildcard match:
-    ///
-    /// - In case of NXDOMAIN, the returned NSEC covers the queried domain
-    ///   that proves that the query name does not exist in the zone.  Note
-    ///   that this does not necessarily prove it doesn't even match a
-    ///   wildcard (even if the result of NXDOMAIN can only happen when
-    ///   there's no matching wildcard either).  It is caller's
-    ///   responsibility to provide a proof that there is no matching
-    ///   wildcard if that proof is necessary.
-    /// - In case of NXRRSET, we need to consider the following cases
-    ///   referring to Section 3.1.3 of RFC4035:
-    ///
-    /// -# (Normal) no data: there is a matching non-wildcard name with a
-    ///    different RR type.  This is the "No Data" case of the RFC.
-    /// -# (Normal) empty non terminal: there is no matching (exact or
-    ///    wildcard) name, but there is a subdomain with an RR of the query
-    ///    name.  This is one case of "Name Error" of the RFC.
-    /// -# Wildcard empty non terminal: similar to 2a, but the empty name
-    ///    is a wildcard, and matches the query name by wildcard expansion.
-    ///    This is a special case of "Name Error" of the RFC.
-    /// -# Wildcard no data: there is no exact match name, but there is a
-    ///    wildcard name that matches the query name with a different type
-    ///    of RR.  This is the "Wildcard No Data" case of the RFC.
-    ///
-    /// In case 1, \c find() returns NSEC of the matching name.
-    ///
-    /// In case 2, \c find() will return NSEC for the interval where the
-    /// empty nonterminal lives. The end of the interval is the subdomain
-    /// causing existence of the empty nonterminal (if there's
-    /// sub.x.example.com, and no record in x.example.com, then
-    /// x.example.com exists implicitly - is the empty nonterminal and
-    /// sub.x.example.com is the subdomain causing it).  Note that this NSEC
-    /// proves not only the existence of empty non terminal name but also
-    /// the non existence of possibly matching wildcard name, because
-    /// there can be no better wildcard match than the exact matching empty
-    /// name.
-    ///
-    /// In case 3, \c find() will return NSEC for the interval where the
-    /// wildcard empty nonterminal lives.   Cases 2 and 3 are especially
-    /// complicated and confusing.  See the examples below.
-    ///
-    /// In case 4, \c find() will return NSEC of the matching wildcard name.
-    ///
-    /// Examples: if zone "example.com" has the following record:
-    /// \code
-    /// a.example.com. NSEC a.b.example.com.
-    /// \endcode
-    /// a call to \c find() for "b.example.com." with the FIND_DNSSEC option
-    /// will result in NXRRSET, and this NSEC will be returned.
-    /// Likewise, if zone "example.org" has the following record,
-    /// \code
-    /// a.example.org. NSEC x.*.b.example.org.
-    /// \endcode
-    /// a call to \c find() for "y.b.example.org" with FIND_DNSSEC will
-    /// result in NXRRSET and this NSEC; \c isWildcard() on the returned
-    /// \c FindContext object will return true.
-    ///
-    /// \exception std::bad_alloc Memory allocation such as for constructing
-    ///  the resulting RRset fails
-    /// \throw OutOfZone The Name \c name is outside of the origin of the
-    /// zone of this ZoneFinder.
-    /// \exception DataSourceError Derived class specific exception, e.g.
-    /// when encountering a bad zone configuration or database connection
-    /// failure.  Although these are considered rare, exceptional events,
-    /// it can happen under relatively usual conditions (unlike memory
-    /// allocation failure).  So, in general, the application is expected
-    /// to catch this exception, either specifically or as a result of
-    /// catching a base exception class, and handle it gracefully.
-    ///
-    /// \param name The domain name to be searched for.
-    /// \param type The RR type to be searched for.
-    /// \param options The search options.
-    /// \return A \c FindContext object enclosing the search result
-    ///         (see above).
-    virtual boost::shared_ptr<Context> find(const isc::dns::Name& name,
-                                            const isc::dns::RRType& type,
-                                            const FindOptions options
-                                            = FIND_DEFAULT) = 0;
-
-    ///
-    /// \brief Finds all RRsets in the given name.
-    ///
-    /// This function works almost exactly in the same way as the find one. The
-    /// only difference is, when the lookup is successful (eg. the code is
-    /// SUCCESS), all the RRsets residing in the named node are
-    /// copied into the \c target parameter and the rrset member of the result
-    /// is NULL. All the other (unsuccessful) cases are handled the same,
-    /// including returning delegations, NSEC/NSEC3 availability and NSEC
-    /// proofs, wildcard information etc. The options parameter works the
-    /// same way and it should conform to the same exception restrictions.
-    ///
-    /// \param name \see find, parameter name
-    /// \param target the successfull result is returned through this
-    /// \param options \see find, parameter options
-    /// \return \see find and it's result
-    virtual boost::shared_ptr<Context> findAll(
-        const isc::dns::Name& name,
-        std::vector<isc::dns::ConstRRsetPtr> &target,
-        const FindOptions options = FIND_DEFAULT) = 0;
-
-    /// A helper structure to represent the search result of \c findNSEC3().
-    ///
-    /// The idea is similar to that of \c FindContext, but \c findNSEC3() has
-    /// special interface and semantics, we use a different structure to
-    /// represent the result.
-    struct FindNSEC3Result {
-        FindNSEC3Result(bool param_matched, uint8_t param_closest_labels,
-                        isc::dns::ConstRRsetPtr param_closest_proof,
-                        isc::dns::ConstRRsetPtr param_next_proof) :
-            matched(param_matched), closest_labels(param_closest_labels),
-            closest_proof(param_closest_proof),
-            next_proof(param_next_proof)
-        {}
-
-        /// true iff closest_proof is a matching NSEC3
-        const bool matched;
-
-        /// The number of labels of the identified closest encloser.
-        const uint8_t closest_labels;
-
-        /// Either the NSEC3 for the closest provable encloser of the given
-        /// name or NSEC3 that covers the name
-        const isc::dns::ConstRRsetPtr closest_proof;
-
-        /// When non NULL, NSEC3 for the next closer name.
-        const isc::dns::ConstRRsetPtr next_proof;
-    };
-
-    /// Search the zone for the NSEC3 RR(s) that prove existence or non
-    /// existence of a give name.
-    ///
-    /// It searches the NSEC3 namespace of the zone (how that namespace is
-    /// implemented can vary in specific data source implementation) for NSEC3
-    /// RRs that match or cover the NSEC3 hash value for the given name.
-    ///
-    /// If \c recursive is false, it will first look for the NSEC3 that has
-    /// a matching hash.  If it doesn't exist, it identifies the covering NSEC3
-    /// for the hash.  In either case the search stops at that point and the
-    /// found NSEC3 RR(set) will be returned in the closest_proof member of
-    /// \c FindNSEC3Result.  \c matched is true or false depending on
-    /// the found NSEC3 is a matched one or covering one.  \c next_proof
-    /// is always NULL.  closest_labels must be equal to the number of
-    /// labels of \c name (and therefore meaningless).
-    ///
-    /// If \c recursive is true, it will continue the search toward the zone
-    /// apex (origin name) until it finds a provable encloser, that is,
-    /// an ancestor of \c name that has a matching NSEC3.  This is the closest
-    /// provable encloser of \c name as defined in RFC5155.  In this case,
-    /// if the found encloser is not equal to \c name, the search should
-    /// have seen a covering NSEC3 for the immediate child of the found
-    /// encloser.  That child name is the next closer name as defined in
-    /// RFC5155.  In this case, this method returns the NSEC3 for the
-    /// closest encloser in \c closest_proof, and the NSEC3 for the next
-    /// closer name in \c next_proof of \c FindNSEC3Result.  This set of
-    /// NSEC3 RRs provide the closest encloser proof as defined in RFC5155.
-    /// closest_labels will be set to the number of labels of the identified
-    /// closest encloser.  This will be useful when the caller needs to
-    /// construct the closest encloser name from the original \c name.
-    /// If, on the other hand, the found closest name is equal to \c name,
-    /// this method simply returns it in \c closest_proof.  \c next_proof
-    /// is set to NULL.  In all cases \c matched is set to true.
-    /// closest_labels will be set to the number of labels of \c name.
-    ///
-    /// When looking for NSEC3, this method retrieves NSEC3 parameters from
-    /// the corresponding zone to calculate hash values.  Actual implementation
-    /// of how to do this will differ in different data sources.  If the
-    /// NSEC3 parameters are not available \c DataSourceError exception
-    /// will be thrown.
-    ///
-    /// \note This implicitly means this method assumes the zone does not
-    /// have more than one set of parameters.  This assumption should be
-    /// reasonable in actual deployment and will help simplify the interface
-    /// and implementation.  But if there's a real need for supporting
-    /// multiple sets of parameters in a single zone, we will have to
-    /// extend this method so that, e.g., the caller can specify the parameter
-    /// set.
-    ///
-    /// In general, this method expects the zone is properly signed with NSEC3
-    /// RRs.  Specifically, it assumes at least the apex node has a matching
-    /// NSEC3 RR (so the search in the recursive mode must always succeed);
-    /// it also assumes that it can retrieve NSEC parameters (iterations,
-    /// algorithm, and salt) from the zone as noted above.  If these
-    /// assumptions aren't met, \c DataSourceError exception will be thrown.
-    ///
-    /// \exception OutOfZone name is not a subdomain of the zone origin
-    /// \exception DataSourceError Low-level or internal datasource errors
-    /// happened, or the zone isn't properly signed with NSEC3
-    /// (NSEC3 parameters cannot be found, no NSEC3s are available, etc).
-    /// \exception std::bad_alloc The underlying implementation involves
-    /// memory allocation and it fails
-    ///
-    /// \param name The name for which NSEC3 RRs are to be found.  It must
-    /// be a subdomain of the zone.
-    /// \param recursive Whether or not search should continue until it finds
-    /// a provable encloser (see above).
-    ///
-    /// \return The search result and whether or not the closest_proof is
-    /// a matching NSEC3, in the form of \c FindNSEC3Result object.
-    virtual FindNSEC3Result
-    findNSEC3(const isc::dns::Name& name, bool recursive) = 0;
-    //@}
-};
-
-/// \brief Operator to combine FindOptions
-///
-/// We would need to manually static-cast the options if we put or
-/// between them, which is undesired with bit-flag options. Therefore
-/// we hide the cast here, which is the simplest solution and it still
-/// provides reasonable level of type safety.
-inline ZoneFinder::FindOptions operator |(ZoneFinder::FindOptions a,
-                                          ZoneFinder::FindOptions b)
-{
-    return (static_cast<ZoneFinder::FindOptions>(static_cast<unsigned>(a) |
-                                                 static_cast<unsigned>(b)));
-}
-
-/// \brief Operator to combine FindResultFlags
-///
-/// Similar to the same operator for \c FindOptions.  Refer to the description
-/// of that function.
-inline ZoneFinder::FindResultFlags operator |(
-    ZoneFinder::FindResultFlags a,
-    ZoneFinder::FindResultFlags b)
-{
-    return (static_cast<ZoneFinder::FindResultFlags>(
-                static_cast<unsigned>(a) | static_cast<unsigned>(b)));
-}
-
-/// \brief A pointer-like type pointing to a \c ZoneFinder object.
-typedef boost::shared_ptr<ZoneFinder> ZoneFinderPtr;
-
-/// \brief A pointer-like type pointing to an immutable \c ZoneFinder object.
-typedef boost::shared_ptr<const ZoneFinder> ConstZoneFinderPtr;
-
-/// \brief A pointer-like type pointing to a \c ZoneFinder::Context object.
-typedef boost::shared_ptr<ZoneFinder::Context> ZoneFinderContextPtr;
-
-/// \brief A pointer-like type pointing to an immutable
-/// \c ZoneFinder::Context object.
-typedef boost::shared_ptr<ZoneFinder::Context> ConstZoneFinderContextPtr;
-
 /// \brief A forward declaration
 class RRsetCollectionBase;
 
+class ZoneFinder;
+
 /// The base class to make updates to a single zone.
 ///
 /// On construction, each derived class object will start a "transaction"

+ 93 - 2
src/lib/datasrc/zone_finder.cc

@@ -12,13 +12,14 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <datasrc/zone_finder.h>
+#include <datasrc/data_source.h>
+
 #include <dns/rdata.h>
 #include <dns/rrset.h>
 #include <dns/rrtype.h>
 #include <dns/rdataclass.h>
 
-#include <datasrc/zone.h>
-
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
@@ -26,6 +27,96 @@ using namespace isc::dns::rdata;
 namespace isc {
 namespace datasrc {
 
+namespace {
+// Identify zone's SOA and return its MINTTL in the form of RRTTL.
+RRTTL
+getMinTTL(ZoneFinder& finder, ConstRRsetPtr rrset) {
+    ConstRRsetPtr soa_rrset;
+    if (rrset->getType() == RRType::SOA()) {
+        // Shortcut: if we are looking at SOA itself (which should be the
+        // case in the expected scenario), we can simply use its RDATA.
+        soa_rrset = rrset;
+    } else {
+        soa_rrset =
+            finder.findAtOrigin(RRType::SOA(), false,
+                                ZoneFinder::FIND_DEFAULT)->rrset;
+    }
+
+    // In a valid zone there is one and only one SOA RR at the origin.
+    // Otherwise either zone data or the data source implementation is broken.
+    if (!soa_rrset || soa_rrset->getRdataCount() != 1) {
+        isc_throw(DataSourceError, "Zone " << rrset->getName().toText(true)
+                  << "/" << rrset->getClass().toText() << " is broken: "
+                  << (!soa_rrset ? "no SOA" : "empty SOA"));
+    }
+
+    return (RRTTL(dynamic_cast<const generic::SOA&>(
+                      soa_rrset->getRdataIterator()->getCurrent()).
+                  getMinimum()));
+}
+
+// Make a fresh copy of given RRset, just replacing RRTTL with the given one.
+RRsetPtr
+copyRRset(const AbstractRRset& rrset, const RRTTL& ttl) {
+    RRsetPtr rrset_copy(new RRset(rrset.getName(), rrset.getClass(),
+                                  rrset.getType(), ttl));
+    for (RdataIteratorPtr rit = rrset.getRdataIterator();
+         !rit->isLast();
+         rit->next()) {
+        rrset_copy->addRdata(rit->getCurrent());
+    }
+
+    ConstRRsetPtr rrsig = rrset.getRRsig();
+    if (rrsig) {
+        RRsetPtr rrsig_copy(new RRset(rrset.getName(), rrset.getClass(),
+                                      RRType::RRSIG(), ttl));
+        for (RdataIteratorPtr rit = rrsig->getRdataIterator();
+             !rit->isLast();
+             rit->next()) {
+            rrsig_copy->addRdata(rit->getCurrent());
+        }
+        rrset_copy->addRRsig(rrsig_copy);
+    }
+
+    return (rrset_copy);
+}
+}
+
+ZoneFinderContextPtr
+ZoneFinder::findAtOrigin(const dns::RRType& type, bool use_minttl,
+                         FindOptions options)
+{
+    ZoneFinderContextPtr context = find(getOrigin(), type, options);
+
+    // If we are requested to use the min TTL and the RRset's RR TTL is larger
+    // than that, we need to make a copy of the RRset, replacing the TTL,
+    // and return a newly created context copying other parameters.
+    if (use_minttl && context->rrset) {
+        const AbstractRRset& rrset = *context->rrset;
+        const RRTTL min_ttl = getMinTTL(*this, context->rrset);
+        if (min_ttl < rrset.getTTL()) {
+            FindResultFlags flags_copy = RESULT_DEFAULT;
+            if (context->isWildcard()) {
+                flags_copy = flags_copy | RESULT_WILDCARD;
+            }
+            if (context->isNSECSigned()) {
+                flags_copy = flags_copy | RESULT_NSEC_SIGNED;
+            } else if (context->isNSEC3Signed()) {
+                flags_copy = flags_copy | RESULT_NSEC3_SIGNED;
+            }
+
+            return (ZoneFinderContextPtr(
+                        new GenericContext(*this, options,
+                                           ResultContext(context->code,
+                                                         copyRRset(rrset,
+                                                                   min_ttl),
+                                                         flags_copy))));
+        }
+    }
+
+    return (context);
+}
+
 isc::dns::ConstRRsetPtr
 ZoneFinder::stripRRsigs(isc::dns::ConstRRsetPtr rp,
                         const FindOptions options) {

+ 809 - 0
src/lib/datasrc/zone_finder.h

@@ -0,0 +1,809 @@
+// 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 DATASRC_ZONE_FINDER_H
+#define DATASRC_ZONE_FINDER_H 1
+
+#include <dns/name.h>
+#include <dns/rrset.h>
+#include <dns/rrtype.h>
+
+#include <datasrc/exceptions.h>
+#include <datasrc/result.h>
+#include <datasrc/rrset_collection_base.h>
+
+#include <utility>
+#include <vector>
+
+namespace isc {
+namespace datasrc {
+
+/// \brief Out of zone exception
+///
+/// This is thrown when a method is called for a name or RRset which
+/// is not in or below the zone.
+class OutOfZone : public ZoneException {
+public:
+    OutOfZone(const char* file, size_t line, const char* what) :
+        ZoneException(file, line, what) {}
+};
+
+/// \brief The base class to search a zone for RRsets
+///
+/// The \c ZoneFinder class is an abstract base class for representing
+/// an object that performs DNS lookups in a specific zone accessible via
+/// a data source.  In general, different types of data sources (in-memory,
+/// database-based, etc) define their own derived classes of \c ZoneFinder,
+/// implementing ways to retrieve the required data through the common
+/// interfaces declared in the base class.  Each concrete \c ZoneFinder
+/// object is therefore (conceptually) associated with a specific zone
+/// of one specific data source instance.
+///
+/// The origin name and the RR class of the associated zone are available
+/// via the \c getOrigin() and \c getClass() methods, respectively.
+///
+/// The most important method of this class is \c find(), which performs
+/// the lookup for a given domain and type.  See the description of the
+/// method for details.
+///
+/// \note It's not clear whether we should request that a zone finder form a
+/// "transaction", that is, whether to ensure the finder is not susceptible
+/// to changes made by someone else than the creator of the finder.  If we
+/// don't request that, for example, two different lookup results for the
+/// same name and type can be different if other threads or programs make
+/// updates to the zone between the lookups.  We should revisit this point
+/// as we gain more experiences.
+class ZoneFinder {
+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.
+    ///
+    /// See the description of \c find() for further details of how
+    /// these results should be interpreted.
+    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
+    };
+
+    /// Special attribute flags on the result of the \c find() method
+    ///
+    /// The flag values defined here are intended to signal to the caller
+    /// that it may need special handling on the result.  This is particularly
+    /// of concern when DNSSEC is requested.  For example, for negative
+    /// responses the caller would want to know whether the zone is signed
+    /// with NSEC or NSEC3 so that it can subsequently provide necessary
+    /// proof of the result.
+    ///
+    /// The caller is generally expected to get access to the information
+    /// via read-only getter methods of \c FindContext so that it won't rely
+    /// on specific details of the representation of the flags.  So these
+    /// definitions are basically only meaningful for data source
+    /// implementations.
+    enum FindResultFlags {
+        RESULT_DEFAULT = 0,       ///< The default flags
+        RESULT_WILDCARD = 1,      ///< find() resulted in a wildcard match
+        RESULT_NSEC_SIGNED = 2,   ///< The zone is signed with NSEC RRs
+        RESULT_NSEC3_SIGNED = 4   ///< The zone is signed with NSEC3 RRs
+    };
+
+    /// Find options.
+    ///
+    /// The option values are used as a parameter for \c find().
+    /// These are values of a bitmask type.  Bitwise operations can be
+    /// performed on these values to express compound options.
+    enum FindOptions {
+        FIND_DEFAULT = 0,       ///< The default options
+        FIND_GLUE_OK = 1,       ///< Allow search under a zone cut
+        FIND_DNSSEC = 2,        ///< Require DNSSEC data in the answer
+                                ///< (RRSIG, NSEC, etc.). The implementation
+                                ///< is allowed to include it even if it is
+                                ///< not set.
+        NO_WILDCARD = 4         ///< Do not try wildcard matching.
+    };
+
+protected:
+    /// \brief A convenient tuple representing a set of find() results.
+    ///
+    /// This helper structure is specifically expected to be used as an input
+    /// for the construct of the \c Context class object used by derived
+    /// ZoneFinder implementations.  This is therefore defined as protected.
+    struct ResultContext {
+        ResultContext(Result code_param,
+                      isc::dns::ConstRRsetPtr rrset_param,
+                      FindResultFlags flags_param = RESULT_DEFAULT) :
+            code(code_param), rrset(rrset_param), flags(flags_param)
+        {}
+        const Result code;
+        const isc::dns::ConstRRsetPtr rrset;
+        const FindResultFlags flags;
+    };
+
+public:
+    /// \brief A helper function to strip RRSIGs when FIND_DNSSEC is not
+    /// requested.
+    static isc::dns::ConstRRsetPtr
+    stripRRsigs(isc::dns::ConstRRsetPtr rp, const FindOptions options);
+
+    /// \brief Context of the result of a find() call.
+    ///
+    /// This class encapsulates results and (possibly) associated context
+    /// of a call to the \c find() method.   The public member variables of
+    /// this class represent the result of the call.  They are a
+    /// straightforward tuple of the result code and a pointer (and
+    /// optionally special flags) to the found RRset.
+    ///
+    /// These member variables will be initialized on construction and never
+    /// change, so for convenience we allow the applications to refer to some
+    /// of the members directly.  For some others we provide read-only accessor
+    /// methods to hide specific representation.
+    ///
+    /// Another role of this class is to provide the interface to some common
+    /// processing logic that may be necessary using the result of \c find().
+    /// Specifically, it's expected to be used in the context of DNS query
+    /// handling, where the caller would need to look into the data source
+    /// again based on the \c find() result.  For example, it would need to
+    /// get A and/or AAAA records for some of the answer or authority RRs.
+    ///
+    /// This class defines (a set of) method(s) that can be commonly used
+    /// for such purposes for any type of data source (as long as it conforms
+    /// to the public \c find() interface).  In some cases, a specific data
+    /// source implementation may want to (and can) optimize the processing
+    /// exploiting its internal data structure and the knowledge of the context
+    /// of the precedent \c find() call.  Such a data source implementation
+    /// can define a derived class of the base Context and override the
+    /// specific virtual method.
+    ///
+    /// This base class defines these common protected methods along with
+    /// some helper pure virtual methods that would be necessary for the
+    /// common methods.  If a derived class wants to use the common version
+    /// of the protected method, it needs to provide expected result through
+    /// their implementation of the pure virtual methods.
+    ///
+    /// This class object is generally expected to be associated with the
+    /// ZoneFinder that originally performed the \c find() call, and expects
+    /// the finder is valid throughout the lifetime of this object.  It's
+    /// caller's responsibility to ensure that assumption.
+    class Context {
+    public:
+        /// \brief The constructor.
+        ///
+        /// \param options The find options specified for the find() call.
+        /// \param result The result of the find() call.
+        Context(FindOptions options, const ResultContext& result) :
+            code(result.code), rrset(result.rrset),
+            flags_(result.flags), options_(options)
+        {}
+
+        /// \brief The destructor.
+        virtual ~Context() {}
+
+        const Result code;
+        const isc::dns::ConstRRsetPtr rrset;
+
+        /// Return true iff find() results in a wildcard match.
+        bool isWildcard() const { return ((flags_ & RESULT_WILDCARD) != 0); }
+
+        /// Return true when the underlying zone is signed with NSEC.
+        ///
+        /// The \c find() implementation allows this to return false if
+        /// \c FIND_DNSSEC isn't specified regardless of whether the zone
+        /// is signed or which of NSEC/NSEC3 is used.
+        ///
+        /// When this is returned, the implementation of find() must ensure
+        /// that \c rrset be a valid NSEC RRset as described in \c find()
+        /// documentation.
+        bool isNSECSigned() const {
+            return ((flags_ & RESULT_NSEC_SIGNED) != 0);
+        }
+
+        /// Return true when the underlying zone is signed with NSEC3.
+        ///
+        /// The \c find() implementation allows this to return false if
+        /// \c FIND_DNSSEC isn't specified regardless of whether the zone
+        /// is signed or which of NSEC/NSEC3 is used.
+        bool isNSEC3Signed() const {
+            return ((flags_ & RESULT_NSEC3_SIGNED) != 0);
+        }
+
+        /// \brief Find and return additional RRsets corresponding to the
+        ///        result of \c find().
+        ///
+        /// If this context is based on a normal find() call that resulted
+        /// in SUCCESS or DELEGATION, it examines the returned RRset (in many
+        /// cases NS, sometimes MX or others), searches the data source for
+        /// specified type of additional RRs for each RDATA of the RRset
+        /// (e.g., A or AAAA for the name server addresses), and stores the
+        /// result in the given vector.  The vector may not be empty; this
+        /// method appends any found RRsets to it, without touching existing
+        /// elements.
+        ///
+        /// If this context is based on a findAll() call that resulted in
+        /// SUCCESS, it performs the same process for each RRset returned in
+        /// the \c findAll() call.
+        ///
+        /// The caller specifies desired RR types of the additional RRsets
+        /// in \c requested_types.  Normally it consists of A and/or AAAA
+        /// types, but other types can be specified.
+        ///
+        /// This method is meaningful only when the precedent find()/findAll()
+        /// call resulted in SUCCESS or DELEGATION.  Otherwise this method
+        /// does nothing.
+        ///
+        /// \note The additional RRsets returned via method are limited to
+        /// ones contained in the zone which the corresponding find/findAll
+        /// call searched (possibly including glues under a zone cut where
+        /// they are applicable).  If the caller needs to get out-of-zone
+        /// additional RRsets, it needs to explicitly finds them by
+        /// identifying the corresponding zone and calls \c find() for it.
+        ///
+        /// \param requested_types A vector of RR types for desired additional
+        ///  RRsets.
+        /// \param result A vector to which any found additional RRsets are
+        /// to be inserted.
+        void getAdditional(
+            const std::vector<isc::dns::RRType>& requested_types,
+            std::vector<isc::dns::ConstRRsetPtr>& result)
+        {
+            // Perform common checks, and delegate the process to the default
+            // or specialized implementation.
+            if (code != SUCCESS && code != DELEGATION) {
+                return;
+            }
+
+            getAdditionalImpl(requested_types, result);
+        }
+
+    protected:
+        /// \brief Return the \c ZoneFinder that created this \c Context.
+        ///
+        /// A derived class implementation can return NULL if it defines
+        /// other protected methods that require a non NULL result from
+        /// this method.  Otherwise it must return a valid, non NULL pointer
+        /// to the \c ZoneFinder object.
+        ///
+        /// When returning non NULL, the ownership of the pointed object
+        /// was not transferred to the caller; it cannot be assumed to be
+        /// valid after the originating \c Context object is destroyed.
+        /// Also, the caller must not try to delete the returned object.
+        virtual ZoneFinder* getFinder() = 0;
+
+        /// \brief Return a vector of RRsets corresponding to findAll() result.
+        ///
+        /// This method returns a set of RRsets that correspond to the
+        /// returned RRsets to a prior \c findAll() call.
+        ///
+        /// A derived class implementation can return NULL if it defines
+        /// other protected methods that require a non NULL result from
+        /// this method.  Otherwise it must return a valid, non NULL pointer
+        /// to a vector that correspond to the expected set of RRsets.
+        ///
+        /// When returning non NULL, the ownership of the pointed object
+        /// was not transferred to the caller; it cannot be assumed to be
+        /// valid after the originating \c Context object is destroyed.
+        /// Also, the caller must not try to delete the returned object.
+        virtual const std::vector<isc::dns::ConstRRsetPtr>*
+        getAllRRsets() const = 0;
+
+        /// \brief Actual implementation of getAdditional().
+        ///
+        /// This base class defines a default implementation that can be
+        /// used for any type of data sources.  A data source implementation
+        /// can override it.
+        ///
+        /// The default version of this implementation requires both
+        /// \c getFinder() and \c getAllRRsets() return valid results.
+        virtual void getAdditionalImpl(
+            const std::vector<isc::dns::RRType>& requested_types,
+            std::vector<isc::dns::ConstRRsetPtr>& result);
+
+    private:
+        const FindResultFlags flags_;
+    protected:
+        const FindOptions options_;
+    };
+
+    /// \brief Generic ZoneFinder context that works for all implementations.
+    ///
+    /// This is a concrete derived class of \c ZoneFinder::Context that
+    /// only use the generic (default) versions of the protected methods
+    /// and therefore work for any data source implementation.
+    ///
+    /// A data source implementation can use this class to create a
+    /// \c Context object as a return value of \c find() or \c findAll()
+    /// method if it doesn't have to optimize specific protected methods.
+    class GenericContext : public Context {
+    public:
+        /// \brief The constructor for the normal find call.
+        ///
+        /// This constructor is expected to be called from the \c find()
+        /// method when it constructs the return value.
+        ///
+        /// \param finder The ZoneFinder on which find() is called.
+        /// \param options See the \c Context class.
+        /// \param result See the \c Context class.
+        GenericContext(ZoneFinder& finder, FindOptions options,
+                       const ResultContext& result) :
+            Context(options, result), finder_(finder)
+        {}
+
+        /// \brief The constructor for the normal findAll call.
+        ///
+        /// This constructor is expected to be called from the \c findAll()
+        /// method when it constructs the return value.
+        ///
+        /// It copies the vector that is to be returned to the caller of
+        /// \c findAll() for possible subsequent use.  Note that it cannot
+        /// simply hold a reference to the vector because the caller may
+        /// alter it after the \c findAll() call.
+        ///
+        /// \param finder The ZoneFinder on which findAll() is called.
+        /// \param options See the \c Context class.
+        /// \param result See the \c Context class.
+        /// \param all_set Reference to the vector given by the caller of
+        ///       \c findAll(), storing the RRsets to be returned.
+        GenericContext(ZoneFinder& finder, FindOptions options,
+                       const ResultContext& result,
+                       const std::vector<isc::dns::ConstRRsetPtr>& all_set) :
+            Context(options, result), finder_(finder), all_set_(all_set)
+        {}
+
+    protected:
+        virtual ZoneFinder* getFinder() { return (&finder_); }
+        virtual const std::vector<isc::dns::ConstRRsetPtr>*
+        getAllRRsets() const {
+            return (&all_set_);
+        }
+
+    private:
+        ZoneFinder& finder_;
+        std::vector<isc::dns::ConstRRsetPtr> all_set_;
+    };
+
+    ///
+    /// \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).
+    ZoneFinder() {}
+public:
+    /// The destructor.
+    virtual ~ZoneFinder() {}
+    //@}
+
+    ///
+    /// \name Getter Methods
+    ///
+    /// These methods should never throw an exception.
+    //@{
+    /// Return the origin name of the zone.
+    virtual isc::dns::Name getOrigin() const = 0;
+
+    /// Return the RR class of the zone.
+    virtual isc::dns::RRClass getClass() const = 0;
+    //@}
+
+    ///
+    /// \name Search Methods
+    ///
+    //@{
+    /// \brief 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.
+    /// - If there is a matching name but no RRset of the search type, it
+    ///   returns the code of \c NXRRSET.  This case includes the search name
+    ///   matches an empty node of the zone.
+    /// - If there is a CNAME RR of the searched name but there is no
+    ///   RR of the searched type of the name (so this type is different from
+    ///   CNAME), it returns the code of \c CNAME and that CNAME RR.
+    ///   Note that if the searched RR type is CNAME, it is considered
+    ///   a successful match, and the code of \c SUCCESS will be returned.
+    /// - If the search name matches a delegation point of DNAME, it returns
+    ///   the code of \c DNAME and that DNAME RR.
+    ///
+    /// No RRset will be returned in the \c NXDOMAIN and \c NXRRSET cases
+    /// (\c rrset member of \c FindContext will be NULL), unless DNSSEC data
+    /// are required.  See below for the cases with DNSSEC.
+    ///
+    /// The returned \c FindContext object can also provide supplemental
+    /// information about the search result via its methods returning a
+    /// boolean value.  Such information may be useful for the caller if
+    /// the caller wants to collect additional DNSSEC proofs based on the
+    /// search result.
+    ///
+    /// The \c options parameter specifies customized behavior of the search.
+    /// Their semantics is as follows (they are or bit-field):
+    ///
+    /// - \c FIND_GLUE_OK Allow search under a zone cut.  By default the search
+    ///   will stop once it encounters a zone cut.  If this option is specified
+    ///   it remembers information about the highest zone cut and continues
+    ///   the search until it finds an exact match for the given name or it
+    ///   detects there is no exact match.  If an exact match is found,
+    ///   RRsets for that name are searched just like the normal case;
+    ///   otherwise, if the search has encountered a zone cut, \c DELEGATION
+    ///   with the information of the highest zone cut will be returned.
+    ///   Note: the term "glue" in the DNS protocol standard may sometimes
+    ///   cause confusion: some people use this term strictly for an address
+    ///   record (type AAAA or A) for the name used in the RDATA of an NS RR;
+    ///   some others seem to give it broader flexibility.  Nevertheless,
+    ///   in this API the "GLUE OK" simply means the search by find() can
+    ///   continue beyond a zone cut; the derived class implementation does
+    ///   not have to, and should not, check whether the type is an address
+    ///   record or whether the query name is pointed by some NS RR.
+    ///   It's up to the caller with which definition of "glue" the search
+    ///   result with this option should be used.
+    /// - \c FIND_DNSSEC Request that DNSSEC data (like NSEC, RRSIGs) are
+    ///   returned with the answer. It is allowed for the data source to
+    ///   include them even when not requested.
+    /// - \c NO_WILDCARD Do not try wildcard matching.  This option is of no
+    ///   use for normal lookups; it's intended to be used to get a DNSSEC
+    ///   proof of the non existence of any matching wildcard or non existence
+    ///   of an exact match when a wildcard match is found.
+    ///
+    /// In general, \c name is expected to be included in the zone, that is,
+    /// it should be equal to or a subdomain of the zone origin.  Otherwise
+    /// this method will return \c NXDOMAIN with an empty RRset.  But such a
+    /// case should rather be considered a caller's bug.
+    ///
+    /// \note For this reason it's probably better to throw an exception
+    /// than returning \c NXDOMAIN.  This point should be revisited in a near
+    /// future version.  In any case applications shouldn't call this method
+    /// for an out-of-zone name.
+    ///
+    /// <b>DNSSEC considerations:</b>
+    /// The result when DNSSEC data are required can be very complicated,
+    /// especially if it involves negative result or wildcard match.
+    /// Specifically, if an application calls this method for DNS query
+    /// processing with DNSSEC data, and if the search result code is
+    /// either \c NXDOMAIN or \c NXRRRSET, and/or \c isWildcard() returns
+    /// true, then the application will need to find additional NSEC or
+    /// NSEC3 records for supplemental proofs.  This method helps the
+    /// application for such post search processing.
+    ///
+    /// First, it tells the application whether the zone is signed with
+    /// NSEC or NSEC3 via the \c isNSEC(3)Signed() method.  Any sanely signed
+    /// zone should be signed with either (and only one) of these two types
+    /// of RRs; however, the application should expect that the zone could
+    /// be broken and these methods could both return false.  But this method
+    /// should ensure that not both of these methods return true.
+    ///
+    /// In case it's signed with NSEC3, there is no further information
+    /// returned from this method.
+    ///
+    /// In case it's signed with NSEC, this method will possibly return
+    /// a related NSEC RRset in the \c rrset member of \c FindContext.
+    /// What kind of NSEC is returned depends on the result code
+    /// (\c NXDOMAIN or \c NXRRSET) and on whether it's a wildcard match:
+    ///
+    /// - In case of NXDOMAIN, the returned NSEC covers the queried domain
+    ///   that proves that the query name does not exist in the zone.  Note
+    ///   that this does not necessarily prove it doesn't even match a
+    ///   wildcard (even if the result of NXDOMAIN can only happen when
+    ///   there's no matching wildcard either).  It is caller's
+    ///   responsibility to provide a proof that there is no matching
+    ///   wildcard if that proof is necessary.
+    /// - In case of NXRRSET, we need to consider the following cases
+    ///   referring to Section 3.1.3 of RFC4035:
+    ///
+    /// -# (Normal) no data: there is a matching non-wildcard name with a
+    ///    different RR type.  This is the "No Data" case of the RFC.
+    /// -# (Normal) empty non terminal: there is no matching (exact or
+    ///    wildcard) name, but there is a subdomain with an RR of the query
+    ///    name.  This is one case of "Name Error" of the RFC.
+    /// -# Wildcard empty non terminal: similar to 2a, but the empty name
+    ///    is a wildcard, and matches the query name by wildcard expansion.
+    ///    This is a special case of "Name Error" of the RFC.
+    /// -# Wildcard no data: there is no exact match name, but there is a
+    ///    wildcard name that matches the query name with a different type
+    ///    of RR.  This is the "Wildcard No Data" case of the RFC.
+    ///
+    /// In case 1, \c find() returns NSEC of the matching name.
+    ///
+    /// In case 2, \c find() will return NSEC for the interval where the
+    /// empty nonterminal lives. The end of the interval is the subdomain
+    /// causing existence of the empty nonterminal (if there's
+    /// sub.x.example.com, and no record in x.example.com, then
+    /// x.example.com exists implicitly - is the empty nonterminal and
+    /// sub.x.example.com is the subdomain causing it).  Note that this NSEC
+    /// proves not only the existence of empty non terminal name but also
+    /// the non existence of possibly matching wildcard name, because
+    /// there can be no better wildcard match than the exact matching empty
+    /// name.
+    ///
+    /// In case 3, \c find() will return NSEC for the interval where the
+    /// wildcard empty nonterminal lives.   Cases 2 and 3 are especially
+    /// complicated and confusing.  See the examples below.
+    ///
+    /// In case 4, \c find() will return NSEC of the matching wildcard name.
+    ///
+    /// Examples: if zone "example.com" has the following record:
+    /// \code
+    /// a.example.com. NSEC a.b.example.com.
+    /// \endcode
+    /// a call to \c find() for "b.example.com." with the FIND_DNSSEC option
+    /// will result in NXRRSET, and this NSEC will be returned.
+    /// Likewise, if zone "example.org" has the following record,
+    /// \code
+    /// a.example.org. NSEC x.*.b.example.org.
+    /// \endcode
+    /// a call to \c find() for "y.b.example.org" with FIND_DNSSEC will
+    /// result in NXRRSET and this NSEC; \c isWildcard() on the returned
+    /// \c FindContext object will return true.
+    ///
+    /// \exception std::bad_alloc Memory allocation such as for constructing
+    ///  the resulting RRset fails
+    /// \throw OutOfZone The Name \c name is outside of the origin of the
+    /// zone of this ZoneFinder.
+    /// \exception DataSourceError Derived class specific exception, e.g.
+    /// when encountering a bad zone configuration or database connection
+    /// failure.  Although these are considered rare, exceptional events,
+    /// it can happen under relatively usual conditions (unlike memory
+    /// allocation failure).  So, in general, the application is expected
+    /// to catch this exception, either specifically or as a result of
+    /// catching a base exception class, and handle it gracefully.
+    ///
+    /// \param name The domain name to be searched for.
+    /// \param type The RR type to be searched for.
+    /// \param options The search options.
+    /// \return A \c FindContext object enclosing the search result
+    ///         (see above).
+    virtual boost::shared_ptr<Context> find(const isc::dns::Name& name,
+                                            const isc::dns::RRType& type,
+                                            const FindOptions options
+                                            = FIND_DEFAULT) = 0;
+
+    /// \brief Search for an RRset of given RR type at the zone origin.
+    ///
+    /// In terms of API this method is equivalent to a call to \c find() where
+    /// the \c name parameter is the zone origin (return value of
+    /// \c getOrigin()) and is redundant.  This method is provided as an
+    /// optimization point for some kind of finder implementations that can
+    /// exploit the fact that the query name is the zone origin and for
+    /// applications that want to possibly benefit from such implementations.
+    ///
+    /// If \c use_minttl is set to \c true and the returned context would
+    /// contain a non NULL RRset, its RR TTL is (possibly) adjusted so that
+    /// it's set to the minimum of its own TTL and the minimum TTL field value
+    /// of the zone's SOA record.  If the RRset contains an RRSIG, its TTL
+    /// is also adjusted in the same way.
+    ///
+    /// The origin of a zone is special in some points: for any valid zone
+    /// there should always be an SOA and at least one NS RR there, which
+    /// also means the origin name is never empty.  Also, the SOA record can
+    /// be used in a DNS response for negative answers, in which case the
+    /// RR TTL must be set to minimum of its own RRTTL and the value of the
+    /// minimum TTL field.  Although these operations can be performed
+    /// through other public interfaces, they can be sometimes suboptimal
+    /// in performance or could be more efficient in a specialized
+    /// implementation.  For example, a specific implementation of
+    /// \c getOrigin() could involve a dynamic creation of a \c Name object,
+    /// which is less efficient; on the other hand, the underlying finder
+    /// implementation may have an efficient way to access RRs of the origin
+    /// in implementation specific way; and, while reconstructing an RRset
+    /// with replacing the TTL is relatively expensive, this can be done
+    /// much faster if the need for it is known beforehand.
+    ///
+    /// If the underlying finder implementation wants to optimize these cases,
+    /// it can do so by specializing the method.  It has the default
+    /// implementation for any other implementations, which should work for
+    /// any finder implementation as long as it conforms to other public
+    /// interfaces.
+    ///
+    /// So, an implementation of a finder does not have to care about this
+    /// method unless it sees the need for optimizing the behavior.
+    /// Also, applications normally do not have to use this interface;
+    /// using the generic \c find() method (with some post call processing)
+    /// can do everything this method can provide.  The default implementation
+    /// may even be slower than such straightforward usage due to the
+    /// internal overhead.  This method should be used if and only if the
+    /// application needs to achieve the possible best performance with an
+    /// optimized finder implementation.
+    ///
+    /// \param type The RR type to be searched for.
+    /// \param use_minttl Whether to adjust the TTL (see the description).
+    /// \param options The search options.  Same for \c find().
+    ///
+    /// \return A \c FindContext object enclosing the search result.
+    ///         See \c find().
+    virtual boost::shared_ptr<Context> findAtOrigin(
+        const isc::dns::RRType& type, bool use_minttl,
+        FindOptions options);
+
+public:
+    ///
+    /// \brief Finds all RRsets in the given name.
+    ///
+    /// This function works almost exactly in the same way as the find one. The
+    /// only difference is, when the lookup is successful (eg. the code is
+    /// SUCCESS), all the RRsets residing in the named node are
+    /// copied into the \c target parameter and the rrset member of the result
+    /// is NULL. All the other (unsuccessful) cases are handled the same,
+    /// including returning delegations, NSEC/NSEC3 availability and NSEC
+    /// proofs, wildcard information etc. The options parameter works the
+    /// same way and it should conform to the same exception restrictions.
+    ///
+    /// \param name \see find, parameter name
+    /// \param target the successfull result is returned through this
+    /// \param options \see find, parameter options
+    /// \return \see find and it's result
+    virtual boost::shared_ptr<Context> findAll(
+        const isc::dns::Name& name,
+        std::vector<isc::dns::ConstRRsetPtr> &target,
+        const FindOptions options = FIND_DEFAULT) = 0;
+
+    /// A helper structure to represent the search result of \c findNSEC3().
+    ///
+    /// The idea is similar to that of \c FindContext, but \c findNSEC3() has
+    /// special interface and semantics, we use a different structure to
+    /// represent the result.
+    struct FindNSEC3Result {
+        FindNSEC3Result(bool param_matched, uint8_t param_closest_labels,
+                        isc::dns::ConstRRsetPtr param_closest_proof,
+                        isc::dns::ConstRRsetPtr param_next_proof) :
+            matched(param_matched), closest_labels(param_closest_labels),
+            closest_proof(param_closest_proof),
+            next_proof(param_next_proof)
+        {}
+
+        /// true iff closest_proof is a matching NSEC3
+        const bool matched;
+
+        /// The number of labels of the identified closest encloser.
+        const uint8_t closest_labels;
+
+        /// Either the NSEC3 for the closest provable encloser of the given
+        /// name or NSEC3 that covers the name
+        const isc::dns::ConstRRsetPtr closest_proof;
+
+        /// When non NULL, NSEC3 for the next closer name.
+        const isc::dns::ConstRRsetPtr next_proof;
+    };
+
+    /// Search the zone for the NSEC3 RR(s) that prove existence or non
+    /// existence of a give name.
+    ///
+    /// It searches the NSEC3 namespace of the zone (how that namespace is
+    /// implemented can vary in specific data source implementation) for NSEC3
+    /// RRs that match or cover the NSEC3 hash value for the given name.
+    ///
+    /// If \c recursive is false, it will first look for the NSEC3 that has
+    /// a matching hash.  If it doesn't exist, it identifies the covering NSEC3
+    /// for the hash.  In either case the search stops at that point and the
+    /// found NSEC3 RR(set) will be returned in the closest_proof member of
+    /// \c FindNSEC3Result.  \c matched is true or false depending on
+    /// the found NSEC3 is a matched one or covering one.  \c next_proof
+    /// is always NULL.  closest_labels must be equal to the number of
+    /// labels of \c name (and therefore meaningless).
+    ///
+    /// If \c recursive is true, it will continue the search toward the zone
+    /// apex (origin name) until it finds a provable encloser, that is,
+    /// an ancestor of \c name that has a matching NSEC3.  This is the closest
+    /// provable encloser of \c name as defined in RFC5155.  In this case,
+    /// if the found encloser is not equal to \c name, the search should
+    /// have seen a covering NSEC3 for the immediate child of the found
+    /// encloser.  That child name is the next closer name as defined in
+    /// RFC5155.  In this case, this method returns the NSEC3 for the
+    /// closest encloser in \c closest_proof, and the NSEC3 for the next
+    /// closer name in \c next_proof of \c FindNSEC3Result.  This set of
+    /// NSEC3 RRs provide the closest encloser proof as defined in RFC5155.
+    /// closest_labels will be set to the number of labels of the identified
+    /// closest encloser.  This will be useful when the caller needs to
+    /// construct the closest encloser name from the original \c name.
+    /// If, on the other hand, the found closest name is equal to \c name,
+    /// this method simply returns it in \c closest_proof.  \c next_proof
+    /// is set to NULL.  In all cases \c matched is set to true.
+    /// closest_labels will be set to the number of labels of \c name.
+    ///
+    /// When looking for NSEC3, this method retrieves NSEC3 parameters from
+    /// the corresponding zone to calculate hash values.  Actual implementation
+    /// of how to do this will differ in different data sources.  If the
+    /// NSEC3 parameters are not available \c DataSourceError exception
+    /// will be thrown.
+    ///
+    /// \note This implicitly means this method assumes the zone does not
+    /// have more than one set of parameters.  This assumption should be
+    /// reasonable in actual deployment and will help simplify the interface
+    /// and implementation.  But if there's a real need for supporting
+    /// multiple sets of parameters in a single zone, we will have to
+    /// extend this method so that, e.g., the caller can specify the parameter
+    /// set.
+    ///
+    /// In general, this method expects the zone is properly signed with NSEC3
+    /// RRs.  Specifically, it assumes at least the apex node has a matching
+    /// NSEC3 RR (so the search in the recursive mode must always succeed);
+    /// it also assumes that it can retrieve NSEC parameters (iterations,
+    /// algorithm, and salt) from the zone as noted above.  If these
+    /// assumptions aren't met, \c DataSourceError exception will be thrown.
+    ///
+    /// \exception OutOfZone name is not a subdomain of the zone origin
+    /// \exception DataSourceError Low-level or internal datasource errors
+    /// happened, or the zone isn't properly signed with NSEC3
+    /// (NSEC3 parameters cannot be found, no NSEC3s are available, etc).
+    /// \exception std::bad_alloc The underlying implementation involves
+    /// memory allocation and it fails
+    ///
+    /// \param name The name for which NSEC3 RRs are to be found.  It must
+    /// be a subdomain of the zone.
+    /// \param recursive Whether or not search should continue until it finds
+    /// a provable encloser (see above).
+    ///
+    /// \return The search result and whether or not the closest_proof is
+    /// a matching NSEC3, in the form of \c FindNSEC3Result object.
+    virtual FindNSEC3Result
+    findNSEC3(const isc::dns::Name& name, bool recursive) = 0;
+    //@}
+};
+
+/// \brief Operator to combine FindOptions
+///
+/// We would need to manually static-cast the options if we put or
+/// between them, which is undesired with bit-flag options. Therefore
+/// we hide the cast here, which is the simplest solution and it still
+/// provides reasonable level of type safety.
+inline ZoneFinder::FindOptions operator |(ZoneFinder::FindOptions a,
+                                          ZoneFinder::FindOptions b)
+{
+    return (static_cast<ZoneFinder::FindOptions>(static_cast<unsigned>(a) |
+                                                 static_cast<unsigned>(b)));
+}
+
+/// \brief Operator to combine FindResultFlags
+///
+/// Similar to the same operator for \c FindOptions.  Refer to the description
+/// of that function.
+inline ZoneFinder::FindResultFlags operator |(
+    ZoneFinder::FindResultFlags a,
+    ZoneFinder::FindResultFlags b)
+{
+    return (static_cast<ZoneFinder::FindResultFlags>(
+                static_cast<unsigned>(a) | static_cast<unsigned>(b)));
+}
+
+/// \brief A pointer-like type pointing to a \c ZoneFinder object.
+typedef boost::shared_ptr<ZoneFinder> ZoneFinderPtr;
+
+/// \brief A pointer-like type pointing to an immutable \c ZoneFinder object.
+typedef boost::shared_ptr<const ZoneFinder> ConstZoneFinderPtr;
+
+/// \brief A pointer-like type pointing to a \c ZoneFinder::Context object.
+typedef boost::shared_ptr<ZoneFinder::Context> ZoneFinderContextPtr;
+
+/// \brief A pointer-like type pointing to an immutable
+/// \c ZoneFinder::Context object.
+typedef boost::shared_ptr<ZoneFinder::Context> ConstZoneFinderContextPtr;
+
+} // end of datasrc
+} // end of isc
+
+#endif  // DATASRC_ZONE_FINDER_H
+
+// Local Variables:
+// mode: c++
+// End:

+ 1 - 1
src/lib/datasrc/zone_finder_context.cc

@@ -19,7 +19,7 @@
 #include <dns/rrtype.h>
 #include <dns/rdataclass.h>
 
-#include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 
 #include <boost/foreach.hpp>
 

src/lib/datasrc/iterator.h → src/lib/datasrc/zone_iterator.h


+ 1 - 1
src/lib/datasrc/zone_loader.cc

@@ -17,7 +17,7 @@
 
 #include <datasrc/client.h>
 #include <datasrc/data_source.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/zone.h>
 #include <datasrc/logger.h>
 #include <datasrc/rrset_collection_base.h>

+ 1 - 1
src/lib/datasrc/zonetable.h

@@ -19,7 +19,7 @@
 
 #include <dns/rrset.h>
 
-#include <datasrc/zone.h>
+#include <datasrc/zone_finder.h>
 
 #include <boost/shared_ptr.hpp>
 

+ 1 - 9
src/lib/dhcp/libdhcp++.cc

@@ -267,20 +267,12 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
     return (offset);
 }
 
-void LibDHCP::packOptions6(isc::util::OutputBuffer &buf,
-                           const isc::dhcp::Option::OptionCollection& options) {
-    for (Option::OptionCollection::const_iterator it = options.begin();
-         it != options.end(); ++it) {
-        it->second->pack(buf);
-    }
-}
-
 void
 LibDHCP::packOptions(isc::util::OutputBuffer& buf,
                      const Option::OptionCollection& options) {
     for (Option::OptionCollection::const_iterator it = options.begin();
          it != options.end(); ++it) {
-        it->second->pack4(buf);
+        it->second->pack(buf);
     }
 }
 

+ 0 - 10
src/lib/dhcp/libdhcp++.h

@@ -88,16 +88,6 @@ public:
                                               uint16_t type,
                                               const OptionBuffer& buf);
 
-    /// Builds collection of options.
-    ///
-    /// Builds raw (on-wire) data for provided collection of options.
-    ///
-    /// @param buf output buffer (assembled options will be stored here)
-    /// @param options collection of options to store to
-    static void packOptions6(isc::util::OutputBuffer& buf,
-                             const isc::dhcp::Option::OptionCollection& options);
-
-
     /// @brief Stores options in a buffer.
     ///
     /// Stores all options defined in options containers in a on-wire

+ 8 - 54
src/lib/dhcp/option.cc

@@ -84,51 +84,14 @@ Option::check() {
 }
 
 void Option::pack(isc::util::OutputBuffer& buf) {
-    switch (universe_) {
-    case V6:
-        return (pack6(buf));
-
-    case V4:
-        return (pack4(buf));
-
-    default:
-        isc_throw(BadValue, "Failed to pack " << type_ << " option as the "
-                  << "universe type is unknown.");
+    // Write a header.
+    packHeader(buf);
+    // Write data.
+    if (!data_.empty()) {
+        buf.writeData(&data_[0], data_.size());
     }
-}
-
-void
-Option::pack4(isc::util::OutputBuffer& buf) {
-    if (universe_ == V4) {
-        // Write a header.
-        packHeader(buf);
-        // Write data.
-        if (!data_.empty()) {
-            buf.writeData(&data_[0], data_.size());
-        }
-        // Write sub-options.
-        packOptions(buf);
-    } else {
-        isc_throw(BadValue, "Invalid universe type " << universe_);
-    }
-
-    return;
-}
-
-void Option::pack6(isc::util::OutputBuffer& buf) {
-    if (universe_ == V6) {
-        // Write a header.
-        packHeader(buf);
-        // Write data.
-        if (!data_.empty()) {
-            buf.writeData(&data_[0], data_.size());
-        }
-        // Write sub-options.
-        packOptions(buf);
-    } else {
-        isc_throw(BadValue, "Invalid universe type " << universe_);
-    }
-    return;
+    // Write sub-options.
+    packOptions(buf);
 }
 
 void
@@ -153,16 +116,7 @@ Option::packHeader(isc::util::OutputBuffer& buf) {
 
 void
 Option::packOptions(isc::util::OutputBuffer& buf) {
-    switch (universe_) {
-    case V4:
-        LibDHCP::packOptions(buf, options_);
-        return;
-    case V6:
-        LibDHCP::packOptions6(buf, options_);
-        return;
-    default:
-        isc_throw(isc::BadValue, "Invalid universe type " << universe_);
-    }
+    LibDHCP::packOptions(buf, options_);
 }
 
 void Option::unpack(OptionBufferConstIter begin,

+ 1 - 23
src/lib/dhcp/option.h

@@ -158,28 +158,13 @@ public:
     ///
     /// Writes option in wire-format to buffer, returns pointer to first unused
     /// byte after stored option (that is useful for writing options one after
-    /// another). Used in DHCPv6 options.
-    ///
-    /// @todo Migrate DHCPv6 code to pack(OutputBuffer& buf) version
+    /// another).
     ///
     /// @param buf pointer to a buffer
     ///
     /// @throw BadValue Universe of the option is neither V4 nor V6.
     virtual void pack(isc::util::OutputBuffer& buf);
 
-    /// @brief Writes option in a wire-format to a buffer.
-    ///
-    /// Method will throw if option storing fails for some reason.
-    ///
-    /// @todo Once old (DHCPv6) implementation is rewritten,
-    /// unify pack4() and pack6() and rename them to just pack().
-    ///
-    /// @param buf output buffer (option will be stored there)
-    ///
-    /// @throw OutOfRange Option type is greater than 255.
-    /// @throw BadValue Universe is not V4.
-    virtual void pack4(isc::util::OutputBuffer& buf);
-
     /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
@@ -317,13 +302,6 @@ public:
     virtual bool equal(const OptionPtr& other) const;
 
 protected:
-    /// Builds raw (over-wire) buffer of this option, including all
-    /// defined suboptions. Version for building DHCPv4 options.
-    ///
-    /// @param buf output buffer (built options will be stored here)
-    ///
-    /// @throw BadValue Universe is not V6.
-    virtual void pack6(isc::util::OutputBuffer& buf);
 
     /// @brief Store option's header in a buffer.
     ///

+ 1 - 1
src/lib/dhcp/option4_addrlst.cc

@@ -64,7 +64,7 @@ Option4AddrLst::Option4AddrLst(uint8_t type, const IOAddress& addr)
 }
 
 void
-Option4AddrLst::pack4(isc::util::OutputBuffer& buf) {
+Option4AddrLst::pack(isc::util::OutputBuffer& buf) {
 
     if (addrs_.size() * V4ADDRESS_LEN > 255) {
         isc_throw(OutOfRange, "DHCPv4 Option4AddrLst " << type_ << " is too big."

+ 6 - 4
src/lib/dhcp/option4_addrlst.h

@@ -29,6 +29,11 @@
 namespace isc {
 namespace dhcp {
 
+/// Forward declaration to Option4AddrLst class.
+class Option4AddrLst;
+
+/// A pointer to the Option4AddrLst object.
+typedef boost::shared_ptr<Option4AddrLst> Option4AddrLstPtr;
 
 /// @brief DHCPv4 Option class for handling list of IPv4 addresses.
 ///
@@ -87,11 +92,8 @@ public:
     ///
     /// Method will throw if option storing fails for some reason.
     ///
-    /// TODO Once old (DHCPv6) implementation is rewritten,
-    /// unify pack4() and pack6() and rename them to just pack().
-    ///
     /// @param buf output buffer (option will be stored there)
-    virtual void pack4(isc::util::OutputBuffer& buf);
+    virtual void pack(isc::util::OutputBuffer& buf);
 
     /// Returns string representation of the option.
     ///

+ 3 - 22
src/lib/dhcp/option_custom.cc

@@ -387,14 +387,10 @@ OptionCustom::dataFieldToText(const OptionDataType data_type,
 }
 
 void
-OptionCustom::pack4(isc::util::OutputBuffer& buf) {
-    if (len() > 255) {
-        isc_throw(OutOfRange, "DHCPv4 Option " << type_
-                  << " value is too high. At most 255 is supported.");
-    }
+OptionCustom::pack(isc::util::OutputBuffer& buf) {
 
-    buf.writeUint8(type_);
-    buf.writeUint8(len() - getHeaderLen());
+    // Pack DHCP header (V4 or V6).
+    packHeader(buf);
 
     // Write data from buffers.
     for (std::vector<OptionBuffer>::const_iterator it = buffers_.begin();
@@ -411,21 +407,6 @@ OptionCustom::pack4(isc::util::OutputBuffer& buf) {
     packOptions(buf);
 }
 
-void
-OptionCustom::pack6(isc::util::OutputBuffer& buf) {
-    buf.writeUint16(type_);
-    buf.writeUint16(len() - getHeaderLen());
-
-    // Write data from buffers.
-    for (std::vector<OptionBuffer>::const_iterator it = buffers_.begin();
-         it != buffers_.end(); ++it) {
-        if (!it->empty()) {
-            buf.writeData(&(*it)[0], it->size());
-        }
-    }
-
-    packOptions(buf);
-}
 
 asiolink::IOAddress
 OptionCustom::readAddress(const uint32_t index) const {

+ 5 - 12
src/lib/dhcp/option_custom.h

@@ -249,6 +249,11 @@ public:
     void writeString(const std::string& text,
                      const uint32_t index = 0);
 
+    /// @brief Writes DHCP option in a wire format to a buffer.
+    ///
+    /// @param buf output buffer (option will be stored there).
+    virtual void pack(isc::util::OutputBuffer& buf);
+
     /// @brief Parses received buffer.
     ///
     /// @param begin iterator to first byte of option data
@@ -278,18 +283,6 @@ public:
     void setData(const OptionBufferConstIter first,
                  const OptionBufferConstIter last);
 
-protected:
-
-    /// @brief Writes DHCPv4 option in a wire format to a buffer.
-    ///
-    /// @param buf output buffer (option will be stored there).
-    virtual void pack4(isc::util::OutputBuffer& buf);
-
-    /// @brief Writes DHCPv6 option in a wire format to a buffer.
-    ///
-    /// @param buf output buffer (built options will be stored here)
-    virtual void pack6(isc::util::OutputBuffer& buf);
-
 private:
 
     /// @brief Verify that the option comprises an array of values.

+ 4 - 10
src/lib/dhcp/option_definition.cc

@@ -216,8 +216,8 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
         const RecordFieldsCollection& records = getRecordFields();
         if (records.size() > values.size()) {
             isc_throw(InvalidOptionValue, "number of data fields for the option"
-                      << " type " << type_ << " is greater than number of values"
-                      << " provided.");
+                      << " type '" <<  getCode() << "' is greater than number"
+                      << " of values provided.");
         }
         for (size_t i = 0; i < records.size(); ++i) {
             writeToBuffer(util::str::trim(values[i]),
@@ -444,14 +444,8 @@ OptionDefinition::writeToBuffer(const std::string& value,
         OptionDataTypeUtil::writeString(value, buf);
         return;
     case OPT_FQDN_TYPE:
-        {
-            // FQDN implementation is not terribly complicated but will require
-            // creation of some additional logic (maybe object) that will parse
-            // the fqdn into labels.
-            isc_throw(isc::NotImplemented, "write of FQDN record into option buffer"
-                      " is not supported yet");
-            return;
-        }
+        OptionDataTypeUtil::writeFqdn(value, buf);
+        return;
     default:
         // We hit this point because invalid option data type has been specified
         // This may be the case because 'empty' or 'record' data type has been

+ 24 - 0
src/lib/dhcp/option_int_array.h

@@ -25,6 +25,23 @@
 namespace isc {
 namespace dhcp {
 
+/// Forward declaration of OptionIntArray.
+template<typename T>
+class OptionIntArray;
+
+/// @defgroup option_int_array_defs Typedefs for OptionIntArray class.
+///
+/// @brief Classes that represent options comprising array of integers.
+///
+/// @{
+typedef OptionIntArray<uint8_t> OptionUint8Array;
+typedef boost::shared_ptr<OptionUint8Array> OptionUint8ArrayPtr;
+typedef OptionIntArray<uint16_t> OptionUint16Array;
+typedef boost::shared_ptr<OptionUint16Array> OptionUint16ArrayPtr;
+typedef OptionIntArray<uint32_t> OptionUint32Array;
+typedef boost::shared_ptr<OptionUint32Array> OptionUint32ArrayPtr;
+/// @}
+
 /// This template class represents DHCP (v4 or v6) option with an
 /// array of integer values. The type of the elements in the array
 /// can be any of the following:
@@ -107,6 +124,13 @@ public:
         unpack(begin, end);
     }
 
+    /// @brief Adds a new value to the array.
+    ///
+    /// @param value a value being added.
+    void addValue(const T value) {
+        values_.push_back(value);
+    }
+
     /// Writes option in wire-format to buf, returns pointer to first unused
     /// byte after stored option.
     ///

+ 5 - 6
src/lib/dhcp/pkt4.cc

@@ -219,7 +219,7 @@ void Pkt4::check() {
     uint8_t msg_type = getType();
     if (msg_type > DHCPLEASEACTIVE) {
         isc_throw(BadValue, "Invalid DHCP message type received: "
-                  << msg_type);
+                  << static_cast<int>(msg_type));
     }
 }
 
@@ -230,10 +230,10 @@ uint8_t Pkt4::getType() const {
     }
 
     // Check if Message Type is specified as OptionInt<uint8_t>
-    boost::shared_ptr<OptionInt<uint8_t> > typeOpt =
+    boost::shared_ptr<OptionInt<uint8_t> > type_opt =
         boost::dynamic_pointer_cast<OptionInt<uint8_t> >(generic);
-    if (typeOpt) {
-        return (typeOpt->getValue());
+    if (type_opt) {
+        return (type_opt->getValue());
     }
 
     // Try to use it as generic option
@@ -253,7 +253,6 @@ void Pkt4::setType(uint8_t dhcp_type) {
     }
 }
 
-
 void Pkt4::repack() {
     bufferOut_.writeData(&data_[0], data_.size());
 }
@@ -263,7 +262,7 @@ Pkt4::toText() {
     stringstream tmp;
     tmp << "localAddr=" << local_addr_.toText() << ":" << local_port_
         << " remoteAddr=" << remote_addr_.toText()
-        << ":" << remote_port_ << ", msgtype=" << getType()
+        << ":" << remote_port_ << ", msgtype=" << static_cast<int>(getType())
         << ", transid=0x" << hex << transid_ << dec << endl;
 
     for (isc::dhcp::Option::OptionCollection::iterator opt=options_.begin();

+ 3 - 3
src/lib/dhcp/pkt6.cc

@@ -90,7 +90,7 @@ Pkt6::packUDP() {
         bufferOut_.writeUint8( (transid_) & 0xff );
 
         // the rest are options
-        LibDHCP::packOptions6(bufferOut_, options_);
+        LibDHCP::packOptions(bufferOut_, options_);
     }
     catch (const Exception& e) {
         /// @todo: throw exception here once we turn this function to void.
@@ -155,8 +155,8 @@ Pkt6::toText() {
     tmp << "localAddr=[" << local_addr_.toText() << "]:" << local_port_
         << " remoteAddr=[" << remote_addr_.toText()
         << "]:" << remote_port_ << endl;
-    tmp << "msgtype=" << msg_type_ << ", transid=0x" << hex << transid_
-        << dec << endl;
+    tmp << "msgtype=" << static_cast<int>(msg_type_) << ", transid=0x" <<
+        hex << transid_ << dec << endl;
     for (isc::dhcp::Option::OptionCollection::iterator opt=options_.begin();
          opt != options_.end();
          ++opt) {

+ 1 - 1
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -273,7 +273,7 @@ TEST_F(LibDhcpTest, packOptions6) {
 
     OutputBuffer assembled(512);
 
-    EXPECT_NO_THROW(LibDHCP::packOptions6(assembled, opts));
+    EXPECT_NO_THROW(LibDHCP::packOptions(assembled, opts));
     EXPECT_EQ(sizeof(v6packed), assembled.getLength());
     EXPECT_EQ(0, memcmp(assembled.getData(), v6packed, sizeof(v6packed)));
 }

+ 2 - 2
src/lib/dhcp/tests/option4_addrlst_unittest.cc

@@ -155,7 +155,7 @@ TEST_F(Option4AddrLstTest, assembly1) {
 
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     ASSERT_EQ(6, opt->len());
@@ -198,7 +198,7 @@ TEST_F(Option4AddrLstTest, assembly4) {
 
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     ASSERT_EQ(18, opt->len()); // 2(header) + 4xsizeof(IPv4addr)

+ 74 - 95
src/lib/dhcp/tests/option_int_array_unittest.cc

@@ -294,6 +294,52 @@ public:
         EXPECT_TRUE(std::equal(buf_.begin(), buf_.begin() + opt_len, out_data.begin()));;
     }
 
+    /// @brief Test ability to set all values.
+    ///
+    /// @tparam T numeric type to perform the test for.
+    template<typename T>
+    void setValuesTest() {
+        const uint16_t opt_code = 100;
+        // Create option with empty vector of values.
+        boost::shared_ptr<OptionIntArray<T> >
+            opt(new OptionIntArray<T>(Option::V6, opt_code));
+        // Initialize vector with some data and pass to the option.
+        std::vector<T> values;
+        for (int i = 0; i < 10; ++i) {
+            values.push_back(numeric_limits<uint8_t>::max() - i);
+        }
+        opt->setValues(values);
+
+        // Check if universe, option type and data was set correctly.
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        std::vector<T> returned_values = opt->getValues();
+        EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    }
+
+    /// @brief Test ability to add values one by one.
+    ///
+    /// @tparam T numeric type to perform the test for.
+    template<typename T>
+    void addValuesTest() {
+        const uint16_t opt_code = 100;
+        // Create option with empty vector of values.
+        boost::shared_ptr<OptionIntArray<T> >
+            opt(new OptionIntArray<T>(Option::V6, opt_code));
+        // Initialize vector with some data and add the same data
+        // to the option.
+        std::vector<T> values;
+        for (int i = 0; i < 10; ++i) {
+            values.push_back(numeric_limits<T>::max() - i);
+            opt->addValue(numeric_limits<T>::max() - i);
+        }
+
+        // Check if universe, option type and data was set correctly.
+        EXPECT_EQ(Option::V6, opt->getUniverse());
+        EXPECT_EQ(opt_code, opt->getType());
+        std::vector<T> returned_values = opt->getValues();
+        EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    }
 
     OptionBuffer buf_;     ///< Option buffer
     OutputBuffer out_buf_; ///< Output buffer
@@ -371,118 +417,51 @@ TEST_F(OptionIntArrayTest, bufferToInt32V6) {
 }
 
 TEST_F(OptionIntArrayTest, setValuesUint8) {
-    const uint16_t opt_code = 100;
-    // Create option with empty vector of values.
-    boost::shared_ptr<OptionIntArray<uint8_t> >
-        opt(new OptionIntArray<uint8_t>(Option::V6, opt_code));
-    // Initialize vector with some data and pass to the option.
-    std::vector<uint8_t> values;
-    for (int i = 0; i < 10; ++i) {
-        values.push_back(numeric_limits<uint8_t>::max() - i);
-    }
-    opt->setValues(values);
-
-    // Check if universe, option type and data was set correctly.
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    std::vector<uint8_t> returned_values = opt->getValues();
-    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    setValuesTest<uint8_t>();
 }
 
 TEST_F(OptionIntArrayTest, setValuesInt8) {
-    const uint16_t opt_code = 100;
-    // Create option with empty vector of values.
-    boost::shared_ptr<OptionIntArray<int8_t> >
-        opt(new OptionIntArray<int8_t>(Option::V6, opt_code));
-    // Initialize vector with some data and pass to the option.
-    std::vector<int8_t> values;
-    for (int i = 0; i < 10; ++i) {
-        values.push_back(numeric_limits<int8_t>::min() + i);
-    }
-    opt->setValues(values);
-
-    // Check if universe, option type and data was set correctly.
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    std::vector<int8_t> returned_values = opt->getValues();
-    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    setValuesTest<int8_t>();
 }
 
 TEST_F(OptionIntArrayTest, setValuesUint16) {
-    const uint16_t opt_code = 101;
-    // Create option with empty vector of values.
-    boost::shared_ptr<OptionIntArray<uint16_t> >
-        opt(new OptionIntArray<uint16_t>(Option::V6, opt_code));
-    // Initialize vector with some data and pass to the option.
-    std::vector<uint16_t> values;
-    for (int i = 0; i < 10; ++i) {
-        values.push_back(numeric_limits<uint16_t>::max() - i);
-    }
-    opt->setValues(values);
-
-    // Check if universe, option type and data was set correctly.
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    std::vector<uint16_t> returned_values = opt->getValues();
-    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    setValuesTest<uint16_t>();
 }
 
 TEST_F(OptionIntArrayTest, setValuesInt16) {
-    const uint16_t opt_code = 101;
-    // Create option with empty vector of values.
-    boost::shared_ptr<OptionIntArray<int16_t> >
-        opt(new OptionIntArray<int16_t>(Option::V6, opt_code));
-    // Initialize vector with some data and pass to the option.
-    std::vector<int16_t> values;
-    for (int i = 0; i < 10; ++i) {
-        values.push_back(numeric_limits<int16_t>::min() + i);
-    }
-    opt->setValues(values);
-
-    // Check if universe, option type and data was set correctly.
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    std::vector<int16_t> returned_values = opt->getValues();
-    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    setValuesTest<int16_t>();
 }
 
 TEST_F(OptionIntArrayTest, setValuesUint32) {
-    const uint32_t opt_code = 101;
-    // Create option with empty vector of values.
-    boost::shared_ptr<OptionIntArray<uint32_t> >
-        opt(new OptionIntArray<uint32_t>(Option::V6, opt_code));
-    // Initialize vector with some data and pass to the option.
-    std::vector<uint32_t> values;
-    for (int i = 0; i < 10; ++i) {
-        values.push_back(numeric_limits<uint32_t>::max() - i);
-    }
-    opt->setValues(values);
-
-    // Check if universe, option type and data was set correctly.
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    std::vector<uint32_t> returned_values = opt->getValues();
-    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+    setValuesTest<uint16_t>();
 }
 
 TEST_F(OptionIntArrayTest, setValuesInt32) {
-    const uint32_t opt_code = 101;
-    // Create option with empty vector of values.
-    boost::shared_ptr<OptionIntArray<int32_t> >
-        opt(new OptionIntArray<int32_t>(Option::V6, opt_code));
-    // Initialize vector with some data and pass to the option.
-    std::vector<int32_t> values;
-    for (int i = 0; i < 10; ++i) {
-        values.push_back(numeric_limits<int32_t>::min() + i);
-    }
-    opt->setValues(values);
+    setValuesTest<int16_t>();
+}
 
-    // Check if universe, option type and data was set correctly.
-    EXPECT_EQ(Option::V6, opt->getUniverse());
-    EXPECT_EQ(opt_code, opt->getType());
-    std::vector<int32_t> returned_values = opt->getValues();
-    EXPECT_TRUE(std::equal(values.begin(), values.end(), returned_values.begin()));
+TEST_F(OptionIntArrayTest, addValuesUint8) {
+    addValuesTest<uint8_t>();
 }
 
+TEST_F(OptionIntArrayTest, addValuesInt8) {
+    addValuesTest<int8_t>();
+}
+
+TEST_F(OptionIntArrayTest, addValuesUint16) {
+    addValuesTest<uint16_t>();
+}
+
+TEST_F(OptionIntArrayTest, addValuesInt16) {
+    addValuesTest<int16_t>();
+}
+
+TEST_F(OptionIntArrayTest, addValuesUint32) {
+    addValuesTest<uint16_t>();
+}
+
+TEST_F(OptionIntArrayTest, addValuesInt32) {
+    addValuesTest<int16_t>();
+}
 
 } // anonymous namespace

+ 7 - 7
src/lib/dhcp/tests/option_unittest.cc

@@ -116,7 +116,7 @@ TEST_F(OptionTest, v4_data1) {
     // now store that option into a buffer
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     // check content of that buffer
@@ -173,7 +173,7 @@ TEST_F(OptionTest, v4_data2) {
     // now store that option into a buffer
     OutputBuffer buf(100);
     EXPECT_NO_THROW(
-        opt->pack4(buf);
+        opt->pack(buf);
     );
 
     // check content of that buffer
@@ -471,7 +471,7 @@ TEST_F(OptionTest, setUintX) {
     // verify setUint8
     opt1->setUint8(255);
     EXPECT_EQ(255, opt1->getUint8());
-    opt1->pack4(outBuf_);
+    opt1->pack(outBuf_);
     EXPECT_EQ(3, opt1->len());
     EXPECT_EQ(3, outBuf_.getLength());
     uint8_t exp1[] = {125, 1, 255};
@@ -480,7 +480,7 @@ TEST_F(OptionTest, setUintX) {
     // verify getUint16
     outBuf_.clear();
     opt2->setUint16(12345);
-    opt2->pack4(outBuf_);
+    opt2->pack(outBuf_);
     EXPECT_EQ(12345, opt2->getUint16());
     EXPECT_EQ(4, opt2->len());
     EXPECT_EQ(4, outBuf_.getLength());
@@ -490,7 +490,7 @@ TEST_F(OptionTest, setUintX) {
     // verify getUint32
     outBuf_.clear();
     opt4->setUint32(0x12345678);
-    opt4->pack4(outBuf_);
+    opt4->pack(outBuf_);
     EXPECT_EQ(0x12345678, opt4->getUint32());
     EXPECT_EQ(6, opt4->len());
     EXPECT_EQ(6, outBuf_.getLength());
@@ -505,7 +505,7 @@ TEST_F(OptionTest, setData) {
                               buf_.begin(), buf_.begin() + 10));
     buf_.resize(20, 1);
     opt1->setData(buf_.begin(), buf_.end());
-    opt1->pack4(outBuf_);
+    opt1->pack(outBuf_);
     ASSERT_EQ(outBuf_.getLength() - opt1->getHeaderLen(), buf_.size());
     const uint8_t* test_data = static_cast<const uint8_t*>(outBuf_.getData());
     EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),
@@ -518,7 +518,7 @@ TEST_F(OptionTest, setData) {
     outBuf_.clear();
     buf_.resize(5, 1);
     opt2->setData(buf_.begin(), buf_.end());
-    opt2->pack4(outBuf_);
+    opt2->pack(outBuf_);
     ASSERT_EQ(outBuf_.getLength() - opt1->getHeaderLen(), buf_.size());
     test_data = static_cast<const uint8_t*>(outBuf_.getData());
     EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),

+ 2 - 2
src/lib/dhcpsrv/alloc_engine.h

@@ -214,8 +214,8 @@ protected:
     /// @param clientid client identifier
     /// @param hwaddr client's hardware address
     /// @param lease lease to be renewed
-    /// @param renewed lease (typically the same passed as lease parameter)
-    ///        or NULL if the lease cannot be renewed
+    /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
+    ///        an address for DISCOVER that is not really allocated (true)
     Lease4Ptr
     renewLease4(const SubnetPtr& subnet,
                 const ClientIdPtr& clientid,

+ 1 - 1
src/lib/dhcpsrv/dbaccess_parser.cc

@@ -48,7 +48,7 @@ DbAccessParser::build(isc::data::ConstElementPtr config_value) {
     // 4. If all is OK, update the stored keyword/value pairs.
 
     // 1. Take a copy of the stored keyword/value pairs.
-    map<string, string> values_copy = values_;
+    std::map<string, string> values_copy = values_;
 
     // 2. Update the copy with the passed keywords.
     BOOST_FOREACH(ConfigPair param, config_value->mapValue()) {

+ 12 - 2
src/lib/dhcpsrv/lease_mgr.h

@@ -114,9 +114,18 @@ public:
 /// leases.
 struct Lease {
 
+    /// @brief Constructor
+    ///
+    /// @param addr IP address
+    /// @param t1 renewal time
+    /// @param t2 rebinding time
+    /// @param valid_lft Lifetime of the lease
+    /// @param subnet_id Subnet identification
+    /// @param cltt Client last transmission time
     Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2,
           uint32_t valid_lft, SubnetID subnet_id, time_t cltt);
 
+    /// @brief Destructor
     virtual ~Lease() {}
 
     /// @brief IPv4 ot IPv6 address
@@ -226,13 +235,14 @@ struct Lease4 : public Lease {
 
     /// @brief Constructor
     ///
-    /// @param addr IPv4 address as unsigned 32-bit integer in network byte
-    ///        order.
+    /// @param addr IPv4 address.
     /// @param hwaddr Hardware address buffer
     /// @param hwaddr_len Length of hardware address buffer
     /// @param clientid Client identification buffer
     /// @param clientid_len Length of client identification buffer
     /// @param valid_lft Lifetime of the lease
+    /// @param t1 renewal time
+    /// @param t2 rebinding time
     /// @param cltt Client last transmission time
     /// @param subnet_id Subnet identification
     Lease4(const isc::asiolink::IOAddress& addr, const uint8_t* hwaddr, size_t hwaddr_len,

+ 1 - 1
src/lib/dhcpsrv/option_space_container.h

@@ -41,7 +41,7 @@ public:
     /// @brief Adds a new item to the option_space.
     ///
     /// @param item reference to the item being added.
-    /// @param name of the option space.
+    /// @param option_space name of the option space.
     void addItem(const ItemType& item, const std::string& option_space) {
         ItemsContainerPtr items = getItems(option_space);
         items->push_back(item);

+ 6 - 7
src/lib/dhcpsrv/subnet.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <asiolink/io_address.h>
+#include <dhcp/option_space.h>
 #include <dhcpsrv/addr_utilities.h>
 #include <dhcpsrv/subnet.h>
 
@@ -44,14 +45,12 @@ bool Subnet::inRange(const isc::asiolink::IOAddress& addr) const {
 }
 
 void
-Subnet::addOption(OptionPtr& option, bool persistent,
+Subnet::addOption(const OptionPtr& option, bool persistent,
                   const std::string& option_space) {
-    // @todo Once the #2313 is merged we need to use the OptionSpace object to
-    // validate the option space name here. For now, let's check that the name
-    // is not empty as the empty namespace has a special meaning here - it is
-    // returned when desired namespace is not found when getOptions is called.
-    if (option_space.empty()) {
-        isc_throw(isc::BadValue, "option space name must not be empty");
+    // Check that the option space name is valid.
+    if (!OptionSpace::validateName(option_space)) {
+        isc_throw(isc::BadValue, "invalid option space name: '"
+                  << option_space << "'");
     }
     validateOption(option);
 

+ 2 - 2
src/lib/dhcpsrv/subnet.h

@@ -69,7 +69,7 @@ public:
         ///
         /// @param opt option
         /// @param persist if true option is always sent.
-        OptionDescriptor(OptionPtr& opt, bool persist)
+        OptionDescriptor(const OptionPtr& opt, bool persist)
             : option(opt), persistent(persist) {};
 
         /// @brief Constructor
@@ -225,7 +225,7 @@ public:
     /// @param option_space name of the option space to add an option to.
     ///
     /// @throw isc::BadValue if invalid option provided.
-    void addOption(OptionPtr& option, bool persistent,
+    void addOption(const OptionPtr& option, bool persistent,
                    const std::string& option_space);
 
     /// @brief Delete all options configured for the subnet.

+ 2 - 2
src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc

@@ -120,7 +120,7 @@ public:
         SCOPED_TRACE(trace_string);
 
         // Construct a map of keyword value pairs.
-        map<string, string> expected;
+        std::map<string, string> expected;
         size_t expected_count = 0;
         for (size_t i = 0; keyval[i] != NULL; i += 2) {
             // Get the value.  This should not be NULL
@@ -147,7 +147,7 @@ public:
              actual != parameters.end(); ++actual) {
 
             // Does the keyword exist in the set of expected keywords?
-            map<string, string>::iterator corresponding =
+            std::map<string, string>::iterator corresponding =
                 expected.find(actual->first);
             ASSERT_TRUE(corresponding != expected.end());
 

+ 1 - 1
src/lib/python/isc/datasrc/client_python.cc

@@ -27,7 +27,7 @@
 #include <datasrc/database.h>
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_accessor.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/client_list.h>
 
 #include <dns/python/name_python.h>

+ 1 - 1
src/lib/python/isc/datasrc/finder_python.cc

@@ -26,7 +26,7 @@
 #include <datasrc/database.h>
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_accessor.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 #include <datasrc/zone.h>
 
 #include <dns/python/name_python.h>

+ 1 - 1
src/lib/python/isc/datasrc/iterator_python.cc

@@ -25,7 +25,7 @@
 #include <datasrc/client.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
-#include <datasrc/iterator.h>
+#include <datasrc/zone_iterator.h>
 
 #include <dns/python/name_python.h>
 #include <dns/python/rrset_python.h>

+ 1 - 1
src/lib/python/isc/datasrc/updater_python.cc

@@ -203,7 +203,7 @@ namespace {
 
 class s_UpdaterRRsetCollection : public s_RRsetCollection {
 public:
-    s_UpdaterRRsetCollection() : s_RRsetCollection() {}
+    s_UpdaterRRsetCollection() : s_RRsetCollection(), base_obj_(NULL) {}
     PyObject* base_obj_;
 };