Fix the assert in bug 1776

In the case where old_router == NULL but sdmap has an entry for the
router, we can currently safely infer that the old_router was not a
bridge.  Add an assert to ensure that this remains true, and fix the
logic not to die with the tor_assert(old_router) call.

diff --git a/changes/bug1776_redux b/changes/bug1776_redux
new file mode 100644 (file)
index 0000000..0e2e5cd
--- /dev/null
+++ b/changes/bug1776_redux
@@ -0,0 +1,5 @@
+  o Major bugfixes
+    - Avoid an assertion failure when we as an authority receive a
+      duplicate upload of a router descriptor that we already have,
+      but which we previously considered an obsolete descriptor.
+      Fixes another case of bug 1776.  Bugfix on 0.2.2.16-alpha.

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 8ed9a7f5e6d287a88116351cf8411efb9db3046f..94f57cc39f377c442efc82e9352c4437a29031ca 100644 (file)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2661,12 +2661,15 @@ signed_descriptor_free(signed_descriptor_t *sd)
   tor_free(sd);
 }
 
-/** Extract a signed_descriptor_t from a routerinfo, and free the routerinfo.
+/** Extract a signed_descriptor_t from a general routerinfo, and free the
+ * routerinfo.
  */
 static signed_descriptor_t *
 signed_descriptor_from_routerinfo(routerinfo_t *ri)
 {
-  signed_descriptor_t *sd = tor_malloc_zero(sizeof(signed_descriptor_t));
+  signed_descriptor_t *sd;
+  tor_assert(ri->purpose == ROUTER_PURPOSE_GENERAL);
+  sd = tor_malloc_zero(sizeof(signed_descriptor_t));
   memcpy(sd, &(ri->cache_info), sizeof(signed_descriptor_t));
   sd->routerlist_index = -1;
   ri->cache_info.signed_descriptor_body = NULL;
@@ -3209,10 +3212,14 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
     /* If we have this descriptor already and the new descriptor is a bridge
      * descriptor, replace it. If we had a bridge descriptor before and the
      * new one is not a bridge descriptor, don't replace it. */
-    tor_assert(old_router);
+
+    /* Only members of routerlist->identity_map can be bridges; we don't
+     * put bridges in old_routers. */
+    const int was_bridge = old_router &&
+      old_router->purpose == ROUTER_PURPOSE_BRIDGE;
+
     if (! (routerinfo_is_a_configured_bridge(router) &&
-            (router->purpose == ROUTER_PURPOSE_BRIDGE ||
-             old_router->purpose != ROUTER_PURPOSE_BRIDGE))) {
+           (router->purpose == ROUTER_PURPOSE_BRIDGE || !was_bridge))) {
       log_info(LD_DIR,
                "Dropping descriptor that we already have for router '%s'",
                router->nickname);