Merge #455 (NXRRSET and NXDOMAIN processing)

git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@4078 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/auth/query.cc

@@ -26,6 +26,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 {
     bool keep_doing = true;
     response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
@@ -59,12 +77,14 @@ Query::process() const {
                 // 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:

src/bin/auth/query.h

@@ -14,6 +14,8 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <exceptions/exceptions.h>
+
 namespace isc {
 namespace dns {
 class Message;
@@ -23,6 +25,7 @@ class RRType;
 
 namespace datasrc {
 class MemoryDataSrc;
+class Zone;
 }
 
 namespace auth {
@@ -100,15 +103,46 @@ public:
     /// 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;
 };
 
 }

src/bin/auth/tests/query_unittest.cc

@@ -28,10 +28,14 @@ using namespace isc::dns;
 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 @@ namespace {
 // 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 @@ public:
 
 private:
     Name origin_;
+    bool has_SOA_;
 };
 
 const Name&
@@ -64,20 +71,24 @@ MockZone::getClass() const {
 }
 
 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, noZone) {
 }
 
 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 @@ TEST_F(QueryTest, matchZone) {
     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 @@ TEST_F(QueryTest, noMatchZone) {
     nomatch_query.process();
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
+
 }