Browse Source

[trac493] Make some modifications as review feedbacks:
1. Change some arguments to const reference.
2. Use RRsetCachePtr instead of boost:shared_ptr<RRsetCache>
3. Make RRsetRef use raw pointer instead of shared_ptr and
move it as internal struct.
4. Clear the AA flag when generate response.
5. Add max ttl limit to normal and negative messages.
6. Add some new unit tests

Ocean Wang 14 years ago
parent
commit
e1b0deba4e

+ 2 - 2
src/lib/cache/message_cache.cc

@@ -31,9 +31,9 @@ using namespace isc::dns;
 using namespace std;
 using namespace std;
 using namespace MessageUtility;
 using namespace MessageUtility;
 
 
-MessageCache::MessageCache(boost::shared_ptr<RRsetCache> rrset_cache,
+MessageCache::MessageCache(const RRsetCachePtr& rrset_cache,
                            uint32_t cache_size, uint16_t message_class,
                            uint32_t cache_size, uint16_t message_class,
-                           boost::shared_ptr<RRsetCache> negative_soa_cache):
+                           const RRsetCachePtr& negative_soa_cache):
     message_class_(message_class),
     message_class_(message_class),
     rrset_cache_(rrset_cache),
     rrset_cache_(rrset_cache),
     negative_soa_cache_(negative_soa_cache),
     negative_soa_cache_(negative_soa_cache),

+ 5 - 6
src/lib/cache/message_cache.h

@@ -23,12 +23,11 @@
 #include "message_entry.h"
 #include "message_entry.h"
 #include <nsas/hash_table.h>
 #include <nsas/hash_table.h>
 #include <nsas/lru_list.h>
 #include <nsas/lru_list.h>
+#include "rrset_cache.h"
 
 
 namespace isc {
 namespace isc {
 namespace cache {
 namespace cache {
 
 
-class RRsetCache;
-
 /// \brief Message Cache
 /// \brief Message Cache
 /// The object of MessageCache represents the cache for class-specific
 /// The object of MessageCache represents the cache for class-specific
 /// messages.
 /// messages.
@@ -45,9 +44,9 @@ public:
     /// \param message_class The class of the message cache
     /// \param message_class The class of the message cache
     /// \param negative_soa_cache The cache that stores the SOA record
     /// \param negative_soa_cache The cache that stores the SOA record
     ///        that comes from negative response message
     ///        that comes from negative response message
-    MessageCache(boost::shared_ptr<RRsetCache> rrset_cache,
+    MessageCache(const RRsetCachePtr& rrset_cache,
                  uint32_t cache_size, uint16_t message_class,
                  uint32_t cache_size, uint16_t message_class,
-                 boost::shared_ptr<RRsetCache> negative_soa_cache);
+                 const RRsetCachePtr& negative_soa_cache);
 
 
     /// \brief Look up message in cache.
     /// \brief Look up message in cache.
     /// \param message generated response message if the message entry
     /// \param message generated response message if the message entry
@@ -87,8 +86,8 @@ protected:
     // Make these variants be protected for easy unittest.
     // Make these variants be protected for easy unittest.
 protected:
 protected:
     uint16_t message_class_; // The class of the message cache.
     uint16_t message_class_; // The class of the message cache.
-    boost::shared_ptr<RRsetCache> rrset_cache_;
+    RRsetCachePtr rrset_cache_;
-    boost::shared_ptr<RRsetCache> negative_soa_cache_;
+    RRsetCachePtr negative_soa_cache_;
     isc::nsas::HashTable<MessageEntry> message_table_;
     isc::nsas::HashTable<MessageEntry> message_table_;
     isc::nsas::LruList<MessageEntry> message_lru_;
     isc::nsas::LruList<MessageEntry> message_lru_;
 };
 };

+ 38 - 7
src/lib/cache/message_entry.cc

@@ -31,9 +31,25 @@ namespace cache {
 
 
 static uint32_t MAX_UINT32 = numeric_limits<uint32_t>::max();
 static uint32_t MAX_UINT32 = numeric_limits<uint32_t>::max();
 
 
+// As with caching positive responses it is sensible for a resolver to
+// limit for how long it will cache a negative response as the protocol
+// supports caching for up to 68 years.  Such a limit should not be
+// greater than that applied to positive answers and preferably be
+// tunable.  Values of one to three hours have been found to work well
+// and would make sensible a default.  Values exceeding one day have
+// been found to be problematic. (sec 5, RFC2308)
+// The default value is 3 hourse (10800 seconds)
+// TODO:Give an option to let user configure
+static uint32_t MAX_NEGATIVE_CACHE_TTL = 10800;
+
+// Sets the maximum time for which the server will cache ordinary (positive) answers. The
+// default is one week (7 days = 604800 seconds)
+// TODO:Give an option to let user configure
+static uint32_t MAX_NORMAL_CACHE_TTL = 604800;
+
 MessageEntry::MessageEntry(const isc::dns::Message& msg,
 MessageEntry::MessageEntry(const isc::dns::Message& msg,
-                           boost::shared_ptr<RRsetCache> rrset_cache,
+                           const RRsetCachePtr& rrset_cache,
-                           boost::shared_ptr<RRsetCache> negative_soa_cache):
+                           const RRsetCachePtr& negative_soa_cache):
     rrset_cache_(rrset_cache),
     rrset_cache_(rrset_cache),
     negative_soa_cache_(negative_soa_cache),
     negative_soa_cache_(negative_soa_cache),
     headerflag_aa_(false),
     headerflag_aa_(false),
@@ -51,7 +67,7 @@ MessageEntry::getRRsetEntries(vector<RRsetEntryPtr>& rrset_entry_vec,
     uint16_t entry_count = answer_count_ + authority_count_ + additional_count_;
     uint16_t entry_count = answer_count_ + authority_count_ + additional_count_;
     rrset_entry_vec.reserve(rrset_entry_vec.size() + entry_count);
     rrset_entry_vec.reserve(rrset_entry_vec.size() + entry_count);
     for (int index = 0; index < entry_count; ++index) {
     for (int index = 0; index < entry_count; ++index) {
-        boost::shared_ptr<RRsetCache> rrset_cache = rrsets_[index].cache_;
+        RRsetCache* rrset_cache = rrsets_[index].cache_;
         RRsetEntryPtr rrset_entry = rrset_cache->lookup(rrsets_[index].name_,
         RRsetEntryPtr rrset_entry = rrset_cache->lookup(rrsets_[index].name_,
                                                         rrsets_[index].type_);
                                                         rrsets_[index].type_);
         if (time_now < rrset_entry->getExpireTime()) {
         if (time_now < rrset_entry->getExpireTime()) {
@@ -106,7 +122,9 @@ MessageEntry::genMessage(const time_t& time_now,
         // Begin message generation. We don't need to add question
         // Begin message generation. We don't need to add question
         // section, since it has been included in the message.
         // section, since it has been included in the message.
         // Set cached header flags.
         // Set cached header flags.
-        msg.setHeaderFlag(Message::HEADERFLAG_AA, headerflag_aa_);
+        // The AA flag bit should be cleared because this is a response from
+        // resolver cache
+        msg.setHeaderFlag(Message::HEADERFLAG_AA, false);
         msg.setHeaderFlag(Message::HEADERFLAG_TC, headerflag_tc_);
         msg.setHeaderFlag(Message::HEADERFLAG_TC, headerflag_tc_);
 
 
         bool dnssec_need = msg.getEDNS().get();
         bool dnssec_need = msg.getEDNS().get();
@@ -216,7 +234,7 @@ MessageEntry::parseSection(const isc::dns::Message& msg,
         RRsetTrustLevel level = getRRsetTrustLevel(msg, rrset_ptr, section);
         RRsetTrustLevel level = getRRsetTrustLevel(msg, rrset_ptr, section);
         RRsetEntryPtr rrset_entry = rrset_cache_->update(*rrset_ptr, level);
         RRsetEntryPtr rrset_entry = rrset_cache_->update(*rrset_ptr, level);
         rrsets_.push_back(RRsetRef(rrset_ptr->getName(), rrset_ptr->getType(),
         rrsets_.push_back(RRsetRef(rrset_ptr->getName(), rrset_ptr->getType(),
-                          rrset_cache_));
+                          rrset_cache_.get()));
 
 
         uint32_t rrset_ttl = rrset_entry->getTTL();
         uint32_t rrset_ttl = rrset_entry->getTTL();
         if (smaller_ttl > rrset_ttl) {
         if (smaller_ttl > rrset_ttl) {
@@ -249,7 +267,7 @@ MessageEntry::parseNegativeResponseAuthoritySection(const isc::dns::Message& msg
         RRsetEntryPtr rrset_entry = rrset_cache_ptr->update(*rrset_ptr, level);
         RRsetEntryPtr rrset_entry = rrset_cache_ptr->update(*rrset_ptr, level);
         rrsets_.push_back(RRsetRef(rrset_ptr->getName(),
         rrsets_.push_back(RRsetRef(rrset_ptr->getName(),
                                    rrset_ptr->getType(),
                                    rrset_ptr->getType(),
-                                   rrset_cache_ptr));
+                                   rrset_cache_ptr.get()));
         uint32_t rrset_ttl = rrset_entry->getTTL();
         uint32_t rrset_ttl = rrset_entry->getTTL();
         if (min_ttl > rrset_ttl) {
         if (min_ttl > rrset_ttl) {
             min_ttl = rrset_ttl;
             min_ttl = rrset_ttl;
@@ -276,14 +294,27 @@ MessageEntry::initMessageEntry(const isc::dns::Message& msg) {
 
 
     uint32_t min_ttl = MAX_UINT32;
     uint32_t min_ttl = MAX_UINT32;
 
 
+    bool isNegativeResponse = MessageUtility::isNegativeResponse(msg);
+
     parseSection(msg, Message::SECTION_ANSWER, min_ttl, answer_count_);
     parseSection(msg, Message::SECTION_ANSWER, min_ttl, answer_count_);
-    if (!MessageUtility::isNegativeResponse(msg)){
+    if (!isNegativeResponse) {
         parseSection(msg, Message::SECTION_AUTHORITY, min_ttl, authority_count_);
         parseSection(msg, Message::SECTION_AUTHORITY, min_ttl, authority_count_);
     } else {
     } else {
         parseNegativeResponseAuthoritySection(msg, min_ttl, authority_count_);
         parseNegativeResponseAuthoritySection(msg, min_ttl, authority_count_);
     }
     }
     parseSection(msg, Message::SECTION_ADDITIONAL, min_ttl, additional_count_);
     parseSection(msg, Message::SECTION_ADDITIONAL, min_ttl, additional_count_);
 
 
+    // Limit the ttl to a prset max-value
+    if (!isNegativeResponse) {
+        if (min_ttl > MAX_NORMAL_CACHE_TTL) {
+            min_ttl = MAX_NORMAL_CACHE_TTL;
+        }
+    } else {
+        if (min_ttl > MAX_NEGATIVE_CACHE_TTL) {
+            min_ttl = MAX_NEGATIVE_CACHE_TTL;
+        }
+    }
+
     expire_time_ = time(NULL) + min_ttl;
     expire_time_ = time(NULL) + min_ttl;
 }
 }
 
 

+ 26 - 26
src/lib/cache/message_entry.h

@@ -21,36 +21,15 @@
 #include <dns/message.h>
 #include <dns/message.h>
 #include <dns/rrset.h>
 #include <dns/rrset.h>
 #include <nsas/nsas_entry.h>
 #include <nsas/nsas_entry.h>
+#include "rrset_cache.h"
 #include "rrset_entry.h"
 #include "rrset_entry.h"
 
 
-
 using namespace isc::nsas;
 using namespace isc::nsas;
 
 
 namespace isc {
 namespace isc {
 namespace cache {
 namespace cache {
 
 
 class RRsetEntry;
 class RRsetEntry;
-class RRsetCache;
-
-/// \brief Information to refer an RRset.
-///
-/// There is no class information here, since the rrsets are cached in
-/// the class-specific rrset cache.
-struct RRsetRef{
-    /// \brief Constructor
-    ///
-    /// \param name The Name for the RRset
-    /// \param type The RRType for the RRrset
-    /// \param cache Which cache the RRset is stored in
-    RRsetRef(const isc::dns::Name& name, const isc::dns::RRType& type,
-            boost::shared_ptr<RRsetCache> cache):
-            name_(name), type_(type), cache_(cache)
-    {}
-
-    isc::dns::Name name_; // Name of rrset.
-    isc::dns::RRType type_; // Type of rrset.
-    boost::shared_ptr<RRsetCache> cache_; //Which cache the RRset is stored
-};
 
 
 /// \brief Message Entry
 /// \brief Message Entry
 ///
 ///
@@ -61,6 +40,27 @@ class MessageEntry : public NsasEntry<MessageEntry> {
 private:
 private:
     MessageEntry(const MessageEntry& source);
     MessageEntry(const MessageEntry& source);
     MessageEntry& operator=(const MessageEntry& source);
     MessageEntry& operator=(const MessageEntry& source);
+
+    /// \brief Information to refer an RRset.
+    ///
+    /// There is no class information here, since the rrsets are cached in
+    /// the class-specific rrset cache.
+    struct RRsetRef{
+        /// \brief Constructor
+        ///
+        /// \param name The Name for the RRset
+        /// \param type The RRType for the RRrset
+        /// \param cache Which cache the RRset is stored in
+        RRsetRef(const isc::dns::Name& name, const isc::dns::RRType& type,
+                RRsetCache* cache):
+                name_(name), type_(type), cache_(cache)
+        {}
+
+        isc::dns::Name name_; // Name of rrset.
+        isc::dns::RRType type_; // Type of rrset.
+        RRsetCache* cache_; //Which cache the RRset is stored
+    };
+
 public:
 public:
 
 
     /// \brief Initialize the message entry object with one dns
     /// \brief Initialize the message entry object with one dns
@@ -75,8 +75,8 @@ public:
     ///        cache is used only for storing SOA rrset from negative
     ///        cache is used only for storing SOA rrset from negative
     ///        response (NXDOMAIN or NOERROR_NODATA)
     ///        response (NXDOMAIN or NOERROR_NODATA)
     MessageEntry(const isc::dns::Message& message,
     MessageEntry(const isc::dns::Message& message,
-                 boost::shared_ptr<RRsetCache> rrset_cache,
+                 const RRsetCachePtr& rrset_cache,
-                 boost::shared_ptr<RRsetCache> negative_soa_cache);
+                 const RRsetCachePtr& negative_soa_cache);
 
 
     /// \brief generate one dns message according
     /// \brief generate one dns message according
     ///        the rrsets information of the message.
     ///        the rrsets information of the message.
@@ -172,9 +172,9 @@ private:
     HashKey* hash_key_ptr_;  // the key for messag entry in hash table.
     HashKey* hash_key_ptr_;  // the key for messag entry in hash table.
 
 
     std::vector<RRsetRef> rrsets_;
     std::vector<RRsetRef> rrsets_;
-    boost::shared_ptr<RRsetCache> rrset_cache_; //Normal rrset cache
+    RRsetCachePtr rrset_cache_; //Normal rrset cache
     // SOA rrset from negative response
     // SOA rrset from negative response
-    boost::shared_ptr<RRsetCache> negative_soa_cache_;
+    RRsetCachePtr negative_soa_cache_;
 
 
     std::string query_name_; // query name of the message.
     std::string query_name_; // query name of the message.
     uint16_t query_class_; // query class of the message.
     uint16_t query_class_; // query class of the message.

+ 5 - 0
src/lib/cache/message_utility.cc

@@ -27,6 +27,11 @@ bool
 hasTheRecordInAuthoritySection(const isc::dns::Message& msg,
 hasTheRecordInAuthoritySection(const isc::dns::Message& msg,
                                const isc::dns::RRType& type)
                                const isc::dns::RRType& type)
 {
 {
+    // isc::dns::Message provide one function hasRRset() should be used to
+    // determine whether the given section has an RRset matching the given
+    // name and type, but currently it is not const-qualified and cannot be
+    // used here
+    // TODO: use hasRRset() function when it is const qualified
     for (RRsetIterator iter = msg.beginSection(Message::SECTION_AUTHORITY);
     for (RRsetIterator iter = msg.beginSection(Message::SECTION_AUTHORITY);
             iter != msg.endSection(Message::SECTION_AUTHORITY);
             iter != msg.endSection(Message::SECTION_AUTHORITY);
             ++iter) {
             ++iter) {

+ 10 - 0
src/lib/cache/message_utility.h

@@ -44,6 +44,16 @@ bool hasTheRecordInAuthoritySection(const isc::dns::Message& msg,
 bool isNegativeResponse(const isc::dns::Message& msg);
 bool isNegativeResponse(const isc::dns::Message& msg);
 
 
 /// \brief Check whether the message can be cached
 /// \brief Check whether the message can be cached
+///        Negative responses without SOA records SHOULD NOT be cached as there
+///        is no way to prevent the negative responses looping forever between a
+///        pair of servers even with a short TTL.
+///        Despite the DNS forming a tree of servers, with various mis-
+///        configurations it is possible to form a loop in the query graph, e.g.
+///        two servers listing each other as forwarders, various lame server
+///        configurations.  Without a TTL count down a cache negative response
+///        when received by the next server would have its TTL reset.  This
+///        negative indication could then live forever circulating between the
+///        servers involved. (Sec 5, RFC2308)
 ///
 ///
 /// \param msg The response message
 /// \param msg The response message
 bool canMessageBeCached(const isc::dns::Message& msg);
 bool canMessageBeCached(const isc::dns::Message& msg);

+ 4 - 4
src/lib/cache/resolver_cache.cc

@@ -44,7 +44,7 @@ ResolverClassCache::ResolverClassCache(const RRClass& cache_class) :
                                       negative_soa_cache_));
                                       negative_soa_cache_));
 }
 }
 
 
-ResolverClassCache::ResolverClassCache(CacheSizeInfo cache_info) :
+ResolverClassCache::ResolverClassCache(const CacheSizeInfo& cache_info) :
     cache_class_(cache_info.cclass)
     cache_class_(cache_info.cclass)
 {
 {
     uint16_t klass = cache_class_.getCode();
     uint16_t klass = cache_class_.getCode();
@@ -115,7 +115,7 @@ ResolverClassCache::update(const isc::dns::Message& msg) {
 }
 }
 
 
 bool
 bool
-ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr rrset_ptr,
+ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr& rrset_ptr,
                                 RRsetCachePtr rrset_cache_ptr)
                                 RRsetCachePtr rrset_cache_ptr)
 {
 {
     RRsetTrustLevel level;
     RRsetTrustLevel level;
@@ -131,7 +131,7 @@ ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr rrset_ptr,
 }
 }
 
 
 bool
 bool
-ResolverClassCache::update(const isc::dns::ConstRRsetPtr rrset_ptr) {
+ResolverClassCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
     // First update local zone, then update rrset cache.
     // First update local zone, then update rrset cache.
     local_zone_data_->update((*rrset_ptr.get()));
     local_zone_data_->update((*rrset_ptr.get()));
     updateRRsetCache(rrset_ptr, rrsets_cache_);
     updateRRsetCache(rrset_ptr, rrsets_cache_);
@@ -221,7 +221,7 @@ ResolverCache::update(const isc::dns::Message& msg) {
 }
 }
 
 
 bool
 bool
-ResolverCache::update(const isc::dns::ConstRRsetPtr rrset_ptr) {
+ResolverCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
     ResolverClassCache* cc = getClassCache(rrset_ptr->getClass());
     ResolverClassCache* cc = getClassCache(rrset_ptr->getClass());
     if (cc) {
     if (cc) {
         return (cc->update(rrset_ptr));
         return (cc->update(rrset_ptr));

+ 4 - 4
src/lib/cache/resolver_cache.h

@@ -90,7 +90,7 @@ public:
     /// \brief Construct Function.
     /// \brief Construct Function.
     /// \param caches_size cache size information for each
     /// \param caches_size cache size information for each
     ///        messages/rrsets of different classes.
     ///        messages/rrsets of different classes.
-    ResolverClassCache(CacheSizeInfo cache_info);
+    ResolverClassCache(const CacheSizeInfo& cache_info);
 
 
     /// \name Lookup Interfaces
     /// \name Lookup Interfaces
     //@{
     //@{
@@ -157,7 +157,7 @@ public:
     ///
     ///
     /// \note The class of the RRset must have been checked. It is not
     /// \note The class of the RRset must have been checked. It is not
     /// here.
     /// here.
-    bool update(const isc::dns::ConstRRsetPtr rrset_ptr);
+    bool update(const isc::dns::ConstRRsetPtr& rrset_ptr);
 
 
     /// \brief Get the RRClass this cache is for
     /// \brief Get the RRClass this cache is for
     ///
     ///
@@ -173,7 +173,7 @@ private:
     /// \return return true if the rrset is updated in the rrset cache,
     /// \return return true if the rrset is updated in the rrset cache,
     ///         or else return false if failed.
     ///         or else return false if failed.
     /// \param rrset_cache_ptr The rrset cache need to be updated.
     /// \param rrset_cache_ptr The rrset cache need to be updated.
-    bool updateRRsetCache(const isc::dns::ConstRRsetPtr rrset_ptr,
+    bool updateRRsetCache(const isc::dns::ConstRRsetPtr& rrset_ptr,
                           RRsetCachePtr rrset_cache_ptr);
                           RRsetCachePtr rrset_cache_ptr);
 
 
     /// \brief Class this cache is for.
     /// \brief Class this cache is for.
@@ -301,7 +301,7 @@ public:
     ///
     ///
     /// \overload
     /// \overload
     ///
     ///
-    bool update(const isc::dns::ConstRRsetPtr rrset_ptr);
+    bool update(const isc::dns::ConstRRsetPtr& rrset_ptr);
 
 
     /// \name Cache Serialization
     /// \name Cache Serialization
     //@{
     //@{

+ 3 - 3
src/lib/cache/tests/message_cache_unittest.cc

@@ -34,9 +34,9 @@ namespace {
 /// its internals.
 /// its internals.
 class DerivedMessageCache: public MessageCache {
 class DerivedMessageCache: public MessageCache {
 public:
 public:
-    DerivedMessageCache(boost::shared_ptr<RRsetCache> rrset_cache,
+    DerivedMessageCache(const RRsetCachePtr& rrset_cache,
                         uint32_t cache_size, uint16_t message_class,
                         uint32_t cache_size, uint16_t message_class,
-                        boost::shared_ptr<RRsetCache> negative_soa_cache):
+                        const RRsetCachePtr& negative_soa_cache):
         MessageCache(rrset_cache, cache_size, message_class, negative_soa_cache)
         MessageCache(rrset_cache, cache_size, message_class, negative_soa_cache)
     {}
     {}
 
 
@@ -95,7 +95,7 @@ TEST_F(MessageCacheTest, testUpdate) {
     EXPECT_TRUE(message_cache_->update(new_msg));
     EXPECT_TRUE(message_cache_->update(new_msg));
     Message new_msg_render(Message::RENDER);
     Message new_msg_render(Message::RENDER);
     EXPECT_TRUE(message_cache_->lookup(qname, RRType::SOA(), new_msg_render));
     EXPECT_TRUE(message_cache_->lookup(qname, RRType::SOA(), new_msg_render));
-    EXPECT_TRUE(new_msg_render.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_FALSE(new_msg_render.getHeaderFlag(Message::HEADERFLAG_AA));
 }
 }
 
 
 }   // namespace
 }   // namespace

+ 31 - 4
src/lib/cache/tests/message_entry_unittest.cc

@@ -39,8 +39,8 @@ namespace {
 class DerivedMessageEntry: public MessageEntry {
 class DerivedMessageEntry: public MessageEntry {
 public:
 public:
     DerivedMessageEntry(const isc::dns::Message& message,
     DerivedMessageEntry(const isc::dns::Message& message,
-                        boost::shared_ptr<RRsetCache> rrset_cache_,
+                        const RRsetCachePtr& rrset_cache_,
-                        boost::shared_ptr<RRsetCache> negative_soa_cache_):
+                        const RRsetCachePtr& negative_soa_cache_):
              MessageEntry(message, rrset_cache_, negative_soa_cache_)
              MessageEntry(message, rrset_cache_, negative_soa_cache_)
     {}
     {}
 
 
@@ -76,7 +76,6 @@ public:
                         message_parse(Message::PARSE),
                         message_parse(Message::PARSE),
                         message_render(Message::RENDER)
                         message_render(Message::RENDER)
     {
     {
-        
         rrset_cache_.reset(new RRsetCache(RRSET_CACHE_DEFAULT_SIZE, class_));
         rrset_cache_.reset(new RRsetCache(RRSET_CACHE_DEFAULT_SIZE, class_));
         negative_soa_cache_.reset(new RRsetCache(NEGATIVE_RRSET_CACHE_DEFAULT_SIZE, class_));
         negative_soa_cache_.reset(new RRsetCache(NEGATIVE_RRSET_CACHE_DEFAULT_SIZE, class_));
     }
     }
@@ -225,7 +224,7 @@ TEST_F(MessageEntryTest, testGenMessage) {
     message_entry.genMessage(time(NULL), msg);
     message_entry.genMessage(time(NULL), msg);
     // Check whether the generated message is same with cached one.
     // Check whether the generated message is same with cached one.
     
     
-    EXPECT_TRUE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
     EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_TC));
     EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_TC));
     EXPECT_EQ(1, sectionRRsetCount(msg, Message::SECTION_ANSWER)); 
     EXPECT_EQ(1, sectionRRsetCount(msg, Message::SECTION_ANSWER)); 
     EXPECT_EQ(1, sectionRRsetCount(msg, Message::SECTION_AUTHORITY)); 
     EXPECT_EQ(1, sectionRRsetCount(msg, Message::SECTION_AUTHORITY)); 
@@ -237,4 +236,32 @@ TEST_F(MessageEntryTest, testGenMessage) {
     EXPECT_EQ(7, msg.getRRCount(Message::SECTION_ADDITIONAL));
     EXPECT_EQ(7, msg.getRRCount(Message::SECTION_ADDITIONAL));
 }
 }
 
 
+TEST_F(MessageEntryTest, testMaxTTL) {
+    messageFromFile(message_parse, "message_large_ttl.wire");
+
+    // The ttl of rrset from Answer and Authority sections are both 604801 seconds
+    RRsetIterator iter = message_parse.beginSection(Message::SECTION_ANSWER);
+    EXPECT_EQ(604801, (*iter)->getTTL().getValue());
+    iter = message_parse.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_EQ(604801, (*iter)->getTTL().getValue());
+
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
+
+    // The ttl is limited to 604800 seconds (7days)
+    EXPECT_EQ(time(NULL) + 604800, message_entry.getExpireTime());
+}
+
+TEST_F(MessageEntryTest, testMaxNegativeTTL) {
+    messageFromFile(message_parse, "message_nxdomain_large_ttl.wire");
+
+    // The ttl of rrset Authority sections are 10801 seconds
+    RRsetIterator iter = message_parse.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_EQ(10801, (*iter)->getTTL().getValue());
+
+    DerivedMessageEntry message_entry(message_parse, rrset_cache_, negative_soa_cache_);
+
+    // The ttl is limited to 10800 seconds (3 hours)
+    EXPECT_EQ(time(NULL) + 10800, message_entry.getExpireTime());
+}
+
 }   // namespace
 }   // namespace

+ 46 - 4
src/lib/cache/tests/negative_cache_unittest.cc

@@ -77,6 +77,8 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
     const RRTTL& soa_ttl = rrset_ptr->getTTL();
     const RRTTL& soa_ttl = rrset_ptr->getTTL();
     EXPECT_EQ(soa_ttl.getValue(), 172800);
     EXPECT_EQ(soa_ttl.getValue(), 172800);
 
 
+    sleep(1);
+
     // Query nonexist.example.com again
     // Query nonexist.example.com again
     Message msg_nxdomain2(Message::PARSE);
     Message msg_nxdomain2(Message::PARSE);
     messageFromFile(msg_nxdomain2, "message_nxdomain_with_soa.wire");
     messageFromFile(msg_nxdomain2, "message_nxdomain_with_soa.wire");
@@ -88,7 +90,14 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
 
 
     // The TTL should equal to the TTL of negative response SOA record
     // The TTL should equal to the TTL of negative response SOA record
     const RRTTL& nxdomain_ttl2 = rrset_ptr->getTTL();
     const RRTTL& nxdomain_ttl2 = rrset_ptr->getTTL();
-    EXPECT_EQ(nxdomain_ttl2.getValue(), 86400);
+    EXPECT_TRUE(86398 <= nxdomain_ttl2.getValue() && nxdomain_ttl2.getValue() <= 86399);
+    // No RRset in ANSWER section
+    EXPECT_TRUE(msg_nxdomain2.getRRCount(Message::SECTION_ANSWER) == 0);
+    // Check that only one SOA record exist in AUTHORITY section
+    EXPECT_TRUE(msg_nxdomain2.getRRCount(Message::SECTION_AUTHORITY) == 1);
+    iter = msg_nxdomain2.beginSection(Message::SECTION_AUTHORITY);
+    rrset_ptr = *iter;
+    EXPECT_TRUE(rrset_ptr->getType() == RRType::SOA());
 
 
     // Check the normal SOA cache again
     // Check the normal SOA cache again
     Message msg_example_com_soa2(Message::PARSE);
     Message msg_example_com_soa2(Message::PARSE);
@@ -100,7 +109,7 @@ TEST_F(NegativeCacheTest, testNXDOMAIN){
     rrset_ptr = *iter;
     rrset_ptr = *iter;
     const RRTTL& soa_ttl2 = rrset_ptr->getTTL();
     const RRTTL& soa_ttl2 = rrset_ptr->getTTL();
     // The TTL should equal to the TTL of SOA record in answer section
     // The TTL should equal to the TTL of SOA record in answer section
-    EXPECT_EQ(soa_ttl2.getValue(), 172800);
+    EXPECT_TRUE(172798 <= soa_ttl2.getValue() && soa_ttl2.getValue() <= 172799);
 }
 }
 
 
 TEST_F(NegativeCacheTest, testNXDOMAINWithoutSOA){
 TEST_F(NegativeCacheTest, testNXDOMAINWithoutSOA){
@@ -131,6 +140,21 @@ TEST_F(NegativeCacheTest, testNXDOMAINCname){
     EXPECT_TRUE(cache->lookup(a_example_org, RRType::A(), RRClass::IN(), msg_nxdomain_cname));
     EXPECT_TRUE(cache->lookup(a_example_org, RRType::A(), RRClass::IN(), msg_nxdomain_cname));
 
 
     EXPECT_EQ(msg_nxdomain_cname.getRcode().getCode(), Rcode::NXDOMAIN().getCode());
     EXPECT_EQ(msg_nxdomain_cname.getRcode().getCode(), Rcode::NXDOMAIN().getCode());
+
+    // It should include 2 CNAME records in Answer section
+    EXPECT_TRUE(msg_nxdomain_cname.getRRCount(Message::SECTION_ANSWER) == 2);
+    RRsetIterator iter = msg_nxdomain_cname.beginSection(Message::SECTION_ANSWER);
+    EXPECT_TRUE((*iter)->getType() == RRType::CNAME());
+    ++iter;
+    EXPECT_TRUE((*iter)->getType() == RRType::CNAME());
+
+    // It should include 1 SOA record in Authority section
+    EXPECT_TRUE(msg_nxdomain_cname.getRRCount(Message::SECTION_AUTHORITY) == 1);
+    iter = msg_nxdomain_cname.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_TRUE((*iter)->getType() == RRType::SOA());
+
+    const RRTTL& soa_ttl = (*iter)->getTTL();
+    EXPECT_EQ(soa_ttl.getValue(), 600);
 }
 }
 
 
 TEST_F(NegativeCacheTest, testNoerrorNodata){
 TEST_F(NegativeCacheTest, testNoerrorNodata){
@@ -173,13 +197,21 @@ TEST_F(NegativeCacheTest, testNoerrorNodata){
     messageFromFile(msg_nodata2, "message_nodata_with_soa.wire");
     messageFromFile(msg_nodata2, "message_nodata_with_soa.wire");
     msg_nodata2.makeResponse();
     msg_nodata2.makeResponse();
 
 
+    sleep(1);
+
     EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(), msg_nodata2));
     EXPECT_TRUE(cache->lookup(example_dot_com, RRType::MX(), RRClass::IN(), msg_nodata2));
+
+    // No answer
+    EXPECT_EQ(msg_nodata2.getRRCount(Message::SECTION_ANSWER), 0);
+    // One SOA record in authority section
+    EXPECT_EQ(msg_nodata2.getRRCount(Message::SECTION_AUTHORITY), 1);
+
     iter = msg_nodata2.beginSection(Message::SECTION_AUTHORITY);
     iter = msg_nodata2.beginSection(Message::SECTION_AUTHORITY);
     rrset_ptr = *iter;
     rrset_ptr = *iter;
 
 
-    // The TTL should equal to the TTL of negative response SOA record
+    // The TTL should equal to the TTL of negative response SOA record and counted down
     const RRTTL& nodata_ttl2 = rrset_ptr->getTTL();
     const RRTTL& nodata_ttl2 = rrset_ptr->getTTL();
-    EXPECT_EQ(nodata_ttl2.getValue(), 86400);
+    EXPECT_TRUE(86398 <= nodata_ttl2.getValue() && nodata_ttl2.getValue() <= 86399);
 }
 }
 
 
 TEST_F(NegativeCacheTest, testReferralResponse){
 TEST_F(NegativeCacheTest, testReferralResponse){
@@ -195,6 +227,16 @@ TEST_F(NegativeCacheTest, testReferralResponse){
 
 
     // The Rcode should be NOERROR
     // The Rcode should be NOERROR
     EXPECT_EQ(msg_cname_referral.getRcode().getCode(), Rcode::NOERROR().getCode());
     EXPECT_EQ(msg_cname_referral.getRcode().getCode(), Rcode::NOERROR().getCode());
+
+    // One CNAME record in Answer section
+    EXPECT_EQ(msg_cname_referral.getRRCount(Message::SECTION_ANSWER), 1);
+    RRsetIterator iter = msg_cname_referral.beginSection(Message::SECTION_ANSWER);
+    EXPECT_EQ((*iter)->getType(), RRType::CNAME());
+
+    // 13 NS records in Authority section
+    EXPECT_EQ(msg_cname_referral.getRRCount(Message::SECTION_AUTHORITY), 13);
+    iter = msg_cname_referral.beginSection(Message::SECTION_AUTHORITY);
+    EXPECT_EQ((*iter)->getType(), RRType::NS());
 }
 }
 
 
 }
 }

+ 1 - 1
src/lib/cache/tests/resolver_cache_unittest.cc

@@ -53,7 +53,7 @@ TEST_F(ResolverCacheTest, testUpdateMessage) {
 
 
     msg.makeResponse();
     msg.makeResponse();
     EXPECT_TRUE(cache->lookup(qname, RRType::SOA(), RRClass::IN(), msg));
     EXPECT_TRUE(cache->lookup(qname, RRType::SOA(), RRClass::IN(), msg));
-    EXPECT_TRUE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
+    EXPECT_FALSE(msg.getHeaderFlag(Message::HEADERFLAG_AA));
 
 
     // Test whether the old message can be updated
     // Test whether the old message can be updated
     Message new_msg(Message::PARSE);
     Message new_msg(Message::PARSE);

+ 31 - 0
src/lib/cache/tests/testdata/message_large_ttl.wire

@@ -0,0 +1,31 @@
+#
+# A response that the TTL is quite large(> 7days)
+#
+##
+## header
+##
+# Transaction ID: 0x0d1f
+# Flags: 0x8580 (Standard query response, No error)
+0d1f 8580
+# Questions: 1
+# Answer RRs: 1
+# Authority RRs: 3
+# Additional RRs: 3
+00 01 00 01 00 01 00 00
+##
+## Query
+##
+# test.example.org: type A, class IN
+04 74 65 73 74 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 00 01 00 01
+##
+## Answer
+##
+# test.example.org: type A, class IN, addr 127.0.0.1
+# TTL: 7 days, 1 second (604801 seconds)
+c0 0c 00 01 00 01 00 09 3a 81 00 04 7f 00 00 01
+##
+## Authority
+##
+# example.org: type NS, class IN, ns ns1.example.org
+# TTL: 7 days, 1 second (604801 seconds)
+c0 11 00 02 00 01 00 09 3a 81 00 06 03 6e 73 31 c0 11

+ 25 - 0
src/lib/cache/tests/testdata/message_nxdomain_large_ttl.wire

@@ -0,0 +1,25 @@
+#
+# Negative response (NXDOMAIN) with large TTL (3hours + 1second)
+#
+##
+## Header
+##
+# Transaction ID: 0xb1fe
+# Flags: 0x8583 (Standard query response, No such name)
+b1fe 8583
+# Questions: 1
+# Authority RRs: 1
+00 01 00 00 00 01 00 00
+##
+## Query
+##
+# c.example.org: type A, class IN
+01 63 07 65 78 61 6d 70 6c 65 03 6f 72 67 00 00 01 00 01
+##
+## Authority
+##
+# example.org: type SOA, class IN, mname ns1.example.org
+# TTL: 3 Hourse, 1 second (10801seconds)
+c0 0e 00 06 00 01 00 00 2a 31 00 22 03 6e 73 31 c0
+0e 05 61 64 6d 69 6e c0 0e 00 00 04 d2 00 00 0e
+10 00 00 07 08 00 24 ea 00 00 00 2a 31