[svn] commit: r4093 - in /branches/trac454: ./ src/bin/auth/ src/bin/auth/tests/ src/bin/auth/tests/testdata/ src/bin/bind10/ src/bin/recurse/tests/ src/lib/asiolink/ src/lib/datasrc/ src/lib/datasrc/tests/ src/lib/datasrc/tests/testdata/ src/lib/log/ src/lib/python/isc/utils/ src/lib/testutils/ src/lib/testutils/testdata/

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Dec 30 09:38:52 UTC 2010


Author: jinmei
Date: Thu Dec 30 09:38:52 2010
New Revision: 4093

Log:
sync with trunk

Added:
    branches/trac454/src/lib/datasrc/tests/testdata/duplicate_rrset.zone
      - copied unchanged from r4092, trunk/src/lib/datasrc/tests/testdata/duplicate_rrset.zone
    branches/trac454/src/lib/testutils/testdata/example.com.zone
      - copied unchanged from r4092, trunk/src/lib/testutils/testdata/example.com.zone
    branches/trac454/src/lib/testutils/testdata/example.net.zone
      - copied unchanged from r4092, trunk/src/lib/testutils/testdata/example.net.zone
    branches/trac454/src/lib/testutils/testdata/example.org.zone
      - copied unchanged from r4092, trunk/src/lib/testutils/testdata/example.org.zone
    branches/trac454/src/lib/testutils/testdata/example.zone
      - copied unchanged from r4092, trunk/src/lib/testutils/testdata/example.zone
    branches/trac454/src/lib/testutils/testdata/iquery_fromWire.spec
      - copied unchanged from r4092, trunk/src/lib/testutils/testdata/iquery_fromWire.spec
    branches/trac454/src/lib/testutils/testdata/iquery_response_fromWire.spec
      - copied unchanged from r4092, trunk/src/lib/testutils/testdata/iquery_response_fromWire.spec
Modified:
    branches/trac454/   (props changed)
    branches/trac454/ChangeLog
    branches/trac454/src/bin/auth/auth_srv.cc
    branches/trac454/src/bin/auth/config.cc
    branches/trac454/src/bin/auth/query.cc
    branches/trac454/src/bin/auth/query.h
    branches/trac454/src/bin/auth/tests/auth_srv_unittest.cc
    branches/trac454/src/bin/auth/tests/config_unittest.cc
    branches/trac454/src/bin/auth/tests/query_unittest.cc
    branches/trac454/src/bin/auth/tests/testdata/   (props changed)
    branches/trac454/src/bin/auth/tests/testdata/queryBadEDNS_fromWire.spec
    branches/trac454/src/bin/bind10/bind10.py.in   (props changed)
    branches/trac454/src/bin/recurse/tests/   (props changed)
    branches/trac454/src/lib/asiolink/   (props changed)
    branches/trac454/src/lib/asiolink/asiolink.h
    branches/trac454/src/lib/datasrc/memory_datasrc.cc
    branches/trac454/src/lib/datasrc/memory_datasrc.h
    branches/trac454/src/lib/datasrc/rbtree.h
    branches/trac454/src/lib/datasrc/tests/memory_datasrc_unittest.cc
    branches/trac454/src/lib/datasrc/tests/rbtree_unittest.cc
    branches/trac454/src/lib/log/   (props changed)
    branches/trac454/src/lib/python/isc/utils/   (props changed)
    branches/trac454/src/lib/testutils/   (props changed)
    branches/trac454/src/lib/testutils/testdata/Makefile.am

Modified: branches/trac454/ChangeLog
==============================================================================
--- branches/trac454/ChangeLog (original)
+++ branches/trac454/ChangeLog Thu Dec 30 09:38:52 2010
@@ -1,3 +1,8 @@
+  141.	[bug]		jinmei
+	b10-auth: Fixed a bug that the authoritative server includes
+	trailing garbage data in responses.  This is a regression due to
+	change #135. (Trac #462, svn r4081)
+
   140.  [func]		y-aharen
 	src/bin/auth: Added a feature to count queries and send counter
 	values to statistics periodically. To support it, added wrapping

Modified: branches/trac454/src/bin/auth/auth_srv.cc
==============================================================================
--- branches/trac454/src/bin/auth/auth_srv.cc (original)
+++ branches/trac454/src/bin/auth/auth_srv.cc Thu Dec 30 09:38:52 2010
@@ -162,33 +162,20 @@
     AuthSrv* server_;
 };
 
-// This is a derived class of \c DNSAnswer, to serve as a
-// callback in the asiolink module.  It takes a completed
-// set of answer data from the DNS lookup and assembles it
-// into a wire-format response.
+// This is a derived class of \c DNSAnswer, to serve as a callback in the
+// asiolink module.  We actually shouldn't do anything in this class because
+// we build complete response messages in the process methods; otherwise
+// the response message will contain trailing garbage.  In future, we should
+// probably even drop the reliance on DNSAnswer.  We don't need the coroutine
+// tricks provided in that framework, and its overhead would be significant
+// in terms of performance consideration for the authoritative server
+// implementation.
 class MessageAnswer : public DNSAnswer {
 public:
-    MessageAnswer(AuthSrv* srv) : server_(srv) {}
-    virtual void operator()(const IOMessage& io_message, MessagePtr message,
-                            OutputBufferPtr buffer) const
-    {
-        MessageRenderer renderer(*buffer);
-        if (io_message.getSocket().getProtocol() == IPPROTO_UDP) {
-            ConstEDNSPtr edns(message->getEDNS());
-            renderer.setLengthLimit(edns ? edns->getUDPSize() :
-                Message::DEFAULT_MAX_UDPSIZE);
-        } else {
-            renderer.setLengthLimit(65535);
-        }
-        message->toWire(renderer);
-        if (server_->getVerbose()) {
-            cerr << "[b10-auth] sending a response (" << renderer.getLength()
-                 << " bytes):\n" << message->toText() << endl;
-        }
-    }
-
-private:
-    AuthSrv* server_;
+    MessageAnswer(AuthSrv*) {}
+    virtual void operator()(const IOMessage&, MessagePtr,
+                            OutputBufferPtr) const
+    {}
 };
 
 // This is a derived class of \c SimpleCallback, to serve

Modified: branches/trac454/src/bin/auth/config.cc
==============================================================================
--- branches/trac454/src/bin/auth/config.cc (original)
+++ branches/trac454/src/bin/auth/config.cc Thu Dec 30 09:38:52 2010
@@ -151,16 +151,21 @@
             isc_throw(AuthConfigError, "Missing zone file for zone: "
                       << origin->str());
         }
-        const result::Result result = memory_datasrc_->addZone(
-            ZonePtr(new MemoryZone(rrclass_, Name(origin->stringValue()))));
+        shared_ptr<MemoryZone> new_zone(new MemoryZone(rrclass_,
+            Name(origin->stringValue())));
+        const result::Result result = memory_datasrc_->addZone(new_zone);
         if (result == result::EXIST) {
             isc_throw(AuthConfigError, "zone "<< origin->str()
                       << " already exists");
         }
 
-        // TODO
-        // then load the zone from 'file', which is currently not implemented.
-        //
+        /*
+         * TODO: Once we have better reloading of configuration (something
+         * else than throwing everything away and loading it again), we will
+         * need the load method to be split into some kind of build and
+         * commit/abort parts.
+         */
+        new_zone->load(file->stringValue());
     }
 }
 

Modified: branches/trac454/src/bin/auth/query.cc
==============================================================================
--- branches/trac454/src/bin/auth/query.cc (original)
+++ branches/trac454/src/bin/auth/query.cc Thu Dec 30 09:38:52 2010
@@ -24,6 +24,24 @@
 
 namespace isc {
 namespace auth {
+
+void
+Query::putSOA(const Zone& zone) const {
+    Zone::FindResult soa_result(zone.find(zone.getOrigin(),
+        RRType::SOA()));
+    if (soa_result.code != Zone::SUCCESS) {
+        isc_throw(NoSOA, "There's no SOA record in zone " <<
+            zone.getOrigin().toText());
+    } else {
+        /*
+         * FIXME:
+         * The const-cast is wrong, but the Message interface seems
+         * to insist.
+         */
+        response_.addRRset(Message::SECTION_AUTHORITY,
+            boost::const_pointer_cast<RRset>(soa_result.rrset));
+    }
+}
 
 void
 Query::process() const {
@@ -59,12 +77,14 @@
                 // TODO : add NS to authority section, fill in additional section.
                 break;
             case Zone::NXDOMAIN:
+                // Just empty answer with SOA in authority section
                 response_.setRcode(Rcode::NXDOMAIN());
-                // TODO : add SOA to authority section
+                putSOA(*result.zone);
                 break;
             case Zone::NXRRSET:
+                // Just empty answer with SOA in authority section
                 response_.setRcode(Rcode::NOERROR());
-                // TODO : add SOA to authority section
+                putSOA(*result.zone);
                 break;
             case Zone::CNAME:
             case Zone::DNAME:

Modified: branches/trac454/src/bin/auth/query.h
==============================================================================
--- branches/trac454/src/bin/auth/query.h (original)
+++ branches/trac454/src/bin/auth/query.h Thu Dec 30 09:38:52 2010
@@ -14,6 +14,8 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <exceptions/exceptions.h>
+
 namespace isc {
 namespace dns {
 class Message;
@@ -23,6 +25,7 @@
 
 namespace datasrc {
 class MemoryDataSrc;
+class Zone;
 }
 
 namespace auth {
@@ -100,15 +103,46 @@
     /// providing compatible behavior may have its own benefit, so this point
     /// should be revisited later.
     ///
-    /// Right now this method never throws an exception, but it may in a
-    /// future version.
+    /// This might throw BadZone or any of its specific subclasses, but that
+    /// shouldn't happen in real-life (as BadZone means wrong data, it should
+    /// have been rejected upon loading).
     void process() const;
+
+    /// \short Bad zone data encountered.
+    ///
+    /// This is thrown when process encounteres misconfigured zone in a way
+    /// it can't continue. This throws, not sets the Rcode, because such
+    /// misconfigured zone should not be present in the data source and
+    /// should have been rejected sooner.
+    struct BadZone : public isc::Exception {
+        BadZone(const char* file, size_t line, const char* what) :
+            Exception(file, line, what)
+        {}
+    };
+
+    /// \short Zone is missing its SOA record.
+    ///
+    /// We tried to add a SOA into the authoritative section, but the zone
+    /// does not contain one.
+    struct NoSOA : public BadZone {
+        NoSOA(const char* file, size_t line, const char* what) :
+            BadZone(file, line, what)
+        {}
+    };
 
 private:
     const isc::datasrc::MemoryDataSrc& memory_datasrc_;
     const isc::dns::Name& qname_;
     const isc::dns::RRType& qtype_;
     isc::dns::Message& response_;
+
+    /**
+     * \short Adds a SOA.
+     *
+     * Adds a SOA of the zone into the authority zone of response_.
+     * Can throw NoSOA.
+     */
+    void putSOA(const isc::datasrc::Zone& zone) const;
 };
 
 }

Modified: branches/trac454/src/bin/auth/tests/auth_srv_unittest.cc
==============================================================================
--- branches/trac454/src/bin/auth/tests/auth_srv_unittest.cc (original)
+++ branches/trac454/src/bin/auth/tests/auth_srv_unittest.cc Thu Dec 30 09:38:52 2010
@@ -15,6 +15,19 @@
 // $Id$
 
 #include <config.h>
+
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include <dns/message.h>
+#include <dns/messagerenderer.h>
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+#include <dns/rrttl.h>
+#include <dns/rdataclass.h>
+
 #include <datasrc/memory_datasrc.h>
 #include <auth/auth_srv.h>
 #include <testutils/srv_unittest.h>
@@ -22,6 +35,7 @@
 
 using namespace isc::cc;
 using namespace isc::dns;
+using namespace isc::dns::rdata;
 using namespace isc::data;
 using namespace isc::xfr;
 using namespace asiolink;
@@ -45,7 +59,103 @@
     MockXfroutClient xfrout;
     AuthSrv server;
     const RRClass rrclass;
+    vector<uint8_t> response_data;
 };
+
+// A helper function that builds a response to version.bind/TXT/CH that
+// should be identical to the response from our builtin (static) data source
+// by default.  The resulting wire-format data will be stored in 'data'.
+void
+createBuiltinVersionResponse(const qid_t qid, vector<uint8_t>& data) {
+    const Name version_name("version.bind");
+    Message message(Message::RENDER);
+
+    UnitTestUtil::createRequestMessage(message, Opcode::QUERY(),
+                                       qid, version_name,
+                                       RRClass::CH(), RRType::TXT());
+    message.setHeaderFlag(Message::HEADERFLAG_QR);
+    message.setHeaderFlag(Message::HEADERFLAG_AA);
+    RRsetPtr rrset_version = RRsetPtr(new RRset(version_name, RRClass::CH(),
+                                                RRType::TXT(), RRTTL(0)));
+    rrset_version->addRdata(generic::TXT(PACKAGE_STRING));
+    message.addRRset(Message::SECTION_ANSWER, rrset_version);
+
+    RRsetPtr rrset_version_ns = RRsetPtr(new RRset(version_name, RRClass::CH(),
+                                                   RRType::NS(), RRTTL(0)));
+    rrset_version_ns->addRdata(generic::NS(version_name));
+    message.addRRset(Message::SECTION_AUTHORITY, rrset_version_ns);
+
+    OutputBuffer obuffer(0);
+    MessageRenderer renderer(obuffer);
+    message.toWire(renderer);
+
+    data.clear();
+    data.assign(static_cast<const uint8_t*>(renderer.getData()),
+                static_cast<const uint8_t*>(renderer.getData()) +
+                renderer.getLength());
+}
+
+// In the following tests we confirm the response data is rendered in
+// wire format in the expected way.
+
+// The most primitive check: checking the result of the processMessage()
+// method
+TEST_F(AuthSrvTest, builtInQuery) {
+    UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
+                                       default_qid, Name("version.bind"),
+                                       RRClass::CH(), RRType::TXT());
+    createRequestPacket(request_message, IPPROTO_UDP);
+    server.processMessage(*io_message, parse_message, response_obuffer,
+                          &dnsserv);
+    createBuiltinVersionResponse(default_qid, response_data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        response_obuffer->getData(),
+                        response_obuffer->getLength(),
+                        &response_data[0], response_data.size());
+}
+
+// Same test emulating the UDPServer class behavior (defined in libasiolink).
+// This is not a good test in that it assumes internal implementation details
+// of UDPServer, but we've encountered a regression due to the introduction
+// of that class, so we add a test for that case to prevent such a regression
+// in future.
+// Besides, the generalization of UDPServer is probably too much for the
+// authoritative only server in terms of performance, and it's quite likely
+// we need to drop it for the authoritative server implementation.
+// At that point we can drop this test, too.
+TEST_F(AuthSrvTest, builtInQueryViaDNSServer) {
+    UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(),
+                                       default_qid, Name("version.bind"),
+                                       RRClass::CH(), RRType::TXT());
+    createRequestPacket(request_message, IPPROTO_UDP);
+
+    (*server.getDNSLookupProvider())(*io_message, parse_message,
+                                     response_obuffer, &dnsserv);
+    (*server.getDNSAnswerProvider())(*io_message, parse_message,
+                                     response_obuffer);
+
+    createBuiltinVersionResponse(default_qid, response_data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        response_obuffer->getData(),
+                        response_obuffer->getLength(),
+                        &response_data[0], response_data.size());
+}
+
+// Same type of test as builtInQueryViaDNSServer but for an error response.
+TEST_F(AuthSrvTest, iqueryViaDNSServer) {
+    createDataFromFile("iquery_fromWire.wire");
+    (*server.getDNSLookupProvider())(*io_message, parse_message,
+                                     response_obuffer, &dnsserv);
+    (*server.getDNSAnswerProvider())(*io_message, parse_message,
+                                     response_obuffer);
+
+    UnitTestUtil::readWireData("iquery_response_fromWire.wire",
+                               response_data);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        response_obuffer->getData(),
+                        response_obuffer->getLength(),
+                        &response_data[0], response_data.size());
+}
 
 // Unsupported requests.  Should result in NOTIMP.
 TEST_F(AuthSrvTest, unsupportedRequest) {

Modified: branches/trac454/src/bin/auth/tests/config_unittest.cc
==============================================================================
--- branches/trac454/src/bin/auth/tests/config_unittest.cc (original)
+++ branches/trac454/src/bin/auth/tests/config_unittest.cc Thu Dec 30 09:38:52 2010
@@ -17,6 +17,7 @@
 #include <exceptions/exceptions.h>
 
 #include <dns/rrclass.h>
+#include <dns/masterload.h>
 
 #include <cc/data.h>
 
@@ -143,33 +144,42 @@
 }
 
 TEST_F(MemoryDatasrcConfigTest, addOneZone) {
-    parser->build(Element::fromJSON(
-                      "[{\"type\": \"memory\","
-                      "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"}]}]"));
-    parser->commit();
-    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.com\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
+    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    // Check it actually loaded something
+    EXPECT_EQ(Zone::SUCCESS, server.getMemoryDataSrc(rrclass)->findZone(
+        Name("ns.example.com.")).zone->find(Name("ns.example.com."),
+        RRType::A()).code);
 }
 
 TEST_F(MemoryDatasrcConfigTest, addMultiZones) {
-    parser->build(Element::fromJSON(
-                      "[{\"type\": \"memory\","
-                      "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"},"
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.com\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"},"
                       "              {\"origin\": \"example.org\","
-                      "               \"file\": \"example.org.zone\"},"
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.org.zone\"},"
                       "              {\"origin\": \"example.net\","
-                      "               \"file\": \"example.net.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.net.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(3, server.getMemoryDataSrc(rrclass)->getZoneCount());
 }
 
 TEST_F(MemoryDatasrcConfigTest, replace) {
-    parser->build(Element::fromJSON(
-                      "[{\"type\": \"memory\","
-                      "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"}]}]"));
-    parser->commit();
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.com\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::SUCCESS,
               server.getMemoryDataSrc(rrclass)->findZone(
@@ -179,31 +189,69 @@
     // should replace the old one.
     delete parser;
     parser = createAuthConfigParser(server, "datasources"); 
-    parser->build(Element::fromJSON(
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
                       "[{\"type\": \"memory\","
                       "  \"zones\": [{\"origin\": \"example.org\","
-                      "               \"file\": \"example.org.zone\"},"
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.org.zone\"},"
                       "              {\"origin\": \"example.net\","
-                      "               \"file\": \"example.net.zone\"}]}]"));
-    parser->commit();
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.net.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(2, server.getMemoryDataSrc(rrclass)->getZoneCount());
     EXPECT_EQ(isc::datasrc::result::NOTFOUND,
               server.getMemoryDataSrc(rrclass)->findZone(
                   Name("example.com")).code);
 }
 
+TEST_F(MemoryDatasrcConfigTest, exception) {
+    // Load a zone
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.com\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
+    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(isc::datasrc::result::SUCCESS,
+              server.getMemoryDataSrc(rrclass)->findZone(
+                  Name("example.com")).code);
+
+    // create a new parser, and try to load something. It will throw,
+    // the given master file should not exist
+    delete parser;
+    parser = createAuthConfigParser(server, "datasources");
+    EXPECT_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.org\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.org.zone\"},"
+                      "              {\"origin\": \"example.net\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/nonexistent.zone\"}]}]")), isc::dns::MasterLoadError);
+    // As that one throwed exception, it is not expected from us to
+    // commit it
+
+    // The original should be untouched
+    EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
+    EXPECT_EQ(isc::datasrc::result::SUCCESS,
+              server.getMemoryDataSrc(rrclass)->findZone(
+                  Name("example.com")).code);
+}
+
 TEST_F(MemoryDatasrcConfigTest, remove) {
-    parser->build(Element::fromJSON(
-                      "[{\"type\": \"memory\","
-                      "  \"zones\": [{\"origin\": \"example.com\","
-                      "               \"file\": \"example.zone\"}]}]"));
-    parser->commit();
+    EXPECT_NO_THROW(parser->build(Element::fromJSON(
+                      "[{\"type\": \"memory\","
+                      "  \"zones\": [{\"origin\": \"example.com\","
+                      "               \"file\": \"" TEST_DATA_DIR
+                      "/example.zone\"}]}]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(1, server.getMemoryDataSrc(rrclass)->getZoneCount());
 
     delete parser;
     parser = createAuthConfigParser(server, "datasources"); 
-    parser->build(Element::fromJSON("[]"));
-    parser->commit();
+    EXPECT_NO_THROW(parser->build(Element::fromJSON("[]")));
+    EXPECT_NO_THROW(parser->commit());
     EXPECT_EQ(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
 }
 
@@ -212,9 +260,11 @@
                      Element::fromJSON(
                          "[{\"type\": \"memory\","
                          "  \"zones\": [{\"origin\": \"example.com\","
-                         "               \"file\": \"example.zone\"},"
+                         "               \"file\": \"" TEST_DATA_DIR
+                         "/example.zone\"},"
                          "              {\"origin\": \"example.com\","
-                         "               \"file\": \"example.com.zone\"}]}]")),
+                         "               \"file\": \"" TEST_DATA_DIR
+                         "/example.com.zone\"}]}]")),
                  AuthConfigError);
 }
 

Modified: branches/trac454/src/bin/auth/tests/query_unittest.cc
==============================================================================
--- branches/trac454/src/bin/auth/tests/query_unittest.cc (original)
+++ branches/trac454/src/bin/auth/tests/query_unittest.cc Thu Dec 30 09:38:52 2010
@@ -28,10 +28,14 @@
 using namespace isc::datasrc;
 using namespace isc::auth;
 
+namespace {
+
 RRsetPtr a_rrset = RRsetPtr(new RRset(Name("www.example.com"),
                                       RRClass::IN(), RRType::A(),
                                       RRTTL(3600)));
-namespace {
+RRsetPtr soa_rrset = RRsetPtr(new RRset(Name("example.com"),
+                                        RRClass::IN(), RRType::SOA(),
+                                        RRTTL(3600)));
 // This is a mock Zone class for testing.
 // It is a derived class of Zone, and simply hardcode the results of find()
 // return SUCCESS for "www.example.com",
@@ -41,7 +45,9 @@
 // else return DNAME
 class MockZone : public Zone {
 public:
-    MockZone() : origin_(Name("example.com"))
+    MockZone(bool has_SOA = true) :
+        origin_(Name("example.com")),
+        has_SOA_(has_SOA)
     {}
     virtual const isc::dns::Name& getOrigin() const;
     virtual const isc::dns::RRClass& getClass() const;
@@ -51,6 +57,7 @@
 
 private:
     Name origin_;
+    bool has_SOA_;
 };
 
 const Name&
@@ -64,20 +71,24 @@
 }
 
 Zone::FindResult
-MockZone::find(const Name& name, const RRType&) const {
+MockZone::find(const Name& name, const RRType& type) const {
     // hardcode the find results
     if (name == Name("www.example.com")) {
-        return FindResult(SUCCESS, a_rrset);
+        return (FindResult(SUCCESS, a_rrset));
+    } else if (name == Name("example.com") && type == RRType::SOA() &&
+        has_SOA_)
+    {
+        return (FindResult(SUCCESS, soa_rrset));
     } else if (name == Name("delegation.example.com")) {
-        return FindResult(DELEGATION, RRsetPtr());
+        return (FindResult(DELEGATION, RRsetPtr()));
     } else if (name == Name("nxdomain.example.com")) {
-        return FindResult(NXDOMAIN, RRsetPtr());
+        return (FindResult(NXDOMAIN, RRsetPtr()));
     } else if (name == Name("nxrrset.example.com")) {
-        return FindResult(NXRRSET, RRsetPtr());
+        return (FindResult(NXRRSET, RRsetPtr()));
     } else if (name == Name("cname.example.com")) {
-        return FindResult(CNAME, RRsetPtr());
+        return (FindResult(CNAME, RRsetPtr()));
     } else {
-        return FindResult(DNAME, RRsetPtr());
+        return (FindResult(DNAME, RRsetPtr()));
     }
 }
 
@@ -106,7 +117,7 @@
 }
 
 TEST_F(QueryTest, matchZone) {
-    // match qname, normal query
+    // add a matching zone.
     memory_datasrc.addZone(ZonePtr(new MockZone()));
     query.process();
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
@@ -119,12 +130,39 @@
     Query nxdomain_query(memory_datasrc, nxdomain_name, qtype, response);
     nxdomain_query.process();
     EXPECT_EQ(Rcode::NXDOMAIN(), response.getRcode());
+    EXPECT_EQ(0, response.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(0, response.getRRCount(Message::SECTION_ADDITIONAL));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+        Name("example.com"), RRClass::IN(), RRType::SOA()));
 
     // NXRRSET
     const Name nxrrset_name(Name("nxrrset.example.com"));
     Query nxrrset_query(memory_datasrc, nxrrset_name, qtype, response);
     nxrrset_query.process();
     EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
+    EXPECT_EQ(0, response.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(0, response.getRRCount(Message::SECTION_ADDITIONAL));
+    EXPECT_TRUE(response.hasRRset(Message::SECTION_AUTHORITY,
+        Name("example.com"), RRClass::IN(), RRType::SOA()));
+}
+
+/*
+ * This tests that when there's no SOA and we need a negative answer. It should
+ * throw in that case.
+ */
+TEST_F(QueryTest, noSOA) {
+    memory_datasrc.addZone(ZonePtr(new MockZone(false)));
+
+    // The NX Domain
+    const Name nxdomain_name(Name("nxdomain.example.com"));
+    Query nxdomain_query(memory_datasrc, nxdomain_name, qtype, response);
+    EXPECT_THROW(nxdomain_query.process(), Query::NoSOA);
+    // Of course, we don't look into the response, as it throwed
+
+    // NXRRSET
+    const Name nxrrset_name(Name("nxrrset.example.com"));
+    Query nxrrset_query(memory_datasrc, nxrrset_name, qtype, response);
+    EXPECT_THROW(nxrrset_query.process(), Query::NoSOA);
 }
 
 TEST_F(QueryTest, noMatchZone) {
@@ -136,4 +174,5 @@
     nomatch_query.process();
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
+
 }

Modified: branches/trac454/src/bin/auth/tests/testdata/queryBadEDNS_fromWire.spec
==============================================================================
--- branches/trac454/src/bin/auth/tests/testdata/queryBadEDNS_fromWire.spec (original)
+++ branches/trac454/src/bin/auth/tests/testdata/queryBadEDNS_fromWire.spec Thu Dec 30 09:38:52 2010
@@ -1,5 +1,5 @@
 #
-# A QUERY message with unsupported version of EDNS..
+# A QUERY message with unsupported version of EDNS.
 #
 
 [header]

Modified: branches/trac454/src/lib/asiolink/asiolink.h
==============================================================================
--- branches/trac454/src/lib/asiolink/asiolink.h (original)
+++ branches/trac454/src/lib/asiolink/asiolink.h Thu Dec 30 09:38:52 2010
@@ -235,7 +235,7 @@
     /// that share the same \c io_service with the authoritative server.
     /// It will eventually be removed once the wrapper interface is
     /// generalized.
-    asio::io_service& get_io_service() { return io_service_.get_io_service(); };
+    asio::io_service& get_io_service() { return io_service_.get_io_service(); }
 private:
     DNSServiceImpl* impl_;
     IOService& io_service_;

Modified: branches/trac454/src/lib/datasrc/memory_datasrc.cc
==============================================================================
--- branches/trac454/src/lib/datasrc/memory_datasrc.cc (original)
+++ branches/trac454/src/lib/datasrc/memory_datasrc.cc Thu Dec 30 09:38:52 2010
@@ -15,9 +15,11 @@
 #include <map>
 #include <cassert>
 #include <boost/shared_ptr.hpp>
+#include <boost/bind.hpp>
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
+#include <dns/masterload.h>
 
 #include <datasrc/memory_datasrc.h>
 #include <datasrc/rbtree.h>
@@ -65,7 +67,7 @@
      * access is without the impl_-> and it will get inlined anyway.
      */
     // Implementation of MemoryZone::add
-    result::Result add(const ConstRRsetPtr& rrset) {
+    result::Result add(const ConstRRsetPtr& rrset, DomainTree* domains) {
         // Sanitize input
         if (!rrset) {
             isc_throw(NullRRset, "The rrset provided is NULL");
@@ -80,7 +82,7 @@
         }
         // Get the node
         DomainNode* node;
-        switch (domains_.insert(name, &node)) {
+        switch (domains->insert(name, &node)) {
             // Just check it returns reasonable results
             case DomainTree::SUCCEED:
             case DomainTree::ALREADYEXIST:
@@ -118,6 +120,22 @@
             // The RRSet of given type was already there
             return (result::EXIST);
         }
+    }
+
+    /*
+     * Same as above, but it checks the return value and if it already exists,
+     * it throws.
+     */
+    void addFromLoad(const ConstRRsetPtr& set, DomainTree* domains) {
+            switch (add(set, domains)) {
+                case result::EXIST:
+                    isc_throw(dns::MasterLoadError, "Duplicate rrset: " <<
+                        set->toText());
+                case result::SUCCESS:
+                    return;
+                default:
+                    assert(0);
+            }
     }
 
     // Maintain intermediate data specific to the search context used in
@@ -235,7 +253,19 @@
 
 result::Result
 MemoryZone::add(const ConstRRsetPtr& rrset) {
-    return (impl_->add(rrset));
+    return (impl_->add(rrset, &impl_->domains_));
+}
+
+
+void
+MemoryZone::load(const string& filename) {
+    // Load it into a temporary tree
+    MemoryZoneImpl::DomainTree tmp;
+    masterLoad(filename.c_str(), getOrigin(), getClass(),
+        boost::bind(&MemoryZoneImpl::addFromLoad, impl_, _1, &tmp));
+    // If it went well, put it inside
+    tmp.swap(impl_->domains_);
+    // And let the old data die with tmp
 }
 
 /// Implementation details for \c MemoryDataSrc hidden from the public

Modified: branches/trac454/src/lib/datasrc/memory_datasrc.h
==============================================================================
--- branches/trac454/src/lib/datasrc/memory_datasrc.h (original)
+++ branches/trac454/src/lib/datasrc/memory_datasrc.h Thu Dec 30 09:38:52 2010
@@ -97,6 +97,30 @@
         { }
     };
 
+    /// \brief Load zone from masterfile.
+    ///
+    /// This loads data from masterfile specified by filename. It replaces
+    /// current content. The masterfile parsing ability is kind of limited,
+    /// see isc::dns::masterLoad.
+    ///
+    /// This throws isc::dns::MasterLoadError if there is problem with loading
+    /// (missing file, malformed, it contains different zone, etc - see
+    /// isc::dns::masterLoad for details).
+    ///
+    /// In case of internal problems, OutOfZone, NullRRset or AssertError could
+    /// be thrown, but they should not be expected. Exceptions caused by
+    /// allocation may be thrown as well.
+    ///
+    /// If anything is thrown, the previous content is preserved (so it can
+    /// be used to update the data, but if user makes a typo, the old one
+    /// is kept).
+    ///
+    /// \param filename The master file to load.
+    ///
+    /// \todo We may need to split it to some kind of build and commit/abort.
+    ///     This will probably be needed when a better implementation of
+    ///     configuration reloading is written.
+    void load(const std::string& filename);
 private:
     /// \name Hidden private data
     //@{

Modified: branches/trac454/src/lib/datasrc/rbtree.h
==============================================================================
--- branches/trac454/src/lib/datasrc/rbtree.h (original)
+++ branches/trac454/src/lib/datasrc/rbtree.h Thu Dec 30 09:38:52 2010
@@ -412,6 +412,16 @@
     Result insert(const isc::dns::Name& name, RBNode<T>** inserted_node);
     //@}
 
+    /// \brief Swaps two tree's contents.
+    ///
+    /// This acts the same as many std::*.swap functions, exchanges the
+    /// contents. This doesn't throw anything.
+    void swap(RBTree<T>& other) {
+        std::swap(root_, other.root_);
+        std::swap(NULLNODE, other.NULLNODE);
+        std::swap(node_count_, other.node_count_);
+    }
+
 private:
     /// \name RBTree balance functions
     //@{

Modified: branches/trac454/src/lib/datasrc/tests/memory_datasrc_unittest.cc
==============================================================================
--- branches/trac454/src/lib/datasrc/tests/memory_datasrc_unittest.cc (original)
+++ branches/trac454/src/lib/datasrc/tests/memory_datasrc_unittest.cc Thu Dec 30 09:38:52 2010
@@ -17,6 +17,7 @@
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
+#include <dns/masterload.h>
 
 #include <datasrc/memory_datasrc.h>
 
@@ -192,18 +193,28 @@
      * \param name The name to ask for.
      * \param rrtype The RRType to ask of.
      * \param result The expected code of the result.
+     * \param check_answer Should a check against equality of the answer be
+     *     done?
      * \param answer The expected rrset, if any should be returned.
+     * \param zone Check different MemoryZone object than zone_ (if NULL,
+     *     uses zone_)
      */
     void findTest(const Name& name, const RRType& rrtype, Zone::Result result,
-        const ConstRRsetPtr& answer = ConstRRsetPtr())
+        bool check_answer = true,
+        const ConstRRsetPtr& answer = ConstRRsetPtr(), MemoryZone *zone = NULL)
     {
+        if (!zone) {
+            zone = &zone_;
+        }
         // The whole block is inside, because we need to check the result and
         // we can't assign to FindResult
         EXPECT_NO_THROW({
-            Zone::FindResult find_result(zone_.find(name, rrtype));
+            Zone::FindResult find_result(zone->find(name, rrtype));
             // Check it returns correct answers
             EXPECT_EQ(result, find_result.code);
-            EXPECT_EQ(answer, find_result.rrset);
+            if (check_answer) {
+                EXPECT_EQ(answer, find_result.rrset);
+            }
         });
     }
 };
@@ -251,22 +262,22 @@
 
     // below the zone cut
     findTest(Name("www.child.example.org"), RRType::A(), Zone::DELEGATION,
-             rr_child_ns_);
+             true, rr_child_ns_);
 
     // at the zone cut
     findTest(Name("child.example.org"), RRType::A(), Zone::DELEGATION,
-             rr_child_ns_);
+             true, rr_child_ns_);
     findTest(Name("child.example.org"), RRType::NS(), Zone::DELEGATION,
-             rr_child_ns_);
+             true, rr_child_ns_);
 
     // finding NS for the apex (origin) node.  This must not be confused
     // with delegation due to the existence of an NS RR.
-    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_);
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, true, rr_ns_);
 
     // unusual case of "nested delegation": the highest cut should be used.
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_grandchild_ns_)));
     findTest(Name("www.grand.child.example.org"), RRType::A(),
-             Zone::DELEGATION, rr_child_ns_); // note: not rr_grandchild_ns_
+             Zone::DELEGATION, true, rr_child_ns_); // note: !rr_grandchild_ns_
 }
 
 // Test adding DNAMEs and resulting delegation handling
@@ -297,8 +308,8 @@
     EXPECT_NO_THROW(EXPECT_EQ(SUCCESS, zone_.add(rr_a_)));
 
     // These two should be successful
-    findTest(origin_, RRType::NS(), Zone::SUCCESS, rr_ns_);
-    findTest(ns_name_, RRType::A(), Zone::SUCCESS, rr_ns_a_);
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, true, rr_ns_);
+    findTest(ns_name_, RRType::A(), Zone::SUCCESS, true, rr_ns_a_);
 
     // These domain exist but don't have the provided RRType
     findTest(origin_, RRType::AAAA(), Zone::NXRRSET);
@@ -309,4 +320,34 @@
     findTest(Name("example.net"), RRType::A(), Zone::NXDOMAIN);
 }
 
-}
+TEST_F(MemoryZoneTest, load) {
+    // Put some data inside the zone
+    EXPECT_NO_THROW(EXPECT_EQ(result::SUCCESS, zone_.add(rr_ns_)));
+    // Loading with different origin should fail
+    EXPECT_THROW(zone_.load(TEST_DATA_DIR "/root.zone"), MasterLoadError);
+    // See the original data is still there, survived the exception
+    findTest(origin_, RRType::NS(), Zone::SUCCESS, true, rr_ns_);
+    // Create correct zone
+    MemoryZone rootzone(class_, Name("."));
+    // Try putting something inside
+    EXPECT_NO_THROW(EXPECT_EQ(result::SUCCESS, rootzone.add(rr_ns_aaaa_)));
+    // Load the zone. It should overwrite/remove the above RRset
+    EXPECT_NO_THROW(rootzone.load(TEST_DATA_DIR "/root.zone"));
+
+    // Now see there are some rrsets (we don't look inside, though)
+    findTest(Name("."), RRType::SOA(), Zone::SUCCESS, false, ConstRRsetPtr(),
+        &rootzone);
+    findTest(Name("."), RRType::NS(), Zone::SUCCESS, false, ConstRRsetPtr(),
+        &rootzone);
+    findTest(Name("a.root-servers.net."), RRType::A(), Zone::SUCCESS, false,
+        ConstRRsetPtr(), &rootzone);
+    // But this should no longer be here
+    findTest(ns_name_, RRType::AAAA(), Zone::NXDOMAIN, true, ConstRRsetPtr(),
+        &rootzone);
+
+    // Try loading zone that is wrong in a different way
+    EXPECT_THROW(zone_.load(TEST_DATA_DIR "/duplicate_rrset.zone"),
+        MasterLoadError);
+}
+
+}

Modified: branches/trac454/src/lib/datasrc/tests/rbtree_unittest.cc
==============================================================================
--- branches/trac454/src/lib/datasrc/tests/rbtree_unittest.cc (original)
+++ branches/trac454/src/lib/datasrc/tests/rbtree_unittest.cc Thu Dec 30 09:38:52 2010
@@ -231,4 +231,34 @@
     str2 << "tree has 13 node(s)\nb. (black)\n     a. (black)\n          NULL\n          NULL\n     d.e.f. (black)[invisible] \n          begin down from d.e.f.\n          w.y. (black)[invisible] \n               begin down from w.y.\n               p. (black)\n                    o. (red)\n                         NULL\n                         NULL\n                    q. (red)\n                         NULL\n                         NULL\n               end down from w.y.\n               x. (red)\n                    NULL\n                    NULL\n               z. (red)\n                    begin down from z.\n                    j. (black)\n                         NULL\n                         NULL\n                    end down from z.\n                    NULL\n                    NULL\n          end down from d.e.f.\n          c. (red)\n               NULL\n               NULL\n          g.h. (red)\n               begin down from g.h.\n               i. (black)\n  
                   NULL\n                    NULL\n               end down from g.h.\n               NULL\n               NULL\n";
     EXPECT_EQ(str.str(), str2.str());
 }
-}
+
+TEST_F(RBTreeTest, swap) {
+    // Store info about the first tree
+    std::ostringstream str1;
+    rbtree.dumpTree(str1);
+    size_t count1(rbtree.getNodeCount());
+
+    // Create second one and store state
+    RBTree<int> tree2;
+    RBNode<int>* node;
+    tree2.insert(Name("second"), &node);
+    std::ostringstream str2;
+    tree2.dumpTree(str2);
+
+    // Swap them
+    ASSERT_NO_THROW(tree2.swap(rbtree));
+
+    // Check their sizes
+    ASSERT_EQ(1, rbtree.getNodeCount());
+    ASSERT_EQ(count1, tree2.getNodeCount());
+
+    // And content
+    std::ostringstream out;
+    rbtree.dumpTree(out);
+    ASSERT_EQ(str2.str(), out.str());
+    out.str("");
+    tree2.dumpTree(out);
+    ASSERT_EQ(str1.str(), out.str());
+}
+
+}

Modified: branches/trac454/src/lib/testutils/testdata/Makefile.am
==============================================================================
--- branches/trac454/src/lib/testutils/testdata/Makefile.am (original)
+++ branches/trac454/src/lib/testutils/testdata/Makefile.am Thu Dec 30 09:38:52 2010
@@ -4,6 +4,7 @@
 BUILT_SOURCES += iqueryresponse_fromWire.wire multiquestion_fromWire.wire
 BUILT_SOURCES += queryBadEDNS_fromWire.wire shortanswer_fromWire.wire
 BUILT_SOURCES += simplequery_fromWire.wire simpleresponse_fromWire.wire
+BUILT_SOURCES += iquery_fromWire.wire iquery_response_fromWire.wire
 
 # NOTE: keep this in sync with real file listing
 # so is included in tarball
@@ -18,6 +19,8 @@
 EXTRA_DIST += shortresponse_fromWire
 EXTRA_DIST += simplequery_fromWire.spec
 EXTRA_DIST += simpleresponse_fromWire.spec
+EXTRA_DIST += iquery_fromWire.spec iquery_response_fromWire.spec
+EXTRA_DIST += example.com.zone example.net.zone example.org.zone example.zone
 
 EXTRA_DIST += example.com
 EXTRA_DIST += example.sqlite3




More information about the bind10-changes mailing list