Merge branch 'master' into trac678

ChangeLog

@@ -1,3 +1,47 @@
+  206.  [func]		shane
+	Add the ability to list the running BIND 10 processes using the
+	command channel. To try this, use "Boss show_processes".
+	(Trac #648, git 451bbb67c2b5d544db2f7deca4315165245d2b3b)
+
+  205.	[bug]		jinmei
+	b10-auth, src/lib/datasrc: fixed a bug where b10-auth could return
+	an empty additional section for delegation even if some glue is
+	crucial when it fails to find some other glue records in its data
+	source.
+	(Trac #646, git 6070acd1c5b2f7a61574eda4035b93b40aab3e2b)
+
+  204.	[bug]		jinmei
+	b10-auth, src/lib/datasrc: class ANY queries were not handled
+	correctly in the generic data source (mainly for sqlite3).  It
+	could crash b10-auth in the worst case, and could result in
+	incorrect responses in some other cases.
+	(Trac #80, git c65637dd41c8d94399bd3e3cee965b694b633339)
+
+  203.  [bug]		zhang likun
+	Fix resolver cache memory leak: when cache is destructed, rrset
+	and message entries in it are not destructed properly.
+	(Trac #643, git aba4c4067da0dc63c97c6356dc3137651755ffce)
+
+  202.  [func]    vorner
+	It is possible to specify a different directory where we look for
+	configuration files (by -p) and different configuration file to
+	use (-c).  Also, it is possible to specify the port on which
+	cmdctl should listen (--cmdctl-port).
+	(Trac #615, git 5514dd78f2d61a222f3069fc94723ca33fb3200b)
+
+  201.  [bug]           jerry
+	src/bin/bindctl: bindctl doesn't show traceback on shutdown.
+	(Trac #588, git 662e99ef050d98e86614c4443326568a0b5be437)
+
+  200.  [bug]           Jelte
+	Fixed a bug where incoming TCP connections were not closed.
+	(Trac #589, git 1d88daaa24e8b1ab27f28be876f40a144241e93b)
+
+  199.  [func]           ocean
+	Cache negative responses (NXDOMAIN/NODATA) from authoritative
+	server for recursive resolver.
+	(Trac #493, git f8fb852bc6aef292555063590c361f01cf29e5ca)
+
   198.	[bug]		jinmei
 	b10-auth, src/lib/datasrc: fixed a bug where hot spot cache failed
 	to reuse cached SOA for negative responses.  Due to this bug
@@ -241,7 +285,7 @@ bind10-devel-20110224 released on February 24, 2011
 	timeout_client for sending an answer back to the client
 	timeout_lookup for stopping the resolving
 	(currently 2 and 3 have the same final effect)
-	(Trac 489, git 578ea7f4ba94dc0d8a3d39231dad2be118e125a2)
+	(Trac #489, git 578ea7f4ba94dc0d8a3d39231dad2be118e125a2)
 
   159.	[func]		smann
 	The resolver now has a configurable set of root servers to start

configure.ac

@@ -663,6 +663,7 @@ AC_CONFIG_FILES([Makefile
                  src/lib/python/isc/net/tests/Makefile
                  src/lib/python/isc/notify/Makefile
                  src/lib/python/isc/notify/tests/Makefile
+                 src/lib/python/isc/testutils/Makefile
                  src/lib/config/Makefile
                  src/lib/config/tests/Makefile
                  src/lib/config/tests/testdata/Makefile
@@ -719,9 +720,8 @@ AC_OUTPUT([doc/version.ent
            src/bin/stats/run_b10-stats_stub.sh
            src/bin/stats/tests/stats_test
            src/bin/bind10/bind10.py
-           src/bin/bind10/tests/bind10_test
-           src/bin/bind10/tests/bind10_test.py
            src/bin/bind10/run_bind10.sh
+           src/bin/bind10/tests/bind10_test.py
            src/bin/bindctl/run_bindctl.sh
            src/bin/bindctl/bindctl_main.py
            src/bin/bindctl/tests/bindctl_test

doc/guide/bind10-guide.xml

@@ -1199,10 +1199,9 @@ TODO
     <title>Incoming Zone Transfers</title>
 
     <para>
-      The <command>b10-xfrin</command> process is started by
-      <command>bind10</command>.
-      It can be manually triggered to request an AXFR zone
-      transfer. When received, it is stored in the BIND 10
+      Incoming zones are transferred using the <command>b10-xfrin</command>
+      process which is started by <command>bind10</command>.
+      When received, the zone is stored in the BIND 10
       data store, and its records can be served by
       <command>b10-auth</command>.
       In combination with <command>b10-zonemgr</command> (for
@@ -1213,8 +1212,22 @@ TODO
     <note><simpara>
      The current development release of BIND 10 only supports
      AXFR. (IXFR is not supported.) 
+
+<!-- TODO: sqlite3 data source only? -->
+
     </simpara></note>
 
+<!-- TODO:
+
+how to tell bind10 you are a secondary?
+
+when will it first attempt to check for new zone? (using REFRESH?)
+what if zonemgr is not running?
+
+what if a NOTIFY is sent?
+
+-->
+
     <para>
        To manually trigger a zone transfer to retrieve a remote zone,
        you may use the <command>bindctl</command> utility.
@@ -1223,6 +1236,9 @@ TODO
        <screen>&gt; <userinput>Xfrin retransfer zone_name="<option>foo.example.org</option>" master=<option>192.0.2.99</option></userinput></screen>
     </para>
 
+<!-- TODO: can that retransfer be used to identify a new zone? -->
+<!-- TODO: what if doesn't exist at that master IP? -->
+
   </chapter>
 
   <chapter id="xfrout">
@@ -1329,28 +1345,34 @@ what is XfroutClient xfr_client??
 
 <!-- TODO: later the above will have some defaults -->
 
-    <para>
-      To enable forwarding, the upstream address and port must be
-      configured to forward queries to, such as:
+    <section>
+      <title>Forwarding</title>
 
-      <screen>
+      <para>
+
+        To enable forwarding, the upstream address and port must be
+        configured to forward queries to, such as:
+
+        <screen>
 &gt; <userinput>config set Resolver/forward_addresses [{ "address": "<replaceable>192.168.1.1</replaceable>", "port": 53 }]</userinput>
 &gt; <userinput>config commit</userinput>
 </screen>
 
-      (Replace <replaceable>192.168.1.1</replaceable> to point to your
-      full resolver.)
-    </para>
+        (Replace <replaceable>192.168.1.1</replaceable> to point to your
+        full resolver.)
+      </para>
 
-    <para>
-      Normal iterative name service can be re-enabled by clearing the
-      forwarding address(es); for example:
+      <para>
+        Normal iterative name service can be re-enabled by clearing the
+        forwarding address(es); for example:
 
-      <screen>
+        <screen>
 &gt; <userinput>config set Resolver/forward_addresses []</userinput>
 &gt; <userinput>config commit</userinput>
 </screen>
-    </para>
+      </para>
+
+    </section>
 
 <!-- TODO: later try this
 

src/bin/auth/main.cc

@@ -163,10 +163,6 @@ main(int argc, char* argv[]) {
                                              my_command_handler);
         cout << "[b10-auth] Configuration channel established." << endl;
 
-        if (uid != NULL) {
-            changeUser(uid);
-        }
-
         xfrin_session = new Session(io_service.get_io_service());
         cout << "[b10-auth] Xfrin session channel created." << endl;
         xfrin_session->establish(NULL);
@@ -190,6 +186,10 @@ main(int argc, char* argv[]) {
         configureAuthServer(*auth_server, config_session->getFullConfig());
         auth_server->updateConfig(ElementPtr());
 
+        if (uid != NULL) {
+            changeUser(uid);
+        }
+
         cout << "[b10-auth] Server started." << endl;
         io_service.run();
 

src/bin/bind10/bind10.py.in

@@ -1,6 +1,6 @@
 #!@PYTHON@
 
-# Copyright (C) 2010  Internet Systems Consortium.
+# Copyright (C) 2010,2011  Internet Systems Consortium.
 #
 # Permission to use, copy, modify, and distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -139,7 +139,8 @@ class ProcessInfo:
         self.restart_schedule = RestartSchedule()
         self.uid = uid
         self.username = username
-        self._spawn()
+        self.process = None
+        self.pid = None
 
     def _preexec_work(self):
         """Function used before running a program that needs to run as a
@@ -186,6 +187,11 @@ class ProcessInfo:
         self.pid = self.process.pid
         self.restart_schedule.set_run_start_time()
 
+    # spawn() and respawn() are the same for now, but in the future they
+    # may have different functionality
+    def spawn(self):
+        self._spawn()
+
     def respawn(self):
         self._spawn()
 
@@ -194,14 +200,21 @@ class CChannelConnectError(Exception): pass
 class BoB:
     """Boss of BIND class."""
     
-    def __init__(self, msgq_socket_file=None, nocache=False, verbose=False,
-    setuid=None, username=None):
+    def __init__(self, msgq_socket_file=None, data_path=None,
+    config_filename=None, nocache=False, verbose=False, setuid=None,
+    username=None, cmdctl_port=None):
         """
             Initialize the Boss of BIND. This is a singleton (only one can run).
         
             The msgq_socket_file specifies the UNIX domain socket file that the
             msgq process listens on.  If verbose is True, then the boss reports
             what it is doing.
+
+            Data path and config filename are passed trough to config manager
+            (if provided) and specify the config file to be used.
+
+            The cmdctl_port is passed to cmdctl and specify on which port it
+            should listen.
         """
         self.cc_session = None
         self.ccs = None
@@ -219,6 +232,9 @@ class BoB:
         self.uid = setuid
         self.username = username
         self.verbose = verbose
+        self.data_path = data_path
+        self.config_filename = config_filename
+        self.cmdctl_port = cmdctl_port
 
     def config_handler(self, new_config):
         # If this is initial update, don't do anything now, leave it to startup
@@ -270,6 +286,14 @@ class BoB:
         answer = isc.config.ccsession.create_answer(0)
         return answer
 
+    def get_processes(self):
+        pids = list(self.processes.keys())
+        pids.sort()
+        process_list = [ ]
+        for pid in pids:
+            process_list.append([pid, self.processes[pid].name])
+        return process_list
+
     def command_handler(self, command, args):
         if self.verbose:
             sys.stdout.write("[bind10] Boss got command: " + command + "\n")
@@ -280,8 +304,13 @@ class BoB:
             if command == "shutdown":
                 self.runnable = False
                 answer = isc.config.ccsession.create_answer(0)
+            elif command == "ping":
+                answer = isc.config.ccsession.create_answer(0, "pong")
+            elif command == "show_processes":
+                answer = isc.config.ccsession. \
+                    create_answer(0, self.get_processes())
             else:
-                answer = isc.config.ccsession.create_answer(1, 
+                answer = isc.config.ccsession.create_answer(1,
                                                             "Unknown command")
         return answer
 
@@ -369,6 +398,7 @@ class BoB:
         c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], c_channel_env,
                                 True, not self.verbose, uid=self.uid,
                                 username=self.username)
+        c_channel.spawn()
         self.processes[c_channel.pid] = c_channel
         self.log_started(c_channel.pid)
 
@@ -390,9 +420,15 @@ class BoB:
             Starts the configuration manager process
         """
         self.log_starting("b10-cfgmgr")
-        bind_cfgd = ProcessInfo("b10-cfgmgr", ["b10-cfgmgr"],
+        args = ["b10-cfgmgr"]
+        if self.data_path is not None:
+            args.append("--data-path=" + self.data_path)
+        if self.config_filename is not None:
+            args.append("--config-filename=" + self.config_filename)
+        bind_cfgd = ProcessInfo("b10-cfgmgr", args,
                                 c_channel_env, uid=self.uid,
                                 username=self.username)
+        bind_cfgd.spawn()
         self.processes[bind_cfgd.pid] = bind_cfgd
         self.log_started(bind_cfgd.pid)
 
@@ -427,6 +463,7 @@ class BoB:
         """
         self.log_starting(name, port, address)
         newproc = ProcessInfo(name, args, c_channel_env)
+        newproc.spawn()
         self.processes[newproc.pid] = newproc
         self.log_started(newproc.pid)
 
@@ -500,8 +537,13 @@ class BoB:
         self.start_simple("b10-stats", c_channel_env)
 
     def start_cmdctl(self, c_channel_env):
-        # XXX: we hardcode port 8080
-        self.start_simple("b10-cmdctl", c_channel_env, 8080)
+        """
+            Starts the command control process
+        """
+        args = ["b10-cmdctl"]
+        if self.cmdctl_port is not None:
+            args.append("--port=" + str(self.cmdctl_port))
+        self.start_process("b10-cmdctl", args, c_channel_env, self.cmdctl_port)
 
     def start_all_processes(self):
         """
@@ -785,6 +827,50 @@ def process_rename(option, opt_str, value, parser):
     """Function that renames the process if it is requested by a option."""
     isc.util.process.rename(value)
 
+def parse_args(args=sys.argv[1:], Parser=OptionParser):
+    """
+    Function for parsing command line arguments. Returns the
+    options object from OptionParser.
+    """
+    parser = Parser(version=VERSION)
+    parser.add_option("-m", "--msgq-socket-file", dest="msgq_socket_file",
+                      type="string", default=None,
+                      help="UNIX domain socket file the b10-msgq daemon will use")
+    parser.add_option("-n", "--no-cache", action="store_true", dest="nocache",
+                      default=False, help="disable hot-spot cache in authoritative DNS server")
+    parser.add_option("-u", "--user", dest="user", type="string", default=None,
+                      help="Change user after startup (must run as root)")
+    parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
+                      help="display more about what is going on")
+    parser.add_option("--pretty-name", type="string", action="callback",
+                      callback=process_rename,
+                      help="Set the process name (displayed in ps, top, ...)")
+    parser.add_option("-c", "--config-file", action="store",
+                      dest="config_file", default=None,
+                      help="Configuration database filename")
+    parser.add_option("-p", "--data-path", dest="data_path",
+                      help="Directory to search for configuration files",
+                      default=None)
+    parser.add_option("--cmdctl-port", dest="cmdctl_port", type="int",
+                      default=None, help="Port of command control")
+    parser.add_option("--pid-file", dest="pid_file", type="string",
+                      default=None,
+                      help="file to dump the PID of the BIND 10 process")
+
+    (options, args) = parser.parse_args(args)
+
+    if options.cmdctl_port is not None:
+        try:
+            isc.net.parse.port_parse(options.cmdctl_port)
+        except ValueError as e:
+            parser.error(e)
+
+    if args:
+        parser.print_help()
+        sys.exit(1)
+
+    return options
+
 def dump_pid(pid_file):
     """
     Dump the PID of the current process to the specified file.  If the given
@@ -814,33 +900,14 @@ def unlink_pid_file(pid_file):
         if error.errno is not errno.ENOENT:
             raise
 
+
 def main():
     global options
     global boss_of_bind
     # Enforce line buffering on stdout, even when not a TTY
     sys.stdout = io.TextIOWrapper(sys.stdout.detach(), line_buffering=True)
 
-    # Parse any command-line options.
-    parser = OptionParser(version=VERSION)
-    parser.add_option("-m", "--msgq-socket-file", dest="msgq_socket_file",
-                      type="string", default=None,
-                      help="UNIX domain socket file the b10-msgq daemon will use")
-    parser.add_option("-n", "--no-cache", action="store_true", dest="nocache",
-                      default=False, help="disable hot-spot cache in authoritative DNS server")
-    parser.add_option("-u", "--user", dest="user", type="string", default=None,
-                      help="Change user after startup (must run as root)")
-    parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
-                      help="display more about what is going on")
-    parser.add_option("--pretty-name", type="string", action="callback",
-                      callback=process_rename,
-                      help="Set the process name (displayed in ps, top, ...)")
-    parser.add_option("--pid-file", dest="pid_file", type="string",
-                      default=None,
-                      help="file to dump the PID of the BIND 10 process")
-    (options, args) = parser.parse_args()
-    if args:
-        parser.print_help()
-        sys.exit(1)
+    options = parse_args()
 
     # Check user ID.
     setuid = None
@@ -890,8 +957,9 @@ def main():
     signal.signal(signal.SIGPIPE, signal.SIG_IGN)
 
     # Go bob!
-    boss_of_bind = BoB(options.msgq_socket_file, options.nocache,
-                       options.verbose, setuid, username)
+    boss_of_bind = BoB(options.msgq_socket_file, options.data_path,
+                       options.config_file, options.nocache, options.verbose,
+                       setuid, username, options.cmdctl_port)
     startup_result = boss_of_bind.startup()
     if startup_result:
         sys.stderr.write("[bind10] Error on startup: %s\n" % startup_result)

src/bin/bind10/bind10.xml

@@ -48,6 +48,8 @@
       <arg><option>-n</option></arg>
       <arg><option>-u <replaceable>user</replaceable></option></arg>
       <arg><option>-v</option></arg>
+      <arg><option>-c<replaceable>config-filename</replaceable></option></arg>
+      <arg><option>-p<replaceable>data_path</replaceable></option></arg>
       <arg><option>--msgq-socket-file <replaceable>file</replaceable></option></arg>
       <arg><option>--no-cache</option></arg>
       <arg><option>--user <replaceable>user</replaceable></option></arg>
@@ -80,6 +82,31 @@
     <para>The arguments are as follows:</para>
 
     <variablelist>
+      <varlistentry>
+        <term>
+          <option>-c</option><replaceable>config-filename</replaceable>,
+          <option>--config-file</option> <replaceable>config-filename</replaceable>
+        </term>
+        <listitem>
+          <para>The configuration filename to use. Can be either absolute or
+          relative to data path. In case it is absolute, value of data path is
+          not considered.</para>
+          <para>Defaults to b10-config.db.</para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+          <option>-p</option><replaceable>data-path</replaceable>,
+          <option>--data-path</option> <replaceable>data-path</replaceable>
+        </term>
+        <listitem>
+          <para>The path where BIND 10 programs look for various data files.
+          Currently only b10-cfgmgr uses it to locate the configuration file,
+          but the usage might be extended for other programs and other types
+          of files.</para>
+        </listitem>
+      </varlistentry>
 
       <varlistentry>
         <term><option>-m</option> <replaceable>file</replaceable>,
@@ -145,6 +172,9 @@ The default is the basename of ARG 0.
   </refsect1>
 
 <!--
+TODO: configuration section
+-->
+<!--
   <refsect1>
     <title>FILES</title>
     <para><filename></filename>

src/bin/bind10/bob.spec

@@ -21,6 +21,16 @@
         "command_name": "shutdown",
         "command_description": "Shut down BIND 10",
         "command_args": []
+      },
+      {
+        "command_name": "ping",
+        "command_description": "Ping the boss process",
+        "command_args": []
+      },
+      {
+        "command_name": "show_processes",
+        "command_description": "List the running BIND 10 processes",
+        "command_args": []
       }
     ]
   }

src/bin/bind10/tests/Makefile.am

@@ -13,5 +13,6 @@ endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/bind10 \
-	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
+	BIND10_MSGQ_SOCKET_FILE=$(abs_top_builddir)/msgq_socket \
+		$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/bind10/tests/bind10_test.py.in

@@ -1,4 +1,19 @@
-from bind10 import ProcessInfo, BoB, dump_pid, unlink_pid_file
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+from bind10 import ProcessInfo, BoB, parse_args, dump_pid, unlink_pid_file
 
 # XXX: environment tests are currently disabled, due to the preprocessor
 #      setup that we have now complicating the environment
@@ -9,6 +24,7 @@ import os
 import signal
 import socket
 from isc.net.addr import IPAddr
+from isc.testutils.parse_args import TestOptParser, OptsError
 
 class TestProcessInfo(unittest.TestCase):
     def setUp(self):
@@ -30,6 +46,7 @@ class TestProcessInfo(unittest.TestCase):
 
     def test_init(self):
         pi = ProcessInfo('Test Process', [ '/bin/echo', 'foo' ])
+        pi.spawn()
         os.dup2(self.old_stdout, sys.stdout.fileno())
         self.assertEqual(pi.name, 'Test Process')
         self.assertEqual(pi.args, [ '/bin/echo', 'foo' ])
@@ -50,12 +67,14 @@ class TestProcessInfo(unittest.TestCase):
     def test_setting_null_stdout(self):
         pi = ProcessInfo('Test Process', [ '/bin/echo', 'foo' ],
                          dev_null_stdout=True)
+        pi.spawn()
         os.dup2(self.old_stdout, sys.stdout.fileno())
         self.assertEqual(pi.dev_null_stdout, True)
         self.assertEqual(os.read(self.pipes[0], 100), b"")
 
     def test_respawn(self):
         pi = ProcessInfo('Test Process', [ '/bin/echo', 'foo' ])
+        pi.spawn()
         # wait for old process to work...
         self.assertEqual(os.read(self.pipes[0], 100), b"foo\n")
         # respawn it
@@ -104,17 +123,19 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.cfg_start_auth, True)
         self.assertEqual(bob.cfg_start_resolver, False)
 
-# Class for testing the BoB start/stop components routines.
+# Class for testing the BoB without actually starting processes.
+# This is used for testing the start/stop components routines and
+# the BoB commands.
 #
-# Although testing that external processes start is outside the scope
+# Testing that external processes start is outside the scope
 # of the unit test, by overriding the process start methods we can check
 # that the right processes are started depending on the configuration
 # options.
-class StartStopCheckBob(BoB):
+class MockBob(BoB):
     def __init__(self):
         BoB.__init__(self)
 
-# Set flags as to which of the overridden methods has been run.
+        # Set flags as to which of the overridden methods has been run.
         self.msgq = False
         self.cfgmgr = False
         self.ccsession = False
@@ -126,6 +147,7 @@ class StartStopCheckBob(BoB):
         self.stats = False
         self.cmdctl = False
         self.c_channel_env = {}
+        self.processes = { }
 
     def read_bind10_config(self):
         # Configuration options are set directly
@@ -133,65 +155,95 @@ class StartStopCheckBob(BoB):
 
     def start_msgq(self, c_channel_env):
         self.msgq = True
+        self.processes[2] = ProcessInfo('b10-msgq', ['/bin/false'])
 
     def start_cfgmgr(self, c_channel_env):
         self.cfgmgr = True
+        self.processes[3] = ProcessInfo('b10-cfgmgr', ['/bin/false'])
 
     def start_ccsession(self, c_channel_env):
         self.ccsession = True
+        self.processes[4] = ProcessInfo('b10-ccsession', ['/bin/false'])
 
     def start_auth(self, c_channel_env):
         self.auth = True
+        self.processes[5] = ProcessInfo('b10-auth', ['/bin/false'])
 
     def start_resolver(self, c_channel_env):
         self.resolver = True
+        self.processes[6] = ProcessInfo('b10-resolver', ['/bin/false'])
 
     def start_xfrout(self, c_channel_env):
         self.xfrout = True
+        self.processes[7] = ProcessInfo('b10-xfrout', ['/bin/false'])
 
     def start_xfrin(self, c_channel_env):
         self.xfrin = True
+        self.processes[8] = ProcessInfo('b10-xfrin', ['/bin/false'])
 
     def start_zonemgr(self, c_channel_env):
         self.zonemgr = True
+        self.processes[9] = ProcessInfo('b10-zonemgr', ['/bin/false'])
 
     def start_stats(self, c_channel_env):
         self.stats = True
+        self.processes[10] = ProcessInfo('b10-stats', ['/bin/false'])
 
     def start_cmdctl(self, c_channel_env):
         self.cmdctl = True
+        self.processes[11] = ProcessInfo('b10-cmdctl', ['/bin/false'])
 
     # We don't really use all of these stop_ methods. But it might turn out
     # someone would add some stop_ method to BoB and we want that one overriden
     # in case he forgets to update the tests.
     def stop_msgq(self):
+        if self.msgq:
+            del self.processes[2]
         self.msgq = False
 
     def stop_cfgmgr(self):
+        if self.cfgmgr:
+            del self.processes[3]
         self.cfgmgr = False
 
     def stop_ccsession(self):
+        if self.ccssession:
+            del self.processes[4]
         self.ccsession = False
 
     def stop_auth(self):
+        if self.auth:
+            del self.processes[5]
         self.auth = False
 
     def stop_resolver(self):
+        if self.resolver:
+            del self.processes[6]
         self.resolver = False
 
     def stop_xfrout(self):
+        if self.xfrout:
+            del self.processes[7]
         self.xfrout = False
 
     def stop_xfrin(self):
+        if self.xfrin:
+            del self.processes[8]
         self.xfrin = False
 
     def stop_zonemgr(self):
+        if self.zonemgr:
+            del self.processes[9]
         self.zonemgr = False
 
     def stop_stats(self):
+        if self.stats:
+            del self.processes[10]
         self.stats = False
 
     def stop_cmdctl(self):
+        if self.cmdctl:
+            del self.processes[11]
         self.cmdctl = False
 
 class TestStartStopProcessesBob(unittest.TestCase):
@@ -251,7 +303,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
     # is specified.
     def test_start_none(self):
         # Create BoB and ensure correct initialization
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         # Start processes and check what was started
@@ -264,7 +316,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
     # Checks the processes started when starting only the auth process
     def test_start_auth(self):
         # Create BoB and ensure correct initialization
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         # Start processes and check what was started
@@ -278,7 +330,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
     # Checks the processes started when starting only the resolver process
     def test_start_resolver(self):
         # Create BoB and ensure correct initialization
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         # Start processes and check what was started
@@ -292,7 +344,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
     # Checks the processes started when starting both auth and resolver process
     def test_start_both(self):
         # Create BoB and ensure correct initialization
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         # Start processes and check what was started
@@ -310,7 +362,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         """
 
         # Create BoB and ensure correct initialization
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         # Start processes (nothing much should be started, as in
@@ -375,7 +427,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         Tests that a process is started only once.
         """
         # Create BoB and ensure correct initialization
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         # Start processes (both)
@@ -401,7 +453,7 @@ class TestStartStopProcessesBob(unittest.TestCase):
         Test that processes are not started by the config handler before
         startup.
         """
-        bob = StartStopCheckBob()
+        bob = MockBob()
         self.check_preconditions(bob)
 
         bob.start_auth = lambda: self.fail("Started auth again")
@@ -412,6 +464,92 @@ class TestStartStopProcessesBob(unittest.TestCase):
 
         bob.config_handler({'start_auth': True, 'start_resolver': True})
 
+class TestBossCmd(unittest.TestCase):
+    def test_ping(self):
+        """
+        Confirm simple ping command works.
+        """
+        bob = MockBob()
+        answer = bob.command_handler("ping", None)
+        self.assertEqual(answer, {'result': [0, 'pong']})
+
+    def test_show_processes(self):
+        """
+        Confirm getting a list of processes works.
+        """
+        bob = MockBob()
+        answer = bob.command_handler("show_processes", None)
+        self.assertEqual(answer, {'result': [0, []]})
+
+    def test_show_processes_started(self):
+        """
+        Confirm getting a list of processes works.
+        """
+        bob = MockBob()
+        bob.start_all_processes()
+        answer = bob.command_handler("show_processes", None)
+        processes = [[2, 'b10-msgq'],
+                     [3, 'b10-cfgmgr'], 
+                     [4, 'b10-ccsession'],
+                     [5, 'b10-auth'],
+                     [7, 'b10-xfrout'],
+                     [8, 'b10-xfrin'], 
+                     [9, 'b10-zonemgr'],
+                     [10, 'b10-stats'], 
+                     [11, 'b10-cmdctl']]
+        self.assertEqual(answer, {'result': [0, processes]})
+
+class TestParseArgs(unittest.TestCase):
+    """
+    This tests parsing of arguments of the bind10 master process.
+    """
+    #TODO: Write tests for the original parsing, bad options, etc.
+    def test_no_opts(self):
+        """
+        Test correct default values when no options are passed.
+        """
+        options = parse_args([], TestOptParser)
+        self.assertEqual(None, options.data_path)
+        self.assertEqual(None, options.config_file)
+        self.assertEqual(None, options.cmdctl_port)
+
+    def test_data_path(self):
+        """
+        Test it can parse the data path.
+        """
+        self.assertRaises(OptsError, parse_args, ['-p'], TestOptParser)
+        self.assertRaises(OptsError, parse_args, ['--data-path'],
+                          TestOptParser)
+        options = parse_args(['-p', '/data/path'], TestOptParser)
+        self.assertEqual('/data/path', options.data_path)
+        options = parse_args(['--data-path=/data/path'], TestOptParser)
+        self.assertEqual('/data/path', options.data_path)
+
+    def test_config_filename(self):
+        """
+        Test it can parse the config switch.
+        """
+        self.assertRaises(OptsError, parse_args, ['-c'], TestOptParser)
+        self.assertRaises(OptsError, parse_args, ['--config-file'],
+                          TestOptParser)
+        options = parse_args(['-c', 'config-file'], TestOptParser)
+        self.assertEqual('config-file', options.config_file)
+        options = parse_args(['--config-file=config-file'], TestOptParser)
+        self.assertEqual('config-file', options.config_file)
+
+    def test_cmdctl_port(self):
+        """
+        Test it can parse the command control port.
+        """
+        self.assertRaises(OptsError, parse_args, ['--cmdctl-port=abc'],
+                                                TestOptParser)
+        self.assertRaises(OptsError, parse_args, ['--cmdctl-port=100000000'],
+                                                TestOptParser)
+        self.assertRaises(OptsError, parse_args, ['--cmdctl-port'],
+                          TestOptParser)
+        options = parse_args(['--cmdctl-port=1234'], TestOptParser)
+        self.assertEqual(1234, options.cmdctl_port)
+
 class TestPIDFile(unittest.TestCase):
     def setUp(self):
         self.pid_file = '@builddir@' + os.sep + 'bind10.pid'

src/bin/bindctl/bindcmd.py

@@ -123,14 +123,19 @@ class BindCmdInterpreter(Cmd):
         '''Parse commands from user and send them to cmdctl. '''
         try:
             if not self.login_to_cmdctl():
-                return 
+                return
 
             self.cmdloop()
+            print('\nExit from bindctl')
         except FailToLogin as err:
             # error already printed when this was raised, ignoring
             pass
         except KeyboardInterrupt:
             print('\nExit from bindctl')
+        except socket.error as err:
+            print('Failed to send request, the connection is closed')
+        except http.client.CannotSendRequest:
+            print('Can not send request, the connection is busy')
 
     def _get_saved_user_info(self, dir, file_name):
         ''' Read all the available username and password pairs saved in 
@@ -192,8 +197,10 @@ class BindCmdInterpreter(Cmd):
                 raise FailToLogin()
 
             if response.status == http.client.OK:
-                print(data + ' login as ' + row[0] )
-                return True 
+                # Is interactive?
+                if sys.stdin.isatty():
+                    print(data + ' login as ' + row[0])
+                return True
 
         count = 0
         print("[TEMP MESSAGE]: username :root  password :bind10")
@@ -273,8 +280,9 @@ class BindCmdInterpreter(Cmd):
         self._update_commands()
 
     def precmd(self, line):
-        self._update_all_modules_info()
-        return line 
+        if line != 'EOF':
+            self._update_all_modules_info()
+        return line
 
     def postcmd(self, stop, line):
         '''Update the prompt after every command, but only if we

src/bin/bindctl/bindctl.1

@@ -22,7 +22,7 @@
 bindctl \- control and configure BIND 10
 .SH "SYNOPSIS"
 .HP \w'\fBbindctl\fR\ 'u
-\fBbindctl\fR [\fB\-a\ \fR\fB\fIaddress\fR\fR] [\fB\-h\fR] [\fB\-c\ \fR\fB\fIfile\fR\fR] [\fB\-p\ \fR\fB\fInumber\fR\fR] [\fB\-\-address\ \fR\fB\fIaddress\fR\fR] [\fB\-\-help\fR] [\fB\-\-certificate\-chain\ \fR\fB\fIfile\fR\fR] [\fB\-\-port\ \fR\fB\fInumber\fR\fR] [\fB\-\-version\fR]
+\fBbindctl\fR [\fB\-a\ \fR\fB\fIaddress\fR\fR] [\fB\-h\fR] [\fB\-c\ \fR\fB\fIfile\fR\fR] [\fB\-p\ \fR\fB\fInumber\fR\fR] [\fB\-\-address\ \fR\fB\fIaddress\fR\fR] [\fB\-\-help\fR] [\fB\-\-certificate\-chain\ \fR\fB\fIfile\fR\fR] [\fB\-\-csv\-file\-dir\fR\fB\fIfile\fR\fR] [\fB\-\-port\ \fR\fB\fInumber\fR\fR] [\fB\-\-version\fR]
 .SH "DESCRIPTION"
 .PP
 The
@@ -52,6 +52,11 @@ daemon\&. The default is 127\&.0\&.0\&.1\&.
 The PEM formatted server certificate validation chain file\&.
 .RE
 .PP
+\fB\-\-csv\-file\-dir\fR\fIfile\fR
+.RS 4
+The directory name in which the user/password CSV file is stored (see AUTHENTICATION)\&. By default this option doesn\'t have any value, in which case the "\&.bind10" directory under the user\'s home directory will be used\&.
+.RE
+.PP
 \fB\-h\fR, \fB\-\-help\fR
 .RS 4
 Display command usage\&.
@@ -85,10 +90,10 @@ Display the version number and exit\&.
 .RE
 .SH "AUTHENTICATION"
 .PP
-The tool will authenticate using a username and password\&. On the first successful login, it will save the details to
-~/\&.bind10/default_user\&.csv
-which will be used for later uses of
-\fBbindctl\fR\&.
+The tool will authenticate using a username and password\&. On the first successful login, it will save the details to a comma\-separated\-value (CSV) file which will be used for later uses of
+\fBbindctl\fR\&. The file name is
+default_user\&.csv
+located under the directory specified by the \-\-csv\-file\-dir option\&.
 .SH "USAGE"
 .PP
 The

src/bin/bindctl/tests/bindctl_test.py

@@ -17,11 +17,16 @@
 import unittest
 import isc.cc.data
 import os
+import io
+import sys
+import socket
+import http.client
 import pwd
 import getpass
 from optparse import OptionParser
 from isc.config.config_data import ConfigData, MultiConfigData
 from isc.config.module_spec import ModuleSpec
+from isc.testutils.parse_args import TestOptParser, OptsError
 from bindctl_main import set_bindctl_options
 from bindctl import cmdparse
 from bindctl import bindcmd
@@ -275,7 +280,33 @@ class FakeCCSession(MultiConfigData):
                  ]
                }
         self.set_specification(ModuleSpec(spec))
-    
+
+
+# fake socket
+class FakeSocket():
+    def __init__(self):
+        self.run = True
+
+    def connect(self, to):
+        if not self.run:
+            raise socket.error
+
+    def close(self):
+        self.run = False
+
+    def send(self, data):
+        if not self.run:
+            raise socket.error
+        return len(data)
+
+    def makefile(self, type):
+        return self
+
+    def sendall(self, data):
+        if not self.run:
+            raise socket.error
+        return len(data)
+
 
 class TestConfigCommands(unittest.TestCase):
     def setUp(self):
@@ -283,7 +314,47 @@ class TestConfigCommands(unittest.TestCase):
         mod_info = ModuleInfo(name = "foo")
         self.tool.add_module_info(mod_info)
         self.tool.config_data = FakeCCSession()
-        
+        self.stdout_backup = sys.stdout
+
+    def test_precmd(self):
+        def update_all_modules_info():
+            raise socket.error
+        def precmd(line):
+            self.tool.precmd(line)
+        self.tool._update_all_modules_info = update_all_modules_info
+        # If line is equals to 'EOF', _update_all_modules_info() shouldn't be called
+        precmd('EOF')
+        self.assertRaises(socket.error, precmd, 'continue')
+
+    def test_run(self):
+        def login_to_cmdctl():
+            return True
+        def cmd_loop():
+            self.tool._send_message("/module_spec", None)
+
+        self.tool.login_to_cmdctl = login_to_cmdctl
+        # rewrite cmdloop() to avoid interactive mode
+        self.tool.cmdloop = cmd_loop
+
+        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.assertRaises(None, self.tool.run())
+        self.assertEqual("Failed to send request, the connection is closed\n",
+                         socket_err_output.getvalue())
+        socket_err_output.close()
+
+        # validate log message for http.client.CannotSendRequest
+        cannot_send_output = io.StringIO()
+        sys.stdout = cannot_send_output
+        self.assertRaises(None, self.tool.run())
+        self.assertEqual("Can not send request, the connection is busy\n",
+                         cannot_send_output.getvalue())
+        cannot_send_output.close()
+
     def test_apply_cfg_command_int(self):
         self.tool.location = '/'
 
@@ -332,10 +403,17 @@ class TestConfigCommands(unittest.TestCase):
         # this should raise a TypeError
         cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=\"a\"")
         self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
-        
+
         cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=[1]")
         self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
 
+    def tearDown(self):
+        sys.stdout = self.stdout_backup
+
+class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
+    def __init__(self):
+        pass
+
 class TestBindCmdInterpreter(unittest.TestCase):
 
     def _create_invalid_csv_file(self, csvfilename):
@@ -360,35 +438,23 @@ class TestBindCmdInterpreter(unittest.TestCase):
         self.assertEqual(new_csv_dir, custom_cmd.csv_file_dir)
 
     def test_get_saved_user_info(self):
+        old_stdout = sys.stdout
+        sys.stdout = open(os.devnull, 'w')
         cmd = bindcmd.BindCmdInterpreter()
         users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
         self.assertEqual([], users)
-        
+
         csvfilename = 'csv_file.csv'
         self._create_invalid_csv_file(csvfilename)
         users = cmd._get_saved_user_info('./', csvfilename)
         self.assertEqual([], users)
         os.remove(csvfilename)
+        sys.stdout = old_stdout
 
 
 class TestCommandLineOptions(unittest.TestCase):
-    class FakeParserError(Exception):
-        """An exception thrown from FakeOptionParser on parser error.
-        """
-        pass
-
-    class FakeOptionParser(OptionParser):
-        """This fake class emulates the OptionParser class with customized
-        error handling for the convenient of tests.
-        """
-        def __init__(self):
-            OptionParser.__init__(self)
-
-        def error(self, msg):
-            raise TestCommandLineOptions.FakeParserError
-
     def setUp(self):
-        self.parser = self.FakeOptionParser()
+        self.parser = TestOptParser()
         set_bindctl_options(self.parser)
 
     def test_csv_file_dir(self):
@@ -401,7 +467,7 @@ class TestCommandLineOptions(unittest.TestCase):
         self.assertEqual('some_dir', options.csv_file_dir)
 
         # missing option arg; should trigger parser error.
-        self.assertRaises(self.FakeParserError, self.parser.parse_args,
+        self.assertRaises(OptsError, self.parser.parse_args,
                           ['--csv-file-dir'])
 
 if __name__== "__main__":

src/bin/cfgmgr/b10-cfgmgr.py.in

@@ -22,6 +22,7 @@ from isc.cc import SessionError
 import isc.util.process
 import signal
 import os
+from optparse import OptionParser
 
 isc.util.process.rename()
 
@@ -41,18 +42,34 @@ if "B10_FROM_SOURCE" in os.environ:
 else:
     PREFIX = "@prefix@"
     DATA_PATH = "@localstatedir@/@PACKAGE@".replace("${prefix}", PREFIX)
+DEFAULT_CONFIG_FILE = "b10-config.db"
 
 cm = None
 
+def parse_options(args=sys.argv[1:], Parser=OptionParser):
+    parser = Parser()
+    parser.add_option("-p", "--data-path", dest="data_path",
+                      help="Directory to search for configuration files " +
+                      "(default=" + DATA_PATH + ")", default=DATA_PATH)
+    parser.add_option("-c", "--config-filename", dest="config_file",
+                      help="Configuration database filename " +
+                      "(default=" + DEFAULT_CONFIG_FILE + ")",
+                      default=DEFAULT_CONFIG_FILE)
+    (options, args) = parser.parse_args(args)
+    if args:
+        parser.error("No non-option arguments allowed")
+    return options
+
 def signal_handler(signal, frame):
     global cm
     if cm:
         cm.running = False
 
 def main():
+    options = parse_options()
     global cm
     try:
-        cm = ConfigManager(DATA_PATH)
+        cm = ConfigManager(options.data_path, options.config_file)
         signal.signal(signal.SIGINT, signal_handler)
         signal.signal(signal.SIGTERM, signal_handler)
         cm.read_config()

src/bin/cfgmgr/b10-cfgmgr.xml

@@ -41,16 +41,13 @@
     </copyright>
   </docinfo>
 
-<!--
   <refsynopsisdiv>
     <cmdsynopsis>
-      <command></command>
-      <arg><option></option></arg>
-      <arg choice="opt"></arg>
-      <arg choice="opt"></arg>
+      <command>b10-cfgmgr</command>
+      <arg><option>-c<replaceable>config-filename</replaceable></option></arg>
+      <arg><option>-p<replaceable>data_path</replaceable></option></arg>
     </cmdsynopsis>
   </refsynopsisdiv>
--->
 
   <refsect1>
     <title>DESCRIPTION</title>
@@ -93,24 +90,38 @@
     </para>
   </refsect1>
 
-<!--
   <refsect1>
     <title>ARGUMENTS</title>
-    <para>
-      <orderedlist numeration="loweralpha">
+
+    <para>The arguments are as follows:</para>
+
+    <variablelist>
+      <varlistentry>
+        <term>
+          <option>-c</option><replaceable>config-filename</replaceable>,
+          <option>--config-filename</option> <replaceable>config-filename</replaceable>
+        </term>
         <listitem>
-          <para>
-          </para>
+          <para>The configuration database filename to use. Can be either
+          absolute or relative to data path.</para>
+          <para>Defaults to b10-config.db</para>
         </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+          <option>-p</option><replaceable>data-path</replaceable>,
+          <option>--data-path</option> <replaceable>data-path</replaceable>
+        </term>
         <listitem>
-          <para>
-          </para>
+          <para>The path where BIND 10 looks for files. The
+          configuration file is looked for here, if it is relative. If it is
+          absolute, the path is ignored.</para>
         </listitem>
-      </orderedlist>
-    </para>
-
+      </varlistentry>
+    </variablelist>
   </refsect1>
--->
+
   <refsect1>
     <title>FILES</title>
 <!-- TODO: fix path -->

src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in

@@ -20,9 +20,10 @@
 import unittest
 import os
 import sys
+from isc.testutils.parse_args import OptsError, TestOptParser
 
 class MyConfigManager:
-    def __init__(self, path):
+    def __init__(self, path, filename):
         self._path = path
         self.read_config_called = False
         self.notify_boss_called = False
@@ -88,6 +89,69 @@ class TestConfigManagerStartup(unittest.TestCase):
 
         sys.modules.pop("b10-cfgmgr")
 
+class TestParseArgs(unittest.TestCase):
+    """
+    Test for the parsing of command line arguments. We provide a different
+    array to parse instead.
+    """
+
+    def test_defaults(self):
+        """
+        Test the default values when no options are provided.
+        """
+        # Pass it empty array, not our arguments
+        b = __import__("b10-cfgmgr")
+        parsed = b.parse_options([], TestOptParser)
+        self.assertEqual(b.DATA_PATH, parsed.data_path)
+        self.assertEqual(b.DEFAULT_CONFIG_FILE, parsed.config_file)
+
+    def test_wrong_args(self):
+        """
+        Test it fails when we pass invalid option.
+        """
+        b = __import__("b10-cfgmgr")
+        self.assertRaises(OptsError, b.parse_options, ['--wrong-option'],
+                          TestOptParser)
+
+    def test_not_arg(self):
+        """
+        Test it fails when there's an argument that's not option
+        (eg. without -- at the beginning).
+        """
+        b = __import__("b10-cfgmgr")
+        self.assertRaises(OptsError, b.parse_options, ['not-option'],
+                          TestOptParser)
+
+    def test_datapath(self):
+        """
+        Test overwriting the data path.
+        """
+        b = __import__("b10-cfgmgr")
+        parsed = b.parse_options(['--data-path=/path'], TestOptParser)
+        self.assertEqual('/path', parsed.data_path)
+        self.assertEqual(b.DEFAULT_CONFIG_FILE, parsed.config_file)
+        parsed = b.parse_options(['-p', '/path'], TestOptParser)
+        self.assertEqual('/path', parsed.data_path)
+        self.assertEqual(b.DEFAULT_CONFIG_FILE, parsed.config_file)
+        self.assertRaises(OptsError, b.parse_options, ['-p'], TestOptParser)
+        self.assertRaises(OptsError, b.parse_options, ['--data-path'],
+                          TestOptParser)
+
+    def test_db_filename(self):
+        """
+        Test setting the configuration database file.
+        """
+        b = __import__("b10-cfgmgr")
+        parsed = b.parse_options(['--config-filename=filename'],
+                                 TestOptParser)
+        self.assertEqual(b.DATA_PATH, parsed.data_path)
+        self.assertEqual("filename", parsed.config_file)
+        parsed = b.parse_options(['-c', 'filename'], TestOptParser)
+        self.assertEqual(b.DATA_PATH, parsed.data_path)
+        self.assertEqual("filename", parsed.config_file)
+        self.assertRaises(OptsError, b.parse_options, ['-c'], TestOptParser)
+        self.assertRaises(OptsError, b.parse_options, ['--config-filename'],
+                          TestOptParser)
 
 if __name__ == '__main__':
     unittest.main()

src/bin/resolver/main.cc

@@ -30,6 +30,7 @@
 #include <exceptions/exceptions.h>
 
 #include <dns/buffer.h>
+#include <dns/rcode.h>
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
 
@@ -180,6 +181,7 @@ main(int argc, char* argv[]) {
                                                              isc::dns::RRClass::IN(),
                                                              "2001:500:3::42"));
         isc::dns::MessagePtr priming_result(new isc::dns::Message(isc::dns::Message::RENDER));
+        priming_result->setRcode(isc::dns::Rcode::NOERROR());
         priming_result->addQuestion(root_question);
         priming_result->addRRset(isc::dns::Message::SECTION_ANSWER, root_ns_rrset);
         priming_result->addRRset(isc::dns::Message::SECTION_ADDITIONAL, root_a_rrset);

src/bin/resolver/response_scrubber.h

@@ -177,7 +177,7 @@
 /// Qu: www.sub.example.com\n
 /// Zo: example.com
 ///
-/// An: <nothing>
+/// An: (nothing)
 ///
 /// Au(1): sub.example.com NS ns0.sub.example.com\n
 /// Au(2): sub.example.com NS ns1.example.net
@@ -312,7 +312,7 @@ public:
     /// QNAME is equal to or in the supplied relationship with the given name.
     ///
     /// \param section Section of the message to be scrubbed.
-    /// \param zone Names against which RRsets should be checked.  Note that
+    /// \param names Names against which RRsets should be checked.  Note that
     /// this is a vector of pointers to Name objects; they are assumed to
     /// independently exist, and the caller retains ownership of them and is
     /// assumed to destroy them when needed.

src/lib/asiolink/dns_lookup.h

@@ -63,8 +63,10 @@ public:
     ///
     /// \param io_message The event message to handle
     /// \param message The DNS MessagePtr that needs handling
+    /// \param answer_message The final answer will be constructed in
+    ///                       this MessagePtr
     /// \param buffer The final answer is put here
-    /// \param DNSServer DNSServer object to use
+    /// \param server DNSServer object to use
     virtual void operator()(const IOMessage& io_message,
                             isc::dns::MessagePtr message,
                             isc::dns::MessagePtr answer_message,

src/lib/asiolink/dns_service.h

@@ -66,8 +66,8 @@ public:
     ///
     /// \param io_service The IOService to work with
     /// \param port the port to listen on
-    /// \param ipv4 If true, listen on ipv4 'any'
-    /// \param ipv6 If true, listen on ipv6 'any'
+    /// \param use_ipv4 If true, listen on ipv4 'any'
+    /// \param use_ipv6 If true, listen on ipv6 'any'
     /// \param checkin Provider for cc-channel events (see \c SimpleCallback)
     /// \param lookup The lookup provider (see \c DNSLookup)
     /// \param answer The answer provider (see \c DNSAnswer)

src/lib/asiolink/io_address.h

@@ -61,7 +61,7 @@ public:
     /// This constructor never throws an exception.
     ///
     /// \param asio_address The ASIO \c ip::address to be converted.
-    IOAddress(const asio::ip::address& asio_adress);
+    IOAddress(const asio::ip::address& asio_address);
     //@}
 
     /// \brief Convert the address to a string.

src/lib/asiolink/io_endpoint.cc

@@ -44,4 +44,17 @@ IOEndpoint::create(const int protocol, const IOAddress& address,
               protocol);
 }
 
+bool
+IOEndpoint::operator==(const IOEndpoint& other) const {
+    return (getProtocol() == other.getProtocol() &&
+            getPort() == other.getPort() &&
+            getFamily() == other.getFamily() &&
+            getAddress() == other.getAddress());
+}
+
+bool
+IOEndpoint::operator!=(const IOEndpoint& other) const {
+    return (!operator==(other));
+}
+
 }

src/lib/asiolink/io_endpoint.h

@@ -89,6 +89,9 @@ public:
     /// \brief Returns the address family of the endpoint.
     virtual short getFamily() const = 0;
 
+    bool operator==(const IOEndpoint& other) const;
+    bool operator!=(const IOEndpoint& other) const;
+
     /// \brief A polymorphic factory of endpoint from address and port.
     ///
     /// This method creates a new instance of (a derived class of)

src/lib/asiolink/io_fetch.cc

@@ -43,6 +43,9 @@
 #include <asiolink/tcp_socket.h>
 #include <asiolink/udp_endpoint.h>
 #include <asiolink/udp_socket.h>
+#include <asiolink/qid_gen.h>
+
+#include <stdint.h>
 
 using namespace asio;
 using namespace isc::dns;
@@ -69,19 +72,20 @@ struct IOFetchData {
     // which is not known until construction of the IOFetch.  Use of a shared
     // pointer here is merely to ensure deletion when the data object is deleted.
     boost::scoped_ptr<IOAsioSocket<IOFetch> > socket;
-                                            ///< Socket to use for I/O
-    boost::scoped_ptr<IOEndpoint> remote;   ///< Where the fetch was sent
-    isc::dns::Question          question;   ///< Question to be asked
-    isc::dns::OutputBufferPtr   msgbuf;     ///< Wire buffer for question
-    isc::dns::OutputBufferPtr   received;   ///< Received data put here
-    IOFetch::Callback*          callback;   ///< Called on I/O Completion
-    asio::deadline_timer        timer;      ///< Timer to measure timeouts
-    IOFetch::Protocol           protocol;   ///< Protocol being used
-    size_t                      cumulative; ///< Cumulative received amount
-    size_t                      expected;   ///< Expected amount of data
-    size_t                      offset;     ///< Offset to receive data
-    bool                        stopped;    ///< Have we stopped running?
-    int                         timeout;    ///< Timeout in ms
+                                             ///< Socket to use for I/O
+    boost::scoped_ptr<IOEndpoint> remote_snd;///< Where the fetch is sent
+    boost::scoped_ptr<IOEndpoint> remote_rcv;///< Where the response came from
+    isc::dns::Question          question;    ///< Question to be asked
+    isc::dns::OutputBufferPtr   msgbuf;      ///< Wire buffer for question
+    isc::dns::OutputBufferPtr   received;    ///< Received data put here
+    IOFetch::Callback*          callback;    ///< Called on I/O Completion
+    asio::deadline_timer        timer;       ///< Timer to measure timeouts
+    IOFetch::Protocol           protocol;    ///< Protocol being used
+    size_t                      cumulative;  ///< Cumulative received amount
+    size_t                      expected;    ///< Expected amount of data
+    size_t                      offset;      ///< Offset to receive data
+    bool                        stopped;     ///< Have we stopped running?
+    int                         timeout;     ///< Timeout in ms
 
     // In case we need to log an error, the origin of the last asynchronous
     // I/O is recorded.  To save time and simplify the code, this is recorded
@@ -91,6 +95,7 @@ struct IOFetchData {
     isc::log::MessageID         origin;     ///< Origin of last asynchronous I/O
     uint8_t                     staging[IOFetch::STAGING_LENGTH];
                                             ///< Temporary array for received data
+    isc::dns::qid_t             qid;         ///< The QID set in the query
 
     /// \brief Constructor
     ///
@@ -121,7 +126,11 @@ struct IOFetchData {
             static_cast<IOAsioSocket<IOFetch>*>(
                 new TCPSocket<IOFetch>(service))
             ),
-        remote((proto == IOFetch::UDP) ?
+        remote_snd((proto == IOFetch::UDP) ?
+            static_cast<IOEndpoint*>(new UDPEndpoint(address, port)) :
+            static_cast<IOEndpoint*>(new TCPEndpoint(address, port))
+            ),
+        remote_rcv((proto == IOFetch::UDP) ?
             static_cast<IOEndpoint*>(new UDPEndpoint(address, port)) :
             static_cast<IOEndpoint*>(new TCPEndpoint(address, port))
             ),
@@ -138,8 +147,21 @@ struct IOFetchData {
         stopped(false),
         timeout(wait),
         origin(ASIO_UNKORIGIN),
-        staging()
+        staging(),
+        qid(QidGenerator::getInstance().generateQid())
     {}
+
+    // Checks if the response we received was ok;
+    // - data contains the buffer we read, as well as the address
+    // we sent to and the address we received from.
+    // length is provided by the operator() in IOFetch.
+    // Addresses must match, number of octets read must be at least
+    // 2, and the first two octets must match the qid of the message
+    // we sent.
+    bool responseOK() {
+        return (*remote_snd == *remote_rcv && cumulative >= 2 &&
+                readUint16(received->getData()) == qid);
+    }
 };
 
 /// IOFetch Constructor - just initialize the private data
@@ -180,7 +202,7 @@ IOFetch::operator()(asio::error_code ec, size_t length) {
         /// declarations.
         {
             Message msg(Message::RENDER);
-            msg.setQid(QidGenerator::getInstance().generateQid());
+            msg.setQid(data_->qid);
             msg.setOpcode(Opcode::QUERY());
             msg.setRcode(Rcode::NOERROR());
             msg.setHeaderFlag(Message::HEADERFLAG_RD);
@@ -202,47 +224,50 @@ IOFetch::operator()(asio::error_code ec, size_t length) {
         // is synchronous (i.e. UDP operation) we bypass the yield.
         data_->origin = ASIO_OPENSOCK;
         if (data_->socket->isOpenSynchronous()) {
-            data_->socket->open(data_->remote.get(), *this);
+            data_->socket->open(data_->remote_snd.get(), *this);
         } else {
-            CORO_YIELD data_->socket->open(data_->remote.get(), *this);
+            CORO_YIELD data_->socket->open(data_->remote_snd.get(), *this);
         }
 
-        // Begin an asynchronous send, and then yield.  When the send completes,
-        // we will resume immediately after this point.
-        data_->origin = ASIO_SENDSOCK;
-        CORO_YIELD data_->socket->asyncSend(data_->msgbuf->getData(),
-            data_->msgbuf->getLength(), data_->remote.get(), *this);
-
-        // Now receive the response.  Since TCP may not receive the entire
-        // message in one operation, we need to loop until we have received
-        // it. (This can't be done within the asyncReceive() method because
-        // each I/O operation will be done asynchronously and between each one
-        // we need to yield ... and we *really* don't want to set up another
-        // coroutine within that method.)  So after each receive (and yield),
-        // we check if the operation is complete and if not, loop to read again.
-        //
-        // Another concession to TCP is that the amount of is contained in the
-        // first two bytes.  This leads to two problems:
-        //
-        // a) We don't want those bytes in the return buffer.
-        // b) They may not both arrive in the first I/O.
-        //
-        // So... we need to loop until we have at least two bytes, then store
-        // the expected amount of data.  Then we need to loop until we have
-        // received all the data before copying it back to the user's buffer.
-        // And we want to minimise the amount of copying...
-
-        data_->origin = ASIO_RECVSOCK;
-        data_->cumulative = 0;          // No data yet received
-        data_->offset = 0;              // First data into start of buffer
         do {
-            CORO_YIELD data_->socket->asyncReceive(data_->staging,
-                                                   static_cast<size_t>(STAGING_LENGTH),
-                                                   data_->offset,
-                                                   data_->remote.get(), *this);
-        } while (!data_->socket->processReceivedData(data_->staging, length,
-                                                     data_->cumulative, data_->offset,
-                                                     data_->expected, data_->received));
+            // Begin an asynchronous send, and then yield.  When the send completes,
+            // we will resume immediately after this point.
+            data_->origin = ASIO_SENDSOCK;
+            CORO_YIELD data_->socket->asyncSend(data_->msgbuf->getData(),
+                data_->msgbuf->getLength(), data_->remote_snd.get(), *this);
+    
+            // Now receive the response.  Since TCP may not receive the entire
+            // message in one operation, we need to loop until we have received
+            // it. (This can't be done within the asyncReceive() method because
+            // each I/O operation will be done asynchronously and between each one
+            // we need to yield ... and we *really* don't want to set up another
+            // coroutine within that method.)  So after each receive (and yield),
+            // we check if the operation is complete and if not, loop to read again.
+            //
+            // Another concession to TCP is that the amount of is contained in the
+            // first two bytes.  This leads to two problems:
+            //
+            // a) We don't want those bytes in the return buffer.
+            // b) They may not both arrive in the first I/O.
+            //
+            // So... we need to loop until we have at least two bytes, then store
+            // the expected amount of data.  Then we need to loop until we have
+            // received all the data before copying it back to the user's buffer.
+            // And we want to minimise the amount of copying...
+    
+            data_->origin = ASIO_RECVSOCK;
+            data_->cumulative = 0;          // No data yet received
+            data_->offset = 0;              // First data into start of buffer
+            data_->received->clear();       // Clear the receive buffer
+            do {
+                CORO_YIELD data_->socket->asyncReceive(data_->staging,
+                                                       static_cast<size_t>(STAGING_LENGTH),
+                                                       data_->offset,
+                                                       data_->remote_rcv.get(), *this);
+            } while (!data_->socket->processReceivedData(data_->staging, length,
+                                                         data_->cumulative, data_->offset,
+                                                         data_->expected, data_->received));
+        } while (!data_->responseOK());
 
         // Finished with this socket, so close it.  This will not generate an
         // I/O error, but reset the origin to unknown in case we change this.
@@ -290,16 +315,16 @@ IOFetch::stop(Result result) {
             case TIME_OUT:
                 if (logger.isDebugEnabled(1)) {
                     logger.debug(20, ASIO_RECVTMO,
-                                 data_->remote->getAddress().toText().c_str(),
-                                 static_cast<int>(data_->remote->getPort()));
+                                 data_->remote_snd->getAddress().toText().c_str(),
+                                 static_cast<int>(data_->remote_snd->getPort()));
                 }
                 break;
 
             case SUCCESS:
                 if (logger.isDebugEnabled(50)) {
                     logger.debug(30, ASIO_FETCHCOMP,
-                                 data_->remote->getAddress().toText().c_str(),
-                                 static_cast<int>(data_->remote->getPort()));
+                                 data_->remote_rcv->getAddress().toText().c_str(),
+                                 static_cast<int>(data_->remote_rcv->getPort()));
                 }
                 break;
 
@@ -308,14 +333,14 @@ IOFetch::stop(Result result) {
                 // allowed but as it is unusual it is logged, but with a lower
                 // debug level than a timeout (which is totally normal).
                 logger.debug(1, ASIO_FETCHSTOP,
-                             data_->remote->getAddress().toText().c_str(),
-                             static_cast<int>(data_->remote->getPort()));
+                             data_->remote_snd->getAddress().toText().c_str(),
+                             static_cast<int>(data_->remote_snd->getPort()));
                 break;
 
             default:
                 logger.error(ASIO_UNKRESULT, static_cast<int>(result),
-                             data_->remote->getAddress().toText().c_str(),
-                             static_cast<int>(data_->remote->getPort()));
+                             data_->remote_snd->getAddress().toText().c_str(),
+                             static_cast<int>(data_->remote_snd->getPort()));
         }
 
         // Stop requested, cancel and I/O's on the socket and shut it down,
@@ -345,10 +370,10 @@ void IOFetch::logIOFailure(asio::error_code ec) {
     static const char* PROTOCOL[2] = {"TCP", "UDP"};
     logger.error(data_->origin,
                  ec.value(),
-                 ((data_->remote->getProtocol() == IPPROTO_TCP) ?
+                 ((data_->remote_snd->getProtocol() == IPPROTO_TCP) ?
                      PROTOCOL[0] : PROTOCOL[1]),
-                 data_->remote->getAddress().toText().c_str(),
-                 static_cast<int>(data_->remote->getPort()));
+                 data_->remote_snd->getAddress().toText().c_str(),
+                 static_cast<int>(data_->remote_snd->getPort()));
 }
 
 } // namespace asiolink

src/lib/asiolink/tcp_server.cc

@@ -110,6 +110,7 @@ TCPServer::operator()(error_code ec, size_t length) {
         CORO_YIELD async_read(*socket_, asio::buffer(data_.get(),
                               TCP_MESSAGE_LENGTHSIZE), *this);
         if (ec) {
+            socket_->close();
             CORO_YIELD return;
         }
 
@@ -122,6 +123,7 @@ TCPServer::operator()(error_code ec, size_t length) {
         }
 
         if (ec) {
+            socket_->close();
             CORO_YIELD return;
         }
 
@@ -156,6 +158,7 @@ TCPServer::operator()(error_code ec, size_t length) {
         // If we don't have a DNS Lookup provider, there's no point in
         // continuing; we exit the coroutine permanently.
         if (lookup_callback_ == NULL) {
+            socket_->close();
             CORO_YIELD return;
         }
 
@@ -173,6 +176,9 @@ TCPServer::operator()(error_code ec, size_t length) {
         // The 'done_' flag indicates whether we have an answer
         // to send back.  If not, exit the coroutine permanently.
         if (!done_) {
+            // TODO: should we keep the connection open for a short time
+            // to see if new requests come in?
+            socket_->close();
             CORO_YIELD return;
         }
 
@@ -194,6 +200,10 @@ TCPServer::operator()(error_code ec, size_t length) {
         // (though we have nothing further to do, so the coroutine
         // will simply exit at that time).
         CORO_YIELD async_write(*socket_, bufs, *this);
+
+        // TODO: should we keep the connection open for a short time
+        // to see if new requests come in?
+        socket_->close();
     }
 }
 

src/lib/asiolink/tests/io_endpoint_unittest.cc

@@ -60,6 +60,62 @@ TEST(IOEndpointTest, createTCPv6) {
     EXPECT_EQ(IPPROTO_TCP, ep->getProtocol());
 }
 
+TEST(IOEndpointTest, equality) {
+    std::vector<const IOEndpoint *> epv;
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1234"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1234"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1234"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1234"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1235"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1235"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1235"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1235"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.1"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.1"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.2"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.2"), 5303));
+    epv.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.2"), 5304));
+    epv.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.2"), 5304));
+
+    for (size_t i = 0; i < epv.size(); ++i) {
+        for (size_t j = 0; j < epv.size(); ++j) {
+            if (i != j) {
+                // We use EXPECT_TRUE/FALSE instead of _EQ here, since
+                // _EQ requires there is an operator<< as well
+                EXPECT_FALSE(*epv[i] == *epv[j]);
+                EXPECT_TRUE(*epv[i] != *epv[j]);
+            }
+        }
+    }
+
+    // Create a second array with exactly the same values. We use create()
+    // again to make sure we get different endpoints
+    std::vector<const IOEndpoint *> epv2;
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1234"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1234"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1234"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1234"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1235"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1235"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("2001:db8::1235"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("2001:db8::1235"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.1"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.1"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.2"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.2"), 5303));
+    epv2.push_back(IOEndpoint::create(IPPROTO_TCP, IOAddress("192.0.2.2"), 5304));
+    epv2.push_back(IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.2"), 5304));
+
+    for (size_t i = 0; i < epv.size(); ++i) {
+        EXPECT_TRUE(*epv[i] == *epv2[i]);
+        EXPECT_FALSE(*epv[i] != *epv2[i]);
+    }
+}
+
 TEST(IOEndpointTest, createIPProto) {
     EXPECT_THROW(IOEndpoint::create(IPPROTO_IP, IOAddress("192.0.2.1"),
                                     53210)->getAddress().toText(),

src/lib/asiolink/tests/io_fetch_unittest.cc

@@ -76,6 +76,7 @@ public:
     // response handler methods in this class) receives the question sent by the
     // fetch object.
     uint8_t         receive_buffer_[MAX_SIZE]; ///< Server receive buffer
+    OutputBufferPtr expected_buffer_;          ///< Data we expect to receive
     vector<uint8_t> send_buffer_;           ///< Server send buffer
     uint16_t        send_cumulative_;       ///< Data sent so far
 
@@ -84,6 +85,11 @@ public:
     string          test_data_;             ///< Large string - here for convenience
     bool            debug_;                 ///< true to enable debug output
     size_t          tcp_send_size_;         ///< Max size of TCP send
+    uint8_t         qid_0;                  ///< First octet of qid
+    uint8_t         qid_1;                  ///< Second octet of qid
+
+    bool            tcp_short_send_;        ///< If set to true, we do not send
+                                            ///  all data in the tcp response
 
     /// \brief Constructor
     IOFetchTest() :
@@ -102,12 +108,16 @@ public:
         cumulative_(0),
         timer_(service_.get_io_service()),
         receive_buffer_(),
+        expected_buffer_(new OutputBuffer(512)),
         send_buffer_(),
         send_cumulative_(0),
         return_data_(""),
         test_data_(""),
         debug_(DEBUG),
-        tcp_send_size_(0)
+        tcp_send_size_(0),
+        qid_0(0),
+        qid_1(0),
+        tcp_short_send_(false)
     {
         // Construct the data buffer for question we expect to receive.
         Message msg(Message::RENDER);
@@ -118,6 +128,8 @@ public:
         msg.addQuestion(question_);
         MessageRenderer renderer(*msgbuf_);
         msg.toWire(renderer);
+        MessageRenderer renderer2(*expected_buffer_);
+        msg.toWire(renderer2);
 
         // Initialize the test data to be returned: tests will return a
         // substring of this data. (It's convenient to have this as a member of
@@ -144,9 +156,14 @@ public:
     /// \param socket Socket to use to send the answer
     /// \param ec ASIO error code, completion code of asynchronous I/O issued
     ///        by the "server" to receive data.
+    /// \param bad_qid If set to true, the QID in the response will be mangled
+    /// \param second_send If set to true, (and bad_qid is too), after the
+    ///        mangled qid response has been sent, a second packet will be
+    ///        sent with the correct QID.
     /// \param length Amount of data received.
     void udpReceiveHandler(udp::endpoint* remote, udp::socket* socket,
-                    error_code ec = error_code(), size_t length = 0) {
+                    error_code ec = error_code(), size_t length = 0,
+                    bool bad_qid = false, bool second_send = false) {
         if (debug_) {
             cout << "udpReceiveHandler(): error = " << ec.value() <<
                     ", length = " << length << endl;
@@ -155,6 +172,8 @@ public:
         // The QID in the incoming data is random so set it to 0 for the
         // data comparison check. (It is set to 0 in the buffer containing
         // the expected data.)
+        qid_0 = receive_buffer_[0];
+        qid_1 = receive_buffer_[1];
         receive_buffer_[0] = receive_buffer_[1] = 0;
 
         // Check that length of the received data and the expected data are
@@ -164,10 +183,23 @@ public:
         static_cast<const uint8_t*>(msgbuf_->getData())));
 
         // Return a message back to the IOFetch object.
-        socket->send_to(asio::buffer(return_data_.c_str(), return_data_.size()),
-                                     *remote);
+        if (!bad_qid) {
+            expected_buffer_->writeUint8At(qid_0, 0);
+            expected_buffer_->writeUint8At(qid_1, 1);
+        } else {
+            expected_buffer_->writeUint8At(qid_0 + 1, 0);
+            expected_buffer_->writeUint8At(qid_1 + 1, 1);
+        }
+        socket->send_to(asio::buffer(expected_buffer_->getData(), length), *remote);
+
+        if (bad_qid && second_send) {
+            expected_buffer_->writeUint8At(qid_0, 0);
+            expected_buffer_->writeUint8At(qid_1, 1);
+            socket->send_to(asio::buffer(expected_buffer_->getData(),
+                            expected_buffer_->getLength()), *remote);
+        }
         if (debug_) {
-            cout << "udpReceiveHandler(): returned " << return_data_.size() <<
+            cout << "udpReceiveHandler(): returned " << expected_buffer_->getLength() <<
                     " bytes to the client" << endl;
         }
     }
@@ -249,18 +281,25 @@ public:
         // field the QID in the received buffer is in the third and fourth
         // bytes.
         EXPECT_EQ(msgbuf_->getLength() + 2, cumulative_);
+        qid_0 = receive_buffer_[2];
+        qid_1 = receive_buffer_[3];
+
         receive_buffer_[2] = receive_buffer_[3] = 0;
         EXPECT_TRUE(equal((receive_buffer_ + 2), (receive_buffer_ + cumulative_ - 2),
             static_cast<const uint8_t*>(msgbuf_->getData())));
 
         // ... and return a message back.  This has to be preceded by a two-byte
         // count field.
+
         send_buffer_.clear();
         send_buffer_.push_back(0);
         send_buffer_.push_back(0);
         writeUint16(return_data_.size(), &send_buffer_[0]);
         copy(return_data_.begin(), return_data_.end(), back_inserter(send_buffer_));
-
+        if (return_data_.size() >= 2) {
+            send_buffer_[2] = qid_0;
+            send_buffer_[3] = qid_1;
+        }
         // Send the data.  This is done in multiple writes with a delay between
         // each to check that the reassembly of TCP packets from fragments works.
         send_cumulative_ = 0;
@@ -298,10 +337,21 @@ public:
             amount = min(tcp_send_size_,
                         (send_buffer_.size() - send_cumulative_));
         }
-        if (debug_) {
-            cout << "tcpSendData(): sending " << amount << " bytes" << endl;
-        }
 
+        // This is for the short send test; reduce the actual amount of
+        // data we send
+        if (tcp_short_send_) {
+            if (debug_) {
+                cout << "tcpSendData(): sending incomplete data (" <<
+                        (amount - 1) << " of " << amount << " bytes)" <<
+                        endl;
+            }
+            --amount;
+        } else {
+            if (debug_) {
+                cout << "tcpSendData(): sending " << amount << " bytes" << endl;
+            }
+        }
 
         // ... and send it.  The amount sent is also passed as the first
         // argument of the send callback, as a check.
@@ -373,10 +423,23 @@ public:
         // when one of the "servers" in this class has sent back return_data_.
         // Check the data is as expected/
         if (expected_ == IOFetch::SUCCESS) {
-            EXPECT_EQ(return_data_.size(), result_buff_->getLength());
-
-            const uint8_t* start = static_cast<const uint8_t*>(result_buff_->getData());
-            EXPECT_TRUE(equal(return_data_.begin(), return_data_.end(), start));
+            // In the case of UDP, we actually send back a real looking packet
+            // in the case of TCP, we send back a 'random' string
+            if (protocol_ == IOFetch::UDP) {
+                EXPECT_EQ(expected_buffer_->getLength(), result_buff_->getLength());
+                EXPECT_EQ(0, memcmp(expected_buffer_->getData(), result_buff_->getData(),
+                          expected_buffer_->getLength()));
+            } else {
+                EXPECT_EQ(return_data_.size(), result_buff_->getLength());
+                // Overwrite the random qid with our own data for the
+                // comparison to succeed
+                if (result_buff_->getLength() >= 2) {
+                    result_buff_->writeUint8At(return_data_[0], 0);
+                    result_buff_->writeUint8At(return_data_[1], 1);
+                }
+                const uint8_t* start = static_cast<const uint8_t*>(result_buff_->getData());
+                EXPECT_TRUE(equal(return_data_.begin(), return_data_.end(), start));
+            }
         }
 
         // ... and cause the run loop to exit.
@@ -452,13 +515,20 @@ public:
     /// Send a query to the server then receives a response.
     ///
     /// \param Test data to return to client
-    void tcpSendReturnTest(const std::string& return_data) {
+    /// \param short_send If true, do not send all data
+    ///                   (should result in timeout)
+    void tcpSendReturnTest(const std::string& return_data, bool short_send = false) {
         if (debug_) {
             cout << "tcpSendReturnTest(): data size = " << return_data.size() << endl;
         }
         return_data_ = return_data;
         protocol_ = IOFetch::TCP;
-        expected_ = IOFetch::SUCCESS;
+        if (short_send) {
+            tcp_short_send_ = true;
+            expected_ = IOFetch::TIME_OUT;
+        } else {
+            expected_ = IOFetch::SUCCESS;
+        }
 
         // Socket into which the connection will be accepted.
         tcp::socket socket(service_.get_io_service());
@@ -481,6 +551,39 @@ public:
         // Tidy up
         socket.close();
     }
+
+    /// Perform a send/receive test over UDP
+    ///
+    /// \param bad_qid If true, do the test where the QID is mangled
+    ///                in the response
+    /// \param second_send If true, do the test where the QID is
+    ///                    mangled in the response, but a second
+    ///                    (correct) packet is used
+    void udpSendReturnTest(bool bad_qid, bool second_send) {
+        protocol_ = IOFetch::UDP;
+
+        // Set up the server.
+        udp::socket socket(service_.get_io_service(), udp::v4());
+        socket.set_option(socket_base::reuse_address(true));
+        socket.bind(udp::endpoint(TEST_HOST, TEST_PORT));
+        return_data_ = "Message returned to the client";
+
+        udp::endpoint remote;
+        socket.async_receive_from(asio::buffer(receive_buffer_, sizeof(receive_buffer_)),
+            remote,
+            boost::bind(&IOFetchTest::udpReceiveHandler, this, &remote, &socket,
+                        _1, _2, bad_qid, second_send));
+        service_.get_io_service().post(udp_fetch_);
+        if (debug_) {
+            cout << "udpSendReceive: async_receive_from posted, waiting for callback" <<
+                    endl;
+        }
+        service_.run();
+
+        socket.close();
+
+        EXPECT_TRUE(run_);;
+    }
 };
 
 // Check the protocol
@@ -507,28 +610,25 @@ TEST_F(IOFetchTest, UdpTimeout) {
 // UDP SendReceive test.  Set up a UDP server then ports a UDP fetch object.
 // This will send question_ to the server and receive the answer back from it.
 TEST_F(IOFetchTest, UdpSendReceive) {
-    protocol_ = IOFetch::UDP;
     expected_ = IOFetch::SUCCESS;
 
-    // Set up the server.
-    udp::socket socket(service_.get_io_service(), udp::v4());
-    socket.set_option(socket_base::reuse_address(true));
-    socket.bind(udp::endpoint(TEST_HOST, TEST_PORT));
-    return_data_ = "Message returned to the client";
-
-    udp::endpoint remote;
-    socket.async_receive_from(asio::buffer(receive_buffer_, sizeof(receive_buffer_)),
-        remote,
-        boost::bind(&IOFetchTest::udpReceiveHandler, this, &remote, &socket,
-                    _1, _2));
-    service_.get_io_service().post(udp_fetch_);
-    if (debug_) {
-        cout << "udpSendReceive: async_receive_from posted, waiting for callback" <<
-                endl;
-    }
-    service_.run();
+    udpSendReturnTest(false, false);
 
-    socket.close();
+    EXPECT_TRUE(run_);;
+}
+
+TEST_F(IOFetchTest, UdpSendReceiveBadQid) {
+    expected_ = IOFetch::TIME_OUT;
+
+    udpSendReturnTest(true, false);
+
+    EXPECT_TRUE(run_);;
+}
+
+TEST_F(IOFetchTest, UdpSendReceiveBadQidResend) {
+    expected_ = IOFetch::SUCCESS;
+
+    udpSendReturnTest(true, true);
 
     EXPECT_TRUE(run_);;
 }
@@ -547,18 +647,20 @@ TEST_F(IOFetchTest, TcpTimeout) {
     timeoutTest(IOFetch::TCP, tcp_fetch_);
 }
 
-// Test with values at or near 0, then at or near the chunk size (16 and 32
+// Test with values at or near 2, then at or near the chunk size (16 and 32
 // bytes, the sizes of the first two packets) then up to 65535.  These are done
 // in separate tests because in practice a new IOFetch is created for each
 // query/response exchange and we don't want to confuse matters in the test
 // by running the test with an IOFetch that has already done one exchange.
-
-TEST_F(IOFetchTest, TcpSendReceive0) {
-    tcpSendReturnTest(test_data_.substr(0, 0));
+//
+// Don't do 0 or 1; the server would not accept the packet
+// (since the length is too short to check the qid)
+TEST_F(IOFetchTest, TcpSendReceive2) {
+    tcpSendReturnTest(test_data_.substr(0, 2));
 }
 
-TEST_F(IOFetchTest, TcpSendReceive1) {
-    tcpSendReturnTest(test_data_.substr(0, 1));
+TEST_F(IOFetchTest, TcpSendReceive3) {
+    tcpSendReturnTest(test_data_.substr(0, 3));
 }
 
 TEST_F(IOFetchTest, TcpSendReceive15) {
@@ -605,4 +707,17 @@ TEST_F(IOFetchTest, TcpSendReceive65535) {
     tcpSendReturnTest(test_data_.substr(0, 65535));
 }
 
+TEST_F(IOFetchTest, TcpSendReceive2ShortSend) {
+    tcpSendReturnTest(test_data_.substr(0, 2), true);
+}
+
+TEST_F(IOFetchTest, TcpSendReceive15ShortSend) {
+    tcpSendReturnTest(test_data_.substr(0, 15), true);
+}
+
+TEST_F(IOFetchTest, TcpSendReceive8192ShortSend) {
+    tcpSendReturnTest(test_data_.substr(0, 8192), true);
+}
+
+
 } // namespace asiolink

src/lib/cache/Makefile.am

@@ -29,5 +29,6 @@ libcache_la_SOURCES  += rrset_entry.h rrset_entry.cc
 libcache_la_SOURCES  += cache_entry_key.h cache_entry_key.cc
 libcache_la_SOURCES  += rrset_copy.h rrset_copy.cc
 libcache_la_SOURCES  += local_zone_data.h local_zone_data.cc
+libcache_la_SOURCES  += message_utility.h message_utility.cc
 
 CLEANFILES = *.gcno *.gcda

src/lib/cache/TODO

@@ -11,4 +11,8 @@
   to expire.
 * When the rrset beging updated is an NS rrset, NSAS should be updated
   together.
+* Share the NXDOMAIN info between different type queries. current implementation
+  can only cache for the type that user quired, for example, if user query A 
+  record of a.example. and the server replied with NXDOMAIN, this should be
+  cached for all the types queries of a.example.
 

src/lib/cache/message_cache.cc

@@ -18,25 +18,34 @@
 #include <nsas/hash_table.h>
 #include <nsas/hash_deleter.h>
 #include "message_cache.h"
+#include "message_utility.h"
 #include "cache_entry_key.h"
 
+namespace isc {
+namespace cache {
+
 using namespace isc::nsas;
 using namespace isc::dns;
 using namespace std;
+using namespace MessageUtility;
 
-namespace isc {
-namespace cache {
-
-MessageCache::MessageCache(boost::shared_ptr<RRsetCache> rrset_cache,
-    uint32_t cache_size, uint16_t message_class):
+MessageCache::MessageCache(const RRsetCachePtr& rrset_cache,
+                           uint32_t cache_size, uint16_t message_class,
+                           const RRsetCachePtr& negative_soa_cache):
     message_class_(message_class),
     rrset_cache_(rrset_cache),
+    negative_soa_cache_(negative_soa_cache),
     message_table_(new NsasEntryCompare<MessageEntry>, cache_size),
     message_lru_((3 * cache_size),
                   new HashDeleter<MessageEntry>(message_table_))
 {
 }
 
+MessageCache::~MessageCache() {
+    // Destroy all the message entries in the cache.
+    message_lru_.clear();
+}
+
 bool
 MessageCache::lookup(const isc::dns::Name& qname,
                      const isc::dns::RRType& qtype,
@@ -63,8 +72,13 @@ MessageCache::lookup(const isc::dns::Name& qname,
 
 bool
 MessageCache::update(const Message& msg) {
+    if (!canMessageBeCached(msg)){
+        return (false);
+    }
+
     QuestionIterator iter = msg.beginQuestion();
-    std::string entry_name = genCacheEntryName((*iter)->getName(), (*iter)->getType());
+    std::string entry_name = genCacheEntryName((*iter)->getName(),
+                                               (*iter)->getType());
     HashKey entry_key = HashKey(entry_name, RRClass(message_class_));
 
     // The simplest way to update is removing the old message entry directly.
@@ -77,7 +91,8 @@ MessageCache::update(const Message& msg) {
         message_lru_.remove(old_msg_entry);
     }
 
-    MessageEntryPtr msg_entry(new MessageEntry(msg, rrset_cache_));
+    MessageEntryPtr msg_entry(new MessageEntry(msg, rrset_cache_,
+                                               negative_soa_cache_));
     message_lru_.add(msg_entry);
     return (message_table_.add(msg_entry, entry_key, true));
 }

src/lib/cache/message_cache.h

@@ -21,12 +21,11 @@
 #include "message_entry.h"
 #include <nsas/hash_table.h>
 #include <nsas/lru_list.h>
+#include "rrset_cache.h"
 
 namespace isc {
 namespace cache {
 
-class RRsetCache;
-
 /// \brief Message Cache
 /// The object of MessageCache represents the cache for class-specific
 /// messages.
@@ -37,12 +36,18 @@ private:
     MessageCache(const MessageCache& source);
     MessageCache& operator=(const MessageCache& source);
 public:
+    /// \param rrset_cache The cache that stores the RRsets that the
+    ///        message entry will points to
     /// \param cache_size The size of message cache.
-    MessageCache(boost::shared_ptr<RRsetCache> rrset_cache_,
-                 uint32_t cache_size, uint16_t message_class);
+    /// \param message_class The class of the message cache
+    /// \param negative_soa_cache The cache that stores the SOA record
+    ///        that comes from negative response message
+    MessageCache(const RRsetCachePtr& rrset_cache,
+                 uint32_t cache_size, uint16_t message_class,
+                 const RRsetCachePtr& negative_soa_cache);
 
     /// \brief Destructor function
-    virtual ~MessageCache() {}
+    virtual ~MessageCache();
 
     /// \brief Look up message in cache.
     /// \param message generated response message if the message entry
@@ -84,7 +89,8 @@ protected:
     // Make these variants be protected for easy unittest.
 protected:
     uint16_t message_class_; // The class of the message cache.
-    boost::shared_ptr<RRsetCache> rrset_cache_;
+    RRsetCachePtr rrset_cache_;
+    RRsetCachePtr negative_soa_cache_;
     isc::nsas::HashTable<MessageEntry> message_table_;
     isc::nsas::LruList<MessageEntry> message_lru_;
 };

src/lib/cache/message_entry.cc

@@ -18,6 +18,7 @@
 #include <dns/message.h>
 #include <nsas/nsas_entry.h>
 #include "message_entry.h"
+#include "message_utility.h"
 #include "rrset_cache.h"
 
 using namespace isc::dns;
@@ -56,9 +57,27 @@ namespace cache {
 
 static uint32_t MAX_UINT32 = numeric_limits<uint32_t>::max();
 
+// As with caching positive responses it is sensible for a resolver to
+// limit for how long it will cache a negative response as the protocol
+// supports caching for up to 68 years.  Such a limit should not be
+// greater than that applied to positive answers and preferably be
+// tunable.  Values of one to three hours have been found to work well
+// and would make sensible a default.  Values exceeding one day have
+// been found to be problematic. (sec 5, RFC2308)
+// The default value is 3 hourse (10800 seconds)
+// TODO:Give an option to let user configure
+static uint32_t MAX_NEGATIVE_CACHE_TTL = 10800;
+
+// Sets the maximum time for which the server will cache ordinary (positive) answers. The
+// default is one week (7 days = 604800 seconds)
+// TODO:Give an option to let user configure
+static uint32_t MAX_NORMAL_CACHE_TTL = 604800;
+
 MessageEntry::MessageEntry(const isc::dns::Message& msg,
-                           boost::shared_ptr<RRsetCache> rrset_cache):
+                           const RRsetCachePtr& rrset_cache,
+                           const RRsetCachePtr& negative_soa_cache):
     rrset_cache_(rrset_cache),
+    negative_soa_cache_(negative_soa_cache),
     headerflag_aa_(false),
     headerflag_tc_(false)
 {
@@ -74,7 +93,8 @@ MessageEntry::getRRsetEntries(vector<RRsetEntryPtr>& rrset_entry_vec,
     uint16_t entry_count = answer_count_ + authority_count_ + additional_count_;
     rrset_entry_vec.reserve(rrset_entry_vec.size() + entry_count);
     for (int index = 0; index < entry_count; ++index) {
-        RRsetEntryPtr rrset_entry = rrset_cache_->lookup(rrsets_[index].name_,
+        RRsetCache* rrset_cache = rrsets_[index].cache_;
+        RRsetEntryPtr rrset_entry = rrset_cache->lookup(rrsets_[index].name_,
                                                         rrsets_[index].type_);
         if (rrset_entry && time_now < rrset_entry->getExpireTime()) {
             rrset_entry_vec.push_back(rrset_entry);
@@ -104,8 +124,9 @@ MessageEntry::addRRset(isc::dns::Message& message,
         end_index = start_index + additional_count_;
     }
 
-    for(uint16_t index = start_index; index < end_index; ++index) {
-        message.addRRset(section, rrset_entry_vec[index]->getRRset(), dnssec_need);
+    for (uint16_t index = start_index; index < end_index; ++index) {
+        message.addRRset(section, rrset_entry_vec[index]->getRRset(),
+                         dnssec_need);
     }
 }
 
@@ -127,7 +148,9 @@ MessageEntry::genMessage(const time_t& time_now,
         // Begin message generation. We don't need to add question
         // section, since it has been included in the message.
         // Set cached header flags.
-        msg.setHeaderFlag(Message::HEADERFLAG_AA, headerflag_aa_);
+        // The AA flag bit should be cleared because this is a response from
+        // resolver cache
+        msg.setHeaderFlag(Message::HEADERFLAG_AA, false);
         msg.setHeaderFlag(Message::HEADERFLAG_TC, headerflag_tc_);
 
         bool dnssec_need = msg.getEDNS().get();
@@ -233,7 +256,8 @@ MessageEntry::parseSection(const isc::dns::Message& msg,
         RRsetPtr rrset_ptr = *iter;
         RRsetTrustLevel level = getRRsetTrustLevel(msg, rrset_ptr, section);
         RRsetEntryPtr rrset_entry = rrset_cache_->update(*rrset_ptr, level);
-        rrsets_.push_back(RRsetRef(rrset_ptr->getName(), rrset_ptr->getType()));
+        rrsets_.push_back(RRsetRef(rrset_ptr->getName(), rrset_ptr->getType(),
+                          rrset_cache_.get()));
 
         uint32_t rrset_ttl = rrset_entry->getTTL();
         if (smaller_ttl > rrset_ttl) {
@@ -247,6 +271,37 @@ MessageEntry::parseSection(const isc::dns::Message& msg,
 }
 
 void
+MessageEntry::parseNegativeResponseAuthoritySection(const isc::dns::Message& msg,
+        uint32_t& min_ttl,
+        uint16_t& rrset_count)
+{
+    uint16_t count = 0;
+    for (RRsetIterator iter = msg.beginSection(Message::SECTION_AUTHORITY);
+            iter != msg.endSection(Message::SECTION_AUTHORITY);
+            ++iter) {
+        RRsetPtr rrset_ptr = *iter;
+        RRsetTrustLevel level = getRRsetTrustLevel(msg, rrset_ptr,
+                                                   Message::SECTION_AUTHORITY);
+        boost::shared_ptr<RRsetCache> rrset_cache_ptr = rrset_cache_;
+        if (rrset_ptr->getType() == RRType::SOA()) {
+            rrset_cache_ptr = negative_soa_cache_;
+        }
+
+        RRsetEntryPtr rrset_entry = rrset_cache_ptr->update(*rrset_ptr, level);
+        rrsets_.push_back(RRsetRef(rrset_ptr->getName(),
+                                   rrset_ptr->getType(),
+                                   rrset_cache_ptr.get()));
+        uint32_t rrset_ttl = rrset_entry->getTTL();
+        if (min_ttl > rrset_ttl) {
+            min_ttl = rrset_ttl;
+        }
+        ++count;
+    }
+
+    rrset_count = count;
+}
+
+void
 MessageEntry::initMessageEntry(const isc::dns::Message& msg) {
     //TODO better way to cache the header flags?
     headerflag_aa_ = msg.getHeaderFlag(Message::HEADERFLAG_AA);
@@ -261,14 +316,30 @@ MessageEntry::initMessageEntry(const isc::dns::Message& msg) {
     query_class_ = (*iter)->getClass().getCode();
 
     uint32_t min_ttl = MAX_UINT32;
+
+    bool isNegativeResponse = MessageUtility::isNegativeResponse(msg);
+
     parseSection(msg, Message::SECTION_ANSWER, min_ttl, answer_count_);
-    parseSection(msg, Message::SECTION_AUTHORITY, min_ttl, authority_count_);
+    if (!isNegativeResponse) {
+        parseSection(msg, Message::SECTION_AUTHORITY, min_ttl, authority_count_);
+    } else {
+        parseNegativeResponseAuthoritySection(msg, min_ttl, authority_count_);
+    }
     parseSection(msg, Message::SECTION_ADDITIONAL, min_ttl, additional_count_);
 
+    // Limit the ttl to a prset max-value
+    if (!isNegativeResponse) {
+        if (min_ttl > MAX_NORMAL_CACHE_TTL) {
+            min_ttl = MAX_NORMAL_CACHE_TTL;
+        }
+    } else {
+        if (min_ttl > MAX_NEGATIVE_CACHE_TTL) {
+            min_ttl = MAX_NEGATIVE_CACHE_TTL;
+        }
+    }
+
     expire_time_ = time(NULL) + min_ttl;
 }
 
 } // namespace cache
 } // namespace isc
-
-

src/lib/cache/message_entry.h

@@ -19,33 +19,15 @@
 #include <dns/message.h>
 #include <dns/rrset.h>
 #include <nsas/nsas_entry.h>
+#include "rrset_cache.h"
 #include "rrset_entry.h"
 
-
 using namespace isc::nsas;
 
 namespace isc {
 namespace cache {
 
 class RRsetEntry;
-class RRsetCache;
-
-/// \brief Information to refer an RRset.
-///
-/// There is no class information here, since the rrsets are cached in
-/// the class-specific rrset cache.
-struct RRsetRef{
-    /// \brief Constructor
-    ///
-    /// \param name The Name for the RRset
-    /// \param type the RRType for the RRrset
-    RRsetRef(const isc::dns::Name& name, const isc::dns::RRType& type):
-            name_(name), type_(type)
-    {}
-
-    isc::dns::Name name_; // Name of rrset.
-    isc::dns::RRType type_; // Type of rrset.
-};
 
 /// \brief Message Entry
 ///
@@ -56,6 +38,27 @@ class MessageEntry : public NsasEntry<MessageEntry> {
 private:
     MessageEntry(const MessageEntry& source);
     MessageEntry& operator=(const MessageEntry& source);
+
+    /// \brief Information to refer an RRset.
+    ///
+    /// There is no class information here, since the rrsets are cached in
+    /// the class-specific rrset cache.
+    struct RRsetRef{
+        /// \brief Constructor
+        ///
+        /// \param name The Name for the RRset
+        /// \param type The RRType for the RRrset
+        /// \param cache Which cache the RRset is stored in
+        RRsetRef(const isc::dns::Name& name, const isc::dns::RRType& type,
+                RRsetCache* cache):
+                name_(name), type_(type), cache_(cache)
+        {}
+
+        isc::dns::Name name_; // Name of rrset.
+        isc::dns::RRType type_; // Type of rrset.
+        RRsetCache* cache_; //Which cache the RRset is stored
+    };
+
 public:
 
     /// \brief Initialize the message entry object with one dns
@@ -66,8 +69,14 @@ public:
     ///        since some new rrset entries may be inserted into
     ///        rrset cache, or the existed rrset entries need
     ///        to be updated.
+    /// \param negative_soa_cache the pointer of RRsetCache. This
+    ///        cache is used only for storing SOA rrset from negative
+    ///        response (NXDOMAIN or NOERROR_NODATA)
     MessageEntry(const isc::dns::Message& message,
-                 boost::shared_ptr<RRsetCache> rrset_cache);
+                 const RRsetCachePtr& rrset_cache,
+                 const RRsetCachePtr& negative_soa_cache);
+
+    ~MessageEntry() { delete hash_key_ptr_; };
 
     /// \brief generate one dns message according
     ///        the rrsets information of the message.
@@ -115,6 +124,16 @@ protected:
                       uint32_t& smaller_ttl,
                       uint16_t& rrset_count);
 
+    /// \brief Parse the RRsets in the authority section of
+    ///        negative response. The SOA RRset need to be located and
+    ///        stored in a seperate cache
+    /// \param msg The message to parse the RRsets from
+    /// \param min_ttl Get the minimum ttl of rrset in the authority section
+    /// \param rrset_count the rrset count of the authority section
+    void parseNegativeResponseAuthoritySection(const isc::dns::Message& msg,
+            uint32_t& min_ttl,
+            uint16_t& rrset_count);
+
     /// \brief Get RRset Trustworthiness
     ///        The algorithm refers to RFC2181 section 5.4.1
     ///        Only the rrset can be updated by the rrsets
@@ -159,7 +178,9 @@ private:
     HashKey* hash_key_ptr_;  // the key for messag entry in hash table.
 
     std::vector<RRsetRef> rrsets_;
-    boost::shared_ptr<RRsetCache> rrset_cache_;
+    RRsetCachePtr rrset_cache_; //Normal rrset cache
+    // SOA rrset from negative response
+    RRsetCachePtr negative_soa_cache_;
 
     std::string query_name_; // query name of the message.
     uint16_t query_class_; // query class of the message.

src/lib/cache/message_utility.cc

@@ -0,0 +1,80 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#include "message_utility.h"
+#include <dns/rcode.h>
+
+using namespace isc::dns;
+
+namespace isc {
+namespace cache {
+namespace MessageUtility{
+
+bool
+hasTheRecordInAuthoritySection(const isc::dns::Message& msg,
+                               const isc::dns::RRType& type)
+{
+    // isc::dns::Message provide one function hasRRset() should be used to
+    // determine whether the given section has an RRset matching the given
+    // name and type, but currently it is not const-qualified and cannot be
+    // used here
+    // TODO: use hasRRset() function when it is const qualified
+    for (RRsetIterator iter = msg.beginSection(Message::SECTION_AUTHORITY);
+            iter != msg.endSection(Message::SECTION_AUTHORITY);
+            ++iter) {
+        RRsetPtr rrset_ptr = *iter;
+        if (rrset_ptr->getType() == type) {
+            return (true);
+        }
+    }
+    return (false);
+}
+
+bool
+isNegativeResponse(const isc::dns::Message& msg) {
+    if (msg.getRcode() == Rcode::NXDOMAIN()) {
+        return (true);
+    } else if (msg.getRcode() == Rcode::NOERROR()) {
+        // no data in the answer section
+        if (msg.getRRCount(Message::SECTION_ANSWER) == 0) {
+            // NODATA type 1/ type 2 (ref sec2.2 of RFC2308)
+            if (hasTheRecordInAuthoritySection(msg, RRType::SOA())) {
+                return (true);
+            } else if (!hasTheRecordInAuthoritySection(msg, RRType::NS())) {
+                // NODATA type 3 (sec2.2 of RFC2308)
+                return (true);
+            }
+        }
+    }
+
+    return (false);
+}
+
+bool
+canMessageBeCached(const isc::dns::Message& msg) {
+    // If the message is a negative response, but no SOA record is found in
+    // the authority section, the message cannot be cached
+    if (isNegativeResponse(msg) &&
+        !hasTheRecordInAuthoritySection(msg, RRType::SOA())){
+        return (false);
+    }
+
+    return (true);
+}
+
+} // namespace MessageUtility
+} // namespace cache
+} // namespace isc

src/lib/cache/message_utility.h

@@ -0,0 +1,66 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#ifndef __MESSAGE_UTILITY_H
+#define __MESSAGE_UTILITY_H
+
+#include <dns/message.h>
+
+namespace isc {
+namespace cache {
+
+/// \brief Some utility functions to extract info from message
+///
+/// We need to check the message before cache it, for example, if no SOA
+/// record is found in the Authority section of NXDOMAIN response, the
+/// message cannot be cached
+namespace MessageUtility{
+
+/// \brief Check whether there is some type of record in
+///        Authority section
+///
+/// \param msg The response message to be checked
+/// \param type The RR type that need to check
+bool hasTheRecordInAuthoritySection(const isc::dns::Message& msg,
+                                    const isc::dns::RRType& type);
+
+/// \brief Check whetehr the message is a negative response
+///        (NXDOMAIN or NOERROR_NODATA)
+///
+/// \param msg The response message
+bool isNegativeResponse(const isc::dns::Message& msg);
+
+/// \brief Check whether the message can be cached
+///        Negative responses without SOA records SHOULD NOT be cached as there
+///        is no way to prevent the negative responses looping forever between a
+///        pair of servers even with a short TTL.
+///        Despite the DNS forming a tree of servers, with various mis-
+///        configurations it is possible to form a loop in the query graph, e.g.
+///        two servers listing each other as forwarders, various lame server
+///        configurations.  Without a TTL count down a cache negative response
+///        when received by the next server would have its TTL reset.  This
+///        negative indication could then live forever circulating between the
+///        servers involved. (Sec 5, RFC2308)
+///
+/// \param msg The response message
+bool canMessageBeCached(const isc::dns::Message& msg);
+
+} // namespace MessageUtility
+} // namespace cache
+} // namespace isc
+
+
+#endif//__MESSAGE_UTILITY_H

src/lib/cache/resolver_cache.cc

@@ -32,12 +32,17 @@ ResolverClassCache::ResolverClassCache(const RRClass& cache_class) :
     local_zone_data_ = LocalZoneDataPtr(new LocalZoneData(cache_class_.getCode()));
     rrsets_cache_ = RRsetCachePtr(new RRsetCache(RRSET_CACHE_DEFAULT_SIZE,
                                                  cache_class_.getCode()));
+    // SOA rrset cache from negative response
+    negative_soa_cache_ = RRsetCachePtr(new RRsetCache(NEGATIVE_RRSET_CACHE_DEFAULT_SIZE,
+                                                       cache_class_.getCode()));
+
     messages_cache_ = MessageCachePtr(new MessageCache(rrsets_cache_,
                                       MESSAGE_CACHE_DEFAULT_SIZE,
-                                      cache_class_.getCode()));
+                                      cache_class_.getCode(),
+                                      negative_soa_cache_));
 }
 
-ResolverClassCache::ResolverClassCache(CacheSizeInfo cache_info) :
+ResolverClassCache::ResolverClassCache(const CacheSizeInfo& cache_info) :
     cache_class_(cache_info.cclass)
 {
     uint16_t klass = cache_class_.getCode();
@@ -45,14 +50,18 @@ ResolverClassCache::ResolverClassCache(CacheSizeInfo cache_info) :
     local_zone_data_ = LocalZoneDataPtr(new LocalZoneData(klass));
     rrsets_cache_ = RRsetCachePtr(new
                         RRsetCache(cache_info.rrset_cache_size, klass));
+    // SOA rrset cache from negative response
+    negative_soa_cache_ = RRsetCachePtr(new RRsetCache(cache_info.rrset_cache_size,
+                                                       klass));
+
     messages_cache_ = MessageCachePtr(new MessageCache(rrsets_cache_,
                                       cache_info.message_cache_size,
-                                      klass));
+                                      klass, negative_soa_cache_));
 }
 
 const RRClass&
 ResolverClassCache::getClass() const {
-    return cache_class_;
+    return (cache_class_);
 }
 
 bool
@@ -104,7 +113,7 @@ ResolverClassCache::update(const isc::dns::Message& msg) {
 }
 
 bool
-ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr rrset_ptr,
+ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr& rrset_ptr,
                                 RRsetCachePtr rrset_cache_ptr)
 {
     RRsetTrustLevel level;
@@ -120,7 +129,7 @@ ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr rrset_ptr,
 }
 
 bool
-ResolverClassCache::update(const isc::dns::ConstRRsetPtr rrset_ptr) {
+ResolverClassCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
     // First update local zone, then update rrset cache.
     local_zone_data_->update((*rrset_ptr.get()));
     updateRRsetCache(rrset_ptr, rrsets_cache_);
@@ -209,7 +218,7 @@ ResolverCache::update(const isc::dns::Message& msg) {
 }
 
 bool
-ResolverCache::update(const isc::dns::ConstRRsetPtr rrset_ptr) {
+ResolverCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
     ResolverClassCache* cc = getClassCache(rrset_ptr->getClass());
     if (cc) {
         return (cc->update(rrset_ptr));
@@ -232,10 +241,10 @@ ResolverClassCache*
 ResolverCache::getClassCache(const isc::dns::RRClass& cache_class) const {
     for (int i = 0; i < class_caches_.size(); ++i) {
         if (class_caches_[i]->getClass() == cache_class) {
-            return class_caches_[i];
+            return (class_caches_[i]);
         }
     }
-    return NULL;
+    return (NULL);
 }
 
 } // namespace cache

src/lib/cache/resolver_cache.h

@@ -32,6 +32,7 @@ class RRsetCache;
 //TODO a better proper default cache size
 #define MESSAGE_CACHE_DEFAULT_SIZE 10000
 #define RRSET_CACHE_DEFAULT_SIZE   20000
+#define NEGATIVE_RRSET_CACHE_DEFAULT_SIZE   10000
 
 /// \brief Cache Size Information.
 ///
@@ -44,7 +45,7 @@ public:
     /// \param cls The RRClass code
     /// \param msg_cache_size The size for the message cache
     /// \param rst_cache_size The size for the RRset cache
-    CacheSizeInfo(const isc::dns::RRClass& cls, 
+    CacheSizeInfo(const isc::dns::RRClass& cls,
                   uint32_t msg_cache_size,
                   uint32_t rst_cache_size):
                     cclass(cls),
@@ -87,7 +88,7 @@ public:
     /// \brief Construct Function.
     /// \param caches_size cache size information for each
     ///        messages/rrsets of different classes.
-    ResolverClassCache(CacheSizeInfo cache_info);
+    ResolverClassCache(const CacheSizeInfo& cache_info);
 
     /// \name Lookup Interfaces
     //@{
@@ -132,6 +133,11 @@ public:
     /// \note the function doesn't do any message validation check,
     ///       the user should make sure the message is valid, and of
     ///       the right class
+    /// TODO: Share the NXDOMAIN info between different type queries
+    ///       current implementation can only cache for the type that
+    ///       user quired, for example, if user query A record of
+    ///       a.example. and the server replied with NXDOMAIN, this
+    ///       should be cached for all the types queries of a.example.
     bool update(const isc::dns::Message& msg);
 
     /// \brief Update the rrset in the cache with the new one.
@@ -149,13 +155,13 @@ public:
     ///
     /// \note The class of the RRset must have been checked. It is not
     /// here.
-    bool update(const isc::dns::ConstRRsetPtr rrset_ptr);
+    bool update(const isc::dns::ConstRRsetPtr& rrset_ptr);
 
     /// \brief Get the RRClass this cache is for
     ///
     /// \return The RRClass of this cache
     const isc::dns::RRClass& getClass() const;
-    
+
 private:
     /// \brief Update rrset cache.
     ///
@@ -165,7 +171,7 @@ private:
     /// \return return true if the rrset is updated in the rrset cache,
     ///         or else return false if failed.
     /// \param rrset_cache_ptr The rrset cache need to be updated.
-    bool updateRRsetCache(const isc::dns::ConstRRsetPtr rrset_ptr,
+    bool updateRRsetCache(const isc::dns::ConstRRsetPtr& rrset_ptr,
                           RRsetCachePtr rrset_cache_ptr);
 
     /// \brief Class this cache is for.
@@ -181,10 +187,13 @@ private:
     /// Cache for rrsets in local zones, rrsets
     /// in it never expire.
     LocalZoneDataPtr local_zone_data_;
+    //@}
 
     /// \brief cache the rrsets parsed from the received message.
     RRsetCachePtr rrsets_cache_;
-    //@}
+
+    /// \brief cache the SOA rrset parsed from the negative response message.
+    RRsetCachePtr negative_soa_cache_;
 };
 
 class ResolverCache {
@@ -289,7 +298,7 @@ public:
     ///
     /// \overload
     ///
-    bool update(const isc::dns::ConstRRsetPtr rrset_ptr);
+    bool update(const isc::dns::ConstRRsetPtr& rrset_ptr);
 
     /// \name Cache Serialization
     //@{

src/lib/cache/rrset_cache.h

@@ -40,12 +40,14 @@ private:
     RRsetCache(const RRsetCache&);
     RRsetCache& operator=(const RRsetCache&);
 public:
-    /// \brief Constructor
+    /// \brief Constructor and Destructor
     ///
     /// \param cache_size the size of rrset cache.
     /// \param rrset_class the class of rrset cache.
     RRsetCache(uint32_t cache_size, uint16_t rrset_class);
-    virtual ~RRsetCache() {}
+    virtual ~RRsetCache() {
+        rrset_lru_.clear(); // Clear the rrset entries in the list.
+    }
     //@}
 
     /// \brief Look up rrset in cache.

src/lib/cache/tests/Makefile.am

@@ -38,6 +38,7 @@ run_unittests_SOURCES  += message_cache_unittest.cc
 run_unittests_SOURCES  += message_entry_unittest.cc
 run_unittests_SOURCES  += local_zone_data_unittest.cc
 run_unittests_SOURCES  += resolver_cache_unittest.cc
+run_unittests_SOURCES  += negative_cache_unittest.cc
 run_unittests_SOURCES  += cache_test_messagefromfile.h
 run_unittests_SOURCES  += cache_test_sectioncount.h
 
@@ -59,7 +60,9 @@ endif
 
 noinst_PROGRAMS = $(TESTS)
 
-EXTRA_DIST = testdata/message_fromWire1
+EXTRA_DIST = testdata/message_cname_referral.wire
+EXTRA_DIST += testdata/message_example_com_soa.wire
+EXTRA_DIST += testdata/message_fromWire1
 EXTRA_DIST += testdata/message_fromWire2
 EXTRA_DIST += testdata/message_fromWire3
 EXTRA_DIST += testdata/message_fromWire4
@@ -68,3 +71,10 @@ EXTRA_DIST += testdata/message_fromWire6
 EXTRA_DIST += testdata/message_fromWire7
 EXTRA_DIST += testdata/message_fromWire8
 EXTRA_DIST += testdata/message_fromWire9
+EXTRA_DIST += testdata/message_large_ttl.wire
+EXTRA_DIST += testdata/message_nodata_with_soa.wire
+EXTRA_DIST += testdata/message_nxdomain_cname.wire
+EXTRA_DIST += testdata/message_nxdomain_large_ttl.wire
+EXTRA_DIST += testdata/message_nxdomain_no_soa.wire
+EXTRA_DIST += testdata/message_nxdomain_with_soa.wire
+EXTRA_DIST += testdata/message_referral.wire

src/lib/cache/tests/message_cache_unittest.cc

@@ -33,9 +33,10 @@ namespace {
 /// its internals.
 class DerivedMessageCache: public MessageCache {
 public:
-    DerivedMessageCache(boost::shared_ptr<RRsetCache> rrset_cache_,
-                        uint32_t cache_size, uint16_t message_class):
-        MessageCache(rrset_cache_, cache_size, message_class)
+    DerivedMessageCache(const RRsetCachePtr& rrset_cache,
+                        uint32_t cache_size, uint16_t message_class,
+                        const RRsetCachePtr& negative_soa_cache):
+        MessageCache(rrset_cache, cache_size, message_class, negative_soa_cache)
     {}
 
     uint16_t messages_count() {
@@ -70,13 +71,16 @@ public:
     {
         uint16_t class_ = RRClass::IN().getCode();
         rrset_cache_.reset(new DerivedRRsetCache(RRSET_CACHE_DEFAULT_SIZE, class_));
+        negative_soa_cache_.reset(new RRsetCache(NEGATIVE_RRSET_CACHE_DEFAULT_SIZE, class_));
         // Set the message cache size to 1, make it easy for unittest.
-        message_cache_.reset(new DerivedMessageCache(rrset_cache_, 1, class_ ));
+        message_cache_.reset(new DerivedMessageCache(rrset_cache_, 1, class_,
+                                                     negative_soa_cache_));
     }
 
 protected:
     boost::shared_ptr<DerivedMessageCache> message_cache_;
     boost::shared_ptr<DerivedRRsetCache> rrset_cache_;
+    RRsetCachePtr negative_soa_cache_;
     Message message_parse;
     Message message_render;
 };
@@ -134,7 +138,7 @@ TEST_F(MessageCacheTest, testUpdate) {
     EXPECT_TRUE(message_cache_->update(new_msg));
     Message new_msg_render(Message::RENDER);
     EXPECT_TRUE(message_cache_->lookup(qname, RRType::SOA(), new_msg_render));
-    EXPECT_TRUE(new_msg_render.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_FALSE(new_msg_render.getHeaderFlag(Message::HEADERFLAG_AA));
 }
 
 TEST_F(MessageCacheTest, testCacheLruBehavior) {

src/lib/cache/tests/message_entry_unittest.cc

@@ -1,5 +1,3 @@
-// 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.
@@ -38,14 +36,15 @@ namespace {
 class DerivedMessageEntry: public MessageEntry {
 public:
     DerivedMessageEntry(const isc::dns::Message& message,
-                        boost::shared_ptr<RRsetCache> rrset_cache_):
-             MessageEntry(message, rrset_cache_)
+                        const RRsetCachePtr& rrset_cache_,
+                        const RRsetCachePtr& negative_soa_cache_):
+             MessageEntry(message, rrset_cache_, negative_soa_cache_)
     {}
 
-    /// \brief Wrap the protected function so that it can be tested.   
+    /// \brief Wrap the protected function so that it can be tested.
     void parseSectionForTest(const Message& msg,
                            const Message::Section& section,
-                           uint32_t& smaller_ttl, 
+                           uint32_t& smaller_ttl,
                            uint16_t& rrset_count)
     {
         parseSection(msg, section, smaller_ttl, rrset_count);
@@ -75,18 +74,20 @@ public:
                         message_render(Message::RENDER)
     {
         rrset_cache_.reset(new RRsetCache(RRSET_CACHE_DEFAULT_SIZE, class_));
+        negative_soa_cache_.reset(new RRsetCache(NEGATIVE_RRSET_CACHE_DEFAULT_SIZE, class_));
     }
 
 protected:
     uint16_t class_;
     RRsetCachePtr rrset_cache_;
+    RRsetCachePtr negative_soa_cache_;
     Message message_parse;
     Message message_render;
 };
 
 TEST_F(MessageEntryTest, testParseRRset) {
     messageFromFile(message_parse, "message_fromWire3");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     uint32_t ttl = MAX_UINT32;
     uint16_t rrset_count = 0;
     message_entry.parseSectionForTest(message_parse, Message::SECTION_ANSWER, ttl, rrset_count);
@@ -106,7 +107,7 @@ TEST_F(MessageEntryTest, testParseRRset) {
 
 TEST_F(MessageEntryTest, testGetRRsetTrustLevel_AA) {
     messageFromFile(message_parse, "message_fromWire3");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
 
     RRsetIterator rrset_iter = message_parse.beginSection(Message::SECTION_ANSWER);
     RRsetTrustLevel level = message_entry.getRRsetTrustLevelForTest(message_parse,
@@ -129,7 +130,7 @@ TEST_F(MessageEntryTest, testGetRRsetTrustLevel_AA) {
 
 TEST_F(MessageEntryTest, testGetRRsetTrustLevel_NONAA) {
     messageFromFile(message_parse, "message_fromWire4");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     RRsetIterator rrset_iter = message_parse.beginSection(Message::SECTION_ANSWER);
     RRsetTrustLevel level = message_entry.getRRsetTrustLevelForTest(message_parse,
                                                                     *rrset_iter,
@@ -151,7 +152,7 @@ TEST_F(MessageEntryTest, testGetRRsetTrustLevel_NONAA) {
 
 TEST_F(MessageEntryTest, testGetRRsetTrustLevel_CNAME) {
     messageFromFile(message_parse, "message_fromWire5");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     RRsetIterator rrset_iter = message_parse.beginSection(Message::SECTION_ANSWER);
     RRsetTrustLevel level = message_entry.getRRsetTrustLevelForTest(message_parse,
                                                                     *rrset_iter,
@@ -167,7 +168,7 @@ TEST_F(MessageEntryTest, testGetRRsetTrustLevel_CNAME) {
 
 TEST_F(MessageEntryTest, testGetRRsetTrustLevel_CNAME_and_DNAME) {
     messageFromFile(message_parse, "message_fromWire7");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     RRsetIterator rrset_iter = message_parse.beginSection(Message::SECTION_ANSWER);
     RRsetTrustLevel level = message_entry.getRRsetTrustLevelForTest(message_parse,
                                                                     *rrset_iter,
@@ -186,7 +187,7 @@ TEST_F(MessageEntryTest, testGetRRsetTrustLevel_CNAME_and_DNAME) {
 
 TEST_F(MessageEntryTest, testGetRRsetTrustLevel_DNAME_and_CNAME) {
     messageFromFile(message_parse, "message_fromWire8");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     RRsetIterator rrset_iter = message_parse.beginSection(Message::SECTION_ANSWER);
     RRsetTrustLevel level = message_entry.getRRsetTrustLevelForTest(message_parse,
                                                                     *rrset_iter,
@@ -214,7 +215,7 @@ TEST_F(MessageEntryTest, testGetRRsetTrustLevel_DNAME_and_CNAME) {
 
 TEST_F(MessageEntryTest, testGetRRsetTrustLevel_DNAME) {
     messageFromFile(message_parse, "message_fromWire6");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     RRsetIterator rrset_iter = message_parse.beginSection(Message::SECTION_ANSWER);
     RRsetTrustLevel level = message_entry.getRRsetTrustLevelForTest(message_parse,
                                                                     *rrset_iter,
@@ -239,7 +240,7 @@ TEST_F(MessageEntryTest, testGetRRsetTrustLevel_DNAME) {
 // is right
 TEST_F(MessageEntryTest, testInitMessageEntry) {
     messageFromFile(message_parse, "message_fromWire3");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     time_t expire_time = message_entry.getExpireTime();
     // 1 second should be enough to do the compare
     EXPECT_TRUE((time(NULL) + 10801) > expire_time);
@@ -247,7 +248,7 @@ TEST_F(MessageEntryTest, testInitMessageEntry) {
 
 TEST_F(MessageEntryTest, testGetRRsetEntries) {
     messageFromFile(message_parse, "message_fromWire3");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     vector<RRsetEntryPtr> vec;
 
     // the time is bigger than the smallest expire time of
@@ -258,15 +259,14 @@ TEST_F(MessageEntryTest, testGetRRsetEntries) {
 
 TEST_F(MessageEntryTest, testGenMessage) {
     messageFromFile(message_parse, "message_fromWire3");
-    DerivedMessageEntry message_entry(message_parse, rrset_cache_);
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
     time_t expire_time = message_entry.getExpireTime();
 
     Message msg(Message::RENDER);
     EXPECT_FALSE(message_entry.genMessage(expire_time + 2, msg));
     message_entry.genMessage(time(NULL), msg);
     // Check whether the generated message is same with cached one.
-
-    EXPECT_TRUE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
     EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_TC));
     EXPECT_EQ(1, sectionRRsetCount(msg, Message::SECTION_ANSWER));
     EXPECT_EQ(1, sectionRRsetCount(msg, Message::SECTION_AUTHORITY));
@@ -278,4 +278,32 @@ TEST_F(MessageEntryTest, testGenMessage) {
     EXPECT_EQ(7, msg.getRRCount(Message::SECTION_ADDITIONAL));
 }
 
+TEST_F(MessageEntryTest, testMaxTTL) {
+    messageFromFile(message_parse, "message_large_ttl.wire");
+
+    // The ttl of rrset from Answer and Authority sections are both 604801 seconds
+    RRsetIterator iter = message_parse.beginSection(Message::SECTION_ANSWER);
+    EXPECT_EQ(604801, (*iter)->getTTL().getValue());
+    iter = message_parse.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_EQ(604801, (*iter)->getTTL().getValue());
+
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
+
+    // The ttl is limited to 604800 seconds (7days)
+    EXPECT_EQ(time(NULL) + 604800, message_entry.getExpireTime());
+}
+
+TEST_F(MessageEntryTest, testMaxNegativeTTL) {
+    messageFromFile(message_parse, "message_nxdomain_large_ttl.wire");
+
+    // The ttl of rrset Authority sections are 10801 seconds
+    RRsetIterator iter = message_parse.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_EQ(10801, (*iter)->getTTL().getValue());
+
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
+
+    // The ttl is limited to 10800 seconds (3 hours)
+    EXPECT_EQ(time(NULL) + 10800, message_entry.getExpireTime());
+}
+
 }   // namespace

src/lib/cache/tests/negative_cache_unittest.cc

@@ -0,0 +1,242 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+#include <config.h>
+#include <string>
+#include <gtest/gtest.h>
+#include <dns/rrset.h>
+#include <dns/rcode.h>
+#include "resolver_cache.h"
+#include "cache_test_messagefromfile.h"
+
+using namespace isc::cache;
+using namespace isc::dns;
+using namespace std;
+
+namespace {
+
+class NegativeCacheTest: public testing::Test{
+public:
+    NegativeCacheTest() {
+        vector<CacheSizeInfo> vec;
+        CacheSizeInfo class_in(RRClass::IN(), 100, 200);
+        vec.push_back(class_in);
+        cache = new ResolverCache(vec);
+    }
+
+    ~NegativeCacheTest() {
+        delete cache;
+    }
+
+    ResolverCache *cache;
+};
+
+TEST_F(NegativeCacheTest, testNXDOMAIN){
+    // NXDOMAIN response for nonexist.example.com
+    Message msg_nxdomain(Message::PARSE);
+    messageFromFile(msg_nxdomain, "message_nxdomain_with_soa.wire");
+    cache->update(msg_nxdomain);
+
+    msg_nxdomain.makeResponse();
+
+    Name non_exist_qname("nonexist.example.com.");
+    EXPECT_TRUE(cache->lookup(non_exist_qname, RRType::A(), RRClass::IN(), msg_nxdomain));
+
+    RRsetIterator iter = msg_nxdomain.beginSection(Message::SECTION_AUTHORITY);
+    RRsetPtr rrset_ptr = *iter;
+
+    // The TTL should equal to the TTL of SOA record
+    const RRTTL& nxdomain_ttl1 = rrset_ptr->getTTL();
+    EXPECT_EQ(nxdomain_ttl1.getValue(), 86400);
+
+    // SOA response for example.com
+    Message msg_example_com_soa(Message::PARSE);
+    messageFromFile(msg_example_com_soa, "message_example_com_soa.wire");
+    cache->update(msg_example_com_soa);
+
+    msg_example_com_soa.makeResponse();
+    Name soa_qname("example.com.");
+    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(), msg_example_com_soa));
+
+    iter = msg_example_com_soa.beginSection(Message::SECTION_ANSWER);
+    rrset_ptr = *iter;
+
+    // The TTL should equal to the TTL of SOA record in answer section
+    const RRTTL& soa_ttl = rrset_ptr->getTTL();
+    EXPECT_EQ(soa_ttl.getValue(), 172800);
+
+    sleep(1);
+
+    // Query nonexist.example.com again
+    Message msg_nxdomain2(Message::PARSE);
+    messageFromFile(msg_nxdomain2, "message_nxdomain_with_soa.wire");
+    msg_nxdomain2.makeResponse();
+
+    EXPECT_TRUE(cache->lookup(non_exist_qname, RRType::A(), RRClass::IN(), msg_nxdomain2));
+    iter = msg_nxdomain2.beginSection(Message::SECTION_AUTHORITY);
+    rrset_ptr = *iter;
+
+    // The TTL should equal to the TTL of negative response SOA record
+    const RRTTL& nxdomain_ttl2 = rrset_ptr->getTTL();
+    EXPECT_TRUE(86398 <= nxdomain_ttl2.getValue() && nxdomain_ttl2.getValue() <= 86399);
+    // No RRset in ANSWER section
+    EXPECT_TRUE(msg_nxdomain2.getRRCount(Message::SECTION_ANSWER) == 0);
+    // Check that only one SOA record exist in AUTHORITY section
+    EXPECT_TRUE(msg_nxdomain2.getRRCount(Message::SECTION_AUTHORITY) == 1);
+    iter = msg_nxdomain2.beginSection(Message::SECTION_AUTHORITY);
+    rrset_ptr = *iter;
+    EXPECT_TRUE(rrset_ptr->getType() == RRType::SOA());
+
+    // Check the normal SOA cache again
+    Message msg_example_com_soa2(Message::PARSE);
+    messageFromFile(msg_example_com_soa2, "message_example_com_soa.wire");
+    msg_example_com_soa2.makeResponse();
+    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(), msg_example_com_soa2));
+
+    iter = msg_example_com_soa2.beginSection(Message::SECTION_ANSWER);
+    rrset_ptr = *iter;
+    const RRTTL& soa_ttl2 = rrset_ptr->getTTL();
+    // The TTL should equal to the TTL of SOA record in answer section
+    EXPECT_TRUE(172798 <= soa_ttl2.getValue() && soa_ttl2.getValue() <= 172799);
+}
+
+TEST_F(NegativeCacheTest, testNXDOMAINWithoutSOA){
+    // NXDOMAIN response for nonexist.example.com
+    Message msg_nxdomain(Message::PARSE);
+    messageFromFile(msg_nxdomain, "message_nxdomain_no_soa.wire");
+    cache->update(msg_nxdomain);
+
+    msg_nxdomain.makeResponse();
+
+    Name non_exist_qname("nonexist.example.com.");
+    // The message should not be cached
+    EXPECT_FALSE(cache->lookup(non_exist_qname, RRType::A(), RRClass::IN(), msg_nxdomain));
+}
+
+TEST_F(NegativeCacheTest, testNXDOMAINCname){
+    // a.example.org points to b.example.org
+    // b.example.org points to c.example.org
+    // c.example.org does not exist
+    Message msg_nxdomain_cname(Message::PARSE);
+    messageFromFile(msg_nxdomain_cname, "message_nxdomain_cname.wire");
+    cache->update(msg_nxdomain_cname);
+
+    msg_nxdomain_cname.makeResponse();
+
+    Name a_example_org("a.example.org.");
+    // The message should be cached
+    EXPECT_TRUE(cache->lookup(a_example_org, RRType::A(), RRClass::IN(), msg_nxdomain_cname));
+
+    EXPECT_EQ(msg_nxdomain_cname.getRcode().getCode(), Rcode::NXDOMAIN().getCode());
+
+    // It should include 2 CNAME records in Answer section
+    EXPECT_TRUE(msg_nxdomain_cname.getRRCount(Message::SECTION_ANSWER) == 2);
+    RRsetIterator iter = msg_nxdomain_cname.beginSection(Message::SECTION_ANSWER);
+    EXPECT_TRUE((*iter)->getType() == RRType::CNAME());
+    ++iter;
+    EXPECT_TRUE((*iter)->getType() == RRType::CNAME());
+
+    // It should include 1 SOA record in Authority section
+    EXPECT_TRUE(msg_nxdomain_cname.getRRCount(Message::SECTION_AUTHORITY) == 1);
+    iter = msg_nxdomain_cname.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_TRUE((*iter)->getType() == RRType::SOA());
+
+    const RRTTL& soa_ttl = (*iter)->getTTL();
+    EXPECT_EQ(soa_ttl.getValue(), 600);
+}
+
+TEST_F(NegativeCacheTest, testNoerrorNodata){
+    // NODATA/NOERROR response for MX type query of example.com
+    Message msg_nodata(Message::PARSE);
+    messageFromFile(msg_nodata, "message_nodata_with_soa.wire");
+    cache->update(msg_nodata);
+
+    msg_nodata.makeResponse();
+
+    Name example_dot_com("example.com.");
+    EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(), msg_nodata));
+
+    RRsetIterator iter = msg_nodata.beginSection(Message::SECTION_AUTHORITY);
+    RRsetPtr rrset_ptr = *iter;
+
+    // The TTL should equal to the TTL of SOA record
+    const RRTTL& nodata_ttl1 = rrset_ptr->getTTL();
+    EXPECT_EQ(nodata_ttl1.getValue(), 86400);
+
+
+    // Normal SOA response for example.com
+    Message msg_example_com_soa(Message::PARSE);
+    messageFromFile(msg_example_com_soa, "message_example_com_soa.wire");
+    cache->update(msg_example_com_soa);
+
+    msg_example_com_soa.makeResponse();
+    Name soa_qname("example.com.");
+    EXPECT_TRUE(cache->lookup(soa_qname, RRType::SOA(), RRClass::IN(), msg_example_com_soa));
+
+    iter = msg_example_com_soa.beginSection(Message::SECTION_ANSWER);
+    rrset_ptr = *iter;
+
+    // The TTL should equal to the TTL of SOA record in answer section
+    const RRTTL& soa_ttl = rrset_ptr->getTTL();
+    EXPECT_EQ(soa_ttl.getValue(), 172800);
+
+    // Query MX record of example.com again
+    Message msg_nodata2(Message::PARSE);
+    messageFromFile(msg_nodata2, "message_nodata_with_soa.wire");
+    msg_nodata2.makeResponse();
+
+    sleep(1);
+
+    EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(), msg_nodata2));
+
+    // No answer
+    EXPECT_EQ(msg_nodata2.getRRCount(Message::SECTION_ANSWER), 0);
+    // One SOA record in authority section
+    EXPECT_EQ(msg_nodata2.getRRCount(Message::SECTION_AUTHORITY), 1);
+
+    iter = msg_nodata2.beginSection(Message::SECTION_AUTHORITY);
+    rrset_ptr = *iter;
+
+    // The TTL should equal to the TTL of negative response SOA record and counted down
+    const RRTTL& nodata_ttl2 = rrset_ptr->getTTL();
+    EXPECT_TRUE(86398 <= nodata_ttl2.getValue() && nodata_ttl2.getValue() <= 86399);
+}
+
+TEST_F(NegativeCacheTest, testReferralResponse){
+    // CNAME exist, but it points to out of zone data, so the server give some reference data
+    Message msg_cname_referral(Message::PARSE);
+    messageFromFile(msg_cname_referral, "message_cname_referral.wire");
+    cache->update(msg_cname_referral);
+
+    msg_cname_referral.makeResponse();
+
+    Name x_example_org("x.example.org.");
+    EXPECT_TRUE(cache->lookup(x_example_org, RRType::A(), RRClass::IN(), msg_cname_referral));
+
+    // The Rcode should be NOERROR
+    EXPECT_EQ(msg_cname_referral.getRcode().getCode(), Rcode::NOERROR().getCode());
+
+    // One CNAME record in Answer section
+    EXPECT_EQ(msg_cname_referral.getRRCount(Message::SECTION_ANSWER), 1);
+    RRsetIterator iter = msg_cname_referral.beginSection(Message::SECTION_ANSWER);
+    EXPECT_EQ((*iter)->getType(), RRType::CNAME());
+
+    // 13 NS records in Authority section
+    EXPECT_EQ(msg_cname_referral.getRRCount(Message::SECTION_AUTHORITY), 13);
+    iter = msg_cname_referral.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_EQ((*iter)->getType(), RRType::NS());
+}
+
+}

src/lib/cache/tests/resolver_cache_unittest.cc

@@ -53,7 +53,7 @@ TEST_F(ResolverCacheTest, testUpdateMessage) {
 
     msg.makeResponse();
     EXPECT_TRUE(cache->lookup(qname, RRType::SOA(), RRClass::IN(), msg));
-    EXPECT_TRUE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
 
     // Test whether the old message can be updated
     Message new_msg(Message::PARSE);

src/lib/cache/tests/testdata/message_cname_referral.wire

@@ -0,0 +1,56 @@
+#
+# Request A record for x.example.org, the CNAME record exist for x.example.org
+# it poinst to x.example.net, but the server has no idea whether x.example.net exist
+# so it give some NS records for reference
+#
+# Transaction ID: 0xaf71
+# Flags: 0x8480 (Standard query response, No error)
+af71 8480
+# Questions: 1
+# Answer RRs: 1
+# Authority RRs: 13
+# Additional RRs: 0
+00 01 00 01 00 0d 00 00
+##
+## query
+##
+# x.example.org: type A, class IN
+##
+## Answer
+##
+# x.example.org: type CNAME, class IN, cname x.example.net
+# TTL: 360s
+01 78 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 00 01 00 01
+c0 0c 00 05 00 01 00 00 0e 10 00 0f 01 78 07 65 78
+61 6d 70 6c 65 03 6e 65 74 00
+##
+## Authority
+##
+# TTL:518400
+# <Root>: type NS, class IN, ns G.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns E.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns J.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns L.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns H.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns I.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns K.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns M.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns F.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns B.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns C.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns D.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns A.ROOT-SERVERS.net
+00 00 02 00 01 00
+07 e9 00 00 11 01 47 0c 52 4f 4f 54 2d 53 45 52
+56 45 52 53 c0 35 00 00 02 00 01 00 07 e9 00 00
+04 01 45 c0 47 00 00 02 00 01 00 07 e9 00 00 04
+01 4a c0 47 00 00 02 00 01 00 07 e9 00 00 04 01
+4c c0 47 00 00 02 00 01 00 07 e9 00 00 04 01 48
+c0 47 00 00 02 00 01 00 07 e9 00 00 04 01 49 c0
+47 00 00 02 00 01 00 07 e9 00 00 04 01 4b c0 47
+00 00 02 00 01 00 07 e9 00 00 04 01 4d c0 47 00
+00 02 00 01 00 07 e9 00 00 04 01 46 c0 47 00 00
+02 00 01 00 07 e9 00 00 04 01 42 c0 47 00 00 02
+00 01 00 07 e9 00 00 04 01 43 c0 47 00 00 02 00
+01 00 07 e9 00 00 04 01 44 c0 47 00 00 02 00 01
+00 07 e9 00 00 04 01 41 c0 47

src/lib/cache/tests/testdata/message_example_com_soa.wire

@@ -0,0 +1,57 @@
+#
+# SOA request response for example.com 
+#
+# Transaction ID: 0x7f36
+# Flags: 0x8400 (Standard query response, No error)
+7f 36 84 00
+# Questions: 1
+00 01
+# Answer RRs: 1
+00 01
+# Authority RRs: 2
+00 02
+# Additional RRs: 0
+00 00
+##
+## Query
+##
+# Name: example.com
+07 65 78 61 6d 70 6c 65 03 63 6f 6d 00
+# Type: SOA (Start of zone of authority)
+00 06
+# Class: IN (0x0001)
+00 01
+##
+## Answers
+##
+# Name: example.com
+c0 0c
+# Type: SOA (Start of zone of authority)
+00 06
+# Class: IN (0x0001)
+00 01
+# Time to live: 2 days (172800s)
+00 02 a3 00
+# Data length: 49
+00 31
+# Primary name server: dns1.icann.org
+04 64 6e 73 31 05 69 63 61 6e 6e 03 6f 72 67 00
+# Responsible authority's mailbox: hostmaster.icann.org
+0a 68 6f 73 74 6d 61 73 74 65 72 c0 2e
+# Serial number: 2010072301
+77 cf 44 ed
+# Refresh interval: 2 hours
+00 00 1c 20
+# Retry interval: 1 hour
+00 00 0e 10
+# Expiration limit: 14 days
+00 12 75 00
+# Minimum TTL: 1 day
+00 01 51 80
+##
+## Authoritative nameservers
+##
+# example.com: type NS, class IN, ns a.iana-servers.net
+c0 0c 00 02 00 01 00 02 a3 00 00 14 01 61 0c 69 61 6e 61 2d 73 65 72 76 65 72 73 03 6e 65 74 00
+# example.com: type NS, class IN, ns b.iana-servers.net
+c0 0c 00 02 00 01 00 02 a3 00 00 04 01 62 c0 68

src/lib/cache/tests/testdata/message_large_ttl.wire

@@ -0,0 +1,31 @@
+#
+# A response that the TTL is quite large(> 7days)
+#
+##
+## header
+##
+# Transaction ID: 0x0d1f
+# Flags: 0x8580 (Standard query response, No error)
+0d1f 8580
+# Questions: 1
+# Answer RRs: 1
+# Authority RRs: 3
+# Additional RRs: 3
+00 01 00 01 00 01 00 00
+##
+## Query
+##
+# test.example.org: type A, class IN
+04 74 65 73 74 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 00 01 00 01
+##
+## Answer
+##
+# test.example.org: type A, class IN, addr 127.0.0.1
+# TTL: 7 days, 1 second (604801 seconds)
+c0 0c 00 01 00 01 00 09 3a 81 00 04 7f 00 00 01
+##
+## Authority
+##
+# example.org: type NS, class IN, ns ns1.example.org
+# TTL: 7 days, 1 second (604801 seconds)
+c0 11 00 02 00 01 00 09 3a 81 00 06 03 6e 73 31 c0 11

src/lib/cache/tests/testdata/message_nodata_with_soa.wire

@@ -0,0 +1,32 @@
+#
+# NOERROR/NODATA response with SOA record
+#
+##
+## header
+##
+#Transaction ID: 0x0284
+#Flags: 0x8500 (Standard query response, No error)
+0284 8500
+#Question:1
+00 01
+#Answer RRs:0
+00 00
+#Authority RRs:1
+00 01
+#Additional RRs:0
+00 00
+##
+## Queries
+##
+# example.com: type MX, class IN
+07 65 78 61 6d 70 6c 65 03 63 6f 6d 00 00 0f 00 01
+##
+## Authoritative nameservers
+##
+# example.com: type SOA, class IN, mname dns1.icann.org
+# TTL:86400
+c0 0c 00
+06 00 01 00 01 51 80 00 31 04 64 6e 73 31 05 69
+63 61 6e 6e 03 6f 72 67 00 0a 68 6f 73 74 6d 61
+73 74 65 72 c0 2e 77 cf 44 ed 00 00 1c 20 00 00
+0e 10 00 12 75 00 00 01 51 80

src/lib/cache/tests/testdata/message_nxdomain_cname.wire

@@ -0,0 +1,36 @@
+#
+# NXDOMAIN response
+# The cname type of a.example.org exist, it points to b.example.org
+# b.example.org points to c.example.org
+# but c.example.org does not exist
+#
+##
+## header
+##
+# Transaction ID: 0xc2aa
+# Flags: 0x8583 (Standard query response, No such name)
+c2aa 8583
+# Questions: 1
+# Answer RRs: 2
+# Authority RRs: 1
+# dditional RRs: 0
+00 01 00 02 00 01 00 00
+##
+## Queries
+##
+# a.example.org: type A, class IN
+01 61 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 00 01 00 01
+##
+## Answers
+##
+# a.example.org: type CNAME, class IN, cname b.example.org
+c0 0c 00 05 00 01 00 00 0e 10 00 04 01 62 c0 0e
+# b.example.org: type CNAME, class IN, cname c.example.org
+c0 2b 00 05 00 01 00 00 0e 10 00 04 01 63 c0 0e
+##
+## Authority
+##
+# example.org: type SOA, class IN, mname ns1.example.org
+c0 0e 00 06 00 01 00 00 02 58 00 22 03 6e 73 31 c0
+0e 05 61 64 6d 69 6e c0 0e 00 00 04 d2 00 00 0e
+10 00 00 07 08 00 24 ea 00 00 00 02 58

src/lib/cache/tests/testdata/message_nxdomain_large_ttl.wire

@@ -0,0 +1,25 @@
+#
+# Negative response (NXDOMAIN) with large TTL (3hours + 1second)
+#
+##
+## Header
+##
+# Transaction ID: 0xb1fe
+# Flags: 0x8583 (Standard query response, No such name)
+b1fe 8583
+# Questions: 1
+# Authority RRs: 1
+00 01 00 00 00 01 00 00
+##
+## Query
+##
+# c.example.org: type A, class IN
+01 63 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 00 01 00 01
+##
+## Authority
+##
+# example.org: type SOA, class IN, mname ns1.example.org
+# TTL: 3 Hourse, 1 second (10801seconds)
+c0 0e 00 06 00 01 00 00 2a 31 00 22 03 6e 73 31 c0
+0e 05 61 64 6d 69 6e c0 0e 00 00 04 d2 00 00 0e
+10 00 00 07 08 00 24 ea 00 00 00 2a 31

src/lib/cache/tests/testdata/message_nxdomain_no_soa.wire

@@ -0,0 +1,26 @@
+#
+# NXDOMAIN response with SOA record
+#
+##
+## Header
+##
+# ID = 0x3da0
+# QR = 1 (response), Opcode = 0, AA = 1, RCODE=3 (NXDOMAIN)
+3da0 8403
+# Question : 1
+00 01
+# Answer : 0
+00 00
+# Authority : 0
+00 00
+# Additional : 0
+00 00
+##
+## Query
+##
+#(4) n  o  n  e  x  i  s  t (7) e  x  a  m  p  l  e (3) c  o  m (0)
+  08 6e 6f 6e 65 78 69 73 74 07 65 78 61 6d 70 6c 65 03 63 6f 6d 00
+# Type:A
+00 01
+# class: IN
+00 01

src/lib/cache/tests/testdata/message_nxdomain_with_soa.wire

@@ -0,0 +1,55 @@
+#
+# NXDOMAIN response with SOA record
+#
+##
+## Header
+##
+# ID = 0x3da0
+# QR = 1 (response), Opcode = 0, AA = 1, RCODE=3 (NXDOMAIN)
+3da0 8403
+# Question : 1
+00 01
+# Answer : 0
+00 00
+# Authority : 1
+00 01
+# Additional : 0
+00 00
+##
+## Query
+##
+#(4) n  o  n  e  x  i  s  t (7) e  x  a  m  p  l  e (3) c  o  m (0)
+  08 6e 6f 6e 65 78 69 73 74 07 65 78 61 6d 70 6c 65 03 63 6f 6d 00
+# Type:A
+00 01
+# class: IN
+00 01
+##
+## Authority
+## 
+# name: example.com
+c0 15
+# Type:SOA
+00 06
+# Class: IN
+00 01
+# TTL: 86400
+00 01 51 80
+# Data Length: 49
+00 31
+# Name Server:
+#(4) d  n  s  1 (5) i   c a  n  n (3) o  r  g (0)
+  04 64 6e 73 31 05 69 63 61 6e 6e 03 6f 72 67 00
+# MX: 
+# (10) h  o   s  t  m  a  s  t  e  r .icann.org.
+  0a   68 6f 73 74 6d 61 73 74 65 72 c0 37
+# Serial Number:2010072301
+77 cf 44 ed
+# Refresh Interval:2 hours
+00 00 1c 20
+# Retry Interval: 1 hour
+00 00 0e 10
+# Expiration: 14 days
+00 12 75 00
+# Minimum TTL 1 day
+00 01 51 80

src/lib/cache/tests/testdata/message_referral.wire

@@ -0,0 +1,36 @@
+#
+# Query x.example.net to nameservr of example.org
+# It will just give some referral info
+#
+#
+# Transaction ID: 0x8b61
+# Flags: 0x8080 (Standard query response, No error)
+8b61 8080
+# Questions: 1
+# Authority RRs: 13
+00 01 00 00 00 0d 00 00
+##
+## Query
+##
+# x.example.net: type A, class IN
+01 78 07 65 78 61 6d 70 6c 65 03 6e 65 74 00 00 01 00 01
+##
+## Authority
+##
+# <Root>: type NS, class IN, ns B.ROOT-SERVERS.net
+# <Root>: type NS, class IN, ns M.ROOT-SERVERS.net
+# ...
+# <Root>: type NS, class IN, ns H.ROOT-SERVERS.net
+00 00 02 00 01 00 07 e9 00 00 11 01 42 0c 52 4f 4f
+54 2d 53 45 52 56 45 52 53 c0 16 00 00 02 00 01
+00 07 e9 00 00 04 01 4d c0 2c 00 00 02 00 01 00
+07 e9 00 00 04 01 44 c0 2c 00 00 02 00 01 00 07
+e9 00 00 04 01 4c c0 2c 00 00 02 00 01 00 07 e9
+00 00 04 01 4b c0 2c 00 00 02 00 01 00 07 e9 00
+00 04 01 43 c0 2c 00 00 02 00 01 00 07 e9 00 00
+04 01 41 c0 2c 00 00 02 00 01 00 07 e9 00 00 04
+01 49 c0 2c 00 00 02 00 01 00 07 e9 00 00 04 01
+45 c0 2c 00 00 02 00 01 00 07 e9 00 00 04 01 46
+c0 2c 00 00 02 00 01 00 07 e9 00 00 04 01 4a c0
+2c 00 00 02 00 01 00 07 e9 00 00 04 01 47 c0 2c
+00 00 02 00 01 00 07 e9 00 00 04 01 48 c0 2c

src/lib/cc/data.h

@@ -222,6 +222,7 @@ public:
 
     /// Sets the ElementPtr at the given key
     /// \param name The key of the Element to set
+    /// \param element The ElementPtr to set at the given key.
     virtual void set(const std::string& name, ConstElementPtr element);
 
     /// Remove the ElementPtr at the given key
@@ -315,10 +316,11 @@ public:
     /// Creates an Element from the given input stream, where we keep
     /// track of the location in the stream for error reporting.
     ///
-    /// \param in The string to parse the element from
+    /// \param in The string to parse the element from.
+    /// \param file The input file name.
     /// \param line A reference to the int where the function keeps
     /// track of the current line.
-    /// \param line A reference to the int where the function keeps
+    /// \param pos A reference to the int where the function keeps
     /// track of the current position within the current line.
     /// \return An ElementPtr that contains the element(s) specified
     /// in the given input stream.
@@ -548,18 +550,18 @@ void merge(ElementPtr element, ConstElementPtr other);
 ///
 /// \brief Insert the Element as a string into stream.
 ///
-/// This method converts the \c ElemetPtr into a string with
+/// This method converts the \c ElementPtr into a string with
 /// \c Element::str() and inserts it into the
 /// output stream \c out.
 ///
 /// This function overloads the global operator<< to behave as described in
 /// ostream::operator<< but applied to \c ElementPtr objects.
 ///
-/// \param os A \c std::ostream object on which the insertion operation is
+/// \param out A \c std::ostream object on which the insertion operation is
 /// performed.
 /// \param e The \c ElementPtr object to insert.
 /// \return A reference to the same \c std::ostream object referenced by
-/// parameter \c os after the insertion operation.
+/// parameter \c out after the insertion operation.
 std::ostream& operator<<(std::ostream& out, const Element& e);
 
 bool operator==(const Element& a, const Element& b);

src/lib/cc/session.h

@@ -99,7 +99,7 @@ namespace isc {
             /// \brief Sets the default timeout for blocking reads
             ///        in this session to the given number of milliseconds
             /// \param milliseconds the timeout for blocking reads in
-            ///        milliseconds, if this is set to 0, reads will block
+            ///        milliseconds; if this is set to 0, reads will block
             ///        forever.
             virtual void setTimeout(size_t milliseconds) = 0;
 

src/lib/config/module_spec.h

@@ -53,6 +53,8 @@ namespace isc { namespace config {
         /// Create a \c ModuleSpec instance with the given data as
         /// the specification
         /// \param e The Element containing the data specification
+        /// \param check If false, the module specification in the file
+        /// is not checked to be of the correct form.
         explicit ModuleSpec(isc::data::ConstElementPtr e,
                             const bool check = true)
             throw(ModuleSpecError);
@@ -86,6 +88,8 @@ namespace isc { namespace config {
         // configuration specification
         /// Validates the given configuration data for this specification.
         /// \param data The base \c Element of the data to check
+        /// \param full If true, all non-optional configuration parameters
+        /// must be specified.
         /// \return true if the data conforms to the specification,
         /// false otherwise.
         bool validateConfig(isc::data::ConstElementPtr data,

src/lib/datasrc/data_source.cc

@@ -48,6 +48,28 @@ using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
+namespace {
+
+struct MatchRRsetForType {
+    MatchRRsetForType(const RRType rrtype) : rrtype_(rrtype) {}
+    bool operator()(RRsetPtr rrset) {
+        return (rrset->getType() == rrtype_);
+    }
+    const RRType rrtype_;
+};
+
+// This is a helper to retrieve a specified RR type of RRset from RRsetList.
+// In our case the data source search logic should ensure that the class is
+// valid.  We use this find logic of our own so that we can support both
+// specific RR class queries (normal case) and class ANY queries.
+RRsetPtr
+findRRsetFromList(RRsetList& list, const RRType rrtype) {
+    RRsetList::iterator it(find_if(list.begin(), list.end(),
+                                   MatchRRsetForType(rrtype)));
+    return (it != list.end() ? *it : RRsetPtr());
+}
+}
+
 namespace isc {
 namespace datasrc {
 
@@ -129,7 +151,7 @@ synthesizeCname(QueryTaskPtr task, RRsetPtr rrset, RRsetList& target) {
     const generic::DNAME& dname = dynamic_cast<const generic::DNAME&>(rd);
     const Name& dname_target(dname.getDname());
 
-    RRsetPtr cname(new RRset(task->qname, task->qclass, RRType::CNAME(),
+    RRsetPtr cname(new RRset(task->qname, rrset->getClass(), RRType::CNAME(),
                              rrset->getTTL()));
 
     const int qnlen = task->qname.getLabelCount();
@@ -569,17 +591,17 @@ hasDelegation(Query& q, QueryTaskPtr task, ZoneInfo& zoneinfo) {
         // Found a referral while getting answer data;
         // send a delegation.
         if (found) {
-            RRsetPtr r = ref.findRRset(RRType::DNAME(), q.qclass());
+            RRsetPtr r = findRRsetFromList(ref, RRType::DNAME());
             if (r != NULL) {
                 RRsetList syn;
                 addToMessage(q, Message::SECTION_ANSWER, r);
                 q.message().setHeaderFlag(Message::HEADERFLAG_AA);
                 synthesizeCname(task, r, syn);
                 if (syn.size() == 1) {
-                    addToMessage(q, Message::SECTION_ANSWER,
-                                 syn.findRRset(RRType::CNAME(), q.qclass()));
-                    chaseCname(q, task, syn.findRRset(RRType::CNAME(),
-                                                      q.qclass()));
+                    RRsetPtr cname_rrset = findRRsetFromList(syn,
+                                                             RRType::CNAME());
+                    addToMessage(q, Message::SECTION_ANSWER, cname_rrset);
+                    chaseCname(q, task, cname_rrset);
                     return (true);
                 }
             }
@@ -612,7 +634,7 @@ addSOA(Query& q, ZoneInfo& zoneinfo) {
     }
 
     addToMessage(q, Message::SECTION_AUTHORITY,
-                 soa.findRRset(RRType::SOA(), q.qclass()));
+                 findRRsetFromList(soa, RRType::SOA()));
     return (DataSrc::SUCCESS);
 }
 
@@ -624,7 +646,7 @@ addNSEC(Query& q, const Name& name, ZoneInfo& zoneinfo) {
     RETERR(doQueryTask(newtask, zoneinfo, nsec));
     if (newtask.flags == 0) {
         addToMessage(q, Message::SECTION_AUTHORITY,
-                     nsec.findRRset(RRType::NSEC(), q.qclass()));
+                     findRRsetFromList(nsec, RRType::NSEC()));
     }
 
     return (DataSrc::SUCCESS);
@@ -828,7 +850,7 @@ tryWildcard(Query& q, QueryTaskPtr task, ZoneInfo& zoneinfo, bool& found) {
         // match the qname), and then continue as if this were a normal
         // answer: if a CNAME, chase the target, otherwise add authority.
         if (cname) {
-            RRsetPtr rrset = wild.findRRset(RRType::CNAME(), q.qclass());
+            RRsetPtr rrset = findRRsetFromList(wild, RRType::CNAME());
             if (rrset != NULL) {
                 rrset->setName(task->qname);
                 addToMessage(q, Message::SECTION_ANSWER, rrset);
@@ -923,7 +945,7 @@ DataSrc::doQuery(Query& q) {
              ((task->qtype == RRType::NSEC() ||
                task->qtype == RRType::DS() ||
                task->qtype == RRType::DNAME()) &&
-              data.findRRset(task->qtype, task->qclass)))) {
+              findRRsetFromList(data, task->qtype)))) {
             task->flags &= ~REFERRAL;
         }
 
@@ -948,9 +970,8 @@ DataSrc::doQuery(Query& q) {
                     // Add the NS records for the enclosing zone to
                     // the authority section.
                     RRsetList auth;
-                    const DataSrc* ds = zoneinfo.getDataSource();
-                    if (!refQuery(q, Name(*zonename), zoneinfo, auth)  ||
-                        !auth.findRRset(RRType::NS(), ds->getClass())) {
+                    if (!refQuery(q, Name(*zonename), zoneinfo, auth) ||
+                        !findRRsetFromList(auth, RRType::NS())) {
                         isc_throw(DataSourceError,
                                   "NS RR not found in " << *zonename << "/" <<
                                   q.qclass());
@@ -983,7 +1004,7 @@ DataSrc::doQuery(Query& q) {
         } else if ((task->flags & CNAME_FOUND) != 0) {
             // The qname node contains a CNAME.  Add a new task to the
             // queue to look up its target.
-            RRsetPtr rrset = data.findRRset(RRType::CNAME(), q.qclass());
+            RRsetPtr rrset = findRRsetFromList(data, RRType::CNAME());
             if (rrset != NULL) {
                 addToMessage(q, task->section, rrset);
                 chaseCname(q, task, rrset);
@@ -1013,6 +1034,13 @@ DataSrc::doQuery(Query& q) {
             continue;
         } else if ((task->flags & (NAME_NOT_FOUND|TYPE_NOT_FOUND)) != 0) {
             // No data found at this qname/qtype.
+
+            // If we were looking for additional data, we should simply
+            // ignore this result.
+            if (task->state == QueryTask::GETADDITIONAL) {
+                continue;
+            }
+
             // If we were looking for answer data, not additional,
             // and the name was not found, we need to find out whether
             // there are any relevant wildcards.

src/lib/datasrc/memory_datasrc.h

@@ -289,7 +289,7 @@ public:
     ///   - \c result::PARTIALMATCH: A zone whose origin is a
     //    super domain of \c name is found (but there is no exact match)
     ///   - \c result::NOTFOUND: For all other cases.
-    /// - \c zone: A <Boost> shared pointer to the found \c Zone object if one
+    /// - \c zone: A "Boost" shared pointer to the found \c Zone object if one
     //  is found; otherwise \c NULL.
     ///
     /// This method never throws an exception.

src/lib/datasrc/tests/datasrc_unittest.cc

@@ -70,7 +70,7 @@ protected:
     }
     void QueryCommon(const RRClass& qclass);
     void createAndProcessQuery(const Name& qname, const RRClass& qclass,
-                               const RRType& qtype);
+                               const RRType& qtype, bool need_dnssec);
 
     HotCache cache;
     MetaDataSrc meta_source;
@@ -82,23 +82,26 @@ protected:
 };
 
 void
-performQuery(DataSrc& data_source, HotCache& cache, Message& message) {
+performQuery(DataSrc& data_source, HotCache& cache, Message& message,
+             bool need_dnssec = true)
+{
     message.setHeaderFlag(Message::HEADERFLAG_AA);
     message.setRcode(Rcode::NOERROR());
-    Query q(message, cache, true);
+    Query q(message, cache, need_dnssec);
     data_source.doQuery(q);
 }
 
 void
 DataSrcTest::createAndProcessQuery(const Name& qname, const RRClass& qclass,
-                                   const RRType& qtype)
+                                   const RRType& qtype,
+                                   bool need_dnssec = true)
 {
     msg.makeResponse();
     msg.setOpcode(Opcode::QUERY());
     msg.addQuestion(Question(qname, qclass, qtype));
     msg.setHeaderFlag(Message::HEADERFLAG_RD);
     qid = msg.getQid();
-    performQuery(meta_source, cache, msg);
+    performQuery(meta_source, cache, msg, need_dnssec);
 }
 
 void
@@ -165,6 +168,59 @@ TEST_F(DataSrcTest, QueryClassAny) {
     QueryCommon(RRClass::ANY());
 }
 
+TEST_F(DataSrcTest, queryClassAnyNegative) {
+    // There was a bug where Class ANY query triggered a crash due to NULL
+    // pointer dereference.  This test checks that condition.
+
+    // NXDOMAIN case
+    createAndProcessQuery(Name("notexistent.example.com"), RRClass::ANY(),
+                          RRType::A());
+    headerCheck(msg, qid, Rcode::NXDOMAIN(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 0, 6, 0);
+
+    // NXRRSET case
+    msg.clear(Message::PARSE);
+    createAndProcessQuery(Name("www.example.com"), RRClass::ANY(),
+                          RRType::TXT());
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 0, 4, 0);
+}
+
+TEST_F(DataSrcTest, queryClassAnyDNAME) {
+    // Class ANY query that would match a DNAME.  Everything including the
+    // synthesized CNAME should be the same as the response to class IN query.
+    createAndProcessQuery(Name("www.dname.example.com"), RRClass::ANY(),
+                          RRType::A(), false);
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 3, 3, 3);
+    rrsetsCheck("dname.example.com. 3600 IN DNAME sql1.example.com.\n"
+                "www.dname.example.com. 3600 IN CNAME www.sql1.example.com.\n"
+                "www.sql1.example.com. 3600 IN A 192.0.2.2\n",
+                msg.beginSection(Message::SECTION_ANSWER),
+                msg.endSection(Message::SECTION_ANSWER));
+
+    // Also check the case of explicit DNAME query.
+    msg.clear(Message::PARSE);
+    createAndProcessQuery(Name("dname.example.com"), RRClass::ANY(),
+                          RRType::DNAME(), false);
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 1, 3, 3);
+    rrsetsCheck("dname.example.com. 3600 IN DNAME sql1.example.com.\n",
+                msg.beginSection(Message::SECTION_ANSWER),
+                msg.endSection(Message::SECTION_ANSWER));
+}
+
+TEST_F(DataSrcTest, queryClassAnyCNAME) {
+    // Similar test for CNAME
+    createAndProcessQuery(Name("foo.example.com"), RRClass::ANY(),
+                          RRType::A(), false);
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | AA_FLAG | RD_FLAG, 1, 1, 0, 0);
+    rrsetsCheck("foo.example.com. 3600 IN CNAME cnametest.example.net.\n",
+                msg.beginSection(Message::SECTION_ANSWER),
+                msg.endSection(Message::SECTION_ANSWER));
+}
+
 TEST_F(DataSrcTest, NSQuery) {
     createAndProcessQuery(Name("example.com"), RRClass::IN(),
                           RRType::NS());
@@ -416,68 +472,36 @@ TEST_F(DataSrcTest, DISABLED_WildcardAgainstMultiLabel) {
 
 TEST_F(DataSrcTest, WildcardCname) {
     // Check that wildcard answers containing CNAMES are followed
-    // correctly
-    createAndProcessQuery(Name("www.wild2.example.com"), RRClass::IN(),
-                          RRType::A());
-
-    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
-                QR_FLAG | AA_FLAG | RD_FLAG, 1, 4, 6, 6);
-
-    RRsetIterator rit = msg.beginSection(Message::SECTION_ANSWER);
-    RRsetPtr rrset = *rit;
-    EXPECT_EQ(Name("www.wild2.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::CNAME(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    RdataIteratorPtr it = rrset->getRdataIterator();
-    EXPECT_EQ("www.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
-
-    ++rit;
-    ++rit;
-    rrset = *rit;
-    EXPECT_EQ(Name("www.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::A(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    it = rrset->getRdataIterator();
-    EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
-
-    rit = msg.beginSection(Message::SECTION_AUTHORITY);
-    rrset = *rit;
-    EXPECT_EQ(Name("*.wild2.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::NSEC(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-    ++rit;
-    ++rit;
-
-    rrset = *rit;
-    EXPECT_EQ(Name("example.com"), rrset->getName());
-    EXPECT_EQ(RRType::NS(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    it = rrset->getRdataIterator();
-    EXPECT_EQ("dns01.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_EQ("dns02.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_EQ("dns03.example.com.", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
-
-    rit = msg.beginSection(Message::SECTION_ADDITIONAL);
-    rrset = *rit;
-    EXPECT_EQ(Name("dns01.example.com"), rrset->getName());
-    EXPECT_EQ(RRType::A(), rrset->getType());
-    EXPECT_EQ(RRClass::IN(), rrset->getClass());
-
-    it = rrset->getRdataIterator();
-    EXPECT_EQ("192.0.2.1", it->getCurrent().toText());
-    it->next();
-    EXPECT_TRUE(it->isLast());
+    // correctly.  It should result in the same response for both
+    // class IN and ANY queries.
+    const RRClass classes[2] = { RRClass::IN(), RRClass::ANY() };
+
+    for (int i = 0; i < sizeof(classes) / sizeof(classes[0]); ++i) {
+        SCOPED_TRACE("Wildcard + CNAME test for class " + classes[i].toText());
+
+        msg.clear(Message::PARSE);
+
+        createAndProcessQuery(Name("www.wild2.example.com"), classes[i],
+                              RRType::A(), false);
+
+        headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                    QR_FLAG | AA_FLAG | RD_FLAG, 1, 2, 3, 3);
+
+        rrsetsCheck("www.wild2.example.com. 3600 IN CNAME www.example.com\n"
+                    "www.example.com. 3600 IN A 192.0.2.1\n",
+                    msg.beginSection(Message::SECTION_ANSWER),
+                    msg.endSection(Message::SECTION_ANSWER));
+        rrsetsCheck("example.com. 3600 IN NS dns01.example.com.\n"
+                    "example.com. 3600 IN NS dns02.example.com.\n"
+                    "example.com. 3600 IN NS dns03.example.com.",
+                    msg.beginSection(Message::SECTION_AUTHORITY),
+                    msg.endSection(Message::SECTION_AUTHORITY));
+        rrsetsCheck("dns01.example.com. 3600 IN A 192.0.2.1\n"
+                    "dns02.example.com. 3600 IN A 192.0.2.2\n"
+                    "dns03.example.com. 3600 IN A 192.0.2.3",
+                    msg.beginSection(Message::SECTION_ADDITIONAL),
+                    msg.endSection(Message::SECTION_ADDITIONAL));
+    }
 }
 
 TEST_F(DataSrcTest, WildcardCnameNodata) {
@@ -667,7 +691,7 @@ TEST_F(DataSrcTest, Cname) {
     EXPECT_EQ(RRClass::IN(), rrset->getClass());
 
     RdataIteratorPtr it = rrset->getRdataIterator();
-    EXPECT_EQ("cnametest.flame.org.", it->getCurrent().toText());
+    EXPECT_EQ("cnametest.example.net.", it->getCurrent().toText());
     it->next();
     EXPECT_TRUE(it->isLast());
 }
@@ -1035,6 +1059,25 @@ TEST_F(DataSrcTest, apexCNAMEZone) {
                  DataSourceError);
 }
 
+TEST_F(DataSrcTest, incompleteGlue) {
+    // One of the NS names belong to a different zone (which is still
+    // authoritative), and the glue is missing in that zone.  We should
+    // still return the existent glue.
+    // (nons.example is also broken in that it doesn't have apex NS, but
+    // that doesn't matter for this test)
+    createAndProcessQuery(Name("www.incompletechild.nons.example"),
+                          RRClass::IN(), RRType::A());
+    headerCheck(msg, qid, Rcode::NOERROR(), opcodeval,
+                QR_FLAG | RD_FLAG, 1, 0, 2, 1);
+    rrsetsCheck("incompletechild.nons.example. 3600 IN NS ns.incompletechild.nons.example.\n"
+                "incompletechild.nons.example. 3600 IN NS nx.nosoa.example.",
+                msg.beginSection(Message::SECTION_AUTHORITY),
+                msg.endSection(Message::SECTION_AUTHORITY));
+    rrsetsCheck("ns.incompletechild.nons.example. 3600 IN A 192.0.2.1",
+                msg.beginSection(Message::SECTION_ADDITIONAL),
+                msg.endSection(Message::SECTION_ADDITIONAL));
+}
+
 // currently fails
 TEST_F(DataSrcTest, DISABLED_synthesizedCnameTooLong) {
     // qname has the possible max length (255 octets).  it matches a DNAME,

src/lib/datasrc/tests/test_datasrc.cc

@@ -154,7 +154,7 @@ const struct RRData example_com_records[] = {
     {"*.wild3.example.com", "RRSIG", "NSEC 5 3 7200 20100410212307 20100311212307 33495 example.com. EuSzh6or8mbvwru2H7fyYeMpW6J8YZ528rabU38V/lMN0TdamghIuCneAvSNaZgwk2MSN1bWpZqB2kAipaM/ZI9/piLlTvVjjOQ8pjk0auwCEqT7Z7Qng3E92O9yVzO+WHT9QZn/fR6t60392In4IvcBGjZyjzQk8njIwbui xGA="},
 
     // foo.example.com
-    {"foo.example.com", "CNAME", "cnametest.flame.org"},
+    {"foo.example.com", "CNAME", "cnametest.example.net"},
     {"foo.example.com", "RRSIG", "CNAME 5 3 3600 20100322084538 20100220084538 33495 example.com. DSqkLnsh0gCeCPVW/Q8viy9GNP+KHmFGfWqyVG1S6koBtGN/VQQ16M4PHZ9Zssmf/JcDVJNIhAChHPE2WJiaPCNGTprsaUshf1Q2vMPVnkrJKgDY8SVRYMptmT8eaT0gGri4KhqRoFpMT5OYfesybwDgfhFSQQAh6ps3bIUsy4o="},
     {"foo.example.com", "NSEC", "mail.example.com. CNAME RRSIG NSEC"},
     {"foo.example.com", "RRSIG", "NSEC 5 3 7200 20100322084538 20100220084538 33495 example.com. RTQwlSqui6StUYye1KCSOEr1d3irndWFqHBpwP7g7n+w8EDXJ8I7lYgwzHvlQt6BLAxe5fUDi7ct8M5hXvsm7FoWPZ5wXH+2/eJUCYxIw4vezKMkMwBP6M/YkJ2CMqY8DppYf60QaLDONQAr7AcK/naSyioeI5h6eaoVitUDMso="},
@@ -199,6 +199,7 @@ const struct RRData example_com_records[] = {
 
     {NULL, NULL, NULL}
 };
+
 const struct RRData example_com_glue_records[] = {
     {"ns1.subzone.example.com", "A", "192.0.2.1"},
     {"ns2.subzone.example.com", "A", "192.0.2.2"},
@@ -247,6 +248,20 @@ const struct RRData nons_example_records[] = {
      "1234 3600 1800 2419200 7200"},
     {"www.nons.example", "A", "192.0.2.1"},
     {"ns.nons.example", "A", "192.0.2.2"},
+
+    // One of the NS names is intentionally non existent in the zone it belongs
+    // to.  This delegation is used to see if we still return the NS and the
+    // existent glue.
+    // (These are not relevant to test the case for the "no NS" case.  We use
+    // this zone to minimize the number of test zones)
+    {"incompletechild.nons.example", "NS", "ns.incompletechild.nons.example"},
+    {"incompletechild.nons.example", "NS", "nx.nosoa.example"},
+
+    {NULL, NULL, NULL}
+};
+
+const struct RRData nons_example_glue_records[] = {
+    {"ns.incompletechild.nons.example", "A", "192.0.2.1"},
     {NULL, NULL, NULL}
 };
 
@@ -298,7 +313,7 @@ const struct ZoneData zone_data[] = {
     { "example.com", "IN", example_com_records, example_com_glue_records },
     { "sql1.example.com", "IN", sql1_example_com_records, empty_records },
     { "loop.example", "IN", loop_example_records, empty_records },
-    { "nons.example", "IN", nons_example_records, empty_records },
+    { "nons.example", "IN", nons_example_records, nons_example_glue_records },
     { "nons-dname.example", "IN", nonsdname_example_records, empty_records },
     { "nosoa.example", "IN", nosoa_example_records, empty_records },
     { "apexcname.example", "IN", nosoa_example_records, empty_records }

src/lib/datasrc/zonetable.h

@@ -107,7 +107,7 @@ public:
     ///   - \c result::PARTIALMATCH: A zone whose origin is a
     ///    super domain of \c name is found (but there is no exact match)
     ///   - \c result::NOTFOUND: For all other cases.
-    /// - \c zone: A <Boost> shared pointer to the found \c Zone object if one
+    /// - \c zone: A "Boost" shared pointer to the found \c Zone object if one
     ///  is found; otherwise \c NULL.
     ///
     /// This method never throws an exception.

src/lib/dns/buffer.h

@@ -356,6 +356,21 @@ public:
     /// \param data The 8-bit integer to be written into the buffer.
     void writeUint8(uint8_t data) { data_.push_back(data); }
 
+    /// \brief Write an unsigned 8-bit integer into the buffer.
+    ///
+    /// The position must be lower than the size of the buffer,
+    /// otherwise an exception of class \c isc::dns::InvalidBufferPosition
+    /// will be thrown.
+    ///
+    /// \param data The 8-bit integer to be written into the buffer.
+    /// \param pos The position in the buffer to write the data.
+    void writeUint8At(uint8_t data, size_t pos) {
+        if (pos + sizeof(data) > data_.size()) {
+            isc_throw(InvalidBufferPosition, "write at invalid position");
+        }
+        data_[pos] = data;
+    }
+
     /// \brief Write an unsigned 16-bit integer in host byte order into the
     /// buffer in network byte order.
     ///

src/lib/dns/edns.h

@@ -213,7 +213,7 @@ public:
     /// \param name The owner name of the OPT RR.  This must be the root name.
     /// \param rrclass The RR class of the OPT RR.
     /// \param rrtype This must specify the OPT RR type.
-    /// \param rrttl The TTL of the OPT RR.
+    /// \param ttl The TTL of the OPT RR.
     /// \param rdata The RDATA of the OPT RR.
     EDNS(const Name& name, const RRClass& rrclass, const RRType& rrtype,
          const RRTTL& ttl, const rdata::Rdata& rdata);
@@ -418,7 +418,7 @@ private:
 /// \param name The owner name of the OPT RR.  This must be the root name.
 /// \param rrclass The RR class of the OPT RR.
 /// \param rrtype This must specify the OPT RR type.
-/// \param rrttl The TTL of the OPT RR.
+/// \param ttl The TTL of the OPT RR.
 /// \param rdata The RDATA of the OPT RR.
 /// \param extended_rcode A placeholder to store the topmost 8 bits of the
 /// extended Rcode.

src/lib/dns/masterload.h

@@ -110,7 +110,7 @@ typedef boost::function<void(RRsetPtr)> MasterLoadCallback;
 ///  but this is not even though it's valid per RFC1035:
 /// \code example.com. IN 3600 A 192.0.2.1
 /// \endcode
-/// - <TTL>, <RRCLASS>, and <RRTYPE> must be recognizable by the \c RRTTL,
+/// - "TTL", "RRCLASS", and "RRTYPE" must be recognizable by the \c RRTTL,
 ///   RRClass and RRType class implementations of this library.  In particular,
 ///   as of this writing TTL must be a decimal number (a convenient extension
 ///   such as "1H" instead of 3600 cannot be used).  Not all standard RR
@@ -213,7 +213,7 @@ typedef boost::function<void(RRsetPtr)> MasterLoadCallback;
 /// \param filename A path to a master zone file to be loaded.
 /// \param origin The origin name of the zone.
 /// \param zone_class The RR class of the zone.
-/// \param callbck A callback functor or function that is to be called
+/// \param callback A callback functor or function that is to be called
 /// for each RRset.
 void masterLoad(const char* const filename, const Name& origin,
                 const RRClass& zone_class, MasterLoadCallback callback);
@@ -231,7 +231,7 @@ void masterLoad(const char* const filename, const Name& origin,
 /// \param input An input stream object that is to emit zone's RRs.
 /// \param origin The origin name of the zone.
 /// \param zone_class The RR class of the zone.
-/// \param callbck A callback functor or function that is to be called for
+/// \param callback A callback functor or function that is to be called for
 /// each RRset.
 void masterLoad(std::istream& input, const Name& origin,
                 const RRClass& zone_class, MasterLoadCallback callback);

src/lib/dns/message.h

@@ -141,7 +141,7 @@ typedef SectionIterator<RRsetPtr> RRsetIterator;
 /// - We may want to provide an "iterator" for all RRsets/RRs for convenience.
 ///   This will be for applications that do not care about performance much,
 ///   so the implementation can only be moderately efficient.
-/// - may want to provide a "find" method for a specified type
+/// - We may want to provide a "find" method for a specified type
 ///   of RR in the message.
 class Message {
 public:
@@ -155,8 +155,8 @@ public:
     ///
     /// Only the defined constants are valid where a header flag is required
     /// in this library (e.g., in \c Message::setHeaderFlag()).
-    /// Since these are enum constants, however, invalid value could be passed
-    /// via casting without an error at compilation time.
+    /// Since these are enum constants, however, an invalid value could be
+    /// passed via casting without an error at compilation time.
     /// It is generally the callee's responsibility to check and reject invalid
     /// values.
     /// Of course, applications shouldn't pass invalid values even if the
@@ -168,7 +168,7 @@ public:
     /// specified flag in the second 16 bits of the DNS Header section
     /// in order to make the internal implementation simpler.
     /// For example, \c HEADERFLAG_QR is defined to be 0x8000 as the QR
-    /// bit is the most significant bit of the 2nd 16 bits of the header.
+    /// bit is the most significant bit of the second 16 bits of the header.
     /// However, applications should not assume this coincidence and
     /// must solely use the enum representations.
     /// Any usage based on the assumption of the underlying values is invalid
@@ -199,8 +199,8 @@ public:
         HEADERFLAG_TC = 0x0200, ///< Truncation
         HEADERFLAG_RD = 0x0100, ///< Recursion desired
         HEADERFLAG_RA = 0x0080, ///< Recursion available
-        HEADERFLAG_AD = 0x0020, ///< DNSSEC checking disabled (RFC4035)
-        HEADERFLAG_CD = 0x0010  ///< Authentic %data (RFC4035)
+        HEADERFLAG_AD = 0x0020, ///< Authentic %data (RFC4035)
+        HEADERFLAG_CD = 0x0010  ///< DNSSEC checking disabled (RFC4035)
     };
 
     /// \brief Constants to specify sections of a DNS message.

src/lib/dns/question.h

@@ -54,13 +54,13 @@ typedef boost::shared_ptr<const Question> ConstQuestionPtr;
 /// class.
 /// This may look odd in that an "RRset" and "Question" are similar from the
 /// protocol point of view: Both are used as a semantics unit of DNS messages;
-/// both share the same set of components, name, RR type and RR class.
+/// both share the same set of components (name, RR type and RR class).
 ///
 /// In fact, BIND9 didn't introduce a separate data structure for Questions,
 /// and use the same \c "rdataset" structure for both RRsets and Questions.
 /// We could take the same approach, but chose to adopt the different design.
 /// One reason for that is because a Question and an RRset are still
-/// different, and a Question might not be cleanly defined if (e.g.) it were
+/// different, and a Question might not be cleanly defined, e.g., if it were
 /// a derived class of some "RRset-like" class.
 /// For example, we couldn't give a reasonable semantics for \c %getTTL() or
 /// \c %setTTL() methods for a Question, since it's not associated with the
@@ -74,14 +74,14 @@ typedef boost::shared_ptr<const Question> ConstQuestionPtr;
 ///
 /// On the other hand, we do not expect a strong need for customizing the
 /// \c Question class, unlike the RRset.
-/// Handling the Question section of a DNS message is relatively a
+/// Handling the "Question" section of a DNS message is relatively a
 /// simple work comparing to RRset-involved operations, so a unified
 /// straightforward implementation should suffice for any use cases
 /// including performance sensitive ones.
 ///
-/// We may, however, still want to have customized version of Question
+/// We may, however, still want to have a customized version of Question
 /// for, e.g, highly optimized behavior, and may revisit this design choice
-/// as we have more experiences with this implementation.
+/// as we have more experience with this implementation.
 ///
 /// One disadvantage of defining RRsets and Questions as unrelated classes
 /// is that we cannot handle them in a polymorphic way.

src/lib/dns/rrset.h

@@ -278,8 +278,6 @@ public:
     /// name when possible in the context of zone dump.  This is a future
     /// TODO item.
     ///
-    /// \param rrset A reference to a (derived class of) \c AbstractRRset object
-    /// whose content is to be converted.
     /// \return A string representation of the RRset.
     virtual std::string toText() const = 0;
 

src/lib/dns/rrttl.h

@@ -118,7 +118,8 @@ public:
     /// If resource allocation in rendering process fails, a corresponding
     /// standard exception will be thrown.
     ///
-    /// \param buffer An output buffer to store the wire data.
+    /// \param renderer DNS message rendering context that encapsulates the
+    /// output buffer in which the RRTTL is to be stored.
     void toWire(MessageRenderer& renderer) const;
     /// \brief Render the \c RRTTL in the wire format.
     ///
@@ -128,8 +129,7 @@ public:
     /// If resource allocation in rendering process fails, a corresponding
     /// standard exception will be thrown.
     ///
-    /// \param renderer DNS message rendering context that encapsulates the
-    /// output buffer in which the RRTTL is to be stored.
+    /// \param buffer An output buffer to store the wire data.
     void toWire(OutputBuffer& buffer) const;
     //@}
 

src/lib/dns/tests/buffer_unittest.cc

@@ -124,10 +124,16 @@ TEST_F(BufferTest, outputBufferWriteat) {
     obuffer.writeUint32(data32);
     expected_size += sizeof(data32);
 
+    // overwrite 2nd byte
+    obuffer.writeUint8At(4, 1);
+    EXPECT_EQ(expected_size, obuffer.getLength()); // length shouldn't change
+    const uint8_t* cp = static_cast<const uint8_t*>(obuffer.getData());
+    EXPECT_EQ(4, *(cp + 1));
+
     // overwrite 2nd and 3rd bytes
     obuffer.writeUint16At(data16, 1);
     EXPECT_EQ(expected_size, obuffer.getLength()); // length shouldn't change
-    const uint8_t* cp = static_cast<const uint8_t*>(obuffer.getData());
+    cp = static_cast<const uint8_t*>(obuffer.getData());
     EXPECT_EQ(2, *(cp + 1));
     EXPECT_EQ(3, *(cp + 2));
 
@@ -138,6 +144,10 @@ TEST_F(BufferTest, outputBufferWriteat) {
     EXPECT_EQ(2, *(cp + 2));
     EXPECT_EQ(3, *(cp + 3));
 
+    EXPECT_THROW(obuffer.writeUint8At(data16, 5),
+                 isc::dns::InvalidBufferPosition);
+    EXPECT_THROW(obuffer.writeUint8At(data16, 4),
+                 isc::dns::InvalidBufferPosition);
     EXPECT_THROW(obuffer.writeUint16At(data16, 3),
                  isc::dns::InvalidBufferPosition);
     EXPECT_THROW(obuffer.writeUint16At(data16, 4),

src/lib/log/dummylog.h

@@ -34,7 +34,7 @@ extern std::string dprefix;
  * \short Temporary interface to logging.
  *
  * This is a temporary function to do logging. It has wrong interface currently
- * and should be replaced by something else. It's main purpose now is to mark
+ * and should be replaced by something else. Its main purpose now is to mark
  * places where logging should happen. When it is removed, compiler will do
  * our work of finding the places.
  *
@@ -51,6 +51,7 @@ extern std::string dprefix;
  *
  * @param message The message to log. The real interface will probably have
  *     more parameters.
+ * \param error_flag TODO
  */
 void dlog(const std::string& message, bool error_flag=false);
 

src/lib/log/filename.h

@@ -131,7 +131,7 @@ public:
     /// \param name Name to expand
     ///
     /// \return Name expanded with stored name
-    std::string useAsDefault(const std::string&) const;
+    std::string useAsDefault(const std::string& name) const;
 
 private:
     /// \brief Split Name into Components

src/lib/log/message_dictionary.h

@@ -116,7 +116,7 @@ public:
     /// const char* and adds them to the dictionary.  The messages are added
     /// using "Add".
     ///
-    /// \param data null-terminated array of const char* alternating ID and
+    /// \param elements null-terminated array of const char* alternating ID and
     /// message text.  This should be an odd number of elements long, the last
     /// elemnent being NULL.  If it is an even number of elements long, the
     /// last ID is ignored.

src/lib/log/xdebuglevel.h

@@ -132,7 +132,7 @@ public:
     /// \return Pointer to the desired logging level object.
     static LevelPtr toLevel(int val, const LevelPtr& defaultLevel);
 
-    /// \param Convert String to Logging Level
+    /// \brief Convert String to Logging Level
     ///
     /// Returns a logging level object corresponding to the given name.  If the
     /// name is invalid, an object of logging level DEBUG (the minimum debug
@@ -143,7 +143,7 @@ public:
     /// \return Pointer to the desired logging level object.
     static LevelPtr toLevelLS(const LogString& sArg);
 
-    /// \param Convert String to Logging Level
+    /// \brief Convert String to Logging Level
     ///
     /// Returns a logging level object corresponding to the given name.  If the
     /// name is invalid, the given default is returned.

src/lib/nsas/Makefile.am

@@ -37,5 +37,6 @@ libnsas_la_SOURCES += zone_entry.cc zone_entry.h
 libnsas_la_SOURCES += fetchable.h
 libnsas_la_SOURCES += address_request_callback.h
 libnsas_la_SOURCES += random_number_generator.h
+libnsas_la_SOURCES += glue_hints.h glue_hints.cc
 
 CLEANFILES = *.gcno *.gcda

src/lib/nsas/glue_hints.cc

@@ -0,0 +1,168 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "glue_hints.h"
+
+#include <stdlib.h>
+
+#include <dns/rrset.h>
+#include <dns/rdata.h>
+#include <dns/rrtype.h>
+#include <dns/rdataclass.h>
+
+#include <asiolink/io_address.h>
+#include <nsas/nameserver_entry.h>
+
+using namespace isc::dns;
+using namespace isc::nsas;
+
+// This is a simple implementation for finding glue
+//
+// It iterates over the AUTHORITY section of the given Message,
+// and for each NS RR it iterates over the ADDITIONAL section to
+// see if there are A or AAAA records.
+//
+// Of course, this could be done more efficiently. One option is to
+// reverse this; check for A and AAAA records (since those will only
+// be there if there actually is glue, while NS records will be present
+// in any delegation). However, it may be even better to let the
+// Response Classifier decide on glue, while it is validating the packet
+//
+// (er, TODO, so to speak. discuss.)
+
+// Helper functions
+namespace {
+    // Add the contents of the given A or AAAA rrset to the given
+    // addressvector
+    //
+    // This creates an 'dummy' NameserverEntry value, because that
+    // is enforced by NameserverAddress. We may want to reconsider
+    // the need for that (perhaps we can change it so that if it is
+    // NULL, all NSAS-related calls to the NameserverAddress object
+    // become nops)
+    void
+    addRRset(std::vector<NameserverAddress>& addresses,
+             const RRsetPtr rrset)
+    {
+        const std::string ns_name = rrset->getName().toText();
+        RdataIteratorPtr rdi = rrset->getRdataIterator();
+        while (!rdi->isLast()) {
+            AddressEntry entry(asiolink::IOAddress(rdi->getCurrent().toText()));
+            boost::shared_ptr<NameserverEntry> ns_entry(new NameserverEntry(ns_name, rrset->getClass()));
+            NameserverAddress ns_address(ns_entry, entry, V4_ONLY);
+            addresses.push_back(ns_address);
+            rdi->next();
+        }
+    }
+}
+
+namespace isc {
+namespace nsas {
+
+GlueHints::GlueHints(const std::string& zone_name,
+                     const isc::dns::Message& delegation_message)
+{
+    for (RRsetIterator rssi = delegation_message.beginSection(Message::SECTION_AUTHORITY);
+         rssi != delegation_message.endSection(Message::SECTION_AUTHORITY);
+         ++rssi) {
+        if ((*rssi)->getType() == RRType::NS() &&
+            (*rssi)->getName().toText() == zone_name) {
+            addGlueForRRset(*rssi, delegation_message);
+        }
+    }
+}
+
+
+bool
+GlueHints::hasGlue(AddressFamily family) const {
+    return ((addresses_v4.size() > 0 && (family == ANY_OK || family == V4_ONLY)) ||
+            (addresses_v6.size() > 0 && (family == ANY_OK || family == V6_ONLY)));
+}
+
+NameserverAddress
+GlueHints::getGlue(AddressFamily family) const {
+    // TODO: once we have a more general random lib, use that. Since
+    // this is simply glue, and we don't need a weighted selection,
+    // for now srandom should be good enough. Once #583 has been merged,
+    // (or better yet, once that one and the weighted random have gone
+    // together in a util lib), we can use that.
+    int max = 0;
+    size_t v4s = addresses_v4.size();
+    size_t v6s = addresses_v6.size();
+
+    if (family == ANY_OK || family == V4_ONLY) {
+        max += v4s;
+    }
+    if (family == ANY_OK || family == V6_ONLY) {
+        max += v6s;
+    }
+
+    assert(max > 0);
+    long int selection = random() % max;
+
+    if (family == ANY_OK) {
+        if (selection < v4s) {
+            return addresses_v4[selection];
+        } else {
+            return addresses_v6[selection-v4s];
+        }
+    } else if (family == V4_ONLY) {
+        return addresses_v4[selection];
+    } else if (family == V6_ONLY) {
+        return addresses_v6[selection];
+    } else {
+        // Unknown family
+        assert(false);
+        // Some compilers want something returned anyway
+        return NameserverAddress();
+    }
+}
+
+// Add the A and AAAA records from the given message for the given
+// NS name to the relevant address vector
+// (A rrsets are added to addresses_v4, AAAA rrsets are added to
+// addresses_v6).
+void
+GlueHints::addGlueForName(const Name& name, const Message& message)
+{
+    for (RRsetIterator rssi = message.beginSection(Message::SECTION_ADDITIONAL);
+         rssi != message.endSection(Message::SECTION_ADDITIONAL);
+         ++rssi) {
+        if ((*rssi)->getName() == name) {
+            if ((*rssi)->getType() == RRType::A()) {
+                addRRset(addresses_v4, *rssi);
+            } else if ((*rssi)->getType() == RRType::AAAA()) {
+                addRRset(addresses_v6, *rssi);
+            }
+        }
+    }
+}
+
+// Add the glue for the given NS RRset in the message to the
+// relevant vectors.
+void
+GlueHints::addGlueForRRset(const RRsetPtr rrset, const Message& message)
+{
+    RdataIteratorPtr rdi = rrset->getRdataIterator();
+    while (!rdi->isLast()) {
+        isc::dns::Name name(dynamic_cast<const rdata::generic::NS&>(
+                        rdi->getCurrent()).getNSName());
+        addGlueForName(name, message);
+        rdi->next();
+    }
+}
+
+
+} // namespace nsas
+} // namespace isc

src/lib/nsas/glue_hints.h

@@ -0,0 +1,71 @@
+// 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 __GLUE_HINTS_H
+#define __GLUE_HINTS_H
+
+#include <vector>
+
+#include <dns/message.h>
+
+#include "nsas_types.h"
+#include "nameserver_address.h"
+
+namespace isc {
+namespace nsas {
+
+class GlueHints {
+public:
+    /// \brief Empty constructor
+    GlueHints() {};
+
+    /// \brief Constructor
+    ///
+    /// Creates a glue hint object, with the glue data found in the
+    /// given packet.
+    ///
+    /// \param zone_name The name of the zone to find glue for
+    /// \param delegation_message The Message that may contain glue
+    GlueHints(const std::string& zone_name,
+              const isc::dns::Message& delegation_message);
+
+    /// \brief Check if there is glue for the given AddressFamily
+    ///
+    /// \param family the AddressFamily to check for glue for
+    /// \return true if there is glue for that family. false if not
+    bool hasGlue(AddressFamily family) const;
+
+    /// \brief Get a random glue address for the given family
+    ///
+    /// ONLY call this if hasGlue() returned true.
+    ///
+    /// \param family the AddressFamily to get glue for
+    /// \return a NameserverAddress specified by the glue
+    NameserverAddress getGlue(AddressFamily family) const;
+
+private:
+    void addGlueForName(const isc::dns::Name& name,
+                        const isc::dns::Message& message);
+    void addGlueForRRset(const isc::dns::RRsetPtr rrset,
+                         const isc::dns::Message& message);
+
+    std::vector<NameserverAddress> addresses_v4;
+    std::vector<NameserverAddress> addresses_v6;
+};
+
+}
+}
+
+
+#endif // __GLUE_HINTS_H

src/lib/nsas/hash.h

@@ -59,7 +59,7 @@ public:
     /// sequence could lead to problems in checking results.
     Hash(uint32_t tablesize, uint32_t maxkeylen = 255, bool randomise = true);
 
-    /// \bool Virtual Destructor
+    /// \brief Virtual Destructor
     virtual ~Hash()
     {}
 

src/lib/nsas/hash_table.h

@@ -126,7 +126,7 @@ public:
     ///
     /// Initialises the hash table.
     ///
-    /// \param CmpFn Compare function (or object) used to compare an object with
+    /// \param cmp Compare function (or object) used to compare an object with
     /// to get the name to be used as a key in the table.  The object should be
     /// created via a "new" as ownership passes to the hash table.  The hash
     /// table will take the responsibility of deleting it.

src/lib/nsas/lru_list.h

@@ -109,6 +109,13 @@ public:
     /// \param element Reference to the element to touch.
     virtual void touch(boost::shared_ptr<T>& element);
 
+    /// \brief Drop All the Elements in the List .
+    ///
+    /// All the elements will be dropped from the list container, and their
+    /// drop handler(if there is one) will be called, when done, the size of
+    /// of list will be 0.
+    virtual void clear();
+
     /// \brief Return Size of the List
     ///
     /// An independent count is kept of the list size, as list.size() may take
@@ -133,7 +140,7 @@ public:
 
     /// \brief Set Maximum Size
     ///
-    /// \param new_size New maximum list size
+    /// \param max_size New maximum list size
     virtual void setMaxSize(uint32_t max_size) {
         max_size_ = max_size;
     }
@@ -228,6 +235,25 @@ void LruList<T>::touch(boost::shared_ptr<T>& element) {
     }
 }
 
+// Clear the list-  when done, the size of list will be 0.
+template <typename T>
+void LruList<T>::clear() {
+    // Protect list against concurrent access
+    isc::locks::scoped_lock<isc::locks::mutex> lock(mutex_);
+
+    // ... and update the count while we have the mutex.
+    count_ = 0;
+    typename std::list<boost::shared_ptr<T> >::iterator iter;
+    if (dropped_) {
+        for (iter = lru_.begin(); iter != lru_.end(); ++iter) {
+            // Call the drop handler.
+            (*dropped_)(iter->get());
+        }
+    }
+
+    lru_.clear();
+}
+
 }   // namespace nsas
 }   // namespace isc
 

src/lib/nsas/nameserver_address.h

@@ -60,10 +60,10 @@ public:
     /// pointed to NameserverEntry which contains the address as well as it's
     /// corresponding index. The user can update it's RTT with the index later.
     ///
-    /// \param namerserver A shared_ptr that points to a NameserverEntry object
+    /// \param nameserver A shared_ptr that points to a NameserverEntry object
     /// the shared_ptr can avoid the NameserverEntry object being dropped while the
     /// request is processing.
-    /// \param index The address's index in NameserverEntry's addresses vector
+    /// \param address The address's index in NameserverEntry's addresses vector
     /// \param family Address family, V4_ONLY or V6_ONLY
     NameserverAddress(const boost::shared_ptr<NameserverEntry>& nameserver,
         const AddressEntry& address, AddressFamily family):

src/lib/nsas/nameserver_address_store.cc

@@ -29,6 +29,7 @@
 #include "nameserver_entry.h"
 #include "nameserver_address_store.h"
 #include "zone_entry.h"
+#include "glue_hints.h"
 #include "address_request_callback.h"
 
 using namespace isc::dns;
@@ -80,7 +81,8 @@ newZone(
 
 void
 NameserverAddressStore::lookup(const string& zone, const RRClass& class_code,
-    boost::shared_ptr<AddressRequestCallback> callback, AddressFamily family)
+    boost::shared_ptr<AddressRequestCallback> callback, AddressFamily family,
+    const GlueHints& glue_hints)
 {
     pair<bool, boost::shared_ptr<ZoneEntry> > zone_obj(zone_hash_->getOrAdd(HashKey(
         zone, class_code), boost::bind(newZone, &resolver_, &zone, &class_code,
@@ -90,7 +92,8 @@ NameserverAddressStore::lookup(const string& zone, const RRClass& class_code,
     } else {
         zone_lru_->touch(zone_obj.second);
     }
-    zone_obj.second->addCallback(callback, family);
+    
+    zone_obj.second->addCallback(callback, family, glue_hints);
 }
 
 void

src/lib/nsas/nameserver_address_store.h

@@ -23,6 +23,7 @@
 #include <resolve/resolver_interface.h>
 
 #include "nsas_types.h"
+#include "glue_hints.h"
 
 namespace isc {
 // Some forward declarations, so we do not need to include so many headers
@@ -60,7 +61,7 @@ public:
     /// tests) should it use to ask questions.
     /// \param zonehashsize Size of the zone hash table.  The default value of
     /// 1009 is the first prime number above 1000.
-    /// \param nshash size Size of the nameserver hash table.  The default
+    /// \param nshashsize Size of the nameserver hash table.  The default
     /// value of 3001 is the first prime number over 3000, and by implication,
     /// there is an assumption that there will be more nameservers than zones
     /// in the store.
@@ -85,7 +86,7 @@ public:
     /// \param family Which address is requested.
     void lookup(const std::string& zone, const dns::RRClass& class_code,
         boost::shared_ptr<AddressRequestCallback> callback, AddressFamily
-        family = ANY_OK);
+        family = ANY_OK, const GlueHints& = GlueHints());
 
     /// \brief cancel the given lookup action
     ///

src/lib/nsas/nameserver_entry.h

@@ -151,7 +151,7 @@ public:
     /// Updates the RTT for a particular address
     ///
     /// \param address Address to update
-    /// \param RTT New RTT for the address
+    /// \param rtt New RTT for the address
     void setAddressRTT(const asiolink::IOAddress& address, uint32_t rtt);
 
     /// \brief Update RTT of the address that corresponding to the index

src/lib/nsas/tests/lru_list_unittest.cc

@@ -251,6 +251,35 @@ TEST_F(LruListTest, Dropped) {
     EXPECT_EQ(0, (entry3_->getClass().getCode() & 0x8000));
 }
 
+// Clear functor tests: tests whether all the elements in
+// the list are dropped properly and the size of list is
+// set to 0.
+TEST_F(LruListTest, Clear) {
+    // Create an object with an expiration handler.
+    LruList<TestEntry> lru(3, new Dropped());
+
+    // Fill the list
+    lru.add(entry1_);
+    lru.add(entry2_);
+    lru.add(entry3_);
+
+    EXPECT_EQ(RRClass::IN(), entry1_->getClass());
+    EXPECT_EQ(RRClass::CH(), entry2_->getClass());
+    EXPECT_EQ(RRClass::HS(), entry3_->getClass());
+
+    EXPECT_EQ(0, (entry1_->getClass().getCode() & 0x8000));
+    EXPECT_EQ(0, (entry2_->getClass().getCode() & 0x8000));
+    EXPECT_EQ(0, (entry3_->getClass().getCode() & 0x8000));
+
+    // Clear the lru list, and check the drop handler run
+    lru.clear();
+    EXPECT_NE(0, (entry1_->getClass().getCode() & 0x8000));
+    EXPECT_NE(0, (entry2_->getClass().getCode() & 0x8000));
+    EXPECT_NE(0, (entry3_->getClass().getCode() & 0x8000));
+ 
+    EXPECT_EQ(0, lru.size());
+}
+
 // Miscellaneous tests - pathological conditions
 TEST_F(LruListTest, Miscellaneous) {
 

src/lib/nsas/zone_entry.cc

@@ -122,7 +122,7 @@ class ZoneEntry::ResolverCallback :
                  * do), so we can just reuse them instead of looking them up in
                  * the table or creating them.
                  */
-                map<string, NameserverPtr> old;
+                std::map<string, NameserverPtr> old;
                 BOOST_FOREACH(const NameserverPtr& ptr, entry_->nameservers_) {
                     old[ptr->getName()] = ptr;
                 }
@@ -224,7 +224,8 @@ class ZoneEntry::ResolverCallback :
 };
 
 void
-ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family) {
+ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family,
+                       const GlueHints& glue_hints) {
     Lock lock(mutex_);
 
     bool ask(false);
@@ -238,11 +239,18 @@ ZoneEntry::addCallback(CallbackPtr callback, AddressFamily family) {
     if (getState() == EXPIRED || getState() == NOT_ASKED) {
         ask = true;
     }
-
+    
     // We do not have the answer right away, just queue the callback
     bool execute(!ask && getState() != IN_PROGRESS &&
         callbacks_[family].empty());
-    callbacks_[family].push_back(callback);
+
+    // Unless there was glue
+    if (ask && glue_hints.hasGlue(family)) {
+        callback->success(glue_hints.getGlue(family));
+    } else {
+        callbacks_[family].push_back(callback);
+    }
+
     if (execute) {
         // Try to process it right away, store if not possible to handle
         process(family, NameserverPtr());

src/lib/nsas/zone_entry.h

@@ -32,6 +32,7 @@
 #include "fetchable.h"
 #include "nsas_types.h"
 #include "random_number_generator.h"
+#include "glue_hints.h"
 
 namespace isc {
 namespace nsas {
@@ -97,9 +98,13 @@ public:
      *
      * \param callback The callback itself.
      * \param family Which address family is acceptable as an answer?
+     * \param glue_hints If a non-empty glue-hints object is passed,
+     *        and the NSAS does not have an immediate answer, it will
+     *        call back immediately with one of the glue hints.
      */
     void addCallback(boost::shared_ptr<AddressRequestCallback>
-        callback, AddressFamily family);
+        callback, AddressFamily family,
+        const GlueHints& glue_hints = GlueHints());
 
     /**
      * \short Remove a callback from the list

src/lib/python/isc/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = datasrc cc config log net notify util 
+SUBDIRS = datasrc cc config log net notify util testutils
 
 python_PYTHON = __init__.py
 

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

@@ -44,25 +44,36 @@ class ConfigManagerData:
     """This class hold the actual configuration information, and
        reads it from and writes it to persistent storage"""
 
-    def __init__(self, data_path, file_name = "b10-config.db"):
+    def __init__(self, data_path, file_name):
         """Initialize the data for the configuration manager, and
            set the version and path for the data store. Initializing
            this does not yet read the database, a call to
-           read_from_file is needed for that."""
+           read_from_file is needed for that.
+
+           In case the file_name is absolute, data_path is ignored
+           and the directory where the file_name lives is used instead.
+           """
         self.data = {}
         self.data['version'] = config_data.BIND10_CONFIG_DATA_VERSION
-        self.data_path = data_path
-        self.db_filename = data_path + os.sep + file_name
+        if os.path.isabs(file_name):
+            self.db_filename = file_name
+            self.data_path = os.path.dirname(file_name)
+        else:
+            self.db_filename = data_path + os.sep + file_name
+            self.data_path = data_path
+
+    def read_from_file(data_path, file_name):
+        """Read the current configuration found in the file file_name.
+           If file_name is absolute, data_path is ignored. Otherwise
+           we look for the file_name in data_path directory.
 
-    def read_from_file(data_path, file_name = "b10-config.db"):
-        """Read the current configuration found in the file at
-           data_path. If the file does not exist, a
-           ConfigManagerDataEmpty exception is raised. If there is a
-           parse error, or if the data in the file has the wrong
-           version, a ConfigManagerDataReadError is raised. In the first
-           case, it is probably safe to log and ignore. In the case of
-           the second exception, the best way is probably to report the
-           error and stop loading the system."""
+           If the file does not exist, a ConfigManagerDataEmpty exception is
+           raised. If there is a parse error, or if the data in the file has
+           the wrong version, a ConfigManagerDataReadError is raised. In the
+           first case, it is probably safe to log and ignore. In the case of
+           the second exception, the best way is probably to report the error
+           and stop loading the system.
+           """
         config = ConfigManagerData(data_path, file_name)
         file = None
         try:
@@ -142,20 +153,24 @@ class ConfigManagerData:
 
 class ConfigManager:
     """Creates a configuration manager. The data_path is the path
-       to the directory containing the b10-config.db file.
+       to the directory containing the configuraton file,
+       database_filename points to the configuration file.
        If session is set, this will be used as the communication
        channel session. If not, a new session will be created.
        The ability to specify a custom session is for testing purposes
        and should not be needed for normal usage."""
-    def __init__(self, data_path, session = None):
+    def __init__(self, data_path, database_filename, session=None):
         """Initialize the configuration manager. The data_path string
            is the path to the directory where the configuration is
-           stored (in <data_path>/b10-config.db). Session is an optional
+           stored (in <data_path>/<database_filename> or in
+           <database_filename>, if it is absolute). The dabase_filename
+           is the config file to load. Session is an optional
            cc-channel session. If this is not given, a new one is
-           created"""
+           created."""
         self.data_path = data_path
+        self.database_filename = database_filename
         self.module_specs = {}
-        self.config = ConfigManagerData(data_path)
+        self.config = ConfigManagerData(data_path, database_filename)
         if session:
             self.cc = session
         else:
@@ -223,17 +238,18 @@ class ConfigManager:
         return commands
 
     def read_config(self):
-        """Read the current configuration from the b10-config.db file
-           at the path specificied at init()"""
+        """Read the current configuration from the file specificied at init()"""
         try:
-            self.config = ConfigManagerData.read_from_file(self.data_path)
+            self.config = ConfigManagerData.read_from_file(self.data_path,
+                                                           self.\
+                                                           database_filename)
         except ConfigManagerDataEmpty:
             # ok, just start with an empty config
-            self.config = ConfigManagerData(self.data_path)
+            self.config = ConfigManagerData(self.data_path,
+                                            self.database_filename)
         
     def write_config(self):
-        """Write the current configuration to the b10-config.db file
-           at the path specificied at init()"""
+        """Write the current configuration to the file specificied at init()"""
         self.config.write_to_file()
 
     def _handle_get_module_spec(self, cmd):

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

@@ -27,9 +27,20 @@ class TestConfigManagerData(unittest.TestCase):
     def setUp(self):
         self.data_path = os.environ['CONFIG_TESTDATA_PATH']
         self.writable_data_path = os.environ['CONFIG_WR_TESTDATA_PATH']
-        self.config_manager_data = ConfigManagerData(self.writable_data_path)
+        self.config_manager_data = ConfigManagerData(self.writable_data_path,
+                                                     file_name="b10-config.db")
         self.assert_(self.config_manager_data)
 
+    def test_abs_file(self):
+        """
+        Test what happens if we give the config manager an absolute path.
+        It shouldn't append the data path to it.
+        """
+        abs_path = self.data_path + os.sep + "b10-config-imaginary.db"
+        data = ConfigManagerData(os.getcwd(), abs_path)
+        self.assertEqual(abs_path, data.db_filename)
+        self.assertEqual(self.data_path, data.data_path)
+
     def test_init(self):
         self.assertEqual(self.config_manager_data.data['version'],
                          config_data.BIND10_CONFIG_DATA_VERSION)
@@ -39,10 +50,10 @@ class TestConfigManagerData(unittest.TestCase):
                          self.writable_data_path + os.sep + "b10-config.db")
 
     def test_read_from_file(self):
-        ConfigManagerData.read_from_file(self.writable_data_path)
+        ConfigManagerData.read_from_file(self.writable_data_path, "b10-config.db")
         self.assertRaises(ConfigManagerDataEmpty,
                           ConfigManagerData.read_from_file,
-                          "doesnotexist")
+                          "doesnotexist", "b10-config.db")
         self.assertRaises(ConfigManagerDataReadError,
                           ConfigManagerData.read_from_file,
                           self.data_path, "b10-config-bad1.db")
@@ -68,8 +79,8 @@ class TestConfigManagerData(unittest.TestCase):
         # by equality of the .data element. If data_path or db_filename
         # are different, but the contents are the same, it's still
         # considered equal
-        cfd1 = ConfigManagerData(self.data_path)
-        cfd2 = ConfigManagerData(self.data_path)
+        cfd1 = ConfigManagerData(self.data_path, file_name="b10-config.db")
+        cfd2 = ConfigManagerData(self.data_path, file_name="b10-config.db")
         self.assertEqual(cfd1, cfd2)
         cfd2.data_path = "some/unknown/path"
         self.assertEqual(cfd1, cfd2)
@@ -85,10 +96,25 @@ class TestConfigManager(unittest.TestCase):
         self.data_path = os.environ['CONFIG_TESTDATA_PATH']
         self.writable_data_path = os.environ['CONFIG_WR_TESTDATA_PATH']
         self.fake_session = FakeModuleCCSession()
-        self.cm = ConfigManager(self.writable_data_path, self.fake_session)
+        self.cm = ConfigManager(self.writable_data_path,
+                                database_filename="b10-config.db",
+                                session=self.fake_session)
         self.name = "TestModule"
         self.spec = isc.config.module_spec_from_file(self.data_path + os.sep + "/spec2.spec")
-    
+
+    def test_paths(self):
+        """
+        Test data_path and database filename is passed trough to
+        underlying ConfigManagerData.
+        """
+        cm = ConfigManager("datapath", "filename", self.fake_session)
+        self.assertEqual("datapath" + os.sep + "filename",
+                         cm.config.db_filename)
+        # It should preserve it while reading
+        cm.read_config()
+        self.assertEqual("datapath" + os.sep + "filename",
+                         cm.config.db_filename)
+
     def test_init(self):
         self.assert_(self.cm.module_specs == {})
         self.assert_(self.cm.data_path == self.writable_data_path)

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

@@ -0,0 +1 @@
+EXTRA_DIST = __init__.py parse_args.py

src/lib/python/isc/testutils/README

@@ -0,0 +1,3 @@
+This contains some shared test code for other modules and python processes.
+That's why it doesn't have its own test subdirectory and why it isn't
+installed.

src/bin/bind10/tests/bind10_test.in

@@ -1,6 +1,4 @@
-#! /bin/sh
-
-# Copyright (C) 2010  Internet Systems Consortium.
+# Copyright (C) 2011  Internet Systems Consortium.
 #
 # Permission to use, copy, modify, and distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -15,18 +13,5 @@
 # NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
-export PYTHON_EXEC
-
-BIND10_PATH=@abs_top_srcdir@/src/bin/bind10
-
-PATH=@abs_top_srcdir@/src/bin/msgq:@abs_top_srcdir@/src/bin/auth:@abs_top_srcdir@/src/bin/bind-cfgd:$PATH
-export PATH
-
-PYTHONPATH=@abs_top_srcdir@/src/lib/python:@abs_top_srcdir@/src/bin/bind10
-export PYTHONPATH
-
-cd ${BIND10_PATH}/tests
-${PYTHON_EXEC} -O bind10_test.py $*
-exec ${PYTHON_EXEC} -O args_test.py $*
-
+# Nothing here, really, it's just to tell python this directory is in
+# module hierarchy

src/lib/python/isc/testutils/parse_args.py

@@ -0,0 +1,30 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+from optparse import OptionParser
+
+class OptsError(Exception):
+    """To know when OptionParser would exit"""
+    pass
+
+class TestOptParser(OptionParser):
+    """
+    We define our own option parser to push into the parsing routine.
+    This one does not exit the whole application on error, it just raises
+    exception. It doesn't change anything else. The application uses the
+    stock one.
+    """
+    def error(self, message):
+        raise OptsError(message)

src/lib/resolve/recursive_query.cc

@@ -226,6 +226,7 @@ private:
     // if we have a response for our query stored already. if
     // so, call handlerecursiveresponse(), if not, we call send()
     void doLookup() {
+        cur_zone_ = ".";
         dlog("doLookup: try cache");
         Message cached_message(Message::RENDER);
         isc::resolve::initResponseMessage(question_, cached_message);
@@ -241,7 +242,6 @@ private:
                 stop();
             }
         } else {
-            cur_zone_ = ".";
             send();
         }
         
@@ -254,11 +254,19 @@ private:
         current_ns_address = address;
         gettimeofday(&current_ns_qsent_time, NULL);
         ++outstanding_events_;
-        IOFetch query(protocol_, io_, question_,
-            current_ns_address.getAddress(),
-            53, buffer_, this,
-            query_timeout_);
-        io_.get_io_service().post(query);
+        if (test_server_.second != 0) {
+            IOFetch query(protocol_, io_, question_,
+                test_server_.first,
+                test_server_.second, buffer_, this,
+                query_timeout_);
+            io_.get_io_service().post(query);
+        } else {
+            IOFetch query(protocol_, io_, question_,
+                current_ns_address.getAddress(),
+                53, buffer_, this,
+                query_timeout_);
+            io_.get_io_service().post(query);
+        }
     }
     
     // 'general' send; if we are in forwarder mode, send a query to
@@ -330,7 +338,7 @@ private:
             isc::resolve::ResponseClassifier::classify(
                 question_, incoming, cname_target, cname_count_);
 
-        bool found_ns_address = false;
+        bool found_ns = false;
             
         switch (category) {
         case isc::resolve::ResponseClassifier::ANSWER:
@@ -381,30 +389,40 @@ private:
 
             // auth section should have at least one RRset
             // and one of them should be an NS (otherwise
-            // classifier should have error'd)
-            // TODO: should we check if it really is subzone?
+            // classifier should have error'd) to a subdomain
             for (RRsetIterator rrsi = incoming.beginSection(Message::SECTION_AUTHORITY);
-                 rrsi != incoming.endSection(Message::SECTION_AUTHORITY) && !found_ns_address;
+                 rrsi != incoming.endSection(Message::SECTION_AUTHORITY) && !found_ns;
                  ++rrsi) {
                 ConstRRsetPtr rrs = *rrsi;
                 if (rrs->getType() == RRType::NS()) {
-                    // TODO: make cur_zone_ a Name instead of a string
-                    // (this requires a few API changes in related
-                    // libraries, so as not to need many conversions)
-                    cur_zone_ = rrs->getName().toText();
-                    dlog("Referred to zone " + cur_zone_);
-                    found_ns_address = true;
-                    break;
+                    NameComparisonResult compare(Name(cur_zone_).compare(rrs->getName()));
+                    if (compare.getRelation() == NameComparisonResult::SUPERDOMAIN) {
+                        // TODO: make cur_zone_ a Name instead of a string
+                        // (this requires a few API changes in related
+                        // libraries, so as not to need many conversions)
+                        cur_zone_ = rrs->getName().toText();
+                        dlog("Referred to zone " + cur_zone_);
+                        found_ns = true;
+                        break;
+                    }
                 }
             }
 
-            if (found_ns_address) {
+            if (found_ns) {
                 // next resolver round
                 // we do NOT use doLookup() here, but send() (i.e. we
                 // skip the cache), since if we had the final answer
                 // instead of a delegation cached, we would have been
                 // there by now.
-                send();
+                GlueHints glue_hints(cur_zone_, incoming);
+
+                // Ask the NSAS for an address, or glue.
+                // This will eventually result in either sendTo()
+                // or stop() being called by nsas_callback_
+                assert(!nsas_callback_out_);
+                nsas_callback_out_ = true;
+                nsas_.lookup(cur_zone_, question_.getClass(),
+                             nsas_callback_, ANY_OK, glue_hints);
                 return false;
             } else {
                 dlog("No NS RRset in referral?");
@@ -478,6 +496,7 @@ public:
         callback_called_(false),
         nsas_(nsas),
         cache_(cache),
+        cur_zone_("."),
         nsas_callback_(new ResolverNSASCallback(this)),
         nsas_callback_out_(false),
         outstanding_events_(0)

src/lib/resolve/tests/Makefile.am

@@ -29,6 +29,7 @@ run_unittests_LDADD +=  $(top_builddir)/src/lib/cache/libcache.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/asiolink/libasiolink.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/resolve/libresolve.la
 run_unittests_LDADD +=  $(top_builddir)/src/lib/dns/libdns++.la
+run_unittests_LDADD +=  $(top_builddir)/src/lib/log/liblog.la
 
 endif
 

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -21,7 +21,6 @@
 #include <gtest/gtest.h>
 #include <boost/bind.hpp>
 
-
 #include <asio.hpp>
 
 #include <dns/buffer.h>
@@ -165,7 +164,7 @@ public:
     /// Sets up the common bits of a response message returned by the handlers.
     ///
     /// \param msg Message buffer in RENDER mode.
-    /// \param qid QIT to set the message to
+    /// \param qid QID to set the message to
     void setCommonMessage(isc::dns::Message& msg, uint16_t qid = 0) {
         msg.setQid(qid);
         msg.setHeaderFlag(Message::HEADERFLAG_QR);
@@ -278,11 +277,8 @@ public:
         // The QID in the incoming data is random so set it to 0 for the
         // data comparison check. (It is set to 0 in the buffer containing
         // the expected data.)
-        uint16_t qid = readUint16(udp_receive_buffer_);
-        udp_receive_buffer_[0] = udp_receive_buffer_[1] = 0;
-
-        // Check that question we received is what was expected.
-        checkReceivedPacket(udp_receive_buffer_, length);
+        // And check that question we received is what was expected.
+        uint16_t qid = checkReceivedPacket(udp_receive_buffer_, length);
 
         // The message returned depends on what state we are in.  Set up
         // common stuff first: bits not mentioned are set to 0.
@@ -433,18 +429,20 @@ public:
 
         // Check that question we received is what was expected.  Note that we
         // have to ignore the two-byte header in order to parse the message.
-        checkReceivedPacket(tcp_receive_buffer_ + 2, length - 2);
+        qid_t qid = checkReceivedPacket(tcp_receive_buffer_ + 2, length - 2);
 
         // Return a message back.  This is a referral to example.org, which
         // should result in another query over UDP.  Note the setting of the
         // QID in the returned message with what was in the received message.
         Message msg(Message::RENDER);
-        setCommonMessage(msg, readUint16(tcp_receive_buffer_));
+        setCommonMessage(msg, qid);
         setReferralExampleOrg(msg);
 
         // Convert to wire format
-        tcp_send_buffer_->clear();
-        MessageRenderer renderer(*tcp_send_buffer_);
+        // Use a temporary buffer for the dns wire data (we copy it
+        // to the 'real' buffer below)
+        OutputBuffer msg_buf(BUFFER_SIZE);
+        MessageRenderer renderer(msg_buf);
         msg.toWire(renderer);
 
         // Expected next state (when checked) is the UDP query to example.org.
@@ -455,16 +453,13 @@ public:
         expected_ = UDP_EXAMPLE_ORG;
         tcp_cumulative_ = 0;
 
-        // We'll write the message in two parts, the count and the message
-        // itself. This saves having to prepend the count onto the start of a
-        // buffer.  When specifying the send handler, the expected size of the
-        // data written is passed as the first parameter so that the handler
-        // can check it.
-        uint8_t count[2];
-        writeUint16(tcp_send_buffer_->getLength(), count);
-        tcp_socket_.async_send(asio::buffer(count, 2),
-                               boost::bind(&RecursiveQueryTest2::tcpSendHandler, this,
-                                           2, _1, _2));
+        // Unless we go through a callback loop we cannot simply use
+        // async_send() multiple times, so we cannot send the size first
+        // followed by the actual data. We copy them to a new buffer
+        // first
+        tcp_send_buffer_->clear();
+        tcp_send_buffer_->writeUint16(msg_buf.getLength());
+        tcp_send_buffer_->writeData(msg_buf.getData(), msg_buf.getLength());
         tcp_socket_.async_send(asio::buffer(tcp_send_buffer_->getData(),
                                             tcp_send_buffer_->getLength()),
                                boost::bind(&RecursiveQueryTest2::tcpSendHandler, this,
@@ -502,7 +497,8 @@ public:
     ///        the case of UDP data, and an offset into the buffer past the
     ///        count field for TCP data.
     /// \param length Length of data.
-    void checkReceivedPacket(uint8_t* data, size_t length) {
+    /// \return The QID of the message
+    qid_t checkReceivedPacket(uint8_t* data, size_t length) {
 
         // Decode the received buffer.
         InputBuffer buffer(data, length);
@@ -514,6 +510,8 @@ public:
 
         Question question = **(message.beginQuestion());
         EXPECT_TRUE(question == *question_);
+
+        return message.getQid();
     }
 };
 
@@ -539,6 +537,7 @@ public:
     virtual void success(const isc::dns::MessagePtr response) {
         if (debug_) {
             cout << "ResolverCallback::success(): answer received" << endl;
+            cout << response->toText() << endl;
         }
 
         // There should be one RR each  in the question and answer sections, and
@@ -607,7 +606,6 @@ private:
 // Sets up the UDP and TCP "servers", then tries a resolution.
 
 TEST_F(RecursiveQueryTest2, Resolve) {
-
     // Set up the UDP server and issue the first read.  The endpoint from which
     // the query is sent is put in udp_endpoint_ when the read completes, which
     // is referenced in the callback as the place to which the response is sent.