[mavericks] use MasterLoader as a backend of masterLoad().

it has always been desirable, but it's now even critical as istream::operator>>
always works with the C++11 definition, which breaks the internal assumption
of the old implementation.  It specifically happens on Mac OS X 10.9.

src/lib/dns/masterload.cc

@@ -19,11 +19,13 @@
 #include <cctype>
 #include <cerrno>
 
+#include <boost/bind.hpp>
 #include <boost/scoped_ptr.hpp>
 
 #include <exceptions/exceptions.h>
 
 #include <dns/masterload.h>
+#include <dns/master_loader.h>
 #include <dns/name.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
@@ -31,6 +33,7 @@
 #include <dns/rrset.h>
 #include <dns/rrttl.h>
 #include <dns/rrtype.h>
+#include <dns/rrcollator.h>
 
 using namespace std;
 using namespace boost;
@@ -39,25 +42,27 @@ using namespace isc::dns::rdata;
 namespace isc {
 namespace dns {
 namespace {
-// A helper function that strips off any comment or whitespace at the end of
-// an RR.
-// This is an incomplete implementation, and cannot handle all such comments;
-// it's considered a short term workaround to deal with some real world
-// cases.
-string
-stripLine(string& s, const Exception& ex) {
-    // Find any ';' in the text data, and locate the position of the last
-    // occurrence.  Note that unless/until we support empty RDATA it
-    // shouldn't be placed at the beginning of the data.
-    const size_t pos_semicolon = s.rfind(';');
-    if (pos_semicolon == 0) {
-        throw ex;
-    } else if (pos_semicolon != string::npos) {
-        s.resize(pos_semicolon);
+void
+callbackWrapper(const RRsetPtr& rrset, MasterLoadCallback callback,
+                const Name* origin)
+{
+    // Origin related validation:
+    //  - reject out-of-zone data
+    //  - reject SOA whose owner is not at the top of zone
+    const NameComparisonResult cmp_result =
+        rrset->getName().compare(*origin);
+    if (cmp_result.getRelation() != NameComparisonResult::EQUAL &&
+        cmp_result.getRelation() != NameComparisonResult::SUBDOMAIN) {
+        isc_throw(MasterLoadError, "Out-of-zone data for " << *origin
+                  << "/" << rrset->getClass() << ": " << rrset->getName());
     }
-    // Remove any trailing whitespace return the resulting text.
-    s.resize(s.find_last_not_of(" \t") + 1);
-    return (s);
+    if (rrset->getType() == RRType::SOA() &&
+        cmp_result.getRelation() != NameComparisonResult::EQUAL) {
+        isc_throw(MasterLoadError, "SOA not at top of zone: "
+                  << *rrset);
+    }
+
+    callback(rrset);
 }
 }
 
@@ -69,154 +74,35 @@ masterLoad(const char* const filename, const Name& origin,
         isc_throw(MasterLoadError, "Name of master file must not be null");
     }
 
-    ifstream ifs;
-    ifs.open(filename, ios_base::in);
-    if (ifs.fail()) {
-        isc_throw(MasterLoadError, "Failed to open master file: " <<
-                  filename << ": " << strerror(errno));
+    RRCollator rr_collator(boost::bind(callbackWrapper, _1, callback, &origin));
+    MasterLoader loader(filename, origin, zone_class,
+                        MasterLoaderCallbacks::getNullCallbacks(),
+                        rr_collator.getCallback());
+    try {
+        loader.load();
+    } catch (const MasterLoaderError& ex) {
+        isc_throw(MasterLoadError, ex.what());
     }
-    masterLoad(ifs, origin, zone_class, callback, filename);
-    ifs.close();
+    rr_collator.flush();
 }
 
 void
 masterLoad(istream& input, const Name& origin, const RRClass& zone_class,
            MasterLoadCallback callback, const char* source)
 {
-    RRsetPtr rrset;
-    ConstRdataPtr prev_rdata;   // placeholder for special case of RRSIGs
-    string line;
-    unsigned int line_count = 1;
-
+    RRCollator rr_collator(boost::bind(callbackWrapper, _1, callback, &origin));
+    MasterLoader loader(input, origin, zone_class,
+                        MasterLoaderCallbacks::getNullCallbacks(),
+                        rr_collator.getCallback());
     if (source == NULL) {
         source = "<unknown>";
     }
-
-    do {
-        getline(input, line);
-        if (input.bad() || (input.fail() && !input.eof())) {
-            isc_throw(MasterLoadError, "Unexpectedly failed to read a line in "
-                                       << source);
-        }
-
-        // blank/comment lines should be simply skipped.
-        if (line.empty() || line[0] == ';') {
-            continue;
-        }
-
-        // The line shouldn't have leading space (which means omitting the
-        // owner name).
-        if (isspace(line[0])) {
-            isc_throw(MasterLoadError, "Leading space in " << source
-                                       << " at line " << line_count);
-        }
-
-        // Parse a single RR
-        istringstream iss(line);
-        string owner_txt, ttl_txt, rrclass_txt, rrtype_txt;
-        stringbuf rdatabuf;
-        iss >> owner_txt >> ttl_txt >> rrclass_txt >> rrtype_txt >> &rdatabuf;
-        if (iss.bad() || iss.fail()) {
-            isc_throw(MasterLoadError, "Parse failure for a valid RR in "
-                      << source << " at line " << line_count);
-        }
-
-        // This simple version doesn't support relative owner names with a
-        // separate origin.
-        if (owner_txt.empty() || *(owner_txt.end() - 1) != '.') {
-            isc_throw(MasterLoadError, "Owner name is not absolute in "
-                      << source << " at line " << line_count);
-        }
-
-        // XXX: this part is a bit tricky (and less efficient).  We are going
-        // to validate the text for the RR parameters, and throw an exception
-        // if any of them is invalid by converting an underlying exception
-        // to MasterLoadError.  To do that, we need to define the corresponding
-        // variables used for RRset construction outside the try-catch block,
-        // but we don't like to use a temporary variable with a meaningless
-        // initial value.  So we define pointers outside the try block
-        // and allocate/initialize the actual objects within the block.
-        // To make it exception safe we use Boost.scoped_ptr.
-        scoped_ptr<const Name> owner;
-        scoped_ptr<const RRTTL> ttl;
-        scoped_ptr<const RRClass> rrclass;
-        scoped_ptr<const RRType> rrtype;
-        ConstRdataPtr rdata;
-        try {
-            owner.reset(new Name(owner_txt));
-            ttl.reset(new RRTTL(ttl_txt));
-            rrclass.reset(new RRClass(rrclass_txt));
-            rrtype.reset(new RRType(rrtype_txt));
-            string rdtext = rdatabuf.str();
-            try {
-                rdata = createRdata(*rrtype, *rrclass, rdtext);
-            } catch (const Exception& ex) {
-                // If the parse for the RDATA fails, check if it has comments
-                // or whitespace at the end, and if so, retry the conversion
-                // after stripping off the comment or whitespace
-                rdata = createRdata(*rrtype, *rrclass, stripLine(rdtext, ex));
-            }
-        } catch (const Exception& ex) {
-            isc_throw(MasterLoadError, "Invalid RR text in " << source <<
-                                       " at line " << line_count << ": "
-                                       << ex.what());
-        }
-
-        // Origin related validation:
-        //  - reject out-of-zone data
-        //  - reject SOA whose owner is not at the top of zone
-        const NameComparisonResult cmp_result = owner->compare(origin);
-        if (cmp_result.getRelation() != NameComparisonResult::EQUAL &&
-            cmp_result.getRelation() != NameComparisonResult::SUBDOMAIN) {
-            isc_throw(MasterLoadError, "Out-of-zone data for " << origin
-                                       << "/" << rrclass_txt << " in "
-                                       << source << " at line "
-                                       << line_count);
-        }
-        if (*rrtype == RRType::SOA() &&
-            cmp_result.getRelation() != NameComparisonResult::EQUAL) {
-            isc_throw(MasterLoadError, "SOA not at top of zone in "
-                      << source << " at line " << line_count);
-        }
-
-        // Reject RR class mismatching
-        if (*rrclass != zone_class) {
-            isc_throw(MasterLoadError, "RR class (" << rrclass_txt
-                      << ") does not match the zone class (" << zone_class
-                      << ") in " << source << " at line " << line_count);
-        }
-
-        // Everything is okay.  Now create/update RRset with the new RR.
-        // If this is the first RR or the RR type/name is new, we are seeing
-        // a new RRset.
-        bool new_rrset = false;
-        if (!rrset || rrset->getType() != *rrtype ||
-            rrset->getName() != *owner) {
-            new_rrset = true;
-        } else if (rrset->getType() == RRType::RRSIG()) {
-            // We are seeing two consecutive RRSIGs of the same name.
-            // They can be combined iff they have the same type covered.
-            if (dynamic_cast<const generic::RRSIG&>(*rdata).typeCovered() !=
-                dynamic_cast<const generic::RRSIG&>(*prev_rdata).typeCovered())
-            {
-                new_rrset = true;
-            }
-        }
-        if (new_rrset) {
-            // Commit the previous RRset, if any.
-            if (rrset) {
-                callback(rrset);
-            }
-            rrset = RRsetPtr(new RRset(*owner, *rrclass, *rrtype, *ttl));
-        }
-        rrset->addRdata(rdata);
-        prev_rdata = rdata;
-    } while (++line_count, !input.eof());
-
-    // Commit the last RRset, if any.
-    if (rrset) {
-        callback(rrset);
+    try {
+        loader.load();
+    } catch (const MasterLoaderError& ex) {
+        isc_throw(MasterLoadError, ex.what());
     }
+    rr_collator.flush();
 }
 
 } // namespace dns

src/lib/dns/masterload.h

@@ -64,86 +64,22 @@ typedef boost::function<void(RRsetPtr)> MasterLoadCallback;
 /// and this function never uses it once it is called.
 /// The callback can freely modify the passed \c RRset.
 ///
-/// This function performs minimum level of validation on the input:
-/// - Each RR is a valid textual representation per the DNS protocol.
-/// - The class of each RR must be identical to the specified RR class.
-/// - The owner name of each RR must be a subdomain of the origin name
-///   (that can be equal to the origin).
-/// - If an SOA RR is included, its owner name must be the origin name.
-/// If any of these validation checks fails, this function throws an
-/// exception of class \c MasterLoadError.
+/// This function internally uses the MasterLoader class, and basically
+/// accepts and rejects input that MasterLoader accepts and rejects,
+/// accordingly.  In addition, this function performs the following validation:
+/// if an SOA RR is included, its owner name must be the origin name.
 ///
 /// It does not perform other semantical checks, however.  For example,
 /// it doesn't check if an NS RR of the origin name is included or if
 /// there is more than one SOA RR.  Such further checks are the caller's
 /// (or the callback's) responsibility.
 ///
-/// <b>Acceptable Format</b>
-///
-/// The current implementation only supports a restricted form of master files
-/// for simplicity.  One easy way to ensure that a handwritten zone file is
-/// acceptable to this implementation is to preprocess it with BIND 9's
-/// named-compilezone tool with both the input and output formats being
-/// "text".
-/// Here is an example:
-/// \code % named-compilezone -f text -F text -o example.com.norm
-///      example.com example.com.zone
-/// \endcode
-/// where example.com.zone is the original zone file for the "example.com"
-/// zone.  The output file is example.com.norm, which should be acceptable
-/// by this implementation.
-///
-/// Below are specific restrictions that this implementation assumes.
-/// Basically, each RR must consist of exactly one line
-/// (so there shouldn't be a multi-line RR) in the following format:
-/// \code  <owner name> <TTL> <RRCLASS> <RRTYPE> <RDATA (single line)>
-/// \endcode
-/// Here are some more details about the restrictions:
-/// - No special directives such as $TTL are supported.
-/// - The owner name must be absolute, that is, it must end with a period.
-/// - "@" is not recognized as a valid owner name.
-/// - Owner names, TTL and RRCLASS cannot be omitted.
-/// - As a corollary, a non blank line must not begin with a space character.
-/// - The order of the RR parameters is fixed, for example, this is acceptable:
-/// \code example.com. 3600 IN A 192.0.2.1
-/// \endcode
-///  but this is not even though it's valid per RFC1035:
-/// \code example.com. IN 3600 A 192.0.2.1
-/// \endcode
-/// - "TTL", "RRCLASS", and "RRTYPE" must be recognizable by the \c RRTTL,
-///   RRClass and RRType class implementations of this library.  In particular,
-///   as of this writing TTL must be a decimal number (a convenient extension
-///   such as "1H" instead of 3600 cannot be used).  Not all standard RR
-///   classes and RR types are supported yet, so the mnemonics of them will
-///   be rejected, too.
-/// - RR TTLs of the same RRset must be the same; even if they are different,
-///   this implementation simply uses the TTL of the first RR.
-///
-/// Blank lines and lines beginning with a semi-colon are allowed, and will
-/// be simply ignored.  Comments cannot coexist with an RR line, however.
-/// For example, this will be rejected:
-/// \code example.com. 3600 IN A 192.0.2.1 ; this is a comment
-/// \endcode
-///
-/// This implementation assumes that RRs of a single RRset are not
-/// interleaved with RRs of a different RRset.
-/// That is, the following sequence shouldn't happen:
-/// \code example.com. 3600 IN A 192.0.2.1
-/// example.com. 3600 IN AAAA 2001:db8::1
-/// example.com. 3600 IN A 192.0.2.2
-/// \endcode
-/// But it does not consider this an error; it will simply regard each RR
-/// as a separate RRset and call the callback with them separately.
-/// It is up to the callback to merge multiple RRsets into one if possible
-/// and necessary.
-///
 /// <b>Exceptions</b>
 ///
 /// This function throws an exception of class \c MasterLoadError in the
 /// following cases:
-/// - Any of the validation checks fails (see the class description).
-/// - The input data is not in the acceptable format (see the details of
-///   the format above).
+/// - Any of the validation checks fails (see above).
+/// - The input data has a syntax error.
 /// - The specified file cannot be opened for loading.
 /// - An I/O error occurs during the loading.
 ///
@@ -196,16 +132,12 @@ typedef boost::function<void(RRsetPtr)> MasterLoadCallback;
 /// The current implementation is in a preliminary level and needs further
 /// extensions.  Some design decisions may also have to be reconsidered as
 /// we gain experiences.  Those include:
-/// - We should be more flexible about the input format.
 /// - We may want to allow optional conditions.  For example, we may want to
 ///   be generous about some validation failures and be able to continue
 ///   parsing.
 /// - Especially if we allow to be generous, we may also want to support
 ///   returning an error code instead of throwing an exception when we
 ///   encounter validation failure.
-/// - We may want to support incremental loading.
-/// - If we add these optional features we may want to introduce a class
-///   that encapsulates loading status and options.
 /// - RRSIGs are handled as separate RRsets, i.e. they are not included in
 ///   the RRset they cover.
 ///

src/lib/dns/tests/masterload_unittest.cc

@@ -267,6 +267,15 @@ TEST_F(MasterLoadTest, loadRRNoComment) {
                                       "\"aaa;bbb\"")));
 }
 
+TEST_F(MasterLoadTest, nonAbsoluteOwner) {
+    // If the owner name is not absolute, the zone origin is assumed to be
+    // its origin.
+    rr_stream << "example.com 3600 IN A 192.0.2.1";
+    masterLoad(rr_stream, origin, zclass, callback);
+    EXPECT_EQ(1, results.size());
+    EXPECT_EQ(results[0]->getName(), Name("example.com.example.com"));
+}
+
 TEST_F(MasterLoadTest, loadRREmptyAndComment) {
     // There's no RDATA (invalid in this case) but a comment.  This position
     // shouldn't cause any disruption and should be treated as a normal error.
@@ -356,11 +365,6 @@ TEST_F(MasterLoadTest, loadBadRRText) {
     stringstream rr_stream6("example.com. 3600 IN A");
     EXPECT_THROW(masterLoad(rr_stream6, origin, zclass, callback),
                  MasterLoadError);
-
-    // owner name is not absolute
-    stringstream rr_stream7("example.com 3600 IN A 192.0.2.1");
-    EXPECT_THROW(masterLoad(rr_stream7, origin, zclass, callback),
-                 MasterLoadError);
 }
 
 // This is a helper callback to test the case the input stream becomes bad