[tor-commits] [tor/master] Fix infinite recursion when connect() fails in microdesc consensus fetch

[nickm at torproject.org]

commit 22ec4d5426eecf1a8021d9b6b95f9a1518e96110
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Feb 23 12:32:15 2011 -0500

    Fix infinite recursion when connect() fails in microdesc consensus fetch
    
    The underlying fix is to stop indicating requests "ns" consensuses by
    putting NULL in their requested_resource field: we already had a
    specialized meaning for requested_resource==NULL, which was (more or
    less) "Treat a failure here as a network failure, since it's too early
    to possibly be a resource or directory failure."  Overloading the two
    meant that very early microdesc consensus download failures would get
    treated as ns consensus download failures, so the failure count there
    would get incremented, but the microdesc download would get retried
    immediately in an infinite loop.
    
    Fix for bug2381.  Diagnosed by mobmix.
---
 src/or/directory.c     |   18 +++++++++++-------
 src/or/networkstatus.c |    2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/or/directory.c b/src/or/directory.c
index 7137fe1..a53a0c6 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -631,9 +631,8 @@ connection_dir_request_failed(dir_connection_t *conn)
       connection_dir_bridge_routerdesc_failed(conn);
     connection_dir_download_routerdesc_failed(conn);
   } else if (conn->_base.purpose == DIR_PURPOSE_FETCH_CONSENSUS) {
-    const char *flavname =
-      conn->requested_resource ? conn->requested_resource : "ns";
-    networkstatus_consensus_download_failed(0, flavname);
+    if (conn->requested_resource)
+      networkstatus_consensus_download_failed(0, conn->requested_resource);
   } else if (conn->_base.purpose == DIR_PURPOSE_FETCH_CERTIFICATE) {
     log_info(LD_DIR, "Giving up on certificate fetch from directory server "
              "at '%s'; retrying",
@@ -1004,8 +1003,14 @@ directory_get_consensus_url(int supports_conditional_consensus,
                             const char *resource)
 {
   char *url = NULL;
-  const char *hyphen = resource ? "-" : "";
-  const char *flavor = resource ? resource : "";
+  const char *hyphen, *flavor;
+  if (resource==NULL || strcmp(resource, "ns")==0) {
+    flavor = ""; /* Request ns consensuses as "", so older servers will work*/
+    hyphen = "";
+  } else {
+    flavor = resource;
+    hyphen = "-";
+  }
 
   if (supports_conditional_consensus) {
     char *authority_id_list;
@@ -1746,8 +1751,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
 
   if (conn->_base.purpose == DIR_PURPOSE_FETCH_CONSENSUS) {
     int r;
-    const char *flavname =
-      conn->requested_resource ? conn->requested_resource : "ns";
+    const char *flavname = conn->requested_resource;
     if (status_code != 200) {
       int severity = (status_code == 304) ? LOG_INFO : LOG_WARN;
       log(severity, LD_DIR,
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index cccb1c0..6387126 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1217,7 +1217,7 @@ update_consensus_networkstatus_downloads(time_t now)
     if (! we_want_to_fetch_flavor(options, i))
       continue;
 
-    resource = i==FLAV_NS ? NULL : networkstatus_get_flavor_name(i);
+    resource = networkstatus_get_flavor_name(i);
 
     if (!download_status_is_ready(&consensus_dl_status[i], now,
                                   CONSENSUS_NETWORKSTATUS_MAX_DL_TRIES))