[2546] Merge branch 'master' into trac2546

src/lib/datasrc/Makefile.am

@@ -38,6 +38,7 @@ libb10_datasrc_la_SOURCES += client_list.h client_list.cc
 libb10_datasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
 libb10_datasrc_la_SOURCES += master_loader_callbacks.h
 libb10_datasrc_la_SOURCES += master_loader_callbacks.cc
+libb10_datasrc_la_SOURCES += zone_loader.h zone_loader.cc
 nodist_libb10_datasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libb10_datasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 

src/lib/datasrc/master_loader_callbacks.cc

@@ -48,6 +48,19 @@ logWarning(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
         arg(name).arg(rrclass).arg(reason);
 }
 
+void
+addRR(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
+      const isc::dns::RRType& type, const isc::dns::RRTTL& ttl,
+      const isc::dns::rdata::RdataPtr& data, ZoneUpdater* updater)
+{
+    // We get description of one RR. The updater takes RRset, so we
+    // wrap it up and push there. It should collate the RRsets of the
+    // same name and type together, since the addRRset should "merge".
+    isc::dns::BasicRRset rrset(name, rrclass, type, ttl);
+    rrset.addRdata(data);
+    updater->addRRset(rrset);
+}
+
 }
 
 isc::dns::MasterLoaderCallbacks
@@ -61,12 +74,9 @@ createMasterLoaderCallbacks(const isc::dns::Name& name,
                                                         rrclass, _1, _2, _3)));
 }
 
-isc::dns::AddRRsetCallback
+isc::dns::AddRRCallback
 createMasterLoaderAddCallback(ZoneUpdater& updater) {
-    return (boost::bind(&ZoneUpdater::addRRset, &updater,
-                        // The callback provides a shared pointer, we
-                        // need the object. This bind unpacks the object.
-                        boost::bind(&isc::dns::RRsetPtr::operator*, _1)));
+    return (boost::bind(addRR, _1, _2, _3, _4, _5, &updater));
 }
 
 }

src/lib/datasrc/master_loader_callbacks.h

@@ -58,7 +58,7 @@ createMasterLoaderCallbacks(const isc::dns::Name& name,
 /// \param updater The zone updater to use.
 /// \return The callback to be passed to MasterLoader.
 /// \throw std::bad_alloc when allocation fails.
-isc::dns::AddRRsetCallback
+isc::dns::AddRRCallback
 createMasterLoaderAddCallback(ZoneUpdater& updater);
 
 }

src/lib/datasrc/memory/memory_client.cc

@@ -209,14 +209,20 @@ private:
     RdataIteratorPtr rdata_iterator_;
     bool separate_rrs_;
     bool ready_;
+    bool examined_rrsigs_;
+    // In case there's nsec3 namespace in the zone, it is represented the same
+    // way as the usual namespace. So we reuse the iterator implementation for
+    // it.
+    ZoneIteratorPtr nsec3_namespace_;
 public:
     MemoryIterator(const RRClass& rrclass,
-                   const ZoneTree& tree, const Name& origin,
-                   bool separate_rrs) :
+                   const ZoneTree& tree, const NSEC3Data* nsec3_data,
+                   const Name& origin, bool separate_rrs) :
         rrclass_(rrclass),
         tree_(tree),
         separate_rrs_(separate_rrs),
-        ready_(true)
+        ready_(true),
+        examined_rrsigs_(false)
     {
         // Find the first node (origin) and preserve the node chain for future
         // searches
@@ -235,10 +241,25 @@ public:
                 rdata_iterator_ = rrset_->getRdataIterator();
             }
         }
+
+        // If we have the NSEC3 namespace, get an iterator for it so we can
+        // delegate to it later.
+        if (nsec3_data != NULL) {
+            nsec3_namespace_ =
+                ZoneIteratorPtr(new MemoryIterator(rrclass,
+                                                   nsec3_data->getNSEC3Tree(),
+                                                   NULL, origin,
+                                                   separate_rrs));
+        }
     }
 
     virtual ConstRRsetPtr getNextRRset() {
         if (!ready_) {
+            // We are done iterating. But in case there's the nsec3 one,
+            // iterate through that one.
+            if (nsec3_namespace_ != ZoneIteratorPtr()) {
+                return (nsec3_namespace_->getNextRRset());
+            }
             isc_throw(Unexpected, "Iterating past the zone end");
         }
         /*
@@ -259,13 +280,19 @@ public:
                     rrset_.reset(new TreeNodeRRset(rrclass_,
                                                    node_, set_node_, true));
                     rdata_iterator_ = rrset_->getRdataIterator();
+                    examined_rrsigs_ = false;
                 }
             }
         }
         if (node_ == NULL) {
             // That's all, folks
             ready_ = false;
-            return (ConstRRsetPtr());
+            if (nsec3_namespace_ != ZoneIteratorPtr()) {
+                // In case we have the NSEC3 namespace, get one from there.
+                return (nsec3_namespace_->getNextRRset());
+            } else {
+                return (ConstRRsetPtr());
+            }
         }
 
         if (separate_rrs_) {
@@ -273,10 +300,24 @@ public:
             // 'current' rdata
             RRsetPtr result(new RRset(rrset_->getName(),
                                       rrset_->getClass(),
-                                      rrset_->getType(),
+                                      // If we are looking into the signature,
+                                      // we need to adjust the type too.
+                                      examined_rrsigs_ ? RRType::RRSIG() :
+                                          rrset_->getType(),
                                       rrset_->getTTL()));
             result->addRdata(rdata_iterator_->getCurrent());
             rdata_iterator_->next();
+            if (!examined_rrsigs_ && rdata_iterator_->isLast()) {
+                // We got to the last RR of the RRset, but we need to look at
+                // the signatures too, if there are any.
+                examined_rrsigs_ = true;
+                const ConstRRsetPtr rrsig = rrset_->getRRsig();
+                if (rrsig != ConstRRsetPtr()) {
+                    rrset_ = rrsig;
+                    rdata_iterator_ = rrsig->getRdataIterator();
+                } // else - no RRSIG. rdata_iterator_ stays at last, next
+                  // condition applies
+            }
             if (rdata_iterator_->isLast()) {
                 // all used up, next.
                 set_node_ = set_node_->getNext();
@@ -286,6 +327,7 @@ public:
                     rrset_.reset(new TreeNodeRRset(rrclass_,
                                                    node_, set_node_, true));
                     rdata_iterator_ = rrset_->getRdataIterator();
+                    examined_rrsigs_ = false;
                 }
             }
             return (result);
@@ -317,7 +359,8 @@ InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
 
     return (ZoneIteratorPtr(new MemoryIterator(
                                 getClass(),
-                                result.zone_data->getZoneTree(), name,
+                                result.zone_data->getZoneTree(),
+                                result.zone_data->getNSEC3Data(), name,
                                 separate_rrs)));
 }
 

src/lib/datasrc/tests/Makefile.am

@@ -60,6 +60,7 @@ run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc
 run_unittests_SOURCES += master_loader_callbacks_test.cc
+run_unittests_SOURCES += zone_loader_unittest.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/master_loader_callbacks_test.cc

@@ -18,6 +18,9 @@
 #include <dns/rrset.h>
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
+#include <dns/rdata.h>
+
+#include <testutils/dnsmessage_test.h>
 
 #include <exceptions/exceptions.h>
 
@@ -40,8 +43,21 @@ public:
     // the correct ones, according to a predefined set in a list.
     virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
         ASSERT_FALSE(expected_rrsets_.empty());
-        // In our tests, pointer equality is enough.
-        EXPECT_EQ(expected_rrsets_.front().get(), &rrset);
+
+        // As the rrsetCheck requires a shared pointer, we need to create
+        // a copy.
+        isc::dns::RRsetPtr copy(new isc::dns::BasicRRset(rrset.getName(),
+                                                         rrset.getClass(),
+                                                         rrset.getType(),
+                                                         rrset.getTTL()));
+        EXPECT_FALSE(rrset.getRRsig()) << "Unexpected RRSIG on rrset, not "
+            "copying. Following check will likely fail as a result.";
+        for (isc::dns::RdataIteratorPtr it(rrset.getRdataIterator());
+             !it->isLast(); it->next()) {
+            copy->addRdata(it->getCurrent());
+        }
+
+        isc::testutils::rrsetCheck(expected_rrsets_.front(), copy);
         // And remove this RRset, as it has been used.
         expected_rrsets_.pop_front();
     }
@@ -67,14 +83,22 @@ protected:
                                                isc::dns::RRClass::IN(), &ok_))
     {}
     // Generate a new RRset, put it to the updater and return it.
-    isc::dns::RRsetPtr generateRRset() {
+    void generateRRset(isc::dns::AddRRCallback callback) {
         const isc::dns::RRsetPtr
             result(new isc::dns::RRset(isc::dns::Name("example.org"),
                                        isc::dns::RRClass::IN(),
                                        isc::dns::RRType::A(),
                                        isc::dns::RRTTL(3600)));
+        const isc::dns::rdata::RdataPtr
+            data(isc::dns::rdata::createRdata(isc::dns::RRType::A(),
+                                              isc::dns::RRClass::IN(),
+                                              "192.0.2.1"));
+
+        result->addRdata(data);
         updater_.expected_rrsets_.push_back(result);
-        return (result);
+
+        callback(result->getName(), result->getClass(), result->getType(),
+                 result->getTTL(), data);
     }
     // An updater to be passed to the context
     MockUpdater updater_;
@@ -112,11 +136,11 @@ TEST_F(MasterLoaderCallbackTest, callbacks) {
 
 // Try adding some RRsets.
 TEST_F(MasterLoaderCallbackTest, addRRset) {
-    isc::dns::AddRRsetCallback
+    isc::dns::AddRRCallback
         callback(createMasterLoaderAddCallback(updater_));
     // Put some of them in.
-    EXPECT_NO_THROW(callback(generateRRset()));
-    EXPECT_NO_THROW(callback(generateRRset()));
+    EXPECT_NO_THROW(generateRRset(callback));
+    EXPECT_NO_THROW(generateRRset(callback));
     // They all get pushed there right away, so there are none in the queue
     EXPECT_TRUE(updater_.expected_rrsets_.empty());
 }

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

@@ -690,6 +690,25 @@ TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
     EXPECT_EQ(ConstRRsetPtr(), iterator2->getNextRRset());
 }
 
+// Test we get RRSIGs and NSEC3s too for iterating with separate RRs
+TEST_F(MemoryClientTest, getIteratorSeparateSigned) {
+    client_->load(Name("example.org"),
+                       TEST_DATA_DIR "/example.org-nsec3-signed.zone");
+    ZoneIteratorPtr iterator(client_->getIterator(Name("example.org"), true));
+    bool seen_rrsig = false, seen_nsec3 = false;
+    for (ConstRRsetPtr rrset = iterator->getNextRRset();
+         rrset != ConstRRsetPtr(); rrset = iterator->getNextRRset()) {
+        if (rrset->getType() == RRType::RRSIG()) {
+            seen_rrsig = true;
+        } else if (rrset->getType() == RRType::NSEC3()) {
+            seen_nsec3 = true;
+        }
+    }
+
+    EXPECT_TRUE(seen_rrsig);
+    EXPECT_TRUE(seen_nsec3);
+}
+
 TEST_F(MemoryClientTest, getIteratorGetSOAThrowsNotImplemented) {
     client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));

src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -0,0 +1,395 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/zone_loader.h>
+#include <datasrc/data_source.h>
+
+#include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/memory/memory_client.h>
+
+#include <dns/rrclass.h>
+#include <dns/name.h>
+#include <dns/rrset.h>
+#include <util/memory_segment_local.h>
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/shared_ptr.hpp>
+#include <string>
+#include <vector>
+
+using isc::dns::RRClass;
+using isc::dns::Name;
+using isc::dns::RRType;
+using isc::dns::ConstRRsetPtr;
+using std::string;
+using std::vector;
+using boost::shared_ptr;
+using namespace isc::datasrc;
+
+namespace {
+
+class MockClient : public DataSourceClient {
+public:
+    MockClient() :
+        commit_called_(false),
+        missing_zone_(false),
+        rrclass_(RRClass::IN())
+    {}
+    virtual FindResult findZone(const Name&) const {
+        isc_throw(isc::NotImplemented, "Method not used in tests");
+    };
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+        getJournalReader(const Name&, uint32_t, uint32_t) const
+    {
+        isc_throw(isc::NotImplemented, "Method not used in tests");
+    }
+    virtual ZoneUpdaterPtr getUpdater(const Name& name, bool replace,
+                                      bool journaling) const;
+    // We store some information about what was happening here.
+    // It is publicly accessible, since this is private testing class
+    // anyway, so no need to dress it fancy into getters. Some are mutable,
+    // since many client methods are const, but we still want to know they
+    // were called.
+    mutable vector<Name> provided_updaters_;
+    // We store string representations of the RRsets. This is simpler than
+    // copying them and we can't really put them into shared pointers, because
+    // we get them as references.
+    vector<string> rrsets_;
+    bool commit_called_;
+    // If set to true, getUpdater returns NULL
+    bool missing_zone_;
+    // The pretended class of the client. Usualy IN, but can be overriden.
+    RRClass rrclass_;
+};
+
+// The updater isn't really correct according to the API. For example,
+// the whole client can be committed only once in its lifetime. The
+// updaters would influence each other if there were more. But we
+// don't need more updaters in the same test, so it doesn't matter
+// and this way, it is much simpler.
+class Updater : public ZoneUpdater {
+public:
+    Updater(MockClient* client) :
+        client_(client),
+        finder_(client_->rrclass_)
+    {}
+    virtual ZoneFinder& getFinder() {
+        return (finder_);
+    }
+    virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
+        if (client_->commit_called_) {
+            isc_throw(DataSourceError, "Add after commit");
+        }
+        client_->rrsets_.push_back(rrset.toText());
+    }
+    virtual void deleteRRset(const isc::dns::AbstractRRset&) {
+        isc_throw(isc::NotImplemented, "Method not used in tests");
+    }
+    virtual void commit() {
+        client_->commit_called_ = true;
+    }
+private:
+    MockClient* client_;
+    class Finder : public ZoneFinder {
+    public:
+        Finder(const RRClass& rrclass) :
+            class_(rrclass)
+        {}
+        virtual RRClass getClass() const {
+            return (class_);
+        }
+        virtual Name getOrigin() const {
+            isc_throw(isc::NotImplemented, "Method not used in tests");
+        }
+        virtual shared_ptr<Context> find(const Name&, const RRType&,
+                                         const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Method not used in tests");
+        }
+        virtual shared_ptr<Context> findAll(const Name&,
+                                            vector<ConstRRsetPtr>&,
+                                            const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Method not used in tests");
+        }
+        virtual FindNSEC3Result findNSEC3(const Name&, bool) {
+            isc_throw(isc::NotImplemented, "Method not used in tests");
+        }
+    private:
+        const RRClass class_;
+    } finder_;
+};
+
+ZoneUpdaterPtr
+MockClient::getUpdater(const Name& name, bool replace, bool journaling) const {
+    if (missing_zone_) {
+        return (ZoneUpdaterPtr());
+    }
+    EXPECT_TRUE(replace);
+    EXPECT_FALSE(journaling);
+    provided_updaters_.push_back(name);
+    // const_cast is bad. But the const on getUpdater seems wrong in the first
+    // place, since updater will be modifying the data there. And the updater
+    // wants to store data into the client so we can examine it later.
+    return (ZoneUpdaterPtr(new Updater(const_cast<MockClient*>(this))));
+}
+
+class ZoneLoaderTest : public ::testing::Test {
+protected:
+    ZoneLoaderTest() :
+        rrclass_(RRClass::IN()),
+        ztable_segment_(memory::ZoneTableSegment::
+                        create(isc::data::NullElement(), rrclass_)),
+        source_client_(ztable_segment_, rrclass_)
+    {}
+    void prepareSource(const Name& zone, const char* filename) {
+        // TODO:
+        // Currently, load uses an urelated implementation. In the long term,
+        // the method will probably be deprecated. At that time, we should
+        // probably prepare the data in some other way (using sqlite3 or
+        // something). This is simpler for now.
+        source_client_.load(zone, string(TEST_DATA_DIR) + "/" + filename);
+    }
+private:
+    const RRClass rrclass_;
+    // This is because of the in-memory client. We use it to read data
+    // from. It is still easier than setting up sqlite3 client, since
+    // we have this one in the linked library.
+
+    // FIXME: We should be destroying it by ZoneTableSegment::destroy.
+    // But the shared pointer won't let us, will it?
+    shared_ptr<memory::ZoneTableSegment> ztable_segment_;
+protected:
+    memory::InMemoryClient source_client_;
+    // This one is mocked. It will help us see what is happening inside.
+    // Also, mocking it is simpler than setting up an sqlite3 client.
+    MockClient destination_client_;
+};
+
+// Use the loader to load an unsigned zone.
+TEST_F(ZoneLoaderTest, copyUnsigned) {
+    prepareSource(Name::ROOT_NAME(), "root.zone");
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(), source_client_);
+    // It gets the updater directly in the constructor
+    ASSERT_EQ(1, destination_client_.provided_updaters_.size());
+    EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
+    // Now load the whole zone
+    loader.load();
+    EXPECT_TRUE(destination_client_.commit_called_);
+    // We don't check the whole zone. We check the first and last and the
+    // count, which should be enough.
+
+    // The count is 34 because we expect the RRs to be separated.
+    EXPECT_EQ(34, destination_client_.rrsets_.size());
+    // Ensure known order.
+    std::sort(destination_client_.rrsets_.begin(),
+              destination_client_.rrsets_.end());
+    EXPECT_EQ(". 518400 IN NS a.root-servers.net.\n",
+              destination_client_.rrsets_.front());
+    EXPECT_EQ("m.root-servers.net. 3600000 IN AAAA 2001:dc3::35\n",
+              destination_client_.rrsets_.back());
+
+    // It isn't possible to try again now
+    EXPECT_THROW(loader.load(), isc::InvalidOperation);
+    EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);
+    // Even 0, which should load nothing, returns the error
+    EXPECT_THROW(loader.loadIncremental(0), isc::InvalidOperation);
+}
+
+// Try loading incrementally.
+TEST_F(ZoneLoaderTest, copyUnsignedIncremental) {
+    prepareSource(Name::ROOT_NAME(), "root.zone");
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(), source_client_);
+
+    // Try loading few RRs first.
+    loader.loadIncremental(10);
+    // We should get the 10 we asked for
+    EXPECT_EQ(10, destination_client_.rrsets_.size());
+    // Not committed yet, we didn't complete the loading
+    EXPECT_FALSE(destination_client_.commit_called_);
+
+    // This is unusual, but allowed. Check it doesn't do anything
+    loader.loadIncremental(0);
+    EXPECT_EQ(10, destination_client_.rrsets_.size());
+    EXPECT_FALSE(destination_client_.commit_called_);
+
+    // We can finish the rest
+    loader.loadIncremental(30);
+    EXPECT_EQ(34, destination_client_.rrsets_.size());
+    EXPECT_TRUE(destination_client_.commit_called_);
+
+    // No more loading now
+    EXPECT_THROW(loader.load(), isc::InvalidOperation);
+    EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);
+    EXPECT_THROW(loader.loadIncremental(0), isc::InvalidOperation);
+}
+
+// Check we can load RRSIGs and NSEC3 (which could break due to them being
+// in separate namespace)
+TEST_F(ZoneLoaderTest, copySigned) {
+    prepareSource(Name("example.org"), "example.org.nsec3-signed");
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      source_client_);
+    loader.load();
+
+    // All the RRs are there, including the ones in NSEC3 namespace
+    EXPECT_EQ(14, destination_client_.rrsets_.size());
+    EXPECT_TRUE(destination_client_.commit_called_);
+    // Same trick with sorting to know where they are
+    std::sort(destination_client_.rrsets_.begin(),
+              destination_client_.rrsets_.end());
+    // Due to the R at the beginning, this one should be last
+    EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN NSEC3 "
+              "1 0 10 AABBCCDD RKOF8QMFRB5F2V9EJHFBVB2JPVSA0DJD A RRSIG\n",
+              destination_client_.rrsets_[0]);
+    EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN RRSIG "
+              "NSEC3 7 3 1200 20120301040838 20120131040838 19562 example.org."
+              " EdwMeepLf//lV+KpCAN+213Scv1rrZyj4i2OwoCP4XxxS3CWGSuvYuKOyfZc8w"
+              "KRcrD/4YG6nZVXE0s5O8NahjBJmDIyVt4WkfZ6QthxGg8ggLVvcD3dFksPyiKHf"
+              "/WrTOZPSsxvN5m/i1Ey6+YWS01Gf3WDCMWDauC7Nmh3CTM=\n",
+              destination_client_.rrsets_[1]);
+}
+
+// If the destination zone does not exist, it throws
+TEST_F(ZoneLoaderTest, copyMissingDestination) {
+    destination_client_.missing_zone_ = true;
+    prepareSource(Name::ROOT_NAME(), "root.zone");
+    EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
+                            source_client_), DataSourceError);
+}
+
+// If the source zone does not exist, it throws
+TEST_F(ZoneLoaderTest, copyMissingSource) {
+    EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
+                            source_client_), DataSourceError);
+}
+
+// The class of the source and destination are different
+TEST_F(ZoneLoaderTest, classMismatch) {
+    destination_client_.rrclass_ = RRClass::CH();
+    prepareSource(Name::ROOT_NAME(), "root.zone");
+    EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
+                            source_client_), isc::InvalidParameter);
+}
+
+// Load an unsigned zone, all at once
+TEST_F(ZoneLoaderTest, loadUnsigned) {
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
+                      TEST_DATA_DIR "/root.zone");
+    // It gets the updater directly in the constructor
+    ASSERT_EQ(1, destination_client_.provided_updaters_.size());
+    EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
+    // Now load the whole zone
+    loader.load();
+    EXPECT_TRUE(destination_client_.commit_called_);
+    // We don't check the whole zone. We check the first and last and the
+    // count, which should be enough.
+
+    // The count is 34 because we expect the RRs to be separated.
+    EXPECT_EQ(34, destination_client_.rrsets_.size());
+    // Ensure known order.
+    std::sort(destination_client_.rrsets_.begin(),
+              destination_client_.rrsets_.end());
+    EXPECT_EQ(". 518400 IN NS a.root-servers.net.\n",
+              destination_client_.rrsets_.front());
+    EXPECT_EQ("m.root-servers.net. 3600000 IN AAAA 2001:dc3::35\n",
+              destination_client_.rrsets_.back());
+
+    // It isn't possible to try again now
+    EXPECT_THROW(loader.load(), isc::InvalidOperation);
+    EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);
+    // Even 0, which should load nothing, returns the error
+    EXPECT_THROW(loader.loadIncremental(0), isc::InvalidOperation);
+}
+
+// Try loading from master file incrementally.
+TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
+                      TEST_DATA_DIR "/root.zone");
+
+    // Try loading few RRs first.
+    loader.loadIncremental(10);
+    // We should get the 10 we asked for
+    EXPECT_EQ(10, destination_client_.rrsets_.size());
+    // Not committed yet, we didn't complete the loading
+    EXPECT_FALSE(destination_client_.commit_called_);
+
+    EXPECT_EQ(10, destination_client_.rrsets_.size());
+    EXPECT_FALSE(destination_client_.commit_called_);
+
+    // We can finish the rest
+    loader.loadIncremental(30);
+    EXPECT_EQ(34, destination_client_.rrsets_.size());
+    EXPECT_TRUE(destination_client_.commit_called_);
+
+    // No more loading now
+    EXPECT_THROW(loader.load(), isc::InvalidOperation);
+    EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);
+    EXPECT_THROW(loader.loadIncremental(0), isc::InvalidOperation);
+}
+
+// If the destination zone does not exist, it throws
+TEST_F(ZoneLoaderTest, loadMissingDestination) {
+    destination_client_.missing_zone_ = true;
+    EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
+                            TEST_DATA_DIR "/root.zone"), DataSourceError);
+}
+
+// Check we can load RRSIGs and NSEC3 (which could break due to them being
+// in separate namespace)
+TEST_F(ZoneLoaderTest, loadSigned) {
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      TEST_DATA_DIR "/example.org.nsec3-signed");
+    loader.load();
+
+    // All the RRs are there, including the ones in NSEC3 namespace
+    EXPECT_EQ(14, destination_client_.rrsets_.size());
+    EXPECT_TRUE(destination_client_.commit_called_);
+    // Same trick with sorting to know where they are
+    std::sort(destination_client_.rrsets_.begin(),
+              destination_client_.rrsets_.end());
+    // Due to the R at the beginning, this one should be last
+    EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN NSEC3 "
+              "1 0 10 AABBCCDD RKOF8QMFRB5F2V9EJHFBVB2JPVSA0DJD A RRSIG\n",
+              destination_client_.rrsets_[0]);
+    EXPECT_EQ("09GM5T42SMIMT7R8DF6RTG80SFMS1NLU.example.org. 1200 IN RRSIG "
+              "NSEC3 7 3 1200 20120301040838 20120131040838 19562 example.org."
+              " EdwMeepLf//lV+KpCAN+213Scv1rrZyj4i2OwoCP4XxxS3CWGSuvYuKOyfZc8w"
+              "KRcrD/4YG6nZVXE0s5O8NahjBJmDIyVt4WkfZ6QthxGg8ggLVvcD3dFksPyiKHf"
+              "/WrTOZPSsxvN5m/i1Ey6+YWS01Gf3WDCMWDauC7Nmh3CTM=\n",
+              destination_client_.rrsets_[1]);
+}
+
+// Test it throws when there's no such file
+TEST_F(ZoneLoaderTest, loadNoSuchFile) {
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
+                      "This file does not exist");
+    EXPECT_THROW(loader.load(), MasterFileError);
+    EXPECT_FALSE(destination_client_.commit_called_);
+}
+
+// And it also throws when there's a syntax error in the master file
+TEST_F(ZoneLoaderTest, loadSyntaxError) {
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
+                      // This is not a master file for sure
+                      // (misusing a file that happens to be there
+                      // already).
+                      TEST_DATA_DIR "/example.org.sqlite3");
+    EXPECT_THROW(loader.load(), MasterFileError);
+    EXPECT_FALSE(destination_client_.commit_called_);
+}
+
+}

src/lib/datasrc/zone_loader.cc

@@ -0,0 +1,132 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/zone_loader.h>
+#include <datasrc/master_loader_callbacks.h>
+
+#include <datasrc/client.h>
+#include <datasrc/data_source.h>
+#include <datasrc/iterator.h>
+#include <datasrc/zone.h>
+
+#include <dns/rrset.h>
+
+using isc::dns::Name;
+using isc::dns::ConstRRsetPtr;
+using isc::dns::MasterLoader;
+
+namespace isc {
+namespace datasrc {
+
+ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
+                       DataSourceClient& source) :
+    // Separate the RRsets as that is possibly faster (the data source doesn't
+    // have to aggregate them) and also because our limit semantics.
+    iterator_(source.getIterator(zone_name, true)),
+    updater_(destination.getUpdater(zone_name, true, false)),
+    complete_(false)
+{
+    // The getIterator should never return NULL. So we check it.
+    // Or should we throw instead?
+    assert(iterator_ != ZoneIteratorPtr());
+    // In case the zone doesn't exist in the destination, throw
+    if (updater_ == ZoneUpdaterPtr()) {
+        isc_throw(DataSourceError, "Zone " << zone_name << " not found in "
+                  "destination data source, can't fill it with data");
+    }
+    // The dereference of zone_finder is safe, if we can get iterator, we can
+    // get a finder.
+    //
+    // TODO: We probably need a getClass on the data source itself.
+    if (source.findZone(zone_name).zone_finder->getClass() !=
+        updater_->getFinder().getClass()) {
+        isc_throw(isc::InvalidParameter,
+                  "Source and destination class mismatch");
+    }
+}
+
+ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
+                       const char* filename) :
+    updater_(destination.getUpdater(zone_name, true, false)),
+    complete_(false),
+    loaded_ok_(true)
+{
+    if (updater_ == ZoneUpdaterPtr()) {
+        isc_throw(DataSourceError, "Zone " << zone_name << " not found in "
+                  "destination data source, can't fill it with data");
+    } else {
+        loader_.reset(new
+                      MasterLoader(filename, zone_name,
+                                   // TODO: Maybe we should have getClass()
+                                   // on the data source?
+                                   updater_->getFinder().getClass(),
+                                   createMasterLoaderCallbacks(zone_name,
+                                       updater_->getFinder().getClass(),
+                                       &loaded_ok_),
+                                   createMasterLoaderAddCallback(*updater_)));
+    }
+}
+
+namespace {
+
+// Copy up to limit RRsets from source to destination
+bool
+copyRRsets(const ZoneUpdaterPtr& destination, const ZoneIteratorPtr& source,
+           size_t limit)
+{
+    size_t loaded = 0;
+    while (loaded < limit) {
+        const ConstRRsetPtr rrset(source->getNextRRset());
+        if (rrset == ConstRRsetPtr()) {
+            // Done loading, no more RRsets in the input.
+            return (true);
+        } else {
+            destination->addRRset(*rrset);
+        }
+        ++loaded;
+    }
+    return (false); // Not yet, there may be more
+}
+
+} // end unnamed namespace
+
+bool
+ZoneLoader::loadIncremental(size_t limit) {
+    if (complete_) {
+        isc_throw(isc::InvalidOperation,
+                  "Loading has been completed previously");
+    }
+
+    if (iterator_ == ZoneIteratorPtr()) {
+        assert(loader_.get() != NULL);
+        try {
+            complete_ = loader_->loadIncremental(limit);
+        } catch (const isc::dns::MasterLoaderError& e) {
+            isc_throw(MasterFileError, e.getMessage().c_str());
+        }
+        if (complete_ && !loaded_ok_) {
+            isc_throw(MasterFileError, "Error while loading master file");
+        }
+    } else {
+        complete_ = copyRRsets(updater_, iterator_, limit);
+    }
+
+    if (complete_) {
+        updater_->commit();
+    }
+    return (complete_);
+}
+
+} // end namespace datasrc
+} // end namespace isc

src/lib/datasrc/zone_loader.h

@@ -0,0 +1,158 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DATASRC_ZONE_LOADER_H
+#define DATASRC_ZONE_LOADER_H
+
+#include <datasrc/data_source.h>
+
+#include <dns/master_loader.h>
+
+#include <cstdlib> // For size_t
+#include <boost/shared_ptr.hpp>
+#include <boost/scoped_ptr.hpp>
+
+namespace isc {
+namespace dns {
+// Forward declaration
+class Name;
+}
+namespace datasrc {
+
+// Forward declarations
+class DataSourceClient;
+class ZoneIterator;
+typedef boost::shared_ptr<ZoneIterator> ZoneIteratorPtr;
+class ZoneUpdater;
+typedef boost::shared_ptr<ZoneUpdater> ZoneUpdaterPtr;
+
+/// \brief Exception thrown when there's a problem with master file.
+///
+/// This is thrown by the ZoneLoader when there's a fatal problem with
+/// a master file being loaded.
+class MasterFileError : public DataSourceError {
+public:
+    MasterFileError(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what)
+    {}
+};
+
+/// \brief Class to load data into a data source client.
+///
+/// This is a small wrapper class that is able to load data into a data source.
+/// It can load either from another data source or from a master file. The
+/// purpose of the class is only to hold the state for incremental loading.
+///
+/// The old content of zone is discarded and no journal is stored.
+class ZoneLoader {
+public:
+    /// \brief Constructor from master file.
+    ///
+    /// This initializes the zone loader to load from a master file.
+    ///
+    /// \param destination The data source into which the loaded data should
+    ///     go.
+    /// \param zone_name The origin of the zone. The class is implicit in the
+    ///     destination.
+    /// \param master_file Path to the master file to read data from.
+    /// \throw DataSourceError in case the zone does not exist in destination.
+    ///     This class does not support creating brand new zones, only loading
+    ///     data into them. In case a new zone is needed, it must be created
+    ///     beforehand.
+    /// \throw DataSourceError in case of other (possibly low-level) errors,
+    ///     such as read-only data source or database error.
+    ZoneLoader(DataSourceClient& destination, const isc::dns::Name& zone_name,
+               const char* master_file);
+
+    /// \brief Constructor from another data source.
+    ///
+    /// This initializes the zone loader to read from another data source.
+    /// It'll effectively copy data from one data source to another.
+    ///
+    /// \param destination The data source into which the loaded data should
+    ///     go.
+    /// \param zone_name The origin of the zone.
+    /// \param source The data source from which the data would be read.
+    /// \throw InvalidParameter in case the class of destination and source
+    ///     differs.
+    /// \throw NotImplemented in case the source data source client doesn't
+    ///     provide an iterator.
+    /// \throw DataSourceError in case the zone does not exist in destination.
+    ///     This class does not support creating brand new zones, only loading
+    ///     data into them. In case a new zone is needed, it must be created
+    ///     beforehand.
+    /// \throw DataSourceError in case the zone does not exist in the source.
+    /// \throw DataSourceError in case of other (possibly low-level) errors,
+    ///     such as read-only data source or database error.
+    ZoneLoader(DataSourceClient& destination, const isc::dns::Name& zone_name,
+               DataSourceClient& source);
+
+    /// \brief Perform the whole load.
+    ///
+    /// This performs the whole loading operation. It may take a long time.
+    ///
+    /// \throw InvalidOperation in case the loading was already completed
+    ///     before this call.
+    /// \throw DataSourceError in case some error (possibly low-level) happens.
+    /// \throw MasterFileError when the master_file is badly formatted or some
+    ///     similar problem is found when loading the master file.
+    void load() {
+        while (!loadIncremental(1000)) { // 1000 is arbitrary largish number
+            // Body intentionally left blank.
+        }
+    }
+
+    /// \brief Load up to limit RRs.
+    ///
+    /// This performs a part of the loading. In case there's enough data in the
+    /// source, it copies limit RRs. It can copy less RRs during the final call
+    /// (when there's less than limit left).
+    ///
+    /// This can be called repeatedly until the whole zone is loaded, having
+    /// pauses in the loading for some purposes (for example reporting
+    /// progress).
+    ///
+    /// \param limit The maximum allowed number of RRs to be loaded during this
+    ///     call.
+    /// \return True in case the loading is completed, false if there's more
+    ///     to load.
+    /// \throw InvalidOperation in case the loading was already completed
+    ///     before this call (by load() or by a loadIncremental that returned
+    ///     true).
+    /// \throw DataSourceError in case some error (possibly low-level) happens.
+    /// \throw MasterFileError when the master_file is badly formatted or some
+    ///     similar problem is found when loading the master file.
+    /// \note If the limit is exactly the number of RRs available to be loaded,
+    ///     the method still returns false and true'll be returned on the next
+    ///     call (which will load 0 RRs). This is because the end of iterator or
+    ///     master file is detected when reading past the end, not when the last
+    ///     one is read.
+    bool loadIncremental(size_t limit);
+private:
+    /// \brief The iterator used as source of data in case of the copy mode.
+    const ZoneIteratorPtr iterator_;
+    /// \brief The destination zone updater
+    const ZoneUpdaterPtr updater_;
+    /// \brief The master loader (for the master file mode)
+    boost::scoped_ptr<isc::dns::MasterLoader> loader_;
+    /// \brief Indicator if loading was completed
+    bool complete_;
+    /// \brief Was the loading successful?
+    bool loaded_ok_;
+};
+
+}
+}
+
+#endif

src/lib/dns/Makefile.am

@@ -102,6 +102,7 @@ libb10_dns___la_SOURCES += labelsequence.h labelsequence.cc
 libb10_dns___la_SOURCES += masterload.h masterload.cc
 libb10_dns___la_SOURCES += master_lexer.h master_lexer.cc
 libb10_dns___la_SOURCES += master_lexer_state.h
+libb10_dns___la_SOURCES += master_loader.h master_loader.cc
 libb10_dns___la_SOURCES += message.h message.cc
 libb10_dns___la_SOURCES += messagerenderer.h messagerenderer.cc
 libb10_dns___la_SOURCES += name.h name.cc

src/lib/dns/master_loader.cc

@@ -0,0 +1,278 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/master_loader.h>
+#include <dns/master_lexer.h>
+#include <dns/name.h>
+#include <dns/rrttl.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+#include <dns/rdata.h>
+
+#include <string>
+#include <memory>
+
+using std::string;
+using std::auto_ptr;
+
+namespace isc {
+namespace dns {
+
+class MasterLoader::MasterLoaderImpl {
+public:
+    MasterLoaderImpl(const char* master_file,
+                     const Name& zone_origin,
+                     const RRClass& zone_class,
+                     const MasterLoaderCallbacks& callbacks,
+                     const AddRRCallback& add_callback,
+                     MasterLoader::Options options) :
+        lexer_(),
+        zone_origin_(zone_origin),
+        zone_class_(zone_class),
+        callbacks_(callbacks),
+        add_callback_(add_callback),
+        options_(options),
+        master_file_(master_file),
+        initialized_(false),
+        ok_(true),
+        many_errors_((options & MANY_ERRORS) != 0),
+        complete_(false),
+        seen_error_(false)
+    {}
+
+    void reportError(const std::string& filename, size_t line,
+                     const std::string& reason)
+    {
+        seen_error_ = true;
+        callbacks_.error(filename, line, reason);
+        if (!many_errors_) {
+            // In case we don't have the lenient mode, every error is fatal
+            // and we throw
+            ok_ = false;
+            complete_ = true;
+            isc_throw(MasterLoaderError, reason.c_str());
+        }
+    }
+
+    void pushSource(const std::string& filename) {
+        std::string error;
+        if (!lexer_.pushSource(filename.c_str(), &error)) {
+            if (initialized_) {
+                // $INCLUDE file
+                reportError(lexer_.getSourceName(), lexer_.getSourceLine(),
+                            error);
+            } else {
+                // Top-level file
+                reportError("", 0, error);
+                ok_ = false;
+            }
+        }
+        initialized_ = true;
+    }
+
+    void pushStreamSource(std::istream& stream) {
+        lexer_.pushSource(stream);
+        initialized_ = true;
+    }
+
+    // Get a string token. Handle it as error if it is not string.
+    const string getString() {
+        lexer_.getNextToken(MasterToken::STRING).getString(string_token_);
+        return (string_token_);
+    }
+
+    bool loadIncremental(size_t count_limit);
+
+private:
+    MasterLexer lexer_;
+    const Name zone_origin_;
+    const RRClass zone_class_;
+    MasterLoaderCallbacks callbacks_;
+    AddRRCallback add_callback_;
+    const MasterLoader::Options options_;
+    const std::string master_file_;
+    std::string string_token_;
+    bool initialized_;
+    bool ok_;                   // Is it OK to continue loading?
+    const bool many_errors_;    // Are many errors allowed (or should we abort
+                                // on the first)
+public:
+    bool complete_;             // All work done.
+    bool seen_error_;           // Was there at least one error during the
+                                // load?
+};
+
+bool
+MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
+    if (count_limit == 0) {
+        isc_throw(isc::InvalidParameter, "Count limit set to 0");
+    }
+    if (complete_) {
+        isc_throw(isc::InvalidOperation,
+                  "Trying to load when already loaded");
+    }
+    if (!initialized_) {
+        pushSource(master_file_);
+    }
+    size_t count = 0;
+    while (ok_ && count < count_limit) {
+        try {
+            // Skip all EOLNs (empty lines) and finish on EOF
+            bool empty = true;
+            do {
+                const MasterToken& empty_token(lexer_.getNextToken());
+                if (empty_token.getType() == MasterToken::END_OF_FILE) {
+                    // TODO: Check if this is the last source, possibly pop
+                    return (true);
+                }
+                empty = empty_token.getType() == MasterToken::END_OF_LINE;
+            } while (empty);
+            // Return the last token, as it was not empty
+            lexer_.ungetToken();
+
+            const MasterToken::StringRegion&
+                name_string(lexer_.getNextToken(MasterToken::QSTRING).
+                            getStringRegion());
+            // TODO $ handling
+            const Name name(name_string.beg, name_string.len,
+                            &zone_origin_);
+            // TODO: Some more flexibility. We don't allow omitting
+            // anything yet
+
+            // The parameters
+            const RRTTL ttl(getString());
+            const RRClass rrclass(getString());
+            const RRType rrtype(getString());
+
+            // TODO: Some more validation?
+            if (rrclass != zone_class_) {
+                // It doesn't really matter much what type of exception
+                // we throw, we catch it just below.
+                isc_throw(isc::BadValue, "Class mismatch: " << rrclass <<
+                          "vs. " << zone_class_);
+            }
+            // TODO: Check if it is SOA, it should be at the origin.
+
+            const rdata::RdataPtr data(rdata::createRdata(rrtype, rrclass,
+                                                          lexer_,
+                                                          &zone_origin_,
+                                                          options_,
+                                                          callbacks_));
+            // In case we get NULL, it means there was error creating
+            // the Rdata. The errors should have been reported by
+            // callbacks_ already. We need to decide if we want to continue
+            // or not.
+            if (data) {
+                add_callback_(name, rrclass, rrtype, ttl, data);
+
+                // Good, we loaded another one
+                ++count;
+            } else {
+                seen_error_ = true;
+                if (!many_errors_) {
+                    ok_ = false;
+                    complete_ = true;
+                    // We don't have the exact error here, but it was reported
+                    // by the error callback.
+                    isc_throw(MasterLoaderError, "Invalid RR data");
+                }
+            }
+        } catch (const MasterLoaderError&) {
+            // This is a hack. We exclude the MasterLoaderError from the
+            // below case. Once we restrict the below to some smaller
+            // exception, we should remove this.
+            throw;
+        } catch (const isc::Exception& e) {
+            // TODO: Once we do #2518, catch only the DNSTextError here,
+            // not isc::Exception. The rest should be just simply
+            // propagated.
+            reportError(lexer_.getSourceName(), lexer_.getSourceLine(),
+                        e.what());
+            // We want to continue. Try to read until the end of line
+            bool end = false;
+            do {
+                const MasterToken& token(lexer_.getNextToken());
+                switch (token.getType()) {
+                    case MasterToken::END_OF_FILE:
+                        callbacks_.warning(lexer_.getSourceName(),
+                                           lexer_.getSourceLine(),
+                                           "File does not end with newline");
+                        // TODO: Try pop in case this is not the only
+                        // source
+                        return (true);
+                    case MasterToken::END_OF_LINE:
+                        end = true;
+                        break;
+                    default:
+                        // Do nothing. This is just to make compiler
+                        // happy
+                        break;
+                }
+            } while (!end);
+        }
+    }
+    // When there was a fatal error and ok is false, we say we are done.
+    return (!ok_);
+}
+
+MasterLoader::MasterLoader(const char* master_file,
+                           const Name& zone_origin,
+                           const RRClass& zone_class,
+                           const MasterLoaderCallbacks& callbacks,
+                           const AddRRCallback& add_callback,
+                           Options options)
+{
+    if (add_callback.empty()) {
+        isc_throw(isc::InvalidParameter, "Empty add RR callback");
+    }
+    impl_ = new MasterLoaderImpl(master_file, zone_origin,
+                                 zone_class, callbacks, add_callback, options);
+}
+
+MasterLoader::MasterLoader(std::istream& stream,
+                           const Name& zone_origin,
+                           const RRClass& zone_class,
+                           const MasterLoaderCallbacks& callbacks,
+                           const AddRRCallback& add_callback,
+                           Options options)
+{
+    if (add_callback.empty()) {
+        isc_throw(isc::InvalidParameter, "Empty add RR callback");
+    }
+    auto_ptr<MasterLoaderImpl> impl(new MasterLoaderImpl("", zone_origin,
+                                                         zone_class, callbacks,
+                                                         add_callback,
+                                                         options));
+    impl->pushStreamSource(stream);
+    impl_ = impl.release();
+}
+
+MasterLoader::~MasterLoader() {
+    delete impl_;
+}
+
+bool
+MasterLoader::loadIncremental(size_t count_limit) {
+    const bool result = impl_->loadIncremental(count_limit);
+    impl_->complete_ = result;
+    return (result);
+}
+
+bool
+MasterLoader::loadedSucessfully() const {
+    return (impl_->complete_ && !impl_->seen_error_);
+}
+
+} // end namespace dns
+} // end namespace isc

src/lib/dns/master_loader.h

@@ -15,20 +15,139 @@
 #ifndef MASTER_LOADER_H
 #define MASTER_LOADER_H
 
+#include <dns/master_loader_callbacks.h>
+
+#include <boost/noncopyable.hpp>
+
 namespace isc {
 namespace dns {
 
-// Placeholder introduced by #2497. The real class should be updated in
-// #2377.
-class MasterLoader {
+class Name;
+class RRClass;
+
+/// \brief Error while loading by MasterLoader without specifying the
+///     MANY_ERRORS option.
+class MasterLoaderError : public isc::Exception {
 public:
+    MasterLoaderError(const char* file, size_t line, const char* what) :
+        Exception(file, line, what)
+    {}
+};
+
+/// \brief A class able to load DNS master files
+///
+/// This class is able to produce a stream of RRs from a master file.
+/// It is able to load all of the master file at once, or by blocks
+/// incrementally.
+///
+/// It reports the loaded RRs and encountered errors by callbacks.
+class MasterLoader : boost::noncopyable {
+public:
+    /// \brief Options how the parsing should work.
     enum Options {
-         MANY_ERRORS, // lenient mode
-         // also eventually some check policies like "check NS name"
+        DEFAULT = 0,       ///< Nothing special.
+        MANY_ERRORS = 1    ///< Lenient mode (see documentation of MasterLoader
+                           ///  constructor).
     };
+
+    /// \brief Constructor
+    ///
+    /// This creates a master loader and provides it with all
+    /// relevant information.
+    ///
+    /// Except for the exceptions listed below, the constructor doesn't
+    /// throw. Most errors (like non-existent master file) are reported
+    /// by the callbacks during load() or loadIncremental().
+    ///
+    /// \param master_file Path to the file to load.
+    /// \param zone_origin The origin of zone to be expected inside
+    ///     the master file. Currently unused, but it is expected to
+    ///     be used for some validation.
+    /// \param zone_class The class of zone to be expected inside the
+    ///     master file.
+    /// \param callbacks The callbacks by which it should report problems.
+    ///     Usually, the callback carries a filename and line number of the
+    ///     input where the problem happens. There's a special case of empty
+    ///     filename and zero line in case the opening of the top-level master
+    ///     file fails.
+    /// \param add_callback The callback which would be called with each
+    ///     loaded RR.
+    /// \param options Options for the parsing, which is bitwise-or of
+    ///     the Options values or DEFAULT. If the MANY_ERRORS option is
+    ///     included, the parser tries to continue past errors. If it
+    ///     is not included, it stops at first encountered error.
+    /// \throw std::bad_alloc when there's not enough memory.
+    /// \throw isc::InvalidParameter if add_callback is empty.
+    MasterLoader(const char* master_file,
+                 const Name& zone_origin,
+                 const RRClass& zone_class,
+                 const MasterLoaderCallbacks& callbacks,
+                 const AddRRCallback& add_callback,
+                 Options options = DEFAULT);
+
+    /// \brief Constructor from a stream
+    ///
+    /// This is a constructor very similar to the previous one. The only
+    /// difference is it doesn't take a filename, but an input stream
+    /// to read the data from. It is expected to be mostly used in tests,
+    /// but it is public as it may possibly be useful for other currently
+    /// unknown purposes.
+    MasterLoader(std::istream& input,
+                 const Name& zone_origin,
+                 const RRClass& zone_class,
+                 const MasterLoaderCallbacks& callbacks,
+                 const AddRRCallback& add_callback,
+                 Options options = DEFAULT);
+
+    /// \brief Destructor
+    ~MasterLoader();
+
+    /// \brief Load some RRs
+    ///
+    /// This method loads at most count_limit RRs and reports them. In case
+    /// an error (either fatal or without MANY_ERRORS) or end of file is
+    /// encountered, they may be less.
+    ///
+    /// \param count_limit Upper limit on the number of RRs loaded.
+    /// \return In case it stops because of the count limit, it returns false.
+    ///     It returns true if the loading is done.
+    /// \throw isc::InvalidOperation when called after loading was done
+    ///     already.
+    /// \throw MasterLoaderError when there's an error in the input master
+    ///     file and the MANY_ERRORS is not specified. It never throws this
+    ///     in case MANY_ERRORS is specified.
+    bool loadIncremental(size_t count_limit);
+
+    /// \brief Load everything
+    ///
+    /// This simply calls loadIncremental until the loading is done.
+    /// \throw isc::InvalidOperation when called after loading was done
+    ///     already.
+    /// \throw MasterLoaderError when there's an error in the input master
+    ///     file and the MANY_ERRORS is not specified. It never throws this
+    ///     in case MANY_ERRORS is specified.
+    void load() {
+        while (!loadIncremental(1000)) { // 1000 = arbitrary largish number
+            // Body intentionally left blank
+        }
+    }
+
+    /// \brief Was the loading successful?
+    ///
+    /// \return true if and only if the loading was complete (after a call of
+    ///     load or after loadIncremental returned true) and there was no
+    ///     error. In other cases, return false.
+    /// \note While this method works even before the loading is complete (by
+    ///     returning false in that case), it is meant to be called only after
+    ///     finishing the load.
+    bool loadedSucessfully() const;
+
+private:
+    class MasterLoaderImpl;
+    MasterLoaderImpl* impl_;
 };
 
-}
-}
+} // end namespace dns
+} // end namespace isc
 
 #endif // MASTER_LOADER_H

src/lib/dns/master_loader_callbacks.h

@@ -23,20 +23,30 @@
 
 namespace isc {
 namespace dns {
+class Name;
+class RRClass;
+class RRType;
+class RRTTL;
+namespace rdata {
+class Rdata;
+typedef boost::shared_ptr<Rdata> RdataPtr;
+}
 
-class AbstractRRset;
-typedef boost::shared_ptr<AbstractRRset> RRsetPtr;
-
-/// \brief Type of callback to add a RRset.
+/// \brief Type of callback to add a RR.
 ///
 /// This type of callback is used by the loader to report another loaded
-/// RRset. The RRset is no longer preserved by the loader and is fully
+/// RR. The Rdata is no longer preserved by the loader and is fully
 /// owned by the callback.
 ///
-/// \param RRset The rrset to add. It does not contain the accompanying
-///     RRSIG (if the zone is signed), they are reported with separate
-///     calls to the callback.
-typedef boost::function<void(const RRsetPtr& rrset)> AddRRsetCallback;
+/// \param name The domain name where the RR belongs.
+/// \param rrclass The class of the RR.
+/// \param rrtype Type of the RR.
+/// \param rrttl Time to live of the RR.
+/// \param rdata The actual carried data of the RR.
+typedef boost::function<void(const Name& name, const RRClass& rrclass,
+                             const RRType& rrtype, const RRTTL& rrttl,
+                             const rdata::RdataPtr& rdata)>
+    AddRRCallback;
 
 /// \brief Set of issue callbacks for a loader.
 ///

src/lib/dns/rrset.h

@@ -770,9 +770,6 @@ public:
     //@{
     /// \brief Return pointer to this RRset's RRSIG RRset
     ///
-    /// \exception NotImplemented Always thrown.  Associated RRSIG RRsets are
-    ///            not supported in this class.
-    ///
     /// \return Null pointer, as this class does not support RRSIG records.
     virtual RRsetPtr getRRsig() const {
         return (RRsetPtr());

src/lib/dns/tests/Makefile.am

@@ -27,6 +27,7 @@ run_unittests_SOURCES += labelsequence_unittest.cc
 run_unittests_SOURCES += messagerenderer_unittest.cc
 run_unittests_SOURCES += master_lexer_token_unittest.cc
 run_unittests_SOURCES += master_lexer_unittest.cc
+run_unittests_SOURCES += master_loader_unittest.cc
 run_unittests_SOURCES += master_lexer_state_unittest.cc
 run_unittests_SOURCES += name_unittest.cc
 run_unittests_SOURCES += nsec3hash_unittest.cc

src/lib/dns/tests/master_loader_unittest.cc

@@ -0,0 +1,341 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/master_loader_callbacks.h>
+#include <dns/master_loader.h>
+#include <dns/rrtype.h>
+#include <dns/rrset.h>
+#include <dns/rrclass.h>
+#include <dns/name.h>
+#include <dns/rdata.h>
+
+#include <gtest/gtest.h>
+#include <boost/bind.hpp>
+#include <boost/scoped_ptr.hpp>
+
+#include <string>
+#include <vector>
+#include <list>
+#include <sstream>
+
+using namespace isc::dns;
+using std::vector;
+using std::string;
+using std::list;
+using std::stringstream;
+using std::endl;
+
+namespace {
+class MasterLoaderTest : public ::testing::Test {
+public:
+    MasterLoaderTest() :
+        callbacks_(boost::bind(&MasterLoaderTest::callback, this,
+                               &errors_, _1, _2, _3),
+                   boost::bind(&MasterLoaderTest::callback, this,
+                               &warnings_, _1, _2, _3))
+    {}
+
+    /// Concatenate file, line, and reason, and add it to either errors
+    /// or warnings
+    void callback(vector<string>* target, const std::string& file, size_t line,
+                  const std::string& reason)
+    {
+        std::stringstream ss;
+        ss << reason << " [" << file << ":" << line << "]";
+        target->push_back(ss.str());
+    }
+
+    void addRRset(const Name& name, const RRClass& rrclass,
+                  const RRType& rrtype, const RRTTL& rrttl,
+                  const rdata::RdataPtr& data) {
+        const RRsetPtr rrset(new BasicRRset(name, rrclass, rrtype, rrttl));
+        rrset->addRdata(data);
+        rrsets_.push_back(rrset);
+    }
+
+    void setLoader(const char* file, const Name& origin,
+                   const RRClass& rrclass, const MasterLoader::Options options)
+    {
+        loader_.reset(new MasterLoader(file, origin, rrclass, callbacks_,
+                                       boost::bind(&MasterLoaderTest::addRRset,
+                                                   this, _1, _2, _3, _4, _5),
+                                       options));
+    }
+
+    void setLoader(std::istream& stream, const Name& origin,
+                   const RRClass& rrclass, const MasterLoader::Options options)
+    {
+        loader_.reset(new MasterLoader(stream, origin, rrclass, callbacks_,
+                                       boost::bind(&MasterLoaderTest::addRRset,
+                                                   this, _1, _2, _3, _4, _5),
+                                       options));
+    }
+
+    static string prepareZone(const string& line, bool include_last) {
+        string result;
+        result += "example.org. 3600 IN SOA ns1.example.org. "
+            "admin.example.org. 1234 3600 1800 2419200 7200\n";
+        result += line;
+        if (include_last) {
+            result += "\n";
+            result += "correct 3600    IN  A 192.0.2.2\n";
+        }
+        return (result);
+    }
+
+    void clear() {
+        warnings_.clear();
+        errors_.clear();
+        rrsets_.clear();
+    }
+
+    // Check the next RR in the ones produced by the loader
+    // Other than passed arguments are checked to be the default for the tests
+    void checkRR(const string& name, const RRType& type, const string& data) {
+        ASSERT_FALSE(rrsets_.empty());
+        RRsetPtr current = rrsets_.front();
+        rrsets_.pop_front();
+
+        EXPECT_EQ(Name(name), current->getName());
+        EXPECT_EQ(type, current->getType());
+        EXPECT_EQ(RRClass::IN(), current->getClass());
+        ASSERT_EQ(1, current->getRdataCount());
+        EXPECT_EQ(0, isc::dns::rdata::createRdata(type, RRClass::IN(), data)->
+                  compare(current->getRdataIterator()->getCurrent()));
+    }
+
+    MasterLoaderCallbacks callbacks_;
+    boost::scoped_ptr<MasterLoader> loader_;
+    vector<string> errors_;
+    vector<string> warnings_;
+    list<RRsetPtr> rrsets_;
+};
+
+// Test simple loading. The zone file contains no tricky things, and nothing is
+// omitted. No RRset contains more than one RR Also no errors or warnings.
+TEST_F(MasterLoaderTest, basicLoad) {
+    setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
+              RRClass::IN(), MasterLoader::MANY_ERRORS);
+
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+
+    checkRR("example.org", RRType::SOA(),
+            "ns1.example.org. admin.example.org. "
+            "1234 3600 1800 2419200 7200");
+    checkRR("example.org", RRType::NS(), "ns1.example.org.");
+    checkRR("www.example.org", RRType::A(), "192.0.2.1");
+}
+
+// Check it works the same when created based on a stream, not filename
+TEST_F(MasterLoaderTest, streamConstructor) {
+    stringstream zone_stream(prepareZone("", true));
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+    checkRR("example.org", RRType::SOA(), "ns1.example.org. "
+            "admin.example.org. 1234 3600 1800 2419200 7200");
+    checkRR("correct.example.org", RRType::A(), "192.0.2.2");
+}
+
+// Try loading data incrementally.
+TEST_F(MasterLoaderTest, incrementalLoad) {
+    setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
+              RRClass::IN(), MasterLoader::MANY_ERRORS);
+
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    EXPECT_FALSE(loader_->loadIncremental(2));
+    EXPECT_FALSE(loader_->loadedSucessfully());
+
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+
+    checkRR("example.org", RRType::SOA(),
+            "ns1.example.org. admin.example.org. "
+            "1234 3600 1800 2419200 7200");
+    checkRR("example.org", RRType::NS(), "ns1.example.org.");
+
+    // The third one is not loaded yet
+    EXPECT_TRUE(rrsets_.empty());
+
+    // Load the rest.
+    EXPECT_TRUE(loader_->loadIncremental(20));
+    EXPECT_TRUE(loader_->loadedSucessfully());
+
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+
+    checkRR("www.example.org", RRType::A(), "192.0.2.1");
+}
+
+// Try loading from file that doesn't exist. There should be single error
+// saying so.
+TEST_F(MasterLoaderTest, invalidFile) {
+    setLoader("This file doesn't exist at all",
+              Name("exmaple.org."), RRClass::IN(), MasterLoader::MANY_ERRORS);
+
+    // Nothing yet. The loader is dormant until invoked.
+    // Is it really what we want?
+    EXPECT_TRUE(errors_.empty());
+
+    loader_->load();
+
+    EXPECT_TRUE(warnings_.empty());
+    EXPECT_TRUE(rrsets_.empty());
+    ASSERT_EQ(1, errors_.size());
+    EXPECT_EQ(0, errors_[0].find("Error opening the input source file: ")) <<
+        "Different error: " << errors_[0];
+}
+
+struct ErrorCase {
+    const char* const line;    // The broken line in master file
+    const char* const problem; // Description of the problem for SCOPED_TRACE
+} const error_cases[] = {
+    { "www...   3600    IN  A   192.0.2.1", "Invalid name" },
+    { "www      FORTNIGHT   IN  A   192.0.2.1", "Invalid TTL" },
+    { "www      3600    XX  A   192.0.2.1", "Invalid class" },
+    { "www      3600    IN  A   bad_ip", "Invalid Rdata" },
+    { "www      3600    IN", "Unexpected EOLN" },
+    { "www      3600    CH  TXT nothing", "Class mismatch" },
+    { "www      \"3600\"  IN  A   192.0.2.1", "Quoted TTL" },
+    { "www      3600    \"IN\"  A   192.0.2.1", "Quoted class" },
+    { "www      3600    IN  \"A\"   192.0.2.1", "Quoted type" },
+    { "unbalanced)paren 3600    IN  A   192.0.2.1", "Token error 1" },
+    { "www  3600    unbalanced)paren    A   192.0.2.1", "Token error 2" },
+    { NULL, NULL }
+};
+
+// Test a broken zone is handled properly. We test several problems,
+// both in strict and lenient mode.
+TEST_F(MasterLoaderTest, brokenZone) {
+    for (const ErrorCase* ec = error_cases; ec->line != NULL; ++ec) {
+        SCOPED_TRACE(ec->problem);
+        const string zone(prepareZone(ec->line, true));
+
+        {
+            SCOPED_TRACE("Strict mode");
+            clear();
+            stringstream zone_stream(zone);
+            setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+                      MasterLoader::DEFAULT);
+            EXPECT_FALSE(loader_->loadedSucessfully());
+            EXPECT_THROW(loader_->load(), MasterLoaderError);
+            EXPECT_FALSE(loader_->loadedSucessfully());
+            EXPECT_EQ(1, errors_.size()) << errors_[0];
+            EXPECT_TRUE(warnings_.empty());
+
+            checkRR("example.org", RRType::SOA(), "ns1.example.org. "
+                    "admin.example.org. 1234 3600 1800 2419200 7200");
+            // In the strict mode, it is aborted. The last RR is not
+            // even attempted.
+            EXPECT_TRUE(rrsets_.empty());
+        }
+
+        {
+            SCOPED_TRACE("Lenient mode");
+            clear();
+            stringstream zone_stream(zone);
+            setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+                      MasterLoader::MANY_ERRORS);
+            EXPECT_FALSE(loader_->loadedSucessfully());
+            EXPECT_NO_THROW(loader_->load());
+            EXPECT_FALSE(loader_->loadedSucessfully());
+            EXPECT_EQ(1, errors_.size());
+            EXPECT_TRUE(warnings_.empty());
+            checkRR("example.org", RRType::SOA(), "ns1.example.org. "
+                    "admin.example.org. 1234 3600 1800 2419200 7200");
+            // This one is below the error one.
+            checkRR("correct.example.org", RRType::A(), "192.0.2.2");
+            EXPECT_TRUE(rrsets_.empty());
+        }
+
+        {
+            SCOPED_TRACE("Error at EOF");
+            // This case is interesting only in the lenient mode.
+            const string zoneEOF(prepareZone(ec->line, false));
+            clear();
+            stringstream zone_stream(zoneEOF);
+            setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+                      MasterLoader::MANY_ERRORS);
+            EXPECT_FALSE(loader_->loadedSucessfully());
+            EXPECT_NO_THROW(loader_->load());
+            EXPECT_FALSE(loader_->loadedSucessfully());
+            EXPECT_EQ(1, errors_.size());
+            // The unexpected EOF warning
+            EXPECT_EQ(1, warnings_.size());
+            checkRR("example.org", RRType::SOA(), "ns1.example.org. "
+                    "admin.example.org. 1234 3600 1800 2419200 7200");
+            EXPECT_TRUE(rrsets_.empty());
+        }
+    }
+}
+
+// Test the constructor rejects empty add callback.
+TEST_F(MasterLoaderTest, emptyCallback) {
+    EXPECT_THROW(MasterLoader(TEST_DATA_SRCDIR "/example.org",
+                              Name("example.org"), RRClass::IN(), callbacks_,
+                              AddRRCallback()), isc::InvalidParameter);
+    // And the same with the second constructor
+    stringstream ss("");
+    EXPECT_THROW(MasterLoader(ss, Name("example.org"), RRClass::IN(),
+                              callbacks_, AddRRCallback()),
+                 isc::InvalidParameter);
+}
+
+// Check it throws when we try to load after loading was complete.
+TEST_F(MasterLoaderTest, loadTwice) {
+    setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
+              RRClass::IN(), MasterLoader::MANY_ERRORS);
+
+    loader_->load();
+    EXPECT_THROW(loader_->load(), isc::InvalidOperation);
+}
+
+// Load 0 items should be rejected
+TEST_F(MasterLoaderTest, loadZero) {
+    setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
+              RRClass::IN(), MasterLoader::MANY_ERRORS);
+    EXPECT_THROW(loader_->loadIncremental(0), isc::InvalidParameter);
+}
+
+// Test there's a warning when the file terminates without end of
+// line.
+TEST_F(MasterLoaderTest, noEOLN) {
+    // No \n at the end
+    const string input("example.org. 3600 IN SOA ns1.example.org. "
+                       "admin.example.org. 1234 3600 1800 2419200 7200");
+    stringstream ss(input);
+    setLoader(ss, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty()) << errors_[0];
+    // There should be one warning about the EOLN
+    EXPECT_EQ(1, warnings_.size());
+    checkRR("example.org", RRType::SOA(), "ns1.example.org. "
+            "admin.example.org. 1234 3600 1800 2419200 7200");
+}
+
+}

src/lib/dns/tests/testdata/Makefile.am

@@ -170,6 +170,7 @@ EXTRA_DIST += tsig_verify1.spec tsig_verify2.spec tsig_verify3.spec
 EXTRA_DIST += tsig_verify4.spec tsig_verify5.spec tsig_verify6.spec
 EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec
 EXTRA_DIST += tsig_verify10.spec
+EXTRA_DIST += example.org
 
 .spec.wire:
 	$(PYTHON) $(top_builddir)/src/lib/util/python/gen_wiredata.py -o $@ $<

src/lib/dns/tests/testdata/example.org

@@ -0,0 +1,15 @@
+example.org.        3600    IN  SOA ( ; The SOA, split across lines for testing
+    ns1.example.org.
+    admin.example.org.
+    1234
+    3600
+    1800
+    2419200
+    7200
+    )
+; Check it accepts quoted name too
+"\101xample.org."        3600    IN  NS ns1.example.org.
+
+
+; Some empty lines here. They are to make sure the loader can skip them.
+www                 3600    IN  A 192.0.2.1 ; Test a relative name as well.