Merge remote-tracking branch 'teor/feature4483-v10-squashed'

changes/bug4483-multiple-consensus-downloads

@@ -0,0 +1,9 @@
+  o Major features (consensus downloads):
+    - Schedule multiple in-progress consensus downloads during client
+      bootstrap. Use the first one that starts downloading, close the
+      rest. This reduces failures when authorities are slow or down.
+      With #15775, it reduces failures due to fallback churn.
+      Implements #4483 (reduce failures when authorities are down).
+      Patch by "teor".
+      Implements IPv4 portions of proposal #210 by "mikeperry" and
+      "teor".

doc/tor.1.txt

@@ -360,7 +360,11 @@ GENERAL OPTIONS
 
 [[FallbackDir]] **FallbackDir** __address__:__port__ orport=__port__ id=__fingerprint__ [weight=__num__]::
     When we're unable to connect to any directory cache for directory info
-    (usually because we don't know about any yet) we try a FallbackDir.
+    (usually because we don't know about any yet) we try a directory authority.
+    Clients also simultaneously try a FallbackDir, to avoid hangs on client
+    startup if a directory authority is down. Clients retry FallbackDirs more
+    often than directory authorities, to reduce the load on the directory
+    authorities.
     By default, the directory authorities are also FallbackDirs. Specifying a
     FallbackDir replaces Tor's default hard-coded FallbackDirs (if any).
 
@@ -2287,10 +2291,18 @@ The following options are used for running a testing Tor network.
        TestingClientDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
        TestingServerConsensusDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
        TestingClientConsensusDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
+       TestingClientBootstrapConsensusAuthorityDownloadSchedule 0, 2,
+           4 (for 40 seconds), 8, 16, 32, 60
+       TestingClientBootstrapConsensusFallbackDownloadSchedule 0, 1,
+           4 (for 40 seconds), 8, 16, 32, 60
+       TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule 0, 1,
+           4 (for 40 seconds), 8, 16, 32, 60
        TestingBridgeDownloadSchedule 60, 30, 30, 60
        TestingClientMaxIntervalWithoutRequest 5 seconds
        TestingDirConnectionMaxStall 30 seconds
        TestingConsensusMaxDownloadTries 80
+       TestingClientBootstrapConsensusMaxDownloadTries 80
+       TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries 80
        TestingDescriptorMaxDownloadTries 80
        TestingMicrodescMaxDownloadTries 80
        TestingCertMaxDownloadTries 80
@@ -2351,6 +2363,36 @@ The following options are used for running a testing Tor network.
     requires that **TestingTorNetwork** is set. (Default: 0, 0, 60, 300, 600,
     1800, 3600, 3600, 3600, 10800, 21600, 43200)
 
+[[TestingClientBootstrapConsensusAuthorityDownloadSchedule]] **TestingClientBootstrapConsensusAuthorityDownloadSchedule** __N__,__N__,__...__::
+    Schedule for when clients should download consensuses from authorities if
+    they are bootstrapping (that is, they don't have a usable, reasonably live
+    consensus). Only used by clients fetching from a list of fallback
+    directory mirrors. This schedule is advanced by (potentially concurrent)
+    connection attempts, unlike other schedules, which are advanced by
+    connection failures. Changing this schedule requires that
+    **TestingTorNetwork** is set. (Default: 10, 11, 3600, 10800, 25200, 54000,
+    111600, 262800)
+
+[[TestingClientBootstrapConsensusFallbackDownloadSchedule]] **TestingClientBootstrapConsensusFallbackDownloadSchedule** __N__,__N__,__...__::
+    Schedule for when clients should download consensuses from fallback
+    directory mirrors if they are bootstrapping (that is, they don't have a
+    usable, reasonably live consensus). Only used by clients fetching from a
+    list of fallback directory mirrors.  This schedule is advanced by
+    (potentially concurrent) connection attempts, unlike other schedules, which
+    are advanced by connection failures. Changing this schedule requires that
+    **TestingTorNetwork** is set. (Default: 0, 1, 4, 11, 3600, 10800, 25200,
+    54000, 111600, 262800)
+
+[[TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule]] **TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule** __N__,__N__,__...__::
+    Schedule for when clients should download consensuses from authorities if
+    they are bootstrapping (that is, they don't have a usable, reasonably live
+    consensus). Only used by clients which don't have or won't fetch from a
+    list of fallback directory mirrors. This schedule is advanced by
+    (potentially concurrent) connection attempts, unlike other schedules,
+    which are advanced by connection failures. Changing this schedule requires
+    that **TestingTorNetwork** is set. (Default: 0, 3, 7, 3600, 10800, 25200,
+    54000, 111600, 262800)
+
 [[TestingBridgeDownloadSchedule]] **TestingBridgeDownloadSchedule** __N__,__N__,__...__::
     Schedule for when clients should download bridge descriptors. Changing this
     requires that **TestingTorNetwork** is set. (Default: 3600, 900, 900, 3600)
@@ -2367,9 +2409,24 @@ The following options are used for running a testing Tor network.
     5 minutes)
 
 [[TestingConsensusMaxDownloadTries]] **TestingConsensusMaxDownloadTries** __NUM__::
-    Try this often to download a consensus before giving up. Changing
+    Try this many times to download a consensus before giving up. Changing
     this requires that **TestingTorNetwork** is set. (Default: 8)
 
+[[TestingClientBootstrapConsensusMaxDownloadTries]] **TestingClientBootstrapConsensusMaxDownloadTries** __NUM__::
+    Try this many times to download a consensus while bootstrapping using
+    fallback directory mirrors before giving up. Changing this requires that
+    **TestingTorNetwork** is set. (Default: 7)
+
+[[TestingClientBootstrapConsensusMaxDownloadTries]] **TestingClientBootstrapConsensusMaxDownloadTries** __NUM__::
+    Try this many times to download a consensus while bootstrapping using
+    only authorities before giving up. Changing this requires that
+    **TestingTorNetwork** is set. (Default: 4)
+
+[[TestingClientBootstrapConsensusMaxInProgressTries]] **TestingClientBootstrapConsensusMaxInProgressTries** __NUM__::
+    Try this many simultaneous connections to download a consensus before
+    waiting for one to complete, timeout, or error out. Changing this
+    requires that **TestingTorNetwork** is set. (Default: 4)
+
 [[TestingDescriptorMaxDownloadTries]] **TestingDescriptorMaxDownloadTries** __NUM__::
     Try this often to download a server descriptor before giving up.
     Changing this requires that **TestingTorNetwork** is set. (Default: 8)

src/common/torint.h

@@ -336,6 +336,32 @@ typedef uint32_t uintptr_t;
 #endif /* time_t_is_signed */
 #endif /* ifndef(TIME_MAX) */
 
+#ifndef TIME_MIN
+
+#ifdef TIME_T_IS_SIGNED
+
+#if (SIZEOF_TIME_T == SIZEOF_INT)
+#define TIME_MIN ((time_t)INT_MIN)
+#elif (SIZEOF_TIME_T == SIZEOF_LONG)
+#define TIME_MIN ((time_t)LONG_MIN)
+#elif (SIZEOF_TIME_T == 8)
+#define TIME_MIN ((time_t)INT64_MIN)
+#else
+#error "Can't define (signed) TIME_MIN"
+#endif
+
+#else
+/* Unsigned case */
+#if (SIZEOF_TIME_T == 4)
+#define TIME_MIN ((time_t)UINT32_MIN)
+#elif (SIZEOF_TIME_T == 8)
+#define TIME_MIN ((time_t)UINT64_MIN)
+#else
+#error "Can't define (unsigned) TIME_MIN"
+#endif
+#endif /* time_t_is_signed */
+#endif /* ifndef(TIME_MIN) */
+
 #ifndef SIZE_MAX
 #if (SIZEOF_SIZE_T == 4)
 #define SIZE_MAX UINT32_MAX

src/or/config.c

@@ -476,10 +476,40 @@ static config_var_t option_vars_[] = {
   V(TestingClientConsensusDownloadSchedule, CSV_INTERVAL, "0, 0, 60, "
                                  "300, 600, 1800, 3600, 3600, 3600, "
                                  "10800, 21600, 43200"),
+  /* With the TestingClientBootstrapConsensus*Download* below:
+   * Clients with only authorities will try:
+   *  - 3 authorities over 10 seconds, then wait 60 minutes.
+   * Clients with authorities and fallbacks will try:
+   *  - 2 authorities and 4 fallbacks over 21 seconds, then wait 60 minutes.
+   * Clients will also retry when an application request arrives.
+   * After a number of failed reqests, clients retry every 3 days + 1 hour.
+   *
+   * Clients used to try 2 authorities over 10 seconds, then wait for
+   * 60 minutes or an application request.
+   *
+   * When clients have authorities and fallbacks available, they use these
+   * schedules: (we stagger the times to avoid thundering herds) */
+  V(TestingClientBootstrapConsensusAuthorityDownloadSchedule, CSV_INTERVAL,
+    "10, 11, 3600, 10800, 25200, 54000, 111600, 262800" /* 3 days + 1 hour */),
+  V(TestingClientBootstrapConsensusFallbackDownloadSchedule, CSV_INTERVAL,
+    "0, 1, 4, 11, 3600, 10800, 25200, 54000, 111600, 262800"),
+  /* When clients only have authorities available, they use this schedule: */
+  V(TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule, CSV_INTERVAL,
+    "0, 3, 7, 3600, 10800, 25200, 54000, 111600, 262800"),
+  /* We don't want to overwhelm slow networks (or mirrors whose replies are
+   * blocked), but we also don't want to fail if only some mirrors are
+   * blackholed. Clients will try 3 directories simultaneously.
+   * (Relays never use simultaneous connections.) */
+  V(TestingClientBootstrapConsensusMaxInProgressTries, UINT, "3"),
   V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "3600, 900, 900, 3600"),
   V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "10 minutes"),
   V(TestingDirConnectionMaxStall, INTERVAL, "5 minutes"),
   V(TestingConsensusMaxDownloadTries, UINT, "8"),
+  /* Since we try connections rapidly and simultaneously, we can afford
+   * to give up earlier. (This protects against overloading directories.) */
+  V(TestingClientBootstrapConsensusMaxDownloadTries, UINT, "7"),
+  /* We want to give up much earlier if we're only using authorities. */
+  V(TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries, UINT, "4"),
   V(TestingDescriptorMaxDownloadTries, UINT, "8"),
   V(TestingMicrodescMaxDownloadTries, UINT, "8"),
   V(TestingCertMaxDownloadTries, UINT, "8"),
@@ -526,10 +556,18 @@ static const config_var_t testing_tor_network_defaults[] = {
                                  "15, 20, 30, 60"),
   V(TestingClientConsensusDownloadSchedule, CSV_INTERVAL, "0, 0, 5, 10, "
                                  "15, 20, 30, 60"),
+  V(TestingClientBootstrapConsensusAuthorityDownloadSchedule, CSV_INTERVAL,
+    "0, 2, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 16, 32, 60"),
+  V(TestingClientBootstrapConsensusFallbackDownloadSchedule, CSV_INTERVAL,
+    "0, 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 16, 32, 60"),
+  V(TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule, CSV_INTERVAL,
+    "0, 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 16, 32, 60"),
   V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "60, 30, 30, 60"),
   V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "5 seconds"),
   V(TestingDirConnectionMaxStall, INTERVAL, "30 seconds"),
   V(TestingConsensusMaxDownloadTries, UINT, "80"),
+  V(TestingClientBootstrapConsensusMaxDownloadTries, UINT, "80"),
+  V(TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries, UINT, "80"),
   V(TestingDescriptorMaxDownloadTries, UINT, "80"),
   V(TestingMicrodescMaxDownloadTries, UINT, "80"),
   V(TestingCertMaxDownloadTries, UINT, "80"),
@@ -3758,10 +3796,16 @@ options_validate(or_options_t *old_options, or_options_t *options,
   CHECK_DEFAULT(TestingClientDownloadSchedule);
   CHECK_DEFAULT(TestingServerConsensusDownloadSchedule);
   CHECK_DEFAULT(TestingClientConsensusDownloadSchedule);
+  CHECK_DEFAULT(TestingClientBootstrapConsensusAuthorityDownloadSchedule);
+  CHECK_DEFAULT(TestingClientBootstrapConsensusFallbackDownloadSchedule);
+  CHECK_DEFAULT(TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule);
   CHECK_DEFAULT(TestingBridgeDownloadSchedule);
   CHECK_DEFAULT(TestingClientMaxIntervalWithoutRequest);
   CHECK_DEFAULT(TestingDirConnectionMaxStall);
   CHECK_DEFAULT(TestingConsensusMaxDownloadTries);
+  CHECK_DEFAULT(TestingClientBootstrapConsensusMaxDownloadTries);
+  CHECK_DEFAULT(TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries);
+  CHECK_DEFAULT(TestingClientBootstrapConsensusMaxInProgressTries);
   CHECK_DEFAULT(TestingDescriptorMaxDownloadTries);
   CHECK_DEFAULT(TestingMicrodescMaxDownloadTries);
   CHECK_DEFAULT(TestingCertMaxDownloadTries);
@@ -3836,11 +3880,41 @@ options_validate(or_options_t *old_options, or_options_t *options,
   }
 
   if (options->TestingConsensusMaxDownloadTries < 2) {
-    REJECT("TestingConsensusMaxDownloadTries must be greater than 1.");
+    REJECT("TestingConsensusMaxDownloadTries must be greater than 2.");
   } else if (options->TestingConsensusMaxDownloadTries > 800) {
     COMPLAIN("TestingConsensusMaxDownloadTries is insanely high.");
   }
 
+  if (options->TestingClientBootstrapConsensusMaxDownloadTries < 2) {
+    REJECT("TestingClientBootstrapConsensusMaxDownloadTries must be greater "
+           "than 2."
+           );
+  } else if (options->TestingClientBootstrapConsensusMaxDownloadTries > 800) {
+    COMPLAIN("TestingClientBootstrapConsensusMaxDownloadTries is insanely "
+             "high.");
+  }
+
+  if (options->TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries
+      < 2) {
+    REJECT("TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries must "
+           "be greater than 2."
+           );
+  } else if (
+        options->TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries
+        > 800) {
+    COMPLAIN("TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries is "
+             "insanely high.");
+  }
+
+  if (options->TestingClientBootstrapConsensusMaxInProgressTries < 1) {
+    REJECT("TestingClientBootstrapConsensusMaxInProgressTries must be greater "
+           "than 0.");
+  } else if (options->TestingClientBootstrapConsensusMaxInProgressTries
+             > 100) {
+    COMPLAIN("TestingClientBootstrapConsensusMaxInProgressTries is insanely "
+             "high.");
+  }
+
   if (options->TestingDescriptorMaxDownloadTries < 2) {
     REJECT("TestingDescriptorMaxDownloadTries must be greater than 1.");
   } else if (options->TestingDescriptorMaxDownloadTries > 800) {

src/or/connection.c

@@ -1618,13 +1618,18 @@ connection_init_accepted_conn(connection_t *conn,
   return 0;
 }
 
-static int
-connection_connect_sockaddr(connection_t *conn,
+/** Take conn, make a nonblocking socket; try to connect to
+ * sa, binding to bindaddr if sa is not localhost. If fail, return -1 and if
+ * applicable put your best guess about errno into *<b>socket_error</b>.
+ * If connected return 1, if EAGAIN return 0.
+ */
+MOCK_IMPL(STATIC int,
+connection_connect_sockaddr,(connection_t *conn,
                             const struct sockaddr *sa,
                             socklen_t sa_len,
                             const struct sockaddr *bindaddr,
                             socklen_t bindaddr_len,
-                            int *socket_error)
+                            int *socket_error))
 {
   tor_socket_t s;
   int inprogress = 0;
@@ -4222,6 +4227,19 @@ connection_write_to_buf_impl_,(const char *string, size_t len,
   }
 }
 
+/** Return a connection_t * from get_connection_array() that satisfies test on
+ * var, and that is not marked for close. */
+#define CONN_GET_TEMPLATE(var, test)               \
+  STMT_BEGIN                                       \
+    smartlist_t *conns = get_connection_array();   \
+    SMARTLIST_FOREACH(conns, connection_t *, var,  \
+    {                                              \
+      if (var && (test) && !var->marked_for_close) \
+        return var;                                \
+    });                                            \
+    return NULL;                                   \
+  STMT_END
+
 /** Return a connection with given type, address, port, and purpose;
  * or NULL if no such connection exists (or if all such connections are marked
  * for close). */
@@ -4230,17 +4248,11 @@ connection_get_by_type_addr_port_purpose(int type,
                                          const tor_addr_t *addr, uint16_t port,
                                          int purpose)
 {
-  smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
-    if (conn->type == type &&
+  CONN_GET_TEMPLATE(conn,
+       (conn->type == type &&
         tor_addr_eq(&conn->addr, addr) &&
         conn->port == port &&
-        conn->purpose == purpose &&
-        !conn->marked_for_close)
-      return conn;
-  });
-  return NULL;
+        conn->purpose == purpose));
 }
 
 /** Return the stream with id <b>id</b> if it is not already marked for
@@ -4249,13 +4261,7 @@ connection_get_by_type_addr_port_purpose(int type,
 connection_t *
 connection_get_by_global_id(uint64_t id)
 {
-  smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
-    if (conn->global_identifier == id)
-      return conn;
-  });
-  return NULL;
+  CONN_GET_TEMPLATE(conn, conn->global_identifier == id);
 }
 
 /** Return a connection of type <b>type</b> that is not marked for close.
@@ -4263,13 +4269,7 @@ connection_get_by_global_id(uint64_t id)
 connection_t *
 connection_get_by_type(int type)
 {
-  smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
-    if (conn->type == type && !conn->marked_for_close)
-      return conn;
-  });
-  return NULL;
+  CONN_GET_TEMPLATE(conn, conn->type == type);
 }
 
 /** Return a connection of type <b>type</b> that is in state <b>state</b>,
@@ -4278,13 +4278,7 @@ connection_get_by_type(int type)
 connection_t *
 connection_get_by_type_state(int type, int state)
 {
-  smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
-    if (conn->type == type && conn->state == state && !conn->marked_for_close)
-      return conn;
-  });
-  return NULL;
+  CONN_GET_TEMPLATE(conn, conn->type == type && conn->state == state);
 }
 
 /** Return a connection of type <b>type</b> that has rendquery equal
@@ -4295,55 +4289,142 @@ connection_t *
 connection_get_by_type_state_rendquery(int type, int state,
                                        const char *rendquery)
 {
-  smartlist_t *conns = get_connection_array();
-
   tor_assert(type == CONN_TYPE_DIR ||
              type == CONN_TYPE_AP || type == CONN_TYPE_EXIT);
   tor_assert(rendquery);
 
-  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
-    if (conn->type == type &&
-        !conn->marked_for_close &&
-        (!state || state == conn->state)) {
-      if (type == CONN_TYPE_DIR &&
+  CONN_GET_TEMPLATE(conn,
+       (conn->type == type &&
+        (!state || state == conn->state)) &&
+        (
+         (type == CONN_TYPE_DIR &&
           TO_DIR_CONN(conn)->rend_data &&
           !rend_cmp_service_ids(rendquery,
                                 TO_DIR_CONN(conn)->rend_data->onion_address))
-        return conn;
-      else if (CONN_IS_EDGE(conn) &&
+         ||
+              (CONN_IS_EDGE(conn) &&
                TO_EDGE_CONN(conn)->rend_data &&
                !rend_cmp_service_ids(rendquery,
                             TO_EDGE_CONN(conn)->rend_data->onion_address))
-        return conn;
-    }
-  } SMARTLIST_FOREACH_END(conn);
-  return NULL;
+         ));
 }
 
+#define CONN_FIRST_AND_FREE_TEMPLATE(sl)       \
+  STMT_BEGIN                                   \
+    if (smartlist_len(sl) > 0) {               \
+      void *first_item = smartlist_get(sl, 0); \
+      smartlist_free(sl);                      \
+      return first_item;                       \
+    } else {                                   \
+      smartlist_free(sl);                      \
+      return NULL;                             \
+    }                                          \
+  STMT_END
+
+
 /** Return a directory connection (if any one exists) that is fetching
- * the item described by <b>state</b>/<b>resource</b> */
+ * the item described by <b>purpose</b>/<b>resource</b>, otherwise return NULL.
+ */
 dir_connection_t *
-connection_dir_get_by_purpose_and_resource(int purpose,
+connection_dir_get_by_purpose_and_resource(
+                                           int purpose,
                                            const char *resource)
 {
-  smartlist_t *conns = get_connection_array();
+  smartlist_t *conns = connection_dir_list_by_purpose_and_resource(
+                                                          purpose,
+                                                          resource);
+  CONN_FIRST_AND_FREE_TEMPLATE(conns);
+}
 
-  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
-    dir_connection_t *dirconn;
-    if (conn->type != CONN_TYPE_DIR || conn->marked_for_close ||
-        conn->purpose != purpose)
-      continue;
-    dirconn = TO_DIR_CONN(conn);
-    if (dirconn->requested_resource == NULL) {
-      if (resource == NULL)
-        return dirconn;
-    } else if (resource) {
-      if (0 == strcmp(resource, dirconn->requested_resource))
-        return dirconn;
-    }
-  } SMARTLIST_FOREACH_END(conn);
+/** Return a new smartlist of dir_connection_t * from get_connection_array()
+ * that satisfy conn_test on connection_t *conn_var, and dirconn_test on
+ * dir_connection_t *dirconn_var. conn_var must be of CONN_TYPE_DIR and not
+ * marked for close to be included in the list. */
+#define DIR_CONN_LIST_TEMPLATE(conn_var, conn_test,             \
+                               dirconn_var, dirconn_test)       \
+  STMT_BEGIN                                                    \
+    smartlist_t *conns = get_connection_array();                \
+    smartlist_t *dir_conns = smartlist_new();                   \
+    SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn_var) {  \
+      if (conn_var && (conn_test)                               \
+          && conn_var->type == CONN_TYPE_DIR                    \
+          && !conn_var->marked_for_close) {                     \
+        dir_connection_t *dirconn_var = TO_DIR_CONN(conn_var);  \
+        if (dirconn_var && (dirconn_test)) {                    \
+          smartlist_add(dir_conns, dirconn_var);                \
+        }                                                       \
+      }                                                         \
+    } SMARTLIST_FOREACH_END(conn_var);                          \
+    return dir_conns;                                           \
+  STMT_END
+
+/** Return a list of directory connections that are fetching the item
+ * described by <b>purpose</b>/<b>resource</b>. If there are none,
+ * return an empty list. This list must be freed using smartlist_free,
+ * but the pointers in it must not be freed.
+ * Note that this list should not be cached, as the pointers in it can be
+ * freed if their connections close. */
+smartlist_t *
+connection_dir_list_by_purpose_and_resource(
+                                            int purpose,
+                                            const char *resource)
+{
+  DIR_CONN_LIST_TEMPLATE(conn,
+                         conn->purpose == purpose,
+                         dirconn,
+                         0 == strcmp_opt(resource,
+                                         dirconn->requested_resource));
+}
 
-  return NULL;
+/** Return a directory connection (if any one exists) that is fetching
+ * the item described by <b>purpose</b>/<b>resource</b>/<b>state</b>,
+ * otherwise return NULL. */
+dir_connection_t *
+connection_dir_get_by_purpose_resource_and_state(
+                                                 int purpose,
+                                                 const char *resource,
+                                                 int state)
+{
+  smartlist_t *conns =
+    connection_dir_list_by_purpose_resource_and_state(
+                                                      purpose,
+                                                      resource,
+                                                      state);
+  CONN_FIRST_AND_FREE_TEMPLATE(conns);
+}
+
+#undef CONN_FIRST_AND_FREE_TEMPLATE
+
+/** Return a list of directory connections that are fetching the item
+ * described by <b>purpose</b>/<b>resource</b>/<b>state</b>. If there are
+ * none, return an empty list. This list must be freed using smartlist_free,
+ * but the pointers in it must not be freed.
+ * Note that this list should not be cached, as the pointers in it can be
+ * freed if their connections close. */
+smartlist_t *
+connection_dir_list_by_purpose_resource_and_state(
+                                                  int purpose,
+                                                  const char *resource,
+                                                  int state)
+{
+  DIR_CONN_LIST_TEMPLATE(conn,
+                         conn->purpose == purpose && conn->state == state,
+                         dirconn,
+                         0 == strcmp_opt(resource,
+                                         dirconn->requested_resource));
+}
+
+#undef DIR_CONN_LIST_TEMPLATE
+
+/** Return an arbitrary active OR connection that isn't <b>this_conn</b>.
+ *
+ * We use this to guess if we should tell the controller that we
+ * didn't manage to connect to any of our bridges. */
+static connection_t *
+connection_get_another_active_or_conn(const or_connection_t *this_conn)
+{
+  CONN_GET_TEMPLATE(conn,
+                    conn != TO_CONN(this_conn) && conn->type == CONN_TYPE_OR);
 }
 
 /** Return 1 if there are any active OR connections apart from
@@ -4354,23 +4435,18 @@ connection_dir_get_by_purpose_and_resource(int purpose,
 int
 any_other_active_or_conns(const or_connection_t *this_conn)
 {
-  smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
-    if (conn == TO_CONN(this_conn)) { /* don't consider this conn */
-      continue;
-    }
-
-    if (conn->type == CONN_TYPE_OR &&
-        !conn->marked_for_close) {
-      log_debug(LD_DIR, "%s: Found an OR connection: %s",
-                __func__, conn->address);
-      return 1;
-    }
-  } SMARTLIST_FOREACH_END(conn);
+  connection_t *conn = connection_get_another_active_or_conn(this_conn);
+  if (conn != NULL) {
+    log_debug(LD_DIR, "%s: Found an OR connection: %s",
+              __func__, conn->address);
+    return 1;
+  }
 
   return 0;
 }
 
+#undef CONN_GET_TEMPLATE
+
 /** Return 1 if <b>conn</b> is a listener conn, else return 0. */
 int
 connection_is_listener(connection_t *conn)

src/or/connection.h

@@ -193,7 +193,57 @@ connection_t *connection_get_by_type_state(int type, int state);
 connection_t *connection_get_by_type_state_rendquery(int type, int state,
                                                      const char *rendquery);
 dir_connection_t *connection_dir_get_by_purpose_and_resource(
-                                           int state, const char *resource);
+                                                  int purpose,
+                                                  const char *resource);
+dir_connection_t *connection_dir_get_by_purpose_resource_and_state(
+                                                  int purpose,
+                                                  const char *resource,
+                                                  int state);
+smartlist_t *connection_dir_list_by_purpose_and_resource(
+                                                  int purpose,
+                                                  const char *resource);
+smartlist_t *connection_dir_list_by_purpose_resource_and_state(
+                                                  int purpose,
+                                                  const char *resource,
+                                                  int state);
+
+#define CONN_LEN_AND_FREE_TEMPLATE(sl) \
+  STMT_BEGIN                           \
+    int len = smartlist_len(sl);       \
+    smartlist_free(sl);                \
+    return len;                        \
+  STMT_END
+
+/** Return a count of directory connections that are fetching the item
+ * described by <b>purpose</b>/<b>resource</b>. */
+static INLINE int
+connection_dir_count_by_purpose_and_resource(
+                                             int purpose,
+                                             const char *resource)
+{
+  smartlist_t *conns = connection_dir_list_by_purpose_and_resource(
+                                                                   purpose,
+                                                                   resource);
+  CONN_LEN_AND_FREE_TEMPLATE(conns);
+}
+
+/** Return a count of directory connections that are fetching the item
+ * described by <b>purpose</b>/<b>resource</b>/<b>state</b>. */
+static INLINE int
+connection_dir_count_by_purpose_resource_and_state(
+                                                   int purpose,
+                                                   const char *resource,
+                                                   int state)
+{
+  smartlist_t *conns =
+    connection_dir_list_by_purpose_resource_and_state(
+                                                      purpose,
+                                                      resource,
+                                                      state);
+  CONN_LEN_AND_FREE_TEMPLATE(conns);
+}
+
+#undef CONN_LEN_AND_FREE_TEMPLATE
 
 int any_other_active_or_conns(const or_connection_t *this_conn);
 
@@ -239,6 +289,13 @@ void connection_buckets_note_empty_ts(uint32_t *timestamp_var,
                                       int tokens_before,
                                       size_t tokens_removed,
                                       const struct timeval *tvnow);
+MOCK_DECL(STATIC int,connection_connect_sockaddr,
+                                            (connection_t *conn,
+                                             const struct sockaddr *sa,
+                                             socklen_t sa_len,
+                                             const struct sockaddr *bindaddr,
+                                             socklen_t bindaddr_len,
+                                             int *socket_error));
 #endif
 
 #endif

src/or/directory.c

@@ -425,14 +425,17 @@ 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().
  */
-MOCK_IMPL(void, directory_get_from_dirserver, (uint8_t dir_purpose,
-                                               uint8_t router_purpose,
-                                               const char *resource,
-                                               int pds_flags))
+MOCK_IMPL(void, directory_get_from_dirserver, (
+                            uint8_t dir_purpose,
+                            uint8_t router_purpose,
+                            const char *resource,
+                            int pds_flags,
+                            download_want_authority_t want_authority))
 {
   const routerstatus_t *rs = NULL;
   const or_options_t *options = get_options();
-  int prefer_authority = directory_fetches_from_authorities(options);
+  int prefer_authority = (directory_fetches_from_authorities(options)
+                          || want_authority == DL_WANT_AUTHORITY);
   int require_authority = 0;
   int get_via_tor = purpose_needs_anonymity(dir_purpose, router_purpose);
   dirinfo_type_t type = dir_fetch_type(dir_purpose, router_purpose, resource);
@@ -958,6 +961,12 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
     return;
   }
 
+  /* ensure we don't make excess connections when we're already downloading
+   * a consensus during bootstrap */
+  if (connection_dir_avoid_extra_connection_for_purpose(dir_purpose)) {
+    return;
+  }
+
   conn = dir_connection_new(tor_addr_family(&addr));
 
   /* set up conn so it's got all the data we need to remember */
@@ -998,6 +1007,9 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
         conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING;
         /* fall through */
       case 0:
+        if (connection_dir_close_consensus_conn_if_extra(conn)) {
+          return;
+        }
         /* queue the command on the outbuf */
         directory_send_command(conn, dir_purpose, 1, resource,
                                payload, payload_len,
@@ -1041,6 +1053,9 @@ directory_initiate_command_rend(const tor_addr_t *_addr,
       connection_mark_for_close(TO_CONN(conn));
       return;
     }
+    if (connection_dir_close_consensus_conn_if_extra(conn)) {
+      return;
+    }
     conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING;
     /* queue the command on the outbuf */
     directory_send_command(conn, dir_purpose, 0, resource,
@@ -3423,8 +3438,205 @@ connection_dir_finished_flushing(dir_connection_t *conn)
   return 0;
 }
 
+/* A helper function for connection_dir_close_consensus_conn_if_extra()
+ * and connection_dir_close_extra_consensus_conns() that returns 0 if
+ * we can't have, or don't want to close, excess consensus connections. */
+int
+connection_dir_would_close_consensus_conn_helper(void)
+{
+  const or_options_t *options = get_options();
+
+  /* we're only interested in closing excess connections if we could
+   * have created any in the first place */
+  if (!networkstatus_consensus_can_use_multiple_directories(options)) {
+    return 0;
+  }
+
+  /* We want to close excess connections downloading a consensus.
+   * If there aren't any excess, we don't have anything to close. */
+  if (!networkstatus_consensus_has_excess_connections()) {
+    return 0;
+  }
+
+  /* If we have excess connections, but none of them are downloading a
+   * consensus, and we are still bootstrapping (that is, we have no usable
+   * consensus), we don't want to close any until one starts downloading. */
+  if (!networkstatus_consensus_is_downloading_usable_flavor()
+      && networkstatus_consensus_is_boostrapping(time(NULL))) {
+    return 0;
+  }
+
+  /* If we have just stopped bootstrapping (that is, just parsed a consensus),
+   * we might still have some excess connections hanging around. So we still
+   * have to check if we want to close any, even if we've stopped
+   * bootstrapping. */
+  return 1;
+}
+
+/* Check if we would close excess consensus connections. If we would, any
+ * new consensus connection would become excess immediately, so return 1.
+ * Otherwise, return 0. */
+int
+connection_dir_avoid_extra_connection_for_purpose(unsigned int purpose)
+{
+  const or_options_t *options = get_options();
+
+  /* We're not interested in connections that aren't fetching a consensus. */
+  if (purpose != DIR_PURPOSE_FETCH_CONSENSUS) {
+    return 0;
+  }
+
+  /* we're only interested in avoiding excess connections if we could
+   * have created any in the first place */
+  if (!networkstatus_consensus_can_use_multiple_directories(options)) {
+    return 0;
+  }
+
+  /* If there are connections downloading a consensus, and we are still
+   * bootstrapping (that is, we have no usable consensus), we can be sure that
+   * any further connections would be excess. */
+  if (networkstatus_consensus_is_downloading_usable_flavor()
+      && networkstatus_consensus_is_boostrapping(time(NULL))) {
+    return 1;
+  }
+
+  return 0;
+}
+
+/* Check if we have excess consensus download connection attempts, and close
+ * conn:
+ * - if we don't have a consensus, and we're downloading a consensus, and conn
+ *   is not downloading a consensus yet, close it;
+ * - if we do have a consensus, conn is excess, close it. */
+int
+connection_dir_close_consensus_conn_if_extra(dir_connection_t *conn)
+{
+  tor_assert(conn);
+  tor_assert(conn->base_.type == CONN_TYPE_DIR);
+
+  /* We're not interested in connections that aren't fetching a consensus. */
+  if (conn->base_.purpose != DIR_PURPOSE_FETCH_CONSENSUS) {
+    return 0;
+  }
+
+  /* The connection has already been closed */
+  if (conn->base_.marked_for_close) {
+    return 0;
+  }
+
+  if (!connection_dir_would_close_consensus_conn_helper()) {
+    return 0;
+  }
+
+  const int we_are_bootstrapping = networkstatus_consensus_is_boostrapping(
+                                                                  time(NULL));
+
+  /* We don't want to check other connections to see if they are downloading,
+   * as this is prone to race-conditions. So leave it for
+   * connection_dir_consider_close_extra_consensus_conns() to clean up.
+   *
+   * But if conn has just started connecting, or we have a consensus already,
+   * we can be sure it's not needed any more. */
+  if (!we_are_bootstrapping
+      || conn->base_.state == DIR_CONN_STATE_CONNECTING) {
+    connection_close_immediate(&conn->base_);
+    connection_mark_for_close(&conn->base_);
+    return -1;
+  }
+
+  return 0;
+}
+
+/* Check if we have excess consensus download connection attempts, and close
+ * them:
+ * - if we don't have a consensus, and we're downloading a consensus, keep an
+ *   earlier connection, or a connection to a fallback directory, and close
+ *   all other connections;
+ * - if we do have a consensus, close all connections: they are all excess. */
+void
+connection_dir_close_extra_consensus_conns(void)
+{
+  if (!connection_dir_would_close_consensus_conn_helper()) {
+    return;
+  }
+
+  int we_are_bootstrapping = networkstatus_consensus_is_boostrapping(
+                                                                  time(NULL));
+
+  const char *usable_resource = networkstatus_get_flavor_name(
+                                                  usable_consensus_flavor());
+  smartlist_t *consens_usable_conns =
+                 connection_dir_list_by_purpose_and_resource(
+                                                  DIR_PURPOSE_FETCH_CONSENSUS,
+                                                  usable_resource);
+
+  /* If we want to keep a connection that's downloading, find a connection to
+   * keep, favouring:
+   * - connections opened earlier (they are likely to have progressed further)
+   * - connections to fallbacks (to reduce the load on authorities) */
+  dir_connection_t *kept_download_conn = NULL;
+  int kept_is_authority = 0;
+  if (we_are_bootstrapping) {
+    SMARTLIST_FOREACH_BEGIN(consens_usable_conns,
+                            dir_connection_t *, d) {
+      tor_assert(d);
+      int d_is_authority = router_digest_is_trusted_dir(d->identity_digest);
+      /* keep the first connection that is past the connecting state, but
+       * prefer fallbacks. */
+      if (d->base_.state != DIR_CONN_STATE_CONNECTING) {
+        if (!kept_download_conn || (kept_is_authority && !d_is_authority)) {
+          kept_download_conn = d;
+          kept_is_authority = d_is_authority;
+          /* we've found the earliest fallback, and want to keep it regardless
+           * of any other connections */
+          if (!kept_is_authority)
+            break;
+        }
+      }
+    } SMARTLIST_FOREACH_END(d);
+  }
+
+  SMARTLIST_FOREACH_BEGIN(consens_usable_conns,
+                          dir_connection_t *, d) {
+    tor_assert(d);
+    /* don't close this connection if it's the one we want to keep */
+    if (kept_download_conn && d == kept_download_conn)
+      continue;
+    /* mark all other connections for close */
+    if (!d->base_.marked_for_close) {
+      connection_close_immediate(&d->base_);
+      connection_mark_for_close(&d->base_);
+    }
+  } SMARTLIST_FOREACH_END(d);
+
+  smartlist_free(consens_usable_conns);
+  consens_usable_conns = NULL;
+
+  /* make sure we've closed all excess connections */
+  const int final_connecting_conn_count =
+              connection_dir_count_by_purpose_resource_and_state(
+                                                DIR_PURPOSE_FETCH_CONSENSUS,
+                                                usable_resource,
+                                                DIR_CONN_STATE_CONNECTING);
+  if (final_connecting_conn_count > 0) {
+    log_warn(LD_BUG, "Expected 0 consensus connections connecting after "
+             "cleanup, got %d.", final_connecting_conn_count);
+  }
+  const int expected_final_conn_count = (we_are_bootstrapping ? 1 : 0);
+  const int final_conn_count =
+              connection_dir_count_by_purpose_and_resource(
+                                                DIR_PURPOSE_FETCH_CONSENSUS,
+                                                usable_resource);
+  if (final_conn_count > expected_final_conn_count) {
+    log_warn(LD_BUG, "Expected %d consensus connections after cleanup, got "
+             "%d.", expected_final_conn_count, final_connecting_conn_count);
+  }
+}
+
 /** Connected handler for directory connections: begin sending data to the
- * server */
+ * server, and return 0, or, if the connection is an excess bootstrap
+ * connection, close all excess bootstrap connections.
+ * Only used when connections don't immediately connect. */
 int
 connection_dir_finished_connecting(dir_connection_t *conn)
 {
@@ -3435,31 +3647,64 @@ connection_dir_finished_connecting(dir_connection_t *conn)
   log_debug(LD_HTTP,"Dir connection to router %s:%u established.",
             conn->base_.address,conn->base_.port);
 
-  conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING; /* start flushing conn */
+  if (connection_dir_close_consensus_conn_if_extra(conn)) {
+    return -1;
+  }
+
+  /* start flushing conn */
+  conn->base_.state = DIR_CONN_STATE_CLIENT_SENDING;
   return 0;
 }
 
 /** Decide which download schedule we want to use based on descriptor type
- * in <b>dls</b> and whether we are acting as directory <b>server</b>, and
- * then return a list of int pointers defining download delays in seconds.
- * Helper function for download_status_increment_failure() and
- * download_status_reset(). */
+ * in <b>dls</b> and <b>options</b>.
+ * Then return a list of int pointers defining download delays in seconds.
+ * Helper function for download_status_increment_failure(),
+ * download_status_reset(), and download_status_increment_attempt(). */
 static const smartlist_t *
-find_dl_schedule_and_len(download_status_t *dls, int server)
+find_dl_schedule(download_status_t *dls, const or_options_t *options)
 {
+  /* XX/teor Replace with dir_server_mode from #12538 */
+  const int dir_server = options->DirPort_set;
+  const int multi_d = networkstatus_consensus_can_use_multiple_directories(
+                                                                    options);
+  const int we_are_bootstrapping = networkstatus_consensus_is_boostrapping(
+                                                                 time(NULL));
+  const int use_fallbacks = networkstatus_consensus_can_use_extra_fallbacks(
+                                                                    options);
   switch (dls->schedule) {
     case DL_SCHED_GENERIC:
-      if (server)
-        return get_options()->TestingServerDownloadSchedule;
-      else
-        return get_options()->TestingClientDownloadSchedule;
+      if (dir_server) {
+        return options->TestingServerDownloadSchedule;
+      } else {
+        return options->TestingClientDownloadSchedule;
+      }
     case DL_SCHED_CONSENSUS:
-      if (server)
-        return get_options()->TestingServerConsensusDownloadSchedule;
-      else
-        return get_options()->TestingClientConsensusDownloadSchedule;
+      if (!multi_d) {
+        return options->TestingServerConsensusDownloadSchedule;
+      } else {
+        if (we_are_bootstrapping) {
+          if (!use_fallbacks) {
+            /* A bootstrapping client without extra fallback directories */
+            return
+         options->TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule;
+          } else if (dls->want_authority) {
+            /* A bootstrapping client with extra fallback directories, but
+             * connecting to an authority */
+            return
+             options->TestingClientBootstrapConsensusAuthorityDownloadSchedule;
+          } else {
+            /* A bootstrapping client connecting to extra fallback directories
+             */
+            return
+              options->TestingClientBootstrapConsensusFallbackDownloadSchedule;
+          }
+        } else {
+          return options->TestingClientConsensusDownloadSchedule;
+        }
+      }
     case DL_SCHED_BRIDGE:
-      return get_options()->TestingBridgeDownloadSchedule;
+      return options->TestingBridgeDownloadSchedule;
     default:
       tor_assert(0);
   }
@@ -3468,54 +3713,168 @@ find_dl_schedule_and_len(download_status_t *dls, int server)
   return NULL;
 }
 
-/** Called when an attempt to download <b>dls</b> has failed with HTTP status
+/* Find the current delay for dls based on schedule.
+ * Set dls->next_attempt_at based on now, and return the delay.
+ * Helper for download_status_increment_failure and
+ * download_status_increment_attempt. */
+STATIC int
+download_status_schedule_get_delay(download_status_t *dls,
+                                   const smartlist_t *schedule,
+                                   time_t now)
+{
+  tor_assert(dls);
+  tor_assert(schedule);
+
+  int delay = INT_MAX;
+  uint8_t dls_schedule_position = (dls->increment_on
+                                   == DL_SCHED_INCREMENT_ATTEMPT
+                                   ? dls->n_download_attempts
+                                   : dls->n_download_failures);
+
+  if (dls_schedule_position < smartlist_len(schedule))
+    delay = *(int *)smartlist_get(schedule, dls_schedule_position);
+  else if (dls_schedule_position == IMPOSSIBLE_TO_DOWNLOAD)
+    delay = INT_MAX;
+  else
+    delay = *(int *)smartlist_get(schedule, smartlist_len(schedule) - 1);
+
+  /* A negative delay makes no sense. Knowing that delay is
+   * non-negative allows us to safely do the wrapping check below. */
+  tor_assert(delay >= 0);
+
+  /* Avoid now+delay overflowing INT_MAX, by comparing with a subtraction
+   * that won't overflow (since delay is non-negative). */
+  if (delay < INT_MAX && now <= INT_MAX - delay) {
+    dls->next_attempt_at = now+delay;
+  } else {
+    dls->next_attempt_at = TIME_MAX;
+  }
+
+  return delay;
+}
+
+/* Log a debug message about item, which increments on increment_action, has
+ * incremented dls_n_download_increments times. The message varies based on
+ * was_schedule_incremented (if not, not_incremented_response is logged), and
+ * the values of increment, dls_next_attempt_at, and now.
+ * Helper for download_status_increment_failure and
+ * download_status_increment_attempt. */
+static void
+download_status_log_helper(const char *item, int was_schedule_incremented,
+                           const char *increment_action,
+                           const char *not_incremented_response,
+                           uint8_t dls_n_download_increments, int increment,
+                           time_t dls_next_attempt_at, time_t now)
+{
+  if (item) {
+    if (!was_schedule_incremented)
+      log_debug(LD_DIR, "%s %s %d time(s); I'll try again %s.",
+                item, increment_action, (int)dls_n_download_increments,
+                not_incremented_response);
+    else if (increment == 0)
+      log_debug(LD_DIR, "%s %s %d time(s); I'll try again immediately.",
+                item, increment_action, (int)dls_n_download_increments);
+    else if (dls_next_attempt_at < TIME_MAX)
+      log_debug(LD_DIR, "%s %s %d time(s); I'll try again in %d seconds.",
+                item, increment_action, (int)dls_n_download_increments,
+                (int)(dls_next_attempt_at-now));
+    else
+      log_debug(LD_DIR, "%s %s %d time(s); Giving up for a while.",
+                item, increment_action, (int)dls_n_download_increments);
+  }
+}
+
+/** Determine when a failed download attempt should be retried.
+ * Called when an attempt to download <b>dls</b> has failed with HTTP status
  * <b>status_code</b>.  Increment the failure count (if the code indicates a
- * real failure) and set <b>dls</b>-\>next_attempt_at to an appropriate time
- * in the future. */
+ * real failure, or if we're a server) and set <b>dls</b>-\>next_attempt_at to
+ * an appropriate time in the future and return it.
+ * If <b>dls->increment_on</b> is DL_SCHED_INCREMENT_ATTEMPT, increment the
+ * failure count, and return a time in the far future for the next attempt (to
+ * avoid an immediate retry). */
 time_t
 download_status_increment_failure(download_status_t *dls, int status_code,
                                   const char *item, int server, time_t now)
 {
-  const smartlist_t *schedule;
-  int increment;
+  int increment = -1;
   tor_assert(dls);
+
+  /* only count the failure if it's permanent, or we're a server */
   if (status_code != 503 || server) {
     if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1)
       ++dls->n_download_failures;
   }
 
-  schedule = find_dl_schedule_and_len(dls, server);
+  if (dls->increment_on == DL_SCHED_INCREMENT_FAILURE) {
+    /* We don't find out that a failure-based schedule has attempted a
+     * connection until that connection fails.
+     * We'll never find out about successful connections, but this doesn't
+     * matter, because schedules are reset after a successful download.
+     */
+    if (dls->n_download_attempts < IMPOSSIBLE_TO_DOWNLOAD-1)
+      ++dls->n_download_attempts;
 
-  if (dls->n_download_failures < smartlist_len(schedule))
-    increment = *(int *)smartlist_get(schedule, dls->n_download_failures);
-  else if (dls->n_download_failures == IMPOSSIBLE_TO_DOWNLOAD)
-    increment = INT_MAX;
-  else
-    increment = *(int *)smartlist_get(schedule, smartlist_len(schedule) - 1);
+    /* only return a failure retry time if this schedule increments on failures
+     */
+    const smartlist_t *schedule = find_dl_schedule(dls, get_options());
+    increment = download_status_schedule_get_delay(dls, schedule, now);
+  }
 
-  if (increment < INT_MAX)
-    dls->next_attempt_at = now+increment;
-  else
-    dls->next_attempt_at = TIME_MAX;
+  download_status_log_helper(item, !dls->increment_on, "failed",
+                             "concurrently", dls->n_download_failures,
+                             increment, dls->next_attempt_at, now);
 
-  if (item) {
-    if (increment == 0)
-      log_debug(LD_DIR, "%s failed %d time(s); I'll try again immediately.",
-                item, (int)dls->n_download_failures);
-    else if (dls->next_attempt_at < TIME_MAX)
-      log_debug(LD_DIR, "%s failed %d time(s); I'll try again in %d seconds.",
-                item, (int)dls->n_download_failures,
-                (int)(dls->next_attempt_at-now));
-    else
-      log_debug(LD_DIR, "%s failed %d time(s); Giving up for a while.",
-                item, (int)dls->n_download_failures);
+  if (dls->increment_on == DL_SCHED_INCREMENT_ATTEMPT) {
+    /* stop this schedule retrying on failure, it will launch concurrent
+     * connections instead */
+    return TIME_MAX;
+  } else {
+    return dls->next_attempt_at;
+  }
+}
+
+/** Determine when the next download attempt should be made when using an
+ * attempt-based (potentially concurrent) download schedule.
+ * Called when an attempt to download <b>dls</b> is being initiated.
+ * Increment the attempt count and set <b>dls</b>-\>next_attempt_at to an
+ * appropriate time in the future and return it.
+ * If <b>dls->increment_on</b> is DL_SCHED_INCREMENT_FAILURE, don't increment
+ * the attempts, and return a time in the far future (to avoid launching a
+ * concurrent attempt). */
+time_t
+download_status_increment_attempt(download_status_t *dls, const char *item,
+                                  time_t now)
+{
+  int delay = -1;
+  tor_assert(dls);
+
+  if (dls->increment_on == DL_SCHED_INCREMENT_FAILURE) {
+    /* this schedule should retry on failure, and not launch any concurrent
+     attempts */
+    log_info(LD_BUG, "Tried to launch an attempt-based connection on a "
+             "failure-based schedule.");
+    return TIME_MAX;
   }
+
+  if (dls->n_download_attempts < IMPOSSIBLE_TO_DOWNLOAD-1)
+    ++dls->n_download_attempts;
+
+  const smartlist_t *schedule = find_dl_schedule(dls, get_options());
+  delay = download_status_schedule_get_delay(dls, schedule, now);
+
+  download_status_log_helper(item, dls->increment_on, "attempted",
+                             "on failure", dls->n_download_attempts,
+                             delay, dls->next_attempt_at, now);
+
   return dls->next_attempt_at;
 }
 
 /** Reset <b>dls</b> so that it will be considered downloadable
  * immediately, and/or to show that we don't need it anymore.
  *
+ * Must be called to initialise a download schedule, otherwise the zeroth item
+ * in the schedule will never be used.
+ *
  * (We find the zeroth element of the download schedule, and set
  * next_attempt_at to be the appropriate offset from 'now'. In most
  * cases this means setting it to 'now', so the item will be immediately
@@ -3524,14 +3883,16 @@ download_status_increment_failure(download_status_t *dls, int status_code,
 void
 download_status_reset(download_status_t *dls)
 {
-  if (dls->n_download_failures == IMPOSSIBLE_TO_DOWNLOAD)
+  if (dls->n_download_failures == IMPOSSIBLE_TO_DOWNLOAD
+      || dls->n_download_attempts == IMPOSSIBLE_TO_DOWNLOAD)
     return; /* Don't reset this. */
 
-  const smartlist_t *schedule = find_dl_schedule_and_len(
-                          dls, get_options()->DirPort_set);
+  const smartlist_t *schedule = find_dl_schedule(dls, get_options());
 
   dls->n_download_failures = 0;
+  dls->n_download_attempts = 0;
   dls->next_attempt_at = time(NULL) + *(int *)smartlist_get(schedule, 0);
+  /* Don't reset dls->want_authority or dls->increment_on */
 }
 
 /** Return the number of failures on <b>dls</b> since the last success (if
@@ -3542,6 +3903,22 @@ download_status_get_n_failures(const download_status_t *dls)
   return dls->n_download_failures;
 }
 
+/** Return the number of attempts to download <b>dls</b> since the last success
+ * (if any). This can differ from download_status_get_n_failures() due to
+ * outstanding concurrent attempts. */
+int
+download_status_get_n_attempts(const download_status_t *dls)
+{
+  return dls->n_download_attempts;
+}
+
+/** Return the next time to attempt to download <b>dls</b>. */
+time_t
+download_status_get_next_attempt_at(const download_status_t *dls)
+{
+  return dls->next_attempt_at;
+}
+
 /** Called when one or more routerdesc (or extrainfo, if <b>was_extrainfo</b>)
  * fetches have failed (with uppercase fingerprints listed in <b>failed</b>,
  * either as descriptor digests or as identity digests based on

src/or/directory.h

@@ -16,10 +16,12 @@ 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);
-MOCK_DECL(void, directory_get_from_dirserver, (uint8_t dir_purpose,
-                                               uint8_t router_purpose,
-                                               const char *resource,
-                                               int pds_flags));
+MOCK_DECL(void, directory_get_from_dirserver, (
+                          uint8_t dir_purpose,
+                          uint8_t router_purpose,
+                          const char *resource,
+                          int pds_flags,
+                          download_want_authority_t want_authority));
 void directory_get_from_all_authorities(uint8_t dir_purpose,
                                         uint8_t router_purpose,
                                         const char *resource);
@@ -72,6 +74,9 @@ void directory_initiate_command(const tor_addr_t *addr,
                                 const char *resource,
                                 const char *payload, size_t payload_len,
                                 time_t if_modified_since);
+int connection_dir_avoid_extra_connection_for_purpose(unsigned int purpose);
+int connection_dir_close_consensus_conn_if_extra(dir_connection_t *conn);
+void connection_dir_close_extra_consensus_conns(void);
 
 #define DSR_HEX       (1<<0)
 #define DSR_BASE64    (1<<1)
@@ -90,6 +95,8 @@ int router_supports_extrainfo(const char *identity_digest, int is_authority);
 time_t download_status_increment_failure(download_status_t *dls,
                                          int status_code, const char *item,
                                          int server, time_t now);
+time_t download_status_increment_attempt(download_status_t *dls,
+                                         const char *item,  time_t now);
 /** Increment the failure count of the download_status_t <b>dls</b>, with
  * the optional status code <b>sc</b>. */
 #define download_status_failed(dls, sc)                                 \
@@ -105,8 +112,9 @@ static inline int
 download_status_is_ready(download_status_t *dls, time_t now,
                          int max_failures)
 {
-  return (dls->n_download_failures <= max_failures
-          && dls->next_attempt_at <= now);
+  int under_failure_limit = (dls->n_download_failures <= max_failures
+                             && dls->n_download_attempts <= max_failures);
+  return (under_failure_limit && dls->next_attempt_at <= now);
 }
 
 static void download_status_mark_impossible(download_status_t *dl);
@@ -115,9 +123,12 @@ static inline void
 download_status_mark_impossible(download_status_t *dl)
 {
   dl->n_download_failures = IMPOSSIBLE_TO_DOWNLOAD;
+  dl->n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD;
 }
 
 int download_status_get_n_failures(const download_status_t *dls);
+int download_status_get_n_attempts(const download_status_t *dls);
+time_t download_status_get_next_attempt_at(const download_status_t *dls);
 
 #ifdef TOR_UNIT_TESTS
 /* Used only by directory.c and test_dir.c */
@@ -131,6 +142,10 @@ STATIC int directory_handle_command_get(dir_connection_t *conn,
                                         const char *headers,
                                         const char *req_body,
                                         size_t req_body_len);
+int connection_dir_would_close_consensus_conn_helper(void);
+STATIC int download_status_schedule_get_delay(download_status_t *dls,
+                                              const smartlist_t *schedule,
+                                              time_t now);
 #endif
 
 #endif

src/or/entrynodes.c

@@ -2205,7 +2205,7 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now)
         log_info(LD_DIR, "Fetching bridge info '%s' from bridge authority.",
                  resource);
         directory_get_from_dirserver(DIR_PURPOSE_FETCH_SERVERDESC,
-                ROUTER_PURPOSE_BRIDGE, resource, 0);
+                ROUTER_PURPOSE_BRIDGE, resource, 0, DL_WANT_AUTHORITY);
       }
     }
   SMARTLIST_FOREACH_END(bridge);

src/or/main.c

@@ -1460,6 +1460,11 @@ run_scheduled_events(time_t now)
     dirvote_act(options, now);
   }
 
+  /* 2d. Cleanup excess consensus bootstrap connections every second.
+   * connection_dir_close_consensus_conn_if_extra() will close connections
+   * that are clearly excess, but this check is more thorough. */
+  connection_dir_close_extra_consensus_conns();
+
   /* 3a. Every second, we examine pending circuits and prune the
    *    ones which have been pending for more than a few seconds.
    *    We do this before step 4, so it can try building more if
@@ -1876,18 +1881,29 @@ check_for_reachability_bw_callback(time_t now, const or_options_t *options)
 static int
 fetch_networkstatus_callback(time_t now, const or_options_t *options)
 {
-  /* 2c. Every minute (or every second if TestingTorNetwork), check
-   * whether we want to download any networkstatus documents. */
+  /* 2c. Every minute (or every second if TestingTorNetwork, or during
+   * client bootstrap), check whether we want to download any networkstatus
+   * documents. */
 
   /* How often do we check whether we should download network status
    * documents? */
-#define networkstatus_dl_check_interval(o) ((o)->TestingTorNetwork ? 1 : 60)
+  const int we_are_bootstrapping = networkstatus_consensus_is_boostrapping(
+                                                                        now);
+  const int prefer_mirrors = !directory_fetches_from_authorities(
+                                                              get_options());
+  int networkstatus_dl_check_interval = 60;
+  /* check more often when testing, or when bootstrapping from mirrors
+   * (connection limits prevent too many connections being made) */
+  if (options->TestingTorNetwork
+      || (we_are_bootstrapping && prefer_mirrors)) {
+    networkstatus_dl_check_interval = 1;
+  }
 
   if (should_delay_dir_fetches(options, NULL))
     return PERIODIC_EVENT_NO_UPDATE;
 
   update_networkstatus_downloads(now);
-  return networkstatus_dl_check_interval(options);
+  return networkstatus_dl_check_interval;
 }
 
 static int

src/or/networkstatus.c

@@ -85,8 +85,30 @@ static time_t time_to_download_next_consensus[N_CONSENSUS_FLAVORS];
 /** Download status for the current consensus networkstatus. */
 static download_status_t consensus_dl_status[N_CONSENSUS_FLAVORS] =
   {
-    { 0, 0, DL_SCHED_CONSENSUS },
-    { 0, 0, DL_SCHED_CONSENSUS },
+    { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
+                                   DL_SCHED_INCREMENT_FAILURE },
+    { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
+                                   DL_SCHED_INCREMENT_FAILURE },
+  };
+
+#define N_CONSENSUS_BOOTSTRAP_SCHEDULES 2
+#define CONSENSUS_BOOTSTRAP_SOURCE_AUTHORITY 0
+#define CONSENSUS_BOOTSTRAP_SOURCE_ANY_DIRSERVER  1
+
+/* Using DL_SCHED_INCREMENT_ATTEMPT on these schedules means that
+ * download_status_increment_failure won't increment these entries.
+ * However, any bootstrap connection failures that occur after we have
+ * a valid consensus will count against the failure counts on the non-bootstrap
+ * schedules. There should only be one of these, as all the others will have
+ * been cancelled. (This doesn't seem to be a significant issue.) */
+static download_status_t
+              consensus_bootstrap_dl_status[N_CONSENSUS_BOOTSTRAP_SCHEDULES] =
+  {
+    { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_AUTHORITY,
+                                   DL_SCHED_INCREMENT_ATTEMPT },
+    /* During bootstrap, DL_WANT_ANY_DIRSERVER means "use fallbacks". */
+    { 0, 0, 0, DL_SCHED_CONSENSUS, DL_WANT_ANY_DIRSERVER,
+                                   DL_SCHED_INCREMENT_ATTEMPT },
   };
 
 /** True iff we have logged a warning about this OR's version being older than
@@ -97,6 +119,10 @@ static int have_warned_about_old_version = 0;
 static int have_warned_about_new_version = 0;
 
 static void routerstatus_list_update_named_server_map(void);
+static void update_consensus_bootstrap_multiple_downloads(
+                                                  time_t now,
+                                                  const or_options_t *options,
+                                                  int we_are_bootstrapping);
 
 /** Forget that we've warned about anything networkstatus-related, so we will
  * give fresh warnings if the same behavior happens again. */
@@ -122,6 +148,9 @@ networkstatus_reset_download_failures(void)
 
   for (i=0; i < N_CONSENSUS_FLAVORS; ++i)
     download_status_reset(&consensus_dl_status[i]);
+
+  for (i=0; i < N_CONSENSUS_BOOTSTRAP_SCHEDULES; ++i)
+    download_status_reset(&consensus_bootstrap_dl_status[i]);
 }
 
 /** Read every cached v3 consensus networkstatus from the disk. */
@@ -734,6 +763,55 @@ we_want_to_fetch_flavor(const or_options_t *options, int flavor)
  * fetching certs before we check whether there is a better one? */
 #define DELAY_WHILE_FETCHING_CERTS (20*60)
 
+/* Check if a downloaded consensus flavor should still wait for certificates
+ * to download now.
+ * If so, return 1. If not, fail dls and return 0. */
+static int
+check_consensus_waiting_for_certs(int flavor, time_t now,
+                                  download_status_t *dls)
+{
+  consensus_waiting_for_certs_t *waiting;
+
+  /* We should always have a known flavor, because we_want_to_fetch_flavor()
+   * filters out unknown flavors. */
+  tor_assert(flavor >= 0 && flavor < N_CONSENSUS_FLAVORS);
+
+  waiting = &consensus_waiting_for_certs[flavor];
+  if (waiting->consensus) {
+    /* XXXX make sure this doesn't delay sane downloads. */
+    if (waiting->set_at + DELAY_WHILE_FETCHING_CERTS > now) {
+      return 1;
+    } else {
+      if (!waiting->dl_failed) {
+        download_status_failed(dls, 0);
+        waiting->dl_failed=1;
+      }
+    }
+  }
+
+  return 0;
+}
+
+/* Return the maximum download tries for a consensus, based on options and
+ * whether we_are_bootstrapping. */
+static int
+consensus_max_download_tries(const or_options_t *options,
+                                        int we_are_bootstrapping)
+{
+  int use_fallbacks = networkstatus_consensus_can_use_extra_fallbacks(options);
+
+  if (we_are_bootstrapping) {
+    if (use_fallbacks) {
+      return options->TestingClientBootstrapConsensusMaxDownloadTries;
+    } else {
+      return
+      options->TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries;
+    }
+  }
+
+  return options->TestingConsensusMaxDownloadTries;
+}
+
 /** If we want to download a fresh consensus, launch a new download as
  * appropriate. */
 static void
@@ -741,12 +819,19 @@ update_consensus_networkstatus_downloads(time_t now)
 {
   int i;
   const or_options_t *options = get_options();
+  const int we_are_bootstrapping = networkstatus_consensus_is_boostrapping(
+                                                                        now);
+  const int use_multi_conn =
+    networkstatus_consensus_can_use_multiple_directories(options);
+
+  if (should_delay_dir_fetches(options, NULL))
+    return;
 
   for (i=0; i < N_CONSENSUS_FLAVORS; ++i) {
     /* XXXX need some way to download unknown flavors if we are caching. */
     const char *resource;
-    consensus_waiting_for_certs_t *waiting;
     networkstatus_t *c;
+    int max_in_progress_conns = 1;
 
     if (! we_want_to_fetch_flavor(options, i))
       continue;
@@ -762,35 +847,166 @@ update_consensus_networkstatus_downloads(time_t now)
 
     resource = networkstatus_get_flavor_name(i);
 
-    /* Let's make sure we remembered to update consensus_dl_status */
-    tor_assert(consensus_dl_status[i].schedule == DL_SCHED_CONSENSUS);
+    /* Check if we already have enough connections in progress */
+    if (we_are_bootstrapping) {
+      max_in_progress_conns =
+        options->TestingClientBootstrapConsensusMaxInProgressTries;
+    }
+    if (connection_dir_count_by_purpose_and_resource(
+                                                  DIR_PURPOSE_FETCH_CONSENSUS,
+                                                  resource)
+        >= max_in_progress_conns) {
+      continue;
+    }
 
-    if (!download_status_is_ready(&consensus_dl_status[i], now,
-                             options->TestingConsensusMaxDownloadTries))
-      continue; /* We failed downloading a consensus too recently. */
-    if (connection_dir_get_by_purpose_and_resource(
-                                DIR_PURPOSE_FETCH_CONSENSUS, resource))
-      continue; /* There's an in-progress download.*/
+    /* Check if we want to launch another download for a usable consensus.
+     * Only used during bootstrap. */
+    if (we_are_bootstrapping && use_multi_conn
+        && i == usable_consensus_flavor()) {
+
+      /* Check if we're already downloading a usable consensus */
+      int consens_conn_count =
+        connection_dir_count_by_purpose_and_resource(
+                                                   DIR_PURPOSE_FETCH_CONSENSUS,
+                                                   resource);
+      int connect_consens_conn_count =
+        connection_dir_count_by_purpose_resource_and_state(
+                                                   DIR_PURPOSE_FETCH_CONSENSUS,
+                                                   resource,
+                                                   DIR_CONN_STATE_CONNECTING);
+
+      if (i == usable_consensus_flavor()
+          && connect_consens_conn_count < consens_conn_count) {
+        continue;
+      }
 
-    waiting = &consensus_waiting_for_certs[i];
-    if (waiting->consensus) {
-      /* XXXX make sure this doesn't delay sane downloads. */
-      if (waiting->set_at + DELAY_WHILE_FETCHING_CERTS > now) {
-        continue; /* We're still getting certs for this one. */
-      } else {
-        if (!waiting->dl_failed) {
-          download_status_failed(&consensus_dl_status[i], 0);
-          waiting->dl_failed=1;
-        }
+      /* Make multiple connections for a bootstrap consensus download */
+      update_consensus_bootstrap_multiple_downloads(now, options,
+                                                    we_are_bootstrapping);
+    } else {
+      /* Check if we failed downloading a consensus too recently */
+      int max_dl_tries = consensus_max_download_tries(options,
+                                                      we_are_bootstrapping);
+
+      /* Let's make sure we remembered to update consensus_dl_status */
+      tor_assert(consensus_dl_status[i].schedule == DL_SCHED_CONSENSUS);
+
+      if (!download_status_is_ready(&consensus_dl_status[i],
+                                    now,
+                                    max_dl_tries)) {
+        continue;
       }
+
+      /* Check if we're waiting for certificates to download */
+      if (check_consensus_waiting_for_certs(i, now, &consensus_dl_status[i]))
+        continue;
+
+      /* Try the requested attempt */
+      log_info(LD_DIR, "Launching %s standard networkstatus consensus "
+               "download.", networkstatus_get_flavor_name(i));
+      directory_get_from_dirserver(DIR_PURPOSE_FETCH_CONSENSUS,
+                                   ROUTER_PURPOSE_GENERAL, resource,
+                                   PDS_RETRY_IF_NO_SERVERS,
+                                   consensus_dl_status[i].want_authority);
     }
+  }
+}
 
-    log_info(LD_DIR, "Launching %s networkstatus consensus download.",
-             networkstatus_get_flavor_name(i));
+/** When we're bootstrapping, launch one or more consensus download
+ * connections, if schedule indicates connection(s) should be made after now.
+ * If is_authority, connect to an authority, otherwise, use a fallback
+ * directory mirror.
+ */
+static void
+update_consensus_bootstrap_attempt_downloads(
+                                      time_t now,
+                                      const or_options_t *options,
+                                      int we_are_bootstrapping,
+                                      download_status_t *dls,
+                                      download_want_authority_t want_authority)
+{
+  int max_dl_tries = consensus_max_download_tries(options,
+                                                  we_are_bootstrapping);
+  const char *resource = networkstatus_get_flavor_name(
+                                                  usable_consensus_flavor());
+
+  /* Let's make sure we remembered to update schedule */
+  tor_assert(dls->schedule == DL_SCHED_CONSENSUS);
+
+  /* Allow for multiple connections in the same second, if the schedule value
+   * is 0. */
+  while (download_status_is_ready(dls, now, max_dl_tries)) {
+    log_info(LD_DIR, "Launching %s bootstrap %s networkstatus consensus "
+             "download.", resource, (want_authority == DL_WANT_AUTHORITY
+                                     ? "authority"
+                                     : "mirror"));
 
     directory_get_from_dirserver(DIR_PURPOSE_FETCH_CONSENSUS,
                                  ROUTER_PURPOSE_GENERAL, resource,
-                                 PDS_RETRY_IF_NO_SERVERS);
+                                 PDS_RETRY_IF_NO_SERVERS, want_authority);
+    /* schedule the next attempt */
+    download_status_increment_attempt(dls, resource, now);
+  }
+}
+
+/** If we're bootstrapping, check the connection schedules and see if we want
+ * to make additional, potentially concurrent, consensus download
+ * connections.
+ * Only call when bootstrapping, and when we want to make additional
+ * connections. Only nodes that satisfy
+ * networkstatus_consensus_can_use_multiple_directories make additonal
+ * connections.
+ */
+static void
+update_consensus_bootstrap_multiple_downloads(time_t now,
+                                              const or_options_t *options,
+                                              int we_are_bootstrapping)
+{
+  const int usable_flavor = usable_consensus_flavor();
+
+  /* make sure we can use multiple connections */
+  if (!networkstatus_consensus_can_use_multiple_directories(options)) {
+    return;
+  }
+
+  /* If we've managed to validate a usable consensus, don't make additonal
+   * connections. */
+  if (!we_are_bootstrapping) {
+    return;
+  }
+
+  /* Launch concurrent consensus download attempt(s) based on the mirror and
+   * authority schedules. Try the mirror first - this makes it slightly more
+   * likely that we'll connect to the fallback first, and then end the
+   * authority connection attempt. */
+
+  /* If a consensus download fails because it's waiting for certificates,
+   * we'll fail both the authority and fallback schedules. This is better than
+   * failing only one of the schedules, and having the other continue
+   * unchecked.
+   */
+
+  /* If we don't have or can't use extra fallbacks, don't try them. */
+  if (networkstatus_consensus_can_use_extra_fallbacks(options)) {
+    download_status_t *dls_f =
+      &consensus_bootstrap_dl_status[CONSENSUS_BOOTSTRAP_SOURCE_ANY_DIRSERVER];
+
+    if (!check_consensus_waiting_for_certs(usable_flavor, now, dls_f)) {
+      /* During bootstrap, DL_WANT_ANY_DIRSERVER means "use fallbacks". */
+      update_consensus_bootstrap_attempt_downloads(now, options,
+                                                   we_are_bootstrapping, dls_f,
+                                                   DL_WANT_ANY_DIRSERVER);
+    }
+  }
+
+  /* Now try an authority. */
+  download_status_t *dls_a =
+    &consensus_bootstrap_dl_status[CONSENSUS_BOOTSTRAP_SOURCE_AUTHORITY];
+
+  if (!check_consensus_waiting_for_certs(usable_flavor, now, dls_a)) {
+    update_consensus_bootstrap_attempt_downloads(now, options,
+                                                 we_are_bootstrapping, dls_a,
+                                                 DL_WANT_AUTHORITY);
   }
 }
 
@@ -1057,6 +1273,100 @@ networkstatus_get_reasonably_live_consensus(time_t now, int flavor)
     return NULL;
 }
 
+/** Check if we're bootstrapping a consensus download. This means that we are
+ *  only using the authorities and fallback directory mirrors to download the
+ * consensus flavour we'll use. */
+int
+networkstatus_consensus_is_boostrapping(time_t now)
+{
+  /* If we don't have a consensus, we must still be bootstrapping */
+  return !networkstatus_get_reasonably_live_consensus(
+                                                  now,
+                                                  usable_consensus_flavor());
+}
+
+/** Check if we can use multiple directories for a consensus download.
+ * Only clients (including bridges, but excluding bridge clients) benefit
+ * from multiple simultaneous consensus downloads. */
+int
+networkstatus_consensus_can_use_multiple_directories(
+                                                  const or_options_t *options)
+{
+  /* If we are a client, bridge, bridge client, or hidden service */
+  return (!directory_fetches_from_authorities(options));
+}
+
+/** Check if we can use fallback directory mirrors for a consensus download.
+ * Only clients that have a list of additional fallbacks can use fallbacks. */
+int
+networkstatus_consensus_can_use_extra_fallbacks(const or_options_t *options)
+{
+  /* If we are a client, and we have additional mirrors, we can use them.
+   * The list length comparisons are a quick way to check if we have any
+   * non-authority fallback directories. If we ever have any authorities that
+   * aren't fallback directories, we will need to change this code. */
+  return (!directory_fetches_from_authorities(options)
+          && (smartlist_len(router_get_fallback_dir_servers())
+              > smartlist_len(router_get_trusted_dir_servers())));
+}
+
+/* Check if there is more than 1 consensus connection retrieving the usable
+ * consensus flavor. If so, return 1, if not, return 0.
+ *
+ * During normal operation, Tor only makes one consensus download
+ * connection. But clients can make multiple simultaneous consensus
+ * connections to improve bootstrap speed and reliability.
+ *
+ * If there is more than one connection, we must have connections left
+ * over from bootstrapping. However, some of the connections may have
+ * completed and been cleaned up, so it is not sufficient to check the
+ * return value of this function to see if a client could make multiple
+ * bootstrap connections. Use
+ * networkstatus_consensus_can_use_multiple_directories()
+ * and networkstatus_consensus_is_boostrapping(). */
+int
+networkstatus_consensus_has_excess_connections(void)
+{
+  const char *usable_resource = networkstatus_get_flavor_name(
+                                                  usable_consensus_flavor());
+  const int consens_conn_usable_count =
+              connection_dir_count_by_purpose_and_resource(
+                                               DIR_PURPOSE_FETCH_CONSENSUS,
+                                               usable_resource);
+  /* The maximum number of connections we want downloading a usable consensus
+   * Always 1, whether bootstrapping or not. */
+  const int max_expected_consens_conn_usable_count = 1;
+
+  if (consens_conn_usable_count > max_expected_consens_conn_usable_count) {
+    return 1;
+  }
+
+  return 0;
+}
+
+/* Is tor currently downloading a consensus of the usable flavor? */
+int
+networkstatus_consensus_is_downloading_usable_flavor(void)
+{
+  const char *usable_resource = networkstatus_get_flavor_name(
+                                                  usable_consensus_flavor());
+  const int consens_conn_usable_count =
+              connection_dir_count_by_purpose_and_resource(
+                                               DIR_PURPOSE_FETCH_CONSENSUS,
+                                               usable_resource);
+
+  const int connect_consens_conn_usable_count =
+              connection_dir_count_by_purpose_resource_and_state(
+                                                DIR_PURPOSE_FETCH_CONSENSUS,
+                                                usable_resource,
+                                                DIR_CONN_STATE_CONNECTING);
+  if (connect_consens_conn_usable_count < consens_conn_usable_count) {
+    return 1;
+  }
+
+  return 0;
+}
+
 /** Given two router status entries for the same router identity, return 1 if
  * if the contents have changed between them. Otherwise, return 0. */
 static int

src/or/networkstatus.h

@@ -70,6 +70,14 @@ MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor,
 networkstatus_t *networkstatus_get_live_consensus(time_t now);
 networkstatus_t *networkstatus_get_reasonably_live_consensus(time_t now,
                                                              int flavor);
+int networkstatus_consensus_is_boostrapping(time_t now);
+int networkstatus_consensus_can_use_multiple_directories(
+                                                const or_options_t *options);
+int networkstatus_consensus_can_use_extra_fallbacks(
+                                                const or_options_t *options);
+int networkstatus_consensus_has_excess_connections(void);
+int networkstatus_consensus_is_downloading_usable_flavor(void);
+
 #define NSSET_FROM_CACHE 1
 #define NSSET_WAS_WAITING_FOR_CERTS 2
 #define NSSET_DONT_DOWNLOAD_CERTS 4

src/or/or.h

@@ -1946,8 +1946,8 @@ typedef enum {
 } saved_location_t;
 #define saved_location_bitfield_t ENUM_BF(saved_location_t)
 
-/** Enumeration: what kind of download schedule are we using for a given
- * object? */
+/** Enumeration: what directory object is being downloaded?
+ * This determines which schedule is selected to perform the download. */
 typedef enum {
   DL_SCHED_GENERIC = 0,
   DL_SCHED_CONSENSUS = 1,
@@ -1955,15 +1955,74 @@ typedef enum {
 } download_schedule_t;
 #define download_schedule_bitfield_t ENUM_BF(download_schedule_t)
 
+/** Enumeration: is the download schedule for downloading from an authority,
+ * or from any available directory mirror?
+ * During bootstrap, "any" means a fallback (or an authority, if there
+ * are no fallbacks).
+ * When we have a valid consensus, "any" means any directory server. */
+typedef enum {
+  DL_WANT_ANY_DIRSERVER = 0,
+  DL_WANT_AUTHORITY = 1,
+} download_want_authority_t;
+#define download_want_authority_bitfield_t \
+                                        ENUM_BF(download_want_authority_t)
+
+/** Enumeration: do we want to increment the schedule position each time a
+ * connection is attempted (these attempts can be concurrent), or do we want
+ * to increment the schedule position after a connection fails? */
+typedef enum {
+  DL_SCHED_INCREMENT_FAILURE = 0,
+  DL_SCHED_INCREMENT_ATTEMPT = 1,
+} download_schedule_increment_t;
+#define download_schedule_increment_bitfield_t \
+                                        ENUM_BF(download_schedule_increment_t)
+
 /** Information about our plans for retrying downloads for a downloadable
- * object. */
+ * directory object.
+ * Each type of downloadable directory object has a corresponding retry
+ * <b>schedule</b>, which can be different depending on whether the object is
+ * being downloaded from an authority or a mirror (<b>want_authority</b>).
+ * <b>next_attempt_at</b> contains the next time we will attempt to download
+ * the object.
+ * For schedules that <b>increment_on</b> failure, <b>n_download_failures</b>
+ * is used to determine the position in the schedule. (Each schedule is a
+ * smartlist of integer delays, parsed from a CSV option.) Every time a
+ * connection attempt fails, <b>n_download_failures</b> is incremented,
+ * the new delay value is looked up from the schedule, and
+ * <b>next_attempt_at</b> is set delay seconds from the time the previous
+ * connection failed. Therefore, at most one failure-based connection can be
+ * in progress for each download_status_t.
+ * For schedules that <b>increment_on</b> attempt, <b>n_download_attempts</b>
+ * is used to determine the position in the schedule. Every time a
+ * connection attempt is made, <b>n_download_attempts</b> is incremented,
+ * the new delay value is looked up from the schedule, and
+ * <b>next_attempt_at</b> is set delay seconds from the time the previous
+ * connection was attempted. Therefore, multiple concurrent attempted-based
+ * connections can be in progress for each download_status_t.
+ * After an object is successfully downloaded, any other concurrent connections
+ * are terminated. A new schedule which starts at position 0 is used for
+ * subsequent downloads of the same object.
+ */
 typedef struct download_status_t {
-  time_t next_attempt_at; /**< When should we try downloading this descriptor
+  time_t next_attempt_at; /**< When should we try downloading this object
                            * again? */
-  uint8_t n_download_failures; /**< Number of failures trying to download the
-                                * most recent descriptor. */
-  download_schedule_bitfield_t schedule : 8;
-
+  uint8_t n_download_failures; /**< Number of failed downloads of the most
+                                * recent object, since the last success. */
+  uint8_t n_download_attempts; /**< Number of (potentially concurrent) attempts
+                                * to download the most recent object, since
+                                * the last success. */
+  download_schedule_bitfield_t schedule : 8; /**< What kind of object is being
+                                              * downloaded? This determines the
+                                              * schedule used for the download.
+                                              */
+  download_want_authority_bitfield_t want_authority : 1; /**< Is the download
+                                              * happening from an authority
+                                              * or a mirror? This determines
+                                              * the schedule used for the
+                                              * download. */
+  download_schedule_increment_bitfield_t increment_on : 1; /**< does this
+                                        * schedule increment on each attempt,
+                                        * or after each failure? */
 } download_status_t;
 
 /** If n_download_failures is this high, the download can never happen. */
@@ -4071,6 +4130,36 @@ typedef struct {
    * on testing networks. */
   smartlist_t *TestingClientConsensusDownloadSchedule;
 
+  /** Schedule for when clients should download consensuses from authorities
+   * if they are bootstrapping (that is, they don't have a usable, reasonably
+   * live consensus).  Only used by clients fetching from a list of fallback
+   * directory mirrors.
+   *
+   * This schedule is incremented by (potentially concurrent) connection
+   * attempts, unlike other schedules, which are incremented by connection
+   * failures.  Only altered on testing networks. */
+  smartlist_t *TestingClientBootstrapConsensusAuthorityDownloadSchedule;
+
+  /** Schedule for when clients should download consensuses from fallback
+   * directory mirrors if they are bootstrapping (that is, they don't have a
+   * usable, reasonably live consensus). Only used by clients fetching from a
+   * list of fallback directory mirrors.
+   *
+   * This schedule is incremented by (potentially concurrent) connection
+   * attempts, unlike other schedules, which are incremented by connection
+   * failures.  Only altered on testing networks. */
+  smartlist_t *TestingClientBootstrapConsensusFallbackDownloadSchedule;
+
+  /** Schedule for when clients should download consensuses from authorities
+   * if they are bootstrapping (that is, they don't have a usable, reasonably
+   * live consensus).  Only used by clients which don't have or won't fetch
+   * from a list of fallback directory mirrors.
+   *
+   * This schedule is incremented by (potentially concurrent) connection
+   * attempts, unlike other schedules, which are incremented by connection
+   * failures.  Only altered on testing networks. */
+  smartlist_t *TestingClientBootstrapConsensusAuthorityOnlyDownloadSchedule;
+
   /** Schedule for when clients should download bridge descriptors.  Only
    * altered on testing networks. */
   smartlist_t *TestingBridgeDownloadSchedule;
@@ -4088,6 +4177,21 @@ typedef struct {
    * up?  Only altered on testing networks. */
   int TestingConsensusMaxDownloadTries;
 
+  /** How many times will a client try to fetch a consensus while
+   * bootstrapping using a list of fallback directories, before it gives up?
+   * Only altered on testing networks. */
+  int TestingClientBootstrapConsensusMaxDownloadTries;
+
+  /** How many times will a client try to fetch a consensus while
+   * bootstrapping using only a list of authorities, before it gives up?
+   * Only altered on testing networks. */
+  int TestingClientBootstrapConsensusAuthorityOnlyMaxDownloadTries;
+
+  /** How many simultaneous in-progress connections will we make when trying
+   * to fetch a consensus before we wait for one to complete, timeout, or
+   * error out?  Only altered on testing networks. */
+  int TestingClientBootstrapConsensusMaxInProgressTries;
+
   /** How many times will we try to download a router's descriptor before
    * giving up?  Only altered on testing networks. */
   int TestingDescriptorMaxDownloadTries;

src/or/routerlist.c

@@ -897,8 +897,10 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now)
 
     if (smartlist_len(fps) > 1) {
       resource = smartlist_join_strings(fps, "", 0, NULL);
+      /* XXX - do we want certs from authorities or mirrors? - teor */
       directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0,
-                                   resource, PDS_RETRY_IF_NO_SERVERS);
+                                   resource, PDS_RETRY_IF_NO_SERVERS,
+                                   DL_WANT_ANY_DIRSERVER);
       tor_free(resource);
     }
     /* else we didn't add any: they were all pending */
@@ -941,8 +943,10 @@ authority_certs_fetch_missing(networkstatus_t *status, time_t now)
 
     if (smartlist_len(fp_pairs) > 1) {
       resource = smartlist_join_strings(fp_pairs, "", 0, NULL);
+      /* XXX - do we want certs from authorities or mirrors? - teor */
       directory_get_from_dirserver(DIR_PURPOSE_FETCH_CERTIFICATE, 0,
-                                   resource, PDS_RETRY_IF_NO_SERVERS);
+                                   resource, PDS_RETRY_IF_NO_SERVERS,
+                                   DL_WANT_ANY_DIRSERVER);
       tor_free(resource);
     }
     /* else they were all pending */
@@ -1358,7 +1362,9 @@ router_get_trusteddirserver_by_digest(const char *digest)
 }
 
 /** Return the dir_server_t for the fallback dirserver whose identity
- * key hashes to <b>digest</b>, or NULL if no such authority is known.
+ * key hashes to <b>digest</b>, or NULL if no such fallback is in the list of
+ * fallback_dir_servers. (fallback_dir_servers is affected by the FallbackDir
+ * and UseDefaultFallbackDirs torrc options.)
  */
 dir_server_t *
 router_get_fallback_dirserver_by_digest(const char *digest)
@@ -1366,6 +1372,9 @@ router_get_fallback_dirserver_by_digest(const char *digest)
   if (!fallback_dir_servers)
     return NULL;
 
+  if (!digest)
+    return NULL;
+
   SMARTLIST_FOREACH(fallback_dir_servers, dir_server_t *, ds,
      {
        if (tor_memeq(ds->digest, digest, DIGEST_LEN))
@@ -1375,6 +1384,17 @@ router_get_fallback_dirserver_by_digest(const char *digest)
   return NULL;
 }
 
+/** Return 1 if any fallback dirserver's identity key hashes to <b>digest</b>,
+ * or 0 if no such fallback is in the list of fallback_dir_servers.
+ * (fallback_dir_servers is affected by the FallbackDir and
+ * UseDefaultFallbackDirs torrc options.)
+ */
+int
+router_digest_is_fallback_dir(const char *digest)
+{
+  return (router_get_fallback_dirserver_by_digest(digest) != NULL);
+}
+
 /** Return the dir_server_t for the directory authority whose
  * v3 identity key hashes to <b>digest</b>, or NULL if no such authority
  * is known.
@@ -4376,14 +4396,14 @@ MOCK_IMPL(STATIC void, initiate_descriptor_downloads,
   tor_free(cp);
 
   if (source) {
-    /* We know which authority we want. */
+    /* We know which authority or directory mirror we want. */
     directory_initiate_command_routerstatus(source, purpose,
                                             ROUTER_PURPOSE_GENERAL,
                                             DIRIND_ONEHOP,
                                             resource, NULL, 0, 0);
   } else {
     directory_get_from_dirserver(purpose, ROUTER_PURPOSE_GENERAL, resource,
-                                 pds_flags);
+                                 pds_flags, DL_WANT_ANY_DIRSERVER);
   }
   tor_free(resource);
 }
@@ -4665,9 +4685,14 @@ launch_dummy_descriptor_download_as_needed(time_t now,
       last_descriptor_download_attempted + DUMMY_DOWNLOAD_INTERVAL < now &&
       last_dummy_download + DUMMY_DOWNLOAD_INTERVAL < now) {
     last_dummy_download = now;
+    /* XX/teor - do we want an authority here, because they are less likely
+     * to give us the wrong address? (See #17782)
+     * I'm leaving the previous behaviour intact, because I don't like
+     * the idea of some relays contacting an authority every 20 minutes. */
     directory_get_from_dirserver(DIR_PURPOSE_FETCH_SERVERDESC,
                                  ROUTER_PURPOSE_GENERAL, "authority.z",
-                                 PDS_RETRY_IF_NO_SERVERS);
+                                 PDS_RETRY_IF_NO_SERVERS,
+                                 DL_WANT_ANY_DIRSERVER);
   }
 }
 

src/or/routerlist.h

@@ -50,6 +50,7 @@ const routerstatus_t *router_pick_directory_server(dirinfo_type_t type,
 dir_server_t *router_get_trusteddirserver_by_digest(const char *d);
 dir_server_t *router_get_fallback_dirserver_by_digest(
                                                    const char *digest);
+int router_digest_is_fallback_dir(const char *digest);
 dir_server_t *trusteddirserver_get_by_v3_auth_digest(const char *d);
 const routerstatus_t *router_pick_trusteddirserver(dirinfo_type_t type,
                                                    int flags);

src/test/Makefile.nmake

@@ -14,7 +14,8 @@ LIBS = ..\..\..\build-alpha\lib\libevent.lib \
 TEST_OBJECTS = test.obj test_addr.obj test_channel.obj test_channeltls.obj \
         test_containers.obj \
 	test_controller_events.obj test_crypto.obj test_data.obj test_dir.obj \
-	test_checkdir.obj test_microdesc.obj test_pt.obj test_util.obj test_config.obj \
+	test_checkdir.obj test_microdesc.obj test_pt.obj test_util.obj \
+        test_config.obj test_connection.obj \
 	test_cell_formats.obj test_relay.obj test_replay.obj \
 	test_scheduler.obj test_introduce.obj test_hs.obj tinytest.obj
 

src/test/include.am

@@ -69,6 +69,7 @@ src_test_test_SOURCES = \
 	src/test/test_circuitmux.c \
 	src/test/test_compat_libevent.c \
 	src/test/test_config.c \
+	src/test/test_connection.c \
 	src/test/test_containers.c \
 	src/test/test_controller.c \
 	src/test/test_controller_events.c \

src/test/test.c

@@ -1141,6 +1141,7 @@ extern struct testcase_t circuitlist_tests[];
 extern struct testcase_t circuitmux_tests[];
 extern struct testcase_t compat_libevent_tests[];
 extern struct testcase_t config_tests[];
+extern struct testcase_t connection_tests[];
 extern struct testcase_t container_tests[];
 extern struct testcase_t controller_tests[];
 extern struct testcase_t controller_event_tests[];
@@ -1196,6 +1197,7 @@ struct testgroup_t testgroups[] = {
   { "circuitmux/", circuitmux_tests },
   { "compat/libevent/", compat_libevent_tests },
   { "config/", config_tests },
+  { "connection/", connection_tests },
   { "container/", container_tests },
   { "control/", controller_tests },
   { "control/event/", controller_event_tests },

src/test/test_config.c

@@ -18,6 +18,7 @@
 #include "entrynodes.h"
 #include "transports.h"
 #include "routerlist.h"
+#include "networkstatus.h"
 
 static void
 test_config_addressmap(void *arg)
@@ -1478,7 +1479,7 @@ test_config_adding_dir_servers(void *arg)
   (void)arg;
 
   /* allocate options */
-  or_options_t *options = tor_malloc(sizeof(or_options_t));
+  or_options_t *options = tor_malloc_zero(sizeof(or_options_t));
 
   /* Allocate and populate configuration lines:
    *
@@ -1487,8 +1488,7 @@ test_config_adding_dir_servers(void *arg)
    * Zeroing the structure has the same effect as initialising to:
    * { NULL, NULL, NULL, CONFIG_LINE_NORMAL, 0};
    */
-  config_line_t *test_dir_authority = tor_malloc(sizeof(config_line_t));
-  memset(test_dir_authority, 0, sizeof(config_line_t));
+  config_line_t *test_dir_authority = tor_malloc_zero(sizeof(config_line_t));
   test_dir_authority->key = tor_strdup("DirAuthority");
   test_dir_authority->value = tor_strdup(
     "D0 orport=9000 "
@@ -1496,16 +1496,16 @@ test_config_adding_dir_servers(void *arg)
     "127.0.0.1:60090 0123 4567 8901 2345 6789 0123 4567 8901 2345 6789"
     );
 
-  config_line_t *test_alt_bridge_authority = tor_malloc(sizeof(config_line_t));
-  memset(test_alt_bridge_authority, 0, sizeof(config_line_t));
+  config_line_t *test_alt_bridge_authority = tor_malloc_zero(
+                                                      sizeof(config_line_t));
   test_alt_bridge_authority->key = tor_strdup("AlternateBridgeAuthority");
   test_alt_bridge_authority->value = tor_strdup(
     "B1 orport=9001 bridge "
     "127.0.0.1:60091 1123 4567 8901 2345 6789 0123 4567 8901 2345 6789"
     );
 
-  config_line_t *test_alt_dir_authority = tor_malloc(sizeof(config_line_t));
-  memset(test_alt_dir_authority, 0, sizeof(config_line_t));
+  config_line_t *test_alt_dir_authority = tor_malloc_zero(
+                                                      sizeof(config_line_t));
   test_alt_dir_authority->key = tor_strdup("AlternateDirAuthority");
   test_alt_dir_authority->value = tor_strdup(
     "A2 orport=9002 "
@@ -1514,8 +1514,8 @@ test_config_adding_dir_servers(void *arg)
     );
 
   /* Use the format specified in the manual page */
-  config_line_t *test_fallback_directory = tor_malloc(sizeof(config_line_t));
-  memset(test_fallback_directory, 0, sizeof(config_line_t));
+  config_line_t *test_fallback_directory = tor_malloc_zero(
+                                                      sizeof(config_line_t));
   test_fallback_directory->key = tor_strdup("FallbackDir");
   test_fallback_directory->value = tor_strdup(
     "127.0.0.1:60093 orport=9003 id=0323456789012345678901234567890123456789"
@@ -1623,6 +1623,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 1);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* fallback_dir_servers */
       const smartlist_t *fallback_servers = router_get_fallback_dir_servers();
@@ -1655,7 +1658,10 @@ test_config_adding_dir_servers(void *arg)
       n_default_fallback_dir = (smartlist_len(fallback_servers) -
                                 n_default_alt_bridge_authority -
                                 n_default_alt_dir_authority);
-      /* If we have a negative count, something has gone really wrong */
+      /* If we have a negative count, something has gone really wrong,
+       * or some authorities aren't being added as fallback directories.
+       * (networkstatus_consensus_can_use_extra_fallbacks depends on all
+       * authorities being fallback directories.) */
       tt_assert(n_default_fallback_dir >= 0);
     }
   }
@@ -1699,6 +1705,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -1837,6 +1846,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we just have the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 0);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -1975,6 +1987,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -2114,6 +2129,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 0);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -2263,6 +2281,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -2414,6 +2435,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 1);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -2574,6 +2598,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -2728,6 +2755,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we just have the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 0);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -2891,6 +2921,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must not have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 0);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -3051,6 +3084,9 @@ test_config_adding_dir_servers(void *arg)
     /* we must have added the default fallback dirs */
     tt_assert(n_add_default_fallback_dir_servers_known_default == 1);
 
+    /* we have more fallbacks than just the authorities */
+    tt_assert(networkstatus_consensus_can_use_extra_fallbacks(options) == 1);
+
     {
       /* trusted_dir_servers */
       const smartlist_t *dir_servers = router_get_trusted_dir_servers();
@@ -3244,6 +3280,48 @@ test_config_default_dir_servers(void *arg)
   or_options_free(opts);
 }
 
+static void
+test_config_use_multiple_directories(void *arg)
+{
+  (void)arg;
+
+  or_options_t *options = tor_malloc_zero(sizeof(or_options_t));
+
+  /* Clients can use multiple directory mirrors for bootstrap */
+  memset(options, 0, sizeof(or_options_t));
+  options->ClientOnly = 1;
+  tt_assert(networkstatus_consensus_can_use_multiple_directories(options)
+            == 1);
+
+  /* Bridge Clients can use multiple directory mirrors for bootstrap */
+  memset(options, 0, sizeof(or_options_t));
+  options->UseBridges = 1;
+  tt_assert(networkstatus_consensus_can_use_multiple_directories(options)
+            == 1);
+
+  /* Bridge Relays (Bridges) must act like clients, and use multiple
+   * directory mirrors for bootstrap */
+  memset(options, 0, sizeof(or_options_t));
+  options->BridgeRelay = 1;
+  tt_assert(networkstatus_consensus_can_use_multiple_directories(options)
+            == 1);
+
+  /* Clients set to FetchDirInfoEarly must fetch it from the authorities */
+  memset(options, 0, sizeof(or_options_t));
+  options->FetchDirInfoEarly = 1;
+  tt_assert(networkstatus_consensus_can_use_multiple_directories(options)
+            == 0);
+
+  /* OR servers must fetch the consensus from the authorities */
+  memset(options, 0, sizeof(or_options_t));
+  options->ORPort_set = 1;
+  tt_assert(networkstatus_consensus_can_use_multiple_directories(options)
+            == 0);
+
+ done:
+  tor_free(options);
+}
+
 #define CONFIG_TEST(name, flags)                          \
   { #name, test_config_ ## name, flags, NULL, NULL }
 
@@ -3258,6 +3336,7 @@ struct testcase_t config_tests[] = {
   CONFIG_TEST(check_or_create_data_subdir, TT_FORK),
   CONFIG_TEST(write_to_data_subdir, TT_FORK),
   CONFIG_TEST(fix_my_family, 0),
+  CONFIG_TEST(use_multiple_directories, 0),
   END_OF_TESTCASES
 };
 

src/test/test_connection.c

@@ -0,0 +1,757 @@
+/* Copyright (c) 2015, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "orconfig.h"
+
+#define CONNECTION_PRIVATE
+#define MAIN_PRIVATE
+
+#include "or.h"
+#include "test.h"
+
+#include "connection.h"
+#include "main.h"
+#include "networkstatus.h"
+#include "rendcache.h"
+#include "directory.h"
+
+static void test_conn_lookup_addr_helper(const char *address,
+                                         int family,
+                                         tor_addr_t *addr);
+
+static void * test_conn_get_basic_setup(const struct testcase_t *tc);
+static int test_conn_get_basic_teardown(const struct testcase_t *tc,
+                                        void *arg);
+
+static void * test_conn_get_rend_setup(const struct testcase_t *tc);
+static int test_conn_get_rend_teardown(const struct testcase_t *tc,
+                                        void *arg);
+
+static void * test_conn_get_rsrc_setup(const struct testcase_t *tc);
+static int test_conn_get_rsrc_teardown(const struct testcase_t *tc,
+                                        void *arg);
+
+/* Arbitrary choice - IPv4 Directory Connection to localhost */
+#define TEST_CONN_TYPE          (CONN_TYPE_DIR)
+/* We assume every machine has IPv4 localhost, is that ok? */
+#define TEST_CONN_ADDRESS       "127.0.0.1"
+#define TEST_CONN_PORT          (12345)
+#define TEST_CONN_ADDRESS_PORT  "127.0.0.1:12345"
+#define TEST_CONN_FAMILY        (AF_INET)
+#define TEST_CONN_STATE         (DIR_CONN_STATE_MIN_)
+#define TEST_CONN_ADDRESS_2     "127.0.0.2"
+
+#define TEST_CONN_BASIC_PURPOSE (DIR_PURPOSE_MIN_)
+
+#define TEST_CONN_REND_ADDR     "cfs3rltphxxvabci"
+#define TEST_CONN_REND_PURPOSE  (DIR_PURPOSE_FETCH_RENDDESC_V2)
+#define TEST_CONN_REND_PURPOSE_SUCCESSFUL (DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2)
+#define TEST_CONN_REND_TYPE_2   (CONN_TYPE_AP)
+#define TEST_CONN_REND_ADDR_2   "icbavxxhptlr3sfc"
+
+#define TEST_CONN_RSRC          (networkstatus_get_flavor_name(FLAV_MICRODESC))
+#define TEST_CONN_RSRC_PURPOSE  (DIR_PURPOSE_FETCH_CONSENSUS)
+#define TEST_CONN_RSRC_STATE_SUCCESSFUL (DIR_CONN_STATE_CLIENT_FINISHED)
+#define TEST_CONN_RSRC_2        (networkstatus_get_flavor_name(FLAV_NS))
+
+#define TEST_CONN_DL_STATE      (DIR_CONN_STATE_CLIENT_SENDING)
+
+#define TEST_CONN_FD_INIT 50
+static int mock_connection_connect_sockaddr_called = 0;
+static int fake_socket_number = TEST_CONN_FD_INIT;
+
+static int
+mock_connection_connect_sockaddr(connection_t *conn,
+                                 const struct sockaddr *sa,
+                                 socklen_t sa_len,
+                                 const struct sockaddr *bindaddr,
+                                 socklen_t bindaddr_len,
+                                 int *socket_error)
+{
+  (void)sa_len;
+  (void)bindaddr;
+  (void)bindaddr_len;
+
+  tor_assert(conn);
+  tor_assert(sa);
+  tor_assert(socket_error);
+
+  mock_connection_connect_sockaddr_called++;
+
+  conn->s = fake_socket_number++;
+  tt_assert(SOCKET_OK(conn->s));
+  /* We really should call tor_libevent_initialize() here. Because we don't,
+   * we are relying on other parts of the code not checking if the_event_base
+   * (and therefore event->ev_base) is NULL.  */
+  tt_assert(connection_add_connecting(conn) == 0);
+
+ done:
+  /* Fake "connected" status */
+  return 1;
+}
+
+static void
+test_conn_lookup_addr_helper(const char *address, int family, tor_addr_t *addr)
+{
+  int rv = 0;
+
+  tt_assert(addr);
+
+  rv = tor_addr_lookup(address, family, addr);
+  /* XXXX - should we retry on transient failure? */
+  tt_assert(rv == 0);
+  tt_assert(tor_addr_is_loopback(addr));
+  tt_assert(tor_addr_is_v4(addr));
+
+  return;
+
+ done:
+  tor_addr_make_null(addr, TEST_CONN_FAMILY);
+}
+
+static void *
+test_conn_get_basic_setup(const struct testcase_t *tc)
+{
+  connection_t *conn = NULL;
+  tor_addr_t addr;
+  int socket_err = 0;
+  int in_progress = 0;
+  (void)tc;
+
+  MOCK(connection_connect_sockaddr,
+       mock_connection_connect_sockaddr);
+
+  init_connection_lists();
+
+  conn = connection_new(TEST_CONN_TYPE, TEST_CONN_FAMILY);
+  tt_assert(conn);
+
+  test_conn_lookup_addr_helper(TEST_CONN_ADDRESS, TEST_CONN_FAMILY, &addr);
+  tt_assert(!tor_addr_is_null(&addr));
+
+  /* XXXX - connection_connect doesn't set these, should it? */
+  tor_addr_copy_tight(&conn->addr, &addr);
+  conn->port = TEST_CONN_PORT;
+  mock_connection_connect_sockaddr_called = 0;
+  in_progress = connection_connect(conn, TEST_CONN_ADDRESS_PORT, &addr,
+                                   TEST_CONN_PORT, &socket_err);
+  tt_assert(mock_connection_connect_sockaddr_called == 1);
+  tt_assert(!socket_err);
+  tt_assert(in_progress == 0 || in_progress == 1);
+
+  /* fake some of the attributes so the connection looks OK */
+  conn->state = TEST_CONN_STATE;
+  conn->purpose = TEST_CONN_BASIC_PURPOSE;
+  assert_connection_ok(conn, time(NULL));
+
+  UNMOCK(connection_connect_sockaddr);
+
+  return conn;
+
+  /* On failure */
+ done:
+  UNMOCK(connection_connect_sockaddr);
+  test_conn_get_basic_teardown(tc, conn);
+
+  /* Returning NULL causes the unit test to fail */
+  return NULL;
+}
+
+static int
+test_conn_get_basic_teardown(const struct testcase_t *tc, void *arg)
+{
+  (void)tc;
+  connection_t *conn = arg;
+
+  tt_assert(conn);
+  assert_connection_ok(conn, time(NULL));
+
+  /* teardown the connection as fast as possible */
+  if (conn->linked_conn) {
+    assert_connection_ok(conn->linked_conn, time(NULL));
+
+    /* We didn't call tor_libevent_initialize(), so event_base was NULL,
+     * so we can't rely on connection_unregister_events() use of event_del().
+     */
+    if (conn->linked_conn->read_event) {
+      tor_free(conn->linked_conn->read_event);
+      conn->linked_conn->read_event = NULL;
+    }
+    if (conn->linked_conn->write_event) {
+      tor_free(conn->linked_conn->write_event);
+      conn->linked_conn->write_event = NULL;
+    }
+
+    connection_free(conn->linked_conn);
+    conn->linked_conn = NULL;
+
+    conn->linked_conn->linked_conn = NULL;
+    if (!conn->linked_conn->marked_for_close) {
+      connection_close_immediate(conn->linked_conn);
+      connection_mark_for_close(conn->linked_conn);
+    }
+  }
+
+  /* We didn't set the events up properly, so we can't use event_del() in
+   * close_closeable_connections() > connection_free()
+   * > connection_unregister_events() */
+  if (conn->read_event) {
+    tor_free(conn->read_event);
+    conn->read_event = NULL;
+  }
+  if (conn->write_event) {
+    tor_free(conn->write_event);
+    conn->write_event = NULL;
+  }
+
+  if (!conn->marked_for_close) {
+    connection_close_immediate(conn);
+    connection_mark_for_close(conn);
+  }
+
+  close_closeable_connections();
+
+  /* The unit test will fail if we return 0 */
+  return 1;
+
+  /* When conn == NULL, we can't cleanup anything */
+ done:
+  return 0;
+}
+
+static void *
+test_conn_get_rend_setup(const struct testcase_t *tc)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t,
+                                    test_conn_get_basic_setup(tc));
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  rend_cache_init();
+
+  /* TODO: use directory_initiate_command_rend() to do this - maybe? */
+  conn->rend_data = tor_malloc_zero(sizeof(rend_data_t));
+  memcpy(conn->rend_data->onion_address,
+         TEST_CONN_REND_ADDR,
+         REND_SERVICE_ADDRESS_LEN+1);
+  conn->rend_data->hsdirs_fp = smartlist_new();
+  conn->base_.purpose = TEST_CONN_REND_PURPOSE;
+
+  assert_connection_ok(&conn->base_, time(NULL));
+  return conn;
+
+  /* On failure */
+ done:
+  test_conn_get_rend_teardown(tc, conn);
+  /* Returning NULL causes the unit test to fail */
+  return NULL;
+}
+
+static int
+test_conn_get_rend_teardown(const struct testcase_t *tc, void *arg)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t, arg);
+  int rv = 0;
+
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  /* avoid a last-ditch attempt to refetch the descriptor */
+  conn->base_.purpose = TEST_CONN_REND_PURPOSE_SUCCESSFUL;
+
+  /* connection_free_() cleans up rend_data */
+  rv = test_conn_get_basic_teardown(tc, arg);
+ done:
+  rend_cache_free_all();
+  return rv;
+}
+
+static void *
+test_conn_get_rsrc_setup(const struct testcase_t *tc)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t,
+                                    test_conn_get_basic_setup(tc));
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  /* TODO: use the canonical function to do this - maybe? */
+  conn->requested_resource = tor_strdup(TEST_CONN_RSRC);
+  conn->base_.purpose = TEST_CONN_RSRC_PURPOSE;
+
+  assert_connection_ok(&conn->base_, time(NULL));
+  return conn;
+
+  /* On failure */
+ done:
+  test_conn_get_rend_teardown(tc, conn);
+  /* Returning NULL causes the unit test to fail */
+  return NULL;
+}
+
+static int
+test_conn_get_rsrc_teardown(const struct testcase_t *tc, void *arg)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t, arg);
+  int rv = 0;
+
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  /* avoid a last-ditch attempt to refetch the consensus */
+  conn->base_.state = TEST_CONN_RSRC_STATE_SUCCESSFUL;
+
+  /* connection_free_() cleans up requested_resource */
+  rv = test_conn_get_basic_teardown(tc, arg);
+ done:
+  return rv;
+}
+
+static void *
+test_conn_download_status_setup(const struct testcase_t *tc)
+{
+  (void)tc;
+
+  /* Don't return NULL, that causes the test to fail */
+  return "ok";
+}
+
+static int
+test_conn_download_status_teardown(const struct testcase_t *tc, void *arg)
+{
+  (void)arg;
+  int rv = 0;
+
+  /* Ignore arg, and just loop through the connection array */
+  SMARTLIST_FOREACH_BEGIN(get_connection_array(), connection_t *, conn) {
+    if (conn) {
+      assert_connection_ok(conn, time(NULL));
+
+      /* connection_free_() cleans up requested_resource */
+      rv = test_conn_get_rsrc_teardown(tc, conn);
+      tt_assert(rv == 1);
+    }
+  } SMARTLIST_FOREACH_END(conn);
+
+ done:
+  return rv;
+}
+
+static dir_connection_t *
+test_conn_download_status_add_a_connection(void)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t,
+                                    test_conn_get_rsrc_setup(NULL));
+
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  return conn;
+
+ done:
+  test_conn_download_status_teardown(NULL, NULL);
+  return NULL;
+}
+
+static struct testcase_setup_t test_conn_get_basic_st = {
+  test_conn_get_basic_setup, test_conn_get_basic_teardown
+};
+
+static struct testcase_setup_t test_conn_get_rend_st = {
+  test_conn_get_rend_setup, test_conn_get_rend_teardown
+};
+
+static struct testcase_setup_t test_conn_get_rsrc_st = {
+  test_conn_get_rsrc_setup, test_conn_get_rsrc_teardown
+};
+
+static struct testcase_setup_t test_conn_download_status_st = {
+  test_conn_download_status_setup, test_conn_download_status_teardown
+};
+
+static void
+test_conn_get_basic(void *arg)
+{
+  connection_t *conn = (connection_t*)arg;
+  tor_addr_t addr, addr2;
+
+  tt_assert(conn);
+  assert_connection_ok(conn, time(NULL));
+
+  test_conn_lookup_addr_helper(TEST_CONN_ADDRESS, TEST_CONN_FAMILY, &addr);
+  tt_assert(!tor_addr_is_null(&addr));
+  test_conn_lookup_addr_helper(TEST_CONN_ADDRESS_2, TEST_CONN_FAMILY, &addr2);
+  tt_assert(!tor_addr_is_null(&addr2));
+
+  /* Check that we get this connection back when we search for it by
+   * its attributes, but get NULL when we supply a different value. */
+
+  tt_assert(connection_get_by_global_id(conn->global_identifier) == conn);
+  tt_assert(connection_get_by_global_id(!conn->global_identifier) == NULL);
+
+  tt_assert(connection_get_by_type(conn->type) == conn);
+  tt_assert(connection_get_by_type(TEST_CONN_TYPE) == conn);
+  tt_assert(connection_get_by_type(!conn->type) == NULL);
+  tt_assert(connection_get_by_type(!TEST_CONN_TYPE) == NULL);
+
+  tt_assert(connection_get_by_type_state(conn->type, conn->state)
+            == conn);
+  tt_assert(connection_get_by_type_state(TEST_CONN_TYPE, TEST_CONN_STATE)
+            == conn);
+  tt_assert(connection_get_by_type_state(!conn->type, !conn->state)
+            == NULL);
+  tt_assert(connection_get_by_type_state(!TEST_CONN_TYPE, !TEST_CONN_STATE)
+            == NULL);
+
+  /* Match on the connection fields themselves */
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &conn->addr,
+                                                     conn->port,
+                                                     conn->purpose)
+            == conn);
+  /* Match on the original inputs to the connection */
+  tt_assert(connection_get_by_type_addr_port_purpose(TEST_CONN_TYPE,
+                                                     &conn->addr,
+                                                     conn->port,
+                                                     conn->purpose)
+            == conn);
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &addr,
+                                                     conn->port,
+                                                     conn->purpose)
+            == conn);
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &conn->addr,
+                                                     TEST_CONN_PORT,
+                                                     conn->purpose)
+            == conn);
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &conn->addr,
+                                                     conn->port,
+                                                     TEST_CONN_BASIC_PURPOSE)
+            == conn);
+  tt_assert(connection_get_by_type_addr_port_purpose(TEST_CONN_TYPE,
+                                                     &addr,
+                                                     TEST_CONN_PORT,
+                                                     TEST_CONN_BASIC_PURPOSE)
+            == conn);
+  /* Then try each of the not-matching combinations */
+  tt_assert(connection_get_by_type_addr_port_purpose(!conn->type,
+                                                     &conn->addr,
+                                                     conn->port,
+                                                     conn->purpose)
+            == NULL);
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &addr2,
+                                                     conn->port,
+                                                     conn->purpose)
+            == NULL);
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &conn->addr,
+                                                     !conn->port,
+                                                     conn->purpose)
+            == NULL);
+  tt_assert(connection_get_by_type_addr_port_purpose(conn->type,
+                                                     &conn->addr,
+                                                     conn->port,
+                                                     !conn->purpose)
+            == NULL);
+  /* Then try everything not-matching */
+  tt_assert(connection_get_by_type_addr_port_purpose(!conn->type,
+                                                     &addr2,
+                                                     !conn->port,
+                                                     !conn->purpose)
+            == NULL);
+  tt_assert(connection_get_by_type_addr_port_purpose(!TEST_CONN_TYPE,
+                                                     &addr2,
+                                                     !TEST_CONN_PORT,
+                                                     !TEST_CONN_BASIC_PURPOSE)
+            == NULL);
+
+ done:
+  ;
+}
+
+static void
+test_conn_get_rend(void *arg)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t, arg);
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  tt_assert(connection_get_by_type_state_rendquery(
+                                            conn->base_.type,
+                                            conn->base_.state,
+                                            conn->rend_data->onion_address)
+            == TO_CONN(conn));
+  tt_assert(connection_get_by_type_state_rendquery(
+                                            TEST_CONN_TYPE,
+                                            TEST_CONN_STATE,
+                                            TEST_CONN_REND_ADDR)
+            == TO_CONN(conn));
+  tt_assert(connection_get_by_type_state_rendquery(TEST_CONN_REND_TYPE_2,
+                                                   !conn->base_.state,
+                                                   "")
+            == NULL);
+  tt_assert(connection_get_by_type_state_rendquery(TEST_CONN_REND_TYPE_2,
+                                                   !TEST_CONN_STATE,
+                                                   TEST_CONN_REND_ADDR_2)
+            == NULL);
+
+ done:
+  ;
+}
+
+#define sl_is_conn_assert(sl, conn) \
+  do { \
+    tt_assert(smartlist_len((sl)) == 1); \
+    tt_assert(smartlist_get((sl), 0) == (conn)); \
+  } while (0)
+
+#define sl_no_conn_assert(sl) \
+  do { \
+    tt_assert(smartlist_len((sl)) == 0); \
+  } while (0)
+
+static void
+test_conn_get_rsrc(void *arg)
+{
+  dir_connection_t *conn = DOWNCAST(dir_connection_t, arg);
+  tt_assert(conn);
+  assert_connection_ok(&conn->base_, time(NULL));
+
+  tt_assert(connection_dir_get_by_purpose_and_resource(
+                                                    conn->base_.purpose,
+                                                    conn->requested_resource)
+            == conn);
+  tt_assert(connection_dir_get_by_purpose_and_resource(
+                                                    TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC)
+            == conn);
+  tt_assert(connection_dir_get_by_purpose_and_resource(
+                                                    !conn->base_.purpose,
+                                                    "")
+            == NULL);
+  tt_assert(connection_dir_get_by_purpose_and_resource(
+                                                    !TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC_2)
+            == NULL);
+
+  tt_assert(connection_dir_get_by_purpose_resource_and_state(
+                                                    conn->base_.purpose,
+                                                    conn->requested_resource,
+                                                    conn->base_.state)
+            == conn);
+  tt_assert(connection_dir_get_by_purpose_resource_and_state(
+                                                    TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC,
+                                                    TEST_CONN_STATE)
+            == conn);
+  tt_assert(connection_dir_get_by_purpose_resource_and_state(
+                                                    !conn->base_.purpose,
+                                                    "",
+                                                    !conn->base_.state)
+            == NULL);
+  tt_assert(connection_dir_get_by_purpose_resource_and_state(
+                                                    !TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC_2,
+                                                    !TEST_CONN_STATE)
+            == NULL);
+
+  sl_is_conn_assert(connection_dir_list_by_purpose_and_resource(
+                                                    conn->base_.purpose,
+                                                    conn->requested_resource),
+                    conn);
+  sl_is_conn_assert(connection_dir_list_by_purpose_and_resource(
+                                                    TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC),
+                    conn);
+  sl_no_conn_assert(connection_dir_list_by_purpose_and_resource(
+                                                    !conn->base_.purpose,
+                                                    ""));
+  sl_no_conn_assert(connection_dir_list_by_purpose_and_resource(
+                                                    !TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC_2));
+
+  sl_is_conn_assert(connection_dir_list_by_purpose_resource_and_state(
+                                                    conn->base_.purpose,
+                                                    conn->requested_resource,
+                                                    conn->base_.state),
+                    conn);
+  sl_is_conn_assert(connection_dir_list_by_purpose_resource_and_state(
+                                                    TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC,
+                                                    TEST_CONN_STATE),
+                    conn);
+  sl_no_conn_assert(connection_dir_list_by_purpose_resource_and_state(
+                                                    !conn->base_.purpose,
+                                                    "",
+                                                    !conn->base_.state));
+  sl_no_conn_assert(connection_dir_list_by_purpose_resource_and_state(
+                                                    !TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC_2,
+                                                    !TEST_CONN_STATE));
+
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                    conn->base_.purpose,
+                                                    conn->requested_resource)
+            == 1);
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                    TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC)
+            == 1);
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                    !conn->base_.purpose,
+                                                    "")
+            == 0);
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                    !TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC_2)
+            == 0);
+
+  tt_assert(connection_dir_count_by_purpose_resource_and_state(
+                                                    conn->base_.purpose,
+                                                    conn->requested_resource,
+                                                    conn->base_.state)
+            == 1);
+  tt_assert(connection_dir_count_by_purpose_resource_and_state(
+                                                    TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC,
+                                                    TEST_CONN_STATE)
+            == 1);
+  tt_assert(connection_dir_count_by_purpose_resource_and_state(
+                                                    !conn->base_.purpose,
+                                                    "",
+                                                    !conn->base_.state)
+            == 0);
+  tt_assert(connection_dir_count_by_purpose_resource_and_state(
+                                                    !TEST_CONN_RSRC_PURPOSE,
+                                                    TEST_CONN_RSRC_2,
+                                                    !TEST_CONN_STATE)
+            == 0);
+
+ done:
+  ;
+}
+
+static void
+test_conn_download_status(void *arg)
+{
+  (void)arg;
+  dir_connection_t *conn = NULL;
+  dir_connection_t *conn2 = NULL;
+  dir_connection_t *conn3 = NULL;
+
+  /* no connections, no excess, not downloading */
+  tt_assert(networkstatus_consensus_has_excess_connections() == 0);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 0);
+
+  /* one connection, no excess, not downloading */
+  conn = test_conn_download_status_add_a_connection();
+  tt_assert(networkstatus_consensus_has_excess_connections() == 0);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 0);
+
+  /* one connection, no excess, but downloading */
+  conn->base_.state = TEST_CONN_DL_STATE;
+  tt_assert(networkstatus_consensus_has_excess_connections() == 0);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+  conn->base_.state = TEST_CONN_STATE;
+
+  /* two connections, excess, but not downloading */
+  conn2 = test_conn_download_status_add_a_connection();
+  tt_assert(networkstatus_consensus_has_excess_connections() == 1);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 0);
+
+  /* two connections, excess, downloading */
+  conn2->base_.state = TEST_CONN_DL_STATE;
+  tt_assert(networkstatus_consensus_has_excess_connections() == 1);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+  conn2->base_.state = TEST_CONN_STATE;
+
+  /* more connections, excess, but not downloading */
+  conn3 = test_conn_download_status_add_a_connection();
+  tt_assert(networkstatus_consensus_has_excess_connections() == 1);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 0);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 0);
+
+  /* more connections, excess, downloading */
+  conn3->base_.state = TEST_CONN_DL_STATE;
+  tt_assert(networkstatus_consensus_has_excess_connections() == 1);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+
+  /* more connections, more downloading */
+  conn2->base_.state = TEST_CONN_DL_STATE;
+  tt_assert(networkstatus_consensus_has_excess_connections() == 1);
+  tt_assert(networkstatus_consensus_is_downloading_usable_flavor() == 1);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+
+  /* now try closing the one that isn't downloading:
+   * these tests won't work unless tor thinks it is bootstrapping */
+  tt_assert(networkstatus_consensus_is_boostrapping(time(NULL)));
+
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                        TEST_CONN_RSRC_PURPOSE,
+                                                        TEST_CONN_RSRC) == 3);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+  tt_assert(connection_dir_close_consensus_conn_if_extra(conn) == -1);
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                        TEST_CONN_RSRC_PURPOSE,
+                                                        TEST_CONN_RSRC) == 2);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+
+  /* now try closing one that is already closed - nothing happens */
+  tt_assert(connection_dir_close_consensus_conn_if_extra(conn) == 0);
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                        TEST_CONN_RSRC_PURPOSE,
+                                                        TEST_CONN_RSRC) == 2);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+
+
+  /* now try closing one that is downloading - it stays open */
+  tt_assert(connection_dir_close_consensus_conn_if_extra(conn2) == 0);
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                        TEST_CONN_RSRC_PURPOSE,
+                                                        TEST_CONN_RSRC) == 2);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+
+  /* now try closing all excess connections */
+  connection_dir_close_extra_consensus_conns();
+  tt_assert(connection_dir_count_by_purpose_and_resource(
+                                                        TEST_CONN_RSRC_PURPOSE,
+                                                        TEST_CONN_RSRC) == 1);
+  tt_assert(connection_dir_avoid_extra_connection_for_purpose(
+                                                 TEST_CONN_RSRC_PURPOSE) == 1);
+
+ done:
+  /* the teardown function removes all the connections */;
+}
+
+#define CONNECTION_TESTCASE(name, fork, setup)                           \
+  { #name, test_conn_##name, fork, &setup, NULL }
+
+struct testcase_t connection_tests[] = {
+  CONNECTION_TESTCASE(get_basic, TT_FORK, test_conn_get_basic_st),
+  CONNECTION_TESTCASE(get_rend,  TT_FORK, test_conn_get_rend_st),
+  CONNECTION_TESTCASE(get_rsrc,  TT_FORK, test_conn_get_rsrc_st),
+  CONNECTION_TESTCASE(download_status,  TT_FORK, test_conn_download_status_st),
+//CONNECTION_TESTCASE(func_suffix, TT_FORK, setup_func_pair),
+  END_OF_TESTCASES
+};
+

src/test/test_dir.c

@@ -3494,6 +3494,435 @@ test_dir_packages(void *arg)
   tor_free(res);
 }
 
+static void
+test_dir_download_status_schedule(void *arg)
+{
+  (void)arg;
+  download_status_t dls_failure = { 0, 0, 0, DL_SCHED_GENERIC,
+                                             DL_WANT_AUTHORITY,
+                                             DL_SCHED_INCREMENT_FAILURE };
+  download_status_t dls_attempt = { 0, 0, 0, DL_SCHED_CONSENSUS,
+                                             DL_WANT_ANY_DIRSERVER,
+                                             DL_SCHED_INCREMENT_ATTEMPT};
+  download_status_t dls_bridge  = { 0, 0, 0, DL_SCHED_BRIDGE,
+                                             DL_WANT_AUTHORITY,
+                                             DL_SCHED_INCREMENT_FAILURE};
+  int increment = -1;
+  int expected_increment = -1;
+  time_t current_time = time(NULL);
+  int delay1 = -1;
+  int delay2 = -1;
+  smartlist_t *schedule = smartlist_new();
+
+  /* Make a dummy schedule */
+  smartlist_add(schedule, (void *)&delay1);
+  smartlist_add(schedule, (void *)&delay2);
+
+  /* check a range of values */
+  delay1 = 1000;
+  increment = download_status_schedule_get_delay(&dls_failure,
+                                                 schedule,
+                                                 TIME_MIN);
+  expected_increment = delay1;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_failure.next_attempt_at == TIME_MIN + expected_increment);
+
+#if TIME_T_IS_SIGNED
+  delay1 = INT_MAX;
+  increment =  download_status_schedule_get_delay(&dls_failure,
+                                                  schedule,
+                                                  -1);
+  expected_increment = delay1;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_failure.next_attempt_at == TIME_MAX);
+#endif
+
+  delay1 = 0;
+  increment = download_status_schedule_get_delay(&dls_attempt,
+                                                 schedule,
+                                                 0);
+  expected_increment = delay1;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_attempt.next_attempt_at == 0 + expected_increment);
+
+  delay1 = 1000;
+  increment = download_status_schedule_get_delay(&dls_attempt,
+                                                 schedule,
+                                                 1);
+  expected_increment = delay1;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_attempt.next_attempt_at == 1 + expected_increment);
+
+  delay1 = INT_MAX;
+  increment = download_status_schedule_get_delay(&dls_bridge,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = delay1;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_bridge.next_attempt_at == TIME_MAX);
+
+  delay1 = 1;
+  increment = download_status_schedule_get_delay(&dls_bridge,
+                                                 schedule,
+                                                 TIME_MAX);
+  expected_increment = delay1;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_bridge.next_attempt_at == TIME_MAX);
+
+  /* see what happens when we reach the end */
+  dls_attempt.n_download_attempts++;
+  dls_bridge.n_download_failures++;
+
+  delay2 = 100;
+  increment = download_status_schedule_get_delay(&dls_attempt,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = delay2;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_attempt.next_attempt_at == current_time + delay2);
+
+  delay2 = 1;
+  increment = download_status_schedule_get_delay(&dls_bridge,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = delay2;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_bridge.next_attempt_at == current_time + delay2);
+
+  /* see what happens when we try to go off the end */
+  dls_attempt.n_download_attempts++;
+  dls_bridge.n_download_failures++;
+
+  delay2 = 5;
+  increment = download_status_schedule_get_delay(&dls_attempt,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = delay2;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_attempt.next_attempt_at == current_time + delay2);
+
+  delay2 = 17;
+  increment = download_status_schedule_get_delay(&dls_bridge,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = delay2;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_bridge.next_attempt_at == current_time + delay2);
+
+  /* see what happens when we reach IMPOSSIBLE_TO_DOWNLOAD */
+  dls_attempt.n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD;
+  dls_bridge.n_download_failures = IMPOSSIBLE_TO_DOWNLOAD;
+
+  delay2 = 35;
+  increment = download_status_schedule_get_delay(&dls_attempt,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = INT_MAX;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_attempt.next_attempt_at == TIME_MAX);
+
+  delay2 = 99;
+  increment = download_status_schedule_get_delay(&dls_bridge,
+                                                 schedule,
+                                                 current_time);
+  expected_increment = INT_MAX;
+  tt_assert(increment == expected_increment);
+  tt_assert(dls_bridge.next_attempt_at == TIME_MAX);
+
+ done:
+  /* the pointers in schedule are allocated on the stack */
+  smartlist_free(schedule);
+}
+
+static void
+test_dir_download_status_increment(void *arg)
+{
+  (void)arg;
+  download_status_t dls_failure = { 0, 0, 0, DL_SCHED_GENERIC,
+    DL_WANT_AUTHORITY,
+    DL_SCHED_INCREMENT_FAILURE };
+  download_status_t dls_attempt = { 0, 0, 0, DL_SCHED_BRIDGE,
+    DL_WANT_ANY_DIRSERVER,
+    DL_SCHED_INCREMENT_ATTEMPT};
+  int delay0 = -1;
+  int delay1 = -1;
+  int delay2 = -1;
+  smartlist_t *schedule = smartlist_new();
+  or_options_t test_options;
+  time_t next_at = TIME_MAX;
+  time_t current_time = time(NULL);
+
+  /* Provide some values for the schedule */
+  delay0 = 10;
+  delay1 = 99;
+  delay2 = 20;
+
+  /* Make the schedule */
+  smartlist_add(schedule, (void *)&delay0);
+  smartlist_add(schedule, (void *)&delay1);
+  smartlist_add(schedule, (void *)&delay2);
+
+  /* Put it in the options */
+  mock_options = &test_options;
+  reset_options(mock_options, &mock_get_options_calls);
+  mock_options->TestingClientDownloadSchedule = schedule;
+  mock_options->TestingBridgeDownloadSchedule = schedule;
+
+  MOCK(get_options, mock_get_options);
+
+  /* Check that a failure reset works */
+  mock_get_options_calls = 0;
+  download_status_reset(&dls_failure);
+  /* we really want to test that it's equal to time(NULL) + delay0, but that's
+   * an unrealiable test, because time(NULL) might change. */
+  tt_assert(download_status_get_next_attempt_at(&dls_failure)
+            >= current_time + delay0);
+  tt_assert(download_status_get_next_attempt_at(&dls_failure)
+            != TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 0);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* avoid timing inconsistencies */
+  dls_failure.next_attempt_at = current_time + delay0;
+
+  /* check that a reset schedule becomes ready at the right time */
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay0 - 1,
+                                     1) == 0);
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay0,
+                                     1) == 1);
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay0 + 1,
+                                     1) == 1);
+
+  /* Check that a failure increment works */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_failure(&dls_failure, 404, "test", 0,
+                                              current_time);
+  tt_assert(next_at == current_time + delay1);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 1);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 1);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* check that an incremented schedule becomes ready at the right time */
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay1 - 1,
+                                     1) == 0);
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay1,
+                                     1) == 1);
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay1 + 1,
+                                     1) == 1);
+
+  /* check that a schedule isn't ready if it's had too many failures */
+  tt_assert(download_status_is_ready(&dls_failure,
+                                     current_time + delay1 + 10,
+                                     0) == 0);
+
+  /* Check that failure increments don't happen on 503 for clients, but that
+   * attempt increments do. */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_failure(&dls_failure, 503, "test", 0,
+                                              current_time);
+  tt_assert(next_at == current_time + delay1);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 1);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 2);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check that failure increments do happen on 503 for servers */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_failure(&dls_failure, 503, "test", 1,
+                                              current_time);
+  tt_assert(next_at == current_time + delay2);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 2);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 3);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check what happens when we run off the end of the schedule */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_failure(&dls_failure, 404, "test", 0,
+                                              current_time);
+  tt_assert(next_at == current_time + delay2);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 3);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 4);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check what happens when we hit the failure limit */
+  mock_get_options_calls = 0;
+  download_status_mark_impossible(&dls_failure);
+  next_at = download_status_increment_failure(&dls_failure, 404, "test", 0,
+                                              current_time);
+  tt_assert(next_at == TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_failure)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(download_status_get_n_attempts(&dls_failure)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check that a failure reset doesn't reset at the limit */
+  mock_get_options_calls = 0;
+  download_status_reset(&dls_failure);
+  tt_assert(download_status_get_next_attempt_at(&dls_failure)
+            == TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_failure)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(download_status_get_n_attempts(&dls_failure)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(mock_get_options_calls == 0);
+
+  /* Check that a failure reset resets just before the limit */
+  mock_get_options_calls = 0;
+  dls_failure.n_download_failures = IMPOSSIBLE_TO_DOWNLOAD - 1;
+  dls_failure.n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD - 1;
+  download_status_reset(&dls_failure);
+  /* we really want to test that it's equal to time(NULL) + delay0, but that's
+   * an unrealiable test, because time(NULL) might change. */
+  tt_assert(download_status_get_next_attempt_at(&dls_failure)
+            >= current_time + delay0);
+  tt_assert(download_status_get_next_attempt_at(&dls_failure)
+            != TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 0);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check that failure increments do happen on attempt-based schedules,
+   * but that the retry is set at the end of time */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_failure(&dls_attempt, 404, "test", 0,
+                                              current_time);
+  tt_assert(next_at == TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_attempt) == 1);
+  tt_assert(download_status_get_n_attempts(&dls_attempt) == 0);
+  tt_assert(mock_get_options_calls == 0);
+
+  /* Check that an attempt reset works */
+  mock_get_options_calls = 0;
+  download_status_reset(&dls_attempt);
+  /* we really want to test that it's equal to time(NULL) + delay0, but that's
+   * an unrealiable test, because time(NULL) might change. */
+  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
+            >= current_time + delay0);
+  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
+            != TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_attempt) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_attempt) == 0);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* avoid timing inconsistencies */
+  dls_attempt.next_attempt_at = current_time + delay0;
+
+  /* check that a reset schedule becomes ready at the right time */
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay0 - 1,
+                                     1) == 0);
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay0,
+                                     1) == 1);
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay0 + 1,
+                                     1) == 1);
+
+  /* Check that an attempt increment works */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_attempt(&dls_attempt, "test",
+                                              current_time);
+  tt_assert(next_at == current_time + delay1);
+  tt_assert(download_status_get_n_failures(&dls_attempt) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_attempt) == 1);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* check that an incremented schedule becomes ready at the right time */
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay1 - 1,
+                                     1) == 0);
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay1,
+                                     1) == 1);
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay1 + 1,
+                                     1) == 1);
+
+  /* check that a schedule isn't ready if it's had too many attempts */
+  tt_assert(download_status_is_ready(&dls_attempt,
+                                     current_time + delay1 + 10,
+                                     0) == 0);
+
+  /* Check what happens when we reach then run off the end of the schedule */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_attempt(&dls_attempt, "test",
+                                              current_time);
+  tt_assert(next_at == current_time + delay2);
+  tt_assert(download_status_get_n_failures(&dls_attempt) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_attempt) == 2);
+  tt_assert(mock_get_options_calls >= 1);
+
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_attempt(&dls_attempt, "test",
+                                              current_time);
+  tt_assert(next_at == current_time + delay2);
+  tt_assert(download_status_get_n_failures(&dls_attempt) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_attempt) == 3);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check what happens when we hit the attempt limit */
+  mock_get_options_calls = 0;
+  download_status_mark_impossible(&dls_attempt);
+  next_at = download_status_increment_attempt(&dls_attempt, "test",
+                                              current_time);
+  tt_assert(next_at == TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_attempt)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(download_status_get_n_attempts(&dls_attempt)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check that an attempt reset doesn't reset at the limit */
+  mock_get_options_calls = 0;
+  download_status_reset(&dls_attempt);
+  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
+            == TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_attempt)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(download_status_get_n_attempts(&dls_attempt)
+            == IMPOSSIBLE_TO_DOWNLOAD);
+  tt_assert(mock_get_options_calls == 0);
+
+  /* Check that an attempt reset resets just before the limit */
+  mock_get_options_calls = 0;
+  dls_attempt.n_download_failures = IMPOSSIBLE_TO_DOWNLOAD - 1;
+  dls_attempt.n_download_attempts = IMPOSSIBLE_TO_DOWNLOAD - 1;
+  download_status_reset(&dls_attempt);
+  /* we really want to test that it's equal to time(NULL) + delay0, but that's
+   * an unrealiable test, because time(NULL) might change. */
+  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
+            >= current_time + delay0);
+  tt_assert(download_status_get_next_attempt_at(&dls_attempt)
+            != TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_attempt) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_attempt) == 0);
+  tt_assert(mock_get_options_calls >= 1);
+
+  /* Check that attempt increments don't happen on failure-based schedules,
+   * and that the attempt is set at the end of time */
+  mock_get_options_calls = 0;
+  next_at = download_status_increment_attempt(&dls_failure, "test",
+                                              current_time);
+  tt_assert(next_at == TIME_MAX);
+  tt_assert(download_status_get_n_failures(&dls_failure) == 0);
+  tt_assert(download_status_get_n_attempts(&dls_failure) == 0);
+  tt_assert(mock_get_options_calls == 0);
+
+ done:
+  /* the pointers in schedule are allocated on the stack */
+  smartlist_free(schedule);
+  UNMOCK(get_options);
+  mock_options = NULL;
+  mock_get_options_calls = 0;
+}
+
 #define DIR_LEGACY(name)                                                   \
   { #name, test_dir_ ## name , TT_FORK, NULL, NULL }
 
@@ -3525,6 +3954,8 @@ struct testcase_t dir_tests[] = {
   DIR(purpose_needs_anonymity, 0),
   DIR(fetch_type, 0),
   DIR(packages, 0),
+  DIR(download_status_schedule, 0),
+  DIR(download_status_increment, 0),
   END_OF_TESTCASES
 };
 

src/test/test_routerlist.c

@@ -12,11 +12,13 @@ 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)
+                        const char *resource, int pds_flags,
+                        download_want_authority_t want_authority)
 {
   (void)dir_purpose;
   (void)router_purpose;
   (void)pds_flags;
+  (void)want_authority;
   tt_assert(resource);
   strlcpy(output, resource, sizeof(output));
  done: