Do not automatically ignore Fast/Stable for exits when ExitNodes is set

This once maybe made sense when ExitNodes meant "Here are 3 exits;
use them all", but now it more typically means "Here are 3
countries; exit from there."  Using non-Fast/Stable exits created a
potential partitioning opportunity and an annoying stability
problem.

(Don't worry about the case where all of our ExitNodes are non-Fast
or non-Stable: we handle that later in the function by retrying with
need_capacity and need_uptime set to 0.)

diff --git a/changes/exitnodes_reliable b/changes/exitnodes_reliable
new file mode 100644 (file)
index 0000000..62ef03a
--- /dev/null
+++ b/changes/exitnodes_reliable
@@ -0,0 +1,7 @@
+  o Minor features:
+    - If ExitNodes is set, still pay attention to the Fast/Stable
+      status of exits when picking exit nodes.  (We used to ignore
+      these flags when ExitNodes was set, on the grounds that people
+      who set exitnodes wanted all of those nodes to get used, but
+      with the ability to pick exits by country and IP range, this
+      doesn't necessarily make sense any more.)

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index b6627a0f8b4155858f69210ba8f07bbb663e8503..714d6365c6fb6edf670683719d3efb0182c33723 100644 (file)
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -2697,20 +2697,12 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime,
       continue; /* not one of our chosen exit nodes */
     }
 
-    if (router_is_unreliable(router, need_uptime, need_capacity, 0) &&
-        !options->ExitNodes) {
-      /* FFFF Someday, differentiate between a routerset that names
-       * routers, and a routerset that names countries, and only do this
-       * check if they've asked for specific exit relays. Or if the country
-       * they ask for is rare. Or something. */
-      /* XXX022-1090 We need to pick a tradeoff here: if we throw it out because
-       * it's unreliable, users might end up with no exit options even
-       * though some options are up. If we don't throw it out, users who
-       * set ExitNodes will have partitioning problems because they'll be
-       * the only folks willing to use this node. */
+    if (router_is_unreliable(router, need_uptime, need_capacity, 0)) {
       n_supported[i] = -1;
-      continue; /* skip routers that are not suitable, unless we have
-                 * ExitNodes set, in which case we asked for it */
+      continue; /* skip routers that are not suitable.  Don't worry if
+                 * this makes us reject all the possible routers: if so,
+                 * we'll retry later in this function with need_update and
+                 * need_capacity set to 0. */
     }
     if (!(router->is_valid || options->_AllowInvalid & ALLOW_INVALID_EXIT)) {
       /* if it's invalid and we don't want it */