Merge branch 'bug27224_take2_squashed'

3 files changed, 195 insertions, 9 deletions

diff --git a/changes/bug27224 b/changes/bug27224
new file mode 100644
index 0000000000..d43890b818
--- /dev/null
+++ b/changes/bug27224
@@ -0,0 +1,5 @@
+  o Minor bugfixes (performance)::
+    - Rework node_is_a_configured_bridge() to no longer
+      call node_get_all_orports(), which was performing too
+      many memory allocations. Fixes bug 27224; bugfix on
+      0.2.3.9.
diff --git a/src/feature/client/bridges.c b/src/feature/client/bridges.c
index 4bffe16036..7f4422f787 100644
--- a/src/feature/client/bridges.c
+++ b/src/feature/client/bridges.c
@@ -31,6 +31,7 @@
 #include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerinfo_st.h"
 #include "feature/nodelist/routerstatus_st.h"
+#include "feature/nodelist/microdesc_st.h"
 
 /** Information about a configured bridge. Currently this just matches the
  * ones in the torrc file, but one day we may be able to learn about new
@@ -283,17 +284,105 @@ routerinfo_is_a_configured_bridge(const routerinfo_t *ri)
   return get_configured_bridge_by_routerinfo(ri) ? 1 : 0;
 }
 
-/** Return 1 if <b>node</b> is one of our configured bridges, else 0. */
+/**
+ * Return 1 iff <b>bridge_list</b> contains entry matching
+ * given; IPv4 address in host byte order (<b>ipv4_addr</b>
+ * and <b>port</b> (and no identity digest) OR it contains an
+ * entry whose identity matches <b>digest</b>. Otherwise,
+ * return 0.
+ */
+static int
+bridge_exists_with_ipv4h_addr_and_port(const uint32_t ipv4_addr,
+                                       const uint16_t port,
+                                       const char *digest)
+{
+  tor_addr_t node_ipv4;
+
+  if (tor_addr_port_is_valid_ipv4h(ipv4_addr, port, 0)) {
+    tor_addr_from_ipv4h(&node_ipv4, ipv4_addr);
+
+   bridge_info_t *bridge =
+    get_configured_bridge_by_addr_port_digest(&node_ipv4,
+                                              port,
+                                              digest);
+
+   return (bridge != NULL);
+  }
+
+  return 0;
+}
+
+/**
+ * Return 1 iff <b>bridge_list</b> contains entry matching given
+ * <b>ipv6_addr</b> and <b>port</b> (and no identity digest) OR
+ * it contains an  entry whose identity matches <b>digest</b>.
+ * Otherwise, return 0.
+ */
+static int
+bridge_exists_with_ipv6_addr_and_port(const tor_addr_t *ipv6_addr,
+                                      const uint16_t port,
+                                      const char *digest)
+{
+  if (!tor_addr_port_is_valid(ipv6_addr, port, 0))
+    return 0;
+
+  bridge_info_t *bridge =
+   get_configured_bridge_by_addr_port_digest(ipv6_addr,
+                                             port,
+                                             digest);
+
+  return (bridge != NULL);
+}
+
+/** Return 1 if <b>node</b> is one of our configured bridges, else 0.
+ * More specifically, return 1 iff: a bridge_info_t object exists in
+ * <b>bridge_list</b> such that: 1) It's identity is equal to node
+ * identity OR 2) It's identity digest is zero, but it matches
+ * address and port of any ORPort in the node.
+ */
 int
 node_is_a_configured_bridge(const node_t *node)
 {
-  int retval = 0;
-  smartlist_t *orports = node_get_all_orports(node);
-  retval = get_configured_bridge_by_orports_digest(node->identity,
-                                                   orports) != NULL;
-  SMARTLIST_FOREACH(orports, tor_addr_port_t *, p, tor_free(p));
-  smartlist_free(orports);
-  return retval;
+  /* First, let's try searching for a bridge with matching identity. */
+  if (BUG(tor_digest_is_zero(node->identity)))
+    return 0;
+
+  if (find_bridge_by_digest(node->identity) != NULL)
+    return 1;
+
+  /* At this point, we have established that no bridge exists with
+   * matching identity digest. However, we still pass it into
+   * bridge_exists_* functions because we want further code to
+   * check for absence of identity digest in a bridge.
+   */
+  if (node->ri) {
+    if (bridge_exists_with_ipv4h_addr_and_port(node->ri->addr,
+                                               node->ri->or_port,
+                                               node->identity))
+      return 1;
+
+    if (bridge_exists_with_ipv6_addr_and_port(&node->ri->ipv6_addr,
+                                              node->ri->ipv6_orport,
+                                              node->identity))
+      return 1;
+  } else if (node->rs) {
+    if (bridge_exists_with_ipv4h_addr_and_port(node->rs->addr,
+                                               node->rs->or_port,
+                                               node->identity))
+      return 1;
+
+    if (bridge_exists_with_ipv6_addr_and_port(&node->rs->ipv6_addr,
+                                              node->rs->ipv6_orport,
+                                              node->identity))
+      return 1;
+  }  else if (node->md) {
+    if (bridge_exists_with_ipv6_addr_and_port(&node->md->ipv6_addr,
+                                              node->md->ipv6_orport,
+                                              node->identity))
+      return 1;
+  }
+
+  return 0;
 }
 
 /** We made a connection to a router at <b>addr</b>:<b>port</b>
@@ -934,4 +1023,3 @@ bridges_free_all(void)
   smartlist_free(bridge_list);
   bridge_list = NULL;
 }
-
diff --git a/src/test/test_bridges.c b/src/test/test_bridges.c
index 07d6b88ed9..1cad5445f4 100644
--- a/src/test/test_bridges.c
+++ b/src/test/test_bridges.c
@@ -16,6 +16,10 @@
 #include "feature/client/bridges.h"
 #include "app/config/config.h"
 #include "feature/client/transports.h"
+#include "feature/nodelist/node_st.h"
+#include "feature/nodelist/routerinfo_st.h"
+#include "feature/nodelist/routerstatus_st.h"
+#include "feature/nodelist/microdesc_st.h"
 
 /* Test suite stuff */
 #include "test/test.h"
@@ -75,6 +79,7 @@ helper_add_bridges_to_bridgelist(void *arg)
   char *bridge5 = tor_strdup("apple 4.4.4.4:4444 "
                              "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA "
                              "foo=abcdefghijklmnopqrstuvwxyz");
+  char *bridge6 = tor_strdup("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:6666");
 
   mark_bridge_list();
 
@@ -93,6 +98,7 @@ helper_add_bridges_to_bridgelist(void *arg)
   ADD_BRIDGE(bridge3);
   ADD_BRIDGE(bridge4);
   ADD_BRIDGE(bridge5);
+  ADD_BRIDGE(bridge6);
 #undef ADD_BRIDGES
 
   sweep_bridge_list();
@@ -585,6 +591,92 @@ test_bridges_get_transport_by_bridge_addrport(void *arg)
   sweep_bridge_list();
 }
 
+static void
+test_bridges_node_is_a_configured_bridge(void *arg)
+{
+  routerinfo_t ri_ipv4 = { .addr = 0x06060606, .or_port = 6666 };
+  routerstatus_t rs_ipv4 = { .addr = 0x06060606, .or_port = 6666 };
+
+  routerinfo_t ri_ipv6 = { .ipv6_orport = 6666 };
+  tor_addr_parse(&(ri_ipv6.ipv6_addr),
+                 "2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+
+  routerstatus_t rs_ipv6 = { .ipv6_orport = 6666 };
+  tor_addr_parse(&(rs_ipv6.ipv6_addr),
+                 "2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+
+  microdesc_t md_ipv6 = { .ipv6_orport = 6666 };
+  tor_addr_parse(&(md_ipv6.ipv6_addr),
+                 "2001:0db8:85a3:0000:0000:8a2e:0370:7334");
+
+  helper_add_bridges_to_bridgelist(arg);
+
+  node_t node_with_digest;
+  memset(&node_with_digest, 0, sizeof(node_with_digest));
+
+  const char fingerprint[HEX_DIGEST_LEN] =
+    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+
+  const char fingerprint2[HEX_DIGEST_LEN] =
+    "ffffffffffffffffffffffffffffffffffffffff";
+
+  base16_decode(node_with_digest.identity, DIGEST_LEN,
+                fingerprint, HEX_DIGEST_LEN);
+
+  node_t node_ri_ipv4 = { .ri = &ri_ipv4 };
+  base16_decode(node_ri_ipv4.identity, DIGEST_LEN,
+                fingerprint2, HEX_DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_ri_ipv4));
+
+  /* This will still match bridge0, since bridge0 has no digest set. */
+  memset(node_ri_ipv4.identity, 0x3f, DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_ri_ipv4));
+
+  /* It won't match bridge1, though, since bridge1 has a digest, and this
+     isn't it! */
+  node_ri_ipv4.ri->addr = 0x06060607;
+  node_ri_ipv4.ri->or_port = 6667;
+  tt_assert(! node_is_a_configured_bridge(&node_ri_ipv4));
+  /* If we set the fingerprint right, though, it will match. */
+  base16_decode(node_ri_ipv4.identity, DIGEST_LEN,
+                "A10C4F666D27364036B562823E5830BC448E046A", HEX_DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_ri_ipv4));
+
+  node_t node_rs_ipv4 = { .rs = &rs_ipv4 };
+  base16_decode(node_rs_ipv4.identity, DIGEST_LEN,
+                fingerprint2, HEX_DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_rs_ipv4));
+
+  node_t node_ri_ipv6 = { .ri = &ri_ipv6 };
+  base16_decode(node_ri_ipv6.identity, DIGEST_LEN,
+                fingerprint2, HEX_DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_ri_ipv6));
+
+  node_t node_rs_ipv6 = { .rs = &rs_ipv6 };
+  base16_decode(node_rs_ipv6.identity, DIGEST_LEN,
+                fingerprint2, HEX_DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_rs_ipv6));
+
+  node_t node_md_ipv6 = { .md = &md_ipv6 };
+  base16_decode(node_md_ipv6.identity, DIGEST_LEN,
+                fingerprint2, HEX_DIGEST_LEN);
+  tt_assert(node_is_a_configured_bridge(&node_md_ipv6));
+
+  mark_bridge_list();
+  sweep_bridge_list();
+
+  tt_assert(!node_is_a_configured_bridge(&node_with_digest));
+  tt_assert(!node_is_a_configured_bridge(&node_ri_ipv4));
+  tt_assert(!node_is_a_configured_bridge(&node_ri_ipv6));
+  tt_assert(!node_is_a_configured_bridge(&node_rs_ipv4));
+  tt_assert(!node_is_a_configured_bridge(&node_rs_ipv6));
+  tt_assert(!node_is_a_configured_bridge(&node_md_ipv6));
+
+ done:
+  mark_bridge_list();
+  sweep_bridge_list();
+}
+
 #undef PT_PRIVATE /* defined(PT_PRIVATE) */
 
 #define B_TEST(name, flags) \
@@ -607,5 +699,6 @@ struct testcase_t bridges_tests[] = {
   B_TEST(get_transport_by_bridge_addrport_no_ptlist, 0),
   B_TEST(get_transport_by_bridge_addrport, 0),
   B_TEST(transport_is_needed, 0),
+  B_TEST(node_is_a_configured_bridge, 0),
   END_OF_TESTCASES
 };