Refactor the interfaces of transport/proxy lookup fns

Returning a tristate is needless here; we can just use the yielded
transport/proxy_type field to tell whether there's a proxy, and have
the return indicate success/failure.

Also, store the proxy_type in the or_connection_t rather than letting
it get out of sync if a configuration reload happens between launching
the or_connection and deciding what to say with it.

5 files changed, 42 insertions, 57 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 98d7ac32a0..aa0e996d17 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -4810,18 +4810,17 @@ find_bridge_by_digest(const char *digest)
  * bridge of ours that uses pluggable transports, place its transport
  * in <b>transport</b>.
  *
- * Return:
- * 0: if transport was found successfully.
- * 1: if <b>addr</b>:<b>port</b> did not match a bridge,
- *    or if matched bridge was not using transports.
- * -1: if we should be using a transport, but the transport could not be found.
+ * Return 0 on success (found a transport, or found a bridge with no
+ * transport, or found no bridge); return -1 if we should be using a
+ * transport, but the transport could not be found.
  */
 int
 find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port,
                                   const transport_t **transport)
 {
+  *transport = NULL;
   if (!bridge_list)
-    return 1;
+    return 0;
 
   SMARTLIST_FOREACH_BEGIN(bridge_list, const bridge_info_t *, bridge) {
     if (tor_addr_eq(&bridge->addr, addr) &&
@@ -4839,7 +4838,8 @@ find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port,
     }
   } SMARTLIST_FOREACH_END(bridge);
 
-  return 1;
+  *transport = NULL;
+  return 0;
 }
 
 /** We need to ask <b>bridge</b> for its server descriptor. */
diff --git a/src/or/connection.c b/src/or/connection.c
index 05937ac864..00f25e6b52 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -4105,47 +4105,45 @@ assert_connection_ok(connection_t *conn, time_t now)
  *  proxy server we are using.
  *  <b>conn</b> contains the connection we are using the proxy for.
  *
- *  Returns:
- *  0: if we were successfull
- *  1: if we are not using a proxy
- *  -1: if we are using a proxy but its addrport could not be
- *  found. */
+ *  Return 0 on success, -1 on failure.
+ */
 int
-get_proxy_addrport(tor_addr_t *addr, uint16_t *port,
-                   connection_t *conn)
+get_proxy_addrport(tor_addr_t *addr, uint16_t *port, int *proxy_type,
+                   const connection_t *conn)
 {
   or_options_t *options = get_options();
 
   if (options->HTTPSProxy) {
     tor_addr_copy(addr, &options->HTTPSProxyAddr);
     *port = options->HTTPSProxyPort;
-    goto done;
+    *proxy_type = PROXY_CONNECT;
+    return 0;
   } else if (options->Socks4Proxy) {
     tor_addr_copy(addr, &options->Socks4ProxyAddr);
     *port = options->Socks4ProxyPort;
-    goto done;
+    *proxy_type = PROXY_SOCKS4;
+    return 0;
   } else if (options->Socks5Proxy) {
     tor_addr_copy(addr, &options->Socks5ProxyAddr);
     *port = options->Socks5ProxyPort;
-    goto done;
+    *proxy_type = PROXY_SOCKS5;
+    return 0;
   } else if (options->ClientTransportPlugin ||
              options->Bridges) {
-    const transport_t *transport=NULL;
+    const transport_t *transport = NULL;
     int r;
     r = find_transport_by_bridge_addrport(&conn->addr, conn->port, &transport);
-    if (r == 0) { /* transport found */
-      tor_assert(transport);
+    if (r<0)
+      return -1;
+    if (transport) { /* transport found */
       tor_addr_copy(addr, &transport->addr);
       *port = transport->port;
-      goto done;
-    } else {
-      return r;
+      *proxy_type = transport->socks_version;
+      return 0;
     }
   }
 
-  return 1;
-
- done: /* proxy found */
+  *proxy_type = PROXY_NONE;
   return 0;
 }
 
@@ -4174,8 +4172,9 @@ log_failed_proxy_connection(connection_t *conn)
 {
   tor_addr_t proxy_addr;
   uint16_t proxy_port;
+  int proxy_type;
 
-  if (get_proxy_addrport(&proxy_addr, &proxy_port, conn) != 0)
+  if (get_proxy_addrport(&proxy_addr, &proxy_port, &proxy_type, conn) != 0)
     return; /* if we have no proxy set up, leave this function. */
 
   log_warn(LD_NET,
diff --git a/src/or/connection.h b/src/or/connection.h
index bf439b9bbc..be3de88aaa 100644
--- a/src/or/connection.h
+++ b/src/or/connection.h
@@ -58,8 +58,8 @@ int connection_connect(connection_t *conn, const char *address,
 int connection_proxy_connect(connection_t *conn, int type);
 int connection_read_proxy_handshake(connection_t *conn);
 void log_failed_proxy_connection(connection_t *conn);
-int get_proxy_addrport(tor_addr_t *addr,
-                       uint16_t *port, connection_t *conn);
+int get_proxy_addrport(tor_addr_t *addr, uint16_t *port, int *proxy_type,
+                       const connection_t *conn);
 
 int retry_all_listeners(smartlist_t *replaced_conns,
                         smartlist_t *new_conns);
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 2de25f6807..a02ec1e268 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -317,7 +317,7 @@ connection_or_finished_flushing(or_connection_t *conn)
 int
 connection_or_finished_connecting(or_connection_t *or_conn)
 {
-  int proxy_type;
+  const int proxy_type = or_conn->proxy_type;
   connection_t *conn;
   tor_assert(or_conn);
   conn = TO_CONN(or_conn);
@@ -327,27 +327,6 @@ connection_or_finished_connecting(or_connection_t *or_conn)
             conn->address,conn->port);
   control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0);
 
-  proxy_type = PROXY_NONE;
-
-  if (get_options()->HTTPSProxy)
-    proxy_type = PROXY_CONNECT;
-  else if (get_options()->Socks4Proxy)
-    proxy_type = PROXY_SOCKS4;
-  else if (get_options()->Socks5Proxy)
-    proxy_type = PROXY_SOCKS5;
-  else if (get_options()->ClientTransportPlugin) {
-    const transport_t *transport=NULL;
-    int r;
-    r = find_transport_by_bridge_addrport(&conn->addr,conn->port,&transport);
-    if (r == 0) {
-      tor_assert(transport);
-      log_debug(LD_GENERAL, "Found transport. Setting proxy type!\n");
-      proxy_type = transport->socks_version;
-    } else if (r == -1) {
-      return -1;
-    }
-  }
-
   if (proxy_type != PROXY_NONE) {
     /* start proxy handshake */
     if (connection_proxy_connect(conn, proxy_type) < 0) {
@@ -846,6 +825,7 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
   int r;
   tor_addr_t proxy_addr;
   uint16_t proxy_port;
+  int proxy_type;
 
   tor_assert(_addr);
   tor_assert(id_digest);
@@ -864,12 +844,15 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
   control_event_or_conn_status(conn, OR_CONN_EVENT_LAUNCHED, 0);
 
   /* If we are using a proxy server, find it and use it. */
-  r = get_proxy_addrport(&proxy_addr, &proxy_port, TO_CONN(conn));
-  if (r == 0) { /* proxy found. */
-    tor_addr_copy(&addr, &proxy_addr);
-    port = proxy_port;
-    conn->_base.proxy_state = PROXY_INFANT;
-  } else if (r == -1) { /* proxy could not be found. */
+  r = get_proxy_addrport(&proxy_addr, &proxy_port, &proxy_type, TO_CONN(conn));
+  if (r == 0) {
+    conn->proxy_type = proxy_type;
+    if (proxy_type != PROXY_NONE) {
+      tor_addr_copy(&addr, &proxy_addr);
+      port = proxy_port;
+      conn->_base.proxy_state = PROXY_INFANT;
+    }
+  } else {
     log_warn(LD_GENERAL, "Tried to connect through proxy, but proxy address "
              "could not be found.");
     connection_free(TO_CONN(conn));
diff --git a/src/or/or.h b/src/or/or.h
index 72d311b430..d1817d47f8 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -230,6 +230,8 @@ typedef enum {
 #define PROXY_CONNECT 1
 #define PROXY_SOCKS4 2
 #define PROXY_SOCKS5 3
+/* !!!! If there is ever a PROXY_* type over 2, we must grow the proxy_type
+ * field in or_connection_t */
 /* pluggable transports proxy type */
 #define PROXY_PLUGGABLE 4
 
@@ -1097,6 +1099,7 @@ typedef struct or_connection_t {
    * router itself has a problem.
    */
   unsigned int is_bad_for_new_circs:1;
+  unsigned int proxy_type:2; /**< One of PROXY_NONE...PROXY_SOCKS5 */
   uint8_t link_proto; /**< What protocol version are we using? 0 for
                        * "none negotiated yet." */
   circid_t next_circ_id; /**< Which circ_id do we try to use next on