[master] Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10

src/lib/dhcp/iface_mgr.cc

@@ -212,8 +212,8 @@ bool IfaceMgr::openSockets4(const uint16_t port) {
              addr != addrs.end();
              ++addr) {
 
-            // Skip IPv6 addresses
-            if (addr->getFamily() != AF_INET) {
+            // Skip all but V4 addresses.
+            if (!addr->isV4()) {
                 continue;
             }
 
@@ -247,8 +247,8 @@ bool IfaceMgr::openSockets6(const uint16_t port) {
              addr != addrs.end();
              ++addr) {
 
-            // skip IPv4 addresses
-            if (addr->getFamily() != AF_INET6) {
+            // Skip all but V6 addresses.
+            if (!addr->isV6()) {
                 continue;
             }
 
@@ -356,12 +356,13 @@ int IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr,
     if (!iface) {
         isc_throw(BadValue, "There is no " << ifname << " interface present.");
     }
-    switch (addr.getFamily()) {
-    case AF_INET:
+    if (addr.isV4()) {
         return openSocket4(*iface, addr, port);
-    case AF_INET6:
+
+    } else if (addr.isV6()) {
         return openSocket6(*iface, addr, port);
-    default:
+
+    } else {
         isc_throw(BadValue, "Failed to detect family of address: "
                   << addr.toText());
     }
@@ -469,7 +470,7 @@ IfaceMgr::getLocalAddress(const IOAddress& remote_addr, const uint16_t port) {
     asio::error_code err_code;
     // If remote address is broadcast address we have to
     // allow this on the socket.
-    if (remote_addr.getAddress().is_v4() &&
+    if (remote_addr.isV4() &&
         (remote_addr == IOAddress(DHCP_IPV4_BROADCAST_ADDRESS))) {
         // Socket has to be open prior to setting the broadcast
         // option. Otherwise set_option will complain about
@@ -556,9 +557,7 @@ int IfaceMgr::openSocket6(Iface& iface, const IOAddress& addr, uint16_t port) {
         addr6.sin6_scope_id = if_nametoindex(iface.getName().c_str());
     }
 
-    memcpy(&addr6.sin6_addr,
-           addr.getAddress().to_v6().to_bytes().data(),
-           sizeof(addr6.sin6_addr));
+    memcpy(&addr6.sin6_addr, &addr.toBytes()[0], sizeof(addr6.sin6_addr));
 #ifdef HAVE_SA_LEN
     addr6.sin6_len = sizeof(addr6);
 #endif
@@ -660,7 +659,7 @@ IfaceMgr::send(const Pkt6Ptr& pkt) {
     to.sin6_family = AF_INET6;
     to.sin6_port = htons(pkt->getRemotePort());
     memcpy(&to.sin6_addr,
-           pkt->getRemoteAddr().getAddress().to_v6().to_bytes().data(),
+           &pkt->getRemoteAddr().toBytes()[0],
            16);
     to.sin6_scope_id = pkt->getIndex();
 
@@ -798,7 +797,7 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
              s != socket_collection.end(); ++s) {
 
             // Only deal with IPv4 addresses.
-            if (s->addr_.getFamily() == AF_INET) {
+            if (s->addr_.isV4()) {
                 names << s->sockfd_ << "(" << iface->getName() << ") ";
 
                 // Add this socket to listening set
@@ -950,8 +949,8 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
         for (SocketCollection::const_iterator s = socket_collection.begin();
              s != socket_collection.end(); ++s) {
 
-            // Only deal with IPv4 addresses.
-            if (s->addr_.getFamily() == AF_INET6) {
+            // Only deal with IPv6 addresses.
+            if (s->addr_.isV6()) {
                 names << s->sockfd_ << "(" << iface->getName() << ") ";
 
                 // Add this socket to listening set

src/lib/dhcp/option4_addrlst.cc

@@ -86,7 +86,7 @@ Option4AddrLst::pack4(isc::util::OutputBuffer& buf) {
 }
 
 void Option4AddrLst::setAddress(const isc::asiolink::IOAddress& addr) {
-    if (addr.getFamily() != AF_INET) {
+    if (!addr.isV4()) {
         isc_throw(BadValue, "Can't store non-IPv4 address in "
                   << "Option4AddrLst option");
     }
@@ -107,7 +107,7 @@ void Option4AddrLst::setAddresses(const AddressContainer& addrs) {
 
 
 void Option4AddrLst::addAddress(const isc::asiolink::IOAddress& addr) {
-    if (addr.getFamily() != AF_INET) {
+    if (!addr.isV4()) {
         isc_throw(BadValue, "Can't store non-IPv4 address in "
                   << "Option4AddrLst option");
     }

src/lib/dhcp/option6_addrlst.cc

@@ -49,7 +49,7 @@ Option6AddrLst::Option6AddrLst(uint16_t type, OptionBufferConstIter begin,
 
 void
 Option6AddrLst::setAddress(const isc::asiolink::IOAddress& addr) {
-    if (addr.getFamily() != AF_INET6) {
+    if (!addr.isV6()) {
         isc_throw(BadValue, "Can't store non-IPv6 address in Option6AddrLst option");
     }
 
@@ -72,7 +72,13 @@ void Option6AddrLst::pack(isc::util::OutputBuffer& buf) {
 
     for (AddressContainer::const_iterator addr=addrs_.begin();
          addr!=addrs_.end(); ++addr) {
-        buf.writeData(addr->getAddress().to_v6().to_bytes().data(), V6ADDRESS_LEN);
+        if (!addr->isV6()) {
+            isc_throw(isc::BadValue, addr->toText()
+                      << " is not an IPv6 address");
+        }
+        // If an address is IPv6 address it should have assumed
+        // length of V6ADDRESS_LEN.
+        buf.writeData(&addr->toBytes()[0], V6ADDRESS_LEN);
     }
 }
 
@@ -104,8 +110,7 @@ std::string Option6AddrLst::toText(int indent /* =0 */) {
 }
 
 uint16_t Option6AddrLst::len() {
-
-    return (OPTION6_HDR_LEN + addrs_.size()*V6ADDRESS_LEN);
+    return (OPTION6_HDR_LEN + addrs_.size() * V6ADDRESS_LEN);
 }
 
 } // end of namespace isc::dhcp

src/lib/dhcp/option6_iaaddr.cc

@@ -51,9 +51,11 @@ void Option6IAAddr::pack(isc::util::OutputBuffer& buf) {
     // length without 4-byte option header
     buf.writeUint16(len() - getHeaderLen());
 
-
-    buf.writeData(addr_.getAddress().to_v6().to_bytes().data(),
-                  isc::asiolink::V6ADDRESS_LEN);
+    if (!addr_.isV6()) {
+        isc_throw(isc::BadValue, addr_.toText()
+                  << " is not an IPv6 address");
+    }
+    buf.writeData(&addr_.toBytes()[0], isc::asiolink::V6ADDRESS_LEN);
 
     buf.writeUint32(preferred_);
     buf.writeUint32(valid_);

src/lib/dhcp/option_custom.cc

@@ -58,14 +58,12 @@ void
 OptionCustom::addArrayDataField(const asiolink::IOAddress& address) {
     checkArrayType();
 
-    if ((address.getFamily() == AF_INET &&
-         definition_.getType() != OPT_IPV4_ADDRESS_TYPE) ||
-        (address.getFamily() == AF_INET6 &&
-         definition_.getType() != OPT_IPV6_ADDRESS_TYPE)) {
+    if ((address.isV4() && definition_.getType() != OPT_IPV4_ADDRESS_TYPE) ||
+        (address.isV6() && definition_.getType() != OPT_IPV6_ADDRESS_TYPE)) {
         isc_throw(BadDataTypeCast, "invalid address specified "
                   << address.toText() << ". Expected a valid IPv"
-                  << (definition_.getType() == OPT_IPV4_ADDRESS_TYPE ? "4" : "6")
-                  << " address.");
+                  << (definition_.getType() == OPT_IPV4_ADDRESS_TYPE ?
+                      "4" : "6") << " address.");
     }
 
     OptionBuffer buf;
@@ -454,10 +452,8 @@ OptionCustom::writeAddress(const asiolink::IOAddress& address,
 
     checkIndex(index);
 
-    if ((address.getFamily() == AF_INET &&
-         buffers_[index].size() != V4ADDRESS_LEN) ||
-        (address.getFamily() == AF_INET6 &&
-         buffers_[index].size() != V6ADDRESS_LEN)) {
+    if ((address.isV4() && buffers_[index].size() != V4ADDRESS_LEN) ||
+        (address.isV6() && buffers_[index].size() != V6ADDRESS_LEN)) {
         isc_throw(BadDataTypeCast, "invalid address specified "
                   << address.toText() << ". Expected a valid IPv"
                   << (buffers_[index].size() == V4ADDRESS_LEN ? "4" : "6")

src/lib/dhcp/option_data_types.cc

@@ -146,27 +146,8 @@ OptionDataTypeUtil::readAddress(const std::vector<uint8_t>& buf,
 void
 OptionDataTypeUtil::writeAddress(const asiolink::IOAddress& address,
                                  std::vector<uint8_t>& buf) {
-    // @todo There is a ticket 2396 submitted, which adds the
-    // functionality to return a buffer representation of
-    // IOAddress. If so, this function can be simplified.
-    if (address.getAddress().is_v4()) {
-        asio::ip::address_v4::bytes_type addr_bytes =
-            address.getAddress().to_v4().to_bytes();
-        // Increase the buffer size by the size of IPv4 address.
-        buf.resize(buf.size() + addr_bytes.size());
-        std::copy_backward(addr_bytes.begin(), addr_bytes.end(),
-                           buf.end());
-    } else if (address.getAddress().is_v6()) {
-        asio::ip::address_v6::bytes_type addr_bytes =
-            address.getAddress().to_v6().to_bytes();
-        // Incresase the buffer size by the size of IPv6 address.
-        buf.resize(buf.size() + addr_bytes.size());
-        std::copy_backward(addr_bytes.begin(), addr_bytes.end(),
-                           buf.end());
-    } else {
-        isc_throw(BadDataTypeCast, "the address " << address.toText()
-                  << " is neither valid IPv4 not IPv6 address.");
-    }
+    const std::vector<uint8_t>& vec = address.toBytes();
+    buf.insert(buf.end(), vec.begin(), vec.end());
 }
 
 void

src/lib/dhcp/option_definition.cc

@@ -379,12 +379,9 @@ OptionDefinition::writeToBuffer(const std::string& value,
     case OPT_IPV6_ADDRESS_TYPE:
         {
             asiolink::IOAddress address(value);
-            if (address.getFamily() != AF_INET &&
-                address.getFamily() != AF_INET6) {
+            if (!address.isV4() && !address.isV6()) {
                 isc_throw(BadDataTypeCast, "provided address " << address.toText()
-                          << " is not a valid "
-                          << (address.getAddress().is_v4() ? "IPv4" : "IPv6")
-                          << " address");
+                          << " is not a valid IPv4 or IPv6 address.");
             }
             OptionDataTypeUtil::writeAddress(address, buf);
             return;

src/lib/dhcp/tests/option_custom_unittest.cc

@@ -38,16 +38,8 @@ public:
     /// @param [out] buf output buffer.
     void writeAddress(const asiolink::IOAddress& address,
                       std::vector<uint8_t>& buf) {
-        short family = address.getFamily();
-        if (family == AF_INET) {
-            asio::ip::address_v4::bytes_type buf_addr =
-                address.getAddress().to_v4().to_bytes();
-            buf.insert(buf.end(), buf_addr.begin(), buf_addr.end());
-        } else if (family == AF_INET6) {
-            asio::ip::address_v6::bytes_type buf_addr =
-                address.getAddress().to_v6().to_bytes();
-            buf.insert(buf.end(), buf_addr.begin(), buf_addr.end());
-        }
+        const std::vector<uint8_t>& vec = address.toBytes();
+        buf.insert(buf.end(), vec.begin(), vec.end());
     }
 
     /// @brief Write integer (signed or unsigned) into a buffer.

src/lib/dhcp/tests/option_data_types_unittest.cc

@@ -34,16 +34,8 @@ public:
     /// @param [out] buf output buffer.
     void writeAddress(const asiolink::IOAddress& address,
                       std::vector<uint8_t>& buf) {
-        short family = address.getFamily();
-        if (family == AF_INET) {
-            asio::ip::address_v4::bytes_type buf_addr =
-                address.getAddress().to_v4().to_bytes();
-            buf.insert(buf.end(), buf_addr.begin(), buf_addr.end());
-        } else if (family == AF_INET6) {
-            asio::ip::address_v6::bytes_type buf_addr =
-                address.getAddress().to_v6().to_bytes();
-            buf.insert(buf.end(), buf_addr.begin(), buf_addr.end());
-        }
+        const std::vector<uint8_t>& vec = address.toBytes();
+        buf.insert(buf.end(), vec.begin(), vec.end());
     }
 
     /// @brief Write integer (signed or unsigned) into a buffer.

src/lib/dhcp/tests/option_definition_unittest.cc

@@ -207,10 +207,9 @@ TEST_F(OptionDefinitionTest, ipv6AddressArray) {
     // Write addresses to the buffer.
     OptionBuffer buf(addrs.size() * asiolink::V6ADDRESS_LEN);
     for (int i = 0; i < addrs.size(); ++i) {
-        asio::ip::address_v6::bytes_type addr_bytes =
-            addrs[i].getAddress().to_v6().to_bytes();
-        ASSERT_EQ(asiolink::V6ADDRESS_LEN, addr_bytes.size());
-        std::copy(addr_bytes.begin(), addr_bytes.end(),
+        const std::vector<uint8_t>& vec = addrs[i].toBytes();
+        ASSERT_EQ(asiolink::V6ADDRESS_LEN, vec.size());
+        std::copy(vec.begin(), vec.end(),
                   buf.begin() + i * asiolink::V6ADDRESS_LEN);
     }
     // Create DHCPv6 option from this buffer. Once option is created it is
@@ -306,10 +305,9 @@ TEST_F(OptionDefinitionTest, ipv4AddressArray) {
     // Write addresses to the buffer.
     OptionBuffer buf(addrs.size() * asiolink::V4ADDRESS_LEN);
     for (int i = 0; i < addrs.size(); ++i) {
-        asio::ip::address_v4::bytes_type addr_bytes =
-            addrs[i].getAddress().to_v4().to_bytes();
-        ASSERT_EQ(asiolink::V4ADDRESS_LEN, addr_bytes.size());
-        std::copy(addr_bytes.begin(), addr_bytes.end(),
+        const std::vector<uint8_t> vec = addrs[i].toBytes();
+        ASSERT_EQ(asiolink::V4ADDRESS_LEN, vec.size());
+        std::copy(vec.begin(), vec.end(),
                   buf.begin() + i * asiolink::V4ADDRESS_LEN);
     }
     // Create DHCPv6 option from this buffer. Once option is created it is
@@ -512,11 +510,10 @@ TEST_F(OptionDefinitionTest, recordIAAddr6) {
     OptionPtr option_v6;
     asiolink::IOAddress addr_v6("2001:0db8::ff00:0042:8329");
     OptionBuffer buf(asiolink::V6ADDRESS_LEN);
-    ASSERT_TRUE(addr_v6.getAddress().is_v6());
-    asio::ip::address_v6::bytes_type addr_bytes =
-        addr_v6.getAddress().to_v6().to_bytes();
-    ASSERT_EQ(asiolink::V6ADDRESS_LEN, addr_bytes.size());
-    std::copy(addr_bytes.begin(), addr_bytes.end(), buf.begin());
+    ASSERT_TRUE(addr_v6.isV6());
+    const std::vector<uint8_t>& vec = addr_v6.toBytes();
+    ASSERT_EQ(asiolink::V6ADDRESS_LEN, vec.size());
+    std::copy(vec.begin(), vec.end(), buf.begin());
 
     for (int i = 0; i < option6_iaaddr_len - asiolink::V6ADDRESS_LEN; ++i) {
         buf.push_back(i);

src/lib/dhcpsrv/addr_utilities.cc

@@ -53,8 +53,11 @@ isc::asiolink::IOAddress firstAddrInPrefix6(const isc::asiolink::IOAddress& pref
     }
 
     // First we copy the whole address as 16 bytes.
+    // We don't check that it is a valid IPv6 address and thus has
+    // the required length because it is already checked by
+    // the calling function.
     uint8_t packed[V6ADDRESS_LEN];
-    memcpy(packed, prefix.getAddress().to_v6().to_bytes().data(), 16);
+    memcpy(packed, &prefix.toBytes()[0], V6ADDRESS_LEN);
 
     // If the length is divisible by 8, it is simple. We just zero out the host
     // part. Otherwise we need to handle the byte that has to be partially
@@ -95,6 +98,9 @@ isc::asiolink::IOAddress firstAddrInPrefix4(const isc::asiolink::IOAddress& pref
         isc_throw(isc::BadValue, "Too large netmask. 0..32 is allowed in IPv4");
     }
 
+    // We don't check that it is a valid IPv4 address and thus has
+    // a required length of 4 bytes because it has been already
+    // checked by the calling function.
     uint32_t addr = prefix;
     return (IOAddress(addr & (~bitMask4[len])));
 }
@@ -132,7 +138,7 @@ isc::asiolink::IOAddress lastAddrInPrefix6(const isc::asiolink::IOAddress& prefi
 
     // First we copy the whole address as 16 bytes.
     uint8_t packed[V6ADDRESS_LEN];
-    memcpy(packed, prefix.getAddress().to_v6().to_bytes().data(), 16);
+    memcpy(packed, &prefix.toBytes()[0], 16);
 
     // if the length is divisible by 8, it is simple. We just fill the host part
     // with ones. Otherwise we need to handle the byte that has to be partially
@@ -168,20 +174,24 @@ namespace isc {
 namespace dhcp {
 
 isc::asiolink::IOAddress firstAddrInPrefix(const isc::asiolink::IOAddress& prefix,
-                                            uint8_t len) {
-    if (prefix.getFamily() == AF_INET) {
-        return firstAddrInPrefix4(prefix, len);
+                                           uint8_t len) {
+    if (prefix.isV4()) {
+        return (firstAddrInPrefix4(prefix, len));
+
     } else {
-        return firstAddrInPrefix6(prefix, len);
+        return (firstAddrInPrefix6(prefix, len));
+
     }
 }
 
 isc::asiolink::IOAddress lastAddrInPrefix(const isc::asiolink::IOAddress& prefix,
                                            uint8_t len) {
-    if (prefix.getFamily() == AF_INET) {
-        return lastAddrInPrefix4(prefix, len);
+    if (prefix.isV4()) {
+        return (lastAddrInPrefix4(prefix, len));
+
     } else {
-        return lastAddrInPrefix6(prefix, len);
+        return (lastAddrInPrefix6(prefix, len));
+
     }
 }
 

src/lib/dhcpsrv/alloc_engine.cc

@@ -30,20 +30,21 @@ AllocEngine::IterativeAllocator::IterativeAllocator()
 
 isc::asiolink::IOAddress
 AllocEngine::IterativeAllocator::increaseAddress(const isc::asiolink::IOAddress& addr) {
+    // Get a buffer holding an address.
+    const std::vector<uint8_t>& vec = addr.toBytes();
+    // Get the address length.
+    const int len = vec.size();
+
+    // Since the same array will be used to hold the IPv4 and IPv6
+    // address we have to make sure that the size of the array
+    // we allocate will work for both types of address.
+    BOOST_STATIC_ASSERT(V4ADDRESS_LEN <= V6ADDRESS_LEN);
     uint8_t packed[V6ADDRESS_LEN];
-    int len;
 
-    // First we copy the whole address as 16 bytes.
-    if (addr.getFamily()==AF_INET) {
-        // IPv4
-        std::memcpy(packed, addr.getAddress().to_v4().to_bytes().data(), 4);
-        len = 4;
-    } else {
-        // IPv6
-        std::memcpy(packed, addr.getAddress().to_v6().to_bytes().data(), 16);
-        len = 16;
-    }
+    // Copy the address. It can be either V4 or V6.
+    std::memcpy(packed, &vec[0], len);
 
+    // Increase the address.
     for (int i = len - 1; i >= 0; --i) {
         ++packed[i];
         if (packed[i] != 0) {

src/lib/dhcpsrv/pool.cc

@@ -34,7 +34,7 @@ Pool4::Pool4(const isc::asiolink::IOAddress& first,
              const isc::asiolink::IOAddress& last)
     :Pool(first, last) {
     // check if specified address boundaries are sane
-    if (first.getFamily() != AF_INET || last.getFamily() != AF_INET) {
+    if (!first.isV4() || !last.isV4()) {
         isc_throw(BadValue, "Invalid Pool4 address boundaries: not IPv4");
     }
 
@@ -48,7 +48,7 @@ Pool4::Pool4(const isc::asiolink::IOAddress& prefix,
     :Pool(prefix, IOAddress("0.0.0.0")) {
 
     // check if the prefix is sane
-    if (prefix.getFamily() != AF_INET) {
+    if (!prefix.isV4()) {
         isc_throw(BadValue, "Invalid Pool4 address boundaries: not IPv4");
     }
 
@@ -67,7 +67,7 @@ Pool6::Pool6(Pool6Type type, const isc::asiolink::IOAddress& first,
     :Pool(first, last), type_(type), prefix_len_(0) {
 
     // check if specified address boundaries are sane
-    if (first.getFamily() != AF_INET6 || last.getFamily() != AF_INET6) {
+    if (!first.isV6() || !last.isV6()) {
         isc_throw(BadValue, "Invalid Pool6 address boundaries: not IPv6");
     }
 
@@ -98,7 +98,7 @@ Pool6::Pool6(Pool6Type type, const isc::asiolink::IOAddress& prefix,
      type_(type), prefix_len_(prefix_len) {
 
     // check if the prefix is sane
-    if (prefix.getFamily() != AF_INET6) {
+    if (!prefix.isV6()) {
         isc_throw(BadValue, "Invalid Pool6 address boundaries: not IPv6");
     }
 

src/lib/dhcpsrv/subnet.cc

@@ -30,8 +30,8 @@ Subnet::Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len,
     :id_(getNextID()), prefix_(prefix), prefix_len_(len), t1_(t1),
      t2_(t2), valid_(valid_lifetime),
      last_allocated_(lastAddrInPrefix(prefix, len)) {
-    if ( (prefix.getFamily() == AF_INET6 && len > 128) ||
-         (prefix.getFamily() == AF_INET && len > 32) ) {
+    if ((prefix.isV6() && len > 128) ||
+        (prefix.isV4() && len > 32)) {
         isc_throw(BadValue, "Invalid prefix length specified for subnet: " << len);
     }
 }
@@ -65,7 +65,7 @@ Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t2,
                  const Triplet<uint32_t>& valid_lifetime)
     :Subnet(prefix, length, t1, t2, valid_lifetime) {
-    if (prefix.getFamily() != AF_INET) {
+    if (!prefix.isV4()) {
         isc_throw(BadValue, "Non IPv4 prefix " << prefix.toText()
                   << " specified in subnet4");
     }
@@ -136,7 +136,7 @@ Subnet6::Subnet6(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& valid_lifetime)
     :Subnet(prefix, length, t1, t2, valid_lifetime),
      preferred_(preferred_lifetime){
-    if (prefix.getFamily() != AF_INET6) {
+    if (!prefix.isV6()) {
         isc_throw(BadValue, "Non IPv6 prefix " << prefix.toText()
                   << " specified in subnet6");
     }

src/lib/dns/master_lexer.cc

@@ -37,7 +37,7 @@ using namespace master_lexer_internal;
 
 struct MasterLexer::MasterLexerImpl {
     MasterLexerImpl() : source_(NULL), token_(MasterToken::NOT_STARTED),
-                        paren_count_(0), last_was_eol_(false),
+                        paren_count_(0), last_was_eol_(true),
                         has_previous_(false),
                         previous_paren_count_(0),
                         previous_was_eol_(false)
@@ -127,6 +127,7 @@ MasterLexer::pushSource(const char* filename, std::string* error) {
 
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
+    impl_->last_was_eol_ = true;
     return (true);
 }
 
@@ -135,6 +136,7 @@ MasterLexer::pushSource(std::istream& input) {
     impl_->sources_.push_back(InputSourcePtr(new InputSource(input)));
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
+    impl_->last_was_eol_ = true;
 }
 
 void

src/lib/dns/master_lexer.h

@@ -54,7 +54,8 @@ public:
         END_OF_LINE, ///< End of line detected
         END_OF_FILE, ///< End of file detected
         INITIAL_WS,  ///< White spaces at the beginning of a line after an
-                     ///< end of line (if asked for detecting it)
+                     ///< end of line or at the beginning of file (if asked
+                     //   for detecting it)
         NOVALUE_TYPE_MAX = INITIAL_WS, ///< Max integer corresponding to
                                        /// no-value (type only) types.
                                        /// Mainly for internal use.

src/lib/dns/master_loader.cc

@@ -26,10 +26,16 @@
 
 #include <string>
 #include <memory>
+#include <vector>
+#include <boost/algorithm/string/predicate.hpp> // for iequals
+#include <boost/shared_ptr.hpp>
 
 using std::string;
 using std::auto_ptr;
+using std::vector;
+using std::pair;
 using boost::algorithm::iequals;
+using boost::shared_ptr;
 
 namespace isc {
 namespace dns {
@@ -58,6 +64,7 @@ public:
                      MasterLoader::Options options) :
         lexer_(),
         zone_origin_(zone_origin),
+        active_origin_(zone_origin),
         zone_class_(zone_class),
         callbacks_(callbacks),
         add_callback_(add_callback),
@@ -66,6 +73,7 @@ public:
         initialized_(false),
         ok_(true),
         many_errors_((options & MANY_ERRORS) != 0),
+        previous_name_(false),
         complete_(false),
         seen_error_(false),
         warn_rfc1035_ttl_(true)
@@ -82,7 +90,10 @@ public:
                 ok_ = false;
             }
         }
+        // Store the current status, so we can recover it upon popSource
+        include_info_.push_back(IncludeInfo(active_origin_, last_name_));
         initialized_ = true;
+        previous_name_ = false;
     }
 
     void pushStreamSource(std::istream& stream) {
@@ -112,6 +123,16 @@ private:
             return (false);
         }
         lexer_.popSource();
+        // Restore original origin and last seen name
+
+        // We move in tandem, there's an extra item included during the
+        // initialization, so we can never run out of them
+        assert(!include_info_.empty());
+        const IncludeInfo& info(include_info_.back());
+        active_origin_ = info.first;
+        last_name_ = info.second;
+        include_info_.pop_back();
+        previous_name_ = false;
         return (true);
     }
 
@@ -121,28 +142,100 @@ private:
         return (string_token_);
     }
 
-    void doInclude() {
-        // First, get the filename to include
-        const string
-            filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
+    MasterToken handleInitialToken();
 
-        // There could be an origin (or maybe not). So try looking
-        const MasterToken name_tok(lexer_.getNextToken(MasterToken::QSTRING,
-                                                       true));
+    void doOrigin(bool is_optional) {
+        // Parse and create the new origin. It is relative to the previous
+        // one.
+        const MasterToken&
+            name_tok(lexer_.getNextToken(MasterToken::QSTRING, is_optional));
 
         if (name_tok.getType() == MasterToken::QSTRING ||
             name_tok.getType() == MasterToken::STRING) {
-            // TODO: Handle the origin. Once we complete #2427.
+
+            const MasterToken::StringRegion&
+                name_string(name_tok.getStringRegion());
+            active_origin_ = Name(name_string.beg, name_string.len,
+                                  &active_origin_);
+            if (name_string.len > 0 &&
+                name_string.beg[name_string.len - 1] != '.') {
+                callbacks_.warning(lexer_.getSourceName(),
+                                   lexer_.getSourceLine(),
+                                   "The new origin is relative, did you really"
+                                   " mean " + active_origin_.toText() + "?");
+            }
         } else {
-            // We return the newline there. This is because after we pop
-            // the source, we want to call eatUntilEOL and this would
-            // eat to the next one.
+            // If it is not optional, we must not get anything but
+            // a string token.
+            assert(is_optional);
+
+            // We return the newline there. This is because we want to
+            // behave the same if there is or isn't the name, leaving the
+            // newline there.
             lexer_.ungetToken();
         }
+    }
+
+    void doInclude() {
+        // First, get the filename to include
+        const string
+            filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
+
+        // There optionally can be an origin, that applies before the include.
+        doOrigin(true);
 
         pushSource(filename);
     }
 
+    // A helper method for loadIncremental(). It parses part of an RR
+    // until it finds the RR type field.  If TTL or RR class is
+    // specified before the RR type, it also recognizes and validates
+    // them.  explicit_ttl will be set to true if this method finds a
+    // valid TTL field.
+    RRType parseRRParams(bool& explicit_ttl, MasterToken rrparam_token) {
+        // Find TTL, class and type.  Both TTL and class are
+        // optional and may occur in any order if they exist. TTL
+        // and class come before type which must exist.
+        //
+        // [<TTL>] [<class>] <type> <RDATA>
+        // [<class>] [<TTL>] <type> <RDATA>
+
+        // named-signzone outputs TTL first, so try parsing it in order
+        // first.
+        if (setCurrentTTL(rrparam_token.getString())) {
+            explicit_ttl = true;
+            rrparam_token = lexer_.getNextToken(MasterToken::STRING);
+        } else {
+            // If it's not a TTL here, continue and try again
+            // after the RR class below.
+        }
+
+        boost::scoped_ptr<RRClass> rrclass;
+        try {
+            rrclass.reset(new RRClass(rrparam_token.getString()));
+            rrparam_token = lexer_.getNextToken(MasterToken::STRING);
+        } catch (const InvalidRRClass&) {
+            // If it's not an rrclass here, use the zone's class.
+            rrclass.reset(new RRClass(zone_class_));
+        }
+
+        // If we couldn't parse TTL earlier in the stream (above), try
+        // again at current location.
+        if (!explicit_ttl &&
+            setCurrentTTL(rrparam_token.getString())) {
+            explicit_ttl = true;
+            rrparam_token = lexer_.getNextToken(MasterToken::STRING);
+        }
+
+        if (*rrclass != zone_class_) {
+            isc_throw(InternalException, "Class mismatch: " << *rrclass <<
+                      "vs. " << zone_class_);
+        }
+
+        // Return the current string token's value as the RRType.
+        return (RRType(rrparam_token.getString()));
+    }
+
     // 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 2^31-1 (0x7fffffff),
@@ -158,7 +251,7 @@ private:
     // 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 > RRTTL::MAX()) {
+        if (ttl > RRTTL::MAX_TTL()) {
             const size_t src_line = lexer_.getSourceLine() -
                 (post_parsing ? 1 : 0);
             callbacks_.warning(lexer_.getSourceName(), src_line,
@@ -248,9 +341,8 @@ private:
         if (iequals(directive, "INCLUDE")) {
             doInclude();
         } else if (iequals(directive, "ORIGIN")) {
-            // TODO: Implement
-            isc_throw(isc::NotImplemented,
-                      "Origin directive not implemented yet");
+            doOrigin(false);
+            eatUntilEOL(true);
         } else if (iequals(directive, "TTL")) {
             setDefaultTTL(RRTTL(getString()), false);
             eatUntilEOL(true);
@@ -268,7 +360,7 @@ private:
                 case MasterToken::END_OF_FILE:
                     callbacks_.warning(lexer_.getSourceName(),
                                        lexer_.getSourceLine(),
-                                       "Unexpected end of file");
+                                       "File does not end with newline");
                     // We don't pop here. The End of file will stay there,
                     // and we'll handle it in the next iteration of
                     // loadIncremental properly.
@@ -292,6 +384,9 @@ private:
 private:
     MasterLexer lexer_;
     const Name zone_origin_;
+    Name active_origin_; // The origin used during parsing
+                         // (modifiable by $ORIGIN)
+    shared_ptr<Name> last_name_; // Last seen name (for INITAL_WS handling)
     const RRClass zone_class_;
     MasterLoaderCallbacks callbacks_;
     AddRRCallback add_callback_;
@@ -307,6 +402,13 @@ private:
     bool ok_;                   // Is it OK to continue loading?
     const bool many_errors_;    // Are many errors allowed (or should we abort
                                 // on the first)
+    // Some info about the outer files from which we include.
+    // The first one is current origin, the second is the last seen name
+    // in that file.
+    typedef pair<Name, shared_ptr<Name> > IncludeInfo;
+    vector<IncludeInfo> include_info_;
+    bool previous_name_; // True if there was a previous name in this file
+                         // (false at the beginning or after an $INCLUDE line)
 public:
     bool complete_;             // All work done.
     bool seen_error_;           // Was there at least one error during the
@@ -315,6 +417,91 @@ public:
                                 // from the previous RR is used.
 };
 
+// A helper method of loadIncremental, parsing the first token of a new line.
+// If it looks like an RR, detect its owner name and return a string token for
+// the next field of the RR.
+// Otherwise, return either END_OF_LINE or END_OF_FILE token depending on
+// whether the loader continues to the next line or completes the load,
+// respectively.  Other corner cases including $-directive handling is done
+// here.
+// For unexpected errors, it throws an exception, which will be handled in
+// loadIncremental.
+MasterToken
+MasterLoader::MasterLoaderImpl::handleInitialToken() {
+    const MasterToken& initial_token =
+        lexer_.getNextToken(MasterLexer::QSTRING | MasterLexer::INITIAL_WS);
+
+    // The most likely case is INITIAL_WS, and then string/qstring.  We
+    // handle them first.
+    if (initial_token.getType() == MasterToken::INITIAL_WS) {
+        const MasterToken& next_token = lexer_.getNextToken();
+        if (next_token.getType() == MasterToken::END_OF_LINE) {
+            return (next_token); // blank line
+        } else if (next_token.getType() == MasterToken::END_OF_FILE) {
+            lexer_.ungetToken(); // handle it in the next iteration.
+            eatUntilEOL(true);  // effectively warn about the unexpected EOF.
+            return (MasterToken(MasterToken::END_OF_LINE));
+        }
+
+        // This means the same name as previous.
+        if (last_name_.get() == NULL) {
+            isc_throw(InternalException, "No previous name to use in "
+                      "place of initial whitespace");
+        } else if (!previous_name_) {
+            callbacks_.warning(lexer_.getSourceName(), lexer_.getSourceLine(),
+                               "Owner name omitted around $INCLUDE, the result "
+                               "might not be as expected");
+        }
+        return (next_token);
+    } else if (initial_token.getType() == MasterToken::STRING ||
+               initial_token.getType() == MasterToken::QSTRING) {
+        // If it is name (or directive), handle it.
+        const MasterToken::StringRegion&
+            name_string(initial_token.getStringRegion());
+
+        if (name_string.len > 0 && name_string.beg[0] == '$') {
+            // This should have either thrown (and the error handler
+            // will read up until the end of line) or read until the
+            // end of line.
+
+            // Exclude the $ from the string on this point.
+            handleDirective(name_string.beg + 1, name_string.len - 1);
+            // So, get to the next line, there's nothing more interesting
+            // in this one.
+            return (MasterToken(MasterToken::END_OF_LINE));
+        }
+
+        // This should be an RR, starting with an owner name.  Construct the
+        // name, and some string token should follow.
+        last_name_.reset(new Name(name_string.beg, name_string.len,
+                                  &active_origin_));
+        previous_name_ = true;
+        return (lexer_.getNextToken(MasterToken::STRING));
+    }
+
+    switch (initial_token.getType()) { // handle less common cases
+    case MasterToken::END_OF_FILE:
+        if (!popSource()) {
+            return (initial_token);
+        } else {
+            // We try to read a token from the popped source
+            // So continue to the next line of that source, but first, make
+            // sure the source is at EOL
+            eatUntilEOL(true);
+            return (MasterToken(MasterToken::END_OF_LINE));
+        }
+    case MasterToken::END_OF_LINE:
+        return (initial_token); // empty line
+    case MasterToken::ERROR:
+        // Error token here.
+        isc_throw(InternalException, initial_token.getErrorText());
+    default:
+        // Some other token (what could that be?)
+        isc_throw(InternalException, "Parser got confused (unexpected "
+                  "token " << initial_token.getType() << ")");
+    }
+}
+
 bool
 MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
     if (count_limit == 0) {
@@ -330,85 +517,32 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
     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) {
-                    if (!popSource()) {
-                        return (true);
-                    } else {
-                        // We try to read a token from the popped source
-                        // So retry the loop, but first, make sure the source
-                        // is at EOL
-                        eatUntilEOL(true);
-                        continue;
-                    }
-                }
-                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());
-
-            if (name_string.len > 0 && name_string.beg[0] == '$') {
-                // This should have either thrown (and the error handler
-                // will read up until the end of line) or read until the
-                // end of line.
-
-                // Exclude the $ from the string on this point.
-                handleDirective(name_string.beg + 1, name_string.len - 1);
-                // So, get to the next line, there's nothing more interesting
-                // in this one.
-                continue;
+            const MasterToken next_token = handleInitialToken();
+            if (next_token.getType() == MasterToken::END_OF_FILE) {
+                return (true);  // we are done
+            } else if (next_token.getType() == MasterToken::END_OF_LINE) {
+                continue;       // nothing more to do in this line
             }
-
-            const Name name(name_string.beg, name_string.len,
-                            &zone_origin_);
-            // TODO: Some more flexibility. We don't allow omitting
-            // anything yet
-
-            // The parameters
-            MasterToken rrparam_token = lexer_.getNextToken();
+            // We are going to parse an RR, have known the owner name,
+            // and are now seeing the next string token in the rest of the RR.
+            assert(next_token.getType() == MasterToken::STRING);
 
             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?
-            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_);
-            }
+            const RRType rrtype = parseRRParams(explicit_ttl, next_token);
             // TODO: Check if it is SOA, it should be at the origin.
 
-            const rdata::RdataPtr rdata(rdata::createRdata(rrtype, rrclass,
-                                                           lexer_,
-                                                           &zone_origin_,
-                                                           options_,
-                                                           callbacks_));
+            const rdata::RdataPtr rdata =
+                rdata::createRdata(rrtype, zone_class_, lexer_,
+                                   &active_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 (rdata) {
-                add_callback_(name, rrclass, rrtype,
+                add_callback_(*last_name_, zone_class_, rrtype,
                               getCurrentTTL(explicit_ttl, rrtype, rdata),
                               rdata);
-
                 // Good, we loaded another one
                 ++count;
             } else {

src/lib/dns/rrttl.h

@@ -293,7 +293,7 @@ public:
     ///
     /// \note At the moment an RRTTL object can have a value larger than
     /// this limit.  We may revisit it in a future version.
-    static const RRTTL& MAX() {
+    static const RRTTL& MAX_TTL() {
         static const RRTTL max_ttl(0x7fffffff);
         return (max_ttl);
     }

src/lib/dns/tests/master_lexer_state_unittest.cc

@@ -190,13 +190,16 @@ TEST_F(MasterLexerStateTest, unbalancedParentheses) {
 }
 
 TEST_F(MasterLexerStateTest, startToComment) {
-    // Begin with 'start', skip space, then encounter a comment.  Skip
+    // Begin with 'start', detect space, then encounter a comment.  Skip
     // the rest of the line, and recognize the new line.  Note that the
     // second ';' is simply ignored.
     ss << "  ;a;\n";
     ss << ";a;";           // Likewise, but the comment ends with EOF.
     lexer.pushSource(ss);
 
+    // Initial whitespace (asked for in common_options)
+    EXPECT_EQ(s_null, State::start(lexer, common_options));
+    EXPECT_EQ(Token::INITIAL_WS, s_crlf.getToken(lexer).getType());
     // Comment ending with EOL
     EXPECT_EQ(s_null, State::start(lexer, common_options));
     EXPECT_EQ(Token::END_OF_LINE, s_crlf.getToken(lexer).getType());

src/lib/dns/tests/master_lexer_unittest.cc

@@ -238,10 +238,8 @@ TEST_F(MasterLexerTest, ungetToken) {
 // Check ungetting token without overriding the start method. We also
 // check it works well with changing options between the calls.
 TEST_F(MasterLexerTest, ungetRealOptions) {
-    ss << "\n    \n";
+    ss << "    \n";
     lexer.pushSource(ss);
-    // Skip the first newline
-    EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType());
 
     // If we call it the usual way, it skips up to the newline and returns
     // it
@@ -254,6 +252,22 @@ TEST_F(MasterLexerTest, ungetRealOptions) {
               lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
 }
 
+// Check the initial whitespace is found even in the first line of included
+// file
+TEST_F(MasterLexerTest, includeAndInitialWS) {
+    ss << "    \n";
+    lexer.pushSource(ss);
+
+    stringstream ss2;
+    ss2 << "    \n";
+
+    EXPECT_EQ(MasterToken::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+    lexer.pushSource(ss2);
+    EXPECT_EQ(MasterToken::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+}
+
 // Test only one token can be ungotten
 TEST_F(MasterLexerTest, ungetTwice) {
     ss << "\n";

src/lib/dns/tests/master_loader_unittest.cc

@@ -50,6 +50,11 @@ public:
                                &warnings_, _1, _2, _3))
     {}
 
+    void TearDown() {
+        // Check there are no more RRs we didn't expect
+        EXPECT_TRUE(rrsets_.empty());
+    }
+
     /// 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,
@@ -127,6 +132,11 @@ public:
                 "1234 3600 1800 2419200 7200");
         checkRR("example.org", RRType::NS(), "ns1.example.org.");
         checkRR("www.example.org", RRType::A(), "192.0.2.1");
+        checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
+    }
+
+    void checkARR(const string& name) {
+        checkRR(name, RRType::A(), "192.0.2.1");
     }
 
     MasterLoaderCallbacks callbacks_;
@@ -185,6 +195,66 @@ TEST_F(MasterLoaderTest, include) {
     }
 }
 
+// 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)) << "Expected on line " <<
+        expected_line;
+}
+
+TEST_F(MasterLoaderTest, origin) {
+    // Various forms of the directive
+    const char* origins[] = {
+        "$origin",
+        "$ORIGIN",
+        "$Origin",
+        "$OrigiN",
+        "\"$ORIGIN\"",
+        NULL
+    };
+    for (const char** origin = origins; *origin != NULL; ++origin) {
+        SCOPED_TRACE(*origin);
+
+        clear();
+        const string directive = *origin;
+        const string input =
+            "@  1H  IN  A   192.0.2.1\n" +
+            directive + " sub.example.org.\n"
+            "\"www\"    1H  IN  A   192.0.2.1\n" +
+            // Relative name in the origin
+            directive + " relative\n"
+            "@  1H  IN  A   192.0.2.1\n"
+            // Origin is _not_ used here (absolute name)
+            "noorigin.example.org.  60M IN  A   192.0.2.1\n";
+        stringstream ss(input);
+        setLoader(ss, Name("example.org."), RRClass::IN(),
+                  MasterLoader::MANY_ERRORS);
+
+        loader_->load();
+        EXPECT_TRUE(loader_->loadedSucessfully());
+        EXPECT_TRUE(errors_.empty());
+        // There's a relative origin in it, we warn about that.
+        EXPECT_EQ(1, warnings_.size());
+        checkCallbackMessage(warnings_.at(0),
+                             "The new origin is relative, did you really mean "
+                             "relative.sub.example.org.?", 4);
+
+        checkARR("example.org");
+        checkARR("www.sub.example.org");
+        checkARR("relative.sub.example.org");
+        checkARR("noorigin.example.org");
+    }
+}
+
 // Test the source is correctly popped even after error
 TEST_F(MasterLoaderTest, popAfterError) {
     const string include_str = "$include " TEST_DATA_SRCDIR
@@ -250,6 +320,7 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
     EXPECT_TRUE(warnings_.empty());
 
     checkRR("www.example.org", RRType::A(), "192.0.2.1");
+    checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
 }
 
 // Try loading from file that doesn't exist. There should be single error
@@ -280,6 +351,45 @@ struct ErrorCase {
     { "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" },
+
+    // Parameter ordering errors
+    { "www      IN      A   3600 192.168.2.7",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Incorrect order of class, TTL and type" },
+    { "www      A       IN  3600 192.168.2.8",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Incorrect order of class, TTL and type" },
+    { "www      3600    A   IN   192.168.2.7",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Incorrect order of class, TTL and type" },
+    { "www      A       3600 IN  192.168.2.8",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Incorrect order of class, TTL and type" },
+
+    // Missing type and Rdata
+    { "www", "unexpected end of input", "Missing type and Rdata" },
+    { "www 3600", "unexpected end of input", "Missing type and Rdata" },
+    { "www IN", "unexpected end of input", "Missing type and Rdata" },
+    { "www 3600 IN", "unexpected end of input", "Missing type and Rdata" },
+    { "www IN 3600", "unexpected end of input", "Missing type and Rdata" },
+
+    // Missing Rdata
+    { "www A",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Missing Rdata" },
+    { "www 3600 A",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Missing Rdata" },
+    { "www IN A",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Missing Rdata" },
+    { "www 3600 IN A",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Missing Rdata" },
+    { "www IN 3600 A",
+      "createRdata from text failed: IN/A RDATA construction from text failed",
+      "Missing 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" },
@@ -291,12 +401,24 @@ struct ErrorCase {
     // 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", 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" },
+    { "$INCLUD " TEST_DATA_SRCDIR "/example.org", "Unknown directive 'INCLUD'",
+        "Include too short" },
+    { "$INCLUDES " TEST_DATA_SRCDIR "/example.org",
+        "Unknown directive 'INCLUDES'", "Include too long" },
+    { "$INCLUDE", "unexpected end of input", "Missing include path" },
+    // The following two error messages are system dependant, omitting
     { "$INCLUDE /file/not/found", NULL, "Include file not found" },
-    { "$INCLUDE /file/not/found and here goes bunch of garbage", NULL,
-        "Include file not found and garbage at the end of line" },
+    { "$INCLUDE /file/not/found example.org. and here goes bunch of garbage",
+        NULL, "Include file not found and garbage at the end of line" },
+    { "$ORIGIN", "unexpected end of input", "Missing origin name" },
+    { "$ORIGIN invalid...name", "duplicate period in invalid...name",
+        "Invalid name for origin" },
+    { "$ORIGIN )brokentoken", "unbalanced parentheses",
+        "Broken token in origin" },
+    { "$ORIGIN example.org. garbage", "Extra tokens at the end of line",
+        "Garbage after origin" },
+    { "$ORIGI name.", "Unknown directive 'ORIGI'", "$ORIGIN too short" },
+    { "$ORIGINAL name.", "Unknown directive 'ORIGINAL'", "$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" },
@@ -307,19 +429,6 @@ struct ErrorCase {
     { 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;
-
-    // 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) {
@@ -394,7 +503,7 @@ TEST_F(MasterLoaderTest, includeWithGarbage) {
     // Include an origin (example.org) because we expect it to be handled
     // soon and we don't want it to break here.
     const string include_str("$INCLUDE " TEST_DATA_SRCDIR
-                             "/example.org example.org bunch of other stuff\n"
+                             "/example.org example.org. bunch of other stuff\n"
                              "www 3600 IN AAAA 2001:db8::1\n");
     stringstream zone_stream(include_str);
     setLoader(zone_stream, Name("example.org."), RRClass::IN(),
@@ -403,6 +512,7 @@ TEST_F(MasterLoaderTest, includeWithGarbage) {
     EXPECT_NO_THROW(loader_->load());
     EXPECT_FALSE(loader_->loadedSucessfully());
     ASSERT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "Extra tokens at the end of line", 1);
     // It says something about extra tokens at the end
     EXPECT_NE(string::npos, errors_[0].find("Extra"));
     EXPECT_TRUE(warnings_.empty());
@@ -410,6 +520,105 @@ 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).
+TEST_F(MasterLoaderTest, originWithGarbage) {
+    const string origin_str = "$ORIGIN www.example.org. More garbage here\n"
+        "@  1H  IN  A   192.0.2.1\n";
+    stringstream ss(origin_str);
+    setLoader(ss, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    EXPECT_NO_THROW(loader_->load());
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    ASSERT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "Extra tokens at the end of line", 1);
+    EXPECT_TRUE(warnings_.empty());
+    checkARR("www.example.org");
+}
+
+// Test we can pass both file to include and the origin to switch
+TEST_F(MasterLoaderTest, includeAndOrigin) {
+    // First, switch origin to something else, so we can check it is
+    // switched back.
+    const string include_string = "$ORIGIN www.example.org.\n"
+        "@  1H  IN  A   192.0.2.1\n"
+        // Then include the file with data and switch origin back
+        "$INCLUDE " TEST_DATA_SRCDIR "/example.org example.org.\n"
+        // Another RR to see the switch survives after we exit include
+        "www    1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    // Successfully load the data
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+    // And check it's the correct data
+    checkARR("www.example.org");
+    checkBasicRRs();
+    checkARR("www.example.org");
+}
+
+// Like above, but the origin after include is bogus. The whole line should
+// be rejected.
+TEST_F(MasterLoaderTest, includeAndBadOrigin) {
+    const string include_string =
+        "$INCLUDE " TEST_DATA_SRCDIR "/example.org example..org.\n"
+        // Another RR to see the switch survives after we exit include
+        "www    1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    EXPECT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "duplicate period in example..org.",
+                         1);
+    EXPECT_TRUE(warnings_.empty());
+    // And check it's the correct data
+    checkARR("www.example.org");
+}
+
+// Check the origin doesn't get outside of the included file.
+TEST_F(MasterLoaderTest, includeOriginRestore) {
+    const string include_string = "$INCLUDE " TEST_DATA_SRCDIR "/origincheck.txt\n"
+        "@  1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    // Successfully load the data
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+    // And check it's the correct data
+    checkARR("www.example.org");
+    checkARR("example.org");
+}
+
+// Check we restore the last name for initial whitespace when returning from
+// include. But we do produce a warning if there's one just ofter the include.
+TEST_F(MasterLoaderTest, includeAndInitialWS) {
+    const string include_string = "xyz  1H  IN  A   192.0.2.1\n"
+        "$INCLUDE " TEST_DATA_SRCDIR "/example.org\n"
+        "   1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    // Successfully load the data
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0),
+                         "Owner name omitted around $INCLUDE, the result might "
+                         "not be as expected", 3);
+    checkARR("xyz.example.org");
+    checkBasicRRs();
+    checkARR("xyz.example.org");
+}
+
 // Test for "$TTL"
 TEST_F(MasterLoaderTest, ttlDirective) {
     stringstream zone_stream;
@@ -473,6 +682,36 @@ TEST_F(MasterLoaderTest, ttlFromPrevious) {
     checkCallbackMessage(warnings_.at(0), "using RFC1035 TTL semantics", 2);
 }
 
+TEST_F(MasterLoaderTest, RRParamsOrdering) {
+    // We test the order and existence of TTL, class and type. See
+    // MasterLoader::MasterLoaderImpl::parseRRParams() for ordering.
+
+    stringstream zone_stream;
+    // <TTL> <class> <type> <RDATA>
+    zone_stream << "a.example.org. 1800 IN A 192.0.2.1\n";
+    // <type> <RDATA>
+    zone_stream << "b.example.org. A 192.0.2.2\n";
+    // <class> <TTL> <type> <RDATA>
+    zone_stream << "c.example.org. IN 3600 A 192.0.2.3\n";
+    // <TTL> <type> <RDATA>
+    zone_stream << "d.example.org. 7200 A 192.0.2.4\n";
+    // <class> <type> <RDATA>
+    zone_stream << "e.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("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(3600));
+    checkRR("d.example.org", RRType::A(), "192.0.2.4", RRTTL(7200));
+    checkRR("e.example.org", RRType::A(), "192.0.2.5", RRTTL(7200));
+
+    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
@@ -529,7 +768,8 @@ TEST_F(MasterLoaderTest, ttlUnknownAndEOF) {
     // 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(), "Unexpected end of file", 1);
+    checkCallbackMessage(*warnings_.rbegin(), "File does not end with newline",
+                         1);
 }
 
 TEST_F(MasterLoaderTest, ttlOverflow) {
@@ -581,6 +821,9 @@ TEST_F(MasterLoaderTest, loadTwice) {
 
     loader_->load();
     EXPECT_THROW(loader_->load(), isc::InvalidOperation);
+    // Don't check them, they are not interesting, so suppress the error
+    // at TearDown
+    rrsets_.clear();
 }
 
 // Load 0 items should be rejected
@@ -602,10 +845,44 @@ TEST_F(MasterLoaderTest, noEOLN) {
 
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
-    EXPECT_TRUE(errors_.empty()) << errors_[0];
+    EXPECT_TRUE(errors_.empty());
     // 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");
 }
+
+// Test it rejects when we don't have the previous name to use in place of
+// initial whitespace
+TEST_F(MasterLoaderTest, noPreviousName) {
+    const string input("    1H  IN  A   192.0.2.1\n");
+    stringstream ss(input);
+    setLoader(ss, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    EXPECT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "No previous name to use in place of "
+                         "initial whitespace", 1);
+    EXPECT_TRUE(warnings_.empty());
+}
+
+// Check we warn if the first RR in an included file has omitted name
+TEST_F(MasterLoaderTest, previousInInclude) {
+    const string input("www 1H  IN  A   192.0.2.1\n"
+                       "$INCLUDE " TEST_DATA_SRCDIR "/omitcheck.txt\n");
+    stringstream ss(input);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    // There should be one warning about the EOLN
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0), "Owner name omitted around "
+                         "$INCLUDE, the result might not be as expected", 1);
+    checkARR("www.example.org");
+    checkARR("www.example.org");
+}
+
 }

src/lib/dns/tests/rrttl_unittest.cc

@@ -266,7 +266,7 @@ TEST_F(RRTTLTest, gthan) {
 }
 
 TEST_F(RRTTLTest, maxTTL) {
-    EXPECT_EQ((1u << 31) - 1, RRTTL::MAX().getValue());
+    EXPECT_EQ((1u << 31) - 1, RRTTL::MAX_TTL().getValue());
 }
 
 // test operator<<.  We simply confirm it appends the result of toText().

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

@@ -172,6 +172,8 @@ EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec
 EXTRA_DIST += tsig_verify10.spec
 EXTRA_DIST += example.org
 EXTRA_DIST += broken.zone
+EXTRA_DIST += origincheck.txt
+EXTRA_DIST += omitcheck.txt
 
 .spec.wire:
 	$(PYTHON) $(top_builddir)/src/lib/util/python/gen_wiredata.py -o $@ $<

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

@@ -13,3 +13,5 @@ example.org.        3600    IN  SOA ( ; The SOA, split across lines for testing
 
 ; 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.
+                    3600    IN  AAAA    2001:db8::1 ; And initial whitespace hanling
+         ; Here be just some space, no RRs

src/lib/dns/tests/testdata/omitcheck.txt

@@ -0,0 +1 @@
+    1H  IN  A   192.0.2.1

src/lib/dns/tests/testdata/origincheck.txt

@@ -0,0 +1,5 @@
+; We change the origin here. We want to check it is not propagated
+; outside of the included file.
+$ORIGIN www.example.org.
+
+@   1H  IN  A   192.0.2.1

src/lib/log/tests/.gitignore

@@ -1,3 +1,5 @@
+/buffer_logger_test
+/buffer_logger_test.sh
 /console_test.sh
 /destination_test.sh
 /init_logger_test