Merge remote-tracking branch 'public/ticket9969'

Conflicts:
	src/or/directory.c
	src/or/routerlist.c
	src/or/routerlist.h
	src/test/include.am
	src/test/test.c

src/or/directory.c

@@ -64,8 +64,6 @@ static void directory_send_command(dir_connection_t *conn,
                              time_t if_modified_since);
 static int directory_handle_command(dir_connection_t *conn);
 static int body_is_plausible(const char *body, size_t body_len, int purpose);
-static int purpose_needs_anonymity(uint8_t dir_purpose,
-                                   uint8_t router_purpose);
 static char *http_get_header(const char *headers, const char *which);
 static void http_set_address_origin(const char *headers, connection_t *conn);
 static void connection_dir_download_routerdesc_failed(dir_connection_t *conn);
@@ -120,7 +118,7 @@ static void directory_initiate_command_rend(const tor_addr_t *addr,
 /** Return true iff the directory purpose <b>dir_purpose</b> (and if it's
  * fetching descriptors, it's fetching them for <b>router_purpose</b>)
  * must use an anonymous connection to a directory. */
-static int
+STATIC int
 purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose)
 {
   if (get_options()->AllDirActionsPrivate)
@@ -198,6 +196,47 @@ dir_conn_purpose_to_string(int purpose)
   return "(unknown)";
 }
 
+/** Return the requisite directory information types. */
+STATIC dirinfo_type_t
+dir_fetch_type(int dir_purpose, int router_purpose, const char *resource)
+{
+  dirinfo_type_t type;
+  switch (dir_purpose) {
+    case DIR_PURPOSE_FETCH_EXTRAINFO:
+      type = EXTRAINFO_DIRINFO;
+      if (router_purpose == ROUTER_PURPOSE_BRIDGE)
+        type |= BRIDGE_DIRINFO;
+      else
+        type |= V3_DIRINFO;
+      break;
+    case DIR_PURPOSE_FETCH_SERVERDESC:
+      if (router_purpose == ROUTER_PURPOSE_BRIDGE)
+        type = BRIDGE_DIRINFO;
+      else
+        type = V3_DIRINFO;
+      break;
+    case DIR_PURPOSE_FETCH_STATUS_VOTE:
+    case DIR_PURPOSE_FETCH_DETACHED_SIGNATURES:
+    case DIR_PURPOSE_FETCH_CERTIFICATE:
+      type = V3_DIRINFO;
+      break;
+    case DIR_PURPOSE_FETCH_CONSENSUS:
+      type = V3_DIRINFO;
+      if (resource && !strcmp(resource, "microdesc"))
+        type |= MICRODESC_DIRINFO;
+      break;
+    case DIR_PURPOSE_FETCH_MICRODESC:
+      type = MICRODESC_DIRINFO;
+      break;
+    default:
+      log_warn(LD_BUG, "Unexpected purpose %d", (int)dir_purpose);
+      type = NO_DIRINFO;
+      break;
+  }
+  return type;
+}
+
+
 /** Return true iff <b>identity_digest</b> is the digest of a router which
  * says that it caches extrainfos.  (If <b>is_authority</b> we always
  * believe that to be true.) */
@@ -386,47 +425,21 @@ directory_pick_generic_dirserver(dirinfo_type_t type, int pds_flags,
  * Use <b>pds_flags</b> as arguments to router_pick_directory_server()
  * or router_pick_trusteddirserver().
  */
-void
+MOCK_IMPL(void, directory_get_from_dirserver, (uint8_t dir_purpose,
-directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
+                                               uint8_t router_purpose,
-                             const char *resource, int pds_flags)
+                                               const char *resource,
+                                               int pds_flags))
 {
   const routerstatus_t *rs = NULL;
   const or_options_t *options = get_options();
   int prefer_authority = directory_fetches_from_authorities(options);
   int require_authority = 0;
   int get_via_tor = purpose_needs_anonymity(dir_purpose, router_purpose);
-  dirinfo_type_t type;
+  dirinfo_type_t type = dir_fetch_type(dir_purpose, router_purpose, resource);
   time_t if_modified_since = 0;
 
-  /* FFFF we could break this switch into its own function, and call
+  if (type == NO_DIRINFO)
-   * it elsewhere in directory.c. -RD */
+    return;
-  switch (dir_purpose) {
-    case DIR_PURPOSE_FETCH_EXTRAINFO:
-      type = EXTRAINFO_DIRINFO |
-             (router_purpose == ROUTER_PURPOSE_BRIDGE ? BRIDGE_DIRINFO :
-                                                        V3_DIRINFO);
-      break;
-    case DIR_PURPOSE_FETCH_SERVERDESC:
-      type = (router_purpose == ROUTER_PURPOSE_BRIDGE ? BRIDGE_DIRINFO :
-                                                        V3_DIRINFO);
-      break;
-    case DIR_PURPOSE_FETCH_STATUS_VOTE:
-    case DIR_PURPOSE_FETCH_DETACHED_SIGNATURES:
-    case DIR_PURPOSE_FETCH_CERTIFICATE:
-      type = V3_DIRINFO;
-      break;
-    case DIR_PURPOSE_FETCH_CONSENSUS:
-      type = V3_DIRINFO;
-      if (resource && !strcmp(resource,"microdesc"))
-        type |= MICRODESC_DIRINFO;
-      break;
-    case DIR_PURPOSE_FETCH_MICRODESC:
-      type = MICRODESC_DIRINFO;
-      break;
-    default:
-      log_warn(LD_BUG, "Unexpected purpose %d", (int)dir_purpose);
-      return;
-  }
 
   if (dir_purpose == DIR_PURPOSE_FETCH_CONSENSUS) {
     int flav = FLAV_NS;
@@ -526,20 +539,16 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
         /* */
         rs = directory_pick_generic_dirserver(type, pds_flags,
                                               dir_purpose);
-        if (!rs) {
+        if (!rs)
-          /*XXXX024 I'm pretty sure this can never do any good, since
-           * rs isn't set. */
           get_via_tor = 1; /* last resort: try routing it via Tor */
-        }
       }
     }
-  } else { /* get_via_tor */
+  }
+
+  if (get_via_tor) {
     /* Never use fascistfirewall; we're going via Tor. */
-    if (1) {
+    pds_flags |= PDS_IGNORE_FASCISTFIREWALL;
-      /* anybody with a non-zero dirport will do. Disregard firewalls. */
+    rs = router_pick_directory_server(type, pds_flags);
-      pds_flags |= PDS_IGNORE_FASCISTFIREWALL;
-      rs = router_pick_directory_server(type, pds_flags);
-    }
   }
 
   /* If we have any hope of building an indirect conn, we know some router
@@ -1271,7 +1280,8 @@ directory_send_command(dir_connection_t *conn,
       return;
   }
 
-  if (strlen(proxystring) + strlen(url) >= 4096) {
+  /* warn in the non-tunneled case */
+  if (direct && (strlen(proxystring) + strlen(url) >= 4096)) {
     log_warn(LD_BUG,
              "Squid does not like URLs longer than 4095 bytes, and this "
              "one is %d bytes long: %s%s",

src/or/directory.h

@@ -16,9 +16,10 @@ int directories_have_accepted_server_descriptor(void);
 void directory_post_to_dirservers(uint8_t dir_purpose, uint8_t router_purpose,
                                   dirinfo_type_t type, const char *payload,
                                   size_t payload_len, size_t extrainfo_len);
-void directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
+MOCK_DECL(void, directory_get_from_dirserver, (uint8_t dir_purpose,
-                                  const char *resource,
+                                               uint8_t router_purpose,
-                                  int pds_flags);
+                                               const char *resource,
+                                               int pds_flags));
 void directory_get_from_all_authorities(uint8_t dir_purpose,
                                         uint8_t router_purpose,
                                         const char *resource);
@@ -120,7 +121,11 @@ int download_status_get_n_failures(const download_status_t *dls);
 
 #ifdef TOR_UNIT_TESTS
 /* Used only by directory.c and test_dir.c */
+
 STATIC int parse_http_url(const char *headers, char **url);
+STATIC int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose);
+STATIC dirinfo_type_t dir_fetch_type(int dir_purpose, int router_purpose,
+                                     const char *resource);
 #endif
 
 #endif

src/or/or.h

@@ -4961,14 +4961,13 @@ typedef struct dir_server_t {
  * or extrainfo documents.
  *
  * Passed to router_pick_directory_server (et al)
- *
- * [XXXX NOTE: This option is only implemented for pick_trusteddirserver,
- *  not pick_directory_server.  If we make it work on pick_directory_server
- *  too, we could conservatively make it only prevent multiple fetches to
- *  the same authority, or we could aggressively make it prevent multiple
- *  fetches to _any_ single directory server.]
  */
 #define PDS_NO_EXISTING_SERVERDESC_FETCH (1<<3)
+/** Flag to indicate that we should not use any directory authority to which
+ * we have an existing directory connection for downloading microdescs.
+ *
+ * Passed to router_pick_directory_server (et al)
+ */
 #define PDS_NO_EXISTING_MICRODESC_FETCH (1<<4)
 
 /** This node is to be chosen as a directory guard, so don't choose any

src/or/routerlist.c

@@ -65,7 +65,7 @@ static int compute_weighted_bandwidths(const smartlist_t *sl,
                                        bandwidth_weight_rule_t rule,
                                        u64_dbl_t **bandwidths_out);
 static const routerstatus_t *router_pick_directory_server_impl(
-                                           dirinfo_type_t auth, int flags);
+                              dirinfo_type_t auth, int flags, int *n_busy_out);
 static const routerstatus_t *router_pick_trusteddirserver_impl(
                 const smartlist_t *sourcelist, dirinfo_type_t auth,
                 int flags, int *n_busy_out);
@@ -1288,22 +1288,32 @@ router_get_fallback_dir_servers(void)
 const routerstatus_t *
 router_pick_directory_server(dirinfo_type_t type, int flags)
 {
+  int busy = 0;
   const routerstatus_t *choice;
 
   if (!routerlist)
     return NULL;
 
-  choice = router_pick_directory_server_impl(type, flags);
+  choice = router_pick_directory_server_impl(type, flags, &busy);
   if (choice || !(flags & PDS_RETRY_IF_NO_SERVERS))
     return choice;
 
+  if (busy) {
+    /* If the reason that we got no server is that servers are "busy",
+     * we must be excluding good servers because we already have serverdesc
+     * fetches with them.  Do not mark down servers up because of this. */
+    tor_assert((flags & (PDS_NO_EXISTING_SERVERDESC_FETCH|
+                         PDS_NO_EXISTING_MICRODESC_FETCH)));
+    return NULL;
+  }
+
   log_info(LD_DIR,
            "No reachable router entries for dirservers. "
            "Trying them all again.");
   /* mark all authdirservers as up again */
   mark_all_dirservers_up(fallback_dir_servers);
   /* try again */
-  choice = router_pick_directory_server_impl(type, flags);
+  choice = router_pick_directory_server_impl(type, flags, NULL);
   return choice;
 }
 
@@ -1413,11 +1423,15 @@ router_pick_dirserver_generic(smartlist_t *sourcelist,
 #define DIR_503_TIMEOUT (60*60)
 
 /** Pick a random running valid directory server/mirror from our
- * routerlist.  Arguments are as for router_pick_directory_server(), except
+ * routerlist.  Arguments are as for router_pick_directory_server(), except:
- * that RETRY_IF_NO_SERVERS is ignored.
+ *
+ * If <b>n_busy_out</b> is provided, set *<b>n_busy_out</b> to the number of
+ * directories that we excluded for no other reason than
+ * PDS_NO_EXISTING_SERVERDESC_FETCH or PDS_NO_EXISTING_MICRODESC_FETCH.
  */
 static const routerstatus_t *
-router_pick_directory_server_impl(dirinfo_type_t type, int flags)
+router_pick_directory_server_impl(dirinfo_type_t type, int flags,
+                                  int *n_busy_out)
 {
   const or_options_t *options = get_options();
   const node_t *result;
@@ -1426,10 +1440,12 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags)
   smartlist_t *overloaded_direct, *overloaded_tunnel;
   time_t now = time(NULL);
   const networkstatus_t *consensus = networkstatus_get_latest_consensus();
-  int requireother = ! (flags & PDS_ALLOW_SELF);
+  const int requireother = ! (flags & PDS_ALLOW_SELF);
-  int fascistfirewall = ! (flags & PDS_IGNORE_FASCISTFIREWALL);
+  const int fascistfirewall = ! (flags & PDS_IGNORE_FASCISTFIREWALL);
-  int for_guard = (flags & PDS_FOR_GUARD);
+  const int no_serverdesc_fetching = (flags & PDS_NO_EXISTING_SERVERDESC_FETCH);
-  int try_excluding = 1, n_excluded = 0;
+  const int no_microdesc_fetching = (flags & PDS_NO_EXISTING_MICRODESC_FETCH);
+  const int for_guard = (flags & PDS_FOR_GUARD);
+  int try_excluding = 1, n_excluded = 0, n_busy = 0;
 
   if (!consensus)
     return NULL;
@@ -1476,7 +1492,24 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags)
     }
 
     /* XXXX IP6 proposal 118 */
-    tor_addr_from_ipv4h(&addr, node->rs->addr);
+    tor_addr_from_ipv4h(&addr, status->addr);
+
+    if (no_serverdesc_fetching && (
+       connection_get_by_type_addr_port_purpose(
+         CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_SERVERDESC)
+    || connection_get_by_type_addr_port_purpose(
+         CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_EXTRAINFO)
+    )) {
+      ++n_busy;
+      continue;
+    }
+
+    if (no_microdesc_fetching && connection_get_by_type_addr_port_purpose(
+      CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_MICRODESC)
+    ) {
+      ++n_busy;
+      continue;
+    }
 
     is_overloaded = status->last_dir_503_at + DIR_503_TIMEOUT > now;
 
@@ -1521,9 +1554,13 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags)
      * not set, try again without excluding nodes. */
     try_excluding = 0;
     n_excluded = 0;
+    n_busy = 0;
     goto retry_without_exclude;
   }
 
+  if (n_busy_out)
+    *n_busy_out = n_busy;
+
   return result ? result->rs : NULL;
 }
 
@@ -4188,51 +4225,56 @@ list_pending_fpsk_downloads(fp_pair_map_t *result)
  * range.)  If <b>source</b> is given, download from <b>source</b>;
  * otherwise, download from an appropriate random directory server.
  */
-static void
+MOCK_IMPL(STATIC void, initiate_descriptor_downloads,
-initiate_descriptor_downloads(const routerstatus_t *source,
+          (const routerstatus_t *source, int purpose, smartlist_t *digests,
-                              int purpose,
+           int lo, int hi, int pds_flags))
-                              smartlist_t *digests,
-                              int lo, int hi, int pds_flags)
 {
-  int i, n = hi-lo;
   char *resource, *cp;
-  size_t r_len;
+  int digest_len, enc_digest_len;
-
+  const char *sep;
-  int digest_len = DIGEST_LEN, enc_digest_len = HEX_DIGEST_LEN;
+  int b64_256;
-  char sep = '+';
+  smartlist_t *tmp;
-  int b64_256 = 0;
 
   if (purpose == DIR_PURPOSE_FETCH_MICRODESC) {
     /* Microdescriptors are downloaded by "-"-separated base64-encoded
      * 256-bit digests. */
     digest_len = DIGEST256_LEN;
-    enc_digest_len = BASE64_DIGEST256_LEN;
+    enc_digest_len = BASE64_DIGEST256_LEN + 1;
-    sep = '-';
+    sep = "-";
     b64_256 = 1;
+  } else {
+    digest_len = DIGEST_LEN;
+    enc_digest_len = HEX_DIGEST_LEN + 1;
+    sep = "+";
+    b64_256 = 0;
   }
 
-  if (n <= 0)
-    return;
   if (lo < 0)
     lo = 0;
   if (hi > smartlist_len(digests))
     hi = smartlist_len(digests);
 
-  r_len = 8 + (enc_digest_len+1)*n;
+  if (hi-lo <= 0)
-  cp = resource = tor_malloc(r_len);
+    return;
-  memcpy(cp, "d/", 2);
+
-  cp += 2;
+  tmp = smartlist_new();
-  for (i = lo; i < hi; ++i) {
+
+  for (; lo < hi; ++lo) {
+    cp = tor_malloc(enc_digest_len);
     if (b64_256) {
-      digest256_to_base64(cp, smartlist_get(digests, i));
+      digest256_to_base64(cp, smartlist_get(digests, lo));
     } else {
-      base16_encode(cp, r_len-(cp-resource),
+      base16_encode(cp, enc_digest_len, smartlist_get(digests, lo), digest_len);
-                    smartlist_get(digests,i), digest_len);
     }
-    cp += enc_digest_len;
+    smartlist_add(tmp, cp);
-    *cp++ = sep;
   }
-  memcpy(cp-1, ".z", 3);
+
+  cp = smartlist_join_strings(tmp, sep, 0, NULL);
+  tor_asprintf(&resource, "d/%s.z", cp);
+
+  SMARTLIST_FOREACH(tmp, char *, cp1, tor_free(cp1));
+  smartlist_free(tmp);
+  tor_free(cp);
 
   if (source) {
     /* We know which authority we want. */
@@ -4247,14 +4289,28 @@ initiate_descriptor_downloads(const routerstatus_t *source,
   tor_free(resource);
 }
 
-/** Max amount of hashes to download per request.
+/** Return the max number of hashes to put in a URL for a given request.
- * Since squid does not like URLs >= 4096 bytes we limit it to 96.
- *   4096 - strlen(http://255.255.255.255/tor/server/d/.z) == 4058
- *   4058/41 (40 for the hash and 1 for the + that separates them) => 98
- *   So use 96 because it's a nice number.
  */
-#define MAX_DL_PER_REQUEST 96
+static int
-#define MAX_MICRODESC_DL_PER_REQUEST 92
+max_dl_per_request(const or_options_t *options, int purpose)
+{
+  /* Since squid does not like URLs >= 4096 bytes we limit it to 96.
+   *   4096 - strlen(http://255.255.255.255/tor/server/d/.z) == 4058
+   *   4058/41 (40 for the hash and 1 for the + that separates them) => 98
+   *   So use 96 because it's a nice number.
+   */
+  int max = 96;
+  if (purpose == DIR_PURPOSE_FETCH_MICRODESC) {
+    max = 92;
+  }
+  /* If we're going to tunnel our connections, we can ask for a lot more
+   * in a request. */
+  if (!directory_fetches_from_authorities(options)) {
+    max = 500;
+  }
+  return max;
+}
+
 /** Don't split our requests so finely that we are requesting fewer than
  * this number per server. */
 #define MIN_DL_PER_REQUEST 4
@@ -4276,92 +4332,90 @@ launch_descriptor_downloads(int purpose,
                             smartlist_t *downloadable,
                             const routerstatus_t *source, time_t now)
 {
-  int should_delay = 0, n_downloadable;
   const or_options_t *options = get_options();
   const char *descname;
+  const int fetch_microdesc = (purpose == DIR_PURPOSE_FETCH_MICRODESC);
+  int n_downloadable = smartlist_len(downloadable);
 
-  tor_assert(purpose == DIR_PURPOSE_FETCH_SERVERDESC ||
+  int i, n_per_request, max_dl_per_req;
-             purpose == DIR_PURPOSE_FETCH_MICRODESC);
+  const char *req_plural = "", *rtr_plural = "";
+  int pds_flags = PDS_RETRY_IF_NO_SERVERS;
 
-  descname = (purpose == DIR_PURPOSE_FETCH_SERVERDESC) ?
+  tor_assert(fetch_microdesc || purpose == DIR_PURPOSE_FETCH_SERVERDESC);
-    "routerdesc" : "microdesc";
+  descname = fetch_microdesc ? "microdesc" : "routerdesc";
+
+  if (!n_downloadable)
+    return;
 
-  n_downloadable = smartlist_len(downloadable);
   if (!directory_fetches_dir_info_early(options)) {
     if (n_downloadable >= MAX_DL_TO_DELAY) {
       log_debug(LD_DIR,
                 "There are enough downloadable %ss to launch requests.",
                 descname);
-      should_delay = 0;
     } else {
-      should_delay = (last_descriptor_download_attempted +
+
-                      options->TestingClientMaxIntervalWithoutRequest) > now;
+      /* should delay */
-      if (!should_delay && n_downloadable) {
+      if ((last_descriptor_download_attempted +
-        if (last_descriptor_download_attempted) {
+          options->TestingClientMaxIntervalWithoutRequest) > now)
-          log_info(LD_DIR,
+        return;
-                   "There are not many downloadable %ss, but we've "
+
-                   "been waiting long enough (%d seconds). Downloading.",
+      if (last_descriptor_download_attempted) {
-                   descname,
+        log_info(LD_DIR,
-                   (int)(now-last_descriptor_download_attempted));
+                 "There are not many downloadable %ss, but we've "
-        } else {
+                 "been waiting long enough (%d seconds). Downloading.",
-          log_info(LD_DIR,
+                 descname,
-                   "There are not many downloadable %ss, but we haven't "
+                 (int)(now-last_descriptor_download_attempted));
-                   "tried downloading descriptors recently. Downloading.",
+      } else {
-                   descname);
+        log_info(LD_DIR,
-        }
+                 "There are not many downloadable %ss, but we haven't "
+                 "tried downloading descriptors recently. Downloading.",
+                 descname);
       }
+
     }
   }
 
-  if (! should_delay && n_downloadable) {
+  if (!authdir_mode_any_nonhidserv(options)) {
-    int i, n_per_request;
+    /* If we wind up going to the authorities, we want to only open one
-    const char *req_plural = "", *rtr_plural = "";
+     * connection to each authority at a time, so that we don't overload
-    int pds_flags = PDS_RETRY_IF_NO_SERVERS;
+     * them.  We do this by setting PDS_NO_EXISTING_SERVERDESC_FETCH
-    if (! authdir_mode_any_nonhidserv(options)) {
+     * regardless of whether we're a cache or not.
-      /* If we wind up going to the authorities, we want to only open one
+     *
-       * connection to each authority at a time, so that we don't overload
+     * Setting this flag can make initiate_descriptor_downloads() ignore
-       * them.  We do this by setting PDS_NO_EXISTING_SERVERDESC_FETCH
+     * requests.  We need to make sure that we do in fact call
-       * regardless of whether we're a cache or not; it gets ignored if we're
+     * update_router_descriptor_downloads() later on, once the connections
-       * not calling router_pick_trusteddirserver.
+     * have succeeded or failed.
-       *
+     */
-       * Setting this flag can make initiate_descriptor_downloads() ignore
+    pds_flags |= fetch_microdesc ?
-       * requests.  We need to make sure that we do in fact call
+      PDS_NO_EXISTING_MICRODESC_FETCH :
-       * update_router_descriptor_downloads() later on, once the connections
+      PDS_NO_EXISTING_SERVERDESC_FETCH;
-       * have succeeded or failed.
-       */
-      pds_flags |= (purpose == DIR_PURPOSE_FETCH_MICRODESC) ?
-        PDS_NO_EXISTING_MICRODESC_FETCH :
-        PDS_NO_EXISTING_SERVERDESC_FETCH;
-    }
-
-    n_per_request = CEIL_DIV(n_downloadable, MIN_REQUESTS);
-    if (purpose == DIR_PURPOSE_FETCH_MICRODESC) {
-      if (n_per_request > MAX_MICRODESC_DL_PER_REQUEST)
-        n_per_request = MAX_MICRODESC_DL_PER_REQUEST;
-    } else {
-      if (n_per_request > MAX_DL_PER_REQUEST)
-        n_per_request = MAX_DL_PER_REQUEST;
-    }
-    if (n_per_request < MIN_DL_PER_REQUEST)
-      n_per_request = MIN_DL_PER_REQUEST;
-
-    if (n_downloadable > n_per_request)
-      req_plural = rtr_plural = "s";
-    else if (n_downloadable > 1)
-      rtr_plural = "s";
-
-    log_info(LD_DIR,
-             "Launching %d request%s for %d %s%s, %d at a time",
-             CEIL_DIV(n_downloadable, n_per_request), req_plural,
-             n_downloadable, descname, rtr_plural, n_per_request);
-    smartlist_sort_digests(downloadable);
-    for (i=0; i < n_downloadable; i += n_per_request) {
-      initiate_descriptor_downloads(source, purpose,
-                                    downloadable, i, i+n_per_request,
-                                    pds_flags);
-    }
-    last_descriptor_download_attempted = now;
   }
+
+  n_per_request = CEIL_DIV(n_downloadable, MIN_REQUESTS);
+  max_dl_per_req = max_dl_per_request(options, purpose);
+
+  if (n_per_request > max_dl_per_req)
+    n_per_request = max_dl_per_req;
+
+  if (n_per_request < MIN_DL_PER_REQUEST)
+    n_per_request = MIN_DL_PER_REQUEST;
+
+  if (n_downloadable > n_per_request)
+    req_plural = rtr_plural = "s";
+  else if (n_downloadable > 1)
+    rtr_plural = "s";
+
+  log_info(LD_DIR,
+           "Launching %d request%s for %d %s%s, %d at a time",
+           CEIL_DIV(n_downloadable, n_per_request), req_plural,
+           n_downloadable, descname, rtr_plural, n_per_request);
+  smartlist_sort_digests(downloadable);
+  for (i=0; i < n_downloadable; i += n_per_request) {
+    initiate_descriptor_downloads(source, purpose,
+                                  downloadable, i, i+n_per_request,
+                                  pds_flags);
+  }
+  last_descriptor_download_attempted = now;
 }
 
 /** For any descriptor that we want that's currently listed in
@@ -4541,7 +4595,7 @@ update_extrainfo_downloads(time_t now)
   routerlist_t *rl;
   smartlist_t *wanted;
   digestmap_t *pending;
-  int old_routers, i;
+  int old_routers, i, max_dl_per_req;
   int n_no_ei = 0, n_pending = 0, n_have = 0, n_delay = 0;
   if (! options->DownloadExtraInfo)
     return;
@@ -4597,9 +4651,11 @@ update_extrainfo_downloads(time_t now)
            n_no_ei, n_have, n_delay, n_pending, smartlist_len(wanted));
 
   smartlist_shuffle(wanted);
-  for (i = 0; i < smartlist_len(wanted); i += MAX_DL_PER_REQUEST) {
+
+  max_dl_per_req = max_dl_per_request(options, DIR_PURPOSE_FETCH_EXTRAINFO);
+  for (i = 0; i < smartlist_len(wanted); i += max_dl_per_req) {
     initiate_descriptor_downloads(NULL, DIR_PURPOSE_FETCH_EXTRAINFO,
-                                  wanted, i, i + MAX_DL_PER_REQUEST,
+                                  wanted, i, i+max_dl_per_req,
                 PDS_RETRY_IF_NO_SERVERS|PDS_NO_EXISTING_SERVERDESC_FETCH);
   }
 

src/or/routerlist.h

@@ -229,6 +229,10 @@ MOCK_DECL(int, router_descriptor_is_older_than, (const routerinfo_t *router,
 MOCK_DECL(STATIC was_router_added_t, extrainfo_insert,
           (routerlist_t *rl, extrainfo_t *ei, int warn_if_incompatible));
 
+MOCK_DECL(STATIC void, initiate_descriptor_downloads,
+          (const routerstatus_t *source, int purpose, smartlist_t *digests,
+           int lo, int hi, int pds_flags));
+
 #endif
 
 #endif

src/test/include.am

@@ -17,44 +17,45 @@ src_test_AM_CPPFLAGS = -DSHARE_DATADIR="\"$(datadir)\"" \
 
 src_test_test_SOURCES = \
 	src/test/test.c \
+	src/test/test_accounting.c \
 	src/test/test_addr.c \
 	src/test/test_buffers.c \
 	src/test/test_cell_formats.c \
+	src/test/test_cell_queue.c \
 	src/test/test_channel.c \
 	src/test/test_channeltls.c \
+	src/test/test_checkdir.c \
 	src/test/test_circuitlist.c \
 	src/test/test_circuitmux.c \
+	src/test/test_config.c \
 	src/test/test_containers.c \
 	src/test/test_controller_events.c \
 	src/test/test_crypto.c \
-	src/test/test_cell_queue.c \
 	src/test/test_data.c \
 	src/test/test_dir.c \
-	src/test/test_checkdir.c \
 	src/test/test_entryconn.c \
 	src/test/test_entrynodes.c \
 	src/test/test_extorport.c \
+	src/test/test_hs.c \
 	src/test/test_introduce.c \
 	src/test/test_logging.c \
 	src/test/test_microdesc.c \
+	src/test/test_nodelist.c \
 	src/test/test_oom.c \
-	src/test/test_accounting.c \
 	src/test/test_options.c \
+	src/test/test_policy.c \
 	src/test/test_pt.c \
-	src/test/test_relaycell.c \
 	src/test/test_relay.c \
+	src/test/test_relaycell.c \
 	src/test/test_replay.c \
 	src/test/test_routerkeys.c \
+	src/test/test_routerlist.c \
+	src/test/test_routerset.c \
 	src/test/test_scheduler.c \
 	src/test/test_socks.c \
+	src/test/test_status.c \
 	src/test/test_threads.c \
 	src/test/test_util.c \
-	src/test/test_config.c \
-	src/test/test_hs.c \
-	src/test/test_nodelist.c \
-	src/test/test_policy.c \
-	src/test/test_status.c \
-	src/test/test_routerset.c \
 	src/ext/tinytest.c
 
 src_test_test_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)

src/test/test.c

@@ -1294,84 +1294,86 @@ static struct testcase_t test_array[] = {
   END_OF_TESTCASES
 };
 
+extern struct testcase_t accounting_tests[];
 extern struct testcase_t addr_tests[];
 extern struct testcase_t buffer_tests[];
-extern struct testcase_t crypto_tests[];
-extern struct testcase_t container_tests[];
-extern struct testcase_t util_tests[];
-extern struct testcase_t dir_tests[];
-extern struct testcase_t checkdir_tests[];
-extern struct testcase_t microdesc_tests[];
-extern struct testcase_t pt_tests[];
-extern struct testcase_t config_tests[];
-extern struct testcase_t introduce_tests[];
-extern struct testcase_t replaycache_tests[];
-extern struct testcase_t relaycell_tests[];
 extern struct testcase_t cell_format_tests[];
-extern struct testcase_t circuitlist_tests[];
-extern struct testcase_t circuitmux_tests[];
 extern struct testcase_t cell_queue_tests[];
-extern struct testcase_t options_tests[];
-extern struct testcase_t socks_tests[];
-extern struct testcase_t entrynodes_tests[];
-extern struct testcase_t thread_tests[];
-extern struct testcase_t extorport_tests[];
-extern struct testcase_t controller_event_tests[];
-extern struct testcase_t logging_tests[];
-extern struct testcase_t hs_tests[];
-extern struct testcase_t nodelist_tests[];
-extern struct testcase_t routerkeys_tests[];
-extern struct testcase_t oom_tests[];
-extern struct testcase_t accounting_tests[];
-extern struct testcase_t policy_tests[];
-extern struct testcase_t status_tests[];
-extern struct testcase_t routerset_tests[];
-extern struct testcase_t router_tests[];
 extern struct testcase_t channel_tests[];
 extern struct testcase_t channeltls_tests[];
-extern struct testcase_t relay_tests[];
+extern struct testcase_t checkdir_tests[];
-extern struct testcase_t scheduler_tests[];
+extern struct testcase_t circuitlist_tests[];
+extern struct testcase_t circuitmux_tests[];
+extern struct testcase_t config_tests[];
+extern struct testcase_t container_tests[];
+extern struct testcase_t controller_event_tests[];
+extern struct testcase_t crypto_tests[];
+extern struct testcase_t dir_tests[];
 extern struct testcase_t entryconn_tests[];
+extern struct testcase_t entrynodes_tests[];
+extern struct testcase_t extorport_tests[];
+extern struct testcase_t hs_tests[];
+extern struct testcase_t introduce_tests[];
+extern struct testcase_t logging_tests[];
+extern struct testcase_t microdesc_tests[];
+extern struct testcase_t nodelist_tests[];
+extern struct testcase_t oom_tests[];
+extern struct testcase_t options_tests[];
+extern struct testcase_t policy_tests[];
+extern struct testcase_t pt_tests[];
+extern struct testcase_t relay_tests[];
+extern struct testcase_t relaycell_tests[];
+extern struct testcase_t replaycache_tests[];
+extern struct testcase_t router_tests[];
+extern struct testcase_t routerkeys_tests[];
+extern struct testcase_t routerlist_tests[];
+extern struct testcase_t routerset_tests[];
+extern struct testcase_t scheduler_tests[];
+extern struct testcase_t socks_tests[];
+extern struct testcase_t status_tests[];
+extern struct testcase_t thread_tests[];
+extern struct testcase_t util_tests[];
 
 static struct testgroup_t testgroups[] = {
   { "", test_array },
-  { "buffer/", buffer_tests },
+  { "accounting/", accounting_tests },
-  { "socks/", socks_tests },
   { "addr/", addr_tests },
-  { "crypto/", crypto_tests },
+  { "buffer/", buffer_tests },
+  { "cellfmt/", cell_format_tests },
+  { "cellqueue/", cell_queue_tests },
+  { "channel/", channel_tests },
+  { "channeltls/", channeltls_tests },
+  { "checkdir/", checkdir_tests },
+  { "circuitlist/", circuitlist_tests },
+  { "circuitmux/", circuitmux_tests },
+  { "config/", config_tests },
   { "container/", container_tests },
+  { "control/", controller_event_tests },
+  { "crypto/", crypto_tests },
+  { "dir/", dir_tests },
+  { "dir/md/", microdesc_tests },
+  { "entryconn/", entryconn_tests },
+  { "entrynodes/", entrynodes_tests },
+  { "extorport/", extorport_tests },
+  { "hs/", hs_tests },
+  { "introduce/", introduce_tests },
+  { "nodelist/", nodelist_tests },
+  { "oom/", oom_tests },
+  { "options/", options_tests },
+  { "policy/" , policy_tests },
+  { "pt/", pt_tests },
+  { "relay/" , relay_tests },
+  { "relaycell/", relaycell_tests },
+  { "replaycache/", replaycache_tests },
+  { "routerkeys/", routerkeys_tests },
+  { "routerlist/", routerlist_tests },
+  { "routerset/" , routerset_tests },
+  { "scheduler/", scheduler_tests },
+  { "socks/", socks_tests },
+  { "status/" , status_tests },
   { "util/", util_tests },
   { "util/logging/", logging_tests },
   { "util/thread/", thread_tests },
-  { "cellfmt/", cell_format_tests },
-  { "cellqueue/", cell_queue_tests },
-  { "dir/", dir_tests },
-  { "checkdir/", checkdir_tests },
-  { "dir/md/", microdesc_tests },
-  { "pt/", pt_tests },
-  { "config/", config_tests },
-  { "replaycache/", replaycache_tests },
-  { "relaycell/", relaycell_tests },
-  { "introduce/", introduce_tests },
-  { "circuitlist/", circuitlist_tests },
-  { "circuitmux/", circuitmux_tests },
-  { "options/", options_tests },
-  { "entrynodes/", entrynodes_tests },
-  { "entryconn/", entryconn_tests },
-  { "extorport/", extorport_tests },
-  { "control/", controller_event_tests },
-  { "hs/", hs_tests },
-  { "nodelist/", nodelist_tests },
-  { "routerkeys/", routerkeys_tests },
-  { "oom/", oom_tests },
-  { "accounting/", accounting_tests },
-  { "policy/" , policy_tests },
-  { "status/" , status_tests },
-  { "routerset/" , routerset_tests },
-  { "channel/", channel_tests },
-  { "channeltls/", channeltls_tests },
-  { "relay/" , relay_tests },
-  { "scheduler/", scheduler_tests },
   END_OF_GROUPS
 };
 

src/test/test_dir.c

@@ -2930,6 +2930,30 @@ test_dir_http_handling(void *args)
   tor_free(url);
 }
 
+static void
+test_dir_purpose_needs_anonymity(void *arg)
+{
+  (void)arg;
+  tt_int_op(1, ==, purpose_needs_anonymity(0, ROUTER_PURPOSE_BRIDGE));
+  tt_int_op(1, ==, purpose_needs_anonymity(0, ROUTER_PURPOSE_GENERAL));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_MICRODESC,
+                                            ROUTER_PURPOSE_GENERAL));
+ done: ;
+}
+
+static void
+test_dir_fetch_type(void *arg)
+{
+  (void)arg;
+  tt_assert(dir_fetch_type(DIR_PURPOSE_FETCH_MICRODESC, ROUTER_PURPOSE_GENERAL,
+                           NULL) == MICRODESC_DIRINFO);
+  tt_assert(dir_fetch_type(DIR_PURPOSE_FETCH_SERVERDESC, ROUTER_PURPOSE_BRIDGE,
+                           NULL) == BRIDGE_DIRINFO);
+  tt_assert(dir_fetch_type(DIR_PURPOSE_FETCH_CONSENSUS, ROUTER_PURPOSE_GENERAL,
+                           "microdesc") == (V3_DIRINFO | MICRODESC_DIRINFO));
+ done: ;
+}
+
 #define DIR_LEGACY(name)                                                   \
   { #name, test_dir_ ## name , TT_FORK, NULL, NULL }
 
@@ -2957,6 +2981,8 @@ struct testcase_t dir_tests[] = {
   DIR_LEGACY(clip_unmeasured_bw_kb_alt),
   DIR(fmt_control_ns, 0),
   DIR(http_handling, 0),
+  DIR(purpose_needs_anonymity, 0),
+  DIR(fetch_type, 0),
   END_OF_TESTCASES
 };
 

src/test/test_nodelist.c

@@ -10,7 +10,7 @@
 #include "nodelist.h"
 #include "test.h"
 
-/** Tese the case when node_get_by_id() returns NULL,
+/** Test the case when node_get_by_id() returns NULL,
  * node_get_verbose_nickname_by_id should return the base 16 encoding
  * of the id.
  */

src/test/test_routerlist.c

@@ -0,0 +1,103 @@
+/* Copyright (c) 2014, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#define ROUTERLIST_PRIVATE
+#include "or.h"
+#include "routerlist.h"
+#include "directory.h"
+#include "test.h"
+
+
+/* 4 digests + 3 sep + pre + post + NULL */
+static char output[4*BASE64_DIGEST256_LEN+3+2+2+1];
+
+static void
+mock_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
+                             const char *resource, int pds_flags)
+{
+  (void)dir_purpose;
+  (void)router_purpose;
+  (void)pds_flags;
+  tt_assert(resource);
+  strlcpy(output, resource, sizeof(output));
+ done:
+  ;
+}
+
+static void
+test_routerlist_initiate_descriptor_downloads(void *arg)
+{
+  const char *prose = "unhurried and wise, we perceive.";
+  smartlist_t *digests = smartlist_new();
+  (void)arg;
+
+  for (int i = 0; i < 20; i++) {
+    smartlist_add(digests, (char*)prose);
+  }
+
+  MOCK(directory_get_from_dirserver, mock_get_from_dirserver);
+  initiate_descriptor_downloads(NULL, DIR_PURPOSE_FETCH_MICRODESC,
+                                digests, 3, 7, 0);
+  UNMOCK(directory_get_from_dirserver);
+
+  tt_str_op(output, OP_EQ, "d/"
+            "dW5odXJyaWVkIGFuZCB3aXNlLCB3ZSBwZXJjZWl2ZS4-"
+            "dW5odXJyaWVkIGFuZCB3aXNlLCB3ZSBwZXJjZWl2ZS4-"
+            "dW5odXJyaWVkIGFuZCB3aXNlLCB3ZSBwZXJjZWl2ZS4-"
+            "dW5odXJyaWVkIGFuZCB3aXNlLCB3ZSBwZXJjZWl2ZS4"
+            ".z");
+
+ done:
+  smartlist_free(digests);
+}
+
+static int count = 0;
+
+static void
+mock_initiate_descriptor_downloads(const routerstatus_t *source,
+                                   int purpose, smartlist_t *digests,
+                                   int lo, int hi, int pds_flags)
+{
+  (void)source;
+  (void)purpose;
+  (void)digests;
+  (void)pds_flags;
+  (void)hi;
+  (void)lo;
+  count += 1;
+}
+
+static void
+test_routerlist_launch_descriptor_downloads(void *arg)
+{
+  smartlist_t *downloadable = smartlist_new();
+  time_t now = time(NULL);
+  char *cp;
+  (void)arg;
+
+  for (int i = 0; i < 100; i++) {
+    cp = tor_malloc(DIGEST256_LEN);
+    tt_assert(cp);
+    crypto_rand(cp, DIGEST256_LEN);
+    smartlist_add(downloadable, cp);
+  }
+
+  MOCK(initiate_descriptor_downloads, mock_initiate_descriptor_downloads);
+  launch_descriptor_downloads(DIR_PURPOSE_FETCH_MICRODESC, downloadable,
+                              NULL, now);
+  tt_int_op(3, ==, count);
+  UNMOCK(initiate_descriptor_downloads);
+
+ done:
+  SMARTLIST_FOREACH(downloadable, char *, cp1, tor_free(cp1));
+  smartlist_free(downloadable);
+}
+
+#define NODE(name, flags) \
+  { #name, test_routerlist_##name, (flags), NULL, NULL }
+
+struct testcase_t routerlist_tests[] = {
+  NODE(initiate_descriptor_downloads, 0),
+  NODE(launch_descriptor_downloads, 0),
+  END_OF_TESTCASES
+};