created arg recursion_count to async_resolve and many other to avoid infinite loops
authorChristian Herdtweck <christian.herdtweck@intra2net.com>
Mon, 4 May 2015 14:09:10 +0000 (16:09 +0200)
committerChristian Herdtweck <christian.herdtweck@intra2net.com>
Mon, 4 May 2015 14:57:59 +0000 (16:57 +0200)
* arg recursion_count replaces cname_count which up to now identified problems in post-processing
* created simple loop test
* added option to configure recursion depth limit to DnsMaster

minor other changes:
* corrected docu for resolved-ip-ttl-threshold option: is not milliseconds but seconds
* add test to ResolverBase: check number of callbacks and warn if suspiciously many

18 files changed:
src/config/option/resolvedipttlthresholdoption.cpp
src/dns/dnscache.cpp
src/dns/dnsmaster.cpp
src/dns/dnsmaster.h
src/dns/dnsresolver.cpp
src/dns/dnsresolver.h
src/dns/ippseudoresolver.h
src/dns/resolverbase.cpp
src/dns/resolverbase.h
src/host/pingscheduler.cpp
src/host/pingscheduler.h
src/main.cpp
test/data/icmp_destinationunreachable.pcap
test/data/icmp_echoreply.pcap
test/data/icmp_echorequest.pcap
test/data/icmp_timeexceeded.pcap
test/test_dns.cpp
test/test_icmppacket.cpp

index a8f9672..66809d5 100644 (file)
@@ -35,7 +35,7 @@ ResolvedIpTtlThresholdOption::ResolvedIpTtlThresholdOption() :
     ConfigurationOption(
         "resolved-ip-ttl-threshold",
         value<int>()->default_value( 10 ),
-        "Minimum time in milliseconds that IP has to be valid when testing for refresh"
+        "Minimum time in seconds that IP has to be valid when testing for refresh"
     )
 {
 }
index 4c524e7..b1c4f33 100644 (file)
@@ -46,7 +46,6 @@ using I2n::Logger::GlobalLogger;
 namespace Config
 {
     int SaveTimerSeconds = 60;
-    int MaxRetrievalRecursions = 10;
 }
 
 // -----------------------------------------------------------------------------
@@ -369,6 +368,8 @@ HostAddressVec DnsCache::get_ips_recursive(const std::string &hostname,
     HostAddressVec result = get_ips(current_host, check_up_to_date);
     int n_recursions = 0;
     uint32_t min_cname_ttl = 0xffff;   // largest possible unsigned 4-byte value
+    int max_recursion_count = DnsMaster::get_instance()
+                                       ->get_max_recursion_count();
     while ( result.empty() )
     {
         current_cname = get_cname(current_host, check_up_to_date);
@@ -376,7 +377,7 @@ HostAddressVec DnsCache::get_ips_recursive(const std::string &hostname,
             break;
 
         current_host = key_for_hostname(current_cname.Host);
-        if (++n_recursions >= Config::MaxRetrievalRecursions)
+        if (++n_recursions >= max_recursion_count)
         {
             GlobalLogger.warning() << "DnsCache: reached recursion limit of "
                 << n_recursions << " in recursive IP retrieval!";
@@ -426,9 +427,11 @@ std::string DnsCache::get_first_outdated_cname(const std::string &hostname,
     std::string first_outdated = hostname;
     Cname cname;
     int n_recursions = 0;
+    int max_recursion_count = DnsMaster::get_instance()
+                                       ->get_max_recursion_count();
     while (true)
     {
-        if (++n_recursions >= Config::MaxRetrievalRecursions)
+        if (++n_recursions >= max_recursion_count)
         {
             GlobalLogger.warning() << "DnsCache: reached recursion limit of "
                 << n_recursions << " in search of outdated CNAMEs!";
index 7235a1e..b0b4049 100644 (file)
@@ -41,6 +41,7 @@ void DnsMaster::create_master(const IoServiceItem &io_serv,
                             const int resolved_ip_ttl_threshold,
                             const int min_time_between_resolves,
                             const int max_address_resolution_attempts,
+                            const int max_recursion_count,
                             const std::string &cache_file)
 {
     GlobalLogger.info() << "DnsMaster: Creating cache";
@@ -54,6 +55,7 @@ void DnsMaster::create_master(const IoServiceItem &io_serv,
                   default_name_server,
                   resolved_ip_ttl_threshold,
                   max_address_resolution_attempts,
+                  max_recursion_count,
                   cache);
 }
 
@@ -61,6 +63,7 @@ void DnsMaster::create_master(const IoServiceItem &io_serv,
                             const boost::asio::ip::address &default_name_server,
                             const int resolved_ip_ttl_threshold,
                             const int max_address_resolution_attempts,
+                            const int max_recursion_count,
                             const DnsCacheItem &cache)
 {
     if (TheOnlyInstance)
@@ -75,6 +78,7 @@ void DnsMaster::create_master(const IoServiceItem &io_serv,
                                          default_name_server,
                                          resolved_ip_ttl_threshold,
                                          max_address_resolution_attempts,
+                                         max_recursion_count,
                                          cache)
                          );
 }
@@ -83,11 +87,13 @@ DnsMaster::DnsMaster(const IoServiceItem &io_serv,
                      const boost::asio::ip::address &default_name_server,
                      const int resolved_ip_ttl_threshold,
                      const int max_address_resolution_attempts,
+                     const int max_recursion_count,
                      const DnsCacheItem &cache)
     : IoService( io_serv )
     , DefaultNameServer( default_name_server )
     , ResolvedIpTtlThreshold( resolved_ip_ttl_threshold )
     , MaxAddressResolutionAttempts( max_address_resolution_attempts )
+    , MaxRecursionCount( max_recursion_count )
     , Cache(cache)
     , ResolverMap()
 {
@@ -221,6 +227,11 @@ int DnsMaster::get_max_address_resolution_attempts() const
     return MaxAddressResolutionAttempts;
 }
 
+int DnsMaster::get_max_recursion_count() const
+{
+    return MaxRecursionCount;
+}
+
 std::string to_string(const DnsIpProtocol &protocol)
 {
     switch (protocol)
index 424d8ba..3b52cdf 100644 (file)
@@ -95,6 +95,7 @@ private:
               const boost::asio::ip::address &default_name_server,
               const int resolved_ip_ttl_threshold,
               const int max_address_resolution_attempts,
+              const int max_recursion_count,
               const DnsCacheItem &cache);
 public:
     static void create_master(const IoServiceItem &io_serv,
@@ -102,11 +103,13 @@ public:
                             const int resolved_ip_ttl_threshold,
                             const int min_time_between_resolves,
                             const int max_address_resolution_attempts,
+                            const int max_recursion_count,
                             const std::string &cache_file);
     static void create_master(const IoServiceItem &io_serv,
                             const boost::asio::ip::address &default_name_server,
                             const int resolved_ip_ttl_threshold,
                             const int max_address_resolution_attempts,
+                            const int max_recursion_count,
                             const DnsCacheItem &cache);  // needed for unit test
     static DnsMasterItem& get_instance();
     ~DnsMaster();
@@ -116,6 +119,7 @@ public:
     //boost::asio::ip::address &get_default_name_server() const;  // unused
     int get_resolved_ip_ttl_threshold() const;
     int get_max_address_resolution_attempts() const;
+    int get_max_recursion_count() const;
 
 // variables
 private:
@@ -123,6 +127,7 @@ private:
     const boost::asio::ip::address DefaultNameServer;
     const int ResolvedIpTtlThreshold;
     const int MaxAddressResolutionAttempts;
+    const int MaxRecursionCount;
     DnsCacheItem Cache;
     resolver_map_type ResolverMap;
 
index 52c485c..634a439 100644 (file)
@@ -106,7 +106,7 @@ DnsResolver::~DnsResolver()
  * copied here code from boost::net::dns::resolve.hpp, since want async
  * operation and that is used only internally, there
  */
-void DnsResolver::do_resolve()
+void DnsResolver::do_resolve(const int recursion_count)
 {
     // check if resolving already
     if (IsResolving)
@@ -138,13 +138,14 @@ void DnsResolver::do_resolve()
     memcpy( &RequestId, message_id.data, sizeof(RequestId) );
     dns_message.id( RequestId );
     GlobalLogger.debug() << LogPrefix << "Request has ID "
-                               << std::showbase << std::hex << dns_message.id();
+                         << std::showbase << std::hex << dns_message.id();
 
     // setup receipt of reply
     Socket.async_receive_from(
             boost::asio::buffer(ReceiveBuffer.get_array()),
             NameServer,
             boost::bind( &DnsResolver::handle_dns_result, this,
+                         recursion_count,
                          boost::asio::placeholders::error,
                          boost::asio::placeholders::bytes_transferred)
     );
@@ -154,7 +155,8 @@ void DnsResolver::do_resolve()
                                         seconds(Config::ResolveTimeoutSeconds));
     ResolveTimeoutTimer.async_wait( boost::bind(
                                       &DnsResolver::handle_resolve_timeout,
-                                      this, boost::asio::placeholders::error) );
+                                      this, recursion_count,
+                                      boost::asio::placeholders::error) );
 
     // send dns request
     dns_message.encode(RequestBuffer);
@@ -170,27 +172,28 @@ void DnsResolver::do_resolve()
         GlobalLogger.warning() << LogPrefix
                                << "Sending of DNS request message failed: "
                                << err.what();
-        schedule_retry();
+        schedule_retry(recursion_count);
         return;
     }
 
     if ( bytes_sent == 0 )
     {
         GlobalLogger.warning() << LogPrefix << "Empty DNS request sent!";
-        schedule_retry();
+        schedule_retry(recursion_count);
         return;
     }
 }
 
 
-void DnsResolver::handle_dns_result(const boost::system::error_code &error,
+void DnsResolver::handle_dns_result(const int recursion_count,
+                                    const boost::system::error_code &error,
                                     const std::size_t bytes_transferred)
 {
     if (error)
     {
         GlobalLogger.info() << LogPrefix << "DNS resolve resulted in error "
                             << error << " --> request retry";
-        schedule_retry();
+        schedule_retry(recursion_count);
         return;
     }
     else if ( OperationCancelled )
@@ -230,25 +233,25 @@ void DnsResolver::handle_dns_result(const boost::system::error_code &error,
     GlobalLogger.debug() << LogPrefix <<"Checking ANSWERS section of dns reply";
     gather_results(result_message.answers(), &result_ips, &result_cnames,
                                                           &result_name_servers);
-    // results should have the logical order
-    // Hostname [ --> cname1 --> cname2 --> ... --> cnameN ] [ --> ips ]
-    // for cname count to be correct; rest of code should not be affected if not
 
     // remember cname list (if there were any)
+    // results should have the logical order
+    // Hostname [ --> cname1 --> cname2 --> ... --> cnameN ] [ --> ips ];
+    // otherwise just have unneccessary cnames in cache
     BOOST_FOREACH( const src_cname_pair &host_and_cname, result_cnames )
         ResolverBase::update_cache(host_and_cname.first, host_and_cname.second);
 
     if ( !result_ips.empty() )
-        handle_ips( result_ips );
+        handle_ips( recursion_count, result_ips );
     else if ( !result_cnames.empty() )
         // no IPs but at least one cname --> find the "last" cname and
         // re-start resolving with that
-        handle_cname(result_cnames);
+        handle_cname(recursion_count, result_cnames);
     else
     {   // no answers --> cannot proceed
         GlobalLogger.warning() << LogPrefix << "No IP nor CNAME received! "
                                << "--> request retry";
-        schedule_retry();
+        schedule_retry(recursion_count);
     }
 }
 
@@ -257,7 +260,7 @@ void DnsResolver::handle_dns_result(const boost::system::error_code &error,
  *
  * can be run on anwers(), autorities() and additional() sections of dns reply
  * messages
- * 
+ *
  * @param rr_list: input list of resource records
  * @param result_ips: output vector of ips
  * @param result_cnames: output vector of cnames
@@ -354,27 +357,29 @@ void DnsResolver::gather_results(const boost::net::dns::rr_list_t *rr_list,
 }
 
 
-void DnsResolver::handle_unavailable()
+void DnsResolver::handle_unavailable(const int recursion_count)
 {
     // schedule new attempt in quite a while
     StaleDataLongtermTimer.expires_from_now(
                                      seconds(Config::StaleDataLongtermSeconds));
     StaleDataLongtermTimer.async_wait(
-            boost::bind( &DnsResolver::wait_timer_timeout_handler,
-                         this, boost::asio::placeholders::error
+            boost::bind( &DnsResolver::wait_timer_timeout_handler, this,
+                         recursion_count,
+                         boost::asio::placeholders::error
             )
     );
     LongtermTimerIsActive = true;
 
     // for now, admit failure
     bool was_success = false;
-    finalize_resolve(was_success);
+    finalize_resolve(was_success, recursion_count);
 }
 
 
-void DnsResolver::handle_ips(const std::vector<host_addr_pair> &result_ips)
+void DnsResolver::handle_ips(const int recursion_count,
+                             const std::vector<host_addr_pair> &result_ips)
 {
-    // received at least one IP which could be for the queried host name 
+    // received at least one IP which could be for the queried host name
     // or the cname at the "end" of the cname list;
     // but all IPs should be for the same
     HostAddressVec addr_list;
@@ -398,16 +403,18 @@ void DnsResolver::handle_ips(const std::vector<host_addr_pair> &result_ips)
 
     // clean up
     bool was_success = true;
-    finalize_resolve(was_success);
+    finalize_resolve(was_success, recursion_count);
 }
 
 
-void DnsResolver::handle_cname(const std::vector<src_cname_pair> &result_cnames)
+void DnsResolver::handle_cname(const int recursion_count,
+                               const std::vector<src_cname_pair> &result_cnames)
 {
     // find the "last" cname in the list
     // Hostname --> cname1 --> cname2 --> ... --> cnameN
     // We assume here that this list might not be in order but that all cnames
-    //   form a single list (form one connected list and not several isolated)
+    //   form a single list (form one connected list and not several independent
+    //   lists)
 
     std::string last_cname = "";
     bool could_be_last;
@@ -438,7 +445,7 @@ void DnsResolver::handle_cname(const std::vector<src_cname_pair> &result_cnames)
         BOOST_FOREACH( const src_cname_pair &host_and_cname, result_cnames )
             GlobalLogger.info() << LogPrefix << host_and_cname.first << " --> "
                                              << host_and_cname.second.Host;
-        handle_unavailable();
+        handle_unavailable(recursion_count);
     }
     else
     {   // check cache for IP for this cname
@@ -448,8 +455,7 @@ void DnsResolver::handle_cname(const std::vector<src_cname_pair> &result_cnames)
         if ( !cached_data.empty() )
         {
             bool was_success = true;
-            int cname_count = 1;  // define cache access as only 1
-            finalize_resolve(was_success, cname_count);
+            finalize_resolve(was_success, recursion_count+1);
         }
         else
         {   // get resolver for canonical name
@@ -458,7 +464,7 @@ void DnsResolver::handle_cname(const std::vector<src_cname_pair> &result_cnames)
             callback_type callback = boost::bind(
                                            &DnsResolver::cname_resolve_callback,
                                            this, _1, _2 );
-            resolver->async_resolve( callback );
+            resolver->async_resolve( callback, recursion_count+1 );
 
             // treat a CNAME as a partial result: not enough to run callbacks
             // from finalize_resolve, but enough to stop timers and reset
@@ -469,8 +475,12 @@ void DnsResolver::handle_cname(const std::vector<src_cname_pair> &result_cnames)
 }
 
 
+/**
+ * the recursion_count here is really the one from the recursion, not the one
+ * forwarded from async_resolve!
+ */
 void DnsResolver::cname_resolve_callback(const bool was_success,
-                                         const int cname_count)
+                                         const int recursion_count)
 {
     if ( OperationCancelled )
     {   // async_resolve was cancelled --> callbacks already called
@@ -480,22 +490,35 @@ void DnsResolver::cname_resolve_callback(const bool was_success,
     }
     else if (was_success)
     {
-        GlobalLogger.debug() << LogPrefix << "CNAME resolution succeeded";
-        finalize_resolve(was_success, cname_count+1);
+        GlobalLogger.debug() << LogPrefix << "CNAME resolution succeeded after "
+                             << recursion_count << " recursions";
+        finalize_resolve(was_success, recursion_count);
     }
     else
     {
-        GlobalLogger.info() << LogPrefix << "CNAME resolution failed";
+        GlobalLogger.info() << LogPrefix << "CNAME resolution failed after "
+                             << recursion_count << " recursions";
         // no use to schedule retry in this case since cname resolver must have
         // failed several times and we can only re-start the same procedure with
         // the same information. But can re-try later
-        handle_unavailable();
+        handle_unavailable(recursion_count);
     }
 }
 
 
+/**
+ * @brief always called at end of resolving process
+ *
+ * runs callbacks, resets timers and checks state consistency; only thing that
+ * is "left alive" is the long-term timer that might cause a re-start of
+ * resolution after a while
+ *
+ * @param was_success: indicates whether resolution was successfull
+ * @param recursion_count number of recursions or (if not successfull) negative
+ *    value indicating who called this function
+ */
 void DnsResolver::finalize_resolve(const bool was_success,
-                                   const int cname_count)
+                                   const int recursion_count)
 {
     // some consistency checks; failure might indicate a situation I had not
     // anticipated during programming but might not be harmfull yet
@@ -516,12 +539,12 @@ void DnsResolver::finalize_resolve(const bool was_success,
     stop_trying(was_success);
 
     // schedule callbacks, clearing callback list
-    ResolverBase::schedule_callbacks(was_success, cname_count);
+    ResolverBase::schedule_callbacks(was_success, recursion_count);
 
     // finalize
     GlobalLogger.notice() << LogPrefix << "finalized resolve"
                           << " with success = " << was_success
-                          << " and cname_count = " << cname_count;
+                          << " and recursion_count = " << recursion_count;
     IsResolving = false;
 }
 
@@ -554,7 +577,7 @@ void DnsResolver::stop_trying(bool was_success)
  * Is true from call to async_resolve until callbacks
  * --> returns true if waiting for result or (short-term) retry
  *
- * However, does NOT tell you if the (long-term) stale timeout is active! 
+ * However, does NOT tell you if the (long-term) stale timeout is active!
  *   That timer has no effect on result, need to check is_waiting_to_resolve
  *   for that
  */
@@ -566,7 +589,7 @@ bool DnsResolver::is_resolving() const
 
 /**
  * returns true if either is_resolving or the long-term timer is active
- * 
+ *
  * is_resolving returns true if the short-term retry timer is active
  */
 bool DnsResolver::is_waiting_to_resolve() const
@@ -606,22 +629,23 @@ void DnsResolver::cancel_resolve()
     if ( IsResolving )
     {
         bool was_success = false;
-        int cname_count = 1;
-        finalize_resolve(was_success, cname_count);
+        int recursion_count = -1;
+        finalize_resolve(was_success, recursion_count);
     }
 
     // also cancel the long-term timer
     StaleDataLongtermTimer.cancel();
     LongtermTimerIsActive = false;
 
-    // set after finalize_resolve, so can check in finalize_resolve that 
+    // set after finalize_resolve, so can check in finalize_resolve that
     // OperationCancelled is never true
     OperationCancelled = true;
 
 }
 
 
-void DnsResolver::handle_resolve_timeout(const boost::system::error_code &error)
+void DnsResolver::handle_resolve_timeout(const int recursion_count,
+                                         const boost::system::error_code &error)
 {
     if ( error ==  boost::asio::error::operation_aborted )   // cancelled
     {
@@ -634,7 +658,7 @@ void DnsResolver::handle_resolve_timeout(const boost::system::error_code &error)
         GlobalLogger.warning() << LogPrefix
                                << "resolve timeout handler received error "
                                << error << " --> request retry";
-        schedule_retry();
+        schedule_retry(recursion_count);
     }
     else if ( OperationCancelled )
     {   // async_resolve was cancelled --> callbacks already called
@@ -645,12 +669,12 @@ void DnsResolver::handle_resolve_timeout(const boost::system::error_code &error)
     else
     {
         GlobalLogger.notice() << LogPrefix << "DNS resolving timed out";
-        schedule_retry();
+        schedule_retry(recursion_count);
     }
 }
 
 
-void DnsResolver::schedule_retry()
+void DnsResolver::schedule_retry(const int recursion_count)
 {
     // cancel timers
     ResolveTimeoutTimer.cancel();
@@ -664,8 +688,8 @@ void DnsResolver::schedule_retry()
     {   // too many re-tries
         GlobalLogger.info() << LogPrefix << "Not scheduling a retry since "
                             << "RetryCount " << RetryCount << " too high";
-        handle_unavailable();   // will call stop_trying i.e. reset RetryCount
-    }
+        handle_unavailable(recursion_count);   // will call stop_trying
+    }                                          // --> reset RetryCount
     else
     {   // schedule retry
         GlobalLogger.info() << LogPrefix << "Scheduling a retry (RetryCount="
@@ -674,11 +698,12 @@ void DnsResolver::schedule_retry()
                 seconds(Config::PauseBeforeRetrySeconds));
         PauseBeforeRetryTimer.async_wait(
                 boost::bind( &DnsResolver::wait_timer_timeout_handler,
-                             this, boost::asio::placeholders::error) );
+                             this, recursion_count,
+                             boost::asio::placeholders::error) );
     }
 }
 
-void DnsResolver::wait_timer_timeout_handler(
+void DnsResolver::wait_timer_timeout_handler( const int recursion_count,
                                          const boost::system::error_code &error)
 {
     if ( error ==  boost::asio::error::operation_aborted )   // cancelled
@@ -694,7 +719,7 @@ void DnsResolver::wait_timer_timeout_handler(
                                << "resolve wait handler received error "
                                << error << "! Try to finalize resolve";
         bool was_success = false;
-        finalize_resolve(was_success);
+        finalize_resolve(was_success, recursion_count);
     }
     else if ( OperationCancelled )
     {   // async_resolve was cancelled --> callbacks already called
@@ -706,7 +731,7 @@ void DnsResolver::wait_timer_timeout_handler(
     {
         GlobalLogger.info() << LogPrefix << "Done waiting --> re-try resolve";
         IsResolving = false;  // will be set to true immediately in do_resolve
-        do_resolve();
+        do_resolve(recursion_count);
     }
 }
 
index 0918fd0..f2d45c3 100644 (file)
@@ -70,22 +70,27 @@ public:
 
 // implementation of ResolverBase::async_resolve
 protected:
-    void do_resolve();
+    void do_resolve(const int recursion_count);
 
 // internal helper functions
 private:
-    void handle_resolve_timeout(const boost::system::error_code &error);
-    void handle_dns_result(const boost::system::error_code &error,
+    void handle_resolve_timeout(const int recursion_count,
+                                const boost::system::error_code &error);
+    void handle_dns_result(const int recursion_count,
+                           const boost::system::error_code &error,
                            const std::size_t bytes_transferred);
-    void handle_unavailable();
-    void handle_ips(const std::vector<host_addr_pair> &result_ips);
-    void handle_cname(const std::vector<src_cname_pair> &result_cnames);
+    void handle_unavailable(const int recursion_count);
+    void handle_ips(const int recursion_count,
+                    const std::vector<host_addr_pair> &result_ips);
+    void handle_cname(const int recursion_count,
+                      const std::vector<src_cname_pair> &result_cnames);
     void cname_resolve_callback(const bool was_success,
-                                const int cname_count);
-    void schedule_retry();
-    void finalize_resolve(const bool success, const int cname_count=0);
+                                const int recursion_count);
+    void schedule_retry(const int recursion_count);
+    void finalize_resolve(const bool success, const int recursion_count);
     void stop_trying(const bool was_success);
-    void wait_timer_timeout_handler(const boost::system::error_code &error);
+    void wait_timer_timeout_handler(const int recursion_count,
+                                    const boost::system::error_code &error);
     void gather_results( const boost::net::dns::rr_list_t *rr_list,
                          std::vector<host_addr_pair> *result_ips,
                          std::vector<src_cname_pair> *result_cnames,
index eeac609..a256a06 100644 (file)
@@ -74,7 +74,7 @@ public:
 
 // implementation of ResolverBase::async_resolve
 protected:
-    void do_resolve()
+    void do_resolve(const int recursion_count)
     {   ResolverBase::schedule_callbacks(true, 0);  }
 };
 #endif
index 9f36e68..362b3b9 100644 (file)
 
 #include <boost/bind.hpp>
 
+#include <logfunc.hpp>
+
+using I2n::Logger::GlobalLogger;
+
 
 ResolverBase::~ResolverBase()
 { }
@@ -32,15 +36,32 @@ ResolverBase::~ResolverBase()
 /**
  * callbacks should be of type
  * void resolve_callback(const bool was_success,
- *                       const int cname_count)
+ *                       const int recursion_count)
  */
-void ResolverBase::async_resolve(const callback_type &callback)
+void ResolverBase::async_resolve(const callback_type &callback,
+                                 const int recursion_count)
 {
-    // remember callback
-    CallbackList.push(callback);
+    // limit depth of recursion
+    if (recursion_count >= DnsMaster::get_instance()->get_max_recursion_count())
+    {   // if recursed too much, schedule callback with was_success = false
+        bool was_success = false;
+        IoService->post( boost::bind(callback, was_success, recursion_count) );
+    }
+    else
+    {
+        // remember callback
+        CallbackList.push(callback);
+
+        // check for trouble
+        if (CallbackList.size() > 10)
+            GlobalLogger.warning() << "ResolverBase: have "
+                << CallbackList.size() << " callback[s] listed for " << Hostname
+                << "!";
+
+        // let subclass do the resolving
+        do_resolve(recursion_count);
+    }
 
-    // let subclass do the resolving
-    do_resolve();
 }
 
 ResolverBase::ResolverBase(const IoServiceItem &io_serv,
@@ -92,14 +113,13 @@ HostAddressVec ResolverBase::get_cached_ips_recursively(const std::string host,
 }
 
 void ResolverBase::schedule_callbacks(const bool was_success,
-                                      const int cname_count)
+                                      const int recursion_count)
 {
     while ( !CallbackList.empty() )
     {
         callback_type callback = CallbackList.front();
         CallbackList.pop();
-        IoService->post( boost::bind( callback,
-                                      was_success, cname_count ) );
+        IoService->post( boost::bind(callback, was_success, recursion_count) );
     }
 }
 
index 8262124..5e1fb6f 100644 (file)
@@ -49,9 +49,10 @@ public:
     /**
      * callbacks should be of type
      * void resolve_callback(const bool was_success,
-     *                       const int cname_count)
+     *                       const int recursion_count)
      */
-    void async_resolve(const callback_type &callback);
+    void async_resolve(const callback_type &callback,
+                       const int recursion_count=0);
     virtual void cancel_resolve() = 0;
     virtual bool is_resolving() const = 0;
     virtual bool is_waiting_to_resolve() const = 0;
@@ -78,7 +79,9 @@ protected:
 
 // functions for subclasses
 protected:
-    virtual void do_resolve() = 0;
+    // recursion_count is guaranteed to be below
+    // DnsMaster::get_max_recursion_count
+    virtual void do_resolve(const int recursion_count) = 0;
 
     // convenient forwards to DnsCache that just insert Hostname
     void update_cache( const HostAddressVec &new_ips ) const;
@@ -92,7 +95,7 @@ protected:
                                        const bool check_up_to_date=false) const;
 
     void schedule_callbacks(const bool was_success,
-                            const int cname_count);
+                            const int recursion_count);
 
 };
 
index 430b5e0..b72dcba 100644 (file)
@@ -410,11 +410,11 @@ void PingScheduler::start_resolving_ping_address()
 }
 
 void PingScheduler::dns_resolve_callback(const bool was_success,
-                                         const int cname_count)
+                                         const int recursion_count)
 {
     GlobalLogger.info() << LogPrefix << "dns resolution finished "
                         << "with success = " << was_success << " "
-                        << "and cname_count = " << cname_count;
+                        << "after " << recursion_count << " recursions";
 
     if ( was_success )
     {
index 0de5711..106f69c 100644 (file)
@@ -82,7 +82,8 @@ private:
     void update_dns_resolver( PingProtocol current_protocol );
 
     void ping_when_ready();
-    void dns_resolve_callback(const bool was_success, const int cname_count);
+    void dns_resolve_callback(const bool was_success,
+                              const int recursion_count);
     void start_resolving_ping_address();
 
     void update_log_prefix();
index 5fffb50..d080334 100644 (file)
@@ -513,12 +513,14 @@ int main( int argc, const char *argv[] )
                                     boost::asio::ip::address::from_string(
                                             configuration->get_nameserver() );
 
+        int max_recursion_count = 10;
         DnsMaster::create_master(
                            io_service,
                            name_server_ip,
                            configuration->get_resolved_ip_ttl_threshold(),
                            configuration->get_min_time_between_resolves(),
                            configuration->get_max_address_resolution_attempts(),
+                           max_recursion_count,
                            configuration->get_dns_cache_file() );
 
         init_pingers( io_service, configuration, status_notifier, &scheduler_list );
index 550a6e2..b1309f6 100644 (file)
Binary files a/test/data/icmp_destinationunreachable.pcap and b/test/data/icmp_destinationunreachable.pcap differ
index 78167f0..8dd7819 100644 (file)
Binary files a/test/data/icmp_echoreply.pcap and b/test/data/icmp_echoreply.pcap differ
index bd34339..765cd35 100644 (file)
Binary files a/test/data/icmp_echorequest.pcap and b/test/data/icmp_echorequest.pcap differ
index b20f769..2f60c50 100644 (file)
Binary files a/test/data/icmp_timeexceeded.pcap and b/test/data/icmp_timeexceeded.pcap differ
index 8273aee..11e7e9d 100644 (file)
@@ -39,17 +39,18 @@ on this file might be covered by the GNU General Public License.
 
 using boost::asio::ip::address;
 
+// constants for master
+address name_server = address::from_string("127.0.0.1");
+const int resolved_ip_ttl_threshold = 5;      // this should be greater than...
+const uint32_t min_time_between_resolves = 3; // ... this!
+const int max_address_resolution_attempts = 2;
+const int max_recursion_count = 10;
+const std::string cache_file = DnsCache::DoNotUseCacheFile;
+
 //------------------------------------------------------------------------------
 // Global Test fixture (created once for test suite)
 //------------------------------------------------------------------------------
 
-// constants for master
-address name_server = address::from_string("127.0.0.1");
-int resolved_ip_ttl_threshold = 5;
-uint32_t min_time_between_resolves = 3; // should be < resolved_ip_ttl_threshold
-int max_address_resolution_attempts = 2;
-std::string cache_file = DnsCache::DoNotUseCacheFile;
-
 // rely on boost that there will only be one instance
 struct GlobalFixture;
 GlobalFixture *global_fixture;
@@ -87,6 +88,7 @@ struct GlobalFixture
                                  name_server,
                                  resolved_ip_ttl_threshold,
                                  max_address_resolution_attempts,
+                                 max_recursion_count,
                                  Cache);
         Master = DnsMaster::get_instance();
 
@@ -162,6 +164,13 @@ struct GlobalFixture
                       Cname("host_ttl_below_thresh.test", 2) );
         }
 
+        {   // create a CNAME loop  cname_loop1.test --> cname_loop2.test
+            //                  --> cname_loop3.test --> cname_loop1.test
+            Cache->update( "cname_loop1.test", Cname("cname_loop2.test", 60) );
+            Cache->update( "cname_loop2.test", Cname("cname_loop3.test", 60) );
+            Cache->update( "cname_loop3.test", Cname("cname_loop1.test", 60) );
+        }
+
         BOOST_TEST_MESSAGE( "Done filling cache." );
     }
 
@@ -390,7 +399,6 @@ BOOST_AUTO_TEST_CASE( cache_outdated_test )
 
 BOOST_AUTO_TEST_CASE( cache_ttl_below_thresh_test )
 {
-    // in fill_cache, 
     HostAddressVec ips = Cache->get_ips("host_ttl_below_thresh.test", false);
     BOOST_REQUIRE_EQUAL( ips.size(), 1 );
     HostAddress ip = ips.front();
@@ -400,6 +408,14 @@ BOOST_AUTO_TEST_CASE( cache_ttl_below_thresh_test )
     BOOST_CHECK_EQUAL( cname.Ttl.get_value(), min_time_between_resolves );
 }
 
+BOOST_AUTO_TEST_CASE( cache_retrieve_loop )
+{
+    BOOST_CHECK_EQUAL( Cache->get_ips_recursive("cname_loop1.test").size(), 0 );
+    BOOST_CHECK_EQUAL( Cache->get_ips_recursive("cname_loop2.test").size(), 0 );
+    BOOST_CHECK_EQUAL( Cache->get_ips_recursive("cname_loop3.test").size(), 0 );
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()    // of TestDnsCache
 
 
@@ -432,16 +448,35 @@ BOOST_AUTO_TEST_CASE( create_resolver_v4 )
 }
 
 
-void resolve_callback(IoServiceItem io_serv, ResolverItem resolver,
-                      const bool was_success, const int cname_count)
+struct ResolveCallback
 {
-    resolver->cancel_resolve();
-    io_serv->stop();
-    BOOST_TEST_MESSAGE( "Stopped io_service" );
-    BOOST_CHECK( was_success );
-    BOOST_CHECK_EQUAL( cname_count, 0 );
-}
+public:
+    IoServiceItem IoService;
+    ResolverItem Resolver;
+    int CallCount;
+    bool LastCallWasSuccess;
+    int LastCallRecursionCount;
+
+    ResolveCallback(IoServiceItem &io_serv, ResolverItem &resolver)
+        : IoService(io_serv)
+        , Resolver(resolver)
+        , CallCount(0)
+        , LastCallWasSuccess(true)
+        , LastCallRecursionCount(-10)
+    {}
+
+    void call(const bool was_success, const int recursion_count)
+    {
+        BOOST_TEST_MESSAGE( "Callback called" );
+        ++CallCount;
 
+        Resolver->cancel_resolve();
+        IoService->stop();
+
+        LastCallWasSuccess = was_success;
+        LastCallRecursionCount = recursion_count;
+    }
+};
 
 BOOST_AUTO_TEST_CASE( do_resolve )
 {
@@ -453,16 +488,26 @@ BOOST_AUTO_TEST_CASE( do_resolve )
     BOOST_CHECK( !resolver->is_waiting_to_resolve() );
 
     // set callback
-    callback_type callback = boost::bind( resolve_callback, IoService, resolver,
+    ResolveCallback *callback_obj = new ResolveCallback(IoService, resolver);
+    // have to manually delete this obj in the end!
+    callback_type callback = boost::bind( &ResolveCallback::call, callback_obj,
                                           _1, _2 );
+
     // start resolving
     resolver->async_resolve(callback);
     BOOST_CHECK( resolver->is_resolving() );
     IoService->run();
     // this will block until io service is stopped in resolve_callback
+    BOOST_TEST_MESSAGE( "Back to synchronous operation" );
+    IoService->reset();
 
     // check for result
+    BOOST_CHECK_EQUAL( callback_obj->CallCount, 1 );
+    BOOST_CHECK( callback_obj->LastCallWasSuccess );
+    BOOST_CHECK_EQUAL( callback_obj->LastCallRecursionCount, 0 );
     BOOST_CHECK( resolver->have_up_to_date_ip() );
+
+    delete callback_obj;
 }
 
 BOOST_AUTO_TEST_SUITE_END()   // of TestDnsResolver
index a8ea85d..0638e5f 100644 (file)
@@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE( EchoRequest )
     BOOST_REQUIRE( packet );
     BOOST_CHECK_EQUAL( packet->get_type_v4(), Icmpv4Type_EchoRequest );
     BOOST_CHECK_EQUAL( packet->get_source_address().to_string(),
-                       "172.16.1.141");
+                       "11.22.33.44");
     BOOST_CHECK_EQUAL( packet->get_destination_address().to_string(),
                        "38.100.128.10");
     BOOST_CHECK_EQUAL( packet->get_icmp_code(), 0 );
@@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE( EchoReply )
     BOOST_CHECK_EQUAL( packet->get_source_address().to_string(),
                        "38.100.128.10");
     BOOST_CHECK_EQUAL( packet->get_destination_address().to_string(),
-                       "172.16.1.141");
+                       "11.22.33.44");
     BOOST_CHECK_EQUAL( packet->get_icmp_code(), 0 );
     BOOST_CHECK_EQUAL( packet->get_icmp_size(), 20 );
     BOOST_CHECK( packet->match_echo_reply(0xaf36, 1,
@@ -122,7 +122,7 @@ BOOST_AUTO_TEST_CASE( TimeExceeded )
     BOOST_CHECK_EQUAL( packet->get_source_address().to_string(),
                        "193.186.7.1");
     BOOST_CHECK_EQUAL( packet->get_destination_address().to_string(),
-                       "172.16.1.141");
+                       "11.22.33.44");
     BOOST_CHECK_EQUAL( packet->get_icmp_code(), 0 );
     BOOST_CHECK_EQUAL( packet->get_icmp_size(), 48 );
     BOOST_CHECK( packet->match_time_exceeded(0x4ae3, 1,
@@ -138,9 +138,9 @@ BOOST_AUTO_TEST_CASE( DestinationUnreachable )
     BOOST_REQUIRE( packet );
     BOOST_CHECK_EQUAL( packet->get_type_v4(), Icmpv4Type_DestinationUnreachable );
     BOOST_CHECK_EQUAL( packet->get_source_address().to_string(),
-                       "172.16.1.254");
+                       "11.22.33.254");
     BOOST_CHECK_EQUAL( packet->get_destination_address().to_string(),
-                       "172.16.1.141");
+                       "11.22.33.44");
     BOOST_CHECK_EQUAL( packet->get_icmp_code(), 3 );
     BOOST_CHECK_EQUAL( packet->get_icmp_size(), 92 );
     BOOST_CHECK( packet->match_destination_unreachable(0x077b, 1,