[master] Merge branch 'trac2905'

src/bin/auth/query.cc

@@ -386,6 +386,12 @@ Query::process(datasrc::ClientList& client_list,
         response_->setHeaderFlag(Message::HEADERFLAG_AA, false);
         response_->setRcode(Rcode::REFUSED());
         return;
+    } else if (!result.finder_) {
+        // We found a matching zone in a data source but its data are not
+        // available.
+        response_->setHeaderFlag(Message::HEADERFLAG_AA, false);
+        response_->setRcode(Rcode::SERVFAIL());
+        return;
     }
     ZoneFinder& zfinder = *result.finder_;
 

src/bin/auth/tests/auth_srv_unittest.cc

@@ -1210,6 +1210,38 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
                 opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
 }
 
+TEST_F(AuthSrvTest, emptyZone) {
+    // Similar to the previous setup, but the configuration has an error
+    // (zone file doesn't exist) and the query should result in SERVFAIL.
+    // Here we check the rcode other header parameters, and statistics.
+
+    const ConstElementPtr config(Element::fromJSON("{"
+        "\"IN\": [{"
+        "   \"type\": \"MasterFiles\","
+        "   \"params\": {\"example.com\": \"nosuchfile.zone\"},"
+        "   \"cache-enable\": true"
+        "}]}"));
+    installDataSrcClientLists(server, configureDataSource(config));
+    createDataFromFile("examplequery_fromWire.wire");
+    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);
+
+    checkAllRcodeCountersZeroExcept(Rcode::SERVFAIL(), 1);
+    ConstElementPtr stats = server.getStatistics()->get("zones")->
+        get("_SERVER_");
+    std::map<std::string, int> expect;
+    expect["request.v4"] = 1;
+    expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
+    expect["responses"] = 1;
+    expect["qrynoauthans"] = 1;
+    expect["rcode.servfail"] = 1;
+    checkStatisticsCounters(stats, expect);
+}
+
 TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) {
     // In this example, we do simple check that query is handled from the
     // query handler class, and confirm it returns no error and a non empty

src/bin/auth/tests/query_unittest.cc

@@ -133,6 +133,12 @@ const char* const unsigned_delegation_nsec3_txt =
     "q81r598950igr1eqvc60aedlq66425b5.example.com. 3600 IN NSEC3 1 1 12 "
     "aabbccdd 0p9mhaveqvm6t7vbl5lop2u3t2rp3tom NS RRSIG\n";
 
+// Name of an "empty" zone: used to simulate the case of
+// configured-but-available zone (due to load errors, etc).
+// Each tested data source client is expected to have this zone (SQLite3
+// currently doesn't have this concept so it's skipped)
+const char* const EMPTY_ZONE_NAME = "empty.example.org";
+
 // A helper function that generates a textual representation of RRSIG RDATA
 // for the given covered type.  The resulting RRSIG may not necessarily make
 // sense in terms of the DNSSEC protocol, but for our testing purposes it's
@@ -799,11 +805,14 @@ createDataSrcClientList(DataSrcType type, DataSourceClient& client) {
         return (boost::shared_ptr<ClientList>(new SingletonList(client)));
     case INMEMORY:
         list.reset(new ConfigurableClientList(RRClass::IN()));
+        // Configure one normal zone and one "empty" zone.
         list->configure(isc::data::Element::fromJSON(
                             "[{\"type\": \"MasterFiles\","
                             "  \"cache-enable\": true, "
                             "  \"params\": {\"example.com\": \"" +
-                            string(TEST_OWN_DATA_BUILDDIR "/example.zone") +
+                            string(TEST_OWN_DATA_BUILDDIR "/example.zone\",") +
+                            + "\"" + EMPTY_ZONE_NAME + "\": \"" +
+                            string(TEST_OWN_DATA_BUILDDIR "/nosuchfile.zone") +
                             "\"}}]"), true);
         return (list);
     case SQLITE3:
@@ -834,39 +843,38 @@ createDataSrcClientList(DataSrcType type, DataSourceClient& client) {
 class MockClient : public DataSourceClient {
 public:
     virtual FindResult findZone(const isc::dns::Name& origin) const {
-        const Name r_origin(origin.reverse());
-        std::map<Name, ZoneFinderPtr>::const_iterator it =
-            zone_finders_.lower_bound(r_origin);
-
-        if (it != zone_finders_.end()) {
-            const NameComparisonResult result =
-                origin.compare((it->first).reverse());
-            if (result.getRelation() == NameComparisonResult::EQUAL) {
-                return (FindResult(result::SUCCESS, it->second));
-            } else if (result.getRelation() == NameComparisonResult::SUBDOMAIN) {
-                return (FindResult(result::PARTIALMATCH, it->second));
-            }
-        }
+        // Identify the next (strictly) larger name than the given 'origin' in
+        // the map.  Its predecessor (if any) is the longest matching name
+        // if it's either an exact match or a super domain; otherwise there's
+        // no match in the map.  See also datasrc/tests/mock_client.cc.
 
-        // If it is at the beginning of the map, then the name was not
-        // found (we have already handled the element the iterator
-        // points to).
-        if (it == zone_finders_.begin()) {
+        // Eliminate the case of empty map to simply the rest of the code
+        if (zone_finders_.empty()) {
             return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
         }
 
-        // Check if the previous element is a partial match.
-        --it;
-        const NameComparisonResult result =
-            origin.compare((it->first).reverse());
-        if (result.getRelation() == NameComparisonResult::SUBDOMAIN) {
-            return (FindResult(result::PARTIALMATCH, it->second));
+        std::map<Name, ZoneFinderPtr>::const_iterator it =
+            zone_finders_.upper_bound(origin);
+        if (it == zone_finders_.begin()) { // no predecessor
+            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
         }
 
-        return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        --it;                   // get the predecessor
+        const result::ResultFlags flags =
+            it->second ? result::FLAGS_DEFAULT : result::ZONE_EMPTY;
+        const NameComparisonResult compar(it->first.compare(origin));
+        switch (compar.getRelation()) {
+        case NameComparisonResult::EQUAL:
+            return (FindResult(result::SUCCESS, it->second, flags));
+        case NameComparisonResult::SUPERDOMAIN:
+            return (FindResult(result::PARTIALMATCH, it->second, flags));
+        default:
+            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
     }
 
-    virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool, bool) const {
+    virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool, bool) const
+    {
         isc_throw(isc::NotImplemented,
                   "Updater isn't supported in the MockClient");
     }
@@ -878,18 +886,21 @@ public:
     }
 
     result::Result addZone(ZoneFinderPtr finder) {
-        // Use the reverse of the name as the key, so we can quickly
-        // find partial matches in the map.
-        zone_finders_[finder->getOrigin().reverse()] = finder;
+        zone_finders_[finder->getOrigin()] = finder;
+        return (result::SUCCESS);
+    }
+
+    // "configure" a zone with no data.  This will cause the ZONE_EMPTY flag
+    // on in finZone().
+    result::Result addEmptyZone(const Name& zone_name) {
+        zone_finders_[zone_name] = ZoneFinderPtr();
         return (result::SUCCESS);
     }
 
 private:
     // Note that because we no longer have the old RBTree, and the new
     // in-memory DomainTree is not useful as it returns const nodes, we
-    // use a std::map instead. In this map, the key is a name stored in
-    // reverse order of labels to aid in finding partial matches
-    // quickly.
+    // use a std::map instead.
     std::map<Name, ZoneFinderPtr> zone_finders_;
 };
 
@@ -916,9 +927,10 @@ protected:
 
         response.setRcode(Rcode::NOERROR());
         response.setOpcode(Opcode::QUERY());
-        // create and add a matching zone.
+        // create and add a matching zone.  One is a "broken, empty" zone.
         mock_finder = new MockZoneFinder();
         mock_client.addZone(ZoneFinderPtr(mock_finder));
+        mock_client.addEmptyZone(Name(EMPTY_ZONE_NAME));
     }
 
     virtual void SetUp() {
@@ -949,6 +961,12 @@ protected:
         setNSEC3HashCreator(NULL);
     }
 
+    bool isEmptyZoneSupported() const {
+        // Not all data sources support the concept of empty zones.
+        // Specifically for this test, SQLite3-based data source doesn't.
+        return (GetParam() != SQLITE3);
+    }
+
     void enableNSEC3(const vector<string>& rrsets_to_add) {
         boost::shared_ptr<ConfigurableClientList> new_list;
         switch (GetParam()) {
@@ -1144,11 +1162,29 @@ TEST_P(QueryTest, noZone) {
     // REFUSED.
     MockClient empty_mock_client;
     SingletonList empty_list(empty_mock_client);
-    EXPECT_NO_THROW(query.process(empty_list, qname, qtype,
-                                  response));
+    EXPECT_NO_THROW(query.process(empty_list, qname, qtype, response));
     EXPECT_EQ(Rcode::REFUSED(), response.getRcode());
 }
 
+TEST_P(QueryTest, emptyZone) {
+    // Query for an "empty (broken)" zone.  If the concept is supported by
+    // the underlying data source, the result should be SERVFAIL; otherwise
+    // it would be handled as a nonexistent zone, resulting in REFUSED.
+    const Rcode expected_rcode =
+        isEmptyZoneSupported() ? Rcode::SERVFAIL() : Rcode::REFUSED();
+
+    query.process(*list_, Name(EMPTY_ZONE_NAME), qtype, response);
+    responseCheck(response, expected_rcode, 0, 0, 0, 0, NULL, NULL, NULL);
+
+    // Same for the partial match case
+    response.clear(isc::dns::Message::RENDER);
+    response.setRcode(Rcode::NOERROR());
+    response.setOpcode(Opcode::QUERY());
+    query.process(*list_, Name(string("www.") + EMPTY_ZONE_NAME), qtype,
+                  response);
+    responseCheck(response, expected_rcode, 0, 0, 0, 0, NULL, NULL, NULL);
+}
+
 TEST_P(QueryTest, exactMatch) {
     EXPECT_NO_THROW(query.process(*list_, qname, qtype, response));
     // find match rrset
@@ -1400,7 +1436,6 @@ TEST_F(QueryTestForMockOnly, badSecureDelegation) {
                                   qtype, response));
 }
 
-
 TEST_P(QueryTest, nxdomain) {
     EXPECT_NO_THROW(query.process(*list_,
                                   Name("nxdomain.example.com"), qtype,

src/lib/datasrc/client.h

@@ -132,8 +132,8 @@ public:
     /// \brief A helper structure to represent the search result of
     /// \c find().
     ///
-    /// This is a straightforward pair of the result code and a share pointer
-    /// to the found zone to represent the result of \c find().
+    /// This is a straightforward tuple of the result code/flags and a shared
+    /// pointer to the found zone to represent the result of \c find().
     /// We use this in order to avoid overloading the return value for both
     /// the result code ("success" or "not found") and the found object,
     /// i.e., avoid using \c NULL to mean "not found", etc.
@@ -146,10 +146,13 @@ public:
     /// variables.
     struct FindResult {
         FindResult(result::Result param_code,
-                   const ZoneFinderPtr param_zone_finder) :
-            code(param_code), zone_finder(param_zone_finder)
+                   const ZoneFinderPtr param_zone_finder,
+                   result::ResultFlags param_flags = result::FLAGS_DEFAULT) :
+            code(param_code), flags(param_flags),
+            zone_finder(param_zone_finder)
         {}
         const result::Result code;
+        const result::ResultFlags flags;
         const ZoneFinderPtr zone_finder;
     };
 
@@ -184,8 +187,12 @@ 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 flags: usually FLAGS_DEFAULT, but if the zone data are not
+    ///   available (possibly because an error was detected at load time)
+    ///   the ZONE_EMPTY flag is set.
     /// - \c zone_finder: Pointer to a \c ZoneFinder object for the found zone
-    /// if one is found; otherwise \c NULL.
+    /// if one is found and is not empty (flags doesn't have ZONE_EMPTY);
+    /// otherwise \c NULL.
     ///
     /// A specific derived version of this method may throw an exception.
     /// This interface does not specify which exceptions can happen (at least
@@ -215,6 +222,9 @@ public:
     /// \throw Others Possibly implementation specific exceptions (it is
     /// not fixed if a concrete implementation of this method can throw
     /// anything else.)
+    /// \throw EmptyZone the zone is supposed to exist in the data source,
+    /// but its content is not available.  This generally means there's an
+    /// error in the content.
     ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.

src/lib/datasrc/client_list.cc

@@ -90,15 +90,15 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
         for (; i < config->size(); ++i) {
             // Extract the parameters
             const ConstElementPtr dconf(config->get(i));
-            const ConstElementPtr typeElem(dconf->get("type"));
-            if (typeElem == ConstElementPtr()) {
+            const ConstElementPtr type_elem(dconf->get("type"));
+            if (type_elem == ConstElementPtr()) {
                 isc_throw(ConfigurationError, "Missing the type option in "
                           "data source no " << i);
             }
-            const string type(typeElem->stringValue());
-            ConstElementPtr paramConf(dconf->get("params"));
-            if (paramConf == ConstElementPtr()) {
-                paramConf.reset(new NullElement());
+            const string type(type_elem->stringValue());
+            ConstElementPtr param_conf(dconf->get("params"));
+            if (param_conf == ConstElementPtr()) {
+                param_conf.reset(new NullElement());
             }
             // Get the name (either explicit, or guess)
             const ConstElementPtr name_elem(dconf->get("name"));
@@ -114,7 +114,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             // no-op.  In the latter case, it's of no use unless cache is
             // allowed; we simply skip building it in that case.
             const DataSourcePair dsrc_pair = getDataSourceClient(type,
-                                                                 paramConf);
+                                                                 param_conf);
             if (!allow_cache && !dsrc_pair.first) {
                 LOG_WARN(logger, DATASRC_LIST_NOT_CACHED).
                     arg(datasrc_name).arg(rrclass_);
@@ -159,18 +159,22 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                         cache_conf->getLoadAction(rrclass_, zname);
                     // in this loop this should be always true
                     assert(load_action);
-                    memory::ZoneWriter writer(zt_segment,
-                        load_action, zname, rrclass_);
-                    writer.load();
+                    // For the initial load, we'll let the writer handle
+                    // loading error and install an empty zone in the table.
+                    memory::ZoneWriter writer(zt_segment, load_action, zname,
+                                              rrclass_, true);
+
+                    std::string error_msg;
+                    writer.load(&error_msg);
+                    if (!error_msg.empty()) {
+                        LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR).arg(zname).
+                            arg(rrclass_).arg(datasrc_name).arg(error_msg);
+                    }
                     writer.install();
                     writer.cleanup();
                 } catch (const NoSuchZone&) {
                     LOG_ERROR(logger, DATASRC_CACHE_ZONE_NOTFOUND).
                         arg(zname).arg(rrclass_).arg(datasrc_name);
-                } catch (const ZoneLoaderException& e) {
-                    LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR).
-                        arg(zname).arg(rrclass_).arg(datasrc_name).
-                        arg(e.what());
                 }
             }
         }
@@ -357,7 +361,7 @@ ConfigurableClientList::getCachedZoneWriter(const Name& name,
                                ZoneWriterPtr(
                                    new memory::ZoneWriter(
                                        *info.ztable_segment_,
-                                       load_action, name, rrclass_))));
+                                       load_action, name, rrclass_, false))));
     }
 
     // We can't find the specified zone.  If a specific data source was

src/lib/datasrc/client_list.h

@@ -231,20 +231,29 @@ public:
     /// of the searched name is needed. Therefore, the call would look like:
     ///
     /// \code FindResult result(list->find(queried_name));
-    ///   FindResult result(list->find(queried_name));
-    ///   if (result.datasrc_) {
-    ///       createTheAnswer(result.finder_);
+    ///   if (result.dsrc_client_) {
+    ///       if (result.finder_) {
+    ///           createTheAnswer(result.finder_);
+    ///       } else { // broken zone, return SERVFAIL
+    ///           createErrorMessage(Rcode.SERVFAIL());
+    ///       }
     ///   } else {
     ///       createNotAuthAnswer();
     /// } \endcode
     ///
+    /// Note that it is possible that \c finder_ is NULL while \c datasrc_
+    /// is not.  This happens if the zone is configured to be served from
+    /// the data source but it is broken in some sense and doesn't hold any
+    /// zone data, e.g., when the zone file has an error or the secondary
+    /// zone hasn't been transferred yet.  The caller needs to expect the case
+    /// and handle it accordingly.
+    ///
     /// The other scenario is manipulating zone data (XfrOut, XfrIn, DDNS,
     /// ...). In this case, the finder itself is not so important. However,
     /// we need an exact match (if we want to manipulate zone data, we must
     /// know exactly, which zone we are about to manipulate). Then the call
     ///
     /// \code FindResult result(list->find(zone_name, true, false));
-    ///   FindResult result(list->find(zone_name, true, false));
     ///   if (result.datasrc_) {
     ///       ZoneUpdaterPtr updater(result.datasrc_->getUpdater(zone_name);
     ///       ...

src/lib/datasrc/datasrc_messages.mes

@@ -374,8 +374,9 @@ database backend (sqlite3, for example) and use it from there.
 During data source configuration, an error was found in the zone data
 when it was being loaded in to memory on the shown data source.  This
 particular zone was not loaded, but data source configuration
-continues, possibly loading other zones into memory. The specific
-error is shown in the message, and should be addressed.
+continues, possibly loading other zones into memory.  Subsequent lookups
+will treat this zone as broken.  The specific error is shown in the
+message, and should be addressed.
 
 % DATASRC_MASTER_LOAD_ERROR %1:%2: Zone '%3/%4' contains error: %5
 There's an error in the given master file. The zone won't be loaded for

src/lib/datasrc/exceptions.h

@@ -56,6 +56,17 @@ public:
         DataSourceError(file, line, what) {}
 };
 
+/// \brief An error indicating a zone is recognized but its content is not
+/// available.
+///
+/// This generally indicates a condition that there's an error in the zone
+/// content and it's not successfully loaded.
+class EmptyZone : public DataSourceError {
+public:
+    EmptyZone(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
 /// Base class for a number of exceptions that are thrown while working
 /// with zones.
 struct ZoneException : public Exception {

src/lib/datasrc/memory/memory_client.cc

@@ -69,11 +69,11 @@ InMemoryClient::findZone(const isc::dns::Name& zone_name) const {
     const ZoneTable::FindResult result(zone_table->findZone(zone_name));
 
     ZoneFinderPtr finder;
-    if (result.code != result::NOTFOUND) {
+    if (result.code != result::NOTFOUND && result.zone_data) {
         finder.reset(new InMemoryZoneFinder(*result.zone_data, getClass()));
     }
 
-    return (DataSourceClient::FindResult(result.code, finder));
+    return (DataSourceClient::FindResult(result.code, finder, result.flags));
 }
 
 const ZoneData*
@@ -242,7 +242,12 @@ InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
     const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
     const ZoneTable::FindResult result(zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
-        isc_throw(NoSuchZone, "No such zone: " + name.toText());
+        isc_throw(NoSuchZone, "no such zone for in-memory iterator: "
+                  << name.toText());
+    }
+    if (!result.zone_data) {
+        isc_throw(EmptyZone, "empty zone for in-memory iterator: "
+                  << name.toText());
     }
 
     return (ZoneIteratorPtr(new MemoryIterator(

src/lib/datasrc/memory/memory_messages.mes

@@ -92,6 +92,12 @@ tried).
 Debug information.  A specific type RRset is requested at a zone origin
 of an in-memory zone and it is found.
 
+% DATASRC_MEMORY_MEM_ADD_EMPTY_ZONE adding an empty zone '%1/%2'
+Debug information. An "empty" zone is being added into the in-memory
+data source.  This is conceptual data indicating the state where the
+zone exists but its content isn't available.  That would be the case,
+for example, a broken zone specified in the configuration.
+
 % DATASRC_MEMORY_MEM_ADD_RRSET adding RRset '%1/%2' into zone '%3'
 Debug information. An RRset is being added to the in-memory data source.
 

src/lib/datasrc/memory/zone_data.cc

@@ -38,6 +38,10 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
+// Definition of a class static constant.  It's public and its address
+// could be needed by applications, so we need an explicit definition.
+const ZoneNode::Flags ZoneData::DNSSEC_SIGNED;
+
 namespace {
 void
 rdataSetDeleter(RRClass rrclass, util::MemorySegment* mem_sgmt,
@@ -179,6 +183,13 @@ ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) {
     return (zone_data);
 }
 
+ZoneData*
+ZoneData::create(util::MemorySegment& mem_sgmt) {
+    ZoneData* zone_data = create(mem_sgmt, Name::ROOT_NAME());
+    zone_data->origin_node_->setFlag(EMPTY_ZONE);
+    return (zone_data);
+}
+
 void
 ZoneData::destroy(util::MemorySegment& mem_sgmt, ZoneData* zone_data,
                   RRClass zone_class)

src/lib/datasrc/memory/zone_data.h

@@ -378,18 +378,28 @@ private:
     /// It never throws an exception.
     ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node);
 
-    // Zone node flags.
+    // Zone node flags.  When adding a new flag, it's generally advisable to
+    // keep existing values so the binary image of the data is as much
+    // backward compatible as possible.  And it can be helpful in practice
+    // for file-mapped data.
 private:
     // Set in the origin node (which always exists at the same address)
     // to indicate whether the zone is signed or not.  Internal use,
     // so defined as private.
     static const ZoneNode::Flags DNSSEC_SIGNED = ZoneNode::FLAG_USER1;
+
 public:
     /// \brief Node flag indicating it is at a "wildcard level"
     ///
     /// This means one of the node's immediate children is a wildcard.
     static const ZoneNode::Flags WILDCARD_NODE = ZoneNode::FLAG_USER2;
 
+private:
+    // Also set in the origin node, indicating this is a special "empty zone",
+    // that could be created only by the corresponding create() method to be
+    // used for some kind of sentinel data.
+    static const ZoneNode::Flags EMPTY_ZONE = ZoneNode::FLAG_USER3;
+
 public:
     /// \brief Allocate and construct \c ZoneData.
     ///
@@ -410,6 +420,23 @@ public:
     static ZoneData* create(util::MemorySegment& mem_sgmt,
                             const dns::Name& zone_origin);
 
+    /// \brief Allocate and construct a special "empty" \c ZoneData.
+    ///
+    /// A ZoneData object created this way holds all internal integrity
+    /// that those created by the other \c create() method have, but is not
+    /// publicly associated with any actual zone data.  It's intended to be
+    /// used as a kind of sentinel data to representing the concept such as
+    /// "broken zone".
+    ///
+    /// Methods calls on empty \c ZoneData object except \c destroy() and
+    /// \c isEmpty() are meaningless, while they shouldn't cause disruption.
+    /// It's caller's responsibility to use empty zone data objects in the
+    /// intended way.
+    ///
+    /// \param mem_sgmt A \c MemorySegment from which memory for the new
+    /// \c ZoneData is allocated.
+    static ZoneData* create(util::MemorySegment& mem_sgmt);
+
     /// \brief Destruct and deallocate \c ZoneData.
     ///
     /// It releases all resource allocated in the internal storage NSEC3 for
@@ -482,6 +509,13 @@ public:
     /// \throw none
     bool isNSEC3Signed() const { return (nsec3_data_); }
 
+    /// \brief Return whether or not the zone data is "empty".
+    ///
+    /// See the description of \c create() for the concept of empty zone data.
+    ///
+    /// \throw None
+    bool isEmpty() const { return (origin_node_->getFlag(EMPTY_ZONE)); }
+
     /// \brief Return NSEC3Data of the zone.
     ///
     /// This method returns non-NULL valid pointer to \c NSEC3Data object

src/lib/datasrc/memory/zone_table.cc

@@ -18,6 +18,8 @@
 #include <datasrc/memory/segment_object_holder.h>
 #include <datasrc/memory/logger.h>
 
+#include <exceptions/exceptions.h>
+
 #include <util/memory_segment.h>
 
 #include <dns/name.h>
@@ -40,7 +42,10 @@ void
 deleteZoneData(util::MemorySegment* mem_sgmt, ZoneData* zone_data,
                RRClass rrclass)
 {
-    if (zone_data != NULL) {
+    // We shouldn't delete empty zone data here; the only empty zone
+    // that can be passed here is the placeholder for broken zones maintained
+    // in the zone table.  It will stay there until the table is destroyed.
+    if (zone_data && !zone_data->isEmpty()) {
         ZoneData::destroy(*mem_sgmt, zone_data, rrclass);
     }
 }
@@ -49,21 +54,30 @@ typedef boost::function<void(ZoneData*)> ZoneDataDeleterType;
 
 ZoneTable*
 ZoneTable::create(util::MemorySegment& mem_sgmt, const RRClass& zone_class) {
-    SegmentObjectHolder<ZoneTableTree, ZoneDataDeleterType> holder(
+    // Create a placeholder "null" zone data
+    SegmentObjectHolder<ZoneData, RRClass> zdholder(mem_sgmt, zone_class);
+    zdholder.set(ZoneData::create(mem_sgmt));
+
+    // create and setup the tree for the table.
+    SegmentObjectHolder<ZoneTableTree, ZoneDataDeleterType> tree_holder(
         mem_sgmt, boost::bind(deleteZoneData, &mem_sgmt, _1, zone_class));
-    holder.set(ZoneTableTree::create(mem_sgmt));
+    tree_holder.set(ZoneTableTree::create(mem_sgmt));
     void* p = mem_sgmt.allocate(sizeof(ZoneTable));
-    ZoneTable* zone_table = new(p) ZoneTable(zone_class, holder.get());
-    holder.release();
+
+    // Build zone table with the created objects.  Its constructor doesn't
+    // throw, so we can release them from the holder at this point.
+    ZoneTable* zone_table = new(p) ZoneTable(zone_class, tree_holder.release(),
+                                             zdholder.release());
     return (zone_table);
 }
 
 void
-ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable)
-{
+ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable) {
     ZoneTableTree::destroy(mem_sgmt, ztable->zones_.get(),
                            boost::bind(deleteZoneData, &mem_sgmt, _1,
                                        ztable->rrclass_));
+    ZoneData::destroy(mem_sgmt, ztable->null_zone_data_.get(),
+                      ztable->rrclass_);
     mem_sgmt.deallocate(ztable, sizeof(ZoneTable));
 }
 
@@ -74,11 +88,33 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_ZONE).
         arg(zone_name).arg(rrclass_);
 
-    if (content == NULL) {
-        isc_throw(isc::BadValue, "Zone content must not be NULL");
+    if (!content || content->isEmpty()) {
+        isc_throw(InvalidParameter,
+                  (content ? "empty data" : "NULL") <<
+                  " is passed to Zone::addZone");
     }
     SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, zone_class);
     holder.set(content);
+
+    const AddResult result =
+        addZoneInternal(mem_sgmt, zone_name, holder.get());
+    holder.release();
+    return (result);
+}
+
+ZoneTable::AddResult
+ZoneTable::addEmptyZone(util::MemorySegment& mem_sgmt, const Name& zone_name) {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_EMPTY_ZONE).
+        arg(zone_name).arg(rrclass_);
+
+    return (addZoneInternal(mem_sgmt, zone_name, null_zone_data_.get()));
+}
+
+ZoneTable::AddResult
+ZoneTable::addZoneInternal(util::MemorySegment& mem_sgmt,
+                           const dns::Name& zone_name,
+                           ZoneData* content)
+{
     // Get the node where we put the zone
     ZoneTableNode* node(NULL);
     switch (zones_->insert(mem_sgmt, zone_name, &node)) {
@@ -93,10 +129,9 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     // Can Not Happen
     assert(node != NULL);
 
-    // We can release now, setData never throws
-    ZoneData* old = node->setData(holder.release());
+    ZoneData* old = node->setData(content);
     if (old != NULL) {
-        return (AddResult(result::EXIST, old));
+        return (AddResult(result::EXIST, old->isEmpty() ? NULL : old));
     } else {
         ++zone_count_;
         return (AddResult(result::SUCCESS, NULL));
@@ -126,10 +161,17 @@ ZoneTable::findZone(const Name& name) const {
         return (FindResult(result::NOTFOUND, NULL));
     }
 
-    // Can Not Happen (remember, NOTFOUND is handled)
+    // Can Not Happen (remember, NOTFOUND is handled).  node should also have
+    // data because the tree is constructed in the way empty nodes would
+    // be "invisible" for find().
     assert(node != NULL);
 
-    return (FindResult(my_result, node->getData()));
+    const ZoneData* zone_data = node->getData();
+    assert(zone_data);
+    const result::ResultFlags flags =
+        zone_data->isEmpty() ? result::ZONE_EMPTY : result::FLAGS_DEFAULT;
+    return (FindResult(my_result, zone_data->isEmpty() ? NULL : zone_data,
+                       flags));
 }
 
 } // end of namespace memory

src/lib/datasrc/memory/zone_table.h

@@ -84,12 +84,16 @@ public:
      };
 
     /// \brief Result data of findZone() method.
+    ///
+    /// See \c findZone() about the semantics of the members.
     struct FindResult {
         FindResult(result::Result param_code,
-                   const ZoneData* param_zone_data) :
-            code(param_code), zone_data(param_zone_data)
+                   const ZoneData* param_zone_data,
+                   result::ResultFlags param_flags = result::FLAGS_DEFAULT) :
+            code(param_code), flags(param_flags), zone_data(param_zone_data)
         {}
         const result::Result code;
+        const result::ResultFlags flags;
         const ZoneData* const zone_data;
     };
 
@@ -99,13 +103,13 @@ private:
     /// An object of this class is always expected to be created by the
     /// allocator (\c create()), so the constructor is hidden as private.
     ///
-    /// This constructor internally involves resource allocation, and if
-    /// it fails, a corresponding standard exception will be thrown.
-    /// It never throws an exception otherwise.
-    ZoneTable(const dns::RRClass& rrclass, ZoneTableTree* zones) :
+    /// This constructor never throws.
+    ZoneTable(const dns::RRClass& rrclass, ZoneTableTree* zones,
+              ZoneData* null_zone_data) :
         rrclass_(rrclass),
         zone_count_(0),
-        zones_(zones)
+        zones_(zones),
+        null_zone_data_(null_zone_data)
     {}
 
 public:
@@ -154,7 +158,7 @@ public:
     /// \throw None.
     size_t getZoneCount() const { return (zone_count_); }
 
-    /// Add a new zone to the \c ZoneTable.
+    /// \brief Add a new zone to the \c ZoneTable.
     ///
     /// This method adds a given zone data to the internal table.
     ///
@@ -165,6 +169,7 @@ public:
     /// accordingly.  On successful return, this method ensures there's no
     /// address relocation.
     ///
+    /// \throw InvalidParameter content is NULL or empty.
     /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
     ///     relocating data.
     /// \throw std::bad_alloc Internal resource allocation fails.
@@ -176,20 +181,43 @@ public:
     /// \param zone_class The RR class of the zone.  It must be the RR class
     ///     that is supposed to be associated to the zone table.
     /// \param content This one should hold the zone content (the ZoneData).
-    ///     The ownership is passed onto the zone table. Must not be null.
-    ///     Must correspond to the name and class and must be allocated from
-    ///     mem_sgmt.
+    ///     The ownership is passed onto the zone table. Must not be null or
+    ///     empty. Must correspond to the name and class and must be allocated
+    ///     from mem_sgmt.
     /// \return \c result::SUCCESS If the zone is successfully
     ///     added to the zone table.
     /// \return \c result::EXIST The zone table already contained
     ///     zone of the same origin. The old data is replaced and returned
-    ///     inside the result.
+    ///     inside the result unless it's empty; if the zone was previously
+    ///     added by \c addEmptyZone(), the data returned is NULL.
     AddResult addZone(util::MemorySegment& mem_sgmt,
                       dns::RRClass zone_class,
                       const dns::Name& zone_name,
                       ZoneData* content);
 
-    /// Find a zone that best matches the given name in the \c ZoneTable.
+    /// \brief Add an empty zone to the \c ZoneTable.
+    ///
+    /// This method is similar to \c addZone(), but adds a conceptual "empty"
+    /// zone of the given zone name to the table.  The added empty zone
+    /// affects subsequent calls to \c addZone() (and \c addEmptyZone() itself)
+    /// and \c findZone() as described for these methods.
+    ///
+    /// The intended meaning of an empty zone in the table is that the zone
+    /// is somehow broken, such as configured to be loaded but loading failed.
+    /// But this class is not aware of such interpretation; it's up to the
+    /// user of the class how to use the concept of empty zones.
+    ///
+    /// It returns an \c AddResult object as described for \c addZone().
+    ///
+    /// The same notes on exception safety as that for \c addZone() applies.
+    ///
+    /// \param mem_sgmt Same as addZone().
+    /// \param zone_name Same as addZone().
+    AddResult addEmptyZone(util::MemorySegment& mem_sgmt,
+                           const dns::Name& zone_name);
+
+    /// \brief Find a zone that best matches the given name in the
+    /// \c ZoneTable.
     ///
     /// It searches the internal storage for a zone that gives the
     /// longest match against \c name, and returns the result in the
@@ -200,8 +228,11 @@ 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_data: corresponding zone data of the found zone; NULL if
-    ///   no matching zone is found.
+    /// - \c flags If the zone is empty (added by \c addEmptyZone()),
+    ///   result::ZONE_EMPTY is set.
+    /// - \c zone_data: corresponding zone data of the found zone if found and
+    ///   non empty; NULL if no matching zone is found or the found zone is
+    ///   empty.
     ///
     /// \throw none
     ///
@@ -213,6 +244,18 @@ private:
     const dns::RRClass rrclass_;
     size_t zone_count_;
     boost::interprocess::offset_ptr<ZoneTableTree> zones_;
+
+    // this is a shared placeholder for broken zones
+    boost::interprocess::offset_ptr<ZoneData> null_zone_data_;
+
+    // Common routine for addZone and addEmptyZone.  This method can throw
+    // util::MemorySegmentGrown, in which case addresses from mem_sgmt
+    // can be relocated.  The caller is responsible for destroying content
+    // on exception, if it needs to be destroyed.  On successful return it
+    // ensures there's been no address relocation.
+    AddResult addZoneInternal(util::MemorySegment& mem_sgmt,
+                              const dns::Name& zone_name,
+                              ZoneData* content);
 };
 }
 }

src/lib/datasrc/memory/zone_writer.cc

@@ -16,6 +16,8 @@
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table_segment.h>
 
+#include <datasrc/exceptions.h>
+
 #include <memory>
 
 using std::auto_ptr;
@@ -27,13 +29,15 @@ namespace memory {
 ZoneWriter::ZoneWriter(ZoneTableSegment& segment,
                        const LoadAction& load_action,
                        const dns::Name& origin,
-                       const dns::RRClass& rrclass) :
+                       const dns::RRClass& rrclass,
+                       bool throw_on_load_error) :
     segment_(segment),
     load_action_(load_action),
     origin_(origin),
     rrclass_(rrclass),
     zone_data_(NULL),
-    state_(ZW_UNUSED)
+    state_(ZW_UNUSED),
+    catch_load_error_(throw_on_load_error)
 {
     if (!segment.isWritable()) {
         isc_throw(isc::InvalidOperation,
@@ -48,17 +52,28 @@ ZoneWriter::~ZoneWriter() {
 }
 
 void
-ZoneWriter::load() {
+ZoneWriter::load(std::string* error_msg) {
     if (state_ != ZW_UNUSED) {
         isc_throw(isc::InvalidOperation, "Trying to load twice");
     }
 
-    zone_data_ = load_action_(segment_.getMemorySegment());
-    segment_.resetHeader();
+    try {
+        zone_data_ = load_action_(segment_.getMemorySegment());
+        segment_.resetHeader();
 
-    if (!zone_data_) {
-        // Bug inside load_action_.
-        isc_throw(isc::InvalidOperation, "No data returned from load action");
+        if (!zone_data_) {
+            // Bug inside load_action_.
+            isc_throw(isc::InvalidOperation,
+                      "No data returned from load action");
+        }
+    } catch (const ZoneLoaderException& ex) {
+        if (!catch_load_error_) {
+            throw;
+        }
+        if (error_msg) {
+            *error_msg = ex.what();
+        }
+        segment_.resetHeader();
     }
 
     state_ = ZW_LOADED;
@@ -70,6 +85,10 @@ ZoneWriter::install() {
         isc_throw(isc::InvalidOperation, "No data to install");
     }
 
+    // Check the internal integrity assumption: we should have non NULL
+    // zone data or we've allowed load error to create an empty zone.
+    assert(zone_data_ || catch_load_error_);
+
     // FIXME: This retry is currently untested, as there seems to be no
     // reasonable way to create a zone table segment with non-local memory
     // segment. Once there is, we should provide the test.
@@ -79,10 +98,10 @@ ZoneWriter::install() {
             if (table == NULL) {
                 isc_throw(isc::Unexpected, "No zone table present");
             }
-            const ZoneTable::AddResult result(table->addZone(
-                                                  segment_.getMemorySegment(),
-                                                  rrclass_, origin_,
-                                                  zone_data_));
+            const ZoneTable::AddResult result(
+                zone_data_ ? table->addZone(segment_.getMemorySegment(),
+                                            rrclass_, origin_, zone_data_) :
+                table->addEmptyZone(segment_.getMemorySegment(), origin_));
             state_ = ZW_INSTALLED;
             zone_data_ = result.zone_data;
         } catch (const isc::util::MemorySegmentGrown&) {}

src/lib/datasrc/memory/zone_writer.h

@@ -42,15 +42,27 @@ class ZoneWriter {
 public:
     /// \brief Constructor
     ///
+    /// If \c catch_load_error is set to true, the \c load() method will
+    /// internally catch load related errors reported as a DataSourceError
+    /// exception, and subsequent \c install() method will add a special
+    /// empty zone to the zone table segment.  If it's set to false, \c load()
+    /// will simply propagate the exception.  This parameter would normally
+    /// be set to false as it's not desirable to install a broken zone;
+    /// however, it would be better to be set to true at the initial loading
+    /// so the zone table recognizes the existence of the zone (and being
+    /// aware that it's broken).
+    ///
     /// \throw isc::InvalidOperation if \c segment is read-only.
     ///
     /// \param segment The zone table segment to store the zone into.
     /// \param load_action The callback used to load data.
     /// \param name The name of the zone.
     /// \param rrclass The class of the zone.
+    /// \param catch_load_error true if loading errors are to be caught
+    /// internally; false otherwise.
     ZoneWriter(ZoneTableSegment& segment,
                const LoadAction& load_action, const dns::Name& name,
-               const dns::RRClass& rrclass);
+               const dns::RRClass& rrclass, bool catch_load_error);
 
     /// \brief Destructor.
     ~ZoneWriter();
@@ -64,6 +76,12 @@ public:
     /// This is the first method you should call on the object. Never call it
     /// multiple times.
     ///
+    /// If the optional parameter \c error_msg is given and non NULL, and
+    /// if the writer object was constructed with \c catch_load_error being
+    /// true, then error_msg will be filled with text indicating the reason
+    /// for the error in case a load error happens.  In other cases any
+    /// passed non NULL error_msg will be intact.
+    ///
     /// \note As this contains reading of files or other data sources, or with
     ///     some other source of the data to load, it may throw quite anything.
     ///     If it throws, do not call any other methods on the object and
@@ -71,14 +89,23 @@ public:
     /// \note After successful load(), you have to call cleanup() some time
     ///     later.
     /// \throw isc::InvalidOperation if called second time.
-    void load();
+    /// \throw DataSourceError load related error (not thrown if constructed
+    /// with catch_load_error being false).
+    ///
+    /// \param error_msg If non NULL, used as a placeholder to store load error
+    /// messages.
+    void load(std::string* error_msg = NULL);
 
     /// \brief Put the changes to effect.
     ///
     /// This replaces the old version of zone with the one previously prepared
     /// by load(). It takes ownership of the old zone data, if any.
     ///
-    /// You may call it only after successful load() and at most once.
+    /// You may call it only after successful load() and at most once.  It
+    /// includes the case the writer is constructed with catch_load_error
+    /// being true and load() encountered and caught a DataSourceError
+    /// exception.  In this case this method installs a special empty zone
+    /// to the table.
     ///
     /// The operation is expected to be fast and is meant to be used inside
     /// a critical section.
@@ -112,10 +139,15 @@ private:
         ZW_CLEANED
     };
     State state_;
+    const bool catch_load_error_;
 };
 
 }
 }
 }
 
-#endif
+#endif  // MEM_ZONE_WRITER_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/result.h

@@ -18,13 +18,10 @@
 namespace isc {
 namespace datasrc {
 namespace result {
-/// Result codes of various public methods of in memory data source
+/// \brief Result codes of various public methods of DataSourceClient.
 ///
 /// The detailed semantics may differ in different methods.
 /// See the description of specific methods for more details.
-///
-/// Note: this is intended to be used from other data sources eventually,
-/// but for now it's specific to in memory data source and its backend.
 enum Result {
     SUCCESS,  ///< The operation is successful.
     EXIST,    ///< The search key is already stored.
@@ -32,8 +29,26 @@ enum Result {
     PARTIALMATCH ///< Only a partial match is found.
 };
 
+/// \brief Flags for supplemental information along with the \c Result
+///
+/// Initially there's only one flag defined, but several flags will be added
+/// later.  One likely case is to indicate a flag that is listed in in-memory
+/// but its content is served in the underlying data source.  This will help
+/// when only a subset of zones are cached in-memory so the lookup code can
+/// efficiently detect whether it doesn't exist or is not just cached.
+/// When more flags are added, the logical-or operation should be allowed
+/// (by defining \c operator|) on these flags.
+enum ResultFlags {
+    FLAGS_DEFAULT = 0,          // no flags
+    ZONE_EMPTY = 1 ///< The zone found is empty, normally meaning it's broken
+};
+
 }
 }
 }
 
-#endif
+#endif  // DATASRC_RESULT_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/tests/client_list_unittest.cc

@@ -171,7 +171,7 @@ public:
                 new memory::ZoneWriter(
                     *dsrc_info.ztable_segment_,
                     cache_conf->getLoadAction(rrclass_, zone),
-                    zone, rrclass_));
+                    zone, rrclass_, false));
             writer->load();
             writer->install();
             writer->cleanup(); // not absolutely necessary, but just in case
@@ -208,6 +208,18 @@ public:
             EXPECT_EQ(dsrc.get(), result.dsrc_client_);
         }
     }
+
+    // check the result with empty (broken) zones.  Right now this can only
+    // happen for in-memory caches.
+    void emptyResult(const ClientList::FindResult& result, bool exact,
+                     const char* trace_txt)
+    {
+        SCOPED_TRACE(trace_txt);
+        ASSERT_FALSE(result.finder_);
+        EXPECT_EQ(exact, result.exact_match_);
+        EXPECT_TRUE(dynamic_cast<InMemoryClient*>(result.dsrc_client_));
+    }
+
     // Configure the list with multiple data sources, according to
     // some configuration. It uses the index as parameter, to be able to
     // loop through the configurations.
@@ -762,7 +774,7 @@ TEST_F(ListTest, masterFiles) {
               list_->getDataSources()[0].data_src_client_);
 
     // And it can search
-    positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "com",
+    positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "root",
                    true);
 
     // If cache is not enabled, nothing is loaded
@@ -820,7 +832,8 @@ TEST_F(ListTest, names) {
 
 TEST_F(ListTest, BadMasterFile) {
     // Configuration should succeed, and the good zones in the list
-    // below should be loaded. No bad zones should be loaded.
+    // below should be loaded.  Bad zones won't be "loaded" in its usual sense,
+    // but are still recognized with conceptual "empty" data.
     const ConstElementPtr elem(Element::fromJSON("["
         "{"
         "   \"type\": \"MasterFiles\","
@@ -856,13 +869,19 @@ TEST_F(ListTest, BadMasterFile) {
 
     positiveResult(list_->find(Name("example.com."), true), ds_[0],
                    Name("example.com."), true, "example.com", true);
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.org."), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("foo.bar"), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.net."), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.edu."), true));
-    EXPECT_TRUE(negative_result_ == list_->find(Name("example.info."), true));
+    // Bad cases: should result in "empty zone", whether the match is exact
+    // or partial.
+    emptyResult(list_->find(Name("foo.bar"), true), true, "foo.bar");
+    emptyResult(list_->find(Name("example.net."), true), true, "example.net");
+    emptyResult(list_->find(Name("example.edu."), true), true, "example.edu");
+    emptyResult(list_->find(Name("example.info."), true), true,
+                "example.info");
+    emptyResult(list_->find(Name("www.example.edu."), false), false,
+                "example.edu, partial");
     positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "root",
                    true);
+    // This one simply doesn't exist.
+    EXPECT_TRUE(list_->find(Name("example.org."), true) == negative_result_);
 }
 
 ConfigurableClientList::CacheStatus

src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -694,6 +694,15 @@ TEST_F(MemoryClientTest, getIterator) {
     EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
 }
 
+TEST_F(MemoryClientTest, getIteratorForEmptyZone) {
+    // trying to load a broken zone (zone file not existent).  It's internally
+    // stored an empty zone.
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/no-such-file.zone", true);
+    // Then getIterator will result in an exception.
+    EXPECT_THROW(client_->getIterator(Name("example.org")), EmptyZone);
+}
+
 TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
     loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
                       TEST_DATA_DIR "/example.org-multiple.zone");
@@ -791,6 +800,35 @@ TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
     // Teardown checks for memory segment leaks
 }
 
+TEST_F(MemoryClientTest, findEmptyZone) {
+    // trying to load a broken zone (zone file not existent).  It's internally
+    // stored an empty zone.
+    loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
+                      TEST_DATA_DIR "/no-such-file.zone", true);
+
+    using namespace isc::datasrc::result;
+
+    // findZone() returns the match, with NULL zone finder and the result
+    // flag indicating it's empty.
+    const DataSourceClient::FindResult result =
+        client_->findZone(Name("example.org"));
+    EXPECT_EQ(SUCCESS, result.code);
+    EXPECT_EQ(ZONE_EMPTY, result.flags);
+    EXPECT_FALSE(result.zone_finder);
+
+    // Same for the case of subdomain match
+    const DataSourceClient::FindResult result_sub =
+        client_->findZone(Name("www.example.org"));
+    EXPECT_EQ(PARTIALMATCH, result_sub.code);
+    EXPECT_EQ(ZONE_EMPTY, result_sub.flags);
+    EXPECT_FALSE(result_sub.zone_finder);
+
+    // findZoneData() will simply NULL (this is for testing only anyway,
+    // so any result would be okay as long as it doesn't cause disruption).
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL),
+              client_->findZoneData(Name("example.org")));
+}
+
 TEST_F(MemoryClientTest, findZoneData) {
     loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_,
                       TEST_DATA_DIR "/example.org-rrsigs.zone");

src/lib/datasrc/tests/memory/zone_data_unittest.cc

@@ -277,4 +277,14 @@ TEST_F(ZoneDataTest, minTTL) {
     zone_data_->setMinTTL(1200);
     EXPECT_EQ(RRTTL(1200), createRRTTL(zone_data_->getMinTTLData()));
 }
+
+TEST_F(ZoneDataTest, emptyData) {
+    // normally create zone data are never "empty"
+    EXPECT_FALSE(zone_data_->isEmpty());
+
+    // zone data instance created by the special create() is made "empty".
+    ZoneData* empty_data = ZoneData::create(mem_sgmt_);
+    EXPECT_TRUE(empty_data->isEmpty());
+    ZoneData::destroy(mem_sgmt_, empty_data, RRClass::IN());
+}
 }

src/lib/datasrc/tests/memory/zone_loader_util.cc

@@ -33,7 +33,8 @@ namespace test {
 
 void
 loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
-                  const dns::RRClass& zclass, const std::string& zone_file)
+                  const dns::RRClass& zclass, const std::string& zone_file,
+                  bool load_error_ok)
 {
     const isc::datasrc::internal::CacheConfig cache_conf(
         "MasterFiles", NULL, *data::Element::fromJSON(
@@ -41,7 +42,7 @@ loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
             " \"params\": {\"" + zname.toText() + "\": \"" + zone_file +
             "\"}}"), true);
     memory::ZoneWriter writer(zt_sgmt, cache_conf.getLoadAction(zclass, zname),
-                              zname, zclass);
+                              zname, zclass, load_error_ok);
     writer.load();
     writer.install();
     writer.cleanup();
@@ -72,7 +73,7 @@ loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
                   const dns::RRClass& zclass, ZoneIterator& iterator)
 {
     memory::ZoneWriter writer(zt_sgmt, IteratorLoader(zclass, zname, iterator),
-                              zname, zclass);
+                              zname, zclass, false);
     writer.load();
     writer.install();
     writer.cleanup();

src/lib/datasrc/tests/memory/zone_loader_util.h

@@ -33,9 +33,14 @@ namespace test {
 /// This function does nothing special, simply provides a shortcut for commonly
 /// used pattern that would be used in tests with a ZoneTableSegment loading
 /// a zone from file into it.
+///
+/// If the optional load_error_ok parameter is set to true, it will create
+/// an internal empty zone in the table when it encounters a loading error.
+/// Otherwise ZoneLoaderException will be thrown in such cases.
 void
 loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname,
-                  const dns::RRClass& zclass, const std::string& zone_file);
+                  const dns::RRClass& zclass, const std::string& zone_file,
+                  bool load_error_ok = false);
 
 /// \brief A shortcut utility to load a specified zone into ZoneTableSegment
 /// from a zone iterator.

src/lib/datasrc/tests/memory/zone_table_unittest.cc

@@ -73,11 +73,19 @@ TEST_F(ZoneTableTest, addZone) {
     // By default there's no zone contained.
     EXPECT_EQ(0, zone_table->getZoneCount());
 
-    // It doesn't accept empty (NULL) zones
+    // It doesn't accept NULL as zone data
     EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1, NULL),
-                 isc::BadValue);
+                 isc::InvalidParameter);
     EXPECT_EQ(0, zone_table->getZoneCount()); // count is still 0
 
+    // or an empty zone data
+    SegmentObjectHolder<ZoneData, RRClass> holder_empty(
+        mem_sgmt_, zclass_);
+    holder_empty.set(ZoneData::create(mem_sgmt_));
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1,
+                                     holder_empty.get()),
+                 isc::InvalidParameter);
+
     SegmentObjectHolder<ZoneData, RRClass> holder1(
         mem_sgmt_, zclass_);
     holder1.set(ZoneData::create(mem_sgmt_, zname1));
@@ -92,9 +100,8 @@ TEST_F(ZoneTableTest, addZone) {
     EXPECT_EQ(static_cast<const ZoneData*>(NULL), holder1.get());
     EXPECT_EQ(1, zone_table->getZoneCount()); // count is now incremented
 
-    // Duplicate add doesn't replace the existing data.
-    SegmentObjectHolder<ZoneData, RRClass> holder2(
-        mem_sgmt_, zclass_);
+    // Duplicate add replaces the existing data wit the newly added one.
+    SegmentObjectHolder<ZoneData, RRClass> holder2(mem_sgmt_, zclass_);
     holder2.set(ZoneData::create(mem_sgmt_, zname1));
     const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
                                                            zname1,
@@ -145,6 +152,46 @@ TEST_F(ZoneTableTest, addZone) {
                  std::bad_alloc);
 }
 
+TEST_F(ZoneTableTest, addEmptyZone) {
+    // By default there's no zone contained.
+    EXPECT_EQ(0, zone_table->getZoneCount());
+
+    // Adding an empty zone.  It should succeed.
+    const ZoneTable::AddResult result1 =
+        zone_table->addEmptyZone(mem_sgmt_, zname1);
+    EXPECT_EQ(result::SUCCESS, result1.code);
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), result1.zone_data);
+    EXPECT_EQ(1, zone_table->getZoneCount());
+
+    // The empty zone can be "found", with the ZONE_EMPTY flag on, and the
+    // returned ZoneData being NULL.
+    const ZoneTable::FindResult fresult1 = zone_table->findZone(zname1);
+    EXPECT_EQ(result::SUCCESS, fresult1.code);
+    EXPECT_EQ(result::ZONE_EMPTY, fresult1.flags);
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), fresult1.zone_data);
+
+    // Replacing an empty zone with non-empty one.  Should be no problem, but
+    // the empty zone data are not returned in the result structure; it's
+    // internal to the ZoneTable implementation.
+    SegmentObjectHolder<ZoneData, RRClass> holder2(mem_sgmt_, zclass_);
+    holder2.set(ZoneData::create(mem_sgmt_, zname1));
+    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder2.release()));
+    EXPECT_EQ(result::EXIST, result2.code);
+    EXPECT_EQ(static_cast<const ZoneData*>(NULL), result2.zone_data);
+    EXPECT_EQ(1, zone_table->getZoneCount());
+
+    // Replacing a non-empty zone with an empty one is also okay.  It's not
+    // different from replacing with another non-empty one.
+    const ZoneTable::AddResult result3 =
+        zone_table->addEmptyZone(mem_sgmt_, zname1);
+    EXPECT_EQ(result::EXIST, result3.code);
+    EXPECT_NE(static_cast<const ZoneData*>(NULL), result3.zone_data);
+    ZoneData::destroy(mem_sgmt_, result3.zone_data, zclass_);
+    EXPECT_EQ(1, zone_table->getZoneCount());
+}
+
 TEST_F(ZoneTableTest, findZone) {
     SegmentObjectHolder<ZoneData, RRClass> holder1(
         mem_sgmt_, zclass_);

src/lib/datasrc/tests/memory/zone_writer_unittest.cc

@@ -15,6 +15,7 @@
 #include <datasrc/memory/zone_writer.h>
 #include <datasrc/memory/zone_table_segment_local.h>
 #include <datasrc/memory/zone_data.h>
+#include <datasrc/exceptions.h>
 
 #include <dns/rrclass.h>
 #include <dns/name.h>
@@ -27,10 +28,13 @@
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
 
+#include <string>
+
 using boost::scoped_ptr;
 using boost::bind;
 using isc::dns::RRClass;
 using isc::dns::Name;
+using isc::datasrc::ZoneLoaderException;
 using namespace isc::datasrc::memory;
 using namespace isc::datasrc::memory::test;
 
@@ -60,9 +64,10 @@ protected:
         writer_(new
             ZoneWriter(*segment_,
                        bind(&ZoneWriterTest::loadAction, this, _1),
-                       Name("example.org"), RRClass::IN())),
+                       Name("example.org"), RRClass::IN(), false)),
         load_called_(false),
         load_throw_(false),
+        load_loader_throw_(false),
         load_null_(false),
         load_data_(false)
     {}
@@ -75,6 +80,7 @@ protected:
     scoped_ptr<ZoneWriter> writer_;
     bool load_called_;
     bool load_throw_;
+    bool load_loader_throw_;
     bool load_null_;
     bool load_data_;
 public:
@@ -87,6 +93,9 @@ public:
         if (load_throw_) {
             throw TestException();
         }
+        if (load_loader_throw_) {
+            isc_throw(ZoneLoaderException, "faked loader exception");
+        }
 
         if (load_null_) {
             // Be nasty to the caller and return NULL, which is forbidden
@@ -130,7 +139,7 @@ TEST_F(ZoneWriterTest, constructForReadOnlySegment) {
     ReadOnlySegment ztable_segment(RRClass::IN(), mem_sgmt);
     EXPECT_THROW(ZoneWriter(ztable_segment,
                             bind(&ZoneWriterTest::loadAction, this, _1),
-                            Name("example.org"), RRClass::IN()),
+                            Name("example.org"), RRClass::IN(), false),
                  isc::InvalidOperation);
 }
 
@@ -243,6 +252,57 @@ TEST_F(ZoneWriterTest, loadThrows) {
     EXPECT_NO_THROW(writer_->cleanup());
 }
 
+// Emulate the situation where load() throws loader error.
+TEST_F(ZoneWriterTest, loadLoaderException) {
+    std::string error_msg;
+
+    // By default, the exception is propagated.
+    load_loader_throw_ = true;
+    EXPECT_THROW(writer_->load(), ZoneLoaderException);
+    // In this case, passed error_msg won't be updated.
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), false));
+    EXPECT_THROW(writer_->load(&error_msg), ZoneLoaderException);
+    EXPECT_EQ("", error_msg);
+
+    // If we specify allowing load error, load() will succeed and install()
+    // adds an empty zone.  Note that we implicitly pass NULL to load()
+    // as it's the default parameter, so the following also confirms it doesn't
+    // cause disruption.
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), true));
+    writer_->load();
+    writer_->install();
+    writer_->cleanup();
+
+    // Check an empty zone has been really installed.
+    using namespace isc::datasrc::result;
+    const ZoneTable* ztable = segment_->getHeader().getTable();
+    ASSERT_TRUE(ztable);
+    const ZoneTable::FindResult result = ztable->findZone(Name("example.org"));
+    EXPECT_EQ(SUCCESS, result.code);
+    EXPECT_EQ(ZONE_EMPTY, result.flags);
+
+    // Allowing an error, and passing a template for the error message.
+    // It will be filled with the reason for the error.
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), true));
+    writer_->load(&error_msg);
+    EXPECT_NE("", error_msg);
+
+    // In case of no error, the placeholder will be intact.
+    load_loader_throw_ = false;
+    error_msg.clear();
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), true));
+    writer_->load(&error_msg);
+    EXPECT_EQ("", error_msg);
+}
+
 // Check the strong exception guarantee - if it throws, nothing happened
 // to the content.
 TEST_F(ZoneWriterTest, retry) {

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -79,7 +79,7 @@ createInMemoryClient(RRClass zclass, const Name& zname) {
         ZoneTableSegment::create(zclass, cache_conf.getSegmentType()));
     memory::ZoneWriter writer(*ztable_segment,
                               cache_conf.getLoadAction(zclass, zname),
-                              zname, zclass);
+                              zname, zclass, false);
     writer.load();
     writer.install();
     writer.cleanup();

src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -320,8 +320,9 @@ protected:
         if (filename) {
             boost::scoped_ptr<memory::ZoneWriter> writer(
                 new memory::ZoneWriter(*ztable_segment_,
-                                       cache_conf.getLoadAction(rrclass_, zone),
-                                       zone, rrclass_));
+                                       cache_conf.getLoadAction(rrclass_,
+                                                                zone),
+                                       zone, rrclass_, false));
             writer->load();
             writer->install();
             writer->cleanup();

tests/lettuce/features/auth_badzone.feature

@@ -2,7 +2,7 @@ Feature: Authoritative DNS server with a bad zone
     This feature set is for testing the execution of the b10-auth
     component when one zone is broken, whereas others are fine. In this
     case, b10-auth should not reject the data source, but reject the bad
-    zone only and serve the good zones anyway.
+    zone only (with SERVFAIL) and serve the good zones anyway.
 
     Scenario: Bad zone
         Given I have bind10 running with configuration auth/auth_badzone.config
@@ -44,6 +44,6 @@ Feature: Authoritative DNS server with a bad zone
         ns2.example.org.        3600    IN      A       192.0.2.4
         """
 
-        A query for www.example.com should have rcode REFUSED
-        A query for www.example.net should have rcode REFUSED
-        A query for www.example.info should have rcode REFUSED
+        A query for www.example.com should have rcode SERVFAIL
+        A query for www.example.net should have rcode SERVFAIL
+        A query for www.example.info should have rcode SERVFAIL