cleanup/correction to the query logic with in memory data source:

 - we should return REFUSED (as opposed to SERVFAIL) when we cannot find a matchign zone as we discussed at the bind10-dev list
 - we shouldn't use the meory data source unless the query class is IN (which is the only supported class at the moment)
 - the "NXRRSET" result of Zone::find() should actually result in "NOERROR" rcode in the response (NXRRSET is dynamic update specific)


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

4 files changed, 42 insertions, 19 deletions

diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 67cd18bd99..35accd3d2e 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -95,6 +95,7 @@ public:
     AbstractSession* xfrin_session_;
 
     /// In-memory data source.  Currently class IN only for simplicity.
+    const RRClass memory_datasrc_class_;
     AuthSrv::MemoryDataSrcPtr memory_datasrc_;
 
     /// Hot spot cache
@@ -116,6 +117,7 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
                          AbstractXfroutClient& xfrout_client) :
     config_session_(NULL), verbose_mode_(false),
     xfrin_session_(NULL),
+    memory_datasrc_class_(RRClass::IN()),
     xfrout_connected_(false),
     xfrout_client_(xfrout_client)
 {
@@ -300,7 +302,7 @@ AuthSrv::getConfigSession() const {
 AuthSrv::ConstMemoryDataSrcPtr
 AuthSrv::getMemoryDataSrc(const RRClass& rrclass) const {
     // XXX: for simplicity, we only support the IN class right now.
-    if (rrclass != RRClass::IN()) {
+    if (rrclass != impl_->memory_datasrc_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
@@ -313,7 +315,7 @@ AuthSrv::setMemoryDataSrc(const isc::dns::RRClass& rrclass,
                           MemoryDataSrcPtr memory_datasrc)
 {
     // XXX: see above
-    if (rrclass != RRClass::IN()) {
+    if (rrclass != impl_->memory_datasrc_class_) {
         isc_throw(InvalidParameter,
                   "Memory data source is not supported for RR class "
                   << rrclass);
@@ -437,14 +439,13 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
     try {
         // If a memory data source is configured call the separate
         // Query::process()
-        if (memory_datasrc_) {
-            ConstQuestionPtr question = *message->beginQuestion();
+        const ConstQuestionPtr question = *message->beginQuestion();
+        if (memory_datasrc_ && memory_datasrc_class_ == question->getClass()) {
             const RRType& qtype = question->getType();
             const Name& qname = question->getName();
-            isc::auth::Query query(*memory_datasrc_, qname, qtype, *message);
-            query.process();
+            auth::Query(*memory_datasrc_, qname, qtype, *message).process();
         } else {
-            isc::datasrc::Query query(*message, cache_, dnssec_ok);
+            datasrc::Query query(*message, cache_, dnssec_ok);
             data_sources_.doQuery(query);
         }
     } catch (const Exception& ex) {
@@ -456,7 +457,6 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
         return (true);
     }
 
-
     MessageRenderer renderer(*buffer);
     const bool udp_buffer =
         (io_message.getSocket().getProtocol() == IPPROTO_UDP);
diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc
index d14c10e2d5..b2567e1a3f 100644
--- a/src/bin/auth/query.cc
+++ b/src/bin/auth/query.cc
@@ -32,9 +32,14 @@ Query::process() const {
     const MemoryDataSrc::FindResult result =
         memory_datasrc_.findZone(qname_);
 
+    // If we have no matching authoritative zone for the query name, return
+    // REFUSED.  In short, this is to be compatible with BIND 9, but the
+    // background discussion is not that simple.  See the relevant topic
+    // at the BIND 10 developers's ML:
+    // https://lists.isc.org/mailman/htdig/bind10-dev/2010-December/001633.html
     if (result.code != result::SUCCESS &&
         result.code != result::PARTIALMATCH) {
-        response_.setRcode(Rcode::SERVFAIL());
+        response_.setRcode(Rcode::REFUSED());
         return;
     }
 
@@ -58,7 +63,7 @@ Query::process() const {
                 // TODO : add SOA to authority section
                 break;
             case Zone::NXRRSET:
-                response_.setRcode(Rcode::NXRRSET());
+                response_.setRcode(Rcode::NOERROR());
                 // TODO : add SOA to authority section
                 break;
             case Zone::CNAME:
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index 275785a9ba..91d8132883 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -394,12 +394,30 @@ TEST_F(AuthSrvTest, updateWithMemoryDataSrc) {
     ASSERT_NE(AuthSrv::MemoryDataSrcPtr(), server.getMemoryDataSrc(rrclass));
     EXPECT_EQ(0, server.getMemoryDataSrc(rrclass)->getZoneCount());
 
-    // The memory data source is empty, should return SERVFAIL rcode.
+    // The memory data source is empty, should return REFUSED rcode.
     createDataFromFile("examplequery_fromWire.wire");
-    server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv);
+    server.processMessage(*io_message, parse_message, response_obuffer,
+                          &dnsserv);
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::REFUSED(),
+                opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
+}
+
+TEST_F(AuthSrvTest, chQueryWithMemoryDataSrc) {
+    // Configure memory data source for class IN
+    updateConfig(&server, "{\"datasources\": "
+                 "[{\"class\": \"IN\", \"type\": \"memory\"}]}", true);
+
+    // This shouldn't affect the result of class CH query
+    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);
     EXPECT_TRUE(dnsserv.hasAnswer());
-    headerCheck(*parse_message, default_qid, Rcode::SERVFAIL(), opcode.getCode(),
-                QR_FLAG, 1, 0, 0, 0);
+    headerCheck(*parse_message, default_qid, Rcode::NOERROR(),
+                opcode.getCode(), QR_FLAG | AA_FLAG, 1, 1, 1, 0);
 }
 
 TEST_F(AuthSrvTest, cacheSlots) {
diff --git a/src/bin/auth/tests/query_unittest.cc b/src/bin/auth/tests/query_unittest.cc
index 6ef6361201..21708f6746 100644
--- a/src/bin/auth/tests/query_unittest.cc
+++ b/src/bin/auth/tests/query_unittest.cc
@@ -100,9 +100,9 @@ protected:
 
 TEST_F(QueryTest, noZone) {
     // There's no zone in the memory datasource.  So the response should have
-    // SERVFAIL.
+    // REFUSED.
     query.process();
-    EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
+    EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
 TEST_F(QueryTest, matchZone) {
@@ -124,16 +124,16 @@ TEST_F(QueryTest, matchZone) {
     const Name nxrrset_name(Name("nxrrset.example.com"));
     Query nxrrset_query(memory_datasrc, nxrrset_name, qtype, response);
     nxrrset_query.process();
-    EXPECT_EQ(Rcode::NXRRSET(), response.getRcode());
+    EXPECT_EQ(Rcode::NOERROR(), response.getRcode());
 }
 
 TEST_F(QueryTest, noMatchZone) {
     // there's a zone in the memory datasource but it doesn't match the qname.
-    // should result in SERVFAIL.
+    // should result in REFUSED.
     memory_datasrc.addZone(ZonePtr(new MockZone()));
     const Name nomatch_name(Name("example.org"));
     Query nomatch_query(memory_datasrc, nomatch_name, qtype, response);
     nomatch_query.process();
-    EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
+    EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 }