Merge #2429

To bring in test tools to check the actual produced error message.

Conflicts:
	src/lib/dns/master_loader.cc
	src/lib/dns/tests/master_loader_unittest.cc

src/lib/dns/master_loader.cc

@@ -15,11 +15,14 @@
 #include <dns/master_loader.h>
 #include <dns/master_lexer.h>
 #include <dns/name.h>
+#include <dns/rdataclass.h>
 #include <dns/rrttl.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 #include <dns/rdata.h>
 
+#include <boost/scoped_ptr.hpp>
+
 #include <string>
 #include <memory>
 #include <vector>
@@ -58,6 +61,7 @@ public:
                      const MasterLoaderCallbacks& callbacks,
                      const AddRRCallback& add_callback,
                      MasterLoader::Options options) :
+        MAX_TTL(0x7fffffff),
         lexer_(),
         zone_origin_(zone_origin),
         active_origin_(zone_origin),
@@ -71,23 +75,10 @@ public:
         many_errors_((options & MANY_ERRORS) != 0),
         previous_name_(false),
         complete_(false),
-        seen_error_(false)
+        seen_error_(false),
+        warn_rfc1035_ttl_(true)
     {}
 
-    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)) {
@@ -105,6 +96,28 @@ public:
         previous_name_ = false;
     }
 
+    void pushStreamSource(std::istream& stream) {
+        lexer_.pushSource(stream);
+        initialized_ = true;
+    }
+
+    bool loadIncremental(size_t count_limit);
+
+private:
+    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());
+        }
+    }
+
     bool popSource() {
         if (lexer_.getSourceCount() == 1) {
             return (false);
@@ -123,19 +136,12 @@ public:
         return (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);
-
     MasterToken handleInitialToken();
 
     void doOrigin(bool is_optional) {
@@ -174,6 +180,114 @@ public:
         pushSource(filename);
     }
 
+    // Upper limit check when recognizing a specific TTL value from the
+    // zone file ($TTL, the RR's TTL field, or the SOA minimum).  RFC2181
+    // Section 8 limits the range of TTL values to unsigned 32-bit integers,
+    // and prohibits transmitting a TTL field exceeding this range.  We
+    // guarantee that by limiting the value at the time of zone
+    // parsing/loading, following what BIND 9 does.  Resetting it to 0
+    // at this point may not be exactly what the RFC states, but the end
+    // result would be the same.  Again, we follow the BIND 9's behavior here.
+    //
+    // post_parsing is true iff this method is called after parsing the entire
+    // RR and the lexer is positioned at the next line.  It's just for
+    // calculating the accurate source line when callback is necessary.
+    void limitTTL(RRTTL& ttl, bool post_parsing) {
+        if (ttl > MAX_TTL) {
+            const size_t src_line = lexer_.getSourceLine() -
+                (post_parsing ? 1 : 0);
+            callbacks_.warning(lexer_.getSourceName(), src_line,
+                               "TTL " + ttl.toText() + " > MAXTTL, "
+                               "setting to 0 per RFC2181");
+            ttl = RRTTL(0);
+        }
+    }
+
+    // Set/reset the default TTL.  Either from $TTL or SOA minimum TTL.
+    // see LimitTTL() for parameter post_parsing.
+    void setDefaultTTL(const RRTTL& ttl, bool post_parsing) {
+        if (!default_ttl_) {
+            default_ttl_.reset(new RRTTL(ttl));
+        } else {
+            *default_ttl_ = ttl;
+        }
+        limitTTL(*default_ttl_, post_parsing);
+    }
+
+    // Set/reset the TTL currently being used.  This can be used the last
+    // resort TTL when no other TTL is known for an RR.
+    void setCurrentTTL(const RRTTL& ttl) {
+        if (!current_ttl_) {
+            current_ttl_.reset(new RRTTL(ttl));
+        } else {
+            *current_ttl_ = ttl;
+        }
+    }
+
+    // Try to set/reset the current TTL from candidate TTL text.  It's possible
+    // it does not actually represent a TTL (which is not immediately
+    // considered an error).  Return true iff it's recognized as a valid TTL
+    // (and only in which case the current TTL is set).
+    bool setCurrentTTL(const string& ttl_txt) {
+        // We use the factory version instead of RRTTL constructor as we
+        // need to expect cases where ttl_txt does not actually represent a TTL
+        // but an RR class or type.
+        RRTTL* ttl = RRTTL::createFromText(ttl_txt, current_ttl_.get());
+        if (ttl != NULL) {
+            if (!current_ttl_) {
+                current_ttl_.reset(ttl);
+            }
+            limitTTL(*current_ttl_, false);
+            return (true);
+        }
+        return (false);
+    }
+
+    // Determine the TTL of the current RR based on the given parsing context.
+    //
+    // explicit_ttl is true iff the TTL is explicitly specified for that RR
+    // (in which case current_ttl_ is set to that TTL).
+    // rrtype is the type of the current RR, and rdata is its RDATA.  They
+    // only matter if the type is SOA and no available TTL is known.  In this
+    // case the minimum TTL of the SOA will be used as the TTL of that SOA
+    // and the default TTL for subsequent RRs.
+    const RRTTL& getCurrentTTL(bool explicit_ttl, const RRType& rrtype,
+                               const rdata::ConstRdataPtr& rdata) {
+        // We've completed parsing the full of RR, and the lexer is already
+        // positioned at the next line.  If we need to call callback,
+        // we need to adjust the line number.
+        const size_t current_line = lexer_.getSourceLine() - 1;
+
+        if (!current_ttl_ && !default_ttl_) {
+            if (rrtype == RRType::SOA()) {
+                callbacks_.warning(lexer_.getSourceName(), current_line,
+                                   "no TTL specified; "
+                                   "using SOA MINTTL instead");
+                const uint32_t ttl_val =
+                    dynamic_cast<const rdata::generic::SOA&>(*rdata).
+                    getMinimum();
+                setDefaultTTL(RRTTL(ttl_val), true);
+                setCurrentTTL(*default_ttl_);
+            } else {
+                // On catching the exception we'll try to reach EOL again,
+                // so we need to unget it now.
+                lexer_.ungetToken();
+                throw InternalException(__FILE__, __LINE__,
+                                        "no TTL specified; load rejected");
+            }
+        } else if (!explicit_ttl && default_ttl_) {
+            setCurrentTTL(*default_ttl_);
+        } else if (!explicit_ttl && warn_rfc1035_ttl_) {
+            // Omitted (class and) TTL values are default to the last
+            // explicitly stated values (RFC 1035, Sec. 5.1).
+            callbacks_.warning(lexer_.getSourceName(), current_line,
+                               "using RFC1035 TTL semantics");
+            warn_rfc1035_ttl_ = false; // we only warn about this once
+        }
+        assert(current_ttl_);
+        return (*current_ttl_);
+    }
+
     void handleDirective(const char* directive, size_t length) {
         if (iequals(directive, "INCLUDE")) {
             doInclude();
@@ -183,10 +297,10 @@ public:
             // because it's shared with the doInclude and that one can't do
             // it.
             eatUntilEOL(true);
-        } else if (iequals(directive, "TTL")) {
             // TODO: Implement
-            isc_throw(isc::NotImplemented,
-                      "TTL directive not implemented yet");
+        } else if (iequals(directive, "TTL")) {
+            setDefaultTTL(RRTTL(getString()), false);
+            eatUntilEOL(true);
         } else {
             isc_throw(InternalException, "Unknown directive '" <<
                       string(directive, directive + length) << "'");
@@ -223,6 +337,11 @@ public:
     }
 
 private:
+    // RFC2181 Section 8 specifies TTLs are unsigned 32-bit integer,
+    // effectively limiting the maximum value to 2^32-1.  This constant
+    // represent a TTL of the max value.
+    const RRTTL MAX_TTL;
+
     MasterLexer lexer_;
     const Name zone_origin_;
     Name active_origin_; // The origin used during parsing
@@ -231,6 +350,12 @@ private:
     const RRClass zone_class_;
     MasterLoaderCallbacks callbacks_;
     AddRRCallback add_callback_;
+    boost::scoped_ptr<RRTTL> default_ttl_; // Default TTL of RRs used when
+                                           // unspecified.  If NULL no default
+                                           // is known.
+    boost::scoped_ptr<RRTTL> current_ttl_; // The TTL used most recently.
+                                           // Initially set to NULL.  Once set
+                                           // always non NULL.
     const MasterLoader::Options options_;
     const std::string master_file_;
     std::string string_token_;
@@ -249,6 +374,8 @@ public:
     bool complete_;             // All work done.
     bool seen_error_;           // Was there at least one error during the
                                 // load?
+    bool warn_rfc1035_ttl_;     // should warn if implicit TTL determination
+                                // from the previous RR is used.
 };
 
 // A helper method of loadIncremental, parsing the first token of a new line.
@@ -366,8 +493,18 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
             // anything yet
 
             // The parameters
-            const RRTTL ttl(next_token.getString());
-            const RRClass rrclass(getString());
+            MasterToken rrparam_token = next_token;
+
+            bool explicit_ttl = false;
+            if (rrparam_token.getType() == MasterToken::STRING) {
+                // Try TTL
+                if (setCurrentTTL(rrparam_token.getString())) {
+                    explicit_ttl = true;
+                    rrparam_token = lexer_.getNextToken();
+                }
+            }
+
+            const RRClass rrclass(rrparam_token.getString());
             const RRType rrtype(getString());
 
             // TODO: Some more validation?
@@ -379,7 +516,7 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
             }
             // TODO: Check if it is SOA, it should be at the origin.
 
-            const rdata::RdataPtr data(rdata::createRdata(rrtype, rrclass,
+            const rdata::RdataPtr rdata(rdata::createRdata(rrtype, rrclass,
                                                           lexer_,
                                                           &active_origin_,
                                                           options_,
@@ -388,9 +525,10 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
             // 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_(*last_name_, rrclass, rrtype, ttl, data);
-
+            if (rdata) {
+                add_callback_(*last_name_, rrclass, rrtype,
+                              getCurrentTTL(explicit_ttl, rrtype, rdata),
+                              rdata);
                 // Good, we loaded another one
                 ++count;
             } else {

src/lib/dns/rdata/generic/soa_6.cc

@@ -16,6 +16,7 @@
 
 #include <string>
 
+#include <boost/static_assert.hpp>
 #include <boost/lexical_cast.hpp>
 
 #include <exceptions/exceptions.h>
@@ -112,6 +113,16 @@ SOA::getSerial() const {
     return (Serial(b.readUint32()));
 }
 
+uint32_t
+SOA::getMinimum() const {
+    // Make sure the buffer access is safe.
+    BOOST_STATIC_ASSERT(sizeof(numdata_) ==
+                        sizeof(uint32_t) * 4 + sizeof(uint32_t));
+
+    InputBuffer b(&numdata_[sizeof(uint32_t) * 4], sizeof(uint32_t));
+    return (b.readUint32());
+}
+
 string
 SOA::toText() const {
     InputBuffer b(numdata_, sizeof(numdata_));

src/lib/dns/rdata/generic/soa_6.h

@@ -35,8 +35,12 @@ public:
     SOA(const Name& mname, const Name& rname, uint32_t serial,
         uint32_t refresh, uint32_t retry, uint32_t expire,
         uint32_t minimum);
+
     /// \brief Returns the serial stored in the SOA.
     Serial getSerial() const;
+
+    /// brief Returns the minimum TTL field value of the SOA.
+    uint32_t getMinimum() const;
 private:
     /// Note: this is a prototype version; we may reconsider
     /// this representation later.

src/lib/dns/rrttl.cc

@@ -57,9 +57,14 @@ Unit units[] = {
 namespace isc {
 namespace dns {
 
-RRTTL::RRTTL(const std::string& ttlstr) {
+namespace {
+bool
+parseTTLStr(const string& ttlstr, uint32_t& ttlval, string* error_txt) {
     if (ttlstr.empty()) {
-        isc_throw(InvalidRRTTL, "Empty TTL string");
+        if (error_txt != NULL) {
+            *error_txt = "Empty TTL string";
+        }
+        return (false);
     }
     // We use a larger data type during the computation. This is because
     // some compilers don't fail when out of range, so we check the range
@@ -80,8 +85,10 @@ RRTTL::RRTTL(const std::string& ttlstr) {
             if (unit == end) {
                 if (units_mode) {
                     // We had some units before. The last one is missing unit.
-                    isc_throw(InvalidRRTTL, "Missing the last unit: " <<
-                              ttlstr);
+                    if (error_txt != NULL) {
+                        *error_txt = "Missing the last unit: " + ttlstr;
+                    }
+                    return (false);
                 } else {
                     // Case without any units at all. Just convert and store
                     // it.
@@ -102,12 +109,18 @@ RRTTL::RRTTL(const std::string& ttlstr) {
                 }
             }
             if (!found) {
-                isc_throw(InvalidRRTTL, "Unknown unit used: " << *unit <<
-                          " in: " << ttlstr);
+                if (error_txt != NULL) {
+                    *error_txt = "Unknown unit used: " +
+                        boost::lexical_cast<string>(*unit) + " in: " + ttlstr;
+                }
+                return (false);
             }
             // Now extract the number.
             if (unit == pos) {
-                isc_throw(InvalidRRTTL, "Missing number in TTL: " << ttlstr);
+                if (error_txt != NULL) {
+                    *error_txt = "Missing number in TTL: " + ttlstr;
+                }
+                return (false);
             }
             const int64_t value = boost::lexical_cast<int64_t>(string(pos,
                                                                       unit));
@@ -118,21 +131,52 @@ RRTTL::RRTTL(const std::string& ttlstr) {
             // there's no need to continue).
             if (value < 0 || value > 0xffffffff || val < 0 ||
                 val > 0xffffffff) {
-                isc_throw(InvalidRRTTL, "Part of TTL out of range: " <<
-                          ttlstr);
+                if (error_txt != NULL) {
+                    *error_txt = "Part of TTL out of range: "  + ttlstr;
+                }
+                return (false);
             }
             // Move to after the unit.
             pos = unit + 1;
         }
     } catch (const boost::bad_lexical_cast&) {
-        isc_throw(InvalidRRTTL, "invalid TTL: " << ttlstr);
+        if (error_txt != NULL) {
+            *error_txt = "invalid TTL: " + ttlstr;
+        }
+        return (false);
     }
 
     if (val >= 0 && val <= 0xffffffff) {
-        ttlval_ = val;
+        ttlval = val;
     } else {
-        isc_throw(InvalidRRTTL, "TTL out of range: " << ttlstr);
+        if (error_txt != NULL) {
+            *error_txt = "TTL out of range: " + ttlstr;
+        }
+        return (false);
+    }
+
+    return (true);
+}
+}
+
+RRTTL::RRTTL(const std::string& ttlstr) {
+    string error_txt;
+    if (!parseTTLStr(ttlstr, ttlval_, &error_txt)) {
+        isc_throw(InvalidRRTTL, error_txt);
+    }
+}
+
+RRTTL*
+RRTTL::createFromText(const string& ttlstr, RRTTL* placeholder) {
+    uint32_t ttlval;
+    if (parseTTLStr(ttlstr, ttlval, NULL)) {
+        if (placeholder != NULL) {
+            *placeholder = RRTTL(ttlval);
+            return (placeholder);
+        }
+        return (new RRTTL(ttlval));
     }
+    return (NULL);
 }
 
 RRTTL::RRTTL(InputBuffer& buffer) {

src/lib/dns/rrttl.h

@@ -61,7 +61,7 @@ public:
 class RRTTL {
 public:
     ///
-    /// \name Constructors and Destructor
+    /// \name Constructors, Factory and Destructor
     ///
     /// Note: We use the default copy constructor and the default copy
     /// assignment operator intentionally.
@@ -72,6 +72,7 @@ public:
     ///
     /// \param ttlval An 32-bit integer of the RRTTL.
     explicit RRTTL(uint32_t ttlval) : ttlval_(ttlval) {}
+
     /// Constructor from a string.
     ///
     /// It accepts either a decimal number, specifying number of seconds. Or,
@@ -87,6 +88,7 @@ public:
     /// \throw InvalidRRTTL in case the string is not recognized as valid
     ///     TTL representation.
     explicit RRTTL(const std::string& ttlstr);
+
     /// Constructor from wire-format data.
     ///
     /// The \c buffer parameter normally stores a complete DNS message
@@ -98,6 +100,35 @@ public:
     ///
     /// \param buffer A buffer storing the wire format data.
     explicit RRTTL(isc::util::InputBuffer& buffer);
+
+    /// A separate factory of RRTTL from text.
+    ///
+    /// This static method is similar to the constructor that takes a string
+    /// object, but works as a factory and reports parsing failure in return
+    /// value.  Normally the constructor version should suffice, but in some
+    /// cases the caller may have to expect mixture of valid and invalid input,
+    /// and may want to minimize the overhead of possible exception handling.
+    /// This version is provided for such purpose.
+    ///
+    /// When the \c placeholder parameter is NULL, it creates a new RRTTL
+    /// object, allocating memory for it; the caller is responsible for
+    /// releasing the memory using the \c delete operator.  If \c placeholder
+    /// is non NULL, it will override the placeholder object with an RRTTL
+    /// corresponding to the given text and return a pointer to the placeholder
+    /// object.  This way, the caller can also minimize the overhead of memory
+    /// allocation if it needs to call this method many times.
+    ///
+    /// If the given text does not represent a valid RRTTL, it returns NULL;
+    /// if \c placeholder is non NULL, it will be intact.
+    ///
+    /// This function never throws the \c InvalidRRTTL exception.
+    ///
+    /// \param ttlstr A string representation of the \c RRTTL.
+    /// \param placeholder If non NULL, an RRTTL object to be overridden
+    /// with an RRTTL for \c ttlstr.
+    /// \return A pointer to the created or overridden RRTTL object.
+    static RRTTL* createFromText(const std::string& ttlstr,
+                                 RRTTL* placeholder);
     ///
     //@}
 

src/lib/dns/tests/master_loader_unittest.cc

@@ -17,11 +17,14 @@
 #include <dns/rrtype.h>
 #include <dns/rrset.h>
 #include <dns/rrclass.h>
+#include <dns/rrttl.h>
 #include <dns/name.h>
 #include <dns/rdata.h>
 
 #include <gtest/gtest.h>
+
 #include <boost/bind.hpp>
+#include <boost/lexical_cast.hpp>
 #include <boost/scoped_ptr.hpp>
 
 #include <string>
@@ -35,6 +38,7 @@ using std::string;
 using std::list;
 using std::stringstream;
 using std::endl;
+using boost::lexical_cast;
 
 namespace {
 class MasterLoaderTest : public ::testing::Test {
@@ -107,7 +111,8 @@ public:
 
     // 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) {
+    void checkRR(const string& name, const RRType& type, const string& data,
+                 const RRTTL& rrttl = RRTTL(3600)) {
         ASSERT_FALSE(rrsets_.empty());
         RRsetPtr current = rrsets_.front();
         rrsets_.pop_front();
@@ -115,6 +120,7 @@ public:
         EXPECT_EQ(Name(name), current->getName());
         EXPECT_EQ(type, current->getType());
         EXPECT_EQ(RRClass::IN(), current->getClass());
+        EXPECT_EQ(rrttl, current->getTTL());
         ASSERT_EQ(1, current->getRdataCount());
         EXPECT_EQ(0, isc::dns::rdata::createRdata(type, RRClass::IN(), data)->
                   compare(current->getRdataIterator()->getCurrent()));
@@ -318,38 +324,60 @@ TEST_F(MasterLoaderTest, invalidFile) {
 
 struct ErrorCase {
     const char* const line;    // The broken line in master file
+    const char* const reason;  // If non NULL, the reason string
     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" },
-    { ")www     3600    IN  A   192.0.2.1", "Token error 3" },
+    { "www...   3600    IN  A   192.0.2.1", NULL, "Invalid name" },
+    { "www      FORTNIGHT   IN  A   192.0.2.1", NULL, "Invalid TTL" },
+    { "www      3600    XX  A   192.0.2.1", NULL, "Invalid class" },
+    { "www      3600    IN  A   bad_ip", NULL, "Invalid Rdata" },
+    { "www      3600    IN", NULL, "Unexpected EOLN" },
+    { "www      3600    CH  TXT nothing", NULL, "Class mismatch" },
+    { "www      \"3600\"  IN  A   192.0.2.1", NULL, "Quoted TTL" },
+    { "www      3600    \"IN\"  A   192.0.2.1", NULL, "Quoted class" },
+    { "www      3600    IN  \"A\"   192.0.2.1", NULL, "Quoted type" },
+    { "unbalanced)paren 3600    IN  A   192.0.2.1", NULL, "Token error 1" },
+    { "www  3600    unbalanced)paren    A   192.0.2.1", NULL,
+      "Token error 2" },
     // Check the unknown directive. The rest looks like ordinary RR,
     // so we see the $ is actually special.
-    { "$UNKNOWN 3600    IN  A   192.0.2.1", "Unknown $ directive" },
-    { "$INCLUD " TEST_DATA_SRCDIR "/example.org", "Include too short" },
-    { "$INCLUDES " TEST_DATA_SRCDIR "/example.org", "Include too long" },
-    { "$INCLUDE", "Missing include path" },
-    { "$INCLUDE /file/not/found", "Include file not found" },
+    { "$UNKNOWN 3600    IN  A   192.0.2.1", NULL, "Unknown $ directive" },
+    { "$INCLUD " TEST_DATA_SRCDIR "/example.org", NULL, "Include too short" },
+    { "$INCLUDES " TEST_DATA_SRCDIR "/example.org", NULL, "Include too long" },
+    { "$INCLUDE", NULL, "Missing include path" },
+    { "$INCLUDE /file/not/found", NULL, "Include file not found" },
     { "$INCLUDE /file/not/found example.org. and here goes bunch of garbage",
-        "Include file not found and garbage at the end of line" },
-    { "$ORIGIN", "Missing origin name" },
-    { "$ORIGIN invalid...name", "Invalid name for origin" },
-    { "$ORIGIN )brokentoken", "Broken token in origin" },
-    { "$ORIGIN example.org. garbage", "Garbage after origin" },
-    { "$ORIGI name.", "$ORIGIN too short" },
-    { "$ORIGINAL name.", "$ORIGIN too long" },
-    { NULL, NULL }
+        NULL, "Include file not found and garbage at the end of line" },
+    { "$ORIGIN", NULL, "Missing origin name" },
+    { "$ORIGIN invalid...name", NULL, "Invalid name for origin" },
+    { "$ORIGIN )brokentoken", NULL, "Broken token in origin" },
+    { "$ORIGIN example.org. garbage", NULL, "Garbage after origin" },
+    { "$ORIGI name.", NULL, "$ORIGIN too short" },
+    { "$ORIGINAL name.", NULL, "$ORIGIN too long" },
+    { "$TTL 100 extra-garbage", "Extra tokens at the end of line",
+      "$TTL with extra token" },
+    { "$TTL", "unexpected end of input", "missing TTL" },
+    { "$TTL No-ttl", "Unknown unit used: N in: No-ttl", "bad TTL" },
+    { "$TTL \"100\"", "invalid TTL: \"100\"", "bad TTL, quoted" },
+    { "$TT 100", "Unknown directive 'TT'", "bad directive, too short" },
+    { "$TTLLIKE 100", "Unknown directive 'TTLLIKE'", "bad directive, extra" },
+    { NULL, NULL, NULL }
 };
 
+// A commonly used helper to check callback message.
+void
+checkCallbackMessage(const string& actual_msg, const string& expected_msg,
+                     size_t expected_line) {
+    // The actual message should begin with the expected message.
+    EXPECT_EQ(0, actual_msg.find(expected_msg)) << "actual message: " <<
+                                                actual_msg << " expected: " <<
+                                                expected_msg;
+
+    // and it should end with "...:<line_num>]"
+    const string line_desc = ":" + lexical_cast<string>(expected_line) + "]";
+    EXPECT_EQ(actual_msg.size() - line_desc.size(), actual_msg.find(line_desc));
+}
+
 // Test a broken zone is handled properly. We test several problems,
 // both in strict and lenient mode.
 TEST_F(MasterLoaderTest, brokenZone) {
@@ -367,6 +395,9 @@ TEST_F(MasterLoaderTest, brokenZone) {
             EXPECT_THROW(loader_->load(), MasterLoaderError);
             EXPECT_FALSE(loader_->loadedSucessfully());
             EXPECT_EQ(1, errors_.size());
+            if (ec->reason != NULL) {
+                checkCallbackMessage(errors_.at(0), ec->reason, 2);
+            }
             EXPECT_TRUE(warnings_.empty());
 
             checkRR("example.org", RRType::SOA(), "ns1.example.org. "
@@ -437,7 +468,8 @@ TEST_F(MasterLoaderTest, includeWithGarbage) {
     checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
 }
 
-// Check we error about garbage at the end of $ORIGIN line (but the line works).
+// Check we error about garbage at the end of $ORIGIN line (but the line
+// works).
 TEST_F(MasterLoaderTest, originWithGarbage) {
     const string origin_str = "$ORIGIN www More garbage here\n"
         "@  1H  IN  A   192.0.2.1\n";
@@ -531,6 +563,159 @@ TEST_F(MasterLoaderTest, includeAndInitialWS) {
     checkARR("xyz.example.org");
 }
 
+// Test for "$TTL"
+TEST_F(MasterLoaderTest, ttlDirective) {
+    stringstream zone_stream;
+
+    // Set the default TTL with $TTL followed by an RR omitting the TTL
+    zone_stream << "$TTL 1800\nexample.org. IN A 192.0.2.1\n";
+    // $TTL can be quoted.  Also testing the case of $TTL being changed.
+    zone_stream << "\"$TTL\" 100\na.example.org. IN A 192.0.2.2\n";
+    // Extended TTL form is accepted.
+    zone_stream << "$TTL 1H\nb.example.org. IN A 192.0.2.3\n";
+    // Matching is case insensitive.
+    zone_stream << "$tTl 360\nc.example.org. IN A 192.0.2.4\n";
+    // Maximum allowable TTL
+    zone_stream << "$TTL 2147483647\nd.example.org. IN A 192.0.2.5\n";
+
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::DEFAULT);
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    checkRR("example.org", RRType::A(), "192.0.2.1", RRTTL(1800));
+    checkRR("a.example.org", RRType::A(), "192.0.2.2", RRTTL(100));
+    checkRR("b.example.org", RRType::A(), "192.0.2.3", RRTTL(3600));
+    checkRR("c.example.org", RRType::A(), "192.0.2.4", RRTTL(360));
+    checkRR("d.example.org", RRType::A(), "192.0.2.5", RRTTL(2147483647));
+}
+
+TEST_F(MasterLoaderTest, ttlFromSOA) {
+    // No $TTL, and the SOA doesn't have an explicit TTL field.  Its minimum
+    // TTL field will be used as the RR's TTL, and it'll be used as the
+    // default TTL for others.
+    stringstream zone_stream("example.org. IN SOA . . 0 0 0 0 1800\n"
+                             "a.example.org. IN A 192.0.2.1\n");
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::DEFAULT);
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    checkRR("example.org", RRType::SOA(), ". . 0 0 0 0 1800", RRTTL(1800));
+    checkRR("a.example.org", RRType::A(), "192.0.2.1", RRTTL(1800));
+
+    // The use of SOA minimum TTL should have caused a warning.
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0),
+                         "no TTL specified; using SOA MINTTL instead", 1);
+}
+
+TEST_F(MasterLoaderTest, ttlFromPrevious) {
+    // No available default TTL.  2nd and 3rd RR will use the TTL of the
+    // 1st RR.  This will result in a warning, but only for the first time.
+    stringstream zone_stream("a.example.org. 1800 IN A 192.0.2.1\n"
+                             "b.example.org. IN A 192.0.2.2\n"
+                             "c.example.org. IN A 192.0.2.3\n");
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::DEFAULT);
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    checkRR("a.example.org", RRType::A(), "192.0.2.1", RRTTL(1800));
+    checkRR("b.example.org", RRType::A(), "192.0.2.2", RRTTL(1800));
+    checkRR("c.example.org", RRType::A(), "192.0.2.3", RRTTL(1800));
+
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0), "using RFC1035 TTL semantics", 2);
+}
+
+TEST_F(MasterLoaderTest, ttlFromPreviousSOA) {
+    // Mixture of the previous two cases: SOA has explicit TTL, followed by
+    // an RR without an explicit TTL.  In this case the minimum TTL won't be
+    // recognized as the "default TTL".
+    stringstream zone_stream("example.org. 100 IN SOA . . 0 0 0 0 1800\n"
+                             "a.example.org. IN A 192.0.2.1\n");
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::DEFAULT);
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+
+    checkRR("example.org", RRType::SOA(), ". . 0 0 0 0 1800", RRTTL(100));
+    checkRR("a.example.org", RRType::A(), "192.0.2.1", RRTTL(100));
+
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0), "using RFC1035 TTL semantics", 2);
+}
+
+TEST_F(MasterLoaderTest, ttlUnknown) {
+    // No available TTL is known for the first RR.
+    stringstream zone_stream("a.example.org. IN A 192.0.2.1\n");
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::DEFAULT);
+    EXPECT_THROW(loader_->load(), MasterLoaderError);
+}
+
+TEST_F(MasterLoaderTest, ttlUnknownAndContinue) {
+    stringstream zone_stream("a.example.org. IN A 192.0.2.1\n"
+                             "b.example.org. 1800 IN A 192.0.2.2\n");
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    checkRR("b.example.org", RRType::A(), "192.0.2.2", RRTTL(1800));
+
+    EXPECT_TRUE(warnings_.empty());
+    EXPECT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "no TTL specified; load rejected", 1);
+}
+
+TEST_F(MasterLoaderTest, ttlUnknownAndEOF) {
+    // Similar to the previous case, but the input will be abruptly terminated
+    // after the offending RR.  This will cause an additional warning.
+    stringstream zone_stream("a.example.org. IN A 192.0.2.1");
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    EXPECT_TRUE(rrsets_.empty());
+
+    EXPECT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "no TTL specified; load rejected", 1);
+
+    // RDATA implementation can complain about it, too.  To be independent of
+    // its details, we focus on the very last warning.
+    EXPECT_FALSE(warnings_.empty());
+    checkCallbackMessage(*warnings_.rbegin(), "File does not end with newline",
+                         1);
+}
+
+TEST_F(MasterLoaderTest, ttlOverflow) {
+    stringstream zone_stream;
+    zone_stream << "example.org. IN SOA . . 0 0 0 0 2147483648\n";
+    zone_stream << "$TTL 3600\n"; // reset to an in-range value
+    zone_stream << "$TTL 2147483649\n" << "a.example.org. IN A 192.0.2.1\n";
+    zone_stream << "$TTL 3600\n"; // reset to an in-range value
+    zone_stream << "b.example.org. 2147483650 IN A 192.0.2.2\n";
+    setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+              MasterLoader::DEFAULT);
+
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_EQ(3, rrsets_.size());
+
+    checkRR("example.org", RRType::SOA(), ". . 0 0 0 0 2147483648", RRTTL(0));
+    checkRR("a.example.org", RRType::A(), "192.0.2.1", RRTTL(0));
+    checkRR("b.example.org", RRType::A(), "192.0.2.2", RRTTL(0));
+
+    EXPECT_EQ(4, warnings_.size());
+    checkCallbackMessage(warnings_.at(1),
+                         "TTL 2147483648 > MAXTTL, setting to 0 per RFC2181",
+                         1);
+    checkCallbackMessage(warnings_.at(2),
+                         "TTL 2147483649 > MAXTTL, setting to 0 per RFC2181",
+                         3);
+    checkCallbackMessage(warnings_.at(3),
+                         "TTL 2147483650 > MAXTTL, setting to 0 per RFC2181",
+                         6);
+}
+
 // Test the constructor rejects empty add callback.
 TEST_F(MasterLoaderTest, emptyCallback) {
     EXPECT_THROW(MasterLoader(TEST_DATA_SRCDIR "/example.org",

src/lib/dns/tests/rdata_soa_unittest.cc

@@ -32,12 +32,14 @@ using namespace isc::dns::rdata;
 
 namespace {
 class Rdata_SOA_Test : public RdataTest {
-    // there's nothing to specialize
+protected:
+    Rdata_SOA_Test() : rdata_soa(Name("ns.example.com"),
+                                 Name("root.example.com"),
+                                 2010012601, 3600, 300, 3600000, 1200)
+    {}
+    const generic::SOA rdata_soa;
 };
 
-const generic::SOA rdata_soa(Name("ns.example.com"), Name("root.example.com"),
-                             2010012601, 3600, 300, 3600000, 1200);
-
 TEST_F(Rdata_SOA_Test, createFromText) {
     //TBD
 }
@@ -86,4 +88,13 @@ TEST_F(Rdata_SOA_Test, getSerial) {
     EXPECT_EQ(2010012601, rdata_soa.getSerial().getValue());
 }
 
+TEST_F(Rdata_SOA_Test, getMinimum) {
+    EXPECT_EQ(1200, rdata_soa.getMinimum());
+
+    // Also check with a very large number (with the MSB being 1).
+    EXPECT_EQ(2154848336u, generic::SOA(Name("ns.example.com"),
+                                        Name("root.example.com"),
+                                        0, 0, 0, 0, 0x80706050).getMinimum());
+}
+
 }

src/lib/dns/tests/rrttl_unittest.cc

@@ -20,6 +20,8 @@
 
 #include <dns/tests/unittest_util.h>
 
+#include <boost/scoped_ptr.hpp>
+
 using namespace std;
 using namespace isc;
 using namespace isc::dns;
@@ -85,6 +87,25 @@ TEST_F(RRTTLTest, fromText) {
     EXPECT_THROW(RRTTL("4294967296"), InvalidRRTTL); // must be 32-bit
 }
 
+TEST_F(RRTTLTest, createFromText) {
+    // If placeholder is NULL, a new RRTTL object is allocated
+    boost::scoped_ptr<RRTTL> ttl_ptr;
+    ttl_ptr.reset(RRTTL::createFromText("3600", NULL));
+    ASSERT_TRUE(ttl_ptr);
+    EXPECT_EQ(RRTTL(3600), *ttl_ptr);
+
+    // If placeholder is non NULL, it will be overwritten
+    RRTTL ttl(3600);
+    EXPECT_NE(static_cast<RRTTL*>(NULL), RRTTL::createFromText("1800", &ttl));
+    EXPECT_EQ(RRTTL(1800), ttl);
+
+    // If text parsing fails, NULL is returned; if placeholder is given,
+    // it will be intact.
+    EXPECT_EQ(static_cast<RRTTL*>(NULL), RRTTL::createFromText("bad", NULL));
+    EXPECT_EQ(static_cast<RRTTL*>(NULL), RRTTL::createFromText("bad", &ttl));
+    EXPECT_EQ(RRTTL(1800), ttl);
+}
+
 void
 checkUnit(unsigned multiply, char suffix) {
     SCOPED_TRACE(string("Unit check with suffix ") + suffix);