[1570] added a real test for "above delegation" that fails and fixed it

in the implementation.

src/bin/auth/query.cc

@@ -257,13 +257,24 @@ Query::addAuthAdditional(ZoneFinder& finder) {
     }
 }
 
+namespace {
+// A simple wrapper for DataSourceClient::findZone().  Normally we can simply
+// check the closest zone to the qname, but for type DS query we need to
+// look into the parent zone.
+DataSourceClient::FindResult
+findZone(const DataSourceClient& client, const Name& qname, RRType qtype) {
+    if (qtype != RRType::DS()) {
+        return (client.findZone(qname));
+    }
+    return (client.findZone(qname.split(1)));
+}
+}
+
 void
 Query::process() {
-    const bool qtype_is_any = (qtype_ == RRType::ANY());
-
-    response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
-    const DataSourceClient::FindResult result =
-        datasrc_client_.findZone(qname_);
+    // Found a zone which is the nearest ancestor to QNAME
+    const DataSourceClient::FindResult result = findZone(datasrc_client_,
+                                                         qname_, qtype_);
 
     // 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
@@ -272,16 +283,19 @@ Query::process() {
     // https://lists.isc.org/mailman/htdig/bind10-dev/2010-December/001633.html
     if (result.code != result::SUCCESS &&
         result.code != result::PARTIALMATCH) {
+        response_.setHeaderFlag(Message::HEADERFLAG_AA, false);
         response_.setRcode(Rcode::REFUSED());
         return;
     }
     ZoneFinder& zfinder = *result.zone_finder;
 
-    // Found a zone which is the nearest ancestor to QNAME, set the AA bit
+    // We have authority for a zone that contain the query name (possibly
+    // indirectly via delegation).  Look into the zone.
     response_.setHeaderFlag(Message::HEADERFLAG_AA);
     response_.setRcode(Rcode::NOERROR());
     std::vector<ConstRRsetPtr> target;
     boost::function0<ZoneFinder::FindResult> find;
+    const bool qtype_is_any = (qtype_ == RRType::ANY());
     if (qtype_is_any) {
         find = boost::bind(&ZoneFinder::findAll, &zfinder, qname_,
                            boost::ref(target), dnssec_opt_);

src/bin/auth/tests/query_unittest.cc

@@ -1613,13 +1613,36 @@ TEST_F(QueryTest, findNSEC3) {
                mock_finder->findNSEC3(Name("nxdomain3.example.com"), false));
 }
 
-// TODO: Check the additional/authority sections are correct. The first one
-// probably misses some of the RRSigs anyway, they need to be added.
-
 // This tests that the DS is returned above the delegation point as
 // an authoritative answer, not a delegation. This is as described in
 // RFC 4035, section 3.1.4.1.
+
+// This mock finder is used for some DS-query test(s) to check if the lookup
+// takes place at the parent zone, not at this (broken) zone.
+class BrokenChildZoneFinder : public MockZoneFinder {
+public:
+    BrokenChildZoneFinder(const Name& origin) :
+        MockZoneFinder(), origin_(origin)
+    {}
+    virtual isc::dns::Name getOrigin() const { return (origin_); }
+    virtual FindResult find(const isc::dns::Name&,
+                            const isc::dns::RRType&,
+                            const FindOptions)
+    {
+        isc_throw(isc::Unexpected,
+                  "BrokenChildZoneFinder::find shouldn't be called");
+    }
+private:
+    const Name origin_;
+};
+
 TEST_F(QueryTest, dsAboveDelegation) {
+    // Pretending to have authority for the child zone, too.
+    memory_client.addZone(ZoneFinderPtr(new BrokenChildZoneFinder(
+                                            Name("delegation.example.com"))));
+
+    // The following will succeed only if the search goes to the parent
+    // zone, not the child one we added above.
     EXPECT_NO_THROW(Query(memory_client, Name("delegation.example.com"),
                           RRType::DS(), response, true).process());